Patchwork [2/2] mtd: cfi: fix writebufsize initialization

login
register
mail settings
Submitter Anatolij Gustschin
Date Feb. 10, 2011, 3:01 p.m.
Message ID <1297350106-16383-3-git-send-email-agust@denx.de>
Download mbox | patch
Permalink /patch/82613/
State New
Headers show

Comments

Anatolij Gustschin - Feb. 10, 2011, 3:01 p.m.
When initializing mtd->writebufsize, we must take into account
possible flash chip interleaving. Wrong writebufsize initialization
caused UBIFS recovery issues resulting in unmountable UBIFS file
system on NOR flash partitions.

Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
 drivers/mtd/chips/cfi_cmdset_0001.c |    2 +-
 drivers/mtd/chips/cfi_cmdset_0002.c |    2 +-
 drivers/mtd/chips/cfi_cmdset_0020.c |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)
Guillaume LECERF - Feb. 10, 2011, 3:13 p.m.
Hi Anatolij.

2011/2/10 Anatolij Gustschin <agust@denx.de>:
> When initializing mtd->writebufsize, we must take into account
> possible flash chip interleaving. Wrong writebufsize initialization
> caused UBIFS recovery issues resulting in unmountable UBIFS file
> system on NOR flash partitions.
> -       mtd->writebufsize = 1 << cfi->cfiq->MaxBufWriteSize;
> +       mtd->writebufsize = cfi_interleave(cfi) << cfi->cfiq->MaxBufWriteSize;


This is exactly what I told you (
http://lists.infradead.org/pipermail/linux-mtd/2010-December/033559.html
).

Could you examine the (not compilable) patches I attached and merge
them with your current patch ?
Artem Bityutskiy - Feb. 10, 2011, 3:27 p.m.
On Thu, 2011-02-10 at 16:13 +0100, Guillaume LECERF wrote:
> Hi Anatolij.
> 
> 2011/2/10 Anatolij Gustschin <agust@denx.de>:
> > When initializing mtd->writebufsize, we must take into account
> > possible flash chip interleaving. Wrong writebufsize initialization
> > caused UBIFS recovery issues resulting in unmountable UBIFS file
> > system on NOR flash partitions.
> > -       mtd->writebufsize = 1 << cfi->cfiq->MaxBufWriteSize;
> > +       mtd->writebufsize = cfi_interleave(cfi) << cfi->cfiq->MaxBufWriteSize;
> 
> 
> This is exactly what I told you (
> http://lists.infradead.org/pipermail/linux-mtd/2010-December/033559.html
> ).

Hehe, indeed :-) Anatolij came to that hard way instead :-)))
Anatolij Gustschin - Feb. 11, 2011, 12:04 p.m.
Hi Guillaume,

On Thu, 10 Feb 2011 16:13:31 +0100
Guillaume LECERF <glecerf@gmail.com> wrote:

> Hi Anatolij.
> 
> 2011/2/10 Anatolij Gustschin <agust@denx.de>:
> > When initializing mtd->writebufsize, we must take into account
> > possible flash chip interleaving. Wrong writebufsize initialization
> > caused UBIFS recovery issues resulting in unmountable UBIFS file
> > system on NOR flash partitions.
> > -       mtd->writebufsize = 1 << cfi->cfiq->MaxBufWriteSize;
> > +       mtd->writebufsize = cfi_interleave(cfi) << cfi->cfiq->MaxBufWriteSize;
> 
> 
> This is exactly what I told you (
> http://lists.infradead.org/pipermail/linux-mtd/2010-December/033559.html
> ).

Yes, this is true. But it couldn't have worked without further fixes
for UBI:

ubiattach -m 8 -d 0 /dev/ubi_ctrl
UBI: attaching mtd8 to ubi0
UBI: physical eraseblock size:   262144 bytes (256 KiB)
UBI: logical eraseblock size:    262016 bytes
UBI: smallest flash I/O unit:    128
UBI: sub-page size:              1
UBI: VID header offset:          64 (aligned 64)
UBI: data offset:                128
UBI error: validate_ec_hdr: bad VID header offset 128, expected 64
UBI error: validate_ec_hdr: bad EC header
Call Trace:
[c3fbbc40] [c0009220] show_stack+0xac/0x1d8 (unreliable)
[c3fbbc90] [c0009378] dump_stack+0x2c/0x44
[c3fbbca0] [c0257d90] ubi_io_read_ec_hdr+0x2d8/0x3b4
[c3fbbcd0] [c025b868] ubi_scan+0x1b4/0xc80
[c3fbbd50] [c02513d8] ubi_attach_mtd_dev+0x5e0/0xf1c
[c3fbbe50] [c0252194] ctrl_cdev_ioctl+0x164/0x234
[c3fbbe90] [c00c0bd4] vfs_ioctl+0x48/0xa8
[c3fbbea0] [c00c0e58] do_vfs_ioctl+0x98/0x704
[c3fbbf10] [c00c1514] sys_ioctl+0x50/0x84
[c3fbbf40] [c0013564] ret_from_syscall+0x0/0x38
--- Exception: c01 at 0xff38b10
    LR = 0xffec8a8
UBI error: ubi_io_read_ec_hdr: validation failed for PEB 0
UBI error: ubi_attach_mtd_dev: failed to attach by scanning, error -22
ubiattach: error!: cannot attach mtd8
           error 22 (Invalid argument)

It works for me now when I use recent UBIFS patches from Artem
([PATCH v2 0/5] UBIFS: fix recovery on CFI NOR).

> Could you examine the (not compilable) patches I attached and merge
> them with your current patch ?

Currently we are seeing two other issues with UBIFS recovery after
power cuts, but these seem not to be caused by to big write buffer.
I'm looking for the reason for the observed corruptions now and
will look at your patches later.

Thanks!
Anatolij
Guillaume LECERF - Feb. 11, 2011, 12:25 p.m.
Hi Anatolij.

2011/2/11 Anatolij Gustschin <agust@denx.de>:
> Currently we are seeing two other issues with UBIFS recovery after
> power cuts, but these seem not to be caused by to big write buffer.
> I'm looking for the reason for the observed corruptions now and
> will look at your patches later.

There is no hurry for you to review my patch, as it is just a
simplification over your modifications.
BTW I will resend it once yours is in.
Artem Bityutskiy - Feb. 11, 2011, 2:36 p.m.
On Thu, 2011-02-10 at 16:01 +0100, Anatolij Gustschin wrote:
> When initializing mtd->writebufsize, we must take into account
> possible flash chip interleaving. Wrong writebufsize initialization
> caused UBIFS recovery issues resulting in unmountable UBIFS file
> system on NOR flash partitions.
> 
> Signed-off-by: Anatolij Gustschin <agust@denx.de>

So should I put this patch to l2 tree or you'll send another one. 

Guillaume, does this patch have your acked-by? For me it looks good.
Guillaume LECERF - Feb. 11, 2011, 3:05 p.m.
Hello Artem.

2011/2/11 Artem Bityutskiy <dedekind1@gmail.com>:
> So should I put this patch to l2 tree or you'll send another one.
>
> Guillaume, does this patch have your acked-by? For me it looks good.

I see no objection.

Acked-by: Guillaume LECERF <glecerf@gmail.com>
Artem Bityutskiy - Feb. 11, 2011, 3:32 p.m.
On Thu, 2011-02-10 at 16:01 +0100, Anatolij Gustschin wrote:
> When initializing mtd->writebufsize, we must take into account
> possible flash chip interleaving. Wrong writebufsize initialization
> caused UBIFS recovery issues resulting in unmountable UBIFS file
> system on NOR flash partitions.
> 
> Signed-off-by: Anatolij Gustschin <agust@denx.de>

Pushed to l2-mtd-2.6.git, thanks!

Patch

diff --git a/drivers/mtd/chips/cfi_cmdset_0001.c b/drivers/mtd/chips/cfi_cmdset_0001.c
index 6ad29b7..d72718a 100644
--- a/drivers/mtd/chips/cfi_cmdset_0001.c
+++ b/drivers/mtd/chips/cfi_cmdset_0001.c
@@ -453,7 +453,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->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 30a098e..637e728 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -354,7 +354,7 @@  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;
+	mtd->writebufsize = cfi_interleave(cfi) << cfi->cfiq->MaxBufWriteSize;
 
 	DEBUG(MTD_DEBUG_LEVEL3, "MTD %s(): write buffer size %d\n",
 		__func__, mtd->writebufsize);
diff --git a/drivers/mtd/chips/cfi_cmdset_0020.c b/drivers/mtd/chips/cfi_cmdset_0020.c
index 30e37f6..cfb3351 100644
--- a/drivers/mtd/chips/cfi_cmdset_0020.c
+++ b/drivers/mtd/chips/cfi_cmdset_0020.c
@@ -239,7 +239,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;
+	mtd->writebufsize = cfi_interleave(cfi) << cfi->cfiq->MaxBufWriteSize;
 	map->fldrv = &cfi_staa_chipdrv;
 	__module_get(THIS_MODULE);
 	mtd->name = map->name;