Re: [RFC PATCH v2 0/4] mtd: nand: add per partition ECC config

[Boris BREZILLON]

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@xxxxxxxxx>:
> > > 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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/