[PATCH 1/3] of: mtd: add helper reading "nand-ecc-algo" from DT

[Rafał Miłecki]

On 24 February 2016 at 14:46, Boris Brezillon
<boris.brezillon at free-electrons.com> wrote:
> On Fri, 12 Feb 2016 19:11:23 +0100
> Rafał Miłecki <zajec5 at gmail.com> wrote:
>
>> This allows specifying algorithm used for NAND ECC. There are two
>> available values: "bch" and "hamming". It's important as having just
>> ECC parameters (step, strength) isn't enough to determine algorithm,
>> e.g. you can't distinct BCH-1 and Hamming.
>>
>> Signed-off-by: Rafał Miłecki <zajec5 at gmail.com>
>> ---
>>  Documentation/devicetree/bindings/mtd/nand.txt |  3 +++
>>  drivers/of/of_mtd.c                            | 33 ++++++++++++++++++++++++++
>>  include/linux/mtd/nand.h                       |  5 ++++
>>  include/linux/of_mtd.h                         |  6 +++++
>>  4 files changed, 47 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
>> index b53f92e..a2c2df5 100644
>> --- a/Documentation/devicetree/bindings/mtd/nand.txt
>> +++ b/Documentation/devicetree/bindings/mtd/nand.txt
>> @@ -3,6 +3,9 @@
>>  - nand-ecc-mode : String, operation mode of the NAND ecc mode.
>>    Supported values are: "none", "soft", "hw", "hw_syndrome", "hw_oob_first",
>>    "soft_bch".
>> +- nand-ecc-algo : string, algorithm of NAND ecc.
>> +               Supported values are: "bch", "hamming". The default one is
>> +               "bch".
>
> I like the idea of specifying which ECC algorithm should be used, and
> this is IMO clearer than putting the information directly in
> nand-ecc-mode (as is already done for the "soft" and "soft_bch" modes,
> which are respectively encoding software hamming and software bch
> implementations).
>
> But shouldn't we take this even further and add new DT properties
> to encode the ECC layout (syndrome, oob_first), and another one to
> specify the implementation type (software or hardware)?
>
> nand-ecc-layout = "nornal", "syndrome" or "oob_first"
> nand-ecc-engine = "none", "soft" or "hw" ("on-die" ?)
>
> Note that it's not something I ask you to do right now, I just want to
> bring the topic on the table.

So far I was fine with whatever drvier/NAND core picked, didn't have
any issue with it. If at some point we'll see a real need of
specifying it manually as well, I guess we should do as you suggested.

-- 
Rafał