Message-ID: <52FBEA38.1050902@gmail.com>
Date:	Wed, 12 Feb 2014 22:40:08 +0100
From:	Boris BREZILLON <b.brezillon.dev@...il.com>
To:	Florian Fainelli <f.fainelli@...il.com>
CC:	David Woodhouse <dwmw2@...radead.org>,
	Brian Norris <computersforpeace@...il.com>,
	"linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Ezequiel Garcia <ezequiel.garcia@...e-electrons.com>,
	Gupta Pekon <pekon@...com>
Subject: Re: [RFC PATCH v2 0/4] mtd: nand: add per partition ECC config

On 12/02/2014 22:20, Boris BREZILLON wrote:
> Hi Florian,
>
> On 12/02/2014 20:49, Florian Fainelli wrote:
>> Hi Boris,
>>
>> 2014-02-11 13:46 GMT-08:00 Boris BREZILLON <b.brezillon.dev@...il.com>:
>>> Hello,
>>>
>>> This patch series is a proposal to add per partition ECC config.
>>>
>>> It defines a new partition type called nand_part which stores a 
>>> pointer to
>>> a nand_ecc_ctrl struct.
>>> This specific nand_ecc_ctrl struct is used in place of the base NAND 
>>> chip
>>> nand_ecc_ctrl struct when accessing NAND chip from nand_part MTD 
>>> device.
>>>
>>> If the partition does not define any ECC config, the NAND chip ECC 
>>> config
>>> is used instead.
>> Although the idea does look nice on the paper, I wonder if it is
>> really useful to have that much complexity in the kernel. Most likely
>> what is happening is that your bootloader partition has a specific ECC
>> scheme due to the CPU bootrom or whatever early bootagent is running
>> on the system. When that is the case, this can be solved in user-space
>> by preparing full pages (main+spare) along with their specific ECC
>> requirements and use the MTD_FILE_MODE_RAW option on your specific MTD
>> partition.
>
> Okay, that should be possible, althought in the sunxi specific case I 
> haven't
> figured out how to generate these ECC data yet (even if it's using BCH 
> algorithms).
>
>> You do not describe what is the end goal of this patchset, which might
>> help figuring out the potential other platforms requirements etc...
>

BTW, here is another case where per partition ECC would have been useful 
:-) :
http://comments.gmane.org/gmane.linux.ports.arm.omap/108083

> My goal is exactly the one you described above, I just thought this 
> would be
> much easier to handle this within the kernel, without having to 
> develop extra
> user space tools.
> Moreover the user would be able to directly manipulate the data on the 
> MTD
> partitions.
>
> This implementation is not that complex and should not impact 
> performances
> (it's just using cur_ecc pointer instead of intern ecc struct).
>
> Most of the extra code introduced by this series come from the new 
> partition type
> handling, which is quite straightforward BTW.
>
> Anyway, I understand your concern. Could MTD maintainers (and NAND driver
> developpers) give there opinion too ?
>
> BTW, I'm also working on a data randomization layer (needed by some 
> MLC/TLC chips)
> within the NAND core framework and I'm using the same per partition 
> approach
> (for the exact same reason: boot0 partition uses a randomizer seed 
> that might not fit
> the NAND chip requirements).
>
> Should I reconsider doing this too ?
>
> Thanks for you comments.
>
> Best Regards,
>
> Boris
>>
>>> This patch series also provides an helper function to parse DT 
>>> defined NAND
>>> partitions (ofnandpart_parse).
>>>
>>> If you want to test it you'll have to replace calls to
>>> mtd_device_parse_register with ofnandpart_parse within your NAND 
>>> controller
>>> driver and implement a driver specific parser function that will 
>>> provide
>>> the ECC config (see ofnandpart_data struct).
>>>
>>> The 4th patch of this series is here as an example on how to move 
>>> from MTD
>>> partitions to NAND partitions.
>>>
>>> Best Regards,
>>>
>>> Boris
>>>
>>> Changes since v1:
>>> - almost everything :-)
>>>
>>> Boris BREZILLON (4):
>>>    mtd: nand: take nand_ecc_ctrl initialization out of nand_scan_tail
>>>    mtd: nand: add support for NAND partitions
>>>    mtd: nand: add DT NAND partition parser
>>>    mtd: nand: add NAND partition support to the sunxi driver
>>>
>>>   drivers/mtd/nand/Kconfig      |    4 +
>>>   drivers/mtd/nand/Makefile     |    2 +
>>>   drivers/mtd/nand/nand_base.c  |  763 
>>> ++++++++++++++++++++++++++++++++---------
>>>   drivers/mtd/nand/ofnandpart.c |  104 ++++++
>>>   drivers/mtd/nand/sunxi_nand.c |   69 +++-
>>>   include/linux/mtd/nand.h      |   54 +++
>>>   6 files changed, 835 insertions(+), 161 deletions(-)
>>>   create mode 100644 drivers/mtd/nand/ofnandpart.c
>>>
>>> -- 
>>> 1.7.9.5
>>>
>>>
>>> ______________________________________________________
>>> Linux MTD discussion mailing list
>>> http://lists.infradead.org/mailman/listinfo/linux-mtd/
>>
>>
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists