[PATCH 00/12] mtd: various "no ECC" and MLC NAND work

[PATCH 00/12] mtd: various "no ECC" and MLC NAND work

[Brian Norris]

Hi,

This patch series includes edited versions of several patches and RFCs -
some changed some not - as well as new material and documentation. To
avoid confusion, I will not include any version numbers yet (i.e.,
everything is "v1"), and any updates from here on out can be v2, v3,
etc. if necessary. I will try to include any important changes in the
comments though.

The key additions in this patch series are:

(1) add replaceable `{read,write}_oob_raw()' functions to
    `struct nand_chip'
(2) add a new ioctl(MEMWRITE)
(3) improve documentation

Patches for category (1) help improve support for reading/writing flash
without ECC on systems that calculate ECC on OOB operations. Category
(2) allows us to write data+OOB in a single operation; this solves
problems with MLC NAND, which can only be written once per page. And
category (3) was sorely needed for old and new functionality alike :)

Generally, these fixes have been tested by using various combinations of
nandwrite on nandsim, SLC NAND flash, and MLC NAND flash. In order to
utilize the new ioctl, I had to make some custom edits. I should have a
few patches soon to support this new stuff.

Note: the first and last patches are bugfixes to patches that are still
queued in l2-mtd-2.6.git. They probably should be squashed into the
relevant patches.

Brian

Brian Norris (12):
  mtd: nand: initialize chip->oob_poi before write
  mtd: support writing OOB without ECC
  mtd: support reading OOB without ECC
  mtd: move mtd_oob_mode_t to shared kernel/user space
  mtd: rename MTD_OOB_* to MTD_OPS_*
  mtd: rename MTD_MODE_* to MTD_FILE_MODE_*
  mtd: add MEMWRITE ioctl
  mtd: nand: document nand_chip.oob_poi
  mtd: document ABI
  mtd: nand: kill member `ops' of `struct nand_chip'
  mtd: kill old field for `struct mtd_info_user'
  mtd: nand: free allocated memory

 drivers/mtd/devices/doc2000.c          |    4 +-
 drivers/mtd/devices/doc2001.c          |    4 +-
 drivers/mtd/devices/doc2001plus.c      |    4 +-
 drivers/mtd/inftlcore.c                |    6 +-
 drivers/mtd/mtdchar.c                  |  115 ++++++++++++++++++++++++--------
 drivers/mtd/mtdpart.c                  |    2 +-
 drivers/mtd/mtdswap.c                  |    6 +-
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c |    2 +-
 drivers/mtd/nand/nand_base.c           |  107 ++++++++++++++++++-----------
 drivers/mtd/nand/nand_bbt.c            |    8 +-
 drivers/mtd/nand/sm_common.c           |    2 +-
 drivers/mtd/nftlcore.c                 |    6 +-
 drivers/mtd/onenand/onenand_base.c     |   42 ++++++------
 drivers/mtd/onenand/onenand_bbt.c      |    2 +-
 drivers/mtd/sm_ftl.c                   |    4 +-
 drivers/mtd/ssfdc.c                    |    2 +-
 drivers/mtd/tests/mtd_oobtest.c        |   24 +++---
 drivers/mtd/tests/mtd_readtest.c       |    2 +-
 drivers/staging/spectra/lld_mtd.c      |    6 +-
 fs/jffs2/wbuf.c                        |    6 +-
 include/linux/mtd/mtd.h                |   18 +-----
 include/linux/mtd/nand.h               |   12 ++-
 include/mtd/mtd-abi.h                  |  115 +++++++++++++++++++++++++++-----
 23 files changed, 329 insertions(+), 170 deletions(-)

-- 
1.7.5.4

[PATCH 01/12] mtd: nand: initialize chip->oob_poi before write

[Brian Norris]

For raw (i.e., w/o ECC) page writes (i.e., w/o OOB), we may not have
initialized and filled the chip->oob_poi buffer. This can end up writing
junk to the flash if we're not careful. Say, for example, we use
`nandwrite -n' (without OOB). Then nand_do_write_ops calls
chip->write_page, which writes OOB data with some previous, junk data.

This fixes a bug with this commit (from l2-mtd-2.6.git):

  commit a8ee364bbf14861d5d0af39c4da06c30441895fb
  mtd: nand_base: always initialise oob_poi before writing OOB data

That commit removed the memset from under a conditional for:

  if (likely(!oob))

and moved it (indirectly) to the `nand_fill_oob()' function, which was
under:

  if (unlikely(oob))

Though the "likely" and "unlikely" can be confusing, these are not the
same conditions :)

And if the buggy commit is going stable, this should go stable (or just
amend it) as well.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Cc: Adam Thomson <adam.thomson@alcatel-lucent.com>
---
If the buggy commit is going into -stable, this should go -stable as
well (or just amend the original).

 drivers/mtd/nand/nand_base.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index d2ee68a..273e6a5 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2227,6 +2227,9 @@ static int nand_do_write_ops(struct mtd_info *mtd, loff_t to,
 			size_t len = min(oobwritelen, oobmaxlen);
 			oob = nand_fill_oob(mtd, oob, len, ops);
 			oobwritelen -= len;
+		} else {
+			/* We still need to erase leftover OOB data */
+			memset(chip->oob_poi, 0xff, mtd->oobsize);
 		}
 
 		ret = chip->write_page(mtd, chip, wbuf, page, cached,
-- 
1.7.5.4

Re: [PATCH 01/12] mtd: nand: initialize chip->oob_poi before write

[Artem Bityutskiy]

On Tue, 2011-08-30 at 18:45 -0700, Brian Norris wrote:
> And if the buggy commit is going stable, this should go stable (or just
> amend it) as well.

Amended Adam's patch, thanks.

-- 
Best Regards,
Artem Bityutskiy

RE: [PATCH 01/12] mtd: nand: initialize chip->oob_poi before write

[THOMSON, Adam (Adam)]

Brian Norris wrote:

> For raw (i.e., w/o ECC) page writes (i.e., w/o OOB), we may 
> not have initialized and filled the chip->oob_poi buffer. 
> This can end up writing junk to the flash if we're not 
> careful. Say, for example, we use `nandwrite -n' (without 
> OOB). Then nand_do_write_ops calls
> chip->write_page, which writes OOB data with some previous, junk data.
> 
> This fixes a bug with this commit (from l2-mtd-2.6.git):
> 
>   commit a8ee364bbf14861d5d0af39c4da06c30441895fb
>   mtd: nand_base: always initialise oob_poi before writing OOB data

That's annoying that I missed that. :( Thanks for spotting said issue and providing a fix.

Regards

Adam
 

[PATCH 02/12] mtd: support writing OOB without ECC

[Brian Norris]

This fixes issues with `nandwrite -n -o' and the MEMWRITEOOB[64] ioctls
on hardware that writes ECC when writing OOB. The problem arises as
follows: `nandwrite -n' can write page data to flash without applying
ECC, but when used with the `-o' option, ECC is applied (incorrectly),
contrary to the `--noecc' option.

I found that this is the case because my hardware computes and writes
ECC data to flash upon either OOB write or page write. Thus, to support
a proper "no ECC" write, my driver must know when we're performing a raw
OOB write vs. a normal ECC OOB write. However, MTD does not pass any raw
mode information to the write_oob functions.  This patch addresses the
problems by:

1) Passing MTD_OOB_RAW down to lower layers, instead of just defaulting
   to MTD_OOB_PLACE
2) Handling MTD_OOB_RAW within the NAND layer's `nand_do_write_oob'
3) Adding a new (replaceable) function pointer in struct ecc_ctrl; this
   function should support writing OOB without ECC data. Current
   hardware often can use the same OOB write function when writing
   either with or without ECC

This was tested with nandsim as well as on actual SLC NAND.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Cc: Jim Quinlan <jim2101024@gmail.com>
---
Note: I removed unnecessary braces that were in a previous version of
this patch.

 drivers/mtd/mtdchar.c        |    3 ++-
 drivers/mtd/nand/nand_base.c |   10 +++++++++-
 include/linux/mtd/nand.h     |    3 +++
 3 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
index b206254..bcb7f05 100644
--- a/drivers/mtd/mtdchar.c
+++ b/drivers/mtd/mtdchar.c
@@ -391,6 +391,7 @@ static int mtd_do_writeoob(struct file *file, struct mtd_info *mtd,
 	uint64_t start, uint32_t length, void __user *ptr,
 	uint32_t __user *retp)
 {
+	struct mtd_file_info *mfi = file->private_data;
 	struct mtd_oob_ops ops;
 	uint32_t retlen;
 	int ret = 0;
@@ -412,7 +413,7 @@ static int mtd_do_writeoob(struct file *file, struct mtd_info *mtd,
 	ops.ooblen = length;
 	ops.ooboffs = start & (mtd->writesize - 1);
 	ops.datbuf = NULL;
-	ops.mode = MTD_OOB_PLACE;
+	ops.mode = (mfi->mode == MTD_MODE_RAW) ? MTD_OOB_RAW : MTD_OOB_PLACE;
 
 	if (ops.ooboffs && ops.ooblen > (mtd->oobsize - ops.ooboffs))
 		return -EINVAL;
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 273e6a5..ded1e5a 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2404,7 +2404,11 @@ static int nand_do_write_oob(struct mtd_info *mtd, loff_t to,
 		chip->pagebuf = -1;
 
 	nand_fill_oob(mtd, ops->oobbuf, ops->ooblen, ops);
-	status = chip->ecc.write_oob(mtd, chip, page & chip->pagemask);
+
+	if (ops->mode == MTD_OOB_RAW)
+		status = chip->ecc.write_oob_raw(mtd, chip, page & chip->pagemask);
+	else
+		status = chip->ecc.write_oob(mtd, chip, page & chip->pagemask);
 
 	if (status)
 		return status;
@@ -3380,6 +3384,10 @@ int nand_scan_tail(struct mtd_info *mtd)
 		BUG();
 	}
 
+	/* For many systems, the standard OOB write also works for raw */
+	if (!chip->ecc.write_oob_raw)
+		chip->ecc.write_oob_raw = chip->ecc.write_oob;
+
 	/*
 	 * The number of bytes available for a client to place data into
 	 * the out of band area.
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 85fef68..5f3fdd9 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -340,6 +340,7 @@ struct nand_hw_control {
  * @read_subpage:	function to read parts of the page covered by ECC.
  * @write_page:	function to write a page according to the ECC generator
  *		requirements.
+ * @write_oob_raw:	function to write chip OOB data without ECC
  * @read_oob:	function to read chip OOB data
  * @write_oob:	function to write chip OOB data
  */
@@ -368,6 +369,8 @@ struct nand_ecc_ctrl {
 			uint32_t offs, uint32_t len, uint8_t *buf);
 	void (*write_page)(struct mtd_info *mtd, struct nand_chip *chip,
 			const uint8_t *buf);
+	int (*write_oob_raw)(struct mtd_info *mtd, struct nand_chip *chip,
+			int page);
 	int (*read_oob)(struct mtd_info *mtd, struct nand_chip *chip, int page,
 			int sndcmd);
 	int (*write_oob)(struct mtd_info *mtd, struct nand_chip *chip,
-- 
1.7.5.4

[PATCH 03/12] mtd: support reading OOB without ECC

[Brian Norris]

This fixes issues with `nanddump -n' and the MEMREADOOB[64] ioctls on
hardware that performs error correction when reading only OOB data. A
driver for such hardware needs to know when we're doing a RAW vs. a
normal write, but mtd_do_read_oob does not pass such information to the
lower layers (e.g., NAND). We should pass MTD_OOB_RAW or MTD_OOB_PLACE
based on the MTD file mode.

For now, most drivers can get away with just setting:

  chip->ecc.read_oob_raw = chip->ecc.read_oob

This is done by default; but for systems that behave as described above,
you must supply your own replacement function.

This was tested with nandsim as well as on actual SLC NAND.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Cc: Jim Quinlan <jim2101024@gmail.com>
---
 drivers/mtd/mtdchar.c        |   14 ++++++++------
 drivers/mtd/nand/nand_base.c |    7 ++++++-
 include/linux/mtd/nand.h     |    3 +++
 3 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
index bcb7f05..d0eaef6 100644
--- a/drivers/mtd/mtdchar.c
+++ b/drivers/mtd/mtdchar.c
@@ -435,9 +435,11 @@ static int mtd_do_writeoob(struct file *file, struct mtd_info *mtd,
 	return ret;
 }
 
-static int mtd_do_readoob(struct mtd_info *mtd, uint64_t start,
-	uint32_t length, void __user *ptr, uint32_t __user *retp)
+static int mtd_do_readoob(struct file *file, struct mtd_info *mtd,
+	uint64_t start, uint32_t length, void __user *ptr,
+	uint32_t __user *retp)
 {
+	struct mtd_file_info *mfi = file->private_data;
 	struct mtd_oob_ops ops;
 	int ret = 0;
 
@@ -455,7 +457,7 @@ static int mtd_do_readoob(struct mtd_info *mtd, uint64_t start,
 	ops.ooblen = length;
 	ops.ooboffs = start & (mtd->writesize - 1);
 	ops.datbuf = NULL;
-	ops.mode = MTD_OOB_PLACE;
+	ops.mode = (mfi->mode == MTD_MODE_RAW) ? MTD_OOB_RAW : MTD_OOB_PLACE;
 
 	if (ops.ooboffs && ops.ooblen > (mtd->oobsize - ops.ooboffs))
 		return -EINVAL;
@@ -716,7 +718,7 @@ static int mtd_ioctl(struct file *file, u_int cmd, u_long arg)
 		if (copy_from_user(&buf, argp, sizeof(buf)))
 			ret = -EFAULT;
 		else
-			ret = mtd_do_readoob(mtd, buf.start, buf.length,
+			ret = mtd_do_readoob(file, mtd, buf.start, buf.length,
 				buf.ptr, &buf_user->start);
 		break;
 	}
@@ -743,7 +745,7 @@ static int mtd_ioctl(struct file *file, u_int cmd, u_long arg)
 		if (copy_from_user(&buf, argp, sizeof(buf)))
 			ret = -EFAULT;
 		else
-			ret = mtd_do_readoob(mtd, buf.start, buf.length,
+			ret = mtd_do_readoob(file, mtd, buf.start, buf.length,
 				(void __user *)(uintptr_t)buf.usr_ptr,
 				&buf_user->length);
 		break;
@@ -1029,7 +1031,7 @@ static long mtd_compat_ioctl(struct file *file, unsigned int cmd,
 		if (copy_from_user(&buf, argp, sizeof(buf)))
 			ret = -EFAULT;
 		else
-			ret = mtd_do_readoob(mtd, buf.start,
+			ret = mtd_do_readoob(file, mtd, buf.start,
 				buf.length, compat_ptr(buf.ptr),
 				&buf_user->start);
 		break;
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index ded1e5a..12732a0 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -1787,7 +1787,10 @@ static int nand_do_read_oob(struct mtd_info *mtd, loff_t from,
 	page = realpage & chip->pagemask;
 
 	while (1) {
-		sndcmd = chip->ecc.read_oob(mtd, chip, page, sndcmd);
+		if (ops->mode == MTD_OOB_RAW)
+			sndcmd = chip->ecc.read_oob_raw(mtd, chip, page, sndcmd);
+		else
+			sndcmd = chip->ecc.read_oob(mtd, chip, page, sndcmd);
 
 		len = min(len, readlen);
 		buf = nand_transfer_oob(chip, buf, ops, len);
@@ -3385,6 +3388,8 @@ int nand_scan_tail(struct mtd_info *mtd)
 	}
 
 	/* For many systems, the standard OOB write also works for raw */
+	if (!chip->ecc.read_oob_raw)
+		chip->ecc.read_oob_raw = chip->ecc.read_oob;
 	if (!chip->ecc.write_oob_raw)
 		chip->ecc.write_oob_raw = chip->ecc.write_oob;
 
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 5f3fdd9..17964c9 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -341,6 +341,7 @@ struct nand_hw_control {
  * @write_page:	function to write a page according to the ECC generator
  *		requirements.
  * @write_oob_raw:	function to write chip OOB data without ECC
+ * @read_oob_raw:	function to read chip OOB data without ECC
  * @read_oob:	function to read chip OOB data
  * @write_oob:	function to write chip OOB data
  */
@@ -371,6 +372,8 @@ struct nand_ecc_ctrl {
 			const uint8_t *buf);
 	int (*write_oob_raw)(struct mtd_info *mtd, struct nand_chip *chip,
 			int page);
+	int (*read_oob_raw)(struct mtd_info *mtd, struct nand_chip *chip,
+			int page, int sndcmd);
 	int (*read_oob)(struct mtd_info *mtd, struct nand_chip *chip, int page,
 			int sndcmd);
 	int (*write_oob)(struct mtd_info *mtd, struct nand_chip *chip,
-- 
1.7.5.4

Re: [PATCH 03/12] mtd: support reading OOB without ECC

[Artem Bityutskiy]

On Tue, 2011-08-30 at 18:45 -0700, Brian Norris wrote:
> -static int mtd_do_readoob(struct mtd_info *mtd, uint64_t start,
> -	uint32_t length, void __user *ptr, uint32_t __user *retp)
> +static int mtd_do_readoob(struct file *file, struct mtd_info *mtd,
> +	uint64_t start, uint32_t length, void __user *ptr,
> +	uint32_t __user *retp)
>  {
> +	struct mtd_file_info *mfi = file->private_data;
>  	struct mtd_oob_ops ops;
>  	int ret = 0;

Why do you pass struct file pointer to this function instead of just
passing the MTD_MODE constant directly? What if the caller does not have
any 'struct file' at all (e.g., at some point someone would want to make
an JFFS2 or YAFFS2 optimization and use this function). Do I miss
something?

If there is not strong reason for passing 'file', could we pass 'int
mode' instead?

-- 
Best Regards,
Artem Bityutskiy

Re: [PATCH 03/12] mtd: support reading OOB without ECC

[Artem Bityutskiy]

On Sun, 2011-09-11 at 14:46 +0300, Artem Bityutskiy wrote:
> On Tue, 2011-08-30 at 18:45 -0700, Brian Norris wrote:
> > -static int mtd_do_readoob(struct mtd_info *mtd, uint64_t start,
> > -	uint32_t length, void __user *ptr, uint32_t __user *retp)
> > +static int mtd_do_readoob(struct file *file, struct mtd_info *mtd,
> > +	uint64_t start, uint32_t length, void __user *ptr,
> > +	uint32_t __user *retp)
> >  {
> > +	struct mtd_file_info *mfi = file->private_data;
> >  	struct mtd_oob_ops ops;
> >  	int ret = 0;
> 
> Why do you pass struct file pointer to this function instead of just
> passing the MTD_MODE constant directly? What if the caller does not have
> any 'struct file' at all (e.g., at some point someone would want to make
> an JFFS2 or YAFFS2 optimization and use this function). Do I miss
> something?
> 
> If there is not strong reason for passing 'file', could we pass 'int
> mode' instead?

Although after looking a bit closer, I think it is fine, sorry. Pushed
to l2-mtd-2.6.git, thanks!

-- 
Best Regards,
Artem Bityutskiy

[PATCH 04/12] mtd: move mtd_oob_mode_t to shared kernel/user space

[Brian Norris]

We will want to use the MTD_OOB_{PLACE,AUTO,RAW} modes in user-space
applications through the introduction of new ioctls, so we should make
this enum a shared type.

This enum is now anonymous and will be used in 8-bit fields (__u8 or
uint8_t).

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/onenand/onenand_base.c |    4 ++--
 include/linux/mtd/mtd.h            |   16 +---------------
 include/mtd/mtd-abi.h              |   15 +++++++++++++++
 3 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c
index de98791..9edc8b1 100644
--- a/drivers/mtd/onenand/onenand_base.c
+++ b/drivers/mtd/onenand/onenand_base.c
@@ -1351,7 +1351,7 @@ static int onenand_read_oob_nolock(struct mtd_info *mtd, loff_t from,
 	struct mtd_ecc_stats stats;
 	int read = 0, thislen, column, oobsize;
 	size_t len = ops->ooblen;
-	mtd_oob_mode_t mode = ops->mode;
+	uint8_t mode = ops->mode;
 	u_char *buf = ops->oobbuf;
 	int ret = 0, readcmd;
 
@@ -2074,7 +2074,7 @@ static int onenand_write_oob_nolock(struct mtd_info *mtd, loff_t to,
 	u_char *oobbuf;
 	size_t len = ops->ooblen;
 	const u_char *buf = ops->oobbuf;
-	mtd_oob_mode_t mode = ops->mode;
+	uint8_t mode = ops->mode;
 
 	to += ops->ooboffs;
 
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index ff7bae0..3b2c678 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -68,20 +68,6 @@ struct mtd_erase_region_info {
 	unsigned long *lockmap;		/* If keeping bitmap of locks */
 };
 
-/*
- * oob operation modes
- *
- * MTD_OOB_PLACE:	oob data are placed at the given offset
- * MTD_OOB_AUTO:	oob data are automatically placed at the free areas
- *			which are defined by the ecclayout
- * MTD_OOB_RAW:		mode to read oob and data without doing ECC checking
- */
-typedef enum {
-	MTD_OOB_PLACE,
-	MTD_OOB_AUTO,
-	MTD_OOB_RAW,
-} mtd_oob_mode_t;
-
 /**
  * struct mtd_oob_ops - oob operation operands
  * @mode:	operation mode
@@ -102,7 +88,7 @@ typedef enum {
  * OOB area.
  */
 struct mtd_oob_ops {
-	mtd_oob_mode_t	mode;
+	uint8_t		mode;
 	size_t		len;
 	size_t		retlen;
 	size_t		ooblen;
diff --git a/include/mtd/mtd-abi.h b/include/mtd/mtd-abi.h
index 3bdda5c..af42c7a 100644
--- a/include/mtd/mtd-abi.h
+++ b/include/mtd/mtd-abi.h
@@ -45,6 +45,21 @@ struct mtd_oob_buf64 {
 	__u64 usr_ptr;
 };
 
+/*
+ * oob operation modes
+ *
+ * MTD_OOB_PLACE:       oob data are placed at the given offset (default)
+ * MTD_OOB_AUTO:        oob data are automatically placed at the free areas
+ *                      which are defined by the internal ecclayout
+ * MTD_OOB_RAW:         mode to read or write oob and data without doing ECC
+ *			checking
+ */
+enum {
+	MTD_OOB_PLACE = 0,
+	MTD_OOB_AUTO = 1,
+	MTD_OOB_RAW = 2,
+};
+
 #define MTD_ABSENT		0
 #define MTD_RAM			1
 #define MTD_ROM			2
-- 
1.7.5.4

Re: [PATCH 04/12] mtd: move mtd_oob_mode_t to shared kernel/user space

[Artem Bityutskiy]

On Tue, 2011-08-30 at 18:45 -0700, Brian Norris wrote:
> -	mtd_oob_mode_t mode = ops->mode;
> +	uint8_t mode = ops->mode;

...
>  struct mtd_oob_ops {
> -	mtd_oob_mode_t	mode;
> +	uint8_t		mode;
>  	size_t		len;
>  	size_t		retlen;
>  	size_t		ooblen;

It is good to use __u8 in ioctls for this field.

But for internal usage there is no need to make it uint8_t, just use
'int' instead. All modern CPUs will anyway reserve 32 bits for this.

And unnecessary usage of the 8-bits restriction only imposes more
unnecessary limitations to the compiler/CPU. Using 'int' is instead
making CPU/compiler use the native integer type.

BTW, you may also re-arrange this data structure and make it 8-bytes
smaller on 64-bit architectures. Indeed, it has 'size_t' and 'uint8_t *'
fields, which are 64-bits, and it has one 'uint32_t        ooboffs;',
which is 32-bits and is also padded. If you put 'int mode' and 'uint32_t
ooboffs' together, you'll save 2 paddings. But this is optional.


-- 
Best Regards,
Artem Bityutskiy

Re: [PATCH 04/12] mtd: move mtd_oob_mode_t to shared kernel/user space

[Artem Bityutskiy]

On Sun, 2011-09-11 at 14:57 +0300, Artem Bityutskiy wrote:
> On Tue, 2011-08-30 at 18:45 -0700, Brian Norris wrote:
> > -	mtd_oob_mode_t mode = ops->mode;
> > +	uint8_t mode = ops->mode;
> 
> ...
> >  struct mtd_oob_ops {
> > -	mtd_oob_mode_t	mode;
> > +	uint8_t		mode;
> >  	size_t		len;
> >  	size_t		retlen;
> >  	size_t		ooblen;
> 
> It is good to use __u8 in ioctls for this field.
> 
> But for internal usage there is no need to make it uint8_t, just use
> 'int' instead. All modern CPUs will anyway reserve 32 bits for this.
> 
> And unnecessary usage of the 8-bits restriction only imposes more
> unnecessary limitations to the compiler/CPU. Using 'int' is instead
> making CPU/compiler use the native integer type.

I've actually amended your patch and used 'unsigned int mode' instead,
and pushed to my three. Please, complain and send new versions if you do
not like that.

> BTW, you may also re-arrange this data structure and make it 8-bytes
> smaller on 64-bit architectures. Indeed, it has 'size_t' and 'uint8_t *'
> fields, which are 64-bits, and it has one 'uint32_t        ooboffs;',
> which is 32-bits and is also padded. If you put 'int mode' and 'uint32_t
> ooboffs' together, you'll save 2 paddings. But this is optional.

Did not do this, though.

Thanks!

-- 
Best Regards,
Artem Bityutskiy

Re: [PATCH 04/12] mtd: move mtd_oob_mode_t to shared kernel/user space

[Brian Norris]

On Sun, Sep 11, 2011 at 8:28 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> On Sun, 2011-09-11 at 14:57 +0300, Artem Bityutskiy wrote:
>> It is good to use __u8 in ioctls for this field.
>>
>> But for internal usage there is no need to make it uint8_t, just use
>> 'int' instead. All modern CPUs will anyway reserve 32 bits for this.
>>
>> And unnecessary usage of the 8-bits restriction only imposes more
>> unnecessary limitations to the compiler/CPU. Using 'int' is instead
>> making CPU/compiler use the native integer type.

Thanks, good suggestion. I think I considered this issue but not very
thoroughly. My only reason for uint8_t here was just to prevent the
temptation to use all 32 bits of the field. But there's no way we'd
use 32 bits for a enumerated mode, i.e., we'd need to have more than
256 modes to run into this issue... I think we'd be having other more
important issues if we get there :)

> I've actually amended your patch and used 'unsigned int mode' instead,
> and pushed to my three. Please, complain and send new versions if you do
> not like that.

No complaint.

Brian

[PATCH 05/12] mtd: rename MTD_OOB_* to MTD_OPS_*

[Brian Norris]

These modes are not necessarily for OOB only. Particularly, MTD_OOB_RAW
affected operations on in-band page data as well. To clarify these
options and to emphasize that their effect is applied per-operation, we
change the primary prefix to MTD_OPS_.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/devices/doc2000.c          |    4 +-
 drivers/mtd/devices/doc2001.c          |    4 +-
 drivers/mtd/devices/doc2001plus.c      |    4 +-
 drivers/mtd/inftlcore.c                |    6 ++--
 drivers/mtd/mtdchar.c                  |   10 ++++---
 drivers/mtd/mtdpart.c                  |    2 +-
 drivers/mtd/mtdswap.c                  |    6 ++--
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c |    2 +-
 drivers/mtd/nand/nand_base.c           |   40 ++++++++++++++++----------------
 drivers/mtd/nand/nand_bbt.c            |    8 +++---
 drivers/mtd/nand/sm_common.c           |    2 +-
 drivers/mtd/nftlcore.c                 |    6 ++--
 drivers/mtd/onenand/onenand_base.c     |   38 +++++++++++++++---------------
 drivers/mtd/onenand/onenand_bbt.c      |    2 +-
 drivers/mtd/sm_ftl.c                   |    4 +-
 drivers/mtd/ssfdc.c                    |    2 +-
 drivers/mtd/tests/mtd_oobtest.c        |   24 +++++++++---------
 drivers/mtd/tests/mtd_readtest.c       |    2 +-
 drivers/staging/spectra/lld_mtd.c      |    6 ++--
 fs/jffs2/wbuf.c                        |    6 ++--
 include/linux/mtd/mtd.h                |    2 +-
 include/mtd/mtd-abi.h                  |   16 ++++++------
 22 files changed, 99 insertions(+), 97 deletions(-)

diff --git a/drivers/mtd/devices/doc2000.c b/drivers/mtd/devices/doc2000.c
index 8c97033..e9fad91 100644
--- a/drivers/mtd/devices/doc2000.c
+++ b/drivers/mtd/devices/doc2000.c
@@ -927,7 +927,7 @@ static int doc_read_oob(struct mtd_info *mtd, loff_t ofs,
 	uint8_t *buf = ops->oobbuf;
 	size_t len = ops->len;
 
-	BUG_ON(ops->mode != MTD_OOB_PLACE);
+	BUG_ON(ops->mode != MTD_OPS_PLACE_OOB);
 
 	ofs += ops->ooboffs;
 
@@ -1091,7 +1091,7 @@ static int doc_write_oob(struct mtd_info *mtd, loff_t ofs,
 	struct DiskOnChip *this = mtd->priv;
 	int ret;
 
-	BUG_ON(ops->mode != MTD_OOB_PLACE);
+	BUG_ON(ops->mode != MTD_OPS_PLACE_OOB);
 
 	mutex_lock(&this->lock);
 	ret = doc_write_oob_nolock(mtd, ofs + ops->ooboffs, ops->len,
diff --git a/drivers/mtd/devices/doc2001.c b/drivers/mtd/devices/doc2001.c
index 3d2b459..a3f7a27 100644
--- a/drivers/mtd/devices/doc2001.c
+++ b/drivers/mtd/devices/doc2001.c
@@ -631,7 +631,7 @@ static int doc_read_oob(struct mtd_info *mtd, loff_t ofs,
 	uint8_t *buf = ops->oobbuf;
 	size_t len = ops->len;
 
-	BUG_ON(ops->mode != MTD_OOB_PLACE);
+	BUG_ON(ops->mode != MTD_OPS_PLACE_OOB);
 
 	ofs += ops->ooboffs;
 
@@ -689,7 +689,7 @@ static int doc_write_oob(struct mtd_info *mtd, loff_t ofs,
 	uint8_t *buf = ops->oobbuf;
 	size_t len = ops->len;
 
-	BUG_ON(ops->mode != MTD_OOB_PLACE);
+	BUG_ON(ops->mode != MTD_OPS_PLACE_OOB);
 
 	ofs += ops->ooboffs;
 
diff --git a/drivers/mtd/devices/doc2001plus.c b/drivers/mtd/devices/doc2001plus.c
index d28c9d9..99351bc 100644
--- a/drivers/mtd/devices/doc2001plus.c
+++ b/drivers/mtd/devices/doc2001plus.c
@@ -834,7 +834,7 @@ static int doc_read_oob(struct mtd_info *mtd, loff_t ofs,
 	uint8_t *buf = ops->oobbuf;
 	size_t len = ops->len;
 
-	BUG_ON(ops->mode != MTD_OOB_PLACE);
+	BUG_ON(ops->mode != MTD_OPS_PLACE_OOB);
 
 	ofs += ops->ooboffs;
 
@@ -919,7 +919,7 @@ static int doc_write_oob(struct mtd_info *mtd, loff_t ofs,
 	uint8_t *buf = ops->oobbuf;
 	size_t len = ops->len;
 
-	BUG_ON(ops->mode != MTD_OOB_PLACE);
+	BUG_ON(ops->mode != MTD_OPS_PLACE_OOB);
 
 	ofs += ops->ooboffs;
 
diff --git a/drivers/mtd/inftlcore.c b/drivers/mtd/inftlcore.c
index 21aa981..652065e 100644
--- a/drivers/mtd/inftlcore.c
+++ b/drivers/mtd/inftlcore.c
@@ -152,7 +152,7 @@ int inftl_read_oob(struct mtd_info *mtd, loff_t offs, size_t len,
 	struct mtd_oob_ops ops;
 	int res;
 
-	ops.mode = MTD_OOB_PLACE;
+	ops.mode = MTD_OPS_PLACE_OOB;
 	ops.ooboffs = offs & (mtd->writesize - 1);
 	ops.ooblen = len;
 	ops.oobbuf = buf;
@@ -172,7 +172,7 @@ int inftl_write_oob(struct mtd_info *mtd, loff_t offs, size_t len,
 	struct mtd_oob_ops ops;
 	int res;
 
-	ops.mode = MTD_OOB_PLACE;
+	ops.mode = MTD_OPS_PLACE_OOB;
 	ops.ooboffs = offs & (mtd->writesize - 1);
 	ops.ooblen = len;
 	ops.oobbuf = buf;
@@ -192,7 +192,7 @@ static int inftl_write(struct mtd_info *mtd, loff_t offs, size_t len,
 	struct mtd_oob_ops ops;
 	int res;
 
-	ops.mode = MTD_OOB_PLACE;
+	ops.mode = MTD_OPS_PLACE_OOB;
 	ops.ooboffs = offs;
 	ops.ooblen = mtd->oobsize;
 	ops.oobbuf = oob;
diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
index d0eaef6..9b76438 100644
--- a/drivers/mtd/mtdchar.c
+++ b/drivers/mtd/mtdchar.c
@@ -221,7 +221,7 @@ static ssize_t mtd_read(struct file *file, char __user *buf, size_t count,loff_t
 		{
 			struct mtd_oob_ops ops;
 
-			ops.mode = MTD_OOB_RAW;
+			ops.mode = MTD_OPS_RAW;
 			ops.datbuf = kbuf;
 			ops.oobbuf = NULL;
 			ops.len = len;
@@ -317,7 +317,7 @@ static ssize_t mtd_write(struct file *file, const char __user *buf, size_t count
 		{
 			struct mtd_oob_ops ops;
 
-			ops.mode = MTD_OOB_RAW;
+			ops.mode = MTD_OPS_RAW;
 			ops.datbuf = kbuf;
 			ops.oobbuf = NULL;
 			ops.ooboffs = 0;
@@ -413,7 +413,8 @@ static int mtd_do_writeoob(struct file *file, struct mtd_info *mtd,
 	ops.ooblen = length;
 	ops.ooboffs = start & (mtd->writesize - 1);
 	ops.datbuf = NULL;
-	ops.mode = (mfi->mode == MTD_MODE_RAW) ? MTD_OOB_RAW : MTD_OOB_PLACE;
+	ops.mode = (mfi->mode == MTD_MODE_RAW) ? MTD_OPS_RAW :
+		MTD_OPS_PLACE_OOB;
 
 	if (ops.ooboffs && ops.ooblen > (mtd->oobsize - ops.ooboffs))
 		return -EINVAL;
@@ -457,7 +458,8 @@ static int mtd_do_readoob(struct file *file, struct mtd_info *mtd,
 	ops.ooblen = length;
 	ops.ooboffs = start & (mtd->writesize - 1);
 	ops.datbuf = NULL;
-	ops.mode = (mfi->mode == MTD_MODE_RAW) ? MTD_OOB_RAW : MTD_OOB_PLACE;
+	ops.mode = (mfi->mode == MTD_MODE_RAW) ? MTD_OPS_RAW :
+		MTD_OPS_PLACE_OOB;
 
 	if (ops.ooboffs && ops.ooblen > (mtd->oobsize - ops.ooboffs))
 		return -EINVAL;
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index c90b7ba..cd7785a 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -130,7 +130,7 @@ static int part_read_oob(struct mtd_info *mtd, loff_t from,
 	if (ops->oobbuf) {
 		size_t len, pages;
 
-		if (ops->mode == MTD_OOB_AUTO)
+		if (ops->mode == MTD_OPS_AUTO_OOB)
 			len = mtd->oobavail;
 		else
 			len = mtd->oobsize;
diff --git a/drivers/mtd/mtdswap.c b/drivers/mtd/mtdswap.c
index 9961063b..910309f 100644
--- a/drivers/mtd/mtdswap.c
+++ b/drivers/mtd/mtdswap.c
@@ -350,7 +350,7 @@ static int mtdswap_read_markers(struct mtdswap_dev *d, struct swap_eb *eb)
 	ops.oobbuf = d->oob_buf;
 	ops.ooboffs = 0;
 	ops.datbuf = NULL;
-	ops.mode = MTD_OOB_AUTO;
+	ops.mode = MTD_OPS_AUTO_OOB;
 
 	ret = mtdswap_read_oob(d, offset, &ops);
 
@@ -389,7 +389,7 @@ static int mtdswap_write_marker(struct mtdswap_dev *d, struct swap_eb *eb,
 
 	ops.ooboffs = 0;
 	ops.oobbuf = (uint8_t *)&n;
-	ops.mode = MTD_OOB_AUTO;
+	ops.mode = MTD_OPS_AUTO_OOB;
 	ops.datbuf = NULL;
 
 	if (marker == MTDSWAP_TYPE_CLEAN) {
@@ -931,7 +931,7 @@ static unsigned int mtdswap_eblk_passes(struct mtdswap_dev *d,
 	struct mtd_oob_ops ops;
 	int ret;
 
-	ops.mode = MTD_OOB_AUTO;
+	ops.mode = MTD_OPS_AUTO_OOB;
 	ops.len = mtd->writesize;
 	ops.ooblen = mtd->ecclayout->oobavail;
 	ops.ooboffs = 0;
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index 99940bd..754ffed 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -1159,7 +1159,7 @@ gpmi_ecc_write_oob(struct mtd_info *mtd, struct nand_chip *chip, int page)
 	 * The BCH will use all the (page + oob).
 	 * Our gpmi_hw_ecclayout can only prohibit the JFFS2 to write the oob.
 	 * But it can not stop some ioctls such MEMWRITEOOB which uses
-	 * MTD_OOB_PLACE. So We have to implement this function to prohibit
+	 * MTD_OPS_PLACE_OOB. So We have to implement this function to prohibit
 	 * these ioctls too.
 	 */
 	return -EPERM;
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 12732a0..e5c6a13 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -1382,12 +1382,12 @@ static uint8_t *nand_transfer_oob(struct nand_chip *chip, uint8_t *oob,
 {
 	switch (ops->mode) {
 
-	case MTD_OOB_PLACE:
-	case MTD_OOB_RAW:
+	case MTD_OPS_PLACE_OOB:
+	case MTD_OPS_RAW:
 		memcpy(oob, chip->oob_poi + ops->ooboffs, len);
 		return oob + len;
 
-	case MTD_OOB_AUTO: {
+	case MTD_OPS_AUTO_OOB: {
 		struct nand_oobfree *free = chip->ecc.layout->oobfree;
 		uint32_t boffs = 0, roffs = ops->ooboffs;
 		size_t bytes = 0;
@@ -1437,7 +1437,7 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
 	int ret = 0;
 	uint32_t readlen = ops->len;
 	uint32_t oobreadlen = ops->ooblen;
-	uint32_t max_oobsize = ops->mode == MTD_OOB_AUTO ?
+	uint32_t max_oobsize = ops->mode == MTD_OPS_AUTO_OOB ?
 		mtd->oobavail : mtd->oobsize;
 
 	uint8_t *bufpoi, *oob, *buf;
@@ -1469,7 +1469,7 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
 			}
 
 			/* Now read the page into the buffer */
-			if (unlikely(ops->mode == MTD_OOB_RAW))
+			if (unlikely(ops->mode == MTD_OPS_RAW))
 				ret = chip->ecc.read_page_raw(mtd, chip,
 							      bufpoi, page);
 			else if (!aligned && NAND_SUBPAGE_READ(chip) && !oob)
@@ -1759,7 +1759,7 @@ static int nand_do_read_oob(struct mtd_info *mtd, loff_t from,
 
 	stats = mtd->ecc_stats;
 
-	if (ops->mode == MTD_OOB_AUTO)
+	if (ops->mode == MTD_OPS_AUTO_OOB)
 		len = chip->ecc.layout->oobavail;
 	else
 		len = mtd->oobsize;
@@ -1787,7 +1787,7 @@ static int nand_do_read_oob(struct mtd_info *mtd, loff_t from,
 	page = realpage & chip->pagemask;
 
 	while (1) {
-		if (ops->mode == MTD_OOB_RAW)
+		if (ops->mode == MTD_OPS_RAW)
 			sndcmd = chip->ecc.read_oob_raw(mtd, chip, page, sndcmd);
 		else
 			sndcmd = chip->ecc.read_oob(mtd, chip, page, sndcmd);
@@ -1865,9 +1865,9 @@ static int nand_read_oob(struct mtd_info *mtd, loff_t from,
 	nand_get_device(chip, mtd, FL_READING);
 
 	switch (ops->mode) {
-	case MTD_OOB_PLACE:
-	case MTD_OOB_AUTO:
-	case MTD_OOB_RAW:
+	case MTD_OPS_PLACE_OOB:
+	case MTD_OPS_AUTO_OOB:
+	case MTD_OPS_RAW:
 		break;
 
 	default:
@@ -2113,12 +2113,12 @@ static uint8_t *nand_fill_oob(struct mtd_info *mtd, uint8_t *oob, size_t len,
 
 	switch (ops->mode) {
 
-	case MTD_OOB_PLACE:
-	case MTD_OOB_RAW:
+	case MTD_OPS_PLACE_OOB:
+	case MTD_OPS_RAW:
 		memcpy(chip->oob_poi + ops->ooboffs, oob, len);
 		return oob + len;
 
-	case MTD_OOB_AUTO: {
+	case MTD_OPS_AUTO_OOB: {
 		struct nand_oobfree *free = chip->ecc.layout->oobfree;
 		uint32_t boffs = 0, woffs = ops->ooboffs;
 		size_t bytes = 0;
@@ -2167,7 +2167,7 @@ static int nand_do_write_ops(struct mtd_info *mtd, loff_t to,
 	uint32_t writelen = ops->len;
 
 	uint32_t oobwritelen = ops->ooblen;
-	uint32_t oobmaxlen = ops->mode == MTD_OOB_AUTO ?
+	uint32_t oobmaxlen = ops->mode == MTD_OPS_AUTO_OOB ?
 				mtd->oobavail : mtd->oobsize;
 
 	uint8_t *oob = ops->oobbuf;
@@ -2236,7 +2236,7 @@ static int nand_do_write_ops(struct mtd_info *mtd, loff_t to,
 		}
 
 		ret = chip->write_page(mtd, chip, wbuf, page, cached,
-				       (ops->mode == MTD_OOB_RAW));
+				       (ops->mode == MTD_OPS_RAW));
 		if (ret)
 			break;
 
@@ -2356,7 +2356,7 @@ static int nand_do_write_oob(struct mtd_info *mtd, loff_t to,
 	pr_debug("%s: to = 0x%08x, len = %i\n",
 			 __func__, (unsigned int)to, (int)ops->ooblen);
 
-	if (ops->mode == MTD_OOB_AUTO)
+	if (ops->mode == MTD_OPS_AUTO_OOB)
 		len = chip->ecc.layout->oobavail;
 	else
 		len = mtd->oobsize;
@@ -2408,7 +2408,7 @@ static int nand_do_write_oob(struct mtd_info *mtd, loff_t to,
 
 	nand_fill_oob(mtd, ops->oobbuf, ops->ooblen, ops);
 
-	if (ops->mode == MTD_OOB_RAW)
+	if (ops->mode == MTD_OPS_RAW)
 		status = chip->ecc.write_oob_raw(mtd, chip, page & chip->pagemask);
 	else
 		status = chip->ecc.write_oob(mtd, chip, page & chip->pagemask);
@@ -2445,9 +2445,9 @@ static int nand_write_oob(struct mtd_info *mtd, loff_t to,
 	nand_get_device(chip, mtd, FL_WRITING);
 
 	switch (ops->mode) {
-	case MTD_OOB_PLACE:
-	case MTD_OOB_AUTO:
-	case MTD_OOB_RAW:
+	case MTD_OPS_PLACE_OOB:
+	case MTD_OPS_AUTO_OOB:
+	case MTD_OPS_RAW:
 		break;
 
 	default:
diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index 6aa8125..c488bcb 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -302,7 +302,7 @@ static int scan_read_raw_oob(struct mtd_info *mtd, uint8_t *buf, loff_t offs,
 	struct mtd_oob_ops ops;
 	int res;
 
-	ops.mode = MTD_OOB_RAW;
+	ops.mode = MTD_OPS_RAW;
 	ops.ooboffs = 0;
 	ops.ooblen = mtd->oobsize;
 
@@ -350,7 +350,7 @@ static int scan_write_bbt(struct mtd_info *mtd, loff_t offs, size_t len,
 {
 	struct mtd_oob_ops ops;
 
-	ops.mode = MTD_OOB_PLACE;
+	ops.mode = MTD_OPS_PLACE_OOB;
 	ops.ooboffs = 0;
 	ops.ooblen = mtd->oobsize;
 	ops.datbuf = buf;
@@ -433,7 +433,7 @@ static int scan_block_fast(struct mtd_info *mtd, struct nand_bbt_descr *bd,
 	ops.oobbuf = buf;
 	ops.ooboffs = 0;
 	ops.datbuf = NULL;
-	ops.mode = MTD_OOB_PLACE;
+	ops.mode = MTD_OPS_PLACE_OOB;
 
 	for (j = 0; j < len; j++) {
 		/*
@@ -672,7 +672,7 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
 	ops.ooblen = mtd->oobsize;
 	ops.ooboffs = 0;
 	ops.datbuf = NULL;
-	ops.mode = MTD_OOB_PLACE;
+	ops.mode = MTD_OPS_PLACE_OOB;
 
 	if (!rcode)
 		rcode = 0xff;
diff --git a/drivers/mtd/nand/sm_common.c b/drivers/mtd/nand/sm_common.c
index b6332e8..2c438ef 100644
--- a/drivers/mtd/nand/sm_common.c
+++ b/drivers/mtd/nand/sm_common.c
@@ -47,7 +47,7 @@ static int sm_block_markbad(struct mtd_info *mtd, loff_t ofs)
 
 	/* As long as this function is called on erase block boundaries
 		it will work correctly for 256 byte nand */
-	ops.mode = MTD_OOB_PLACE;
+	ops.mode = MTD_OPS_PLACE_OOB;
 	ops.ooboffs = 0;
 	ops.ooblen = mtd->oobsize;
 	ops.oobbuf = (void *)&oob;
diff --git a/drivers/mtd/nftlcore.c b/drivers/mtd/nftlcore.c
index 93d6fc6..272e3c0 100644
--- a/drivers/mtd/nftlcore.c
+++ b/drivers/mtd/nftlcore.c
@@ -147,7 +147,7 @@ int nftl_read_oob(struct mtd_info *mtd, loff_t offs, size_t len,
 	struct mtd_oob_ops ops;
 	int res;
 
-	ops.mode = MTD_OOB_PLACE;
+	ops.mode = MTD_OPS_PLACE_OOB;
 	ops.ooboffs = offs & mask;
 	ops.ooblen = len;
 	ops.oobbuf = buf;
@@ -168,7 +168,7 @@ int nftl_write_oob(struct mtd_info *mtd, loff_t offs, size_t len,
 	struct mtd_oob_ops ops;
 	int res;
 
-	ops.mode = MTD_OOB_PLACE;
+	ops.mode = MTD_OPS_PLACE_OOB;
 	ops.ooboffs = offs & mask;
 	ops.ooblen = len;
 	ops.oobbuf = buf;
@@ -191,7 +191,7 @@ static int nftl_write(struct mtd_info *mtd, loff_t offs, size_t len,
 	struct mtd_oob_ops ops;
 	int res;
 
-	ops.mode = MTD_OOB_PLACE;
+	ops.mode = MTD_OPS_PLACE_OOB;
 	ops.ooboffs = offs & mask;
 	ops.ooblen = mtd->oobsize;
 	ops.oobbuf = oob;
diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c
index 9edc8b1..65a8e18 100644
--- a/drivers/mtd/onenand/onenand_base.c
+++ b/drivers/mtd/onenand/onenand_base.c
@@ -1125,7 +1125,7 @@ static int onenand_mlc_read_ops_nolock(struct mtd_info *mtd, loff_t from,
 	pr_debug("%s: from = 0x%08x, len = %i\n", __func__, (unsigned int)from,
 			(int)len);
 
-	if (ops->mode == MTD_OOB_AUTO)
+	if (ops->mode == MTD_OPS_AUTO_OOB)
 		oobsize = this->ecclayout->oobavail;
 	else
 		oobsize = mtd->oobsize;
@@ -1170,7 +1170,7 @@ static int onenand_mlc_read_ops_nolock(struct mtd_info *mtd, loff_t from,
 			thisooblen = oobsize - oobcolumn;
 			thisooblen = min_t(int, thisooblen, ooblen - oobread);
 
-			if (ops->mode == MTD_OOB_AUTO)
+			if (ops->mode == MTD_OPS_AUTO_OOB)
 				onenand_transfer_auto_oob(mtd, oobbuf, oobcolumn, thisooblen);
 			else
 				this->read_bufferram(mtd, ONENAND_SPARERAM, oobbuf, oobcolumn, thisooblen);
@@ -1229,7 +1229,7 @@ static int onenand_read_ops_nolock(struct mtd_info *mtd, loff_t from,
 	pr_debug("%s: from = 0x%08x, len = %i\n", __func__, (unsigned int)from,
 			(int)len);
 
-	if (ops->mode == MTD_OOB_AUTO)
+	if (ops->mode == MTD_OPS_AUTO_OOB)
 		oobsize = this->ecclayout->oobavail;
 	else
 		oobsize = mtd->oobsize;
@@ -1291,7 +1291,7 @@ static int onenand_read_ops_nolock(struct mtd_info *mtd, loff_t from,
 			thisooblen = oobsize - oobcolumn;
 			thisooblen = min_t(int, thisooblen, ooblen - oobread);
 
-			if (ops->mode == MTD_OOB_AUTO)
+			if (ops->mode == MTD_OPS_AUTO_OOB)
 				onenand_transfer_auto_oob(mtd, oobbuf, oobcolumn, thisooblen);
 			else
 				this->read_bufferram(mtd, ONENAND_SPARERAM, oobbuf, oobcolumn, thisooblen);
@@ -1363,7 +1363,7 @@ static int onenand_read_oob_nolock(struct mtd_info *mtd, loff_t from,
 	/* Initialize return length value */
 	ops->oobretlen = 0;
 
-	if (mode == MTD_OOB_AUTO)
+	if (mode == MTD_OPS_AUTO_OOB)
 		oobsize = this->ecclayout->oobavail;
 	else
 		oobsize = mtd->oobsize;
@@ -1409,7 +1409,7 @@ static int onenand_read_oob_nolock(struct mtd_info *mtd, loff_t from,
 			break;
 		}
 
-		if (mode == MTD_OOB_AUTO)
+		if (mode == MTD_OPS_AUTO_OOB)
 			onenand_transfer_auto_oob(mtd, buf, column, thislen);
 		else
 			this->read_bufferram(mtd, ONENAND_SPARERAM, buf, column, thislen);
@@ -1487,10 +1487,10 @@ static int onenand_read_oob(struct mtd_info *mtd, loff_t from,
 	int ret;
 
 	switch (ops->mode) {
-	case MTD_OOB_PLACE:
-	case MTD_OOB_AUTO:
+	case MTD_OPS_PLACE_OOB:
+	case MTD_OPS_AUTO_OOB:
 		break;
-	case MTD_OOB_RAW:
+	case MTD_OPS_RAW:
 		/* Not implemented yet */
 	default:
 		return -EINVAL;
@@ -1908,7 +1908,7 @@ static int onenand_write_ops_nolock(struct mtd_info *mtd, loff_t to,
 	if (!len)
 		return 0;
 
-	if (ops->mode == MTD_OOB_AUTO)
+	if (ops->mode == MTD_OPS_AUTO_OOB)
 		oobsize = this->ecclayout->oobavail;
 	else
 		oobsize = mtd->oobsize;
@@ -1945,7 +1945,7 @@ static int onenand_write_ops_nolock(struct mtd_info *mtd, loff_t to,
 				/* We send data to spare ram with oobsize
 				 * to prevent byte access */
 				memset(oobbuf, 0xff, mtd->oobsize);
-				if (ops->mode == MTD_OOB_AUTO)
+				if (ops->mode == MTD_OPS_AUTO_OOB)
 					onenand_fill_auto_oob(mtd, oobbuf, oob, oobcolumn, thisooblen);
 				else
 					memcpy(oobbuf + oobcolumn, oob, thisooblen);
@@ -2084,7 +2084,7 @@ static int onenand_write_oob_nolock(struct mtd_info *mtd, loff_t to,
 	/* Initialize retlen, in case of early exit */
 	ops->oobretlen = 0;
 
-	if (mode == MTD_OOB_AUTO)
+	if (mode == MTD_OPS_AUTO_OOB)
 		oobsize = this->ecclayout->oobavail;
 	else
 		oobsize = mtd->oobsize;
@@ -2128,7 +2128,7 @@ static int onenand_write_oob_nolock(struct mtd_info *mtd, loff_t to,
 		/* We send data to spare ram with oobsize
 		 * to prevent byte access */
 		memset(oobbuf, 0xff, mtd->oobsize);
-		if (mode == MTD_OOB_AUTO)
+		if (mode == MTD_OPS_AUTO_OOB)
 			onenand_fill_auto_oob(mtd, oobbuf, buf, column, thislen);
 		else
 			memcpy(oobbuf + column, buf, thislen);
@@ -2217,10 +2217,10 @@ static int onenand_write_oob(struct mtd_info *mtd, loff_t to,
 	int ret;
 
 	switch (ops->mode) {
-	case MTD_OOB_PLACE:
-	case MTD_OOB_AUTO:
+	case MTD_OPS_PLACE_OOB:
+	case MTD_OPS_AUTO_OOB:
 		break;
-	case MTD_OOB_RAW:
+	case MTD_OPS_RAW:
 		/* Not implemented yet */
 	default:
 		return -EINVAL;
@@ -2603,7 +2603,7 @@ static int onenand_default_block_markbad(struct mtd_info *mtd, loff_t ofs)
 	struct bbm_info *bbm = this->bbm;
 	u_char buf[2] = {0, 0};
 	struct mtd_oob_ops ops = {
-		.mode = MTD_OOB_PLACE,
+		.mode = MTD_OPS_PLACE_OOB,
 		.ooblen = 2,
 		.oobbuf = buf,
 		.ooboffs = 0,
@@ -3171,7 +3171,7 @@ static int do_otp_lock(struct mtd_info *mtd, loff_t from, size_t len,
 		this->command(mtd, ONENAND_CMD_RESET, 0, 0);
 		this->wait(mtd, FL_RESETING);
 	} else {
-		ops.mode = MTD_OOB_PLACE;
+		ops.mode = MTD_OPS_PLACE_OOB;
 		ops.ooblen = len;
 		ops.oobbuf = buf;
 		ops.ooboffs = 0;
@@ -3677,7 +3677,7 @@ static int flexonenand_check_blocks_erased(struct mtd_info *mtd, int start, int
 	int i, ret;
 	int block;
 	struct mtd_oob_ops ops = {
-		.mode = MTD_OOB_PLACE,
+		.mode = MTD_OPS_PLACE_OOB,
 		.ooboffs = 0,
 		.ooblen	= mtd->oobsize,
 		.datbuf	= NULL,
diff --git a/drivers/mtd/onenand/onenand_bbt.c b/drivers/mtd/onenand/onenand_bbt.c
index 3b9a2a9..09956c4 100644
--- a/drivers/mtd/onenand/onenand_bbt.c
+++ b/drivers/mtd/onenand/onenand_bbt.c
@@ -80,7 +80,7 @@ static int create_bbt(struct mtd_info *mtd, uint8_t *buf, struct nand_bbt_descr
 	startblock = 0;
 	from = 0;
 
-	ops.mode = MTD_OOB_PLACE;
+	ops.mode = MTD_OPS_PLACE_OOB;
 	ops.ooblen = readlen;
 	ops.oobbuf = buf;
 	ops.len = ops.ooboffs = ops.retlen = ops.oobretlen = 0;
diff --git a/drivers/mtd/sm_ftl.c b/drivers/mtd/sm_ftl.c
index 311a9e3..d927641 100644
--- a/drivers/mtd/sm_ftl.c
+++ b/drivers/mtd/sm_ftl.c
@@ -256,7 +256,7 @@ static int sm_read_sector(struct sm_ftl *ftl,
 	if (!oob)
 		oob = &tmp_oob;
 
-	ops.mode = ftl->smallpagenand ? MTD_OOB_RAW : MTD_OOB_PLACE;
+	ops.mode = ftl->smallpagenand ? MTD_OPS_RAW : MTD_OPS_PLACE_OOB;
 	ops.ooboffs = 0;
 	ops.ooblen = SM_OOB_SIZE;
 	ops.oobbuf = (void *)oob;
@@ -336,7 +336,7 @@ static int sm_write_sector(struct sm_ftl *ftl,
 	if (ftl->unstable)
 		return -EIO;
 
-	ops.mode = ftl->smallpagenand ? MTD_OOB_RAW : MTD_OOB_PLACE;
+	ops.mode = ftl->smallpagenand ? MTD_OPS_RAW : MTD_OPS_PLACE_OOB;
 	ops.len = SM_SECTOR_SIZE;
 	ops.datbuf = buffer;
 	ops.ooboffs = 0;
diff --git a/drivers/mtd/ssfdc.c b/drivers/mtd/ssfdc.c
index 5f917f0..976e3d2 100644
--- a/drivers/mtd/ssfdc.c
+++ b/drivers/mtd/ssfdc.c
@@ -169,7 +169,7 @@ static int read_raw_oob(struct mtd_info *mtd, loff_t offs, uint8_t *buf)
 	struct mtd_oob_ops ops;
 	int ret;
 
-	ops.mode = MTD_OOB_RAW;
+	ops.mode = MTD_OPS_RAW;
 	ops.ooboffs = 0;
 	ops.ooblen = OOB_SIZE;
 	ops.oobbuf = buf;
diff --git a/drivers/mtd/tests/mtd_oobtest.c b/drivers/mtd/tests/mtd_oobtest.c
index dec92ae..6bcff42 100644
--- a/drivers/mtd/tests/mtd_oobtest.c
+++ b/drivers/mtd/tests/mtd_oobtest.c
@@ -131,7 +131,7 @@ static int write_eraseblock(int ebnum)
 
 	for (i = 0; i < pgcnt; ++i, addr += mtd->writesize) {
 		set_random_data(writebuf, use_len);
-		ops.mode      = MTD_OOB_AUTO;
+		ops.mode      = MTD_OPS_AUTO_OOB;
 		ops.len       = 0;
 		ops.retlen    = 0;
 		ops.ooblen    = use_len;
@@ -184,7 +184,7 @@ static int verify_eraseblock(int ebnum)
 
 	for (i = 0; i < pgcnt; ++i, addr += mtd->writesize) {
 		set_random_data(writebuf, use_len);
-		ops.mode      = MTD_OOB_AUTO;
+		ops.mode      = MTD_OPS_AUTO_OOB;
 		ops.len       = 0;
 		ops.retlen    = 0;
 		ops.ooblen    = use_len;
@@ -211,7 +211,7 @@ static int verify_eraseblock(int ebnum)
 		if (use_offset != 0 || use_len < mtd->ecclayout->oobavail) {
 			int k;
 
-			ops.mode      = MTD_OOB_AUTO;
+			ops.mode      = MTD_OPS_AUTO_OOB;
 			ops.len       = 0;
 			ops.retlen    = 0;
 			ops.ooblen    = mtd->ecclayout->oobavail;
@@ -276,7 +276,7 @@ static int verify_eraseblock_in_one_go(int ebnum)
 	size_t len = mtd->ecclayout->oobavail * pgcnt;
 
 	set_random_data(writebuf, len);
-	ops.mode      = MTD_OOB_AUTO;
+	ops.mode      = MTD_OPS_AUTO_OOB;
 	ops.len       = 0;
 	ops.retlen    = 0;
 	ops.ooblen    = len;
@@ -507,7 +507,7 @@ static int __init mtd_oobtest_init(void)
 		addr0 += mtd->erasesize;
 
 	/* Attempt to write off end of OOB */
-	ops.mode      = MTD_OOB_AUTO;
+	ops.mode      = MTD_OPS_AUTO_OOB;
 	ops.len       = 0;
 	ops.retlen    = 0;
 	ops.ooblen    = 1;
@@ -527,7 +527,7 @@ static int __init mtd_oobtest_init(void)
 	}
 
 	/* Attempt to read off end of OOB */
-	ops.mode      = MTD_OOB_AUTO;
+	ops.mode      = MTD_OPS_AUTO_OOB;
 	ops.len       = 0;
 	ops.retlen    = 0;
 	ops.ooblen    = 1;
@@ -551,7 +551,7 @@ static int __init mtd_oobtest_init(void)
 		       "block is bad\n");
 	else {
 		/* Attempt to write off end of device */
-		ops.mode      = MTD_OOB_AUTO;
+		ops.mode      = MTD_OPS_AUTO_OOB;
 		ops.len       = 0;
 		ops.retlen    = 0;
 		ops.ooblen    = mtd->ecclayout->oobavail + 1;
@@ -571,7 +571,7 @@ static int __init mtd_oobtest_init(void)
 		}
 
 		/* Attempt to read off end of device */
-		ops.mode      = MTD_OOB_AUTO;
+		ops.mode      = MTD_OPS_AUTO_OOB;
 		ops.len       = 0;
 		ops.retlen    = 0;
 		ops.ooblen    = mtd->ecclayout->oobavail + 1;
@@ -595,7 +595,7 @@ static int __init mtd_oobtest_init(void)
 			goto out;
 
 		/* Attempt to write off end of device */
-		ops.mode      = MTD_OOB_AUTO;
+		ops.mode      = MTD_OPS_AUTO_OOB;
 		ops.len       = 0;
 		ops.retlen    = 0;
 		ops.ooblen    = mtd->ecclayout->oobavail;
@@ -615,7 +615,7 @@ static int __init mtd_oobtest_init(void)
 		}
 
 		/* Attempt to read off end of device */
-		ops.mode      = MTD_OOB_AUTO;
+		ops.mode      = MTD_OPS_AUTO_OOB;
 		ops.len       = 0;
 		ops.retlen    = 0;
 		ops.ooblen    = mtd->ecclayout->oobavail;
@@ -655,7 +655,7 @@ static int __init mtd_oobtest_init(void)
 		addr = (i + 1) * mtd->erasesize - mtd->writesize;
 		for (pg = 0; pg < cnt; ++pg) {
 			set_random_data(writebuf, sz);
-			ops.mode      = MTD_OOB_AUTO;
+			ops.mode      = MTD_OPS_AUTO_OOB;
 			ops.len       = 0;
 			ops.retlen    = 0;
 			ops.ooblen    = sz;
@@ -683,7 +683,7 @@ static int __init mtd_oobtest_init(void)
 			continue;
 		set_random_data(writebuf, mtd->ecclayout->oobavail * 2);
 		addr = (i + 1) * mtd->erasesize - mtd->writesize;
-		ops.mode      = MTD_OOB_AUTO;
+		ops.mode      = MTD_OPS_AUTO_OOB;
 		ops.len       = 0;
 		ops.retlen    = 0;
 		ops.ooblen    = mtd->ecclayout->oobavail * 2;
diff --git a/drivers/mtd/tests/mtd_readtest.c b/drivers/mtd/tests/mtd_readtest.c
index 836792d..587e1e3 100644
--- a/drivers/mtd/tests/mtd_readtest.c
+++ b/drivers/mtd/tests/mtd_readtest.c
@@ -66,7 +66,7 @@ static int read_eraseblock_by_page(int ebnum)
 		if (mtd->oobsize) {
 			struct mtd_oob_ops ops;
 
-			ops.mode      = MTD_OOB_PLACE;
+			ops.mode      = MTD_OPS_PLACE_OOB;
 			ops.len       = 0;
 			ops.retlen    = 0;
 			ops.ooblen    = mtd->oobsize;
diff --git a/drivers/staging/spectra/lld_mtd.c b/drivers/staging/spectra/lld_mtd.c
index 2bd3466..a9c309a 100644
--- a/drivers/staging/spectra/lld_mtd.c
+++ b/drivers/staging/spectra/lld_mtd.c
@@ -340,7 +340,7 @@ u16 mtd_Read_Page_Main_Spare(u8 *read_data, u32 Block,
 		struct mtd_oob_ops ops;
 		int ret;
 
-		ops.mode = MTD_OOB_AUTO;
+		ops.mode = MTD_OPS_AUTO_OOB;
 		ops.datbuf = read_data;
 		ops.len = DeviceInfo.wPageDataSize;
 		ops.oobbuf = read_data + DeviceInfo.wPageDataSize + BTSIG_OFFSET;
@@ -400,7 +400,7 @@ u16 mtd_Write_Page_Main_Spare(u8 *write_data, u32 Block,
 		struct mtd_oob_ops ops;
 		int ret;
 
-		ops.mode = MTD_OOB_AUTO;
+		ops.mode = MTD_OPS_AUTO_OOB;
 		ops.datbuf = write_data;
 		ops.len = DeviceInfo.wPageDataSize;
 		ops.oobbuf = write_data + DeviceInfo.wPageDataSize + BTSIG_OFFSET;
@@ -473,7 +473,7 @@ u16 mtd_Read_Page_Spare(u8 *read_data, u32 Block,
 		struct mtd_oob_ops ops;
 		int ret;
 
-		ops.mode = MTD_OOB_AUTO;
+		ops.mode = MTD_OPS_AUTO_OOB;
 		ops.datbuf = NULL;
 		ops.len = 0;
 		ops.oobbuf = read_data;
diff --git a/fs/jffs2/wbuf.c b/fs/jffs2/wbuf.c
index a10fb24..b09e51d 100644
--- a/fs/jffs2/wbuf.c
+++ b/fs/jffs2/wbuf.c
@@ -1025,7 +1025,7 @@ int jffs2_check_oob_empty(struct jffs2_sb_info *c,
 	int cmlen = min_t(int, c->oobavail, OOB_CM_SIZE);
 	struct mtd_oob_ops ops;
 
-	ops.mode = MTD_OOB_AUTO;
+	ops.mode = MTD_OPS_AUTO_OOB;
 	ops.ooblen = NR_OOB_SCAN_PAGES * c->oobavail;
 	ops.oobbuf = c->oobbuf;
 	ops.len = ops.ooboffs = ops.retlen = ops.oobretlen = 0;
@@ -1068,7 +1068,7 @@ int jffs2_check_nand_cleanmarker(struct jffs2_sb_info *c,
 	struct mtd_oob_ops ops;
 	int ret, cmlen = min_t(int, c->oobavail, OOB_CM_SIZE);
 
-	ops.mode = MTD_OOB_AUTO;
+	ops.mode = MTD_OPS_AUTO_OOB;
 	ops.ooblen = cmlen;
 	ops.oobbuf = c->oobbuf;
 	ops.len = ops.ooboffs = ops.retlen = ops.oobretlen = 0;
@@ -1094,7 +1094,7 @@ int jffs2_write_nand_cleanmarker(struct jffs2_sb_info *c,
 	struct mtd_oob_ops ops;
 	int cmlen = min_t(int, c->oobavail, OOB_CM_SIZE);
 
-	ops.mode = MTD_OOB_AUTO;
+	ops.mode = MTD_OPS_AUTO_OOB;
 	ops.ooblen = cmlen;
 	ops.oobbuf = (uint8_t *)&oob_cleanmarker;
 	ops.len = ops.ooboffs = ops.retlen = ops.oobretlen = 0;
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index 3b2c678..e02e65b 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -79,7 +79,7 @@ struct mtd_erase_region_info {
  * @ooblen:	number of oob bytes to write/read
  * @oobretlen:	number of oob bytes written/read
  * @ooboffs:	offset of oob data in the oob area (only relevant when
- *		mode = MTD_OOB_PLACE)
+ *		mode = MTD_OPS_PLACE_OOB)
  * @datbuf:	data buffer - if NULL only oob data are read/written
  * @oobbuf:	oob data buffer
  *
diff --git a/include/mtd/mtd-abi.h b/include/mtd/mtd-abi.h
index af42c7a..d30990e 100644
--- a/include/mtd/mtd-abi.h
+++ b/include/mtd/mtd-abi.h
@@ -46,18 +46,18 @@ struct mtd_oob_buf64 {
 };
 
 /*
- * oob operation modes
+ * MTD operation modes
  *
- * MTD_OOB_PLACE:       oob data are placed at the given offset (default)
- * MTD_OOB_AUTO:        oob data are automatically placed at the free areas
- *                      which are defined by the internal ecclayout
- * MTD_OOB_RAW:         mode to read or write oob and data without doing ECC
+ * MTD_OPS_PLACE_OOB:	oob data are placed at the given offset (default)
+ * MTD_OPS_AUTO_OOB:	oob data are automatically placed at the free areas
+ *			which are defined by the internal ecclayout
+ * MTD_OPS_RAW:		mode to read or write oob and data without doing ECC
  *			checking
  */
 enum {
-	MTD_OOB_PLACE = 0,
-	MTD_OOB_AUTO = 1,
-	MTD_OOB_RAW = 2,
+	MTD_OPS_PLACE_OOB = 0,
+	MTD_OPS_AUTO_OOB = 1,
+	MTD_OPS_RAW = 2,
 };
 
 #define MTD_ABSENT		0
-- 
1.7.5.4

Re: [PATCH 05/12] mtd: rename MTD_OOB_* to MTD_OPS_*

[Artem Bityutskiy]

On Tue, 2011-08-30 at 18:45 -0700, Brian Norris wrote:
> These modes are not necessarily for OOB only. Particularly, MTD_OOB_RAW
> affected operations on in-band page data as well. To clarify these
> options and to emphasize that their effect is applied per-operation, we
> change the primary prefix to MTD_OPS_.
> 
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>

This patch depends on the other ones, so I have to wait for v2, sorry.

-- 
Best Regards,
Artem Bityutskiy

Re: [PATCH 05/12] mtd: rename MTD_OOB_* to MTD_OPS_*

[Artem Bityutskiy]

On Sun, 2011-09-11 at 15:10 +0300, Artem Bityutskiy wrote:
> On Tue, 2011-08-30 at 18:45 -0700, Brian Norris wrote:
> > These modes are not necessarily for OOB only. Particularly, MTD_OOB_RAW
> > affected operations on in-band page data as well. To clarify these
> > options and to emphasize that their effect is applied per-operation, we
> > change the primary prefix to MTD_OPS_.
> > 
> > Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> 
> This patch depends on the other ones, so I have to wait for v2, sorry.

Actually pushed to my tree now, thanks!

-- 
Best Regards,
Artem Bityutskiy

[PATCH 06/12] mtd: rename MTD_MODE_* to MTD_FILE_MODE_*

[Brian Norris]

These modes hold their state only for the life of their file descriptor,
and they overlap functionality with the MTD_OPS_* modes. Particularly,
MTD_MODE_RAW and MTD_OPS_RAW cover the same function: to provide raw
(i.e., without ECC) access to the flash. In fact, although it may not be
clear, MTD_MODE_RAW implied that operations should enable the
MTD_OPS_RAW mode.

Thus, we should be specific on what each mode means. This is a start,
where MTD_FILE_MODE_* actually represents a "file mode," not necessarily
a true global MTD mode.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/mtdchar.c |   36 ++++++++++++++++++------------------
 include/mtd/mtd-abi.h |    8 ++++----
 2 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
index 9b76438..4004f2b 100644
--- a/drivers/mtd/mtdchar.c
+++ b/drivers/mtd/mtdchar.c
@@ -211,13 +211,13 @@ static ssize_t mtd_read(struct file *file, char __user *buf, size_t count,loff_t
 		len = min_t(size_t, count, size);
 
 		switch (mfi->mode) {
-		case MTD_MODE_OTP_FACTORY:
+		case MTD_FILE_MODE_OTP_FACTORY:
 			ret = mtd->read_fact_prot_reg(mtd, *ppos, len, &retlen, kbuf);
 			break;
-		case MTD_MODE_OTP_USER:
+		case MTD_FILE_MODE_OTP_USER:
 			ret = mtd->read_user_prot_reg(mtd, *ppos, len, &retlen, kbuf);
 			break;
-		case MTD_MODE_RAW:
+		case MTD_FILE_MODE_RAW:
 		{
 			struct mtd_oob_ops ops;
 
@@ -302,10 +302,10 @@ static ssize_t mtd_write(struct file *file, const char __user *buf, size_t count
 		}
 
 		switch (mfi->mode) {
-		case MTD_MODE_OTP_FACTORY:
+		case MTD_FILE_MODE_OTP_FACTORY:
 			ret = -EROFS;
 			break;
-		case MTD_MODE_OTP_USER:
+		case MTD_FILE_MODE_OTP_USER:
 			if (!mtd->write_user_prot_reg) {
 				ret = -EOPNOTSUPP;
 				break;
@@ -313,7 +313,7 @@ static ssize_t mtd_write(struct file *file, const char __user *buf, size_t count
 			ret = mtd->write_user_prot_reg(mtd, *ppos, len, &retlen, kbuf);
 			break;
 
-		case MTD_MODE_RAW:
+		case MTD_FILE_MODE_RAW:
 		{
 			struct mtd_oob_ops ops;
 
@@ -368,13 +368,13 @@ static int otp_select_filemode(struct mtd_file_info *mfi, int mode)
 		if (!mtd->read_fact_prot_reg)
 			ret = -EOPNOTSUPP;
 		else
-			mfi->mode = MTD_MODE_OTP_FACTORY;
+			mfi->mode = MTD_FILE_MODE_OTP_FACTORY;
 		break;
 	case MTD_OTP_USER:
 		if (!mtd->read_fact_prot_reg)
 			ret = -EOPNOTSUPP;
 		else
-			mfi->mode = MTD_MODE_OTP_USER;
+			mfi->mode = MTD_FILE_MODE_OTP_USER;
 		break;
 	default:
 		ret = -EINVAL;
@@ -413,7 +413,7 @@ static int mtd_do_writeoob(struct file *file, struct mtd_info *mtd,
 	ops.ooblen = length;
 	ops.ooboffs = start & (mtd->writesize - 1);
 	ops.datbuf = NULL;
-	ops.mode = (mfi->mode == MTD_MODE_RAW) ? MTD_OPS_RAW :
+	ops.mode = (mfi->mode == MTD_FILE_MODE_RAW) ? MTD_OPS_RAW :
 		MTD_OPS_PLACE_OOB;
 
 	if (ops.ooboffs && ops.ooblen > (mtd->oobsize - ops.ooboffs))
@@ -458,7 +458,7 @@ static int mtd_do_readoob(struct file *file, struct mtd_info *mtd,
 	ops.ooblen = length;
 	ops.ooboffs = start & (mtd->writesize - 1);
 	ops.datbuf = NULL;
-	ops.mode = (mfi->mode == MTD_MODE_RAW) ? MTD_OPS_RAW :
+	ops.mode = (mfi->mode == MTD_FILE_MODE_RAW) ? MTD_OPS_RAW :
 		MTD_OPS_PLACE_OOB;
 
 	if (ops.ooboffs && ops.ooblen > (mtd->oobsize - ops.ooboffs))
@@ -849,7 +849,7 @@ static int mtd_ioctl(struct file *file, u_int cmd, u_long arg)
 		if (copy_from_user(&mode, argp, sizeof(int)))
 			return -EFAULT;
 
-		mfi->mode = MTD_MODE_NORMAL;
+		mfi->mode = MTD_FILE_MODE_NORMAL;
 
 		ret = otp_select_filemode(mfi, mode);
 
@@ -865,11 +865,11 @@ static int mtd_ioctl(struct file *file, u_int cmd, u_long arg)
 			return -ENOMEM;
 		ret = -EOPNOTSUPP;
 		switch (mfi->mode) {
-		case MTD_MODE_OTP_FACTORY:
+		case MTD_FILE_MODE_OTP_FACTORY:
 			if (mtd->get_fact_prot_info)
 				ret = mtd->get_fact_prot_info(mtd, buf, 4096);
 			break;
-		case MTD_MODE_OTP_USER:
+		case MTD_FILE_MODE_OTP_USER:
 			if (mtd->get_user_prot_info)
 				ret = mtd->get_user_prot_info(mtd, buf, 4096);
 			break;
@@ -893,7 +893,7 @@ static int mtd_ioctl(struct file *file, u_int cmd, u_long arg)
 	{
 		struct otp_info oinfo;
 
-		if (mfi->mode != MTD_MODE_OTP_USER)
+		if (mfi->mode != MTD_FILE_MODE_OTP_USER)
 			return -EINVAL;
 		if (copy_from_user(&oinfo, argp, sizeof(oinfo)))
 			return -EFAULT;
@@ -937,17 +937,17 @@ static int mtd_ioctl(struct file *file, u_int cmd, u_long arg)
 		mfi->mode = 0;
 
 		switch(arg) {
-		case MTD_MODE_OTP_FACTORY:
-		case MTD_MODE_OTP_USER:
+		case MTD_FILE_MODE_OTP_FACTORY:
+		case MTD_FILE_MODE_OTP_USER:
 			ret = otp_select_filemode(mfi, arg);
 			break;
 
-		case MTD_MODE_RAW:
+		case MTD_FILE_MODE_RAW:
 			if (!mtd->read_oob || !mtd->write_oob)
 				return -EOPNOTSUPP;
 			mfi->mode = arg;
 
-		case MTD_MODE_NORMAL:
+		case MTD_FILE_MODE_NORMAL:
 			break;
 		default:
 			ret = -EINVAL;
diff --git a/include/mtd/mtd-abi.h b/include/mtd/mtd-abi.h
index d30990e..1885aa9 100644
--- a/include/mtd/mtd-abi.h
+++ b/include/mtd/mtd-abi.h
@@ -200,10 +200,10 @@ struct mtd_ecc_stats {
  * Read/write file modes for access to MTD
  */
 enum mtd_file_modes {
-	MTD_MODE_NORMAL = MTD_OTP_OFF,
-	MTD_MODE_OTP_FACTORY = MTD_OTP_FACTORY,
-	MTD_MODE_OTP_USER = MTD_OTP_USER,
-	MTD_MODE_RAW,
+	MTD_FILE_MODE_NORMAL = MTD_OTP_OFF,
+	MTD_FILE_MODE_OTP_FACTORY = MTD_OTP_FACTORY,
+	MTD_FILE_MODE_OTP_USER = MTD_OTP_USER,
+	MTD_FILE_MODE_RAW,
 };
 
 #endif /* __MTD_ABI_H__ */
-- 
1.7.5.4

[PATCH 07/12] mtd: add MEMWRITE ioctl

[Brian Norris]

Implement a new ioctl for writing both page data and OOB to flash at the
same time. This ioctl is intended to be a generic interface that can
replace other ioctls (MEMWRITEOOB and MEMWRITEOOB64) and cover the
functionality of several other old ones, e.g., MEMWRITE can:

* write autoplaced OOB instead of using ECCGETLAYOUT (deprecated) and
  working around the reserved areas
* write raw (no ECC) OOB instead of using MTDFILEMODE to set the
  per-file-descriptor MTD_FILE_MODE_RAW
* write raw (no ECC) data instead of using MTDFILEMODE
  (MTD_FILE_MODE_RAW) and using standard character device "write"

This ioctl is especially useful for MLC NAND, which cannot be written
twice (i.e., we cannot successfully write the page data and OOB in two
separate operations). Instead, MEMWRITE can write both in a single
operation.

Note that this ioctl is not affected by the MTD file mode (i.e.,
MTD_FILE_MODE_RAW vs. MTD_FILE_MODE_NORMAL), since it receives its write
mode as an input parameter.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
RFC -> PATCH "v1": * the ioctl is now named MEMWRITE instead of
                     MEMWRITEDATAOOB for simplicity and to emphasize its
                     generic, multi-purpose functionality.
                   * ioctl functionality was moved inside its own
                     function to help with readability.
                   * padding was added to the renamed
                     `struct mtd_write_req'
                   * user buffers were split into separate OOB and data
                     buffers
                   * other small fixes?

 drivers/mtd/mtdchar.c |   56 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/mtd/mtd-abi.h |   11 +++++++++
 2 files changed, 67 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
index 4004f2b..1547e2a 100644
--- a/drivers/mtd/mtdchar.c
+++ b/drivers/mtd/mtdchar.c
@@ -566,6 +566,55 @@ static int mtd_blkpg_ioctl(struct mtd_info *mtd,
 	}
 }
 
+static int mtd_write_ioctl(struct mtd_info *mtd,
+		struct mtd_write_req __user *argp)
+{
+	struct mtd_write_req req;
+	struct mtd_oob_ops ops;
+	void __user *usr_data, *usr_oob;
+	int ret;
+
+	if (copy_from_user(&req, argp, sizeof(req)) ||
+			!access_ok(VERIFY_READ, req.usr_data, req.len) ||
+			!access_ok(VERIFY_READ, req.usr_oob, req.ooblen))
+		return -EFAULT;
+	if (!mtd->write_oob)
+		return -EOPNOTSUPP;
+
+	ops.mode = req.mode;
+	ops.len = (size_t)req.len;
+	ops.ooblen = (size_t)req.ooblen;
+	ops.ooboffs = 0;
+
+	usr_data = (void __user *)(uintptr_t)req.usr_data;
+	usr_oob = (void __user *)(uintptr_t)req.usr_oob;
+
+	if (req.usr_data) {
+		ops.datbuf = memdup_user(usr_data, ops.len);
+		if (IS_ERR(ops.datbuf))
+			return PTR_ERR(ops.datbuf);
+	} else {
+		ops.datbuf = NULL;
+	}
+
+	if (req.usr_oob) {
+		ops.oobbuf = memdup_user(usr_oob, ops.ooblen);
+		if (IS_ERR(ops.oobbuf)) {
+			kfree(ops.datbuf);
+			return PTR_ERR(ops.oobbuf);
+		}
+	} else {
+		ops.oobbuf = NULL;
+	}
+
+	ret = mtd->write_oob(mtd, (loff_t)req.start, &ops);
+
+	kfree(ops.datbuf);
+	kfree(ops.oobbuf);
+
+	return ret;
+}
+
 static int mtd_ioctl(struct file *file, u_int cmd, u_long arg)
 {
 	struct mtd_file_info *mfi = file->private_data;
@@ -753,6 +802,13 @@ static int mtd_ioctl(struct file *file, u_int cmd, u_long arg)
 		break;
 	}
 
+	case MEMWRITE:
+	{
+		ret = mtd_write_ioctl(mtd,
+		      (struct mtd_write_req __user *)arg);
+		break;
+	}
+
 	case MEMLOCK:
 	{
 		struct erase_info_user einfo;
diff --git a/include/mtd/mtd-abi.h b/include/mtd/mtd-abi.h
index 1885aa9..0a21ef5 100644
--- a/include/mtd/mtd-abi.h
+++ b/include/mtd/mtd-abi.h
@@ -60,6 +60,16 @@ enum {
 	MTD_OPS_RAW = 2,
 };
 
+struct mtd_write_req {
+	__u64 start;
+	__u32 len;
+	__u32 ooblen;
+	__u64 usr_data;
+	__u64 usr_oob;
+	__u8 mode;
+	__u8 padding[7];
+};
+
 #define MTD_ABSENT		0
 #define MTD_RAM			1
 #define MTD_ROM			2
@@ -147,6 +157,7 @@ struct otp_info {
 #define MEMWRITEOOB64		_IOWR('M', 21, struct mtd_oob_buf64)
 #define MEMREADOOB64		_IOWR('M', 22, struct mtd_oob_buf64)
 #define MEMISLOCKED		_IOR('M', 23, struct erase_info_user)
+#define MEMWRITE		_IOWR('M', 24, struct mtd_write_req)
 
 /*
  * Obsolete legacy interface. Keep it in order not to break userspace
-- 
1.7.5.4

[PATCH v2 07/12] mtd: add MEMWRITE ioctl

[Brian Norris]

Implement a new ioctl for writing both page data and OOB to flash at the
same time. This ioctl is intended to be a generic interface that can
replace other ioctls (MEMWRITEOOB and MEMWRITEOOB64) and cover the
functionality of several other old ones, e.g., MEMWRITE can:

* write autoplaced OOB instead of using ECCGETLAYOUT (deprecated) and
  working around the reserved areas
* write raw (no ECC) OOB instead of using MTDFILEMODE to set the
  per-file-descriptor MTD_FILE_MODE_RAW
* write raw (no ECC) data instead of using MTDFILEMODE
  (MTD_FILE_MODE_RAW) and using standard character device "write"

This ioctl is especially useful for MLC NAND, which cannot be written
twice (i.e., we cannot successfully write the page data and OOB in two
separate operations). Instead, MEMWRITE can write both in a single
operation.

Note that this ioctl is not affected by the MTD file mode (i.e.,
MTD_FILE_MODE_RAW vs. MTD_FILE_MODE_NORMAL), since it receives its write
mode as an input parameter.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
RFC -> PATCH "v1": * the ioctl is now named MEMWRITE instead of
                     MEMWRITEDATAOOB for simplicity and to emphasize its
                     generic, multi-purpose functionality.
                   * ioctl functionality was moved inside its own
                     function to help with readability.
                   * padding was added to the renamed
                     `struct mtd_write_req'
                   * user buffers were split into separate OOB and data
                     buffers
                   * other small fixes?

v1 -> v2:          Change __u32 to __u64 for future-proof compatability;
                   Is this a sane enough idea?

 drivers/mtd/mtdchar.c |   56 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/mtd/mtd-abi.h |   11 +++++++++
 2 files changed, 67 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
index 4004f2b..1547e2a 100644
--- a/drivers/mtd/mtdchar.c
+++ b/drivers/mtd/mtdchar.c
@@ -566,6 +566,55 @@ static int mtd_blkpg_ioctl(struct mtd_info *mtd,
 	}
 }
 
+static int mtd_write_ioctl(struct mtd_info *mtd,
+		struct mtd_write_req __user *argp)
+{
+	struct mtd_write_req req;
+	struct mtd_oob_ops ops;
+	void __user *usr_data, *usr_oob;
+	int ret;
+
+	if (copy_from_user(&req, argp, sizeof(req)) ||
+			!access_ok(VERIFY_READ, req.usr_data, req.len) ||
+			!access_ok(VERIFY_READ, req.usr_oob, req.ooblen))
+		return -EFAULT;
+	if (!mtd->write_oob)
+		return -EOPNOTSUPP;
+
+	ops.mode = req.mode;
+	ops.len = (size_t)req.len;
+	ops.ooblen = (size_t)req.ooblen;
+	ops.ooboffs = 0;
+
+	usr_data = (void __user *)(uintptr_t)req.usr_data;
+	usr_oob = (void __user *)(uintptr_t)req.usr_oob;
+
+	if (req.usr_data) {
+		ops.datbuf = memdup_user(usr_data, ops.len);
+		if (IS_ERR(ops.datbuf))
+			return PTR_ERR(ops.datbuf);
+	} else {
+		ops.datbuf = NULL;
+	}
+
+	if (req.usr_oob) {
+		ops.oobbuf = memdup_user(usr_oob, ops.ooblen);
+		if (IS_ERR(ops.oobbuf)) {
+			kfree(ops.datbuf);
+			return PTR_ERR(ops.oobbuf);
+		}
+	} else {
+		ops.oobbuf = NULL;
+	}
+
+	ret = mtd->write_oob(mtd, (loff_t)req.start, &ops);
+
+	kfree(ops.datbuf);
+	kfree(ops.oobbuf);
+
+	return ret;
+}
+
 static int mtd_ioctl(struct file *file, u_int cmd, u_long arg)
 {
 	struct mtd_file_info *mfi = file->private_data;
@@ -753,6 +802,13 @@ static int mtd_ioctl(struct file *file, u_int cmd, u_long arg)
 		break;
 	}
 
+	case MEMWRITE:
+	{
+		ret = mtd_write_ioctl(mtd,
+		      (struct mtd_write_req __user *)arg);
+		break;
+	}
+
 	case MEMLOCK:
 	{
 		struct erase_info_user einfo;
diff --git a/include/mtd/mtd-abi.h b/include/mtd/mtd-abi.h
index 1885aa9..1a16046 100644
--- a/include/mtd/mtd-abi.h
+++ b/include/mtd/mtd-abi.h
@@ -60,6 +60,16 @@ enum {
 	MTD_OPS_RAW = 2,
 };
 
+struct mtd_write_req {
+	__u64 start;
+	__u64 len;
+	__u64 ooblen;
+	__u64 usr_data;
+	__u64 usr_oob;
+	__u8 mode;
+	__u8 padding[7];
+};
+
 #define MTD_ABSENT		0
 #define MTD_RAM			1
 #define MTD_ROM			2
@@ -147,6 +157,7 @@ struct otp_info {
 #define MEMWRITEOOB64		_IOWR('M', 21, struct mtd_oob_buf64)
 #define MEMREADOOB64		_IOWR('M', 22, struct mtd_oob_buf64)
 #define MEMISLOCKED		_IOR('M', 23, struct erase_info_user)
+#define MEMWRITE		_IOWR('M', 24, struct mtd_write_req)
 
 /*
  * Obsolete legacy interface. Keep it in order not to break userspace
-- 
1.7.5.4

Re: [PATCH v2 07/12] mtd: add MEMWRITE ioctl

[Artem Bityutskiy]

On Fri, 2011-09-09 at 09:59 -0700, Brian Norris wrote:
> Implement a new ioctl for writing both page data and OOB to flash at the
> same time. This ioctl is intended to be a generic interface that can
> replace other ioctls (MEMWRITEOOB and MEMWRITEOOB64) and cover the
> functionality of several other old ones, e.g., MEMWRITE can:

Took v2.

-- 
Best Regards,
Artem Bityutskiy

[PATCH 08/12] mtd: nand: document nand_chip.oob_poi

[Brian Norris]

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 include/linux/mtd/nand.h |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 17964c9..0b3d464 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -434,7 +434,8 @@ struct nand_buffers {
  * @chip_delay:		[BOARDSPECIFIC] chip dependent delay for transferring
  *			data from array to read regs (tR).
  * @state:		[INTERN] the current state of the NAND device
- * @oob_poi:		poison value buffer
+ * @oob_poi:		"poison value buffer," used for laying out OOB data
+ *			before writing
  * @page_shift:		[INTERN] number of address bits in a page (column
  *			address bits).
  * @phys_erase_shift:	[INTERN] number of address bits in a physical eraseblock
-- 
1.7.5.4

Re: [PATCH 08/12] mtd: nand: document nand_chip.oob_poi

[Artem Bityutskiy]

On Tue, 2011-08-30 at 18:45 -0700, Brian Norris wrote:
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>

Pushed this one, thanks!

-- 
Best Regards,
Artem Bityutskiy

[PATCH 09/12] mtd: document ABI

[Brian Norris]

We're missing a lot of important documentation in include/mtd/mtd-abi.h:

* add a simple description of each ioctl (feel free to expand!)
* give full explanations of recently added and modified operations
* explain the usage of "RAW" that appear in different modes and types of
  operations
* fix some comment style along the way

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 include/linux/mtd/mtd.h |    2 +-
 include/mtd/mtd-abi.h   |   86 +++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 74 insertions(+), 14 deletions(-)

diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index e02e65b..ead70ed 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -79,7 +79,7 @@ struct mtd_erase_region_info {
  * @ooblen:	number of oob bytes to write/read
  * @oobretlen:	number of oob bytes written/read
  * @ooboffs:	offset of oob data in the oob area (only relevant when
- *		mode = MTD_OPS_PLACE_OOB)
+ *		mode = MTD_OPS_PLACE_OOB or MTD_OPS_RAW)
  * @datbuf:	data buffer - if NULL only oob data are read/written
  * @oobbuf:	oob data buffer
  *
diff --git a/include/mtd/mtd-abi.h b/include/mtd/mtd-abi.h
index 0a21ef5..d54ccf3 100644
--- a/include/mtd/mtd-abi.h
+++ b/include/mtd/mtd-abi.h
@@ -45,14 +45,18 @@ struct mtd_oob_buf64 {
 	__u64 usr_ptr;
 };
 
-/*
+/**
  * MTD operation modes
  *
- * MTD_OPS_PLACE_OOB:	oob data are placed at the given offset (default)
- * MTD_OPS_AUTO_OOB:	oob data are automatically placed at the free areas
+ * @MTD_OPS_PLACE_OOB:	OOB data are placed at the given offset (default)
+ * @MTD_OPS_AUTO_OOB:	OOB data are automatically placed at the free areas
  *			which are defined by the internal ecclayout
- * MTD_OPS_RAW:		mode to read or write oob and data without doing ECC
- *			checking
+ * @MTD_OPS_RAW:	data are transferred as-is, with no error correction;
+ *			this mode implies %MTD_OPS_PLACE_OOB
+ *
+ * These modes can be passed to ioctl(MEMWRITE) and are also used internally.
+ * See notes on "MTD file modes" for discussion on %MTD_OPS_RAW vs.
+ * %MTD_FILE_MODE_RAW.
  */
 enum {
 	MTD_OPS_PLACE_OOB = 0,
@@ -60,6 +64,22 @@ enum {
 	MTD_OPS_RAW = 2,
 };
 
+/**
+ * struct mtd_write_req - data structure for requesting a write operation
+ *
+ * @start:	start address
+ * @len:	length of data buffer
+ * @ooblen:	length of OOB buffer
+ * @usr_data:	user-provided data buffer
+ * @usr_oob:	user-provided OOB buffer
+ * @mode:	MTD mode (see "MTD operation modes")
+ * @padding:	reserved, must be set to 0
+ *
+ * This structure supports ioctl(MEMWRITE) operations, allowing data and/or OOB
+ * writes in various modes. To write to OOB-only, set @usr_data == NULL, and to
+ * write data-only, set @usr_oob == NULL. However, setting both @usr_data and
+ * @usr_oob to NULL is not allowed.
+ */
 struct mtd_write_req {
 	__u64 start;
 	__u32 len;
@@ -84,13 +104,13 @@ struct mtd_write_req {
 #define MTD_NO_ERASE		0x1000	/* No erase necessary */
 #define MTD_POWERUP_LOCK	0x2000	/* Always locked after reset */
 
-// Some common devices / combinations of capabilities
+/* Some common devices / combinations of capabilities */
 #define MTD_CAP_ROM		0
 #define MTD_CAP_RAM		(MTD_WRITEABLE | MTD_BIT_WRITEABLE | MTD_NO_ERASE)
 #define MTD_CAP_NORFLASH	(MTD_WRITEABLE | MTD_BIT_WRITEABLE)
 #define MTD_CAP_NANDFLASH	(MTD_WRITEABLE)
 
-/* ECC byte placement */
+/* Obsolete ECC byte placement modes (used with obsolete MEMGETOOBSEL) */
 #define MTD_NANDECC_OFF		0	// Switch off ECC (Not recommended)
 #define MTD_NANDECC_PLACE	1	// Use the given placement in the structure (YAFFS1 legacy mode)
 #define MTD_NANDECC_AUTOPLACE	2	// Use the default placement scheme
@@ -105,10 +125,10 @@ struct mtd_write_req {
 struct mtd_info_user {
 	__u8 type;
 	__u32 flags;
-	__u32 size;	 // Total size of the MTD
+	__u32 size;	/* Total size of the MTD */
 	__u32 erasesize;
 	__u32 writesize;
-	__u32 oobsize;   // Amount of OOB data per block (e.g. 16)
+	__u32 oobsize;	/* Amount of OOB data per block (e.g. 16) */
 	/* The below two fields are obsolete and broken, do not use them
 	 * (TODO: remove at some point) */
 	__u32 ecctype;
@@ -117,9 +137,9 @@ struct mtd_info_user {
 
 struct region_info_user {
 	__u32 offset;		/* At which this region starts,
-					 * from the beginning of the MTD */
-	__u32 erasesize;		/* For this region */
-	__u32 numblocks;		/* Number of blocks in this region */
+				 * from the beginning of the MTD */
+	__u32 erasesize;	/* For this region */
+	__u32 numblocks;	/* Number of blocks in this region */
 	__u32 regionindex;
 };
 
@@ -135,28 +155,54 @@ struct otp_info {
  * Try to avoid adding a new ioctl with the same ioctl number.
  */
 
+/* Get basic MTD characteristics info (better to use sysfs) */
 #define MEMGETINFO		_IOR('M', 1, struct mtd_info_user)
+/* Erase segment of MTD */
 #define MEMERASE		_IOW('M', 2, struct erase_info_user)
+/* Write out-of-band data from MTD */
 #define MEMWRITEOOB		_IOWR('M', 3, struct mtd_oob_buf)
+/* Read out-of-band data from MTD */
 #define MEMREADOOB		_IOWR('M', 4, struct mtd_oob_buf)
+/* Lock a chip (for MTD that supports it) */
 #define MEMLOCK			_IOW('M', 5, struct erase_info_user)
+/* Unlock a chip (for MTD that supports it) */
 #define MEMUNLOCK		_IOW('M', 6, struct erase_info_user)
+/* Get the number of different erase regions */
 #define MEMGETREGIONCOUNT	_IOR('M', 7, int)
+/* Get information about the erase region for a specific index */
 #define MEMGETREGIONINFO	_IOWR('M', 8, struct region_info_user)
+/* Get info about OOB modes (e.g., RAW, PLACE, AUTO) - legacy interface */
 #define MEMGETOOBSEL		_IOR('M', 10, struct nand_oobinfo)
+/* Check if an eraseblock is bad */
 #define MEMGETBADBLOCK		_IOW('M', 11, __kernel_loff_t)
+/* Mark an eraseblock as bad */
 #define MEMSETBADBLOCK		_IOW('M', 12, __kernel_loff_t)
+/* Set OTP (One-Time Programmable) mode (factory vs. user) */
 #define OTPSELECT		_IOR('M', 13, int)
+/* Get number of OTP (One-Time Programmable) regions */
 #define OTPGETREGIONCOUNT	_IOW('M', 14, int)
+/* Get all OTP (One-Time Programmable) info about MTD */
 #define OTPGETREGIONINFO	_IOW('M', 15, struct otp_info)
+/* Lock a given range of user data (must be in mode %MTD_FILE_MODE_OTP_USER) */
 #define OTPLOCK			_IOR('M', 16, struct otp_info)
+/* Get ECC layout (deprecated) */
 #define ECCGETLAYOUT		_IOR('M', 17, struct nand_ecclayout_user)
+/* Get statistics about corrected/uncorrected errors */
 #define ECCGETSTATS		_IOR('M', 18, struct mtd_ecc_stats)
+/* Set MTD mode on a per-file-descriptor basis (see "MTD file modes") */
 #define MTDFILEMODE		_IO('M', 19)
+/* Erase segment of MTD (supports 64-bit address) */
 #define MEMERASE64		_IOW('M', 20, struct erase_info_user64)
+/* Write data to OOB (64-bit version) */
 #define MEMWRITEOOB64		_IOWR('M', 21, struct mtd_oob_buf64)
+/* Read data from OOB (64-bit version) */
 #define MEMREADOOB64		_IOWR('M', 22, struct mtd_oob_buf64)
+/* Check if chip is locked (for MTD that supports it) */
 #define MEMISLOCKED		_IOR('M', 23, struct erase_info_user)
+/*
+ * Most generic write interface; can write in-band and/or out-of-band in various
+ * modes (see "struct mtd_write_req")
+ */
 #define MEMWRITE		_IOWR('M', 24, struct mtd_write_req)
 
 /*
@@ -208,7 +254,21 @@ struct mtd_ecc_stats {
 };
 
 /*
- * Read/write file modes for access to MTD
+ * MTD file modes - for read/write access to MTD
+ *
+ * @MTD_FILE_MODE_NORMAL:	OTP disabled, ECC enabled
+ * @MTD_FILE_MODE_OTP_FACTORY:	OTP enabled in factory mode
+ * @MTD_FILE_MODE_OTP_USER:	OTP enabled in user mode
+ * @MTD_FILE_MODE_RAW:		OTP disabled, ECC disabled
+ *
+ * These modes can be set via ioctl(MTDFILEMODE). The mode mode will be retained
+ * separately for each open file descriptor.
+ *
+ * Note: %MTD_FILE_MODE_RAW provides the same functionality as %MTD_OPS_RAW -
+ * raw access to the flash, without error correction or autoplacement schemes.
+ * Wherever possible, the MTD_OPS_* mode will override the MTD_FILE_MODE_* mode
+ * (e.g., when using ioctl(MEMWRITE)), but in some cases, the MTD_FILE_MODE is
+ * used out of necessity (e.g., `write()', ioctl(MEMWRITEOOB64)).
  */
 enum mtd_file_modes {
 	MTD_FILE_MODE_NORMAL = MTD_OTP_OFF,
-- 
1.7.5.4

Re: [PATCH 09/12] mtd: document ABI

[Artem Bityutskiy]

On Tue, 2011-08-30 at 18:45 -0700, Brian Norris wrote:
> We're missing a lot of important documentation in include/mtd/mtd-abi.h:
> 
> * add a simple description of each ioctl (feel free to expand!)
> * give full explanations of recently added and modified operations
> * explain the usage of "RAW" that appear in different modes and types of
>   operations
> * fix some comment style along the way
> 
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>

Great piece of work, pushed, thanks a lot!

-- 
Best Regards,
Artem Bityutskiy

[PATCH 10/12] mtd: nand: kill member `ops' of `struct nand_chip'

[Brian Norris]

The nand_chip.ops field is a struct that is passed around globally with
no particular reason. Every time it is used, it could just as easily be
replaced with a local struct that is updated on each operation. So make
it local.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/nand/nand_base.c |   46 ++++++++++++++++++++++-------------------
 include/linux/mtd/nand.h     |    3 --
 2 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index e5c6a13..de2d1c4 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -406,6 +406,8 @@ static int nand_default_block_markbad(struct mtd_info *mtd, loff_t ofs)
 	if (chip->bbt_options & NAND_BBT_USE_FLASH)
 		ret = nand_update_bbt(mtd, ofs);
 	else {
+		struct mtd_oob_ops ops;
+
 		nand_get_device(chip, mtd, FL_WRITING);
 
 		/*
@@ -414,13 +416,12 @@ static int nand_default_block_markbad(struct mtd_info *mtd, loff_t ofs)
 		 * procedure. We write two bytes per location, so we dont have
 		 * to mess with 16 bit access.
 		 */
+		ops.len = ops.ooblen = 2;
+		ops.datbuf = NULL;
+		ops.oobbuf = buf;
+		ops.ooboffs = chip->badblockpos & ~0x01;
 		do {
-			chip->ops.len = chip->ops.ooblen = 2;
-			chip->ops.datbuf = NULL;
-			chip->ops.oobbuf = buf;
-			chip->ops.ooboffs = chip->badblockpos & ~0x01;
-
-			ret = nand_do_write_oob(mtd, ofs, &chip->ops);
+			ret = nand_do_write_oob(mtd, ofs, &ops);
 
 			i++;
 			ofs += mtd->writesize;
@@ -1573,6 +1574,7 @@ static int nand_read(struct mtd_info *mtd, loff_t from, size_t len,
 		     size_t *retlen, uint8_t *buf)
 {
 	struct nand_chip *chip = mtd->priv;
+	struct mtd_oob_ops ops;
 	int ret;
 
 	/* Do not allow reads past end of device */
@@ -1583,13 +1585,13 @@ static int nand_read(struct mtd_info *mtd, loff_t from, size_t len,
 
 	nand_get_device(chip, mtd, FL_READING);
 
-	chip->ops.len = len;
-	chip->ops.datbuf = buf;
-	chip->ops.oobbuf = NULL;
+	ops.len = len;
+	ops.datbuf = buf;
+	ops.oobbuf = NULL;
 
-	ret = nand_do_read_ops(mtd, from, &chip->ops);
+	ret = nand_do_read_ops(mtd, from, &ops);
 
-	*retlen = chip->ops.retlen;
+	*retlen = ops.retlen;
 
 	nand_release_device(mtd);
 
@@ -2278,6 +2280,7 @@ static int panic_nand_write(struct mtd_info *mtd, loff_t to, size_t len,
 			    size_t *retlen, const uint8_t *buf)
 {
 	struct nand_chip *chip = mtd->priv;
+	struct mtd_oob_ops ops;
 	int ret;
 
 	/* Do not allow reads past end of device */
@@ -2292,13 +2295,13 @@ static int panic_nand_write(struct mtd_info *mtd, loff_t to, size_t len,
 	/* Grab the device */
 	panic_nand_get_device(chip, mtd, FL_WRITING);
 
-	chip->ops.len = len;
-	chip->ops.datbuf = (uint8_t *)buf;
-	chip->ops.oobbuf = NULL;
+	ops.len = len;
+	ops.datbuf = (uint8_t *)buf;
+	ops.oobbuf = NULL;
 
-	ret = nand_do_write_ops(mtd, to, &chip->ops);
+	ret = nand_do_write_ops(mtd, to, &ops);
 
-	*retlen = chip->ops.retlen;
+	*retlen = ops.retlen;
 	return ret;
 }
 
@@ -2316,6 +2319,7 @@ static int nand_write(struct mtd_info *mtd, loff_t to, size_t len,
 			  size_t *retlen, const uint8_t *buf)
 {
 	struct nand_chip *chip = mtd->priv;
+	struct mtd_oob_ops ops;
 	int ret;
 
 	/* Do not allow reads past end of device */
@@ -2326,13 +2330,13 @@ static int nand_write(struct mtd_info *mtd, loff_t to, size_t len,
 
 	nand_get_device(chip, mtd, FL_WRITING);
 
-	chip->ops.len = len;
-	chip->ops.datbuf = (uint8_t *)buf;
-	chip->ops.oobbuf = NULL;
+	ops.len = len;
+	ops.datbuf = (uint8_t *)buf;
+	ops.oobbuf = NULL;
 
-	ret = nand_do_write_ops(mtd, to, &chip->ops);
+	ret = nand_do_write_ops(mtd, to, &ops);
 
-	*retlen = chip->ops.retlen;
+	*retlen = ops.retlen;
 
 	nand_release_device(mtd);
 
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 0b3d464..904131b 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -427,7 +427,6 @@ struct nand_buffers {
  * @ecc:		[BOARDSPECIFIC] ECC control structure
  * @buffers:		buffer structure for read/write
  * @hwcontrol:		platform-specific hardware control structure
- * @ops:		oob operation operands
  * @erase_cmd:		[INTERN] erase command write function, selectable due
  *			to AND support.
  * @scan_bbt:		[REPLACEABLE] function to scan bad block table
@@ -535,8 +534,6 @@ struct nand_chip {
 	struct nand_buffers *buffers;
 	struct nand_hw_control hwcontrol;
 
-	struct mtd_oob_ops ops;
-
 	uint8_t *bbt;
 	struct nand_bbt_descr *bbt_td;
 	struct nand_bbt_descr *bbt_md;
-- 
1.7.5.4

Re: [PATCH 10/12] mtd: nand: kill member `ops' of `struct nand_chip'

[Artem Bityutskiy]

On Tue, 2011-08-30 at 18:45 -0700, Brian Norris wrote:
> The nand_chip.ops field is a struct that is passed around globally with
> no particular reason. Every time it is used, it could just as easily be
> replaced with a local struct that is updated on each operation. So make
> it local.
> 
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>

Looks like a good clean-up, thanks, pushed to my tree.

-- 
Best Regards,
Artem Bityutskiy

[PATCH 11/12] mtd: kill old field for `struct mtd_info_user'

[Brian Norris]

The ecctype and eccsize fields have been obsolete for a while. Since they
don't have any users, we can kill them and leave padding in their place
for now.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/mtdchar.c |    4 ++--
 include/mtd/mtd-abi.h |    5 +----
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
index 1547e2a..8feb5fd 100644
--- a/drivers/mtd/mtdchar.c
+++ b/drivers/mtd/mtdchar.c
@@ -672,8 +672,8 @@ static int mtd_ioctl(struct file *file, u_int cmd, u_long arg)
 		info.erasesize	= mtd->erasesize;
 		info.writesize	= mtd->writesize;
 		info.oobsize	= mtd->oobsize;
-		/* The below fields are obsolete */
-		info.ecctype	= -1;
+		/* The below field is obsolete */
+		info.padding	= 0;
 		if (copy_to_user(argp, &info, sizeof(struct mtd_info_user)))
 			return -EFAULT;
 		break;
diff --git a/include/mtd/mtd-abi.h b/include/mtd/mtd-abi.h
index d54ccf3..8138767 100644
--- a/include/mtd/mtd-abi.h
+++ b/include/mtd/mtd-abi.h
@@ -129,10 +129,7 @@ struct mtd_info_user {
 	__u32 erasesize;
 	__u32 writesize;
 	__u32 oobsize;	/* Amount of OOB data per block (e.g. 16) */
-	/* The below two fields are obsolete and broken, do not use them
-	 * (TODO: remove at some point) */
-	__u32 ecctype;
-	__u32 eccsize;
+	__u64 padding;	/* Old obsolete field; do not use */
 };
 
 struct region_info_user {
-- 
1.7.5.4

Re: [PATCH 11/12] mtd: kill old field for `struct mtd_info_user'

[Artem Bityutskiy]

On Tue, 2011-08-30 at 18:45 -0700, Brian Norris wrote:
> The ecctype and eccsize fields have been obsolete for a while. Since they
> don't have any users, we can kill them and leave padding in their place
> for now.

Actually not for now, but forever :-) Pushed to my tree, thanks!

-- 
Best Regards,
Artem Bityutskiy

[PATCH 12/12] mtd: nand: free allocated memory

[Brian Norris]

This fixes a problem from:

  commit f2f4772692cf3f31eb73d46337c7f10bd031bc26
  mtd: nand: remove NAND_BBT_SCANBYTE1AND6 option

In reverting pieces of another commit, I accidentally removed a line
that shold have stayed. We should free up the memory we allocated upon
releasing our device.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
This should probably just be squashed in with the buggy commit (it
hasn't been brought upstream yet...)

 drivers/mtd/nand/nand_base.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index de2d1c4..c9767b5 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3533,6 +3533,11 @@ void nand_release(struct mtd_info *mtd)
 	kfree(chip->bbt);
 	if (!(chip->options & NAND_OWN_BUFFERS))
 		kfree(chip->buffers);
+
+	/* Free bad block descriptor memory */
+	if (chip->badblock_pattern && chip->badblock_pattern->options
+			& NAND_BBT_DYNAMICSTRUCT)
+		kfree(chip->badblock_pattern);
 }
 EXPORT_SYMBOL_GPL(nand_release);
 
-- 
1.7.5.4

Re: [PATCH 12/12] mtd: nand: free allocated memory

[Artem Bityutskiy]

On Tue, 2011-08-30 at 18:45 -0700, Brian Norris wrote:
> This fixes a problem from:
> 
>   commit f2f4772692cf3f31eb73d46337c7f10bd031bc26
>   mtd: nand: remove NAND_BBT_SCANBYTE1AND6 option

Amended that commit, thanks!

-- 
Best Regards,
Artem Bityutskiy