[MTD] [NAND] remove len/ooblen confusion.

As was discussed between Ricard Wanderlöf, David Woodhouse, Artem
Bityutskiy and me, the current API for reading/writing OOB is confusing.

The thing that introduces confusion is the need to specify ops.len
together with ops.ooblen for reads/writes that concern only OOB not data
area. So, ops.len is overloaded: when ops.datbuf != NULL it serves to
specify the length of the data read, and when ops.datbuf == NULL, it
serves to specify the full OOB read length.

The patch inlined below is the slightly updated version of the previous
patch serving the same purpose, but with the new Artem's comments taken
into account.

Artem, BTW, thanks a lot for your valuable input!

Signed-off-by: Vitaly Wool <[e-mail]>
Signed-off-by: David Woodhouse <[e-mail]>

drivers/mtd/inftlcore.c

···
 	ops.ooblen = len;
 	ops.oobbuf = buf;
 	ops.datbuf = NULL;
-	ops.len = len;
 
 	res = mtd->read_oob(mtd, offs & ~(mtd->writesize - 1), &ops);
-	*retlen = ops.retlen;
+	*retlen = ops.oobretlen;
 	return res;
 }
 
···
 	ops.ooblen = len;
 	ops.oobbuf = buf;
 	ops.datbuf = NULL;
-	ops.len = len;
 
 	res = mtd->write_oob(mtd, offs & ~(mtd->writesize - 1), &ops);
-	*retlen = ops.retlen;
+	*retlen = ops.oobretlen;
 	return res;
 }
 

drivers/mtd/mtdchar.c

···
 		if (ret)
 			return ret;
 
-		ops.len = buf.length;
 		ops.ooblen = buf.length;
 		ops.ooboffs = buf.start & (mtd->oobsize - 1);
 		ops.datbuf = NULL;
 		ops.mode = MTD_OOB_PLACE;
 
-		if (ops.ooboffs && ops.len > (mtd->oobsize - ops.ooboffs))
+		if (ops.ooboffs && ops.ooblen > (mtd->oobsize - ops.ooboffs))
 			return -EINVAL;
 
 		ops.oobbuf = kmalloc(buf.length, GFP_KERNEL);
···
 		buf.start &= ~(mtd->oobsize - 1);
 		ret = mtd->write_oob(mtd, buf.start, &ops);
 
-		if (copy_to_user(argp + sizeof(uint32_t), &ops.retlen,
+		if (copy_to_user(argp + sizeof(uint32_t), &ops.oobretlen,
 				 sizeof(uint32_t)))
 			ret = -EFAULT;
 
···
 		if (ret)
 			return ret;
 
-		ops.len = buf.length;
 		ops.ooblen = buf.length;
 		ops.ooboffs = buf.start & (mtd->oobsize - 1);
 		ops.datbuf = NULL;
···
 		buf.start &= ~(mtd->oobsize - 1);
 		ret = mtd->read_oob(mtd, buf.start, &ops);
 
-		if (put_user(ops.retlen, (uint32_t __user *)argp))
+		if (put_user(ops.oobretlen, (uint32_t __user *)argp))
 			ret = -EFAULT;
-		else if (ops.retlen && copy_to_user(buf.ptr, ops.oobbuf,
-						    ops.retlen))
+		else if (ops.oobretlen && copy_to_user(buf.ptr, ops.oobbuf,
+						    ops.oobretlen))
 			ret = -EFAULT;
 
 		kfree(ops.oobbuf);

drivers/mtd/mtdconcat.c

···
 	struct mtd_oob_ops devops = *ops;
 	int i, err, ret = 0;
 
-	ops->retlen = 0;
+	ops->retlen = ops->oobretlen = 0;
 
 	for (i = 0; i < concat->num_subdev; i++) {
 		struct mtd_info *subdev = concat->subdev[i];
···
 
 		err = subdev->read_oob(subdev, from, &devops);
 		ops->retlen += devops.retlen;
+		ops->oobretlen += devops.oobretlen;
 
 		/* Save information about bitflips! */
 		if (unlikely(err)) {
···
 				return err;
 		}
 
-		devops.len = ops->len - ops->retlen;
-		if (!devops.len)
-			return ret;
-
-		if (devops.datbuf)
+		if (devops.datbuf) {
+			devops.len = ops->len - ops->retlen;
+			if (!devops.len)
+				return ret;
 			devops.datbuf += devops.retlen;
-		if (devops.oobbuf)
-			devops.oobbuf += devops.ooblen;
+		}
+		if (devops.oobbuf) {
+			devops.ooblen = ops->ooblen - ops->oobretlen;
+			if (!devops.ooblen)
+				return ret;
+			devops.oobbuf += ops->oobretlen;
+		}
 
 		from = 0;
 	}
···
 		if (err)
 			return err;
 
-		devops.len = ops->len - ops->retlen;
-		if (!devops.len)
-			return 0;
-
-		if (devops.datbuf)
+		if (devops.datbuf) {
+			devops.len = ops->len - ops->retlen;
+			if (!devops.len)
+				return 0;
 			devops.datbuf += devops.retlen;
-		if (devops.oobbuf)
-			devops.oobbuf += devops.ooblen;
+		}
+		if (devops.oobbuf) {
+			devops.ooblen = ops->ooblen - ops->oobretlen;
+			if (!devops.ooblen)
+				return 0;
+			devops.oobbuf += devops.oobretlen;
+		}
 		to = 0;
 	}
 	return -EINVAL;

drivers/mtd/mtdpart.c

···
 
 	if (from >= mtd->size)
 		return -EINVAL;
-	if (from + ops->len > mtd->size)
+	if (ops->datbuf && from + ops->len > mtd->size)
 		return -EINVAL;
 	res = part->master->read_oob(part->master, from + part->offset, ops);
 
···
 
 	if (to >= mtd->size)
 		return -EINVAL;
-	if (to + ops->len > mtd->size)
+	if (ops->datbuf && to + ops->len > mtd->size)
 		return -EINVAL;
 	return part->master->write_oob(part->master, to + part->offset, ops);
 }

drivers/mtd/nand/nand_base.c

···
  * @chip:	nand chip structure
  * @oob:	oob destination address
  * @ops:	oob ops structure
+ * @len:	size of oob to transfer
  */
 static uint8_t *nand_transfer_oob(struct nand_chip *chip, uint8_t *oob,
-				  struct mtd_oob_ops *ops)
+				  struct mtd_oob_ops *ops, size_t len)
 {
-	size_t len = ops->ooblen;
-
 	switch(ops->mode) {
 
 	case MTD_OOB_PLACE:
···
 	int sndcmd = 1;
 	int ret = 0;
 	uint32_t readlen = ops->len;
+	uint32_t oobreadlen = ops->ooblen;
 	uint8_t *bufpoi, *oob, *buf;
 
 	stats = mtd->ecc_stats;
···
 
 			if (unlikely(oob)) {
 				/* Raw mode does data:oob:data:oob */
-				if (ops->mode != MTD_OOB_RAW)
-					oob = nand_transfer_oob(chip, oob, ops);
-				else
-					buf = nand_transfer_oob(chip, buf, ops);
+				if (ops->mode != MTD_OOB_RAW) {
+					int toread = min(oobreadlen,
+						chip->ecc.layout->oobavail);
+					if (toread) {
+						oob = nand_transfer_oob(chip,
+							oob, ops, toread);
+						oobreadlen -= toread;
+					}
+				} else
+					buf = nand_transfer_oob(chip,
+						buf, ops, mtd->oobsize);
 			}
 
 			if (!(chip->options & NAND_NO_READRDY)) {
···
 	}
 
 	ops->retlen = ops->len - (size_t) readlen;
+	if (oob)
+		ops->oobretlen = ops->ooblen - oobreadlen;
 
 	if (ret)
 		return ret;
···
 	int page, realpage, chipnr, sndcmd = 1;
 	struct nand_chip *chip = mtd->priv;
 	int blkcheck = (1 << (chip->phys_erase_shift - chip->page_shift)) - 1;
-	int readlen = ops->len;
+	int readlen = ops->ooblen;
+	int len;
 	uint8_t *buf = ops->oobbuf;
 
 	DEBUG(MTD_DEBUG_LEVEL3, "nand_read_oob: from = 0x%08Lx, len = %i\n",
 	      (unsigned long long)from, readlen);
+
+	if (ops->mode == MTD_OOB_RAW)
+		len = mtd->oobsize;
+	else
+		len = chip->ecc.layout->oobavail;
 
 	chipnr = (int)(from >> chip->chip_shift);
 	chip->select_chip(mtd, chipnr);
···
 
 	while(1) {
 		sndcmd = chip->ecc.read_oob(mtd, chip, page, sndcmd);
-		buf = nand_transfer_oob(chip, buf, ops);
+
+		len = min(len, readlen);
+		buf = nand_transfer_oob(chip, buf, ops, len);
 
 		if (!(chip->options & NAND_NO_READRDY)) {
 			/*
···
 				nand_wait_ready(mtd);
 		}
 
-		readlen -= ops->ooblen;
+		readlen -= len;
 		if (!readlen)
 			break;
 
···
 			sndcmd = 1;
 	}
 
-	ops->retlen = ops->len;
+	ops->oobretlen = ops->ooblen;
 	return 0;
 }
 
···
 	ops->retlen = 0;
 
 	/* Do not allow reads past end of device */
-	if ((from + ops->len) > mtd->size) {
+	if (ops->datbuf && (from + ops->len) > mtd->size) {
 		DEBUG(MTD_DEBUG_LEVEL0, "nand_read_oob: "
 		      "Attempt read beyond end of device\n");
 		return -EINVAL;
···
 	}
 
 	ops->retlen = ops->len - writelen;
+	if (unlikely(oob))
+		ops->oobretlen = ops->ooblen;
 	return ret;
 }
 
···
 	struct nand_chip *chip = mtd->priv;
 
 	DEBUG(MTD_DEBUG_LEVEL3, "nand_write_oob: to = 0x%08x, len = %i\n",
-	      (unsigned int)to, (int)ops->len);
+	      (unsigned int)to, (int)ops->ooblen);
 
 	/* Do not allow write past end of page */
-	if ((ops->ooboffs + ops->len) > mtd->oobsize) {
+	if ((ops->ooboffs + ops->ooblen) > mtd->oobsize) {
 		DEBUG(MTD_DEBUG_LEVEL0, "nand_write_oob: "
 		      "Attempt to write past end of page\n");
 		return -EINVAL;
···
 	if (status)
 		return status;
 
-	ops->retlen = ops->len;
+	ops->oobretlen = ops->ooblen;
 
 	return 0;
 }
···
 	ops->retlen = 0;
 
 	/* Do not allow writes past end of device */
-	if ((to + ops->len) > mtd->size) {
+	if (ops->datbuf && (to + ops->len) > mtd->size) {
 		DEBUG(MTD_DEBUG_LEVEL0, "nand_read_oob: "
 		      "Attempt read beyond end of device\n");
 		return -EINVAL;

drivers/mtd/nand/nand_bbt.c

···
 	struct mtd_oob_ops ops;
 	int j, ret;
 
-	ops.len = mtd->oobsize;
 	ops.ooblen = mtd->oobsize;
 	ops.oobbuf = buf;
 	ops.ooboffs = 0;
···
 				       "bad block table\n");
 			}
 			/* Read oob data */
-			ops.len = (len >> this->page_shift) * mtd->oobsize;
+			ops.ooblen = (len >> this->page_shift) * mtd->oobsize;
 			ops.oobbuf = &buf[len];
 			res = mtd->read_oob(mtd, to + mtd->writesize, &ops);
-			if (res < 0 || ops.retlen != ops.len)
+			if (res < 0 || ops.oobretlen != ops.ooblen)
 				goto outerr;
 
 			/* Calc the byte offset in the buffer */

drivers/mtd/nftlcore.c

···
 	ops.ooblen = len;
 	ops.oobbuf = buf;
 	ops.datbuf = NULL;
-	ops.len = len;
 
 	res = mtd->read_oob(mtd, offs & ~(mtd->writesize - 1), &ops);
-	*retlen = ops.retlen;
+	*retlen = ops.oobretlen;
 	return res;
 }
 
···
 	ops.ooblen = len;
 	ops.oobbuf = buf;
 	ops.datbuf = NULL;
-	ops.len = len;
 
 	res = mtd->write_oob(mtd, offs & ~(mtd->writesize - 1), &ops);
-	*retlen = ops.retlen;
+	*retlen = ops.oobretlen;
 	return res;
 }
 

drivers/mtd/ssfdc.c

···
 
 	ops.mode = MTD_OOB_RAW;
 	ops.ooboffs = 0;
-	ops.ooblen = mtd->oobsize;
-	ops.len = OOB_SIZE;
+	ops.ooblen = OOB_SIZE;
 	ops.oobbuf = buf;
 	ops.datbuf = NULL;
 
 	ret = mtd->read_oob(mtd, offs, &ops);
-	if (ret < 0 || ops.retlen != OOB_SIZE)
+	if (ret < 0 || ops.oobretlen != OOB_SIZE)
 		return -1;
 
 	return 0;

fs/jffs2/wbuf.c

···
 	int oobsize = c->mtd->oobsize;
 	struct mtd_oob_ops ops;
 
-	ops.len = NR_OOB_SCAN_PAGES * oobsize;
-	ops.ooblen = oobsize;
+	ops.ooblen = NR_OOB_SCAN_PAGES * oobsize;
 	ops.oobbuf = c->oobbuf;
 	ops.ooboffs = 0;
 	ops.datbuf = NULL;
···
 		return ret;
 	}
 
-	if (ops.retlen < ops.len) {
+	if (ops.oobretlen < ops.ooblen) {
 		D1(printk(KERN_WARNING "jffs2_check_oob_empty(): Read OOB "
 			  "returned short read (%zd bytes not %d) for block "
-			  "at %08x\n", ops.retlen, ops.len, jeb->offset));
+			  "at %08x\n", ops.oobretlen, ops.ooblen, jeb->offset));
 		return -EIO;
 	}
 
···
 	}
 
 	/* we know, we are aligned :) */
-	for (page = oobsize; page < ops.len; page += sizeof(long)) {
+	for (page = oobsize; page < ops.ooblen; page += sizeof(long)) {
 		long dat = *(long *)(&ops.oobbuf[page]);
 		if(dat != -1)
 			return 1;
···
 		return 2;
 	}
 
-	ops.len = oobsize;
 	ops.ooblen = oobsize;
 	ops.oobbuf = c->oobbuf;
 	ops.ooboffs = 0;
···
 		return ret;
 	}
 
-	if (ops.retlen < ops.len) {
+	if (ops.oobretlen < ops.ooblen) {
 		D1 (printk (KERN_WARNING "jffs2_check_nand_cleanmarker(): "
 			    "Read OOB return short read (%zd bytes not %d) "
-			    "for block at %08x\n", ops.retlen, ops.len,
+			    "for block at %08x\n", ops.oobretlen, ops.ooblen,
 			    jeb->offset));
 		return -EIO;
 	}
···
 	n.nodetype = cpu_to_je16(JFFS2_NODETYPE_CLEANMARKER);
 	n.totlen = cpu_to_je32(8);
 
-	ops.len = c->fsdata_len;
-	ops.ooblen = c->fsdata_len;;
+	ops.ooblen = c->fsdata_len;
 	ops.oobbuf = (uint8_t *)&n;
 	ops.ooboffs = c->fsdata_pos;
 	ops.datbuf = NULL;
···
 			  jeb->offset, ret));
 		return ret;
 	}
-	if (ops.retlen != ops.len) {
+	if (ops.oobretlen != ops.ooblen) {
 		D1(printk(KERN_WARNING "jffs2_write_nand_cleanmarker(): "
 			  "Short write for block at %08x: %zd not %d\n",
-			  jeb->offset, ops.retlen, ops.len));
+			  jeb->offset, ops.oobretlen, ops.ooblen));
 		return -EIO;
 	}
 	return 0;

include/linux/mtd/mtd.h

···
  * struct mtd_oob_ops - oob operation operands
  * @mode:	operation mode
  *
- * @len:	number of bytes to write/read. When a data buffer is given
- *		(datbuf != NULL) this is the number of data bytes. When
- *		no data buffer is available this is the number of oob bytes.
+ * @len:	number of data bytes to write/read
  *
- * @retlen:	number of bytes written/read. When a data buffer is given
- *		(datbuf != NULL) this is the number of data bytes. When
- *		no data buffer is available this is the number of oob bytes.
+ * @retlen:	number of data bytes written/read
  *
- * @ooblen:	number of oob bytes per page
+ * @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)
  * @datbuf:	data buffer - if NULL only oob data are read/written
···
 	size_t		len;
 	size_t		retlen;
 	size_t		ooblen;
+	size_t		oobretlen;
 	uint32_t	ooboffs;
 	uint8_t		*datbuf;
 	uint8_t		*oobbuf;