Re: [PATCH v4 1/9] mtd: sharpslpart: add sharpslpart MTD partition parser
On Sat, Aug 12, 2017 at 11:35 PM, Brian Norris wrote: > Hi Andrea, > > I'm sorry this had to wait so long, but then...you didn't actually > address several of my comments from the last time :( > > This does look better, but still quite a few smaller notes. Hopefully > next one will be merge-able... > Brian, thank you again for the time spent reviewing this patch and for the guidance. I did not dare to send all the changes together in the v4 so I kept the management of the structure as last step (remember this code is coming from 2.4 sources and it was supposed to keep the logical to physical table in memory). So I will send v5 very soon without the global structure and with these last refinements you're suggesting. I was really awaiting your comments about the many changes done and my implementation (see macros...). I'll comment briefly but substantially I agree with your observations. > On Wed, Jun 28, 2017 at 10:30:28PM +0200, Andrea Adami wrote: >> The Sharp SL Series (Zaurus) PXA handhelds have 16/64/128M of NAND flash >> and share the same layout of the first 7M partition, managed by Sharp FTL. >> >> The purpose of this self-contained patch is to add a common parser and >> remove the hardcoded sizes in the board files (these devices are not yet >> converted to devicetree). >> Users will have benefits because the mtdparts= tag will not be necessary >> anymore and they will be free to repartition the little sized flash. >> >> The obsolete bootloader can not pass the partitioning info to modern >> kernels anymore so it has to be read from flash at known logical addresses. >> (see http://www.h5.dion.ne.jp/~rimemoon/zaurus/memo_006.htm ) >> >> In kernel, under arch/arm/mach-pxa we have already 8 machines: >> MACH_POODLE, MACH_CORGI, MACH_SHEPERD, MACH_HUSKY, MACH_AKITA, MACH_SPITZ, >> MACH_BORZOI, MACH_TOSA. >> Lost after the 2.4 vendor kernel are MACH_BOXER and MACH_TERRIER. >> >> Almost every model has different factory partitioning: add to this the >> units can be repartitioned by users with userspace tools (nandlogical) >> and installers for popular (back then) linux distributions. >> >> The Parameter Area in the first (boot) partition extends from 0x0004 to >> 0x0007bfff (176k) and contains two copies of the partition table: >> ... >> 0x0006: Partition Info1 16k >> 0x00064000: Partition Info2 16k >> 0x00668000: Model 16k >> ... >> >> The first 7M partition is managed by the Sharp FTL reserving 5% + 1 blocks >> for wear-leveling: some blocks are remapped and one layer of translation >> (logical to physical) is necessary. >> >> There isn't much documentation about this FTL in the 2.4 sources, just the >> MTD methods for reading and writing using logical addresses and the block >> management (wear-leveling, use counter). >> For the purpose of the MTD parser only the read part of the code was taken. >> >> The NAND drivers that can use this parser are sharpsl.c and tmio_nand.c. >> >> Signed-off-by: Andrea Adami >> --- >> drivers/mtd/Kconfig | 8 + >> drivers/mtd/Makefile | 1 + >> drivers/mtd/sharpslpart.c | 391 >> ++ >> 3 files changed, 400 insertions(+) >> create mode 100644 drivers/mtd/sharpslpart.c >> >> diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig >> index e83a279..b196a69 100644 >> --- a/drivers/mtd/Kconfig >> +++ b/drivers/mtd/Kconfig >> @@ -155,6 +155,14 @@ config MTD_BCM47XX_PARTS >> This provides partitions parser for devices based on BCM47xx >> boards. >> >> +config MTD_SHARPSL_PARTS >> + tristate "Sharp SL Series NAND flash partition parser" >> + depends on MTD_NAND_SHARPSL || MTD_NAND_TMIO || COMPILE_TEST >> + help >> + This provides the read-only FTL logic necessary to read the partition >> + table from the NAND flash of Sharp SL Series (Zaurus) and the MTD >> + partition parser using this code. >> + > > This has conflicts now; probably should go in > drivers/mtd/parsers/Kconfig now. We should also probably move the other > parsers there, but that's not your problem. > Ok, I'll move the parser there. >> comment "User Modules And Translation Layers" >> >> # >> diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile >> index 99bb9a1..e5ef07f 100644 >> --- a/drivers/mtd/Makefile >> +++ b/drivers/mtd/Makefile >> @@ -13,6 +13,7 @@ obj-$(CONFIG_MTD_AFS_PARTS) += afs.o >> obj-$(CONFIG_MTD_AR7_PARTS) += ar7part.o >> obj-$(CONFIG_MTD_BCM63XX_PARTS) += bcm63xxpart.o >> obj-$(CONFIG_MTD_BCM47XX_PARTS) += bcm47xxpart.o >> +obj-$(CONFIG_MTD_SHARPSL_PARTS) += sharpslpart.o > > Also has conflicts now. Might as well move to drivers/mtd/parsers/? Sure > >> >> # 'Users' - code which presents functionality to userspace. >> obj-$(CONFIG_MTD_BLKDEVS)+= mtd_blkdevs.o >> diff --git a/drivers/mtd/sharpslpart.c b/drivers/mtd/sharpslpart.c >> new file mode 100644 >> index 000..02b721b >> --- /dev/null >> +++ b/drivers/mtd/sharps
Re: [PATCH v4 1/9] mtd: sharpslpart: add sharpslpart MTD partition parser
Hi Andrea, I'm sorry this had to wait so long, but then...you didn't actually address several of my comments from the last time :( This does look better, but still quite a few smaller notes. Hopefully next one will be merge-able... On Wed, Jun 28, 2017 at 10:30:28PM +0200, Andrea Adami wrote: > The Sharp SL Series (Zaurus) PXA handhelds have 16/64/128M of NAND flash > and share the same layout of the first 7M partition, managed by Sharp FTL. > > The purpose of this self-contained patch is to add a common parser and > remove the hardcoded sizes in the board files (these devices are not yet > converted to devicetree). > Users will have benefits because the mtdparts= tag will not be necessary > anymore and they will be free to repartition the little sized flash. > > The obsolete bootloader can not pass the partitioning info to modern > kernels anymore so it has to be read from flash at known logical addresses. > (see http://www.h5.dion.ne.jp/~rimemoon/zaurus/memo_006.htm ) > > In kernel, under arch/arm/mach-pxa we have already 8 machines: > MACH_POODLE, MACH_CORGI, MACH_SHEPERD, MACH_HUSKY, MACH_AKITA, MACH_SPITZ, > MACH_BORZOI, MACH_TOSA. > Lost after the 2.4 vendor kernel are MACH_BOXER and MACH_TERRIER. > > Almost every model has different factory partitioning: add to this the > units can be repartitioned by users with userspace tools (nandlogical) > and installers for popular (back then) linux distributions. > > The Parameter Area in the first (boot) partition extends from 0x0004 to > 0x0007bfff (176k) and contains two copies of the partition table: > ... > 0x0006: Partition Info1 16k > 0x00064000: Partition Info2 16k > 0x00668000: Model 16k > ... > > The first 7M partition is managed by the Sharp FTL reserving 5% + 1 blocks > for wear-leveling: some blocks are remapped and one layer of translation > (logical to physical) is necessary. > > There isn't much documentation about this FTL in the 2.4 sources, just the > MTD methods for reading and writing using logical addresses and the block > management (wear-leveling, use counter). > For the purpose of the MTD parser only the read part of the code was taken. > > The NAND drivers that can use this parser are sharpsl.c and tmio_nand.c. > > Signed-off-by: Andrea Adami > --- > drivers/mtd/Kconfig | 8 + > drivers/mtd/Makefile | 1 + > drivers/mtd/sharpslpart.c | 391 > ++ > 3 files changed, 400 insertions(+) > create mode 100644 drivers/mtd/sharpslpart.c > > diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig > index e83a279..b196a69 100644 > --- a/drivers/mtd/Kconfig > +++ b/drivers/mtd/Kconfig > @@ -155,6 +155,14 @@ config MTD_BCM47XX_PARTS > This provides partitions parser for devices based on BCM47xx > boards. > > +config MTD_SHARPSL_PARTS > + tristate "Sharp SL Series NAND flash partition parser" > + depends on MTD_NAND_SHARPSL || MTD_NAND_TMIO || COMPILE_TEST > + help > + This provides the read-only FTL logic necessary to read the partition > + table from the NAND flash of Sharp SL Series (Zaurus) and the MTD > + partition parser using this code. > + This has conflicts now; probably should go in drivers/mtd/parsers/Kconfig now. We should also probably move the other parsers there, but that's not your problem. > comment "User Modules And Translation Layers" > > # > diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile > index 99bb9a1..e5ef07f 100644 > --- a/drivers/mtd/Makefile > +++ b/drivers/mtd/Makefile > @@ -13,6 +13,7 @@ obj-$(CONFIG_MTD_AFS_PARTS) += afs.o > obj-$(CONFIG_MTD_AR7_PARTS) += ar7part.o > obj-$(CONFIG_MTD_BCM63XX_PARTS) += bcm63xxpart.o > obj-$(CONFIG_MTD_BCM47XX_PARTS) += bcm47xxpart.o > +obj-$(CONFIG_MTD_SHARPSL_PARTS) += sharpslpart.o Also has conflicts now. Might as well move to drivers/mtd/parsers/? > > # 'Users' - code which presents functionality to userspace. > obj-$(CONFIG_MTD_BLKDEVS)+= mtd_blkdevs.o > diff --git a/drivers/mtd/sharpslpart.c b/drivers/mtd/sharpslpart.c > new file mode 100644 > index 000..02b721b > --- /dev/null > +++ b/drivers/mtd/sharpslpart.c > @@ -0,0 +1,391 @@ > +/* > + * sharpslpart.c - MTD partition parser for NAND flash using the SHARP FTL > + * for logical addressing, as used on the PXA models of the SHARP SL Series. > + * > + * Copyright (C) 2017 Andrea Adami > + * > + * Based on 2.4 sources: > + * drivers/mtd/nand/sharp_sl_logical.c > + * linux/include/asm-arm/sharp_nand_logical.h > + * > + * Copyright (C) 2002 SHARP > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the
