Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752370AbdHXJUC (ORCPT ); Thu, 24 Aug 2017 05:20:02 -0400 Received: from mail-wm0-f45.google.com ([74.125.82.45]:37860 "EHLO mail-wm0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751687AbdHXJT7 (ORCPT ); Thu, 24 Aug 2017 05:19:59 -0400 MIME-Version: 1.0 In-Reply-To: <20170822145424.54e57b94@bbrezillon> References: <1503394972-12541-1-git-send-email-andrea.adami@gmail.com> <20170822145424.54e57b94@bbrezillon> From: Andrea Adami Date: Thu, 24 Aug 2017 11:19:56 +0200 Message-ID: Subject: Re: [PATCH v6] mtd: sharpslpart: Add sharpslpart partition parser To: Boris Brezillon Cc: linux-mtd@lists.infradead.org, David Woodhouse , Brian Norris , Marek Vasut , Richard Weinberger , Cyrille Pitchen , =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= , Haojian Zhuang , Dmitry Eremin-Solenikov , Robert Jarzmik , linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by nfs id v7O9K8wo022593 Content-Length: 23550 Lines: 696 Boris, thanks for the review. I have made the required changes apart th elast one: error values to be returned.. Please see my comments. On Tue, Aug 22, 2017 at 2:54 PM, Boris Brezillon wrote: > Hi Andrea, > > Le Tue, 22 Aug 2017 11:42:52 +0200, > Andrea Adami 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 >> --- > > Please put a changelog here. Sure, I'll take the one from the cover-letter. > >> 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. Fixed > >> +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 >> + * >> + * 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 >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +/* 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/ Fixed > >> +#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 Used these, nicer > >> + >> +#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). Maybe here I can add that this FTL was originally tailored for devices 16KiB erasesize matching the size of the PARAM_BLOCK_ above thus using two separate blocks for the partition tables. Should I add an ASCII-art? Pls see http://www.h5.dion.ne.jp/~rimemoon/zaurus/memo_006.htm BTW the link was in the cover-letter 0/9, I'll put it here together with the changelog. > > You'd better describe what this struct is used for here. Seems self-explanatory..2 fields commented. What would you add 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. Yes, done > >> +}; >> + >> +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. > Ok done, I prefer this syntax, like in the other ftl using read_oob(). > 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. > You are right, I removed the mask alltogether. >> + 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? > Originally in case of error it did return the largest values. I can return a negative int but then I'd have to check for log_no >0 Besides, what are the 'regular' error codes you'd use here? For the missing FTL we do return -EINVAL later so I'd say this is the error for the wrong oob fingerprint. About the parity error, it does return UINT_MAX -1 so to allow very large log_num. This value is always bigger than log_max so it is skipped in the actual code but the read continues. Should we change the philosophy of the old 2.4 code and exit in case of parity errors? >> + >> + /* reserved */ >> + if (us == BLOCK_IS_RESERVED) >> + return BLOCK_IS_RESERVED; >> + else > > You can drop the 'else' here. Done > >> + 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)). Right, very nice. Done. Can I keep BLOCK_UNMASK COMPLEMENT = 1 ? > >> +} >> + >> +static int sharpsl_nand_init_logical(struct mtd_info *mtd, u32 partition_size, > > s/sharpsl_nand_init_logical/sharpsl_nand_init_ftl/ Oh yes, renamed > > partition_size is always set to SHARPSL_FTL_PART_SIZE. Please drop this > argument. > Ok, done >> + 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. > Done for all occurrences >> + >> + /* 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) Ok, changed. > >> + 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/ > Yes, renamed as well >> +{ >> + 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 > Ok >> + if (!err && retlen != len) >> + err = -EIO; > > and here. > Ok (was taken from mtdtest_read() fwiw) >> + 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. Ok > >> + */ >> +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. > Nice, done >> + ftl); >> + if (ret) >> + goto exit; > > Well, you have nothing to cleanup in the exit path, so you can return > directly here. > Ah, I have exaggerated here :) Done all three >> + >> + /* 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? > It is unneeded, is a copy&paste remnant. Fixed. >> + 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. Very nice. I was mumbling around it... Done > >> + >> + 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 "); >> +MODULE_DESCRIPTION("MTD partitioning for NAND flash on Sharp SL Series"); > Thank you very much. Code is getting better and better. Andrea