[PATCH 0/8] mtd: work on mtdraw

[PATCH 0/8] mtd: work on mtdraw

[Sascha Hauer]

I did several cleanups on the mtdraw device until I finally found the
bug that nagged me. The alignment check when writing is wrong. On write
entry we have to check if the offset is raw page size aligned and not
writesize aligned. With that I could successfully use fastboot to write
a raw image to NAND.

Sascha Hauer (8):
  mtd: mtdraw: Simplify error path
  mtd: mtdraw: use dev_* for printing
  mtd: mtdraw: do not write empty buffers
  mtd: mtdraw: pass mtdraw around
  mtd: mtdraw: add raw page size to private data
  mtd: mtdraw: fail when writing fails
  mtd: mtdraw: replace "block" with "page"
  mtd: mtdraw: fix wrong alignment check

 drivers/mtd/mtdraw.c | 124 +++++++++++++++++++++----------------------
 1 file changed, 62 insertions(+), 62 deletions(-)

-- 
2.19.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 1/8] mtd: mtdraw: Simplify error path

[Sascha Hauer]

We do not need a printf when writing fails, it's the callers job
to report this. Remove printf and simplify the error path a bit.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mtd/mtdraw.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/mtd/mtdraw.c b/drivers/mtd/mtdraw.c
index 4f7c3b836c..d0dde0dee2 100644
--- a/drivers/mtd/mtdraw.c
+++ b/drivers/mtd/mtdraw.c
@@ -146,11 +146,11 @@ static ssize_t mtdraw_read(struct cdev *cdev, void *buf, size_t count,
 		count -= ret;
 		retlen += ret;
 	}
+
 	if (ret < 0)
-		printf("err %zd\n", ret);
-	else
-		ret = retlen;
-	return ret;
+		return ret;
+
+	return retlen;
 }
 
 #ifdef CONFIG_MTD_WRITE
@@ -231,12 +231,10 @@ static ssize_t mtdraw_write(struct cdev *cdev, const void *buf, size_t count,
 		retlen += count;
 	}
 
-	if (ret < 0) {
-		printf("err %d\n", ret);
+	if (ret < 0)
 		return ret;
-	} else {
-		return retlen;
-	}
+
+	return retlen;
 }
 
 static int mtdraw_erase(struct cdev *cdev, loff_t count, loff_t offset)
-- 
2.19.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/8] mtd: mtdraw: use dev_* for printing

[Sascha Hauer]

We have a struct device_d *, so use dev_* for printing instead of
printf.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mtd/mtdraw.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/mtdraw.c b/drivers/mtd/mtdraw.c
index d0dde0dee2..3ad9de80a2 100644
--- a/drivers/mtd/mtdraw.c
+++ b/drivers/mtd/mtdraw.c
@@ -252,7 +252,8 @@ static int mtdraw_erase(struct cdev *cdev, loff_t count, loff_t offset)
 	erase.len = mtd->erasesize;
 
 	while (count > 0) {
-		debug("erase 0x%08llx len: 0x%08llx\n", erase.addr, erase.len);
+		dev_dbg(&mtd->class_dev, "erase 0x%08llx len: 0x%08llx\n",
+			erase.addr, erase.len);
 
 		if (!mtd->allow_erasebad)
 			ret = mtd_block_isbad(mtd, erase.addr);
@@ -260,7 +261,8 @@ static int mtdraw_erase(struct cdev *cdev, loff_t count, loff_t offset)
 			ret = 0;
 
 		if (ret > 0) {
-			printf("Skipping bad block at 0x%08llx\n", erase.addr);
+			dev_info(&mtd->class_dev, "Skipping bad block at 0x%08llx\n",
+				 erase.addr);
 		} else {
 			ret = mtd_erase(mtd, &erase);
 			if (ret)
-- 
2.19.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/8] mtd: mtdraw: use dev_* for printing

[Sam Ravnborg]

Hi Sascha.

My OCD triggered here...

On Fri, Sep 21, 2018 at 01:18:14PM +0200, Sascha Hauer wrote:
> We have a struct device_d *, so use dev_* for printing instead of
> printf.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/mtd/mtdraw.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/mtdraw.c b/drivers/mtd/mtdraw.c
> index d0dde0dee2..3ad9de80a2 100644
> --- a/drivers/mtd/mtdraw.c
> +++ b/drivers/mtd/mtdraw.c
> @@ -252,7 +252,8 @@ static int mtdraw_erase(struct cdev *cdev, loff_t count, loff_t offset)
>  	erase.len = mtd->erasesize;
>  
>  	while (count > 0) {
> -		debug("erase 0x%08llx len: 0x%08llx\n", erase.addr, erase.len);
> +		dev_dbg(&mtd->class_dev, "erase 0x%08llx len: 0x%08llx\n",
> +			erase.addr, erase.len);
"erase" with lower case "e"

>  
>  		if (!mtd->allow_erasebad)
>  			ret = mtd_block_isbad(mtd, erase.addr);
> @@ -260,7 +261,8 @@ static int mtdraw_erase(struct cdev *cdev, loff_t count, loff_t offset)
>  			ret = 0;
>  
>  		if (ret > 0) {
> -			printf("Skipping bad block at 0x%08llx\n", erase.addr);
> +			dev_info(&mtd->class_dev, "Skipping bad block at 0x%08llx\n",
> +				 erase.addr);
Skipping with upper case "S".

Feel free to ignore.

	Sam

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/8] mtd: mtdraw: use dev_* for printing

[Sascha Hauer]

On Fri, Sep 21, 2018 at 04:33:19PM +0200, Sam Ravnborg wrote:
> Hi Sascha.
> 
> My OCD triggered here...
> 
> On Fri, Sep 21, 2018 at 01:18:14PM +0200, Sascha Hauer wrote:
> > We have a struct device_d *, so use dev_* for printing instead of
> > printf.
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  drivers/mtd/mtdraw.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/mtd/mtdraw.c b/drivers/mtd/mtdraw.c
> > index d0dde0dee2..3ad9de80a2 100644
> > --- a/drivers/mtd/mtdraw.c
> > +++ b/drivers/mtd/mtdraw.c
> > @@ -252,7 +252,8 @@ static int mtdraw_erase(struct cdev *cdev, loff_t count, loff_t offset)
> >  	erase.len = mtd->erasesize;
> >  
> >  	while (count > 0) {
> > -		debug("erase 0x%08llx len: 0x%08llx\n", erase.addr, erase.len);
> > +		dev_dbg(&mtd->class_dev, "erase 0x%08llx len: 0x%08llx\n",
> > +			erase.addr, erase.len);
> "erase" with lower case "e"
> 
> >  
> >  		if (!mtd->allow_erasebad)
> >  			ret = mtd_block_isbad(mtd, erase.addr);
> > @@ -260,7 +261,8 @@ static int mtdraw_erase(struct cdev *cdev, loff_t count, loff_t offset)
> >  			ret = 0;
> >  
> >  		if (ret > 0) {
> > -			printf("Skipping bad block at 0x%08llx\n", erase.addr);
> > +			dev_info(&mtd->class_dev, "Skipping bad block at 0x%08llx\n",
> > +				 erase.addr);
> Skipping with upper case "S".
> 
> Feel free to ignore.

Yeah, it's inconsistent. It was inconsistent before and I can't decide
which one I like better, so I keep it as is for now ;)

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 3/8] mtd: mtdraw: do not write empty buffers

[Sascha Hauer]

When a page is completely 0xff then do not write it since users
expect pages to be writable when they are empty.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mtd/mtdraw.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mtd/mtdraw.c b/drivers/mtd/mtdraw.c
index 3ad9de80a2..bfae795bd7 100644
--- a/drivers/mtd/mtdraw.c
+++ b/drivers/mtd/mtdraw.c
@@ -165,6 +165,9 @@ static ssize_t mtdraw_blkwrite(struct mtd_info *mtd, const void *buf,
 	struct mtd_oob_ops ops;
 	int ret;
 
+	if (mtd_buf_all_ff(buf, mtd->writesize + mtd->oobsize))
+		return 0;
+
 	ops.mode = MTD_OPS_RAW;
 	ops.ooboffs = 0;
 	ops.datbuf = (void *)buf;
-- 
2.19.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 4/8] mtd: mtdraw: pass mtdraw around

[Sascha Hauer]

Pass mtdraw around internally instead of mtd. mtd can be accessed
easily from mtdraw, so we can also drop to_mtd().

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mtd/mtdraw.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/mtd/mtdraw.c b/drivers/mtd/mtdraw.c
index bfae795bd7..089dc54eb4 100644
--- a/drivers/mtd/mtdraw.c
+++ b/drivers/mtd/mtdraw.c
@@ -77,14 +77,10 @@ static struct mtdraw *to_mtdraw(struct cdev *cdev)
 	return cdev->priv;
 }
 
-static struct mtd_info *to_mtd(struct cdev *cdev)
+static unsigned int mtdraw_offset_to_block(struct mtdraw *mtdraw, loff_t offset)
 {
-	struct mtdraw *mtdraw = to_mtdraw(cdev);
-	return mtdraw->mtd;
-}
+	struct mtd_info *mtd = mtdraw->mtd;
 
-static unsigned int mtdraw_offset_to_block(struct mtd_info *mtd, loff_t offset)
-{
 	u64 ofs64 = offset;
 
 	do_div(ofs64, mtd->writesize + mtd->oobsize);
@@ -128,12 +124,13 @@ err:
 static ssize_t mtdraw_read(struct cdev *cdev, void *buf, size_t count,
 			    loff_t offset, ulong flags)
 {
-	struct mtd_info *mtd = to_mtd(cdev);
+	struct mtdraw *mtdraw = to_mtdraw(cdev);
+	struct mtd_info *mtd = mtdraw->mtd;
 	ssize_t retlen = 0, ret = 1, toread;
 	ulong numblock;
 	int skip;
 
-	numblock = mtdraw_offset_to_block(mtd, offset);
+	numblock = mtdraw_offset_to_block(mtdraw, offset);
 	skip = offset - numblock * (mtd->writesize + mtd->oobsize);
 
 	while (ret > 0 && count > 0) {
@@ -154,9 +151,11 @@ static ssize_t mtdraw_read(struct cdev *cdev, void *buf, size_t count,
 }
 
 #ifdef CONFIG_MTD_WRITE
-static loff_t mtdraw_raw_to_mtd_offset(struct mtd_info *mtd, loff_t offset)
+static loff_t mtdraw_raw_to_mtd_offset(struct mtdraw *mtdraw, loff_t offset)
 {
-	return (loff_t)mtdraw_offset_to_block(mtd, offset) * mtd->writesize;
+	struct mtd_info *mtd = mtdraw->mtd;
+
+	return (loff_t)mtdraw_offset_to_block(mtdraw, offset) * mtd->writesize;
 }
 
 static ssize_t mtdraw_blkwrite(struct mtd_info *mtd, const void *buf,
@@ -190,13 +189,13 @@ static ssize_t mtdraw_write(struct cdev *cdev, const void *buf, size_t count,
 			    loff_t offset, ulong flags)
 {
 	struct mtdraw *mtdraw = to_mtdraw(cdev);
-	struct mtd_info *mtd = to_mtd(cdev);
+	struct mtd_info *mtd = mtdraw->mtd;
 	int bsz = mtd->writesize + mtd->oobsize;
 	ulong numblock;
 	size_t retlen = 0, tofill;
 	int ret = 0;
 
-	numblock = mtdraw_offset_to_block(mtd, offset);
+	numblock = mtdraw_offset_to_block(mtdraw, offset);
 
 	if (mtdraw->write_fill &&
 	    mtdraw->write_ofs + mtdraw->write_fill != offset)
@@ -213,13 +212,13 @@ static ssize_t mtdraw_write(struct cdev *cdev, const void *buf, size_t count,
 	}
 
 	if (mtdraw->write_fill == bsz) {
-		numblock = mtdraw_offset_to_block(mtd, mtdraw->write_ofs);
+		numblock = mtdraw_offset_to_block(mtdraw, mtdraw->write_ofs);
 		ret = mtdraw_blkwrite(mtd, mtdraw->writebuf,
 				      mtd->writesize * numblock);
 		mtdraw->write_fill = 0;
 	}
 
-	numblock = mtdraw_offset_to_block(mtd, offset);
+	numblock = mtdraw_offset_to_block(mtdraw, offset);
 	while (ret >= 0 && count >= bsz) {
 		ret = mtdraw_blkwrite(mtd, buf + retlen,
 				   mtd->writesize * numblock++);
@@ -242,12 +241,13 @@ static ssize_t mtdraw_write(struct cdev *cdev, const void *buf, size_t count,
 
 static int mtdraw_erase(struct cdev *cdev, loff_t count, loff_t offset)
 {
-	struct mtd_info *mtd = to_mtd(cdev);
+	struct mtdraw *mtdraw = to_mtdraw(cdev);
+	struct mtd_info *mtd = mtdraw->mtd;
 	struct erase_info erase;
 	int ret;
 
-	offset = mtdraw_raw_to_mtd_offset(mtd, offset);
-	count = mtdraw_raw_to_mtd_offset(mtd, count);
+	offset = mtdraw_raw_to_mtd_offset(mtdraw, offset);
+	count = mtdraw_raw_to_mtd_offset(mtdraw, count);
 
 	memset(&erase, 0, sizeof(erase));
 	erase.mtd = mtd;
-- 
2.19.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 5/8] mtd: mtdraw: add raw page size to private data

[Sascha Hauer]

The raw page size is used many times in the driver, so add a variable
to the private data instead of calculating it each time again.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mtd/mtdraw.c | 40 ++++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/mtd/mtdraw.c b/drivers/mtd/mtdraw.c
index 089dc54eb4..6518ae34d5 100644
--- a/drivers/mtd/mtdraw.c
+++ b/drivers/mtd/mtdraw.c
@@ -48,6 +48,7 @@
  * @write_fill: number of bytes in writebuf
  * @write_ofs: offset in character device (mtdraw) where last write(s) stored
  * bytes because of unaligned writes (ie. remain of writesize+oobsize write)
+ * @rps: raw page size (data + oob)
  *
  * The mtdraw device must allow unaligned writes. This is enabled by a write buffer which gathers data to issue mtd->write_oob() with full page+oob data.
  * Suppose writesize=512, oobsize=16.
@@ -70,6 +71,7 @@ struct mtdraw {
 	void *writebuf;
 	int write_fill;
 	int write_ofs;
+	unsigned int rps;
 };
 
 static struct mtdraw *to_mtdraw(struct cdev *cdev)
@@ -79,27 +81,26 @@ static struct mtdraw *to_mtdraw(struct cdev *cdev)
 
 static unsigned int mtdraw_offset_to_block(struct mtdraw *mtdraw, loff_t offset)
 {
-	struct mtd_info *mtd = mtdraw->mtd;
-
 	u64 ofs64 = offset;
 
-	do_div(ofs64, mtd->writesize + mtd->oobsize);
+	do_div(ofs64, mtdraw->rps);
 
 	return ofs64;
 }
 
-static ssize_t mtdraw_read_unaligned(struct mtd_info *mtd, void *dst,
+static ssize_t mtdraw_read_unaligned(struct mtdraw *mtdraw, void *dst,
 				     size_t count, int skip, ulong offset)
 {
+	struct mtd_info *mtd = mtdraw->mtd;
 	struct mtd_oob_ops ops;
 	ssize_t ret;
 	int partial = 0;
 	void *tmp = dst;
 
-	if (skip || count < mtd->writesize + mtd->oobsize)
+	if (skip || count < mtdraw->rps)
 		partial = 1;
 	if (partial)
-		tmp = malloc(mtd->writesize + mtd->oobsize);
+		tmp = malloc(mtdraw->rps);
 	if (!tmp)
 		return -ENOMEM;
 	ops.mode = MTD_OPS_RAW;
@@ -131,12 +132,11 @@ static ssize_t mtdraw_read(struct cdev *cdev, void *buf, size_t count,
 	int skip;
 
 	numblock = mtdraw_offset_to_block(mtdraw, offset);
-	skip = offset - numblock * (mtd->writesize + mtd->oobsize);
+	skip = offset - numblock * (mtdraw->rps);
 
 	while (ret > 0 && count > 0) {
-		toread = min_t(int, count,
-				mtd->writesize + mtd->oobsize - skip);
-		ret = mtdraw_read_unaligned(mtd, buf, toread,
+		toread = min_t(int, count, mtdraw->rps - skip);
+		ret = mtdraw_read_unaligned(mtdraw, buf, toread,
 					    skip, numblock++ * mtd->writesize);
 		buf += ret;
 		skip = 0;
@@ -158,13 +158,14 @@ static loff_t mtdraw_raw_to_mtd_offset(struct mtdraw *mtdraw, loff_t offset)
 	return (loff_t)mtdraw_offset_to_block(mtdraw, offset) * mtd->writesize;
 }
 
-static ssize_t mtdraw_blkwrite(struct mtd_info *mtd, const void *buf,
+static ssize_t mtdraw_blkwrite(struct mtdraw *mtdraw, const void *buf,
 			       ulong offset)
 {
+	struct mtd_info *mtd = mtdraw->mtd;
 	struct mtd_oob_ops ops;
 	int ret;
 
-	if (mtd_buf_all_ff(buf, mtd->writesize + mtd->oobsize))
+	if (mtd_buf_all_ff(buf, mtdraw->rps))
 		return 0;
 
 	ops.mode = MTD_OPS_RAW;
@@ -190,7 +191,6 @@ static ssize_t mtdraw_write(struct cdev *cdev, const void *buf, size_t count,
 {
 	struct mtdraw *mtdraw = to_mtdraw(cdev);
 	struct mtd_info *mtd = mtdraw->mtd;
-	int bsz = mtd->writesize + mtd->oobsize;
 	ulong numblock;
 	size_t retlen = 0, tofill;
 	int ret = 0;
@@ -204,23 +204,23 @@ static ssize_t mtdraw_write(struct cdev *cdev, const void *buf, size_t count,
 		return -EINVAL;
 
 	if (mtdraw->write_fill) {
-		tofill = min_t(size_t, count, bsz - mtdraw->write_fill);
+		tofill = min_t(size_t, count, mtdraw->rps - mtdraw->write_fill);
 		mtdraw_fillbuf(mtdraw, buf, tofill);
 		offset += tofill;
 		count -= tofill;
 		retlen += tofill;
 	}
 
-	if (mtdraw->write_fill == bsz) {
+	if (mtdraw->write_fill == mtdraw->rps) {
 		numblock = mtdraw_offset_to_block(mtdraw, mtdraw->write_ofs);
-		ret = mtdraw_blkwrite(mtd, mtdraw->writebuf,
+		ret = mtdraw_blkwrite(mtdraw, mtdraw->writebuf,
 				      mtd->writesize * numblock);
 		mtdraw->write_fill = 0;
 	}
 
 	numblock = mtdraw_offset_to_block(mtdraw, offset);
-	while (ret >= 0 && count >= bsz) {
-		ret = mtdraw_blkwrite(mtd, buf + retlen,
+	while (ret >= 0 && count >= mtdraw->rps) {
+		ret = mtdraw_blkwrite(mtdraw, buf + retlen,
 				   mtd->writesize * numblock++);
 		count -= ret;
 		retlen += ret;
@@ -305,12 +305,12 @@ static int add_mtdraw_device(struct mtd_info *mtd, const char *devname, void **p
 		return 0;
 
 	mtdraw = xzalloc(sizeof(*mtdraw));
+	mtdraw->rps = mtd->writesize + mtd->oobsize;
 	mtdraw->writebuf = xmalloc(RAW_WRITEBUF_SIZE);
 	mtdraw->mtd = mtd;
 
 	mtdraw->cdev.ops = (struct cdev_operations *)&mtd_raw_fops;
-	mtdraw->cdev.size = mtd_div_by_wb(mtd->size, mtd) *
-		(mtd->writesize + mtd->oobsize);
+	mtdraw->cdev.size = mtd_div_by_wb(mtd->size, mtd) * mtdraw->rps;
 	mtdraw->cdev.name = basprintf("%s.raw", mtd->cdev.name);
 	mtdraw->cdev.priv = mtdraw;
 	mtdraw->cdev.dev = &mtd->class_dev;
-- 
2.19.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 5/8] mtd: mtdraw: add raw page size to private data

[Sam Ravnborg]

Hi Sascha.

On Fri, Sep 21, 2018 at 01:18:17PM +0200, Sascha Hauer wrote:
> The raw page size is used many times in the driver, so add a variable
> to the private data instead of calculating it each time again.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/mtd/mtdraw.c | 40 ++++++++++++++++++++--------------------
>  1 file changed, 20 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/mtd/mtdraw.c b/drivers/mtd/mtdraw.c
> index 089dc54eb4..6518ae34d5 100644
> --- a/drivers/mtd/mtdraw.c
> +++ b/drivers/mtd/mtdraw.c
> @@ -48,6 +48,7 @@
>   * @write_fill: number of bytes in writebuf
>   * @write_ofs: offset in character device (mtdraw) where last write(s) stored
>   * bytes because of unaligned writes (ie. remain of writesize+oobsize write)
> + * @rps: raw page size (data + oob)
From the code I would say that a (writesize + oobsize) was more correct.
Like we use in the line above too.

> @@ -131,12 +132,11 @@ static ssize_t mtdraw_read(struct cdev *cdev, void *buf, size_t count,
>  	int skip;
>  
>  	numblock = mtdraw_offset_to_block(mtdraw, offset);
> -	skip = offset - numblock * (mtd->writesize + mtd->oobsize);
> +	skip = offset - numblock * (mtdraw->rps);

Here the () can be dropped.


	Sam

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 5/8] mtd: mtdraw: add raw page size to private data

[Sascha Hauer]

On Fri, Sep 21, 2018 at 04:42:40PM +0200, Sam Ravnborg wrote:
> Hi Sascha.
> 
> On Fri, Sep 21, 2018 at 01:18:17PM +0200, Sascha Hauer wrote:
> > The raw page size is used many times in the driver, so add a variable
> > to the private data instead of calculating it each time again.
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  drivers/mtd/mtdraw.c | 40 ++++++++++++++++++++--------------------
> >  1 file changed, 20 insertions(+), 20 deletions(-)
> > 
> > diff --git a/drivers/mtd/mtdraw.c b/drivers/mtd/mtdraw.c
> > index 089dc54eb4..6518ae34d5 100644
> > --- a/drivers/mtd/mtdraw.c
> > +++ b/drivers/mtd/mtdraw.c
> > @@ -48,6 +48,7 @@
> >   * @write_fill: number of bytes in writebuf
> >   * @write_ofs: offset in character device (mtdraw) where last write(s) stored
> >   * bytes because of unaligned writes (ie. remain of writesize+oobsize write)
> > + * @rps: raw page size (data + oob)
> From the code I would say that a (writesize + oobsize) was more correct.
> Like we use in the line above too.
> 
> > @@ -131,12 +132,11 @@ static ssize_t mtdraw_read(struct cdev *cdev, void *buf, size_t count,
> >  	int skip;
> >  
> >  	numblock = mtdraw_offset_to_block(mtdraw, offset);
> > -	skip = offset - numblock * (mtd->writesize + mtd->oobsize);
> > +	skip = offset - numblock * (mtdraw->rps);
> 
> Here the () can be dropped.

Ok, fixed. Thanks

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 6/8] mtd: mtdraw: fail when writing fails

[Sascha Hauer]

When writing a block fails then fail the whole write process. There's
no point in returning the written bytes so far as this would only
indicate the caller to write the remaining bytes again which would
then fail.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mtd/mtdraw.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/mtd/mtdraw.c b/drivers/mtd/mtdraw.c
index 6518ae34d5..fb45c2bbc9 100644
--- a/drivers/mtd/mtdraw.c
+++ b/drivers/mtd/mtdraw.c
@@ -158,12 +158,11 @@ static loff_t mtdraw_raw_to_mtd_offset(struct mtdraw *mtdraw, loff_t offset)
 	return (loff_t)mtdraw_offset_to_block(mtdraw, offset) * mtd->writesize;
 }
 
-static ssize_t mtdraw_blkwrite(struct mtdraw *mtdraw, const void *buf,
+static int mtdraw_blkwrite(struct mtdraw *mtdraw, const void *buf,
 			       ulong offset)
 {
 	struct mtd_info *mtd = mtdraw->mtd;
 	struct mtd_oob_ops ops;
-	int ret;
 
 	if (mtd_buf_all_ff(buf, mtdraw->rps))
 		return 0;
@@ -174,10 +173,7 @@ static ssize_t mtdraw_blkwrite(struct mtdraw *mtdraw, const void *buf,
 	ops.len = mtd->writesize;
 	ops.oobbuf = (void *)buf + mtd->writesize;
 	ops.ooblen = mtd->oobsize;
-	ret = mtd_write_oob(mtd, offset, &ops);
-	if (!ret)
-		ret = ops.retlen + ops.oobretlen;
-	return ret;
+	return mtd_write_oob(mtd, offset, &ops);
 }
 
 static void mtdraw_fillbuf(struct mtdraw *mtdraw, const void *src, int nbbytes)
@@ -215,6 +211,8 @@ static ssize_t mtdraw_write(struct cdev *cdev, const void *buf, size_t count,
 		numblock = mtdraw_offset_to_block(mtdraw, mtdraw->write_ofs);
 		ret = mtdraw_blkwrite(mtdraw, mtdraw->writebuf,
 				      mtd->writesize * numblock);
+		if (ret)
+			return ret;
 		mtdraw->write_fill = 0;
 	}
 
@@ -222,20 +220,19 @@ static ssize_t mtdraw_write(struct cdev *cdev, const void *buf, size_t count,
 	while (ret >= 0 && count >= mtdraw->rps) {
 		ret = mtdraw_blkwrite(mtdraw, buf + retlen,
 				   mtd->writesize * numblock++);
+		if (ret)
+			return ret;
 		count -= ret;
 		retlen += ret;
 		offset += ret;
 	}
 
-	if (ret >= 0 && count) {
+	if (count) {
 		mtdraw->write_ofs = offset - mtdraw->write_fill;
 		mtdraw_fillbuf(mtdraw, buf + retlen, count);
 		retlen += count;
 	}
 
-	if (ret < 0)
-		return ret;
-
 	return retlen;
 }
 
-- 
2.19.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 7/8] mtd: mtdraw: replace "block" with "page"

[Sascha Hauer]

When the code says "block" it actually means "page" as this is what
the code handles. Rename the functions and variable names accordingly.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mtd/mtdraw.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/mtd/mtdraw.c b/drivers/mtd/mtdraw.c
index fb45c2bbc9..18a94cc07f 100644
--- a/drivers/mtd/mtdraw.c
+++ b/drivers/mtd/mtdraw.c
@@ -79,7 +79,7 @@ static struct mtdraw *to_mtdraw(struct cdev *cdev)
 	return cdev->priv;
 }
 
-static unsigned int mtdraw_offset_to_block(struct mtdraw *mtdraw, loff_t offset)
+static unsigned int mtdraw_offset_to_page(struct mtdraw *mtdraw, loff_t offset)
 {
 	u64 ofs64 = offset;
 
@@ -128,16 +128,16 @@ static ssize_t mtdraw_read(struct cdev *cdev, void *buf, size_t count,
 	struct mtdraw *mtdraw = to_mtdraw(cdev);
 	struct mtd_info *mtd = mtdraw->mtd;
 	ssize_t retlen = 0, ret = 1, toread;
-	ulong numblock;
+	ulong numpage;
 	int skip;
 
-	numblock = mtdraw_offset_to_block(mtdraw, offset);
-	skip = offset - numblock * (mtdraw->rps);
+	numpage = mtdraw_offset_to_page(mtdraw, offset);
+	skip = offset - numpage * mtdraw->rps;
 
 	while (ret > 0 && count > 0) {
 		toread = min_t(int, count, mtdraw->rps - skip);
 		ret = mtdraw_read_unaligned(mtdraw, buf, toread,
-					    skip, numblock++ * mtd->writesize);
+					    skip, numpage++ * mtd->writesize);
 		buf += ret;
 		skip = 0;
 		count -= ret;
@@ -155,10 +155,10 @@ static loff_t mtdraw_raw_to_mtd_offset(struct mtdraw *mtdraw, loff_t offset)
 {
 	struct mtd_info *mtd = mtdraw->mtd;
 
-	return (loff_t)mtdraw_offset_to_block(mtdraw, offset) * mtd->writesize;
+	return (loff_t)mtdraw_offset_to_page(mtdraw, offset) * mtd->writesize;
 }
 
-static int mtdraw_blkwrite(struct mtdraw *mtdraw, const void *buf,
+static int mtdraw_pagewrite(struct mtdraw *mtdraw, const void *buf,
 			       ulong offset)
 {
 	struct mtd_info *mtd = mtdraw->mtd;
@@ -187,16 +187,16 @@ static ssize_t mtdraw_write(struct cdev *cdev, const void *buf, size_t count,
 {
 	struct mtdraw *mtdraw = to_mtdraw(cdev);
 	struct mtd_info *mtd = mtdraw->mtd;
-	ulong numblock;
+	ulong numpage;
 	size_t retlen = 0, tofill;
 	int ret = 0;
 
-	numblock = mtdraw_offset_to_block(mtdraw, offset);
+	numpage = mtdraw_offset_to_page(mtdraw, offset);
 
 	if (mtdraw->write_fill &&
 	    mtdraw->write_ofs + mtdraw->write_fill != offset)
 		return -EINVAL;
-	if (mtdraw->write_fill == 0 && offset - numblock * mtd->writesize != 0)
+	if (mtdraw->write_fill == 0 && offset - numpage * mtd->writesize != 0)
 		return -EINVAL;
 
 	if (mtdraw->write_fill) {
@@ -208,23 +208,23 @@ static ssize_t mtdraw_write(struct cdev *cdev, const void *buf, size_t count,
 	}
 
 	if (mtdraw->write_fill == mtdraw->rps) {
-		numblock = mtdraw_offset_to_block(mtdraw, mtdraw->write_ofs);
-		ret = mtdraw_blkwrite(mtdraw, mtdraw->writebuf,
-				      mtd->writesize * numblock);
+		numpage = mtdraw_offset_to_page(mtdraw, mtdraw->write_ofs);
+		ret = mtdraw_pagewrite(mtdraw, mtdraw->writebuf,
+				      mtd->writesize * numpage);
 		if (ret)
 			return ret;
 		mtdraw->write_fill = 0;
 	}
 
-	numblock = mtdraw_offset_to_block(mtdraw, offset);
+	numpage = mtdraw_offset_to_page(mtdraw, offset);
 	while (ret >= 0 && count >= mtdraw->rps) {
-		ret = mtdraw_blkwrite(mtdraw, buf + retlen,
-				   mtd->writesize * numblock++);
+		ret = mtdraw_pagewrite(mtdraw, buf + retlen,
+				   mtd->writesize * numpage++);
 		if (ret)
 			return ret;
-		count -= ret;
-		retlen += ret;
-		offset += ret;
+		count -= mtdraw->rps;
+		retlen += mtdraw->rps;
+		offset += mtdraw->rps;
 	}
 
 	if (count) {
-- 
2.19.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 8/8] mtd: mtdraw: fix wrong alignment check

[Sascha Hauer]

The write offset must be raw page size aligned on entry, not page size
aligned.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mtd/mtdraw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/mtdraw.c b/drivers/mtd/mtdraw.c
index 18a94cc07f..d94629dc41 100644
--- a/drivers/mtd/mtdraw.c
+++ b/drivers/mtd/mtdraw.c
@@ -196,7 +196,7 @@ static ssize_t mtdraw_write(struct cdev *cdev, const void *buf, size_t count,
 	if (mtdraw->write_fill &&
 	    mtdraw->write_ofs + mtdraw->write_fill != offset)
 		return -EINVAL;
-	if (mtdraw->write_fill == 0 && offset - numpage * mtd->writesize != 0)
+	if (mtdraw->write_fill == 0 && offset - numpage * mtdraw->rps != 0)
 		return -EINVAL;
 
 	if (mtdraw->write_fill) {
-- 
2.19.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox