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

login
register
mail settings
Submitter Anatolij Gustschin
Date Dec. 16, 2010, 10:42 p.m.
Message ID <1292539339-25111-3-git-send-email-agust@denx.de>
Download mbox | patch
Permalink /patch/75814/
State New
Headers show

Comments

Anatolij Gustschin - Dec. 16, 2010, 10:42 p.m.
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(-)
Artem Bityutskiy - Dec. 19, 2010, 5:45 p.m.
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.
Guillaume LECERF - Dec. 20, 2010, 4:36 p.m.
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.
Artem Bityutskiy - Dec. 21, 2010, 2:45 p.m.
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?
Guillaume LECERF - Dec. 30, 2010, 10:32 a.m.
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 - Dec. 30, 2010, 11:49 a.m.
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 :(
Artem Bityutskiy - Jan. 5, 2011, 8:27 a.m.
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.
Anatolij Gustschin - Jan. 6, 2011, 10:48 a.m.
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
Anatolij Gustschin - Jan. 6, 2011, 10:49 a.m.
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
Anatolij Gustschin - Jan. 6, 2011, 11 a.m.
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
Anatolij Gustschin - Jan. 6, 2011, 11:13 a.m.
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
Artem Bityutskiy - Jan. 6, 2011, 11:14 a.m.
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.

Patch

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;