[PATCH 1/2] mtd oob: do not register oob device for devices without oob

[PATCH 1/2] mtd oob: do not register oob device for devices without oob

[Sascha Hauer]

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

diff --git a/drivers/mtd/mtdoob.c b/drivers/mtd/mtdoob.c
index c7bf40c..e5d8039 100644
--- a/drivers/mtd/mtdoob.c
+++ b/drivers/mtd/mtdoob.c
@@ -73,6 +73,9 @@ static int add_mtdoob_device(struct mtd_info *mtd, char *devname, void **priv)
 {
 	struct mtdoob *mtdoob;
 
+	if (mtd->oobsize == 0)
+		return 0;
+
 	mtdoob = xzalloc(sizeof(*mtdoob));
 	mtdoob->cdev.ops = &mtd_ops_oob;
 	mtdoob->cdev.size = (mtd->size / mtd->writesize) * mtd->oobsize;
-- 
1.7.10.4


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

[PATCH 2/2] mtd core: call driver write function with complete buffer

[Sascha Hauer]

mtd->write is supposed to loop around pages internally, no need
to do this in mtd_write. This fixes a huge write performance drop
with the m25p80 driver when it was converted to a mtd driver recently.
Since mtd->writesize is 1 for this driver mtd_write ended up doing
single byte writes on the flash.

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

diff --git a/drivers/mtd/core.c b/drivers/mtd/core.c
index 8601787..b5916da 100644
--- a/drivers/mtd/core.c
+++ b/drivers/mtd/core.c
@@ -62,65 +62,15 @@ static 	ssize_t mtd_read(struct cdev *cdev, void* buf, size_t count,
 #define MTDPGALG(x) ((x) & ~(mtd->writesize - 1))
 
 #ifdef CONFIG_MTD_WRITE
-static int all_ff(const void *buf, int len)
-{
-	int i;
-	const uint8_t *p = buf;
-
-	for (i = 0; i < len; i++)
-		if (p[i] != 0xFF)
-			return 0;
-	return 1;
-}
-
 static ssize_t mtd_write(struct cdev* cdev, const void *buf, size_t _count,
 			  loff_t _offset, ulong flags)
 {
 	struct mtd_info *mtd = cdev->priv;
-	size_t retlen, now;
-	int ret = 0;
-	void *wrbuf = NULL;
-	size_t count = _count;
-	unsigned long offset = _offset;
-
-	if (NOTALIGNED(offset)) {
-		printf("offset 0x%0lx not page aligned\n", offset);
-		return -EINVAL;
-	}
-
-	dev_dbg(cdev->dev, "write: offset: 0x%08lx count: 0x%zx\n", offset, count);
-	while (count) {
-		now = count > mtd->writesize ? mtd->writesize : count;
-
-		if (NOTALIGNED(now)) {
-			dev_dbg(cdev->dev, "not aligned: %d %ld\n",
-				mtd->writesize,
-				(offset % mtd->writesize));
-			wrbuf = xmalloc(mtd->writesize);
-			memset(wrbuf, 0xff, mtd->writesize);
-			memcpy(wrbuf + (offset % mtd->writesize), buf, now);
-			if (!all_ff(wrbuf, mtd->writesize))
-				ret = mtd->write(mtd, MTDPGALG(offset),
-						  mtd->writesize, &retlen,
-						  wrbuf);
-			free(wrbuf);
-		} else {
-			if (!all_ff(buf, mtd->writesize))
-				ret = mtd->write(mtd, offset, now, &retlen,
-						  buf);
-			dev_dbg(cdev->dev,
-				"offset: 0x%08lx now: 0x%zx retlen: 0x%zx\n",
-				offset, now, retlen);
-		}
-		if (ret)
-			goto out;
+	size_t retlen;
+	int ret;
 
-		offset += now;
-		count -= now;
-		buf += now;
-	}
+	ret = mtd->write(mtd, _offset, _count, &retlen, buf);
 
-out:
 	return ret ? ret : _count;
 }
 #endif
-- 
1.7.10.4


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

Re: [PATCH 2/2] mtd core: call driver write function with complete buffer

[Robert Jarzmik]

Sascha Hauer <s.hauer@pengutronix.de> writes:

> mtd->write is supposed to loop around pages internally, no need
> to do this in mtd_write. This fixes a huge write performance drop
> with the m25p80 driver when it was converted to a mtd driver recently.
> Since mtd->writesize is 1 for this driver mtd_write ended up doing
> single byte writes on the flash.

Both patches look good to me.
Of course they change a bit the behaviour, as previously mtd_write() was
skipping "all 0xff" pages writes, and it doesn't do it anymore. But I don't see
any negative impact out of my head here.

As they are already in -next, no need for an Acked-by :)

Cheers.

--
Robert

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

Re: [PATCH 2/2] mtd core: call driver write function with complete buffer

[Jean-Christophe PLAGNIOL-VILLARD]

On 20:03 Thu 29 Nov     , Sascha Hauer wrote:
> mtd->write is supposed to loop around pages internally, no need
> to do this in mtd_write. This fixes a huge write performance drop
> with the m25p80 driver when it was converted to a mtd driver recently.
> Since mtd->writesize is 1 for this driver mtd_write ended up doing
> single byte writes on the flash.
this is the right patch for the flash

but this break my nand on 9x5

## Total Size      = 0x0002948e = 169102 Bytes

erasing partition /dev/nand0.barebox.bb


flashing barebox.bin to /dev/nand0.barebox.bb

	[#################################################################]
nand_write: Attempt to write not page aligned data
CRC32 for barebox.bin 0x00000000 ... 0x0002948d ==> 0x4cfca554
CRC32 for /dev/nand0.barebox.bb 0x00000000 ... 0x0002948d ==> 0x61dca32f != 0x61dca32f ** ERROR **

Best Regards,
J.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/mtd/core.c |   56 +++-------------------------------------------------
>  1 file changed, 3 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/mtd/core.c b/drivers/mtd/core.c
> index 8601787..b5916da 100644
> --- a/drivers/mtd/core.c
> +++ b/drivers/mtd/core.c
> @@ -62,65 +62,15 @@ static 	ssize_t mtd_read(struct cdev *cdev, void* buf, size_t count,
>  #define MTDPGALG(x) ((x) & ~(mtd->writesize - 1))
>  
>  #ifdef CONFIG_MTD_WRITE
> -static int all_ff(const void *buf, int len)
> -{
> -	int i;
> -	const uint8_t *p = buf;
> -
> -	for (i = 0; i < len; i++)
> -		if (p[i] != 0xFF)
> -			return 0;
> -	return 1;
> -}
> -
>  static ssize_t mtd_write(struct cdev* cdev, const void *buf, size_t _count,
>  			  loff_t _offset, ulong flags)
>  {
>  	struct mtd_info *mtd = cdev->priv;
> -	size_t retlen, now;
> -	int ret = 0;
> -	void *wrbuf = NULL;
> -	size_t count = _count;
> -	unsigned long offset = _offset;
> -
> -	if (NOTALIGNED(offset)) {
> -		printf("offset 0x%0lx not page aligned\n", offset);
> -		return -EINVAL;
> -	}
> -
> -	dev_dbg(cdev->dev, "write: offset: 0x%08lx count: 0x%zx\n", offset, count);
> -	while (count) {
> -		now = count > mtd->writesize ? mtd->writesize : count;
> -
> -		if (NOTALIGNED(now)) {
> -			dev_dbg(cdev->dev, "not aligned: %d %ld\n",
> -				mtd->writesize,
> -				(offset % mtd->writesize));
> -			wrbuf = xmalloc(mtd->writesize);
> -			memset(wrbuf, 0xff, mtd->writesize);
> -			memcpy(wrbuf + (offset % mtd->writesize), buf, now);
> -			if (!all_ff(wrbuf, mtd->writesize))
> -				ret = mtd->write(mtd, MTDPGALG(offset),
> -						  mtd->writesize, &retlen,
> -						  wrbuf);
> -			free(wrbuf);
> -		} else {
> -			if (!all_ff(buf, mtd->writesize))
> -				ret = mtd->write(mtd, offset, now, &retlen,
> -						  buf);
> -			dev_dbg(cdev->dev,
> -				"offset: 0x%08lx now: 0x%zx retlen: 0x%zx\n",
> -				offset, now, retlen);
> -		}
> -		if (ret)
> -			goto out;
> +	size_t retlen;
> +	int ret;
>  
> -		offset += now;
> -		count -= now;
> -		buf += now;
> -	}
> +	ret = mtd->write(mtd, _offset, _count, &retlen, buf);
>  
> -out:
>  	return ret ? ret : _count;
>  }
>  #endif
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

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

[PATCH 1/1] mtd: nand: fix unaligned write support

[Jean-Christophe PLAGNIOL-VILLARD]

via cdev we may write unaligned data the code was drop in the commit
0a4b1c7e440a81819eb2137f923573a3055dc7a2
mtd core: call driver write function with complete buffer

which is true for spi flashes but the code is still mandatory on nand

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 drivers/mtd/core.c            |    3 --
 drivers/mtd/nand/nand_write.c |   66 ++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 65 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/core.c b/drivers/mtd/core.c
index b5916da..8908f22 100644
--- a/drivers/mtd/core.c
+++ b/drivers/mtd/core.c
@@ -58,9 +58,6 @@ static 	ssize_t mtd_read(struct cdev *cdev, void* buf, size_t count,
 	return retlen;
 }
 
-#define NOTALIGNED(x) (x & (mtd->writesize - 1)) != 0
-#define MTDPGALG(x) ((x) & ~(mtd->writesize - 1))
-
 #ifdef CONFIG_MTD_WRITE
 static ssize_t mtd_write(struct cdev* cdev, const void *buf, size_t _count,
 			  loff_t _offset, ulong flags)
diff --git a/drivers/mtd/nand/nand_write.c b/drivers/mtd/nand/nand_write.c
index 5ed04ce..e49db43 100644
--- a/drivers/mtd/nand/nand_write.c
+++ b/drivers/mtd/nand/nand_write.c
@@ -363,7 +363,7 @@ int nand_do_write_ops(struct mtd_info *mtd, loff_t to,
  *
  * NAND write with ECC
  */
-int nand_write(struct mtd_info *mtd, loff_t to, size_t len,
+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;
@@ -386,6 +386,70 @@ int nand_write(struct mtd_info *mtd, loff_t to, size_t len,
 	return ret;
 }
 
+static int all_ff(const void *buf, int len)
+{
+	int i;
+	const uint8_t *p = buf;
+
+	for (i = 0; i < len; i++)
+		if (p[i] != 0xFF)
+			return 0;
+	return 1;
+}
+
+#define MTD_NOTALIGNED(x) (x & (mtd->writesize - 1)) != 0
+#define MTDPGALG(x) ((x) & ~(mtd->writesize - 1))
+
+int nand_write(struct mtd_info *mtd, loff_t _offset, size_t _count,
+			  size_t *retlen, const uint8_t *buf)
+{
+	size_t now;
+	int ret = 0;
+	void *wrbuf = NULL;
+	size_t count = _count;
+	unsigned long offset = _offset;
+
+	if (MTD_NOTALIGNED(offset)) {
+		printf("offset 0x%0lx not page aligned\n", offset);
+		return -EINVAL;
+	}
+
+	dev_dbg(mtd->parent, "write: offset: 0x%08lx count: 0x%zx\n", offset, count);
+	while (count) {
+		now = count > mtd->writesize ? mtd->writesize : count;
+
+		if (MTD_NOTALIGNED(now)) {
+			dev_dbg(mtd->parent, "not aligned: %d %ld\n",
+				mtd->writesize,
+				(offset % mtd->writesize));
+			wrbuf = xmalloc(mtd->writesize);
+			memset(wrbuf, 0xff, mtd->writesize);
+			memcpy(wrbuf + (offset % mtd->writesize), buf, now);
+			if (!all_ff(wrbuf, mtd->writesize))
+				ret = __nand_write(mtd, MTDPGALG(offset),
+						  mtd->writesize, retlen,
+						  wrbuf);
+			free(wrbuf);
+		} else {
+			if (!all_ff(buf, mtd->writesize))
+				ret = __nand_write(mtd, offset, now, retlen,
+						  buf);
+			dev_dbg(mtd->parent,
+				"offset: 0x%08lx now: 0x%zx retlen: 0x%zx\n",
+				offset, now, *retlen);
+		}
+		if (ret)
+			goto out;
+
+		offset += now;
+		count -= now;
+		buf += now;
+	}
+
+out:
+	return ret ? ret : _count;
+}
+
 /**
  * nand_do_write_oob - [MTD Interface] NAND write out-of-band
  * @mtd:	MTD device structure
-- 
1.7.10.4


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

Re: [PATCH 1/1] mtd: nand: fix unaligned write support

[Sascha Hauer]

On Wed, Dec 05, 2012 at 06:15:20PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> via cdev we may write unaligned data the code was drop in the commit
> 0a4b1c7e440a81819eb2137f923573a3055dc7a2
> mtd core: call driver write function with complete buffer
> 
> which is true for spi flashes but the code is still mandatory on nand

I suggest the following simpler approach.

Sascha


From 5248abe7e59f60085b9a2affd3db11a6ac0a9da9 Mon Sep 17 00:00:00 2001
From: Sascha Hauer <s.hauer@pengutronix.de>
Date: Wed, 5 Dec 2012 19:24:41 +0100
Subject: [PATCH] mtd nand: allow partial page writes

The nand layer handles partial page writes just fine. If the start or
end of the data is not page aligned, the nand layer will copy the data
to a temporary page buffer.
Remove the check which disallows partial page writes since this is what
we want to do on barebox when for example an image is written to nand
which is not padded to page size.

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

diff --git a/drivers/mtd/nand/nand_write.c b/drivers/mtd/nand/nand_write.c
index 5ed04ce..9997127 100644
--- a/drivers/mtd/nand/nand_write.c
+++ b/drivers/mtd/nand/nand_write.c
@@ -273,13 +273,6 @@ int nand_do_write_ops(struct mtd_info *mtd, loff_t to,
 	if (!writelen)
 		return 0;
 
-	/* reject writes, which are not page aligned */
-	if (NOTALIGNED(to) || NOTALIGNED(ops->len)) {
-		printk(KERN_NOTICE "nand_write: "
-		       "Attempt to write not page aligned data\n");
-		return -EINVAL;
-	}
-
 	column = to & (mtd->writesize - 1);
 	subpage = column || (writelen & (mtd->writesize - 1));
 
-- 
1.7.10.4

-- 
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

Re: [PATCH 1/1] mtd: nand: fix unaligned write support

[Jean-Christophe PLAGNIOL-VILLARD]

On 19:28 Wed 05 Dec     , Sascha Hauer wrote:
> On Wed, Dec 05, 2012 at 06:15:20PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > via cdev we may write unaligned data the code was drop in the commit
> > 0a4b1c7e440a81819eb2137f923573a3055dc7a2
> > mtd core: call driver write function with complete buffer
> > 
> > which is true for spi flashes but the code is still mandatory on nand
> 
> I suggest the following simpler approach.
> 
> Sascha
> 
yes it work here

Best Regards,
J.
> 
> From 5248abe7e59f60085b9a2affd3db11a6ac0a9da9 Mon Sep 17 00:00:00 2001
> From: Sascha Hauer <s.hauer@pengutronix.de>
> Date: Wed, 5 Dec 2012 19:24:41 +0100
> Subject: [PATCH] mtd nand: allow partial page writes
> 
> The nand layer handles partial page writes just fine. If the start or
> end of the data is not page aligned, the nand layer will copy the data
> to a temporary page buffer.
> Remove the check which disallows partial page writes since this is what
> we want to do on barebox when for example an image is written to nand
> which is not padded to page size.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/mtd/nand/nand_write.c |    7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/drivers/mtd/nand/nand_write.c b/drivers/mtd/nand/nand_write.c
> index 5ed04ce..9997127 100644
> --- a/drivers/mtd/nand/nand_write.c
> +++ b/drivers/mtd/nand/nand_write.c
> @@ -273,13 +273,6 @@ int nand_do_write_ops(struct mtd_info *mtd, loff_t to,
>  	if (!writelen)
>  		return 0;
>  
> -	/* reject writes, which are not page aligned */
> -	if (NOTALIGNED(to) || NOTALIGNED(ops->len)) {
> -		printk(KERN_NOTICE "nand_write: "
> -		       "Attempt to write not page aligned data\n");
> -		return -EINVAL;
> -	}
> -
>  	column = to & (mtd->writesize - 1);
>  	subpage = column || (writelen & (mtd->writesize - 1));
>  
> -- 
> 1.7.10.4
> 
> -- 
> 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