DataFlash & writesize

DataFlash & writesize

[Artem B. Bityutskiy]

Hello Andrew,

AFAIR you are the guy who provided DataFlash drivers. May I ask you to 
fix them as I suspect they may be broken in mtd-2.6.git.

We've introduced a mtd->writesize field recently and it has to be filled 
by the flash driver. The field contains the minimal Flash's I/O unit 
size. For NOR it is 1, for NAND it is NAND page, or less if multiple 
writes to a NAND page are supported, etc. I once read a DataFlash 
datasheet and suspect this has to be DataFlash sector size, but I'm not 
sure.

So, I'm asking you to initialize mtd->writesize correctly.

Thanks.

-- 
Best Regards,
Artem B. Bityutskiy,
St.-Petersburg, Russia.

Re: DataFlash & writesize

[Andrew Victor]

hi,

> So, I'm asking you to initialize mtd->writesize correctly.

Since the writesize change is now in mainline git, here is a trivial
patch to set mtd->writesize for DataFlash.
[writesize = erasesize = pagesize]

(It compiles, but not tested).


Signed-off-by: Andrew Victor <andrew@sanpeople.com>


diff -urN linux-2.6.17-git5.orig/drivers/mtd/devices/mtd_dataflash.c linux-2.6.17-git5/drivers/mtd/devices/mtd_dataflash.c
--- linux-2.6.17-git5.orig/drivers/mtd/devices/mtd_dataflash.c	Tue May 30 11:41:52 2006
+++ linux-2.6.17-git5/drivers/mtd/devices/mtd_dataflash.c	Fri Jun 23 12:07:09 2006
@@ -478,6 +478,7 @@
 	device->name = (pdata && pdata->name) ? pdata->name : priv->name;
 	device->size = nr_pages * pagesize;
 	device->erasesize = pagesize;
+	device->writesize = pagesize;
 	device->owner = THIS_MODULE;
 	device->type = MTD_DATAFLASH;
 	device->flags = MTD_CAP_NORFLASH;

Re: DataFlash & writesize

[Artem B. Bityutskiy]

On Fri, 2006-06-23 at 12:27 +0200, Andrew Victor wrote:
> Since the writesize change is now in mainline git, here is a trivial
> patch to set mtd->writesize for DataFlash.
> [writesize = erasesize = pagesize]
> 

Andrew,

I made the same yesterday and it is in GIT, but thank you anyway!

-- 
Best Regards,
Artem B. Bityutskiy,
St.-Petersburg, Russia.

Re: DataFlash & writesize

[David Brownell]

On Friday 23 June 2006 3:43 am, Artem B. Bityutskiy wrote:
> On Fri, 2006-06-23 at 12:27 +0200, Andrew Victor wrote:
> > Since the writesize change is now in mainline git, here is a trivial
> > patch to set mtd->writesize for DataFlash.
> > [writesize = erasesize = pagesize]
> > 
> 
> Andrew,
> 
> I made the same yesterday and it is in GIT, but thank you anyway!

What are the intended semantics for "writesize" by the way?

I noticed I was getting that first BUG_ON in mtdcore:add_mtd_device
since, with NOR CFI cmdset 1 flash, with current GIT.  Seems like
writesie should have been set up already.

- Dave

Re: DataFlash & writesize

[Josh Boyer]

On 6/26/06, David Brownell <david-b@pacbell.net> wrote:
> On Friday 23 June 2006 3:43 am, Artem B. Bityutskiy wrote:
> > On Fri, 2006-06-23 at 12:27 +0200, Andrew Victor wrote:
> > > Since the writesize change is now in mainline git, here is a trivial
> > > patch to set mtd->writesize for DataFlash.
> > > [writesize = erasesize = pagesize]
> > >
> >
> > Andrew,
> >
> > I made the same yesterday and it is in GIT, but thank you anyway!
>
> What are the intended semantics for "writesize" by the way?
>
> I noticed I was getting that first BUG_ON in mtdcore:add_mtd_device
> since, with NOR CFI cmdset 1 flash, with current GIT.  Seems like
> writesie should have been set up already.

Which git tree and when?  That was a bug that should have been fixed
last week in the mtd-2.6.git tree.  And it should also go to Linus
ASAP.

josh

Re: DataFlash & writesize

[David Brownell]

On Monday 26 June 2006 2:42 pm, Josh Boyer wrote:
> On 6/26/06, David Brownell <david-b@pacbell.net> wrote:
> > On Friday 23 June 2006 3:43 am, Artem B. Bityutskiy wrote:
> > > On Fri, 2006-06-23 at 12:27 +0200, Andrew Victor wrote:
> > > > Since the writesize change is now in mainline git, here is a trivial
> > > > patch to set mtd->writesize for DataFlash.
> > > > [writesize = erasesize = pagesize]
> > > >
> > >
> > > Andrew,
> > >
> > > I made the same yesterday and it is in GIT, but thank you anyway!
> >
> > What are the intended semantics for "writesize" by the way?

?

> > I noticed I was getting that first BUG_ON in mtdcore:add_mtd_device
> > since, with NOR CFI cmdset 1 flash, with current GIT.  Seems like
> > writesie should have been set up already.
> 
> Which git tree and when?  

Kernel.org GIT tree, pulled yesterday.

> That was a bug that should have been fixed 
> last week in the mtd-2.6.git tree.  And it should also go to Linus
> ASAP.
> 
> josh
> 

Re: DataFlash & writesize

[Thomas Gleixner]

On Mon, 2006-06-26 at 15:27 -0700, David Brownell wrote:
> > Which git tree and when?  
> 
> Kernel.org GIT tree, pulled yesterday.

Does not have the latest fixups. MTD git has. Sorry for the
inconveniance.

	tglx

Re: DataFlash & writesize

[Artem B. Bityutskiy]

Hello David,

David Brownell wrote:
> What are the intended semantics for "writesize" by the way?
Design-wise, it an attempt to generalize MTD. Different flashes have 
different minimal I/O unit size, like 1 byte in case of NOR, 16 bytes in 
case of ECCed NOR, 512/2K in case of NAND, etc. There were no such a 
field before and we had to do ugly things like


switch (mtd->type) {
	case MTD_NORFLASH:
		blah();
		break;
	case MTD_NANDFLASH:
		blah1();

etc.

Here is a comment from include/linux/mtd.h:

/* Minimal writable flash unit size. In case of NOR flash it is 1 (even
  * though individual bits can be cleared), in case of NAND flash it is
  * one NAND page (or half, or one-fourths of it), in case of ECC-ed NOR
  * it is of ECC block size, etc. It is illegal to have writesize = 0.
  * Any driver registering a struct mtd_info must ensure a writesize of
  * 1 or larger.
  */

> 
> I noticed I was getting that first BUG_ON in mtdcore:add_mtd_device
> since, with NOR CFI cmdset 1 flash, with current GIT.  Seems like
> writesie should have been set up already.

We fixed all drivers and now it should be fine - see commit 
17ffc7ba6d7ea68b8d5f55a5ca1b87163e69720d

-- 
Best Regards,
Artem B. Bityutskiy,
St.-Petersburg, Russia.

Re: DataFlash & writesize

[Artem B. Bityutskiy]

David,

David Brownell wrote:
> What are the intended semantics for "writesize" by the way?

Design-wise, it an attempt to generalize MTD. Different flashes have 
different minimal I/O unit size, like 1 byte in case of NOR, 16 bytes in 
case of ECCed NOR, 512/2K in case of NAND, etc. There were no such a 
field before and we had to do ugly things like


switch (mtd->type) {
	case MTD_NORFLASH:
		blah();
		break;
	case MTD_NANDFLASH:
		blah1();

etc.

Here is a comment from include/linux/mtd.h:

/* Minimal writable flash unit size. In case of NOR flash it is 1 (even
  * though individual bits can be cleared), in case of NAND flash it is
  * one NAND page (or half, or one-fourths of it), in case of ECC-ed NOR
  * it is of ECC block size, etc. It is illegal to have writesize = 0.
  * Any driver registering a struct mtd_info must ensure a writesize of
  * 1 or larger.
  */

> I noticed I was getting that first BUG_ON in mtdcore:add_mtd_device
> since, with NOR CFI cmdset 1 flash, with current GIT.  Seems like
> writesie should have been set up already.

We fixed all drivers and now it should be fine - see commit 
17ffc7ba6d7ea68b8d5f55a5ca1b87163e69720d

-- 
Best Regards,
Artem B. Bityutskiy,
St.-Petersburg, Russia.

Re: DataFlash & writesize

[David Brownell]

On Tuesday 27 June 2006 12:05 am, Artem B. Bityutskiy wrote:
> David,
> 
> David Brownell wrote:
> > What are the intended semantics for "writesize" by the way?
> 
> Design-wise, it an attempt to generalize MTD. Different flashes have 
> different minimal I/O unit size, like 1 byte in case of NOR, 16 bytes in 
> case of ECCed NOR, 512/2K in case of NAND, etc.

OK, thanks.  For some reason the "writesize" I found didn't include
the <linux/mtd/mtd.h> comment you mentioned... I guess that was the
one in mtd-abi.h instead.


> > I noticed I was getting that first BUG_ON in mtdcore:add_mtd_device
> > since, with NOR CFI cmdset 1 flash, with current GIT.  Seems like
> > writesie should have been set up already.
> 
> We fixed all drivers and now it should be fine - see commit 
> 17ffc7ba6d7ea68b8d5f55a5ca1b87163e69720d

It's not fixed upstream yet.

- Dave