[PATCH v2 0/6] Fix UBIFS recovery issues observed on NOR flash

[PATCH v2 0/6] Fix UBIFS recovery issues observed on NOR flash

[Anatolij Gustschin]

This v2 patch series is the next attempt to fix UBIFS recovery
issues on NOR flash partitions. Previously only one patch
has been submitted. Now, as requested by Artem, the patch
is split into several patches and additionally the setting
of writebufsize for NAND/OneNAND flash is also done. Comments
from Artem to the previous patch have been addressed, too.

Anatolij Gustschin (6):
  mtd: add writebufsize field to mtd_info struct
  mtd: cfi: add writebufsize initialization
  mtd: nand: add mtd->writebufsize initialization
  mtd: onenand: add mtd->writebufsize initialization
  mtd: initialize writebufsize in the MTD object of a partition
  UBI: use mtd->writebufsize to set minimal I/O unit size

 drivers/mtd/chips/cfi_cmdset_0001.c |    1 +
 drivers/mtd/chips/cfi_cmdset_0002.c |    4 ++++
 drivers/mtd/chips/cfi_cmdset_0020.c |    1 +
 drivers/mtd/mtdconcat.c             |    1 +
 drivers/mtd/mtdpart.c               |    1 +
 drivers/mtd/nand/nand_base.c        |    1 +
 drivers/mtd/onenand/onenand_base.c  |    1 +
 drivers/mtd/ubi/build.c             |   19 ++++++++++++++++++-
 include/linux/mtd/mtd.h             |   13 +++++++++++++
 9 files changed, 41 insertions(+), 1 deletions(-)

[PATCH v2 1/6] mtd: add writebufsize field to mtd_info struct

[Anatolij Gustschin]

This field will be used to indicate the write buffer size
of the MTD device. UBI will set it's minimal I/O unit size
(min_io_size) to the indicated write buffer size. By this
change we intend to fix failed recovery of UBIFS partitions
we currently observe on NOR flash when mounting the partition
after unclean unmount.

Currently the min_io_size is set to mtd->writesize (which is 1
byte for NOR flash). But flash programming is often done from
prepared write buffer containing multiple bytes and is performed
in one programming operation which could be interrupted by a power
cut or a system reset causing corrupted (partially written) areas
in a flash sector. Knowing the size of potentially corrupted areas
UBIFS scanning and recovery algorithms are able to perform
successful recovery.

In case of NOR flash minimal I/O size must be equal to the
maximal size of the write buffer used by embedded flash
programming algorithm. In case of NAND flash mtd->writebufsize
should be equivalent to mtd->writesize.

The subsequent patches will add mtd->writebufsize initialization
where needed.

Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
 include/linux/mtd/mtd.h |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index fe8d77e..3c6db0c 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -144,6 +144,19 @@ struct mtd_info {
 	 */
 	uint32_t writesize;
 
+	/*
+	 * Size of the write buffer used by the MTD.
+	 * MTD devices having a write buffer can write multiple writesize
+	 * chunks at a time. E.g. while writing 4 * writesize bytes to a
+	 * device with 2 * writesize bytes buffer the MTD driver can (but
+	 * doesn't have to) do 2 writesize operations, but not 4.
+	 * In case of NAND flash writebufsize should be equivalent to
+	 * writesize. In case of NOR flash writebufsize is the size of the
+	 * write buffer used by the MTD device to program multiple bytes
+	 * at a time (in one programming operation).
+	 */
+	uint32_t writebufsize;
+
 	uint32_t oobsize;   // Amount of OOB data per block (e.g. 16)
 	uint32_t oobavail;  // Available OOB bytes per block
 
-- 
1.7.1

Re: [PATCH v2 1/6] mtd: add writebufsize field to mtd_info struct

[Artem Bityutskiy]

On Thu, 2010-12-16 at 23:42 +0100, Anatolij Gustschin wrote:
> This field will be used to indicate the write buffer size
> of the MTD device. UBI will set it's minimal I/O unit size
> (min_io_size) to the indicated write buffer size. By this
> change we intend to fix failed recovery of UBIFS partitions
> we currently observe on NOR flash when mounting the partition
> after unclean unmount.
> 
> Currently the min_io_size is set to mtd->writesize (which is 1
> byte for NOR flash). But flash programming is often done from
> prepared write buffer containing multiple bytes and is performed
> in one programming operation which could be interrupted by a power
> cut or a system reset causing corrupted (partially written) areas
> in a flash sector. Knowing the size of potentially corrupted areas
> UBIFS scanning and recovery algorithms are able to perform
> successful recovery.
> 
> In case of NOR flash minimal I/O size must be equal to the
> maximal size of the write buffer used by embedded flash
> programming algorithm. In case of NAND flash mtd->writebufsize
> should be equivalent to mtd->writesize.
> 
> The subsequent patches will add mtd->writebufsize initialization
> where needed.
> 
> Signed-off-by: Anatolij Gustschin <agust@denx.de>

I've tweaked your commentary a bit and pushed to l2-mtd-2.6.git, please,
check.

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

Re: [PATCH v2 1/6] mtd: add writebufsize field to mtd_info struct

[Anatolij Gustschin]

On Sun, 19 Dec 2010 19:45:31 +0200
Artem Bityutskiy <dedekind1@gmail.com> wrote:

> I've tweaked your commentary a bit and pushed to l2-mtd-2.6.git, please,
> check.

It is ok. Thanks!

Anatolij

[PATCH v2 2/6] mtd: cfi: add writebufsize initialization

[Anatolij Gustschin]

Initialize mtd->writebufsize to the value obtained
by CFI query command at probe time.

Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
 drivers/mtd/chips/cfi_cmdset_0001.c |    1 +
 drivers/mtd/chips/cfi_cmdset_0002.c |    4 ++++
 drivers/mtd/chips/cfi_cmdset_0020.c |    1 +
 3 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0001.c b/drivers/mtd/chips/cfi_cmdset_0001.c
index 44cbfc0..a8c3e1c 100644
--- a/drivers/mtd/chips/cfi_cmdset_0001.c
+++ b/drivers/mtd/chips/cfi_cmdset_0001.c
@@ -455,6 +455,7 @@ struct mtd_info *cfi_cmdset_0001(struct map_info *map, int primary)
 	mtd->flags   = MTD_CAP_NORFLASH;
 	mtd->name    = map->name;
 	mtd->writesize = 1;
+	mtd->writebufsize = 1 << cfi->cfiq->MaxBufWriteSize;
 
 	mtd->reboot_notifier.notifier_call = cfi_intelext_reboot;
 
diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index 9d68ab9..54dbd28 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -428,6 +428,10 @@ struct mtd_info *cfi_cmdset_0002(struct map_info *map, int primary)
 	mtd->flags   = MTD_CAP_NORFLASH;
 	mtd->name    = map->name;
 	mtd->writesize = 1;
+	mtd->writebufsize = 1 << cfi->cfiq->MaxBufWriteSize;
+
+	DEBUG(MTD_DEBUG_LEVEL3, "MTD %s(): write buffer size %d\n",
+		__func__, mtd->writebufsize);
 
 	mtd->reboot_notifier.notifier_call = cfi_amdstd_reboot;
 
diff --git a/drivers/mtd/chips/cfi_cmdset_0020.c b/drivers/mtd/chips/cfi_cmdset_0020.c
index 314af1f..c04b765 100644
--- a/drivers/mtd/chips/cfi_cmdset_0020.c
+++ b/drivers/mtd/chips/cfi_cmdset_0020.c
@@ -238,6 +238,7 @@ static struct mtd_info *cfi_staa_setup(struct map_info *map)
 	mtd->resume = cfi_staa_resume;
 	mtd->flags = MTD_CAP_NORFLASH & ~MTD_BIT_WRITEABLE;
 	mtd->writesize = 8; /* FIXME: Should be 0 for STMicro flashes w/out ECC */
+	mtd->writebufsize = 1 << cfi->cfiq->MaxBufWriteSize;
 	map->fldrv = &cfi_staa_chipdrv;
 	__module_get(THIS_MODULE);
 	mtd->name = map->name;
-- 
1.7.1

Re: [PATCH v2 2/6] mtd: cfi: add writebufsize initialization

[Artem Bityutskiy]

On Thu, 2010-12-16 at 23:42 +0100, Anatolij Gustschin wrote:
> Initialize mtd->writebufsize to the value obtained
> by CFI query command at probe time.
> 
> Signed-off-by: Anatolij Gustschin <agust@denx.de>

Pushed patches 2-5 as is.

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

Re: [PATCH v2 2/6] mtd: cfi: add writebufsize initialization

[Guillaume LECERF]

Hello.

2010/12/16 Anatolij Gustschin <agust@denx.de>:
> Initialize mtd->writebufsize to the value obtained
> by CFI query command at probe time.
>
> Signed-off-by: Anatolij Gustschin <agust@denx.de>
> ---
> --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
> @@ -428,6 +428,10 @@ struct mtd_info *cfi_cmdset_0002(struct map_info *map, int primary)
>        mtd->flags   = MTD_CAP_NORFLASH;
>        mtd->name    = map->name;
>        mtd->writesize = 1;
> +       mtd->writebufsize = 1 << cfi->cfiq->MaxBufWriteSize;

What about wbufsize initialized in cfi_amdstd_write_buffers() to
"cfi_interleave(cfi) << cfi->cfiq->MaxBufWriteSize" ?
I don't know this piece of code, but I just wanted to point it to you.

-- 
Guillaume LECERF
GeeXboX developer - www.geexbox.org

Re: [PATCH v2 2/6] mtd: cfi: add writebufsize initialization

[Artem Bityutskiy]

On Mon, 2010-12-20 at 17:36 +0100, ext Guillaume LECERF wrote:
> Hello.
> 
> 2010/12/16 Anatolij Gustschin <agust@denx.de>:
> > Initialize mtd->writebufsize to the value obtained
> > by CFI query command at probe time.
> >
> > Signed-off-by: Anatolij Gustschin <agust@denx.de>
> > ---
> > --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> > +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
> > @@ -428,6 +428,10 @@ struct mtd_info *cfi_cmdset_0002(struct map_info *map, int primary)
> >        mtd->flags   = MTD_CAP_NORFLASH;
> >        mtd->name    = map->name;
> >        mtd->writesize = 1;
> > +       mtd->writebufsize = 1 << cfi->cfiq->MaxBufWriteSize;
> 
> What about wbufsize initialized in cfi_amdstd_write_buffers() to
> "cfi_interleave(cfi) << cfi->cfiq->MaxBufWriteSize" ?
> I don't know this piece of code, but I just wanted to point it to you.

Yes, fair question, Anatolij?

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

Re: [PATCH v2 2/6] mtd: cfi: add writebufsize initialization

[Guillaume LECERF]

[-- Attachment #1: Type: text/plain, Size: 222 bytes --]

2010/12/21 Artem Bityutskiy <dedekind1@gmail.com>:
> Yes, fair question, Anatolij?

Hi all.

Anatolij, what about such patches ?
Note : this is totally untested ;)

-- 
Guillaume LECERF
GeeXboX developer - www.geexbox.org

[-- Attachment #2: 04-v2-2-6-mtd-cfi-add-writebufsiz.patch --]
[-- Type: application/octet-stream, Size: 2210 bytes --]

[v2,2/6] mtd: cfi: add writebufsize initialization

From: Anatolij Gustschin <agust@denx.de>

Initialize mtd->writebufsize to the value obtained
by CFI query command at probe time.

Signed-off-by: Anatolij Gustschin <agust@denx.de>
Signed-off-by: Guillaume LECERF <glecerf@gmail.com>
---
 drivers/mtd/chips/cfi_cmdset_0001.c |    1 +
 drivers/mtd/chips/cfi_cmdset_0002.c |    4 ++++
 drivers/mtd/chips/cfi_cmdset_0020.c |    1 +
 3 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0001.c b/drivers/mtd/chips/cfi_cmdset_0001.c
index 44cbfc0..178f87b 100644
--- a/drivers/mtd/chips/cfi_cmdset_0001.c
+++ b/drivers/mtd/chips/cfi_cmdset_0001.c
@@ -455,6 +455,7 @@ struct mtd_info *cfi_cmdset_0001(struct map_info *map, int primary)
 	mtd->flags   = MTD_CAP_NORFLASH;
 	mtd->name    = map->name;
 	mtd->writesize = 1;
+	mtd->writebufsize = cfi_interleave(cfi) << cfi->cfiq->MaxBufWriteSize;
 
 	mtd->reboot_notifier.notifier_call = cfi_intelext_reboot;
 
diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index a43ab45..e94f483 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -440,6 +440,10 @@ struct mtd_info *cfi_cmdset_0002(struct map_info *map, int primary)
 	mtd->flags   = MTD_CAP_NORFLASH;
 	mtd->name    = map->name;
 	mtd->writesize = 1;
+	mtd->writebufsize = cfi_interleave(cfi) << cfi->cfiq->MaxBufWriteSize;
+
+	DEBUG(MTD_DEBUG_LEVEL3, "MTD %s(): write buffer size %d\n",
+	      __func__, mtd->writebufsize);
 
 	mtd->reboot_notifier.notifier_call = cfi_amdstd_reboot;
 
diff --git a/drivers/mtd/chips/cfi_cmdset_0020.c b/drivers/mtd/chips/cfi_cmdset_0020.c
index 314af1f..ed56ad3 100644
--- a/drivers/mtd/chips/cfi_cmdset_0020.c
+++ b/drivers/mtd/chips/cfi_cmdset_0020.c
@@ -238,6 +238,7 @@ static struct mtd_info *cfi_staa_setup(struct map_info *map)
 	mtd->resume = cfi_staa_resume;
 	mtd->flags = MTD_CAP_NORFLASH & ~MTD_BIT_WRITEABLE;
 	mtd->writesize = 8; /* FIXME: Should be 0 for STMicro flashes w/out ECC */
+	mtd->writebufsize = cfi_interleave(cfi) << cfi->cfiq->MaxBufWriteSize;
 	map->fldrv = &cfi_staa_chipdrv;
 	__module_get(THIS_MODULE);
 	mtd->name = map->name;

[-- Attachment #3: 08-patch.patch --]
[-- Type: application/octet-stream, Size: 5236 bytes --]



From: Guillaume LECERF <glecerf@gmail.com>


---
 drivers/mtd/chips/cfi_cmdset_0001.c |    8 +++-----
 drivers/mtd/chips/cfi_cmdset_0002.c |    3 +--
 drivers/mtd/chips/cfi_cmdset_0020.c |   11 +++++------
 drivers/mtd/chips/cfi_probe.c       |    3 ++-
 4 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0001.c b/drivers/mtd/chips/cfi_cmdset_0001.c
index 178f87b..6dbc44a 100644
--- a/drivers/mtd/chips/cfi_cmdset_0001.c
+++ b/drivers/mtd/chips/cfi_cmdset_0001.c
@@ -1650,16 +1650,15 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
 	struct cfi_private *cfi = map->fldrv_priv;
 	map_word status, write_cmd, datum;
 	unsigned long cmd_adr;
-	int ret, wbufsize, word_gap, words;
+	int ret, word_gap, words;
 	const struct kvec *vec;
 	unsigned long vec_seek;
 	unsigned long initial_adr;
 	int initial_len = len;
 
-	wbufsize = cfi_interleave(cfi) << cfi->cfiq->MaxBufWriteSize;
 	adr += chip->start;
 	initial_adr = adr;
-	cmd_adr = adr & ~(wbufsize-1);
+	cmd_adr = adr & ~(mtd->writebufsize - 1);
 
 	/* Let's determine this according to the interleave only once */
 	write_cmd = (cfi->cfiq->P_ID != P_ID_INTEL_PERFORMANCE) ? CMD(0xe8) : CMD(0xe9);
@@ -1807,7 +1806,6 @@ static int cfi_intelext_writev (struct mtd_info *mtd, const struct kvec *vecs,
 {
 	struct map_info *map = mtd->priv;
 	struct cfi_private *cfi = map->fldrv_priv;
-	int wbufsize = cfi_interleave(cfi) << cfi->cfiq->MaxBufWriteSize;
 	int ret = 0;
 	int chipnum;
 	unsigned long ofs, vec_seek, i;
@@ -1826,7 +1824,7 @@ static int cfi_intelext_writev (struct mtd_info *mtd, const struct kvec *vecs,
 
 	do {
 		/* We must not cross write block boundaries */
-		int size = wbufsize - (ofs & (wbufsize-1));
+		int size = mtd->writebufsize - (ofs & (mtd->writebufsize - 1));
 
 		if (size > len)
 			size = len;
diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index e94f483..c4f8675 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -1492,7 +1492,6 @@ static int cfi_amdstd_write_buffers(struct mtd_info *mtd, loff_t to, size_t len,
 {
 	struct map_info *map = mtd->priv;
 	struct cfi_private *cfi = map->fldrv_priv;
-	int wbufsize = cfi_interleave(cfi) << cfi->cfiq->MaxBufWriteSize;
 	int ret = 0;
 	int chipnum;
 	unsigned long ofs;
@@ -1528,7 +1527,7 @@ static int cfi_amdstd_write_buffers(struct mtd_info *mtd, loff_t to, size_t len,
 	/* Write buffer is worth it only if more than one word to write... */
 	while (len >= map_bankwidth(map) * 2) {
 		/* We must not cross write block boundaries */
-		int size = wbufsize - (ofs & (wbufsize-1));
+		int size = mtd->writebufsize - (ofs & (mtd->writebufsize - 1));
 
 		if (size > len)
 			size = len;
diff --git a/drivers/mtd/chips/cfi_cmdset_0020.c b/drivers/mtd/chips/cfi_cmdset_0020.c
index ed56ad3..bd8a550 100644
--- a/drivers/mtd/chips/cfi_cmdset_0020.c
+++ b/drivers/mtd/chips/cfi_cmdset_0020.c
@@ -426,15 +426,14 @@ static inline int do_write_buffer(struct map_info *map, struct flchip *chip,
 	map_word status, status_OK;
 	unsigned long cmd_adr, timeo;
 	DECLARE_WAITQUEUE(wait, current);
-	int wbufsize, z;
+	int z;
 
         /* M58LW064A requires bus alignment for buffer wriets -- saw */
         if (adr & (map_bankwidth(map)-1))
             return -EINVAL;
 
-        wbufsize = cfi_interleave(cfi) << cfi->cfiq->MaxBufWriteSize;
         adr += chip->start;
-	cmd_adr = adr & ~(wbufsize-1);
+	cmd_adr = adr & ~(mtd->writebufsize - 1);
 
 	/* Let's determine this according to the interleave only once */
         status_OK = CMD(0x80);
@@ -610,7 +609,6 @@ static int cfi_staa_write_buffers (struct mtd_info *mtd, loff_t to,
 {
 	struct map_info *map = mtd->priv;
 	struct cfi_private *cfi = map->fldrv_priv;
-	int wbufsize = cfi_interleave(cfi) << cfi->cfiq->MaxBufWriteSize;
 	int ret = 0;
 	int chipnum;
 	unsigned long ofs;
@@ -624,14 +622,15 @@ static int cfi_staa_write_buffers (struct mtd_info *mtd, loff_t to,
 
 #ifdef DEBUG_CFI_FEATURES
 	printk("%s: map_bankwidth(map)[%x]\n", __func__, map_bankwidth(map));
-	printk("%s: chipnum[%x] wbufsize[%x]\n", __func__, chipnum, wbufsize);
+	printk(KERN_DEBUG "%s: chipnum[%x] mtd->writebufsize[%x]\n", __func__,
+	       chipnum, mtd->writebufsize);
 	printk("%s: ofs[%x] len[%x]\n", __func__, ofs, len);
 #endif
 
         /* Write buffer is worth it only if more than one word to write... */
         while (len > 0) {
 		/* We must not cross write block boundaries */
-		int size = wbufsize - (ofs & (wbufsize-1));
+		int size = mtd->writebufsize - (ofs & (mtd->writebufsize - 1));
 
                 if (size > len)
                     size = len;
diff --git a/drivers/mtd/chips/cfi_probe.c b/drivers/mtd/chips/cfi_probe.c
index d255352..8f3135f 100644
--- a/drivers/mtd/chips/cfi_probe.c
+++ b/drivers/mtd/chips/cfi_probe.c
@@ -377,7 +377,8 @@ static void print_cfi_ident(struct cfi_ident *cfip)
 		break;
 	}
 
-	printk("Max. bytes in buffer write: 0x%x\n", 1<< cfip->MaxBufWriteSize);
+	printk(KERN_DEBUG "Max. bytes in buffer write: 0x%x\n",
+	       cfi_interleave(cfi) << cfip->MaxBufWriteSize);
 	printk("Number of Erase Block Regions: %d\n", cfip->NumEraseRegions);
 
 }

Re: [PATCH v2 2/6] mtd: cfi: add writebufsize initialization

[Guillaume LECERF]

2010/12/30 Guillaume LECERF <glecerf@gmail.com>:
> Anatolij, what about such patches ?
> Note : this is totally untested ;)

Please disregard 08-patch.patch, I think it does not even compile :(


-- 
Guillaume LECERF
GeeXboX developer - www.geexbox.org

Re: [PATCH v2 2/6] mtd: cfi: add writebufsize initialization

[Artem Bityutskiy]

On Thu, 2010-12-30 at 12:49 +0100, Guillaume LECERF wrote:
> 2010/12/30 Guillaume LECERF <glecerf@gmail.com>:
> > Anatolij, what about such patches ?
> > Note : this is totally untested ;)
> 
> Please disregard 08-patch.patch, I think it does not even compile :(

Anatolij is not answering, probably he is just very busy or having
holidays or something. I think it is better to wait a little with these
patches and let Anatolij look at this. So I'm removing them from my l2
tree so far and save them in the 'anatolij' branch of the l2 tree.

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

Re: [PATCH v2 2/6] mtd: cfi: add writebufsize initialization

[Anatolij Gustschin]

Hi,

On Wed, 05 Jan 2011 10:27:21 +0200
Artem Bityutskiy <dedekind1@gmail.com> wrote:

> On Thu, 2010-12-30 at 12:49 +0100, Guillaume LECERF wrote:
> > 2010/12/30 Guillaume LECERF <glecerf@gmail.com>:
> > > Anatolij, what about such patches ?
> > > Note : this is totally untested ;)
> > 
> > Please disregard 08-patch.patch, I think it does not even compile :(
> 
> Anatolij is not answering, probably he is just very busy or having
> holidays or something. I think it is better to wait a little with these
> patches and let Anatolij look at this. So I'm removing them from my l2
> tree so far and save them in the 'anatolij' branch of the l2 tree.

Sorry for not answering quickly, I was very busy and then was very
ill. The two patches from Guillaume are wrong. Please apply my patches
again.

Thanks,
Anatolij

Re: [PATCH v2 2/6] mtd: cfi: add writebufsize initialization

[Artem Bityutskiy]

On Thu, 2011-01-06 at 12:13 +0100, Anatolij Gustschin wrote:
> Hi,
> 
> On Wed, 05 Jan 2011 10:27:21 +0200
> Artem Bityutskiy <dedekind1@gmail.com> wrote:
> 
> > On Thu, 2010-12-30 at 12:49 +0100, Guillaume LECERF wrote:
> > > 2010/12/30 Guillaume LECERF <glecerf@gmail.com>:
> > > > Anatolij, what about such patches ?
> > > > Note : this is totally untested ;)
> > > 
> > > Please disregard 08-patch.patch, I think it does not even compile :(
> > 
> > Anatolij is not answering, probably he is just very busy or having
> > holidays or something. I think it is better to wait a little with these
> > patches and let Anatolij look at this. So I'm removing them from my l2
> > tree so far and save them in the 'anatolij' branch of the l2 tree.
> 
> Sorry for not answering quickly, I was very busy and then was very
> ill. The two patches from Guillaume are wrong. Please apply my patches
> again.

Yes, done, thanks.

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

Re: [PATCH v2 2/6] mtd: cfi: add writebufsize initialization

[Anatolij Gustschin]

Hi,

On Thu, 30 Dec 2010 11:32:11 +0100
Guillaume LECERF <glecerf@gmail.com> wrote:

> 2010/12/21 Artem Bityutskiy <dedekind1@gmail.com>:
> > Yes, fair question, Anatolij?
> 
> Hi all.
> 
> Anatolij, what about such patches ?
> Note : this is totally untested ;)

NACK. This is wrong. We need to export the real physical write buffer
size of _one_ chip to UBI. Otherwise we will run into the same trouble
and will end up with an unmountable UBIFS partition again.
It doesn't even make sense to test your two patches, it won't work.

Thanks,
Anatolij

Re: [PATCH v2 2/6] mtd: cfi: add writebufsize initialization

[Anatolij Gustschin]

Hi,

On Tue, 21 Dec 2010 16:45:24 +0200
Artem Bityutskiy <dedekind1@gmail.com> wrote:

> On Mon, 2010-12-20 at 17:36 +0100, ext Guillaume LECERF wrote:
> > Hello.
> > 
> > 2010/12/16 Anatolij Gustschin <agust@denx.de>:
> > > Initialize mtd->writebufsize to the value obtained
> > > by CFI query command at probe time.
> > >
> > > Signed-off-by: Anatolij Gustschin <agust@denx.de>
> > > ---
> > > --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> > > +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
> > > @@ -428,6 +428,10 @@ struct mtd_info *cfi_cmdset_0002(struct map_info *map, int primary)
> > >        mtd->flags   = MTD_CAP_NORFLASH;
> > >        mtd->name    = map->name;
> > >        mtd->writesize = 1;
> > > +       mtd->writebufsize = 1 << cfi->cfiq->MaxBufWriteSize;
> > 
> > What about wbufsize initialized in cfi_amdstd_write_buffers() to
> > "cfi_interleave(cfi) << cfi->cfiq->MaxBufWriteSize" ?
> > I don't know this piece of code, but I just wanted to point it to you.
> 
> Yes, fair question, Anatolij?

Just answered it.

Thanks,
Anatolij

Re: [PATCH v2 2/6] mtd: cfi: add writebufsize initialization

[Anatolij Gustschin]

On Mon, 20 Dec 2010 17:36:57 +0100
Guillaume LECERF <glecerf@gmail.com> wrote:

> Hello.

Hello.

> 2010/12/16 Anatolij Gustschin <agust@denx.de>:
> > Initialize mtd->writebufsize to the value obtained
> > by CFI query command at probe time.
> >
> > Signed-off-by: Anatolij Gustschin <agust@denx.de>
> > ---
> > --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> > +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
> > @@ -428,6 +428,10 @@ struct mtd_info *cfi_cmdset_0002(struct map_info *map, int primary)
> >        mtd->flags   = MTD_CAP_NORFLASH;
> >        mtd->name    = map->name;
> >        mtd->writesize = 1;
> > +       mtd->writebufsize = 1 << cfi->cfiq->MaxBufWriteSize;
> 
> What about wbufsize initialized in cfi_amdstd_write_buffers() to
> "cfi_interleave(cfi) << cfi->cfiq->MaxBufWriteSize" ?
> I don't know this piece of code, but I just wanted to point it to you.

wbufsize is initialized there so that it takes into account the
chip interleaving. If we e.g. have 64 byte write buffer in each
chip and two 16-bit chips in 32-bit bank, the wbufsize will be
initialized to 128 since the cfi_interleave(cfi) will return 2 in
this case. It is really local to the cfi driver. For UBI to work
correctly we need to export the the real physical write buffer size
of one chip, nothing else. So my patch is correct.

Anatolij

[PATCH v2 3/6] mtd: nand: add mtd->writebufsize initialization

[Anatolij Gustschin]

Initialize mtd->writebufsize to be equal to mtd->writesize.

Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
 drivers/mtd/nand/nand_base.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 5dd7ae4..4915067 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3439,6 +3439,7 @@ int nand_scan_tail(struct mtd_info *mtd)
 	mtd->resume = nand_resume;
 	mtd->block_isbad = nand_block_isbad;
 	mtd->block_markbad = nand_block_markbad;
+	mtd->writebufsize = mtd->writesize;
 
 	/* propagate ecc.layout to mtd_info */
 	mtd->ecclayout = chip->ecc.layout;
-- 
1.7.1

[PATCH v2 4/6] mtd: onenand: add mtd->writebufsize initialization

[Anatolij Gustschin]

Initialize mtd->writebufsize to be equal to mtd->writesize.

Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
 drivers/mtd/onenand/onenand_base.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c
index 2d7c90d..d552821 100644
--- a/drivers/mtd/onenand/onenand_base.c
+++ b/drivers/mtd/onenand/onenand_base.c
@@ -4076,6 +4076,7 @@ int onenand_scan(struct mtd_info *mtd, int maxchips)
 	mtd->block_isbad = onenand_block_isbad;
 	mtd->block_markbad = onenand_block_markbad;
 	mtd->owner = THIS_MODULE;
+	mtd->writebufsize = mtd->writesize;
 
 	/* Unlock whole block */
 	this->unlock_all(mtd);
-- 
1.7.1

[PATCH v2 5/6] mtd: initialize writebufsize in the MTD object of a partition

[Anatolij Gustschin]

Propagate the writebufsize to the partition's MTD object so
that UBI can set correct value for it's minimal I/O size
using the writebufsize field of MTD object of the partition.

By previous patches we added proper writebufsize field
initialization. Next patch can now change UBI to use
this field for setting the minimal I/O size.

Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
 drivers/mtd/mtdconcat.c |    1 +
 drivers/mtd/mtdpart.c   |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/mtdconcat.c b/drivers/mtd/mtdconcat.c
index bf8de09..5f5777b 100644
--- a/drivers/mtd/mtdconcat.c
+++ b/drivers/mtd/mtdconcat.c
@@ -776,6 +776,7 @@ struct mtd_info *mtd_concat_create(struct mtd_info *subdev[],	/* subdevices to c
 	concat->mtd.size = subdev[0]->size;
 	concat->mtd.erasesize = subdev[0]->erasesize;
 	concat->mtd.writesize = subdev[0]->writesize;
+	concat->mtd.writebufsize = subdev[0]->writebufsize;
 	concat->mtd.subpage_sft = subdev[0]->subpage_sft;
 	concat->mtd.oobsize = subdev[0]->oobsize;
 	concat->mtd.oobavail = subdev[0]->oobavail;
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 1047ff0..b910a37 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -384,6 +384,7 @@ static struct mtd_part *allocate_partition(struct mtd_info *master,
 	slave->mtd.flags = master->flags & ~part->mask_flags;
 	slave->mtd.size = part->size;
 	slave->mtd.writesize = master->writesize;
+	slave->mtd.writebufsize = master->writebufsize;
 	slave->mtd.oobsize = master->oobsize;
 	slave->mtd.oobavail = master->oobavail;
 	slave->mtd.subpage_sft = master->subpage_sft;
-- 
1.7.1

[PATCH v2 6/6] UBI: use mtd->writebufsize to set minimal I/O unit size

[Anatolij Gustschin]

Previously we used mtd->writesize field to set UBI's minimal
I/O unit size. This sometimes caused UBIFS recovery issues
when mounting an uncleanly unmounted UBIFS partition on NOR
flash since mtd->writesize is 1 byte for NOR flash. The
MTD CFI driver however often performs writing multiple
bytes in one programming operation using the chip's write
buffer. We have to use the size of this write buffer as
a minimal I/O unit size for UBI on NOR flash to fix the
observed UBIFS recovery issues.

Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
 drivers/mtd/ubi/build.c |   19 ++++++++++++++++++-
 1 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index 5ebe280..c2d5310 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -672,7 +672,24 @@ static int io_init(struct ubi_device *ubi)
 		ubi->nor_flash = 1;
 	}
 
-	ubi->min_io_size = ubi->mtd->writesize;
+	/*
+	 * Sets minimal I/O unit size (min_io_size) for UBI. On NAND flash
+	 * min_io_size should be equivalent to ubi->mtd->writesize.
+	 * In case of NOR flash minimal I/O size must be equal to the size
+	 * of the write buffer used by internal flash programming algorithm.
+	 * This requirement results from the fact that the flash programming
+	 * operation could be interrupted by a power cut or a system reset
+	 * causing corrupted (partially written) areas in a NOR flash sector.
+	 * Knowing the size of potentially corrupted areas UBIFS scanning
+	 * and recovery algorithms are able to perform successful recovery.
+	 */
+	if (ubi->mtd->type == MTD_NANDFLASH)
+		ubi_assert(ubi->mtd->writebufsize == ubi->mtd->writesize);
+	else if (ubi->mtd->type == MTD_NORFLASH)
+		ubi_assert(ubi->mtd->writebufsize > 0);
+
+	ubi->min_io_size = ubi->mtd->writebufsize;
+
 	ubi->hdrs_min_io_size = ubi->mtd->writesize >> ubi->mtd->subpage_sft;
 
 	/*
-- 
1.7.1

Re: [PATCH v2 6/6] UBI: use mtd->writebufsize to set minimal I/O unit size

[Artem Bityutskiy]

On Thu, 2010-12-16 at 23:42 +0100, Anatolij Gustschin wrote:
> Previously we used mtd->writesize field to set UBI's minimal
> I/O unit size. This sometimes caused UBIFS recovery issues
> when mounting an uncleanly unmounted UBIFS partition on NOR
> flash since mtd->writesize is 1 byte for NOR flash. The
> MTD CFI driver however often performs writing multiple
> bytes in one programming operation using the chip's write
> buffer. We have to use the size of this write buffer as
> a minimal I/O unit size for UBI on NOR flash to fix the
> observed UBIFS recovery issues.
> 
> Signed-off-by: Anatolij Gustschin <agust@denx.de>

I've re-wrote the commentary and pushed, please, check.

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

Re: [PATCH v2 6/6] UBI: use mtd->writebufsize to set minimal I/O unit size

[Anatolij Gustschin]

On Sun, 19 Dec 2010 19:46:19 +0200
Artem Bityutskiy <dedekind1@gmail.com> wrote:

> On Thu, 2010-12-16 at 23:42 +0100, Anatolij Gustschin wrote:
> > Previously we used mtd->writesize field to set UBI's minimal
> > I/O unit size. This sometimes caused UBIFS recovery issues
> > when mounting an uncleanly unmounted UBIFS partition on NOR
> > flash since mtd->writesize is 1 byte for NOR flash. The
> > MTD CFI driver however often performs writing multiple
> > bytes in one programming operation using the chip's write
> > buffer. We have to use the size of this write buffer as
> > a minimal I/O unit size for UBI on NOR flash to fix the
> > observed UBIFS recovery issues.
> > 
> > Signed-off-by: Anatolij Gustschin <agust@denx.de>
> 
> I've re-wrote the commentary and pushed, please, check.

Checked, it is ok. Thanks!

Anatolij