Hi Andrea, Le Tue, 22 Aug 2017 11:42:52 +0200, Andrea Adami <[email protected]> a écrit :
> 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 0x00040000 to > 0x0007bfff (176k) and contains two copies of the partition table: > ... > 0x00060000: 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 <[email protected]> > --- Please put a changelog here. > drivers/mtd/parsers/Kconfig | 7 + > drivers/mtd/parsers/Makefile | 1 + > drivers/mtd/parsers/sharpslpart.c | 382 > ++++++++++++++++++++++++++++++++++++++ > 3 files changed, 390 insertions(+) > create mode 100644 drivers/mtd/parsers/sharpslpart.c > > diff --git a/drivers/mtd/parsers/Kconfig b/drivers/mtd/parsers/Kconfig > index d206b3c..f371022 100644 > --- a/drivers/mtd/parsers/Kconfig > +++ b/drivers/mtd/parsers/Kconfig > @@ -6,3 +6,10 @@ config MTD_PARSER_TRX > may contain up to 3/4 partitions (depending on the version). > This driver will parse TRX header and report at least two partitions: > kernel and rootfs. Missing blank line. > +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. > diff --git a/drivers/mtd/parsers/Makefile b/drivers/mtd/parsers/Makefile > index 4d9024e..5b1bcc3 100644 > --- a/drivers/mtd/parsers/Makefile > +++ b/drivers/mtd/parsers/Makefile > @@ -1 +1,2 @@ > obj-$(CONFIG_MTD_PARSER_TRX) += parser_trx.o > +obj-$(CONFIG_MTD_SHARPSL_PARTS) += sharpslpart.o > diff --git a/drivers/mtd/parsers/sharpslpart.c > b/drivers/mtd/parsers/sharpslpart.c > new file mode 100644 > index 0000000..b2333ae > --- /dev/null > +++ b/drivers/mtd/parsers/sharpslpart.c > @@ -0,0 +1,382 @@ > +/* > + * 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 <[email protected]> > + * > + * 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 implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + */ > + > +#include <linux/kernel.h> > +#include <linux/slab.h> > +#include <linux/module.h> > +#include <linux/types.h> > +#include <linux/mtd/mtd.h> > +#include <linux/mtd/partitions.h> > + > +/* oob structure */ > +#define NAND_NOOB_LOGADDR_00 8 > +#define NAND_NOOB_LOGADDR_01 9 > +#define NAND_NOOB_LOGADDR_10 10 > +#define NAND_NOOB_LOGADDR_11 11 > +#define NAND_NOOB_LOGADDR_20 12 > +#define NAND_NOOB_LOGADDR_21 13 > + > +#define BLOCK_IS_RESERVED 0xffff > +#define BLOCK_UNMASK 0x07fe > +#define BLOCK_UNMASK_COMPLEMENT 1 > + > +/* factory defaults */ > +#define SHARPSL_NAND_PARTS 3 > +#define SHARPSL_FTL_PART_SIZE (7 * 1024 * 1024) Nit: s/7 * 1024 * 1024/7 * SZ_1M/ > +#define PARAM_BLOCK_PARTITIONINFO1 0x00060000 > +#define PARAM_BLOCK_PARTITIONINFO2 0x00064000 I'd rename the macro differently. The term block is a bit confusing here since AFAIU the Param Area does not necessarily fit in a single eraseblock. How about: #define PARAM_AREA_PARTINFO1_LADDR 0x60000 #define PARAM_AREA_PARTINFO2_LADDR 0x64000 or simply #define SHARPSL_PARTINFO1_LADDR 0x60000 #define SHARPSL_PARTINFO2_LADDR 0x64000 > + > +#define BOOT_MAGIC 0x424f4f54 > +#define FSRO_MAGIC 0x4653524f > +#define FSRW_MAGIC 0x46535257 > + > +/** > + * struct sharpsl_ftl - Sharp FTL Logical Table > + * @logmax: number of logical blocks > + * @log2phy: the logical-to-physical table > + * > + * Stripped down from 2.4 sources for read-only purposes. Not sure this information is really useful, especially since you don't provide a link to the 2.4 sources (or maybe I missed it). You'd better describe what this struct is used for here. > + */ > +struct sharpsl_ftl { > + u_int logmax; > + u_int *log2phy; Can you replace u_int by unsigned int and u_char by u8? That goes for the whole file. > +}; > + > +static int sharpsl_nand_read_oob(struct mtd_info *mtd, loff_t offs, size_t > len, AFAICT, len is always set to mtd->oobsize. You can just drop this parameter and set ops->ooblen to mtd->oobsize. > + uint8_t *buf) > +{ > + loff_t mask = mtd->writesize - 1; mask is equivalent to mtd->writesize_mask. > + struct mtd_oob_ops ops; struct mtd_oob_ops ops = { }; so that you don't have to initialize all fields if they are set to 0 or NULL. You could also directly fill this object when declaring it: struct mtd_oob_ops ops = { .mode = MTD_OPS_PLACE_OOB, .ooblen = mtd->oobsize, .oobbuf = buf, }; > + int ret; > + > + ops.mode = MTD_OPS_PLACE_OOB; > + ops.ooboffs = offs & mask; Hm, I'm pretty sure this is wrong. mask is a page mask, not the oob area mask. It works because offs is always aligned on a page, but you can directly assign ops.ooboffs to 0 and you should be good. > + ops.ooblen = len; > + ops.oobbuf = buf; > + ops.datbuf = NULL; > + > + ret = mtd_read_oob(mtd, offs & ~mask, &ops); > + if (ret != 0 || len != ops.oobretlen) > + return -1; > + > + return 0; > +} > + > +/* > + * The logical block number assigned to a physical block is stored in the OOB > + * of the first page, in 3 16-bit copies with the following layout: > + * > + * 01234567 89abcdef > + * -------- -------- > + * ECC BB xyxyxy > + * > + * When reading we check that the first two copies agree. > + * In case of error, matching is tried using the following pairs. > + * Reserved values 0xffff mean the block is kept for wear leveling. > + * > + * 01234567 89abcdef > + * -------- -------- > + * ECC BB xyxy oob[8]==oob[10] && oob[9]==oob[11] -> byte0=8 byte1=9 > + * ECC BB xyxy oob[10]==oob[12] && oob[11]==oob[13] -> byte0=10 > byte1=11 > + * ECC BB xy xy oob[12]==oob[8] && oob[13]==oob[9] -> byte0=12 > byte1=13 > + * > + */ > +static u_int sharpsl_nand_get_logical_num(u_char *oob) > +{ > + u16 us; > + int good0, good1; > + > + if (oob[NAND_NOOB_LOGADDR_00] == oob[NAND_NOOB_LOGADDR_10] && > + oob[NAND_NOOB_LOGADDR_01] == oob[NAND_NOOB_LOGADDR_11]) { > + good0 = NAND_NOOB_LOGADDR_00; > + good1 = NAND_NOOB_LOGADDR_01; > + } else if (oob[NAND_NOOB_LOGADDR_10] == oob[NAND_NOOB_LOGADDR_20] && > + oob[NAND_NOOB_LOGADDR_11] == oob[NAND_NOOB_LOGADDR_21]) { > + good0 = NAND_NOOB_LOGADDR_10; > + good1 = NAND_NOOB_LOGADDR_11; > + } else if (oob[NAND_NOOB_LOGADDR_20] == oob[NAND_NOOB_LOGADDR_00] && > + oob[NAND_NOOB_LOGADDR_21] == oob[NAND_NOOB_LOGADDR_01]) { > + good0 = NAND_NOOB_LOGADDR_20; > + good1 = NAND_NOOB_LOGADDR_21; > + } else { > + /* wrong oob fingerprint, maybe here by mistake? */ > + return UINT_MAX; > + } > + > + us = oob[good0] | oob[good1] << 8; > + > + /* parity check */ > + if (hweight16(us) & BLOCK_UNMASK_COMPLEMENT) > + return (UINT_MAX - 1); Why not making this function return an int and use regular error codes for the parity and wrong fingerprint errors? > + > + /* reserved */ > + if (us == BLOCK_IS_RESERVED) > + return BLOCK_IS_RESERVED; > + else You can drop the 'else' here. > + return (us & BLOCK_UNMASK) >> 1; So, this is a 10bit value, right? Why not doing the >> 1 first so that your BLOCK_UNMASK mask really looks like a valid mask (0x3ff, which can also be defined as GENMASK(9, 0)). > +} > + > +static int sharpsl_nand_init_logical(struct mtd_info *mtd, u32 > partition_size, s/sharpsl_nand_init_logical/sharpsl_nand_init_ftl/ partition_size is always set to SHARPSL_FTL_PART_SIZE. Please drop this argument. > + struct sharpsl_ftl *ftl) > +{ > + u_int block_num, log_num, phymax; > + loff_t block_adr; > + u_char *oob; > + int i, ret; > + > + oob = kzalloc(mtd->oobsize, GFP_KERNEL); > + if (!oob) > + return -ENOMEM; > + > + /* initialize management structure */ > + phymax = (partition_size / mtd->erasesize); You can use mtd_div_by_eb() here. > + > + /* FTL reserves 5% of the blocks + 1 spare */ > + ftl->logmax = ((phymax * 95) / 100) - 1; > + > + ftl->log2phy = kmalloc_array(ftl->logmax, sizeof(u_int), sizeof(*ftl->log2phy) > + GFP_KERNEL); > + if (!ftl->log2phy) { > + ret = -ENOMEM; > + goto exit; > + } > + > + /* initialize ftl->log2phy */ > + for (i = 0; i < ftl->logmax; i++) > + ftl->log2phy[i] = UINT_MAX; > + > + /* create physical-logical table */ > + for (block_num = 0; block_num < phymax; block_num++) { > + block_adr = block_num * mtd->erasesize; > + > + if (mtd_block_isbad(mtd, block_adr)) > + continue; > + > + if (sharpsl_nand_read_oob(mtd, block_adr, mtd->oobsize, oob)) > + continue; > + > + /* get logical block */ > + log_num = sharpsl_nand_get_logical_num(oob); > + > + /* FTL is not used? Exit here if the oob fingerprint is wrong */ > + if (log_num == UINT_MAX) { > + pr_info("sharpslpart: Sharp SL FTL not found.\n"); > + ret = -EINVAL; > + goto exit; > + } > + > + /* skip out of range and not unique values */ > + if (log_num < ftl->logmax) { > + if (ftl->log2phy[log_num] == UINT_MAX) > + ftl->log2phy[log_num] = block_num; > + } > + } > + > + pr_info("Sharp SL FTL: %d blocks used (%d logical, %d reserved)\n", > + phymax, ftl->logmax, > + phymax - ftl->logmax); > + > + ret = 0; > +exit: > + kfree(oob); > + return ret; > +} > + > +void sharpsl_nand_cleanup_logical(struct sharpsl_ftl *ftl) s/sharpsl_nand_cleanup_logical/sharpsl_nand_cleanup_ftl/ > +{ > + kfree(ftl->log2phy); > +} > + > +static int sharpsl_nand_read_laddr(struct mtd_info *mtd, > + loff_t from, > + size_t len, > + u_char *buf, > + struct sharpsl_ftl *ftl) > +{ > + u_int log_num, log_new; > + u_int block_num; > + loff_t block_adr; > + loff_t block_ofs; > + size_t retlen; > + int err; > + > + log_num = (u32)from / mtd->erasesize; Please use mtd_div_by_eb(). > + log_new = ((u32)from + len - 1) / mtd->erasesize; Ditto. And I'm not sure log_new is a good name for this variable. Could be last_log_num or something like that. > + > + if (len <= 0 || log_num >= ftl->logmax || log_new > log_num) > + return -EINVAL; > + > + block_num = ftl->log2phy[log_num]; > + block_adr = block_num * mtd->erasesize; > + block_ofs = (u32)from % mtd->erasesize; mtd_mod_by_eb(). > + > + err = mtd_read(mtd, block_adr + block_ofs, len, &retlen, buf); > + /* Ignore corrected ECC errors */ > + if (mtd_is_bitflip(err)) > + err = 0; Please add a blank line here > + if (!err && retlen != len) > + err = -EIO; and here. > + if (err) > + pr_err("sharpslpart: error, read failed at %#llx\n", > + block_adr + block_ofs); > + > + return err; > +} > + > +/* > + * MTD Partition Parser > + * > + * Sample values read from SL-C860 > + * > + * # cat /proc/mtd > + * dev: size erasesize name > + * mtd0: 006d0000 00020000 "Filesystem" > + * mtd1: 00700000 00004000 "smf" > + * mtd2: 03500000 00004000 "root" > + * mtd3: 04400000 00004000 "home" > + * > + * PARTITIONINFO1 > + * 0x00060000: 00 00 00 00 00 00 70 00 42 4f 4f 54 00 00 00 00 > ......p.BOOT.... > + * 0x00060010: 00 00 70 00 00 00 c0 03 46 53 52 4f 00 00 00 00 > ..p.....FSRO.... > + * 0x00060020: 00 00 c0 03 00 00 00 04 46 53 52 57 00 00 00 00 > ........FSRW.... > + * 0x00060030: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff > ................ > + * You can drop this extra empty line. > + */ > +struct sharpsl_nand_partinfo { > + __le32 start; > + __le32 end; > + __be32 magic; > + u32 reserved; > +}; > + > +static int sharpsl_nand_read_partinfo(struct mtd_info *master, > + loff_t from, > + size_t len, > + struct sharpsl_nand_partinfo *buf, > + struct sharpsl_ftl *ftl) > +{ > + int ret; > + > + ret = sharpsl_nand_read_laddr(master, (u32)from, len, (u_char *)buf, The u32 cast is unneeded here, and I'd suggest to change the sharpsl_nand_read_laddr() prototype to take a 'void *' instead of a 'u_char *', so that you can also get rid of this cast. > + ftl); > + if (ret) > + goto exit; Well, you have nothing to cleanup in the exit path, so you can return directly here. > + > + /* check for magics */ > + if (be32_to_cpu(buf[0].magic) != BOOT_MAGIC || > + be32_to_cpu(buf[1].magic) != FSRO_MAGIC || > + be32_to_cpu(buf[2].magic) != FSRW_MAGIC) { > + pr_err("sharpslpart: magic values mismatch\n"); > + ret = -EINVAL; > + goto exit; Ditto. > + } > + > + /* fixup for hardcoded value 64 MiB (for older models) */ > + buf[2].end = cpu_to_le32(master->size); > + > + /* extra sanity check */ > + if (le32_to_cpu(buf[0].end) <= le32_to_cpu(buf[0].start) || > + le32_to_cpu(buf[1].start) < le32_to_cpu(buf[0].end) || > + le32_to_cpu(buf[1].end) <= le32_to_cpu(buf[1].start) || > + le32_to_cpu(buf[2].start) < le32_to_cpu(buf[1].end) || > + le32_to_cpu(buf[2].end) <= le32_to_cpu(buf[2].start)) { > + pr_err("sharpslpart: partition sizes mismatch\n"); > + ret = -EINVAL; > + goto exit; Ditto. > + } > + > + ret = 0; > +exit: > + return ret; > +} > + > +static int sharpsl_parse_mtd_partitions(struct mtd_info *master, > + const struct mtd_partition **pparts, > + struct mtd_part_parser_data *data) > +{ > + struct sharpsl_ftl ftl; > + struct sharpsl_nand_partinfo buf[SHARPSL_NAND_PARTS]; > + struct mtd_partition *sharpsl_nand_parts; > + int err, ret; > + > + /* init logical mgmt (FTL) */ > + err = sharpsl_nand_init_logical(master, SHARPSL_FTL_PART_SIZE, &ftl); > + if (err) > + return err; > + > + /* read and validate first partition table */ > + pr_info("sharpslpart: using first partition table\n"); > + err = sharpsl_nand_read_partinfo(master, > + PARAM_BLOCK_PARTITIONINFO1, > + sizeof(buf), buf, &ftl); > + if (err) { > + /* fallback: read second partition table */ > + pr_warn("sharpslpart: retry using second partition table\n"); > + ret = sharpsl_nand_read_partinfo(master, > + PARAM_BLOCK_PARTITIONINFO2, > + sizeof(buf), buf, &ftl); Why do you need a different variable to store the second sharpsl_nand_read_partinfo() return code? > + if (ret) { > + pr_err("sharpslpart: partition tables are invalid\n"); > + sharpsl_nand_cleanup_logical(&ftl); > + return ret; > + } > + } > + > + /* cleanup logical mgmt (FTL) */ > + sharpsl_nand_cleanup_logical(&ftl); How about: pr_info("sharpslpart: try reading first partition table\n"); err = sharpsl_nand_read_partinfo(master, PARAM_BLOCK_PARTITIONINFO1, sizeof(buf), buf, &ftl); if (err) { /* fallback: read second partition table */ pr_warn("sharpslpart: first partition table is invalid, retry using second partition table\n"); err = sharpsl_nand_read_partinfo(master, PARAM_BLOCK_PARTITIONINFO2, sizeof(buf), buf, &ftl); } /* cleanup logical mgmt (FTL) */ sharpsl_nand_cleanup_logical(&ftl); if (err) { pr_err("sharpslpart: both partition tables are invalid\n"); return ret; } so that you have a single place where you cleanup the FTL. > + > + sharpsl_nand_parts = kzalloc(sizeof(*sharpsl_nand_parts) * > + SHARPSL_NAND_PARTS, GFP_KERNEL); > + if (!sharpsl_nand_parts) > + return -ENOMEM; > + > + /* original names */ > + sharpsl_nand_parts[0].name = "smf"; > + sharpsl_nand_parts[0].offset = le32_to_cpu(buf[0].start); > + sharpsl_nand_parts[0].size = le32_to_cpu(buf[0].end) - > + le32_to_cpu(buf[0].start); > + > + sharpsl_nand_parts[1].name = "root"; > + sharpsl_nand_parts[1].offset = le32_to_cpu(buf[1].start); > + sharpsl_nand_parts[1].size = le32_to_cpu(buf[1].end) - > + le32_to_cpu(buf[1].start); > + > + sharpsl_nand_parts[2].name = "home"; > + sharpsl_nand_parts[2].offset = le32_to_cpu(buf[2].start); > + sharpsl_nand_parts[2].size = le32_to_cpu(buf[2].end) - > + le32_to_cpu(buf[2].start); > + > + *pparts = sharpsl_nand_parts; > + return SHARPSL_NAND_PARTS; > +} > + > +static struct mtd_part_parser sharpsl_mtd_parser = { > + .parse_fn = sharpsl_parse_mtd_partitions, > + .name = "sharpslpart", > +}; > +module_mtd_part_parser(sharpsl_mtd_parser); > + > +MODULE_LICENSE("GPL"); > +MODULE_AUTHOR("Andrea Adami <[email protected]>"); > +MODULE_DESCRIPTION("MTD partitioning for NAND flash on Sharp SL Series");
