NAND ECC Layout, sysfs question

PATCH Superseded Brian Norris 4 participants 21 messages Aug 5, 2010 – Sep 20, 2010

Subsystems (11)

Arm (32-bit) Arm sub-architectures Arm i.MX and Layerscape Memory technology devices (MTD) +7 more subsystems

lore

Share & tools

lore

Brian Norris

author

2010-08-05 17:53 UTC #0 lore raw

Hello,

I'm trying to replace the following structure (in mtd-abi.h) and its corresponding ioctl with a larger, dynamic form exported via sysfs. We're running out of room in the eccpos and oobfree arrays on larger chips:

#define MTD_MAX_OOBFREE_ENTRIES 8
/*
* ECC layout control structure. Exported to userspace for
* diagnosis and to allow creation of raw images */

struct nand_ecclayout {
        __u32 eccbytes;
        __u32 eccpos[64];
        __u32 oobavail;
        struct nand_oobfree oobfree[MTD_MAX_OOBFREE_ENTRIES];
};

First, is it a smart idea to use sysfs?

Second, if we go with sysfs, there seem to be many potential ways to structure the "eccpos" and "oobfree" arrays. Bearing in mind the convention of either one entry per file or a space-separated array of values as well as the file limit of PAGE_SIZE (approx. 4096 bytes?), there are several options with which I'm having difficulty. Formats could be as follows:
1) Single file: 0 1 2 3 6 7 10 12
2) Single file: 0-3 6-7 10 12
3) Multiple files:

    file 0: 0 3
    file 1: 6 7
    file 2: 10 10
    file 3: 12 12

It seemed to me like (3) was the cleanest while ensuring we fit under PAGE_SIZE even if the OOB gets to be 1000+ bytes, so no one ever has to come back and redo this :) Unfortunately, directories and a dynamic number of attributes/files aren't really straightforward, hence my next questions...

Third, if I were to create directories under the "mtdX" sysfs directory, what's the best way to do this? Explicitly creating and filling in a kobject? Or creating a new "device"? Or are there other, better options?

I've already coded parts of a few of these options, but they all have various difficulties and can easily become ugly pieces of code (at least with my limited skill at sysfs).

Thanks,
Brian

Artem Bityutskiy

2010-08-05 18:18 UTC #1 lore raw

Quoted 5 lines · Brian Norris

Hello,
I'm trying to replace the following structure (in mtd-abi.h) and its corresponding ioctl with a larger, dynamic form exported via sysfs. We're running out of room in the eccpos and oobfree arrays on larger chips:

I personally hate that internal information like eccpos is exposed to user-space. I think a good an not error-prone SW should be written in a way the ECC is completely hidden and handled by driver. And it is better not to use OOB.

But there may be people who want to use OOB. But then ECC and free space in OOB should be considered as virtual contiguous array, and the driver places it properly. It is not user-spaces' business to know where ECC bytes are placed. There was some OOB mode for this, do not remember its name.

Do you really need to know ECC layout? What for? Would be nice if everything but the above mode of working with OOB / ECC would be considered as legacy and died...

Quoted 34 lines

#define MTD_MAX_OOBFREE_ENTRIES 8
/*
* ECC layout control structure. Exported to userspace for
* diagnosis and to allow creation of raw images */
struct nand_ecclayout {
         __u32 eccbytes;
         __u32 eccpos[64];
         __u32 oobavail;
         struct nand_oobfree oobfree[MTD_MAX_OOBFREE_ENTRIES];
};
First, is it a smart idea to use sysfs?
Second, if we go with sysfs, there seem to be many potential ways to structure the "eccpos" and "oobfree" arrays. Bearing in mind the convention of either one entry per file or a space-separated array of values as well as the file limit of PAGE_SIZE (approx. 4096 bytes?), there are several options with which I'm having difficulty. Formats could be as follows:
1) Single file: 0 1 2 3 6 7 10 12
2) Single file: 0-3 6-7 10 12
3) Multiple files:
     file 0: 0 3
     file 1: 6 7
     file 2: 10 10
     file 3: 12 12
It seemed to me like (3) was the cleanest while ensuring we fit under PAGE_SIZE even if the OOB gets to be 1000+ bytes, so no one ever has to come back and redo this :) Unfortunately, directories and a dynamic number of attributes/files aren't really straightforward, hence my next questions...

Quoted 7 lines

Third, if I were to create directories under the "mtdX" sysfs directory, what's the best way to do this? Explicitly creating and filling in a kobject? Or creating a new "device"? Or are there other, better options?
I've already coded parts of a few of these options, but they all have various difficulties and can easily become ugly pieces of code (at least with my limited skill at sysfs).

If you really have to do this, I think a new ioctl with some extra space for future extentions is better. sysfs is not good choice, imo.

Signature · 2 lines

Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Brian Norris

author

2010-08-07 00:11 UTC #2 lore raw

PATCH 1/1 mtd: Expand nand_ecc_layout, deprecate ioctl ECCGETLAYOUT

Hello,

I have addressed this patch to two mailing lists (and several other "maintainers") because, although this primarily deals with the MTD subsystem, it affects drivers, etc. from the ARM architecture at least.

Refer to the following page if you want the "full" conversation:
http://lists.infradead.org/pipermail/linux-mtd/2010-August/031337.html

Quoted 5 lines · Artem Bityutskiy

Quoted 3 lines · Brian Norris
I'm trying to replace the following structure (in mtd-abi.h) and its corresponding ioctl with a larger, dynamic form exported via sysfs. We're running out of room in the eccpos and oobfree arrays on larger chips:

Quoted 8 lines

I personally hate that internal information like eccpos is exposed to user-space. I think a good an not error-prone SW should be written in a way the ECC is completely hidden and handled by driver. And it is better not to use OOB.
Do you really need to know ECC layout? What for? Would be nice if everything but the above mode of working with OOB / ECC would be considered as legacy and died...

Well, we actually only needed the larger struct for the kernel space, so maybe we can ignore most of the new sysfs/ioctl ideas for now.

Quoted 9 lines

Quoted 6 lines
struct nand_ecclayout {
          __u32 eccbytes;
          __u32 eccpos[64];
          __u32 oobavail;
          struct nand_oobfree oobfree[MTD_MAX_OOBFREE_ENTRIES];
};
If you really have to do this, I think a new ioctl with some extra space for future extentions is better. sysfs is not good choice, imo.

After some work, I agree; sysfs isn't a good choice. Perhaps we don't even need full user-space access to the kernel struct; instead, we can deprecate the ECCGETLAYOUT ioctl, expand the nand_ecclayout as follows, and possibly add different user-space functions to access the ECC/OOB info in a uniform fashion in the future.

Let me know if I have over-/under-commented the code or if there are better ways to accomplish the objectives mentioned. The patch/commit message follows.

Thanks,
Brian

-----------------------------------------------------------------------------

struct nand_ecclayout is too small for many new chips; OOB regions can be as large as 448 bytes and may increase more in the future. Thus, copying that struct to user-space with the ECCGETLAYOUT ioctl is not a good idea; the ioctl would have to be updated every time there's a change to the current largest size.

Instead, the whole NAND system should switch over to using a struct that is entirely private to the kernel (i.e., the larger struct nand_ecclayout_l). A new function is provided to convert from the new to the old in order to allow the deprecated ioctl to continue to work with truncated data. Perhaps the ioctl, the conversion process, and the struct nand_ecclayout can be removed altogether in the future.

Note: There are comments in nand/davinci_nand.c::nand_davinci_probe() regarding this issue; this driver (and maybe others) can be updated to account for extra space. As far as I can tell, though, all kernel drivers *can* use nand_ecclayout_l as a drop-in replacement and ignore its benefits.

Signed-off-by Brian Norris <[email protected]>

---
 arch/arm/mach-pxa/spitz.c                 |    2 +-
 arch/arm/plat-samsung/dev-nand.c          |    2 +-
 arch/arm/plat-samsung/include/plat/nand.h |    2 +-
 drivers/mtd/mtdchar.c                     |   48 +++++++++++++++++++++++++++--
 drivers/mtd/nand/atmel_nand.c             |    4 +-
 drivers/mtd/nand/bcm_umi_bch.c            |    6 ++--
 drivers/mtd/nand/bf5xx_nand.c             |    2 +-
 drivers/mtd/nand/cafe_nand.c              |    4 +-
 drivers/mtd/nand/davinci_nand.c           |   11 ++++--
 drivers/mtd/nand/diskonchip.c             |    2 +-
 drivers/mtd/nand/fsl_elbc_nand.c          |    8 ++--
 drivers/mtd/nand/mxc_nand.c               |   10 +++---
 drivers/mtd/nand/nand_base.c              |    8 ++--
 drivers/mtd/nand/nomadik_nand.c           |    2 +-
 drivers/mtd/nand/pxa3xx_nand.c            |    4 +-
 drivers/mtd/nand/rtc_from4.c              |    2 +-
 drivers/mtd/nand/s3c2410.c                |    2 +-
 drivers/mtd/nand/sh_flctl.c               |    4 +-
 drivers/mtd/onenand/onenand_base.c        |    6 ++--
 fs/jffs2/wbuf.c                           |    2 +-
 include/linux/mtd/inftl.h                 |    2 +-
 include/linux/mtd/mtd.h                   |   17 +++++++++-
 include/linux/mtd/nand.h                  |    6 ++--
 include/linux/mtd/nftl.h                  |    2 +-
 include/linux/mtd/onenand.h               |    2 +-
 include/linux/mtd/partitions.h            |    2 +-
 include/linux/mtd/sharpsl.h               |    2 +-
 27 files changed, 112 insertions(+), 52 deletions(-)

arch/arm/mach-pxa/spitz.c +1 -1

index 4d2413e..6542aaf 100644
@@ -775,7 +775,7 @@ static struct nand_bbt_descr sharpsl_akita_bbt = {
 	.pattern = scan_ff_pattern
 };
 
-static struct nand_ecclayout akita_oobinfo = {
+static struct nand_ecclayout_l akita_oobinfo = {
 	.eccbytes = 24,
 	.eccpos = {
 		   0x5, 0x1, 0x2, 0x3, 0x6, 0x7, 0x15, 0x11,

arch/arm/plat-samsung/dev-nand.c +1 -1

index 3a7b889..0109307 100644
@@ -70,7 +70,7 @@ static int __init s3c_nand_copy_set(struct s3c2410_nand_set *set)
 
 	if (set->ecc_layout) {
 		ptr = kmemdup(set->ecc_layout,
-			      sizeof(struct nand_ecclayout), GFP_KERNEL);
+			      sizeof(*set->ecc_layout), GFP_KERNEL);
 		set->ecc_layout = ptr;
 
 		if (!ptr)

arch/arm/plat-samsung/include/plat/nand.h +1 -1

index b64115f..49dedc9 100644
@@ -38,7 +38,7 @@ struct s3c2410_nand_set {
 	char			*name;
 	int			*nr_map;
 	struct mtd_partition	*partitions;
-	struct nand_ecclayout	*ecc_layout;
+	struct nand_ecclayout_l	*ecc_layout;
 };
 
 struct s3c2410_platform_nand {

drivers/mtd/mtdchar.c +45 -3

65 lines not on this page · raw message

drivers/mtd/nand/atmel_nand.c +2 -2

index ccce0f0..6d856d9 100644
@@ -64,7 +64,7 @@ module_param(on_flash_bbt, int, 0);
  * the bytes have to be consecutives to avoid
  * several NAND_CMD_RNDOUT during read
  */
-static struct nand_ecclayout atmel_oobinfo_large = {
+static struct nand_ecclayout_l atmel_oobinfo_large = {
 	.eccbytes = 4,
 	.eccpos = {60, 61, 62, 63},
 	.oobfree = {

@@ -77,7 +77,7 @@ static struct nand_ecclayout atmel_oobinfo_large = {
  * the bytes have to be consecutives to avoid
  * several NAND_CMD_RNDOUT during read
  */
-static struct nand_ecclayout atmel_oobinfo_small = {
+static struct nand_ecclayout_l atmel_oobinfo_small = {
 	.eccbytes = 4,
 	.eccpos = {0, 1, 2, 3},
 	.oobfree = {

drivers/mtd/nand/bcm_umi_bch.c +3 -3

index a930666..63be36a 100644
@@ -32,7 +32,7 @@ static void bcm_umi_bch_write_page_hwecc(struct mtd_info *mtd,
 ** nand_hw_eccoob
 ** New oob placement block for use with hardware ecc generation.
 */
-static struct nand_ecclayout nand_hw_eccoob_512 = {
+static struct nand_ecclayout_l nand_hw_eccoob_512 = {
 	/* Reserve 5 for BI indicator */
 	.oobfree = {
 #if (NAND_ECC_NUM_BYTES > 3)

@@ -48,7 +48,7 @@ static struct nand_ecclayout nand_hw_eccoob_512 = {
 ** We treat the OOB for a 2K page as if it were 4 512 byte oobs,
 ** except the BI is at byte 0.
 */
-static struct nand_ecclayout nand_hw_eccoob_2048 = {
+static struct nand_ecclayout_l nand_hw_eccoob_2048 = {
 	/* Reserve 0 as BI indicator */
 	.oobfree = {
 #if (NAND_ECC_NUM_BYTES > 10)

@@ -69,7 +69,7 @@ static struct nand_ecclayout nand_hw_eccoob_2048 = {
 
 /* We treat the OOB for a 4K page as if it were 8 512 byte oobs,
  * except the BI is at byte 0. */
-static struct nand_ecclayout nand_hw_eccoob_4096 = {
+static struct nand_ecclayout_l nand_hw_eccoob_4096 = {
 	/* Reserve 0 as BI indicator */
 	.oobfree = {
 #if (NAND_ECC_NUM_BYTES > 10)

drivers/mtd/nand/bf5xx_nand.c +1 -1

index a382e3d..3752541 100644
@@ -119,7 +119,7 @@ static struct nand_bbt_descr bootrom_bbt = {
 	.pattern = bbt_pattern,
 };
 
-static struct nand_ecclayout bootrom_ecclayout = {
+static struct nand_ecclayout_l bootrom_ecclayout = {
 	.eccbytes = 24,
 	.eccpos = {
 		0x8 * 0, 0x8 * 0 + 1, 0x8 * 0 + 2,

drivers/mtd/nand/cafe_nand.c +2 -2

index db1dfc5..d989627 100644
@@ -457,7 +457,7 @@ static int cafe_nand_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 	return 0;
 }
 
-static struct nand_ecclayout cafe_oobinfo_2048 = {
+static struct nand_ecclayout_l cafe_oobinfo_2048 = {
 	.eccbytes = 14,
 	.eccpos = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13},
 	.oobfree = {{14, 50}}

@@ -492,7 +492,7 @@ static struct nand_bbt_descr cafe_bbt_mirror_descr_2048 = {
 	.pattern = cafe_mirror_pattern_2048
 };
 
-static struct nand_ecclayout cafe_oobinfo_512 = {
+static struct nand_ecclayout_l cafe_oobinfo_512 = {
 	.eccbytes = 14,
 	.eccpos = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13},
 	.oobfree = {{14, 2}}

drivers/mtd/nand/davinci_nand.c +7 -4

index 2ac7367..c0141b1 100644
@@ -55,7 +55,7 @@
 struct davinci_nand_info {
 	struct mtd_info		mtd;
 	struct nand_chip	chip;
-	struct nand_ecclayout	ecclayout;
+	struct nand_ecclayout_l	ecclayout;
 
 	struct device		*dev;
 	struct clk		*clk;

@@ -514,7 +514,7 @@ static void __init nand_dm6446evm_flash_init(struct davinci_nand_info *info)
  * ten ECC bytes plus the manufacturer's bad block marker byte, and
  * and not overlapping the default BBT markers.
  */
-static struct nand_ecclayout hwecc4_small __initconst = {
+static struct nand_ecclayout_l hwecc4_small __initconst = {
 	.eccbytes = 10,
 	.eccpos = { 0, 1, 2, 3, 4,
 		/* offset 5 holds the badblock marker */

@@ -530,7 +530,7 @@ static struct nand_ecclayout hwecc4_small __initconst = {
  * storing ten ECC bytes plus the manufacturer's bad block marker byte,
  * and not overlapping the default BBT markers.
  */
-static struct nand_ecclayout hwecc4_2048 __initconst = {
+static struct nand_ecclayout_l hwecc4_2048 __initconst = {
 	.eccbytes = 40,
 	.eccpos = {
 		/* at the end of spare sector */

@@ -745,10 +745,13 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
 		}
 
 		/* 4KiB page chips are not yet supported. The eccpos from
-		 * nand_ecclayout cannot hold 80 bytes and change to eccpos[]
+		 * nand_ecclayout_l cannot hold 80 bytes and change to eccpos[]
 		 * breaks userspace ioctl interface with mtd-utils. Once we
 		 * resolve this issue, NAND_ECC_HW_OOB_FIRST mode can be used
 		 * for the 4KiB page chips.
+		 *
+		 * TODO: Note that nand_ecclayout_l has been introduced to
+		 *   replace nand_ecclayout and can hold plenty of OOB entries.
 		 */
 		dev_warn(&pdev->dev, "no 4-bit ECC support yet "
 				"for 4KiB-page NAND\n");

drivers/mtd/nand/diskonchip.c +1 -1

index 51315f5..5d29d6a 100644
@@ -1051,7 +1051,7 @@ static int doc200x_correct_data(struct mtd_info *mtd, u_char *dat,
  * safer.  The only problem with it is that any code that parses oobfree must
  * be able to handle out-of-order segments.
  */
-static struct nand_ecclayout doc200x_oobinfo = {
+static struct nand_ecclayout_l doc200x_oobinfo = {
 	.eccbytes = 6,
 	.eccpos = {0, 1, 2, 3, 4, 5},
 	.oobfree = {{8, 8}, {6, 2}}

drivers/mtd/nand/fsl_elbc_nand.c +4 -4

index 3f38fb8..b88a078 100644
@@ -85,28 +85,28 @@ struct fsl_elbc_ctrl {
 /* These map to the positions used by the FCM hardware ECC generator */
 
 /* Small Page FLASH with FMR[ECCM] = 0 */
-static struct nand_ecclayout fsl_elbc_oob_sp_eccm0 = {
+static struct nand_ecclayout_l fsl_elbc_oob_sp_eccm0 = {
 	.eccbytes = 3,
 	.eccpos = {6, 7, 8},
 	.oobfree = { {0, 5}, {9, 7} },
 };
 
 /* Small Page FLASH with FMR[ECCM] = 1 */
-static struct nand_ecclayout fsl_elbc_oob_sp_eccm1 = {
+static struct nand_ecclayout_l fsl_elbc_oob_sp_eccm1 = {
 	.eccbytes = 3,
 	.eccpos = {8, 9, 10},
 	.oobfree = { {0, 5}, {6, 2}, {11, 5} },
 };
 
 /* Large Page FLASH with FMR[ECCM] = 0 */
-static struct nand_ecclayout fsl_elbc_oob_lp_eccm0 = {
+static struct nand_ecclayout_l fsl_elbc_oob_lp_eccm0 = {
 	.eccbytes = 12,
 	.eccpos = {6, 7, 8, 22, 23, 24, 38, 39, 40, 54, 55, 56},
 	.oobfree = { {1, 5}, {9, 13}, {25, 13}, {41, 13}, {57, 7} },
 };
 
 /* Large Page FLASH with FMR[ECCM] = 1 */
-static struct nand_ecclayout fsl_elbc_oob_lp_eccm1 = {
+static struct nand_ecclayout_l fsl_elbc_oob_lp_eccm1 = {
 	.eccbytes = 12,
 	.eccpos = {8, 9, 10, 24, 25, 26, 40, 41, 42, 56, 57, 58},
 	.oobfree = { {1, 7}, {11, 13}, {27, 13}, {43, 13}, {59, 5} },

drivers/mtd/nand/mxc_nand.c +5 -5

index 3657a6e..fddddaa 100644
@@ -165,13 +165,13 @@ struct mxc_nand_host {
 };
 
 /* OOB placement block for use with hardware ecc generation */
-static struct nand_ecclayout nandv1_hw_eccoob_smallpage = {
+static struct nand_ecclayout_l nandv1_hw_eccoob_smallpage = {
 	.eccbytes = 5,
 	.eccpos = {6, 7, 8, 9, 10},
 	.oobfree = {{0, 5}, {12, 4}, }
 };
 
-static struct nand_ecclayout nandv1_hw_eccoob_largepage = {
+static struct nand_ecclayout_l nandv1_hw_eccoob_largepage = {
 	.eccbytes = 20,
 	.eccpos = {6, 7, 8, 9, 10, 22, 23, 24, 25, 26,
 		   38, 39, 40, 41, 42, 54, 55, 56, 57, 58},

@@ -179,7 +179,7 @@ static struct nand_ecclayout nandv1_hw_eccoob_largepage = {
 };
 
 /* OOB description for 512 byte pages with 16 byte OOB */
-static struct nand_ecclayout nandv2_hw_eccoob_smallpage = {
+static struct nand_ecclayout_l nandv2_hw_eccoob_smallpage = {
 	.eccbytes = 1 * 9,
 	.eccpos = {
 		 7,  8,  9, 10, 11, 12, 13, 14, 15

@@ -190,7 +190,7 @@ static struct nand_ecclayout nandv2_hw_eccoob_smallpage = {
 };
 
 /* OOB description for 2048 byte pages with 64 byte OOB */
-static struct nand_ecclayout nandv2_hw_eccoob_largepage = {
+static struct nand_ecclayout_l nandv2_hw_eccoob_largepage = {
 	.eccbytes = 4 * 9,
 	.eccpos = {
 		 7,  8,  9, 10, 11, 12, 13, 14, 15,

@@ -956,7 +956,7 @@ static int __init mxcnd_probe(struct platform_device *pdev)
 	struct mxc_nand_host *host;
 	struct resource *res;
 	int err = 0, nr_parts = 0;
-	struct nand_ecclayout *oob_smallpage, *oob_largepage;
+	struct nand_ecclayout_l *oob_smallpage, *oob_largepage;
 
 	/* Allocate memory for MTD device structure and private data */
 	host = kzalloc(sizeof(struct mxc_nand_host) + NAND_MAX_PAGESIZE +

drivers/mtd/nand/nand_base.c +4 -4

index ee6a6f8..a7bfaa4 100644
@@ -53,7 +53,7 @@
 #endif
 
 /* Define default oob placement schemes for large and small page devices */
-static struct nand_ecclayout nand_oob_8 = {
+static struct nand_ecclayout_l nand_oob_8 = {
 	.eccbytes = 3,
 	.eccpos = {0, 1, 2},
 	.oobfree = {

@@ -63,7 +63,7 @@ static struct nand_ecclayout nand_oob_8 = {
 		 .length = 2}}
 };
 
-static struct nand_ecclayout nand_oob_16 = {
+static struct nand_ecclayout_l nand_oob_16 = {
 	.eccbytes = 6,
 	.eccpos = {0, 1, 2, 3, 6, 7},
 	.oobfree = {

@@ -71,7 +71,7 @@ static struct nand_ecclayout nand_oob_16 = {
 		 . length = 8}}
 };
 
-static struct nand_ecclayout nand_oob_64 = {
+static struct nand_ecclayout_l nand_oob_64 = {
 	.eccbytes = 24,
 	.eccpos = {
 		   40, 41, 42, 43, 44, 45, 46, 47,

@@ -82,7 +82,7 @@ static struct nand_ecclayout nand_oob_64 = {
 		 .length = 38}}
 };
 
-static struct nand_ecclayout nand_oob_128 = {
+static struct nand_ecclayout_l nand_oob_128 = {
 	.eccbytes = 48,
 	.eccpos = {
 		   80, 81, 82, 83, 84, 85, 86, 87,

drivers/mtd/nand/nomadik_nand.c +1 -1

index 8c0b693..ed5f2ba 100644
@@ -45,7 +45,7 @@ struct nomadik_nand_host {
 	struct nand_bbt_descr *bbt_desc;
 };
 
-static struct nand_ecclayout nomadik_ecc_layout = {
+static struct nand_ecclayout_l nomadik_ecc_layout = {
 	.eccbytes = 3 * 4,
 	.eccpos = { /* each subpage has 16 bytes: pos 2,3,4 hosts ECC */
 		0x02, 0x03, 0x04,

drivers/mtd/nand/pxa3xx_nand.c +2 -2

index e02fa4f..ecb3742 100644
@@ -1159,13 +1159,13 @@ static int pxa3xx_nand_init_buff(struct pxa3xx_nand_info *info)
 	return 0;
 }
 
-static struct nand_ecclayout hw_smallpage_ecclayout = {
+static struct nand_ecclayout_l hw_smallpage_ecclayout = {
 	.eccbytes = 6,
 	.eccpos = {8, 9, 10, 11, 12, 13 },
 	.oobfree = { {2, 6} }
 };
 
-static struct nand_ecclayout hw_largepage_ecclayout = {
+static struct nand_ecclayout_l hw_largepage_ecclayout = {
 	.eccbytes = 24,
 	.eccpos = {
 		40, 41, 42, 43, 44, 45, 46, 47,

drivers/mtd/nand/rtc_from4.c +1 -1

index a033c4c..0c28637 100644
@@ -141,7 +141,7 @@ static struct rs_control *rs_decoder;
 /*
  *      hardware specific Out Of Band information
  */
-static struct nand_ecclayout rtc_from4_nand_oobinfo = {
+static struct nand_ecclayout_l rtc_from4_nand_oobinfo = {
 	.eccbytes = 32,
 	.eccpos = {
 		   0, 1, 2, 3, 4, 5, 6, 7,

drivers/mtd/nand/s3c2410.c +1 -1

index 33d832d..d18fc6d 100644
@@ -64,7 +64,7 @@ static const int clock_stop = 0;
 /* new oob placement block for use with hardware ecc generation
  */
 
-static struct nand_ecclayout nand_hw_eccoob = {
+static struct nand_ecclayout_l nand_hw_eccoob = {
 	.eccbytes = 3,
 	.eccpos = {0, 1, 2},
 	.oobfree = {{8, 8}}

drivers/mtd/nand/sh_flctl.c +2 -2

index 546c2f0..91d67bc 100644
@@ -33,7 +33,7 @@
 #include <linux/mtd/partitions.h>
 #include <linux/mtd/sh_flctl.h>
 
-static struct nand_ecclayout flctl_4secc_oob_16 = {
+static struct nand_ecclayout_l flctl_4secc_oob_16 = {
 	.eccbytes = 10,
 	.eccpos = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9},
 	.oobfree = {

@@ -41,7 +41,7 @@ static struct nand_ecclayout flctl_4secc_oob_16 = {
 		. length = 4} },
 };
 
-static struct nand_ecclayout flctl_4secc_oob_64 = {
+static struct nand_ecclayout_l flctl_4secc_oob_64 = {
 	.eccbytes = 10,
 	.eccpos = {48, 49, 50, 51, 52, 53, 54, 55, 56, 57},
 	.oobfree = {

drivers/mtd/onenand/onenand_base.c +3 -3

index a2bb520..dcb9683 100644
@@ -69,7 +69,7 @@ MODULE_PARM_DESC(otp,	"Corresponding behaviour of OneNAND in OTP"
  *  onenand_oob_128 - oob info for Flex-Onenand with 4KB page
  *  For now, we expose only 64 out of 80 ecc bytes
  */
-static struct nand_ecclayout onenand_oob_128 = {
+static struct nand_ecclayout_l onenand_oob_128 = {
 	.eccbytes	= 64,
 	.eccpos		= {
 		6, 7, 8, 9, 10, 11, 12, 13, 14, 15,

@@ -89,7 +89,7 @@ static struct nand_ecclayout onenand_oob_128 = {
 /**
  * onenand_oob_64 - oob info for large (2KB) page
  */
-static struct nand_ecclayout onenand_oob_64 = {
+static struct nand_ecclayout_l onenand_oob_64 = {
 	.eccbytes	= 20,
 	.eccpos		= {
 		8, 9, 10, 11, 12,

@@ -106,7 +106,7 @@ static struct nand_ecclayout onenand_oob_64 = {
 /**
  * onenand_oob_32 - oob info for middle (1KB) page
  */
-static struct nand_ecclayout onenand_oob_32 = {
+static struct nand_ecclayout_l onenand_oob_32 = {
 	.eccbytes	= 10,
 	.eccpos		= {
 		8, 9, 10, 11, 12,

fs/jffs2/wbuf.c +1 -1

index 07ee154..e639133 100644
@@ -1145,7 +1145,7 @@ int jffs2_write_nand_badblock(struct jffs2_sb_info *c, struct jffs2_eraseblock *
 
 int jffs2_nand_flash_setup(struct jffs2_sb_info *c)
 {
-	struct nand_ecclayout *oinfo = c->mtd->ecclayout;
+	struct nand_ecclayout_l *oinfo = c->mtd->ecclayout;
 
 	if (!c->mtd->oobsize)
 		return 0;

include/linux/mtd/inftl.h +1 -1

index 64ee53c..9e3a43e 100644
@@ -44,7 +44,7 @@ struct INFTLrecord {
         unsigned int nb_blocks;		/* number of physical blocks */
         unsigned int nb_boot_blocks;	/* number of blocks used by the bios */
         struct erase_info instr;
-        struct nand_ecclayout oobinfo;
+	struct nand_ecclayout_l oobinfo;
 };
 
 int INFTL_mount(struct INFTLrecord *s);

include/linux/mtd/mtd.h +16 -1

index 43b7d72..b60da00 100644
@@ -98,6 +98,21 @@ struct mtd_oob_ops {
 	uint8_t		*oobbuf;
 };
 
+#define MTD_MAX_OOBFREE_ENTRIES_LARGE	32
+#define MTD_MAX_ECCPOS_ENTRIES_LARGE	448
+#define MTD_MAX_ECCPOS_ENTRIES_OLD	64	/* Previous maximum */
+/*
+ * Correct ECC layout control structure. This replaces old nand_ecclayout
+ * that is exported via ECCGETLAYOUT ioctll. It should be expandable in the
+ * future simply by the above macros.
+ */
+struct nand_ecclayout_l {
+	__u32 eccbytes;
+	__u32 eccpos[MTD_MAX_ECCPOS_ENTRIES_LARGE];
+	__u32 oobavail;
+	struct nand_oobfree oobfree[MTD_MAX_OOBFREE_ENTRIES_LARGE];
+};
+
 struct mtd_info {
 	u_char type;
 	uint32_t flags;

@@ -135,7 +150,7 @@ struct mtd_info {
 	int index;
 
 	/* ecc layout structure pointer - read only ! */
-	struct nand_ecclayout *ecclayout;
+	struct nand_ecclayout_l *ecclayout;
 
 	/* Data for variable erase regions. If numeraseregions is zero,
 	 * it means that the whole device has erasesize as given above.

include/linux/mtd/nand.h +3 -3

index 50f3aa0..adc081f 100644
@@ -270,7 +270,7 @@ struct nand_ecc_ctrl {
 	int			total;
 	int			prepad;
 	int			postpad;
-	struct nand_ecclayout	*layout;
+	struct nand_ecclayout_l	*layout;
 	void			(*hwctl)(struct mtd_info *mtd, int mode);
 	int			(*calculate)(struct mtd_info *mtd,
 					     const uint8_t *dat,

@@ -415,7 +415,7 @@ struct nand_chip {
 
 	uint8_t		*oob_poi;
 	struct nand_hw_control  *controller;
-	struct nand_ecclayout	*ecclayout;
+	struct nand_ecclayout_l	*ecclayout;
 
 	struct nand_ecc_ctrl ecc;
 	struct nand_buffers *buffers;

@@ -506,7 +506,7 @@ struct platform_nand_chip {
 	int			chip_offset;
 	int			nr_partitions;
 	struct mtd_partition	*partitions;
-	struct nand_ecclayout	*ecclayout;
+	struct nand_ecclayout_l	*ecclayout;
 	int			chip_delay;
 	unsigned int		options;
 	const char		**part_probe_types;

include/linux/mtd/nftl.h +1 -1

index dcaf611..1d0839f 100644
@@ -35,7 +35,7 @@ struct NFTLrecord {
         unsigned int nb_blocks;		/* number of physical blocks */
         unsigned int nb_boot_blocks;	/* number of blocks used by the bios */
         struct erase_info instr;
-	struct nand_ecclayout oobinfo;
+	struct nand_ecclayout_l oobinfo;
 };
 
 int NFTL_mount(struct NFTLrecord *s);

include/linux/mtd/onenand.h +1 -1

index 0c8815b..c946ec6 100644
@@ -132,7 +132,7 @@ struct onenand_chip {
 #endif
 
 	int			subpagesize;
-	struct nand_ecclayout	*ecclayout;
+	struct nand_ecclayout_l	*ecclayout;
 
 	void			*bbm;

include/linux/mtd/partitions.h +1 -1

index 274b619..ae390d5 100644
@@ -39,7 +39,7 @@ struct mtd_partition {
 	uint64_t size;			/* partition size */
 	uint64_t offset;		/* offset within the master MTD space */
 	uint32_t mask_flags;		/* master MTD flags to mask out for this partition */
-	struct nand_ecclayout *ecclayout;	/* out of band layout for this partition (NAND only)*/
+	struct nand_ecclayout_l *ecclayout;	/* out of band layout for this partition (NAND only) */
 };
 
 #define MTDPART_OFS_NXTBLK	(-2)

include/linux/mtd/sharpsl.h +1 -1

index 25f4d2a..15fa381 100644
@@ -14,7 +14,7 @@
 
 struct sharpsl_nand_platform_data {
 	struct nand_bbt_descr	*badblock_pattern;
-	struct nand_ecclayout	*ecc_layout;
+	struct nand_ecclayout_l	*ecc_layout;
 	struct mtd_partition	*partitions;
 	unsigned int		nr_partitions;
 };

Signature · 1 line

1.7.0.4

Brian Norris

author

2010-08-18 18:07 UTC #3 lore raw

v2 cover Deprecate ECCGETLAYOUT

The following two patches are implementations of two different strategies to deprecate the ioctl ECCGETLAYOUT. This ioctl is not very useful and imposes restrictions on expanding the corresponding kernel struct nand_ecclayout. Either patch will leave functionality intact but allow the kernel-space layouts to expand in the future.

Strategy 1: Create new struct nand_ecclayout_l and convert all kernel
code to use this struct.
Strategy 2: Rename old struct nand_ecclayout_user and modify
nand_ecclayout to be used in the kernel only.

Either strategy employs a new function that will shrink the larger layout to fit the size of the old one.

Please let me know if either of these will have unintended consequences. I believe I have tested these sufficiently in both simulation and hardware.

Thanks,
Brian

Brian Norris (2):
mtd: nand: Expand nand_ecc_layout, deprecate ioctl ECCGETLAYOUT
mtd: nand: Expand nand_ecc_layout, deprecate ioctl ECCGETLAYOUT

Brian Norris

author

2010-08-18 20:38 UTC #4 lore raw

PATCH v3 1/2 mtd: Expand nand_ecc_layout, deprecate ioctl ECCGETLAYOUT

Ugh, this is ACTUALLY strategy 1 of 2 strategies proposed for this deprecation. I managed to send the same one twice...

Signed-off-by Brian Norris <[email protected]>

---
 arch/arm/mach-pxa/spitz.c                       |    2 +-
 arch/arm/plat-samsung/dev-nand.c                |    2 +-
 arch/arm/plat-samsung/include/plat/nand.h       |    2 +-
 arch/mips/include/asm/mach-jz4740/jz4740_nand.h |    2 +-
 arch/mips/jz4740/board-qi_lb60.c                |    4 +-
 drivers/mtd/mtdchar.c                           |   48 +++++++++++++++++++++-
 drivers/mtd/nand/atmel_nand.c                   |    4 +-
 drivers/mtd/nand/bcm_umi_bch.c                  |    6 +-
 drivers/mtd/nand/bf5xx_nand.c                   |    2 +-
 drivers/mtd/nand/cafe_nand.c                    |    4 +-
 drivers/mtd/nand/davinci_nand.c                 |   11 +++--
 drivers/mtd/nand/denali.c                       |    4 +-
 drivers/mtd/nand/diskonchip.c                   |    2 +-
 drivers/mtd/nand/fsl_elbc_nand.c                |    8 ++--
 drivers/mtd/nand/mxc_nand.c                     |   10 ++--
 drivers/mtd/nand/nand_base.c                    |    8 ++--
 drivers/mtd/nand/nomadik_nand.c                 |    2 +-
 drivers/mtd/nand/pxa3xx_nand.c                  |    4 +-
 drivers/mtd/nand/rtc_from4.c                    |    2 +-
 drivers/mtd/nand/s3c2410.c                      |    2 +-
 drivers/mtd/nand/sh_flctl.c                     |    4 +-
 drivers/mtd/nand/sm_common.c                    |    4 +-
 drivers/mtd/onenand/onenand_base.c              |    6 +-
 fs/jffs2/wbuf.c                                 |    2 +-
 include/linux/mtd/inftl.h                       |    2 +-
 include/linux/mtd/mtd.h                         |   17 ++++++++-
 include/linux/mtd/nand.h                        |    6 +-
 include/linux/mtd/nftl.h                        |    2 +-
 include/linux/mtd/onenand.h                     |    2 +-
 include/linux/mtd/partitions.h                  |    2 +-
 include/linux/mtd/sharpsl.h                     |    2 +-
 31 files changed, 119 insertions(+), 59 deletions(-)

arch/arm/mach-pxa/spitz.c +1 -1

index 1cd99cb..1a8cdc3 100644
@@ -757,7 +757,7 @@ static struct nand_bbt_descr spitz_nand_bbt = {
 	.pattern	= scan_ff_pattern
 };
 
-static struct nand_ecclayout akita_oobinfo = {
+static struct nand_ecclayout_l akita_oobinfo = {
 	.oobfree	= { {0x08, 0x09} },
 	.eccbytes	= 24,
 	.eccpos		= {

arch/arm/plat-samsung/dev-nand.c +1 -1

index 3a7b889..0109307 100644
@@ -70,7 +70,7 @@ static int __init s3c_nand_copy_set(struct s3c2410_nand_set *set)
 
 	if (set->ecc_layout) {
 		ptr = kmemdup(set->ecc_layout,
-			      sizeof(struct nand_ecclayout), GFP_KERNEL);
+			      sizeof(*set->ecc_layout), GFP_KERNEL);
 		set->ecc_layout = ptr;
 
 		if (!ptr)

arch/arm/plat-samsung/include/plat/nand.h +1 -1

index b64115f..49dedc9 100644
@@ -38,7 +38,7 @@ struct s3c2410_nand_set {
 	char			*name;
 	int			*nr_map;
 	struct mtd_partition	*partitions;
-	struct nand_ecclayout	*ecc_layout;
+	struct nand_ecclayout_l	*ecc_layout;
 };
 
 struct s3c2410_platform_nand {

arch/mips/include/asm/mach-jz4740/jz4740_nand.h +1 -1

index bb5b9a4..79a43e5 100644
@@ -23,7 +23,7 @@ struct jz_nand_platform_data {
 	int			num_partitions;
 	struct mtd_partition	*partitions;
 
-	struct nand_ecclayout	*ecc_layout;
+	struct nand_ecclayout_l	*ecc_layout;
 
 	unsigned int busy_gpio;

arch/mips/jz4740/board-qi_lb60.c +2 -2

index 5742bb4..acefaa1 100644
@@ -48,7 +48,7 @@ static bool is_avt2;
 #define QI_LB60_GPIO_KEYIN8		JZ_GPIO_PORTD(26)
 
 /* NAND */
-static struct nand_ecclayout qi_lb60_ecclayout_1gb = {
+static struct nand_ecclayout_l qi_lb60_ecclayout_1gb = {
 /*	.eccbytes = 36,
 	.eccpos = {
 		6,  7,  8,  9,  10, 11, 12, 13,

@@ -84,7 +84,7 @@ static struct mtd_partition qi_lb60_partitions_1gb[] = {
 	},
 };
 
-static struct nand_ecclayout qi_lb60_ecclayout_2gb = {
+static struct nand_ecclayout_l qi_lb60_ecclayout_2gb = {
 /*	.eccbytes = 72,
 	.eccpos = {
 		12, 13, 14, 15, 16, 17, 18, 19,

drivers/mtd/mtdchar.c +45 -3

65 lines not on this page · raw message

drivers/mtd/nand/atmel_nand.c +2 -2

index ccce0f0..6d856d9 100644
@@ -64,7 +64,7 @@ module_param(on_flash_bbt, int, 0);
  * the bytes have to be consecutives to avoid
  * several NAND_CMD_RNDOUT during read
  */
-static struct nand_ecclayout atmel_oobinfo_large = {
+static struct nand_ecclayout_l atmel_oobinfo_large = {
 	.eccbytes = 4,
 	.eccpos = {60, 61, 62, 63},
 	.oobfree = {

@@ -77,7 +77,7 @@ static struct nand_ecclayout atmel_oobinfo_large = {
  * the bytes have to be consecutives to avoid
  * several NAND_CMD_RNDOUT during read
  */
-static struct nand_ecclayout atmel_oobinfo_small = {
+static struct nand_ecclayout_l atmel_oobinfo_small = {
 	.eccbytes = 4,
 	.eccpos = {0, 1, 2, 3},
 	.oobfree = {

drivers/mtd/nand/bcm_umi_bch.c +3 -3

index a930666..63be36a 100644
@@ -32,7 +32,7 @@ static void bcm_umi_bch_write_page_hwecc(struct mtd_info *mtd,
 ** nand_hw_eccoob
 ** New oob placement block for use with hardware ecc generation.
 */
-static struct nand_ecclayout nand_hw_eccoob_512 = {
+static struct nand_ecclayout_l nand_hw_eccoob_512 = {
 	/* Reserve 5 for BI indicator */
 	.oobfree = {
 #if (NAND_ECC_NUM_BYTES > 3)

@@ -48,7 +48,7 @@ static struct nand_ecclayout nand_hw_eccoob_512 = {
 ** We treat the OOB for a 2K page as if it were 4 512 byte oobs,
 ** except the BI is at byte 0.
 */
-static struct nand_ecclayout nand_hw_eccoob_2048 = {
+static struct nand_ecclayout_l nand_hw_eccoob_2048 = {
 	/* Reserve 0 as BI indicator */
 	.oobfree = {
 #if (NAND_ECC_NUM_BYTES > 10)

@@ -69,7 +69,7 @@ static struct nand_ecclayout nand_hw_eccoob_2048 = {
 
 /* We treat the OOB for a 4K page as if it were 8 512 byte oobs,
  * except the BI is at byte 0. */
-static struct nand_ecclayout nand_hw_eccoob_4096 = {
+static struct nand_ecclayout_l nand_hw_eccoob_4096 = {
 	/* Reserve 0 as BI indicator */
 	.oobfree = {
 #if (NAND_ECC_NUM_BYTES > 10)

drivers/mtd/nand/bf5xx_nand.c +1 -1

index a382e3d..3752541 100644
@@ -119,7 +119,7 @@ static struct nand_bbt_descr bootrom_bbt = {
 	.pattern = bbt_pattern,
 };
 
-static struct nand_ecclayout bootrom_ecclayout = {
+static struct nand_ecclayout_l bootrom_ecclayout = {
 	.eccbytes = 24,
 	.eccpos = {
 		0x8 * 0, 0x8 * 0 + 1, 0x8 * 0 + 2,

drivers/mtd/nand/cafe_nand.c +2 -2

index db1dfc5..d989627 100644
@@ -457,7 +457,7 @@ static int cafe_nand_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 	return 0;
 }
 
-static struct nand_ecclayout cafe_oobinfo_2048 = {
+static struct nand_ecclayout_l cafe_oobinfo_2048 = {
 	.eccbytes = 14,
 	.eccpos = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13},
 	.oobfree = {{14, 50}}

@@ -492,7 +492,7 @@ static struct nand_bbt_descr cafe_bbt_mirror_descr_2048 = {
 	.pattern = cafe_mirror_pattern_2048
 };
 
-static struct nand_ecclayout cafe_oobinfo_512 = {
+static struct nand_ecclayout_l cafe_oobinfo_512 = {
 	.eccbytes = 14,
 	.eccpos = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13},
 	.oobfree = {{14, 2}}

drivers/mtd/nand/davinci_nand.c +7 -4

index 2ac7367..c0141b1 100644
@@ -55,7 +55,7 @@
 struct davinci_nand_info {
 	struct mtd_info		mtd;
 	struct nand_chip	chip;
-	struct nand_ecclayout	ecclayout;
+	struct nand_ecclayout_l	ecclayout;
 
 	struct device		*dev;
 	struct clk		*clk;

@@ -514,7 +514,7 @@ static void __init nand_dm6446evm_flash_init(struct davinci_nand_info *info)
  * ten ECC bytes plus the manufacturer's bad block marker byte, and
  * and not overlapping the default BBT markers.
  */
-static struct nand_ecclayout hwecc4_small __initconst = {
+static struct nand_ecclayout_l hwecc4_small __initconst = {
 	.eccbytes = 10,
 	.eccpos = { 0, 1, 2, 3, 4,
 		/* offset 5 holds the badblock marker */

@@ -530,7 +530,7 @@ static struct nand_ecclayout hwecc4_small __initconst = {
  * storing ten ECC bytes plus the manufacturer's bad block marker byte,
  * and not overlapping the default BBT markers.
  */
-static struct nand_ecclayout hwecc4_2048 __initconst = {
+static struct nand_ecclayout_l hwecc4_2048 __initconst = {
 	.eccbytes = 40,
 	.eccpos = {
 		/* at the end of spare sector */

@@ -745,10 +745,13 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
 		}
 
 		/* 4KiB page chips are not yet supported. The eccpos from
-		 * nand_ecclayout cannot hold 80 bytes and change to eccpos[]
+		 * nand_ecclayout_l cannot hold 80 bytes and change to eccpos[]
 		 * breaks userspace ioctl interface with mtd-utils. Once we
 		 * resolve this issue, NAND_ECC_HW_OOB_FIRST mode can be used
 		 * for the 4KiB page chips.
+		 *
+		 * TODO: Note that nand_ecclayout_l has been introduced to
+		 *   replace nand_ecclayout and can hold plenty of OOB entries.
 		 */
 		dev_warn(&pdev->dev, "no 4-bit ECC support yet "
 				"for 4KiB-page NAND\n");

drivers/mtd/nand/denali.c +2 -2

index 532fe07..adf40a3 100644
@@ -1383,12 +1383,12 @@ static void denali_hw_init(struct denali_nand_info *denali)
  * correction
  * */
 #define ECC_8BITS	14
-static struct nand_ecclayout nand_8bit_oob = {
+static struct nand_ecclayout_l nand_8bit_oob = {
 	.eccbytes = 14,
 };
 
 #define ECC_15BITS	26
-static struct nand_ecclayout nand_15bit_oob = {
+static struct nand_ecclayout_l nand_15bit_oob = {
 	.eccbytes = 26,
 };

drivers/mtd/nand/diskonchip.c +1 -1

index b7f8de7..f86e428 100644
@@ -1050,7 +1050,7 @@ static int doc200x_correct_data(struct mtd_info *mtd, u_char *dat,
  * safer.  The only problem with it is that any code that parses oobfree must
  * be able to handle out-of-order segments.
  */
-static struct nand_ecclayout doc200x_oobinfo = {
+static struct nand_ecclayout_l doc200x_oobinfo = {
 	.eccbytes = 6,
 	.eccpos = {0, 1, 2, 3, 4, 5},
 	.oobfree = {{8, 8}, {6, 2}}

drivers/mtd/nand/fsl_elbc_nand.c +4 -4

index 80de0bf..68af230 100644
@@ -85,28 +85,28 @@ struct fsl_elbc_ctrl {
 /* These map to the positions used by the FCM hardware ECC generator */
 
 /* Small Page FLASH with FMR[ECCM] = 0 */
-static struct nand_ecclayout fsl_elbc_oob_sp_eccm0 = {
+static struct nand_ecclayout_l fsl_elbc_oob_sp_eccm0 = {
 	.eccbytes = 3,
 	.eccpos = {6, 7, 8},
 	.oobfree = { {0, 5}, {9, 7} },
 };
 
 /* Small Page FLASH with FMR[ECCM] = 1 */
-static struct nand_ecclayout fsl_elbc_oob_sp_eccm1 = {
+static struct nand_ecclayout_l fsl_elbc_oob_sp_eccm1 = {
 	.eccbytes = 3,
 	.eccpos = {8, 9, 10},
 	.oobfree = { {0, 5}, {6, 2}, {11, 5} },
 };
 
 /* Large Page FLASH with FMR[ECCM] = 0 */
-static struct nand_ecclayout fsl_elbc_oob_lp_eccm0 = {
+static struct nand_ecclayout_l fsl_elbc_oob_lp_eccm0 = {
 	.eccbytes = 12,
 	.eccpos = {6, 7, 8, 22, 23, 24, 38, 39, 40, 54, 55, 56},
 	.oobfree = { {1, 5}, {9, 13}, {25, 13}, {41, 13}, {57, 7} },
 };
 
 /* Large Page FLASH with FMR[ECCM] = 1 */
-static struct nand_ecclayout fsl_elbc_oob_lp_eccm1 = {
+static struct nand_ecclayout_l fsl_elbc_oob_lp_eccm1 = {
 	.eccbytes = 12,
 	.eccpos = {8, 9, 10, 24, 25, 26, 40, 41, 42, 56, 57, 58},
 	.oobfree = { {1, 7}, {11, 13}, {27, 13}, {43, 13}, {59, 5} },

drivers/mtd/nand/mxc_nand.c +5 -5

index fcf8ceb..77b4d9e 100644
@@ -165,13 +165,13 @@ struct mxc_nand_host {
 };
 
 /* OOB placement block for use with hardware ecc generation */
-static struct nand_ecclayout nandv1_hw_eccoob_smallpage = {
+static struct nand_ecclayout_l nandv1_hw_eccoob_smallpage = {
 	.eccbytes = 5,
 	.eccpos = {6, 7, 8, 9, 10},
 	.oobfree = {{0, 5}, {12, 4}, }
 };
 
-static struct nand_ecclayout nandv1_hw_eccoob_largepage = {
+static struct nand_ecclayout_l nandv1_hw_eccoob_largepage = {
 	.eccbytes = 20,
 	.eccpos = {6, 7, 8, 9, 10, 22, 23, 24, 25, 26,
 		   38, 39, 40, 41, 42, 54, 55, 56, 57, 58},

@@ -179,7 +179,7 @@ static struct nand_ecclayout nandv1_hw_eccoob_largepage = {
 };
 
 /* OOB description for 512 byte pages with 16 byte OOB */
-static struct nand_ecclayout nandv2_hw_eccoob_smallpage = {
+static struct nand_ecclayout_l nandv2_hw_eccoob_smallpage = {
 	.eccbytes = 1 * 9,
 	.eccpos = {
 		 7,  8,  9, 10, 11, 12, 13, 14, 15

@@ -190,7 +190,7 @@ static struct nand_ecclayout nandv2_hw_eccoob_smallpage = {
 };
 
 /* OOB description for 2048 byte pages with 64 byte OOB */
-static struct nand_ecclayout nandv2_hw_eccoob_largepage = {
+static struct nand_ecclayout_l nandv2_hw_eccoob_largepage = {
 	.eccbytes = 4 * 9,
 	.eccpos = {
 		 7,  8,  9, 10, 11, 12, 13, 14, 15,

@@ -956,7 +956,7 @@ static int __init mxcnd_probe(struct platform_device *pdev)
 	struct mxc_nand_host *host;
 	struct resource *res;
 	int err = 0, nr_parts = 0;
-	struct nand_ecclayout *oob_smallpage, *oob_largepage;
+	struct nand_ecclayout_l *oob_smallpage, *oob_largepage;
 
 	/* Allocate memory for MTD device structure and private data */
 	host = kzalloc(sizeof(struct mxc_nand_host) + NAND_MAX_PAGESIZE +

drivers/mtd/nand/nand_base.c +4 -4

index a3c7473..322e608 100644
@@ -52,7 +52,7 @@
 #endif
 
 /* Define default oob placement schemes for large and small page devices */
-static struct nand_ecclayout nand_oob_8 = {
+static struct nand_ecclayout_l nand_oob_8 = {
 	.eccbytes = 3,
 	.eccpos = {0, 1, 2},
 	.oobfree = {

@@ -62,7 +62,7 @@ static struct nand_ecclayout nand_oob_8 = {
 		 .length = 2}}
 };
 
-static struct nand_ecclayout nand_oob_16 = {
+static struct nand_ecclayout_l nand_oob_16 = {
 	.eccbytes = 6,
 	.eccpos = {0, 1, 2, 3, 6, 7},
 	.oobfree = {

@@ -70,7 +70,7 @@ static struct nand_ecclayout nand_oob_16 = {
 		 . length = 8}}
 };
 
-static struct nand_ecclayout nand_oob_64 = {
+static struct nand_ecclayout_l nand_oob_64 = {
 	.eccbytes = 24,
 	.eccpos = {
 		   40, 41, 42, 43, 44, 45, 46, 47,

@@ -81,7 +81,7 @@ static struct nand_ecclayout nand_oob_64 = {
 		 .length = 38}}
 };
 
-static struct nand_ecclayout nand_oob_128 = {
+static struct nand_ecclayout_l nand_oob_128 = {
 	.eccbytes = 48,
 	.eccpos = {
 		   80, 81, 82, 83, 84, 85, 86, 87,

drivers/mtd/nand/nomadik_nand.c +1 -1

index 8c0b693..ed5f2ba 100644
@@ -45,7 +45,7 @@ struct nomadik_nand_host {
 	struct nand_bbt_descr *bbt_desc;
 };
 
-static struct nand_ecclayout nomadik_ecc_layout = {
+static struct nand_ecclayout_l nomadik_ecc_layout = {
 	.eccbytes = 3 * 4,
 	.eccpos = { /* each subpage has 16 bytes: pos 2,3,4 hosts ECC */
 		0x02, 0x03, 0x04,

drivers/mtd/nand/pxa3xx_nand.c +2 -2

index 4d89f37..a16b00a 100644
@@ -1159,13 +1159,13 @@ static int pxa3xx_nand_init_buff(struct pxa3xx_nand_info *info)
 	return 0;
 }
 
-static struct nand_ecclayout hw_smallpage_ecclayout = {
+static struct nand_ecclayout_l hw_smallpage_ecclayout = {
 	.eccbytes = 6,
 	.eccpos = {8, 9, 10, 11, 12, 13 },
 	.oobfree = { {2, 6} }
 };
 
-static struct nand_ecclayout hw_largepage_ecclayout = {
+static struct nand_ecclayout_l hw_largepage_ecclayout = {
 	.eccbytes = 24,
 	.eccpos = {
 		40, 41, 42, 43, 44, 45, 46, 47,

drivers/mtd/nand/rtc_from4.c +1 -1

index 67440b5..e538caf 100644
@@ -140,7 +140,7 @@ static struct rs_control *rs_decoder;
 /*
  *      hardware specific Out Of Band information
  */
-static struct nand_ecclayout rtc_from4_nand_oobinfo = {
+static struct nand_ecclayout_l rtc_from4_nand_oobinfo = {
 	.eccbytes = 32,
 	.eccpos = {
 		   0, 1, 2, 3, 4, 5, 6, 7,

drivers/mtd/nand/s3c2410.c +1 -1

index 33d832d..d18fc6d 100644
@@ -64,7 +64,7 @@ static const int clock_stop = 0;
 /* new oob placement block for use with hardware ecc generation
  */
 
-static struct nand_ecclayout nand_hw_eccoob = {
+static struct nand_ecclayout_l nand_hw_eccoob = {
 	.eccbytes = 3,
 	.eccpos = {0, 1, 2},
 	.oobfree = {{8, 8}}

drivers/mtd/nand/sh_flctl.c +2 -2

index 546c2f0..91d67bc 100644
@@ -33,7 +33,7 @@
 #include <linux/mtd/partitions.h>
 #include <linux/mtd/sh_flctl.h>
 
-static struct nand_ecclayout flctl_4secc_oob_16 = {
+static struct nand_ecclayout_l flctl_4secc_oob_16 = {
 	.eccbytes = 10,
 	.eccpos = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9},
 	.oobfree = {

@@ -41,7 +41,7 @@ static struct nand_ecclayout flctl_4secc_oob_16 = {
 		. length = 4} },
 };
 
-static struct nand_ecclayout flctl_4secc_oob_64 = {
+static struct nand_ecclayout_l flctl_4secc_oob_64 = {
 	.eccbytes = 10,
 	.eccpos = {48, 49, 50, 51, 52, 53, 54, 55, 56, 57},
 	.oobfree = {

drivers/mtd/nand/sm_common.c +2 -2

index 4a8f367..09e67e2 100644
@@ -10,7 +10,7 @@
 #include <linux/mtd/nand.h>
 #include "sm_common.h"
 
-static struct nand_ecclayout nand_oob_sm = {
+static struct nand_ecclayout_l nand_oob_sm = {
 	.eccbytes = 6,
 	.eccpos = {8, 9, 10, 13, 14, 15},
 	.oobfree = {

@@ -26,7 +26,7 @@ static struct nand_ecclayout nand_oob_sm = {
 /* If you use smftl, it will bypass this and work correctly */
 /* If you not, then you break SmartMedia compliance anyway */
 
-static struct nand_ecclayout nand_oob_sm_small = {
+static struct nand_ecclayout_l nand_oob_sm_small = {
 	.eccbytes = 3,
 	.eccpos = {0, 1, 2},
 	.oobfree = {

drivers/mtd/onenand/onenand_base.c +3 -3

index a2bb520..dcb9683 100644
@@ -69,7 +69,7 @@ MODULE_PARM_DESC(otp,	"Corresponding behaviour of OneNAND in OTP"
  *  onenand_oob_128 - oob info for Flex-Onenand with 4KB page
  *  For now, we expose only 64 out of 80 ecc bytes
  */
-static struct nand_ecclayout onenand_oob_128 = {
+static struct nand_ecclayout_l onenand_oob_128 = {
 	.eccbytes	= 64,
 	.eccpos		= {
 		6, 7, 8, 9, 10, 11, 12, 13, 14, 15,

@@ -89,7 +89,7 @@ static struct nand_ecclayout onenand_oob_128 = {
 /**
  * onenand_oob_64 - oob info for large (2KB) page
  */
-static struct nand_ecclayout onenand_oob_64 = {
+static struct nand_ecclayout_l onenand_oob_64 = {
 	.eccbytes	= 20,
 	.eccpos		= {
 		8, 9, 10, 11, 12,

@@ -106,7 +106,7 @@ static struct nand_ecclayout onenand_oob_64 = {
 /**
  * onenand_oob_32 - oob info for middle (1KB) page
  */
-static struct nand_ecclayout onenand_oob_32 = {
+static struct nand_ecclayout_l onenand_oob_32 = {
 	.eccbytes	= 10,
 	.eccpos		= {
 		8, 9, 10, 11, 12,

fs/jffs2/wbuf.c +1 -1

index 07ee154..e639133 100644
@@ -1145,7 +1145,7 @@ int jffs2_write_nand_badblock(struct jffs2_sb_info *c, struct jffs2_eraseblock *
 
 int jffs2_nand_flash_setup(struct jffs2_sb_info *c)
 {
-	struct nand_ecclayout *oinfo = c->mtd->ecclayout;
+	struct nand_ecclayout_l *oinfo = c->mtd->ecclayout;
 
 	if (!c->mtd->oobsize)
 		return 0;

include/linux/mtd/inftl.h +1 -1

index 64ee53c..9e3a43e 100644
@@ -44,7 +44,7 @@ struct INFTLrecord {
         unsigned int nb_blocks;		/* number of physical blocks */
         unsigned int nb_boot_blocks;	/* number of blocks used by the bios */
         struct erase_info instr;
-        struct nand_ecclayout oobinfo;
+	struct nand_ecclayout_l oobinfo;
 };
 
 int INFTL_mount(struct INFTLrecord *s);

include/linux/mtd/mtd.h +16 -1

index 8485e42..33fb765 100644
@@ -110,6 +110,21 @@ struct mtd_oob_ops {
 	uint8_t		*oobbuf;
 };
 
+#define MTD_MAX_OOBFREE_ENTRIES_LARGE	32
+#define MTD_MAX_ECCPOS_ENTRIES_LARGE	448
+#define MTD_MAX_ECCPOS_ENTRIES_OLD	64	/* Previous maximum */
+/*
+ * Correct ECC layout control structure. This replaces old nand_ecclayout
+ * that is exported via ECCGETLAYOUT ioctll. It should be expandable in the
+ * future simply by the above macros.
+ */
+struct nand_ecclayout_l {
+	__u32 eccbytes;
+	__u32 eccpos[MTD_MAX_ECCPOS_ENTRIES_LARGE];
+	__u32 oobavail;
+	struct nand_oobfree oobfree[MTD_MAX_OOBFREE_ENTRIES_LARGE];
+};
+
 struct mtd_info {
 	u_char type;
 	uint32_t flags;

@@ -147,7 +162,7 @@ struct mtd_info {
 	int index;
 
 	/* ecc layout structure pointer - read only ! */
-	struct nand_ecclayout *ecclayout;
+	struct nand_ecclayout_l *ecclayout;
 
 	/* Data for variable erase regions. If numeraseregions is zero,
 	 * it means that the whole device has erasesize as given above.

include/linux/mtd/nand.h +3 -3

index 102e12c..0cb1bc9 100644
@@ -270,7 +270,7 @@ struct nand_ecc_ctrl {
 	int			total;
 	int			prepad;
 	int			postpad;
-	struct nand_ecclayout	*layout;
+	struct nand_ecclayout_l	*layout;
 	void			(*hwctl)(struct mtd_info *mtd, int mode);
 	int			(*calculate)(struct mtd_info *mtd,
 					     const uint8_t *dat,

@@ -415,7 +415,7 @@ struct nand_chip {
 
 	uint8_t		*oob_poi;
 	struct nand_hw_control  *controller;
-	struct nand_ecclayout	*ecclayout;
+	struct nand_ecclayout_l	*ecclayout;
 
 	struct nand_ecc_ctrl ecc;
 	struct nand_buffers *buffers;

@@ -506,7 +506,7 @@ struct platform_nand_chip {
 	int			chip_offset;
 	int			nr_partitions;
 	struct mtd_partition	*partitions;
-	struct nand_ecclayout	*ecclayout;
+	struct nand_ecclayout_l	*ecclayout;
 	int			chip_delay;
 	unsigned int		options;
 	const char		**part_probe_types;

include/linux/mtd/nftl.h +1 -1

index b059629..51214dd 100644
@@ -50,7 +50,7 @@ struct NFTLrecord {
         unsigned int nb_blocks;		/* number of physical blocks */
         unsigned int nb_boot_blocks;	/* number of blocks used by the bios */
         struct erase_info instr;
-	struct nand_ecclayout oobinfo;
+	struct nand_ecclayout_l oobinfo;
 };
 
 int NFTL_mount(struct NFTLrecord *s);

include/linux/mtd/onenand.h +1 -1

index 0c8815b..c946ec6 100644
@@ -132,7 +132,7 @@ struct onenand_chip {
 #endif
 
 	int			subpagesize;
-	struct nand_ecclayout	*ecclayout;
+	struct nand_ecclayout_l	*ecclayout;
 
 	void			*bbm;

include/linux/mtd/partitions.h +1 -1

index 274b619..ae390d5 100644
@@ -39,7 +39,7 @@ struct mtd_partition {
 	uint64_t size;			/* partition size */
 	uint64_t offset;		/* offset within the master MTD space */
 	uint32_t mask_flags;		/* master MTD flags to mask out for this partition */
-	struct nand_ecclayout *ecclayout;	/* out of band layout for this partition (NAND only)*/
+	struct nand_ecclayout_l *ecclayout;	/* out of band layout for this partition (NAND only) */
 };
 
 #define MTDPART_OFS_NXTBLK	(-2)

include/linux/mtd/sharpsl.h +1 -1

index 25f4d2a..15fa381 100644
@@ -14,7 +14,7 @@
 
 struct sharpsl_nand_platform_data {
 	struct nand_bbt_descr	*badblock_pattern;
-	struct nand_ecclayout	*ecc_layout;
+	struct nand_ecclayout_l	*ecc_layout;
 	struct mtd_partition	*partitions;
 	unsigned int		nr_partitions;
 };

Signature · 1 line

1.7.0.4

Shinya Kuribayashi

2010-08-20 00:50 UTC #5 lore raw

Re: [PATCH v2 0/2] Deprecate ECCGETLAYOUT

In reply to Brian Norris

Hi,

[PATCH] mtd: Expand nand_ecc_layout, deprecate ioctl ECCGETLAYOUT

[PATCH v2 0/2] Deprecate ECCGETLAYOUT
[PATCH v2 1/2] mtd: nand: Expand nand_ecc_layout, deprecate ioctl ECCGETLAYOUT
[PATCH v2 2/2] mtd: nand: Expand nand_ecc_layout, deprecate ioctl ECCGETLAYOUT

[PATCH v3 1/2] mtd: Expand nand_ecc_layout, deprecate ioctl ECCGETLAYOUT
[PATCH v3 2/2] (I can't find this in my inbox, by the way)

I'd like to see the info about what's updated in v2, v3 when revised.

By the way.

Quoted 3 lines · Brian Norris

Brian Norris (2):
mtd: nand: Expand nand_ecc_layout, deprecate ioctl ECCGETLAYOUT
mtd: nand: Expand nand_ecc_layout, deprecate ioctl ECCGETLAYOUT

Two patches with the same title. Similar symptom can be seen in your latest v3 post (we can't find [PATCH v3 2/2]). Perhaps something weird is in your repository (or git settings).

Signature · 2 lines

Shinya Kuribayashi
Renesas Electronics

Brian Norris

author

2010-08-20 15:16 UTC #6 lore raw

Re: [PATCH v2 0/2] Deprecate ECCGETLAYOUT

Hi,

Sorry for the problems. I will try to explain.

Quoted 2 lines · Shinya Kuribayashi

[PATCH v3 1/2] mtd: Expand nand_ecc_layout, deprecate ioctl ECCGETLAYOUT
[PATCH v3 2/2] (I can't find this in my inbox, by the way)

There was no PATCH v3 2/2. See below.

Quoted 1 line

I'd like to see the info about what's updated in v2, v3 when revised.

The only difference is that I actually sent the correct patch :) Again, I explain below.

Quoted 10 lines

By the way.
Quoted 3 lines · Brian Norris
Brian Norris (2):
mtd: nand: Expand nand_ecc_layout, deprecate ioctl ECCGETLAYOUT
mtd: nand: Expand nand_ecc_layout, deprecate ioctl ECCGETLAYOUT
Two patches with the same title. Similar symptom can be seen in your latest v3 post (we can't find [PATCH v3 2/2]). Perhaps something weird is in your repository (or git settings).

No problem with git, just my inability to express my "2 independent patch" problem properly.

First, I sent my original patch 08/06/2010:
[PATCH] mtd: Expand nand_ecc_layout, deprecate ioctl ECCGETLAYOUT

Realizing this may not necessarily have been the *best* way to accomplish my goal, I sent v2:
[PATCH v2 1/2] mtd: nand: Expand...
[PATCH v2 2/2] mtd: nand: Expand...

These two patches were supposed to be two different, independent approaches to the same task; with discussion over how best to do this, we should settle on one of the two patches. I decided (for good or for bad) that they didn't need a new title - they were essentially the same code with a slight twist. However, instead of sending two different patches, I sent the same one twice (my mistake).

Finally, I sent v3 to include just the missing patch:
[PATCH v3 1/2] mtd: Expand...

I only included the missing patch and called it v3.

So this leaves the important patch set as the following two independent, slightly different approaches to the same problem; we should choose between them:
[PATCH v3 1/2] mtd: Expand...
[PATCH v2 2/2] mtd: nand: Expand...

Sorry if my naming, numbering, and failure of intelligence in sending e-mail confused anyone. I can resend with distinct names and correct numbering if requested.

Thanks,
Brian

Shinya Kuribayashi

2010-08-23 04:12 UTC #7 lore raw

Re: [PATCH v2 0/2] Deprecate ECCGETLAYOUT

On 8/21/2010 12:15 AM, Brian Norris wrote:
[ ... ]

Quoted 9 lines

So this leaves the important patch set as the following two independent, slightly different approaches to the same problem; we should choose between them:
[PATCH v3 1/2] mtd: Expand...
[PATCH v2 2/2] mtd: nand: Expand...
Sorry if my naming, numbering, and failure of intelligence in sending e-mail confused anyone. I can resend with distinct names and correct numbering if requested.

Thanks for the explanation, got it. I'm following that thread and already know what's discussed and proposed, but couldn't follow the changes made between numbered / revised patches with similar subjects.

Signature · 2 lines

Shinya Kuribayashi
Renesas Electronics

Artem Bityutskiy

2010-08-24 10:46 UTC #8 lore raw

Re: [PATCH v2 0/2] Deprecate ECCGETLAYOUT

In reply to Brian Norris

Quoted 5 lines · Brian Norris

So this leaves the important patch set as the following two independent, slightly different approaches to the same problem; we should choose between them:
[PATCH v3 1/2] mtd: Expand...
[PATCH v2 2/2] mtd: nand: Expand...

Could you please re-send up-to date (against current mtd-2.6.git tree) patches which implement your strategy #2? I'd add them to my l2 tree.

Signature · 2 lines

Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Brian Norris

2010-08-25 01:12 UTC #9 lore raw

PATCH v4 1/1 mtd: nand: Expand nand_ecc_layout, deprecate ioctl ECCGETLAYOUT

Hello,

My e-mail address has changed, since I am no longer working at Broadcom. I will still be able to track messages to my old account if the MTD mailing list is CC'd.

Quoted 10 lines · Artem Bityutskiy

Quoted 5 lines · Brian Norris
So this leaves the important patch set as the following two independent, slightly different approaches to the same problem; we should choose between them:
[PATCH v3 1/2] mtd: Expand...
[PATCH v2 2/2] mtd: nand: Expand...
Could you please re-send up-to date (against current mtd-2.6.git tree) patches which implement your strategy #2? I'd add them to my l2 tree.

Sure; ditch the first strategy, and go with this "v4" patch. It is (going forward) the *only* patch (no PATCH 1/2, 2/2 nonsense). AFAIK this is against the latest mtd-2.6.git and has no major changes from previous "versions".

Note that on the same subject (different thread) David suggested our new struct be allocated dynamically:

Quoted 2 lines · David Woodhouse

Perhaps these buffers should be dynamically allocated and we should ditch the MAX macros altogether?

For the moment, I think that statically declared sizes are the best solution mainly for the following reason: they don't require major changes to other drivers. If we dynamically allocate these, then many drivers will probably need a whole to adapt considerably.

Besides, I don't have the time now to work on a whole new dynamic approach. Perhaps I can address this in the future, or it can be left to someone else.

---------------------------------------------------------------------------

Instead, the old nand_ecclayout is renamed to nand_ecclayout_user and a new struct nand_ecclayout is created that can accomodate larger sizes and expand without affecting the user-space. struct nand_ecclayout can still be used in board drivers without modification -- at least for now.

A new function is provided to convert from the new to the old in order to allow the deprecated ioctl to continue to work with truncated data. Perhaps the ioctl, the conversion process, and the struct nand_ecclayout_user can be removed altogether in the future.

Note: There are comments in nand/davinci_nand.c::nand_davinci_probe() regarding this issue; this driver (and maybe others) can be updated to account for extra space. All kernel drivers can use the expanded nand_ecclayout as a drop-in replacement and ignore its benefits.

Signed-off-by Brian Norris <[email protected]>

---
 drivers/mtd/mtdchar.c           |   48 ++++++++++++++++++++++++++++++++++++--
 drivers/mtd/nand/davinci_nand.c |    3 ++
 include/linux/mtd/mtd.h         |   15 ++++++++++++
 include/linux/mtd/partitions.h  |    2 +-
 include/mtd/mtd-abi.h           |    4 +-
 include/mtd/mtd-user.h          |    2 +-
 6 files changed, 67 insertions(+), 7 deletions(-)

drivers/mtd/mtdchar.c +45 -3

index a825002..24d35ba 100644
@@ -477,6 +477,39 @@ static int mtd_do_readoob(struct mtd_info *mtd, uint64_t start,
 	return ret;
 }
 
+/*
+ * Copies (and truncates, if necessary) data from the larger struct,
+ * nand_ecclayout, to the smaller, deprecated layout struct,
+ * nand_ecclayout_user. This is necessary only to suppport the deprecated
+ * API ioctl ECCGETLAYOUT while allowing all new functionality to use
+ * nand_ecclayout flexibly (i.e. the struct may change size in new
+ * releases without requiring major rewrites).
+ */
+static int shrink_ecclayout(const struct nand_ecclayout *from,
+		struct nand_ecclayout_user *to)
+{
+	int i;
+
+	if (!from || !to)
+		return -EINVAL;
+
+	memset(to, 0, sizeof(*to));
+
+	to->eccbytes = min((int)from->eccbytes, MTD_MAX_ECCPOS_ENTRIES_OLD);
+	for (i = 0; i < to->eccbytes; i++)
+		to->eccpos[i] = from->eccpos[i];
+
+	for (i = 0; i < MTD_MAX_OOBFREE_ENTRIES; i++) {
+		if (from->oobfree[i].length == 0 &&
+				from->oobfree[i].offset == 0)
+			break;
+		to->oobavail += from->oobfree[i].length;
+		to->oobfree[i] = from->oobfree[i];
+	}
+
+	return 0;
+}
+
 static int mtd_ioctl(struct file *file, u_int cmd, u_long arg)
 {
 	struct mtd_file_info *mfi = file->private_data;

@@ -812,14 +845,23 @@ static int mtd_ioctl(struct file *file, u_int cmd, u_long arg)
 	}
 #endif
 
+	/* This ioctl is being deprecated - it truncates the ecc layout */
 	case ECCGETLAYOUT:
 	{
+		struct nand_ecclayout_user *usrlay;
+
 		if (!mtd->ecclayout)
 			return -EOPNOTSUPP;
 
-		if (copy_to_user(argp, mtd->ecclayout,
-				 sizeof(struct nand_ecclayout)))
-			return -EFAULT;
+		usrlay = kmalloc(sizeof(*usrlay), GFP_KERNEL);
+		if (!usrlay)
+			return -ENOMEM;
+
+		shrink_ecclayout(mtd->ecclayout, usrlay);
+
+		if (copy_to_user(argp, usrlay, sizeof(*usrlay)))
+			ret = -EFAULT;
+		kfree(usrlay);
 		break;
 	}

drivers/mtd/nand/davinci_nand.c +3 -0

index 2ac7367..70698e8 100644
@@ -749,6 +749,9 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
 		 * breaks userspace ioctl interface with mtd-utils. Once we
 		 * resolve this issue, NAND_ECC_HW_OOB_FIRST mode can be used
 		 * for the 4KiB page chips.
+		 *
+		 * TODO: Note that nand_ecclayout has now been expanded and can
+		 *  hold plenty of OOB entries.
 		 */
 		dev_warn(&pdev->dev, "no 4-bit ECC support yet "
 				"for 4KiB-page NAND\n");

include/linux/mtd/mtd.h +15 -0

index 8485e42..03a1e95 100644
@@ -110,6 +110,21 @@ struct mtd_oob_ops {
 	uint8_t		*oobbuf;
 };
 
+#define MTD_MAX_OOBFREE_ENTRIES_LARGE	32
+#define MTD_MAX_ECCPOS_ENTRIES_LARGE	448
+#define MTD_MAX_ECCPOS_ENTRIES_OLD	64	/* Previous maximum */
+/*
+ * Correct ECC layout control structure. This replaces old nand_ecclayout
+ * (mtd-abi.h) that is exported via ECCGETLAYOUT ioctl. It should be expandable
+ *  in the future simply by the above macros.
+ */
+struct nand_ecclayout {
+	__u32 eccbytes;
+	__u32 eccpos[MTD_MAX_ECCPOS_ENTRIES_LARGE];
+	__u32 oobavail;
+	struct nand_oobfree oobfree[MTD_MAX_OOBFREE_ENTRIES_LARGE];
+};
+
 struct mtd_info {
 	u_char type;
 	uint32_t flags;

include/linux/mtd/partitions.h +1 -1

index 274b619..930c8ac 100644
@@ -39,7 +39,7 @@ struct mtd_partition {
 	uint64_t size;			/* partition size */
 	uint64_t offset;		/* offset within the master MTD space */
 	uint32_t mask_flags;		/* master MTD flags to mask out for this partition */
-	struct nand_ecclayout *ecclayout;	/* out of band layout for this partition (NAND only)*/
+	struct nand_ecclayout *ecclayout;	/* out of band layout for this partition (NAND only) */
 };
 
 #define MTDPART_OFS_NXTBLK	(-2)

include/mtd/mtd-abi.h +2 -2

index 4debb45..5bce083 100644
@@ -119,7 +119,7 @@ struct otp_info {
 #define OTPGETREGIONCOUNT	_IOW('M', 14, int)
 #define OTPGETREGIONINFO	_IOW('M', 15, struct otp_info)
 #define OTPLOCK			_IOR('M', 16, struct otp_info)
-#define ECCGETLAYOUT		_IOR('M', 17, struct nand_ecclayout)
+#define ECCGETLAYOUT		_IOR('M', 17, struct nand_ecclayout_user)
 #define ECCGETSTATS		_IOR('M', 18, struct mtd_ecc_stats)
 #define MTDFILEMODE		_IO('M', 19)
 #define MEMERASE64		_IOW('M', 20, struct erase_info_user64)

@@ -148,7 +148,7 @@ struct nand_oobfree {
  * ECC layout control structure. Exported to userspace for
  * diagnosis and to allow creation of raw images
  */
-struct nand_ecclayout {
+struct nand_ecclayout_user {
 	__u32 eccbytes;
 	__u32 eccpos[64];
 	__u32 oobavail;

include/mtd/mtd-user.h +1 -1

index aa3c2f8..83327c8 100644
@@ -29,6 +29,6 @@ typedef struct mtd_info_user mtd_info_t;
 typedef struct erase_info_user erase_info_t;
 typedef struct region_info_user region_info_t;
 typedef struct nand_oobinfo nand_oobinfo_t;
-typedef struct nand_ecclayout nand_ecclayout_t;
+typedef struct nand_ecclayout_user nand_ecclayout_t;
 
 #endif /* __MTD_USER_H__ */

Signature · 1 line

1.7.0.4

Artem Bityutskiy

2010-08-30 10:21 UTC #10 lore raw

Re: [PATCH v4] mtd: nand: Expand nand_ecc_layout, deprecate ioctl ECCGETLAYOUT

Quoted 3 lines · Brian Norris

My e-mail address has changed, since I am no longer working at Broadcom. I will still be able to track messages to my old account if the MTD mailing list is CC'd.

Oh, does it mean you will stop loving MTD and we won't see steady flow of improvements for you? :-( BTW, I think you have been doing great job
- MTD subsystem needs love badly!

Quoted 2 lines

Note that on the same subject (different thread) David suggested our new struct be allocated dynamically:

Yes, but I agree with your arguments and also think it is ok to keep it simple for now. So I'm applying this to my l2 tree, and then it is up to dwmw2 to take it or not.

But I also have some requests about commentaries, so if you can re-send another version of this patch, it would be nice. But I take this one for now, it is good enough.

Quoted 8 lines

+/*
+ * Copies (and truncates, if necessary) data from the larger struct,
+ * nand_ecclayout, to the smaller, deprecated layout struct,
+ * nand_ecclayout_user. This is necessary only to suppport the deprecated
+ * API ioctl ECCGETLAYOUT while allowing all new functionality to use
+ * nand_ecclayout flexibly (i.e. the struct may change size in new
+ * releases without requiring major rewrites).
+ */

I think a similar comment should exist in linux/mtd/mtd.h. Indeed, that file is our API with user-space, and our users will probably look at it, and it is nice to document the situation with 'struct nand_ecclayout_user' there.

include/linux/mtd/mtd.h struct mtd_oob_ops { · line 120

Quoted 8 lines

include/linux/mtd/mtd.h +8 -0

+#define MTD_MAX_OOBFREE_ENTRIES_LARGE	32
+#define MTD_MAX_ECCPOS_ENTRIES_LARGE	448
+#define MTD_MAX_ECCPOS_ENTRIES_OLD	64	/* Previous maximum */
+/*
+ * Correct ECC layout control structure. This replaces old nand_ecclayout
+ * (mtd-abi.h) that is exported via ECCGETLAYOUT ioctl. It should be expandable
+ *  in the future simply by the above macros.
+ */

I find this comment confusing. First, "Correct ECC" -> "Internal ECC", because one could think "Correct ECC structure" means something like "structure which describes ECC corrections" or something like that.

Also, I'd avoid mentioning things like "old nand_ecclayout", because with time this will be confusing. Could you please imagine that you are an MTD newbie reading the code in 2012 - you have no idea what was happening in the past in the ancient 2010.

Thanks!

Signature · 2 lines

Best Regards,
Artem Bityutskiy (Битюцкий Артём)

Artem Bityutskiy

2010-09-18 17:24 UTC #11 lore raw

Re: [PATCH v4] mtd: nand: Expand nand_ecc_layout, deprecate ioctl ECCGETLAYOUT

Quoted 47 lines · Artem Bityutskiy

Quoted 3 lines · Brian Norris
My e-mail address has changed, since I am no longer working at Broadcom. I will still be able to track messages to my old account if the MTD mailing list is CC'd.
Oh, does it mean you will stop loving MTD and we won't see steady flow of improvements for you? :-( BTW, I think you have been doing great job
- MTD subsystem needs love badly!
Quoted 2 lines
Note that on the same subject (different thread) David suggested our new struct be allocated dynamically:
Yes, but I agree with your arguments and also think it is ok to keep it simple for now. So I'm applying this to my l2 tree, and then it is up to dwmw2 to take it or not.
But I also have some requests about commentaries, so if you can re-send another version of this patch, it would be nice. But I take this one for now, it is good enough.
Quoted 8 lines
+/*
+ * Copies (and truncates, if necessary) data from the larger struct,
+ * nand_ecclayout, to the smaller, deprecated layout struct,
+ * nand_ecclayout_user. This is necessary only to suppport the deprecated
+ * API ioctl ECCGETLAYOUT while allowing all new functionality to use
+ * nand_ecclayout flexibly (i.e. the struct may change size in new
+ * releases without requiring major rewrites).
+ */
I think a similar comment should exist in linux/mtd/mtd.h. Indeed, that file is our API with user-space, and our users will probably look at it, and it is nice to document the situation with 'struct nand_ecclayout_user' there.
include/linux/mtd/mtd.h struct mtd_oob_ops { · line 120
Quoted 8 lines
include/linux/mtd/mtd.h +8 -0
+#define MTD_MAX_OOBFREE_ENTRIES_LARGE	32
+#define MTD_MAX_ECCPOS_ENTRIES_LARGE	448
+#define MTD_MAX_ECCPOS_ENTRIES_OLD	64	/* Previous maximum */
+/*
+ * Correct ECC layout control structure. This replaces old nand_ecclayout
+ * (mtd-abi.h) that is exported via ECCGETLAYOUT ioctl. It should be expandable
+ *  in the future simply by the above macros.
+ */
I find this comment confusing. First, "Correct ECC" -> "Internal ECC", because one could think "Correct ECC structure" means something like "structure which describes ECC corrections" or something like that.

Quoted 4 lines

Also, I'd avoid mentioning things like "old nand_ecclayout", because with time this will be confusing. Could you please imagine that you are an MTD newbie reading the code in 2012 - you have no idea what was happening in the past in the ancient 2010.

Brian,

would you address the small things I noticed in a follow-up patch?

Signature · 2 lines

Best Regards,
Artem Bityutskiy (Битюцкий Артём)

Brian Norris

2010-09-18 20:04 UTC #12 lore raw

Re: [PATCH v4] mtd: nand: Expand nand_ecc_layout, deprecate ioctl ECCGETLAYOUT

Quoted 3 lines · Artem Bityutskiy

Brian,
would you address the small things I noticed in a follow-up patch?

Artem,

Yes, I will. I'm sorry for the delay; I've been a little busy and putting some things off. Would you like a patch for the comments against your l2 tree or try and squash it with my previous commit and base it off of mtd-2.6 or some other option?

Brian

Artem Bityutskiy

2010-09-19 07:17 UTC #13 lore raw

Re: [PATCH v4] mtd: nand: Expand nand_ecc_layout, deprecate ioctl ECCGETLAYOUT

Quoted 11 lines · Brian Norris

Quoted 3 lines · Artem Bityutskiy
Brian,
would you address the small things I noticed in a follow-up patch?
Artem,
Yes, I will. I'm sorry for the delay; I've been a little busy and putting some things off. Would you like a patch for the comments against your l2 tree or try and squash it with my previous commit and base it off of mtd-2.6 or some other option?

No, against l2 tree is ok, I'll put it on top, thanks!

Signature · 2 lines

Best Regards,
Artem Bityutskiy (Битюцкий Артём)

Brian Norris

2010-09-20 06:28 UTC #14 lore raw

Re: [PATCH v4] mtd: nand: Expand nand_ecc_layout, deprecate ioctl ECCGETLAYOUT

In reply to Artem Bityutskiy

Hello,

Now that I've finally gotten around to working on this, I have questions and comments.

Quoted 9 lines · Artem Bityutskiy

Quoted 8 lines · Artem Bityutskiy
Quoted 3 lines · Brian Norris
My e-mail address has changed, since I am no longer working at Broadcom. I will still be able to track messages to my old account if the MTD mailing list is CC'd.
Oh, does it mean you will stop loving MTD and we won't see steady flow of improvements for you? :-( BTW, I think you have been doing great job
- MTD subsystem needs love badly!

No, this doesn't mean I will stop loving MTD, although as may be readily apparent by my delay, I may work a bit slower :) In fact, I will probably be picking up some more work shortly.

Quoted 9 lines

Quoted 9 lines
Quoted 8 lines
+/*
+ * Copies (and truncates, if necessary) data from the larger struct,
+ * nand_ecclayout, to the smaller, deprecated layout struct,
+ * nand_ecclayout_user. This is necessary only to suppport the deprecated
+ * API ioctl ECCGETLAYOUT while allowing all new functionality to use
+ * nand_ecclayout flexibly (i.e. the struct may change size in new
+ * releases without requiring major rewrites).
+ */

Quoted 6 lines

Quoted 4 lines
I think a similar comment should exist in linux/mtd/mtd.h. Indeed, that file is our API with user-space, and our users will probably look at it, and it is nice to document the situation with 'struct nand_ecclayout_user' there.
From the context, I assume you meant include/mtd/mtd-abi.h, not

linux/mtd/mtd.h; am I correct?

I will send the patch out shortly under the assumption that I am correct.

Brian

Brian Norris

2010-09-20 06:57 UTC #15 lore raw

PATCH 1/1 mtd: Edit comments on deprecation of ioctl ECCGETLAYOUT

** Applies to l2-mtd-2.6.git: master **

There were some improvements and additions necessary in the comments explaining of the expansion of nand_ecclayout, the introduction of nand_ecclayout_user, and the deprecation of the ioctl ECCGETLAYOUT.

Also, I found a better placement for the macro MTD_MAX_ECCPOS_ENTRIES; next to the definition of MTD_MAX_OOBFREE_ENTRIES in mtd-abi.h. The macro is really only important for the ioctl code (found in drivers/mtd/mtdchar.c) but since there are small edits being made to the user-space header, I figured this is a better location.

Signed-off-by Brian Norris <[email protected]>

---
 drivers/mtd/mtdchar.c   |    2 +-
 include/linux/mtd/mtd.h |    8 ++++----
 include/mtd/mtd-abi.h   |   11 ++++++++---
 3 files changed, 13 insertions(+), 8 deletions(-)

drivers/mtd/mtdchar.c +1 -1

index 44868cf..5895de7 100644
@@ -495,7 +495,7 @@ static int shrink_ecclayout(const struct nand_ecclayout *from,
 
 	memset(to, 0, sizeof(*to));
 
-	to->eccbytes = min((int)from->eccbytes, MTD_MAX_ECCPOS_ENTRIES_OLD);
+	to->eccbytes = min((int)from->eccbytes, MTD_MAX_ECCPOS_ENTRIES);
 	for (i = 0; i < to->eccbytes; i++)
 		to->eccpos[i] = from->eccpos[i];

include/linux/mtd/mtd.h +4 -4

index 03a1e95..fe8d77e 100644
@@ -112,11 +112,11 @@ struct mtd_oob_ops {
 
 #define MTD_MAX_OOBFREE_ENTRIES_LARGE	32
 #define MTD_MAX_ECCPOS_ENTRIES_LARGE	448
-#define MTD_MAX_ECCPOS_ENTRIES_OLD	64	/* Previous maximum */
 /*
- * Correct ECC layout control structure. This replaces old nand_ecclayout
- * (mtd-abi.h) that is exported via ECCGETLAYOUT ioctl. It should be expandable
- *  in the future simply by the above macros.
+ * Internal ECC layout control structure. For historical reasons, there is a
+ * similar, smaller struct nand_ecclayout_user (in mtd-abi.h) that is retained
+ * for export to user-space via the ECCGETLAYOUT ioctl.
+ * nand_ecclayout should be expandable in the future simply by the above macros.
  */
 struct nand_ecclayout {
 	__u32 eccbytes;

include/mtd/mtd-abi.h +8 -3

index 5bce083..a57c4cb 100644
@@ -144,13 +144,18 @@ struct nand_oobfree {
 };
 
 #define MTD_MAX_OOBFREE_ENTRIES	8
+#define MTD_MAX_ECCPOS_ENTRIES	64
 /*
- * ECC layout control structure. Exported to userspace for
- * diagnosis and to allow creation of raw images
+ * OBSOLETE: ECC layout control structure. Exported to user-space via ioctl
+ * ECCGETLAYOUT for backwards compatbility and should not be mistaken as a
+ * complete set of ECC information. The ioctl truncates the larger internal
+ * structure to retain binary compatibility with the static declaration of the
+ * ioctl. Note that the "MTD_MAX_..._ENTRIES" macros represent the max size of
+ * the user struct, not the MAX size of the internal struct nand_ecclayout.
  */
 struct nand_ecclayout_user {
 	__u32 eccbytes;
-	__u32 eccpos[64];
+	__u32 eccpos[MTD_MAX_ECCPOS_ENTRIES];
 	__u32 oobavail;
 	struct nand_oobfree oobfree[MTD_MAX_OOBFREE_ENTRIES];
 };

Signature · 1 line

1.7.0.4

Artem Bityutskiy

Applied

2010-09-20 08:51 UTC #16 lore raw

Re: [PATCH] mtd: Edit comments on deprecation of ioctl ECCGETLAYOUT

Quoted 7 lines · Brian Norris

** Applies to l2-mtd-2.6.git: master **
There were some improvements and additions necessary in the comments explaining of the expansion of nand_ecclayout, the introduction of nand_ecclayout_user, and the deprecation of the ioctl ECCGETLAYOUT.

Quoted 7 lines

Also, I found a better placement for the macro MTD_MAX_ECCPOS_ENTRIES; next to the definition of MTD_MAX_OOBFREE_ENTRIES in mtd-abi.h. The macro is really only important for the ioctl code (found in drivers/mtd/mtdchar.c) but since there are small edits being made to the user-space header, I figured this is a better location.
Signed-off-by Brian Norris <[email protected]>

Pushed to l2-mtd-2.6.git / master, thanks.

Note, I rebased the l2 tree an put this patch next to the "parent" patch:

commit 728d7eef6c0ff50cb9b327ff7c02830c707f2bbc

Author: Brian Norris <[email protected]>
Date:   Tue Aug 24 18:12:00 2010 -0700

mtd: nand: expand nand_ecc_layout, deprecate ioctl ECCGETLAYOUT

This is just more logical to keep them together.

Thanks!

Signature · 2 lines

Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Artem Bityutskiy

2010-09-20 07:54 UTC #17 lore raw

Re: [PATCH v4] mtd: nand: Expand nand_ecc_layout, deprecate ioctl ECCGETLAYOUT

In reply to Brian Norris

Quoted 7 lines · Brian Norris

Quoted 3 lines
Quoted 3 lines
Oh, does it mean you will stop loving MTD and we won't see steady flow of improvements for you? :-( BTW, I think you have been doing great job
- MTD subsystem needs love badly!
No, this doesn't mean I will stop loving MTD, although as may be readily apparent by my delay, I may work a bit slower :) In fact, I will probably be picking up some more work shortly.

Nice :-)

Quoted 2 lines

From the context, I assume you meant include/mtd/mtd-abi.h, not linux/mtd/mtd.h; am I correct?

Probably yes :-)

Signature · 2 lines

Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Artem Bityutskiy

2010-08-24 10:43 UTC #18 lore raw

Re: [PATCH v2 0/2] Deprecate ECCGETLAYOUT

In reply to Brian Norris

Quoted 10 lines · Brian Norris

The following two patches are implementations of two different strategies to deprecate the ioctl ECCGETLAYOUT. This ioctl is not very useful and imposes restrictions on expanding the corresponding kernel struct nand_ecclayout. Either patch will leave functionality intact but allow the kernel-space layouts to expand in the future.
Strategy 1: Create new struct nand_ecclayout_l and convert all kernel
code to use this struct.
Strategy 2: Rename old struct nand_ecclayout_user and modify
nand_ecclayout to be used in the kernel only.

I think nand_ecclayout_l is silly name. Strategy 2 looks nicer, let's stick to that one.

Signature · 2 lines

Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Brian Norris

author

2010-08-18 18:07 UTC #19 lore raw

PATCH v2 1/2 mtd: nand: Expand nand_ecc_layout, deprecate ioctl ECCGETLAYOUT

In reply to Brian Norris

This is strategy 2 of 2 strategies proposed for this deprecation.

Instead, the old nand_ecclayout is renamed to nand_ecclayout_user and a new struct nand_ecclayout is created that can accomodate larger sizes and expand without affecting the user-space.

Signed-off-by Brian Norris <[email protected]>

---
 drivers/mtd/mtdchar.c           |   48 ++++++++++++++++++++++++++++++++++++--
 drivers/mtd/nand/davinci_nand.c |    3 ++
 include/linux/mtd/mtd.h         |   15 ++++++++++++
 include/linux/mtd/partitions.h  |    2 +-
 include/mtd/mtd-abi.h           |    4 +-
 include/mtd/mtd-user.h          |    2 +-
 6 files changed, 67 insertions(+), 7 deletions(-)

drivers/mtd/mtdchar.c +45 -3

index a825002..24d35ba 100644
@@ -477,6 +477,39 @@ static int mtd_do_readoob(struct mtd_info *mtd, uint64_t start,
 	return ret;
 }
 
+/*
+ * Copies (and truncates, if necessary) data from the larger struct,
+ * nand_ecclayout, to the smaller, deprecated layout struct,
+ * nand_ecclayout_user. This is necessary only to suppport the deprecated
+ * API ioctl ECCGETLAYOUT while allowing all new functionality to use
+ * nand_ecclayout flexibly (i.e. the struct may change size in new
+ * releases without requiring major rewrites).
+ */
+static int shrink_ecclayout(const struct nand_ecclayout *from,
+		struct nand_ecclayout_user *to)
+{
+	int i;
+
+	if (!from || !to)
+		return -EINVAL;
+
+	memset(to, 0, sizeof(*to));
+
+	to->eccbytes = min((int)from->eccbytes, MTD_MAX_ECCPOS_ENTRIES_OLD);
+	for (i = 0; i < to->eccbytes; i++)
+		to->eccpos[i] = from->eccpos[i];
+
+	for (i = 0; i < MTD_MAX_OOBFREE_ENTRIES; i++) {
+		if (from->oobfree[i].length == 0 &&
+				from->oobfree[i].offset == 0)
+			break;
+		to->oobavail += from->oobfree[i].length;
+		to->oobfree[i] = from->oobfree[i];
+	}
+
+	return 0;
+}
+
 static int mtd_ioctl(struct file *file, u_int cmd, u_long arg)
 {
 	struct mtd_file_info *mfi = file->private_data;

@@ -812,14 +845,23 @@ static int mtd_ioctl(struct file *file, u_int cmd, u_long arg)
 	}
 #endif
 
+	/* This ioctl is being deprecated - it truncates the ecc layout */
 	case ECCGETLAYOUT:
 	{
+		struct nand_ecclayout_user *usrlay;
+
 		if (!mtd->ecclayout)
 			return -EOPNOTSUPP;
 
-		if (copy_to_user(argp, mtd->ecclayout,
-				 sizeof(struct nand_ecclayout)))
-			return -EFAULT;
+		usrlay = kmalloc(sizeof(*usrlay), GFP_KERNEL);
+		if (!usrlay)
+			return -ENOMEM;
+
+		shrink_ecclayout(mtd->ecclayout, usrlay);
+
+		if (copy_to_user(argp, usrlay, sizeof(*usrlay)))
+			ret = -EFAULT;
+		kfree(usrlay);
 		break;
 	}

drivers/mtd/nand/davinci_nand.c +3 -0

index 2ac7367..70698e8 100644
@@ -749,6 +749,9 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
 		 * breaks userspace ioctl interface with mtd-utils. Once we
 		 * resolve this issue, NAND_ECC_HW_OOB_FIRST mode can be used
 		 * for the 4KiB page chips.
+		 *
+		 * TODO: Note that nand_ecclayout has now been expanded and can
+		 *  hold plenty of OOB entries.
 		 */
 		dev_warn(&pdev->dev, "no 4-bit ECC support yet "
 				"for 4KiB-page NAND\n");

include/linux/mtd/mtd.h +15 -0

index 8485e42..03a1e95 100644
@@ -110,6 +110,21 @@ struct mtd_oob_ops {
 	uint8_t		*oobbuf;
 };
 
+#define MTD_MAX_OOBFREE_ENTRIES_LARGE	32
+#define MTD_MAX_ECCPOS_ENTRIES_LARGE	448
+#define MTD_MAX_ECCPOS_ENTRIES_OLD	64	/* Previous maximum */
+/*
+ * Correct ECC layout control structure. This replaces old nand_ecclayout
+ * (mtd-abi.h) that is exported via ECCGETLAYOUT ioctl. It should be expandable
+ *  in the future simply by the above macros.
+ */
+struct nand_ecclayout {
+	__u32 eccbytes;
+	__u32 eccpos[MTD_MAX_ECCPOS_ENTRIES_LARGE];
+	__u32 oobavail;
+	struct nand_oobfree oobfree[MTD_MAX_OOBFREE_ENTRIES_LARGE];
+};
+
 struct mtd_info {
 	u_char type;
 	uint32_t flags;

include/linux/mtd/partitions.h +1 -1

index 274b619..930c8ac 100644
@@ -39,7 +39,7 @@ struct mtd_partition {
 	uint64_t size;			/* partition size */
 	uint64_t offset;		/* offset within the master MTD space */
 	uint32_t mask_flags;		/* master MTD flags to mask out for this partition */
-	struct nand_ecclayout *ecclayout;	/* out of band layout for this partition (NAND only)*/
+	struct nand_ecclayout *ecclayout;	/* out of band layout for this partition (NAND only) */
 };
 
 #define MTDPART_OFS_NXTBLK	(-2)

include/mtd/mtd-abi.h +2 -2

index 4debb45..5bce083 100644
@@ -119,7 +119,7 @@ struct otp_info {
 #define OTPGETREGIONCOUNT	_IOW('M', 14, int)
 #define OTPGETREGIONINFO	_IOW('M', 15, struct otp_info)
 #define OTPLOCK			_IOR('M', 16, struct otp_info)
-#define ECCGETLAYOUT		_IOR('M', 17, struct nand_ecclayout)
+#define ECCGETLAYOUT		_IOR('M', 17, struct nand_ecclayout_user)
 #define ECCGETSTATS		_IOR('M', 18, struct mtd_ecc_stats)
 #define MTDFILEMODE		_IO('M', 19)
 #define MEMERASE64		_IOW('M', 20, struct erase_info_user64)

@@ -148,7 +148,7 @@ struct nand_oobfree {
  * ECC layout control structure. Exported to userspace for
  * diagnosis and to allow creation of raw images
  */
-struct nand_ecclayout {
+struct nand_ecclayout_user {
 	__u32 eccbytes;
 	__u32 eccpos[64];
 	__u32 oobavail;

include/mtd/mtd-user.h +1 -1

index aa3c2f8..83327c8 100644
@@ -29,6 +29,6 @@ typedef struct mtd_info_user mtd_info_t;
 typedef struct erase_info_user erase_info_t;
 typedef struct region_info_user region_info_t;
 typedef struct nand_oobinfo nand_oobinfo_t;
-typedef struct nand_ecclayout nand_ecclayout_t;
+typedef struct nand_ecclayout_user nand_ecclayout_t;
 
 #endif /* __MTD_USER_H__ */

Signature · 1 line

1.7.0.4

Brian Norris

author

2010-08-18 18:07 UTC #20 lore raw

PATCH v2 2/2 mtd: nand: Expand nand_ecc_layout, deprecate ioctl ECCGETLAYOUT

In reply to Brian Norris

This is strategy 2 of 2 strategies proposed for this deprecation.

Instead, the old nand_ecclayout is renamed to nand_ecclayout_user and a new struct nand_ecclayout is created that can accomodate larger sizes and expand without affecting the user-space.

Signed-off-by Brian Norris <[email protected]>

---
 drivers/mtd/mtdchar.c           |   48 ++++++++++++++++++++++++++++++++++++--
 drivers/mtd/nand/davinci_nand.c |    3 ++
 include/linux/mtd/mtd.h         |   15 ++++++++++++
 include/linux/mtd/partitions.h  |    2 +-
 include/mtd/mtd-abi.h           |    4 +-
 include/mtd/mtd-user.h          |    2 +-
 6 files changed, 67 insertions(+), 7 deletions(-)

drivers/mtd/mtdchar.c +45 -3

index a825002..24d35ba 100644
@@ -477,6 +477,39 @@ static int mtd_do_readoob(struct mtd_info *mtd, uint64_t start,
 	return ret;
 }
 
+/*
+ * Copies (and truncates, if necessary) data from the larger struct,
+ * nand_ecclayout, to the smaller, deprecated layout struct,
+ * nand_ecclayout_user. This is necessary only to suppport the deprecated
+ * API ioctl ECCGETLAYOUT while allowing all new functionality to use
+ * nand_ecclayout flexibly (i.e. the struct may change size in new
+ * releases without requiring major rewrites).
+ */
+static int shrink_ecclayout(const struct nand_ecclayout *from,
+		struct nand_ecclayout_user *to)
+{
+	int i;
+
+	if (!from || !to)
+		return -EINVAL;
+
+	memset(to, 0, sizeof(*to));
+
+	to->eccbytes = min((int)from->eccbytes, MTD_MAX_ECCPOS_ENTRIES_OLD);
+	for (i = 0; i < to->eccbytes; i++)
+		to->eccpos[i] = from->eccpos[i];
+
+	for (i = 0; i < MTD_MAX_OOBFREE_ENTRIES; i++) {
+		if (from->oobfree[i].length == 0 &&
+				from->oobfree[i].offset == 0)
+			break;
+		to->oobavail += from->oobfree[i].length;
+		to->oobfree[i] = from->oobfree[i];
+	}
+
+	return 0;
+}
+
 static int mtd_ioctl(struct file *file, u_int cmd, u_long arg)
 {
 	struct mtd_file_info *mfi = file->private_data;

@@ -812,14 +845,23 @@ static int mtd_ioctl(struct file *file, u_int cmd, u_long arg)
 	}
 #endif
 
+	/* This ioctl is being deprecated - it truncates the ecc layout */
 	case ECCGETLAYOUT:
 	{
+		struct nand_ecclayout_user *usrlay;
+
 		if (!mtd->ecclayout)
 			return -EOPNOTSUPP;
 
-		if (copy_to_user(argp, mtd->ecclayout,
-				 sizeof(struct nand_ecclayout)))
-			return -EFAULT;
+		usrlay = kmalloc(sizeof(*usrlay), GFP_KERNEL);
+		if (!usrlay)
+			return -ENOMEM;
+
+		shrink_ecclayout(mtd->ecclayout, usrlay);
+
+		if (copy_to_user(argp, usrlay, sizeof(*usrlay)))
+			ret = -EFAULT;
+		kfree(usrlay);
 		break;
 	}

drivers/mtd/nand/davinci_nand.c +3 -0

index 2ac7367..70698e8 100644
@@ -749,6 +749,9 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
 		 * breaks userspace ioctl interface with mtd-utils. Once we
 		 * resolve this issue, NAND_ECC_HW_OOB_FIRST mode can be used
 		 * for the 4KiB page chips.
+		 *
+		 * TODO: Note that nand_ecclayout has now been expanded and can
+		 *  hold plenty of OOB entries.
 		 */
 		dev_warn(&pdev->dev, "no 4-bit ECC support yet "
 				"for 4KiB-page NAND\n");

include/linux/mtd/mtd.h +15 -0

index 8485e42..03a1e95 100644
@@ -110,6 +110,21 @@ struct mtd_oob_ops {
 	uint8_t		*oobbuf;
 };
 
+#define MTD_MAX_OOBFREE_ENTRIES_LARGE	32
+#define MTD_MAX_ECCPOS_ENTRIES_LARGE	448
+#define MTD_MAX_ECCPOS_ENTRIES_OLD	64	/* Previous maximum */
+/*
+ * Correct ECC layout control structure. This replaces old nand_ecclayout
+ * (mtd-abi.h) that is exported via ECCGETLAYOUT ioctl. It should be expandable
+ *  in the future simply by the above macros.
+ */
+struct nand_ecclayout {
+	__u32 eccbytes;
+	__u32 eccpos[MTD_MAX_ECCPOS_ENTRIES_LARGE];
+	__u32 oobavail;
+	struct nand_oobfree oobfree[MTD_MAX_OOBFREE_ENTRIES_LARGE];
+};
+
 struct mtd_info {
 	u_char type;
 	uint32_t flags;

include/linux/mtd/partitions.h +1 -1

index 274b619..930c8ac 100644
@@ -39,7 +39,7 @@ struct mtd_partition {
 	uint64_t size;			/* partition size */
 	uint64_t offset;		/* offset within the master MTD space */
 	uint32_t mask_flags;		/* master MTD flags to mask out for this partition */
-	struct nand_ecclayout *ecclayout;	/* out of band layout for this partition (NAND only)*/
+	struct nand_ecclayout *ecclayout;	/* out of band layout for this partition (NAND only) */
 };
 
 #define MTDPART_OFS_NXTBLK	(-2)

include/mtd/mtd-abi.h +2 -2

index 4debb45..5bce083 100644
@@ -119,7 +119,7 @@ struct otp_info {
 #define OTPGETREGIONCOUNT	_IOW('M', 14, int)
 #define OTPGETREGIONINFO	_IOW('M', 15, struct otp_info)
 #define OTPLOCK			_IOR('M', 16, struct otp_info)
-#define ECCGETLAYOUT		_IOR('M', 17, struct nand_ecclayout)
+#define ECCGETLAYOUT		_IOR('M', 17, struct nand_ecclayout_user)
 #define ECCGETSTATS		_IOR('M', 18, struct mtd_ecc_stats)
 #define MTDFILEMODE		_IO('M', 19)
 #define MEMERASE64		_IOW('M', 20, struct erase_info_user64)

@@ -148,7 +148,7 @@ struct nand_oobfree {
  * ECC layout control structure. Exported to userspace for
  * diagnosis and to allow creation of raw images
  */
-struct nand_ecclayout {
+struct nand_ecclayout_user {
 	__u32 eccbytes;
 	__u32 eccpos[64];
 	__u32 oobavail;

include/mtd/mtd-user.h +1 -1

index aa3c2f8..83327c8 100644
@@ -29,6 +29,6 @@ typedef struct mtd_info_user mtd_info_t;
 typedef struct erase_info_user erase_info_t;
 typedef struct region_info_user region_info_t;
 typedef struct nand_oobinfo nand_oobinfo_t;
-typedef struct nand_ecclayout nand_ecclayout_t;
+typedef struct nand_ecclayout_user nand_ecclayout_t;
 
 #endif /* __MTD_USER_H__ */

Signature · 1 line

1.7.0.4