Re: [PATCH v3] mtd: atmel_nand: make PMECC lookup table and offset property optional

[Josh Wu <josh.wu@atmel.com>]

Hi, Mark

On 10/13/2014 7:31 PM, Josh Wu wrote:
> Hi, Mark
>
> On 10/13/2014 7:16 PM, Mark Rutland wrote:
>> On Sat, Oct 11, 2014 at 11:01:50AM +0100, Josh Wu wrote:
>>> From: Josh Wu <Josh.wu@atmel.com>
>>>
>>> If there is no PMECC lookup table stored in ROM, or lookup table 
>>> offset is
>>> not specified, PMECC driver should build it in DDR by itself.
>>>
>>> That make the PMECC driver work for some board which doesn't has PMECC
>>> lookup table in ROM.
>>>
>>> The PMECC use the BCH algorithm, so based on the build_gf_tables()
>>> function in lib/bch.c, we can build the Galois Field lookup table.
>>>
>>> For more information can refer to section 5.4 of PMECC controller
>>> application note:
>>>     http://www.atmel.com/images/doc11127.pdf
>>>
>>> Signed-off-by: Josh Wu <josh.wu@atmel.com>
>>> Cc: devicetree@vger.kernel.org
>>> ---
>>> v1 -> v2:
>>>    make create_lookup_table() static.
>>>
>>> v2 -> v3:
>>>    rewrite the build_gf_tables() function based on lib/bch.c.
>>>    add error handling in create_lookup_table().
>>>
>>>   .../devicetree/bindings/mtd/atmel-nand.txt         |  6 +-
>>>   drivers/mtd/nand/atmel_nand.c                      | 81 
>>> ++++++++++++++++++++--
>>>   drivers/mtd/nand/atmel_nand_ecc.h                  |  4 ++
>>>   3 files changed, 85 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mtd/atmel-nand.txt 
>>> b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
>>> index 6edc3b6..1fe6dde 100644
>>> --- a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
>>> +++ b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
>>> @@ -5,7 +5,9 @@ Required properties:
>>>   - reg : should specify localbus address and size used for the chip,
>>>       and hardware ECC controller if available.
>>>       If the hardware ECC is PMECC, it should contain address and 
>>> size for
>>> -    PMECC, PMECC Error Location controller and ROM which has lookup 
>>> tables.
>>> +    PMECC and PMECC Error Location controller.
>>> +    The PMECC lookup table address and size in ROM is optional. If not
>>> +    specified, driver will build it in runtime.
>>>   - atmel,nand-addr-offset : offset for the address latch.
>>>   - atmel,nand-cmd-offset : offset for the command latch.
>>>   - #address-cells, #size-cells : Must be present if the device has 
>>> sub-nodes
>>> @@ -27,7 +29,7 @@ Optional properties:
>>>     are: 512, 1024.
>>>   - atmel,pmecc-lookup-table-offset : includes two offsets of lookup 
>>> table in ROM
>>>     for different sector size. First one is for sector size 512, the 
>>> next is for
>>> -  sector size 1024.
>>> +  sector size 1024. If not specified, driver will build the table 
>>> in runtime.
>> I'm not sure we need to mention this in the binding.
>>
>> That said, if we can build this dynamically, can't we always do so, and
>> never need this property?
>
> Since board like at91sam9x5, sama5d3xek has a rom lookup table. And 
> the sama5d4ek has no rom lookup table.
> To present this, I make this property as optional.
>
> But yes, the pmecc lookup table related properties can be removed as 
> driver can build it in runtime.
> The cost is we need to use more memory to store the table.

In precisely, the table need to 32k bytes memory for 512-sector, and 64k 
bytes for 1024-sector.

Also I spend time to testing the performance of this version.
Compare with the version which use the SRAM rom lookup table, this 
version (build table in runtim) cost about 5~10ms more.

So I prefer to keep this property as optional.

Best Regards,
Josh Wu

>
>
>>
>> Mark.
> Best Regards,
> Josh Wu
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/