public inbox for linux-mtd@lists.infradead.org
 help / color / mirror / Atom feed
* Initialising NAND OOB with Software ECC
@ 2003-02-13 20:20 Phil Thompson
  2003-02-14  9:05 ` Thomas Gleixner
  0 siblings, 1 reply; 14+ messages in thread
From: Phil Thompson @ 2003-02-13 20:20 UTC (permalink / raw)
  To: linux-mtd

What's the standard method for initialising the OOB area for software ECC when 
flashing a new JFFS2 image?

The MEMWRITEDATA ioctl looks at first to be the method to use because it uses 
mtd->write_ecc(). However 0 (ie. NAND_ECC_NONE) is always passed as the last 
parameter which disables writing the ECC data. Wouldn't it be more flexible 
to allow user space to specify the OOB format in the ioctl? Or have another 
ioctl to support writing ECC data?

I know I can use MEMWRITEOOB to write the data, but that means that I have to 
compute it in user space - which seems pointless given the kernel can do it.

Thanks,
Phil

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Initialising NAND OOB with Software ECC
  2003-02-13 20:20 Initialising NAND OOB with Software ECC Phil Thompson
@ 2003-02-14  9:05 ` Thomas Gleixner
  2003-02-14  9:46   ` Phil Thompson
  0 siblings, 1 reply; 14+ messages in thread
From: Thomas Gleixner @ 2003-02-14  9:05 UTC (permalink / raw)
  To: linux-mtd

On Thursday 13 February 2003 21:20, Phil Thompson wrote:
> What's the standard method for initialising the OOB area for software ECC
> when flashing a new JFFS2 image?
>
> The MEMWRITEDATA ioctl looks at first to be the method to use because it
> uses mtd->write_ecc(). However 0 (ie. NAND_ECC_NONE) is always passed as
> the last parameter which disables writing the ECC data. Wouldn't it be more
> flexible to allow user space to specify the OOB format in the ioctl? Or
> have another ioctl to support writing ECC data?
>
> I know I can use MEMWRITEOOB to write the data, but that means that I have
> to compute it in user space - which seems pointless given the kernel can do
> it.
True. I planned to make an ioctrl so you can write with the appropriate ECC 
selector to NAND, but I hadnt hat the time to do so.
Poke me enough and maybe I find some time

-- 
Thomas
________________________________________________________________________
linutronix - competence in embedded & realtime linux
http://www.linutronix.de
mail: tglx at linutronix.de

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Initialising NAND OOB with Software ECC
  2003-02-14  9:05 ` Thomas Gleixner
@ 2003-02-14  9:46   ` Phil Thompson
  2003-02-14 11:00     ` Thomas Gleixner
  0 siblings, 1 reply; 14+ messages in thread
From: Phil Thompson @ 2003-02-14  9:46 UTC (permalink / raw)
  To: linux-mtd

On Friday 14 February 2003 9:05 am, Thomas Gleixner wrote:
> On Thursday 13 February 2003 21:20, Phil Thompson wrote:
> > What's the standard method for initialising the OOB area for software ECC
> > when flashing a new JFFS2 image?
> >
> > The MEMWRITEDATA ioctl looks at first to be the method to use because it
> > uses mtd->write_ecc(). However 0 (ie. NAND_ECC_NONE) is always passed as
> > the last parameter which disables writing the ECC data. Wouldn't it be
> > more flexible to allow user space to specify the OOB format in the ioctl?
> > Or have another ioctl to support writing ECC data?
> >
> > I know I can use MEMWRITEOOB to write the data, but that means that I
> > have to compute it in user space - which seems pointless given the kernel
> > can do it.
>
> True. I planned to make an ioctrl so you can write with the appropriate ECC
> selector to NAND, but I hadnt hat the time to do so.
> Poke me enough and maybe I find some time

I'm happy to do it and send you a patch.

How do you want it implemented?

Just a new ioctl that sets the selector for subsequent MEMWRITEDATA and 
MEMREADDATA calls?

Is there a need for an ioctl to return the current selector?

Is there a need to validate the selector, or just leave it up to MEMWRITEDATA 
and MEMREADDATA to deal with?

IMHO it would be cleaner to replace the struct mtd_oob_buf argument to 
MEMWRITEDATA and MEMREADDATA with a structure that included the selector to 
use for that operation. But that would break binary compatibility. Is this an 
option at this stage?

Phil

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Initialising NAND OOB with Software ECC
  2003-02-14  9:46   ` Phil Thompson
@ 2003-02-14 11:00     ` Thomas Gleixner
  2003-02-14 11:36       ` Phil Thompson
  0 siblings, 1 reply; 14+ messages in thread
From: Thomas Gleixner @ 2003-02-14 11:00 UTC (permalink / raw)
  To: linux-mtd

On Friday 14 February 2003 10:46, Phil Thompson wrote:
> How do you want it implemented?
> Just a new ioctl that sets the selector for subsequent MEMWRITEDATA and
> MEMREADDATA calls?
Not a good idea.

> IMHO it would be cleaner to replace the struct mtd_oob_buf argument to
> MEMWRITEDATA and MEMREADDATA with a structure that included the selector to
> use for that operation. But that would break binary compatibility. Is this
> an option at this stage?
Yep, as it's for NAND only and as there are no tools, which are usefull, I 
think we can do this and implement it in a clean way. Then we can overhaul 
nandtest utils in mtd/utils to give an example. This should include a 
"copyImageToNAND" utility with support for other filesystems too.

My suggestion:

struct nand_ctrl {
	int	oob_select;	// selector for OOB usage
	int	data_lenght;	// length of data
	int	oob_length;	// length of oob-buffer
};
oob_length can be 0. If its not zero it must be 8 * (data_length / 256) for 
write and 12 * (data_length / 256) for read, as nand_read_ecc appends the 
ecc_status result behind each 8 byte OOB.
struct nand_oob_read {
	char	oob_data[8];
	int	ecc_status;
}

JFFS2 needs no oob buffer for write, but YAFFS does.

data_length for write should be checked, if it is a multiple of page size 
(mtd->oobblock), which is either 256 or 512 at the moment. Deny writing parts 
of pages.

David: any complaints ?

-- 
Thomas
________________________________________________________________________
linutronix - competence in embedded & realtime linux
http://www.linutronix.de
mail: tglx at linutronix.de

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Initialising NAND OOB with Software ECC
  2003-02-14 11:00     ` Thomas Gleixner
@ 2003-02-14 11:36       ` Phil Thompson
  2003-02-14 13:02         ` Thomas Gleixner
  0 siblings, 1 reply; 14+ messages in thread
From: Phil Thompson @ 2003-02-14 11:36 UTC (permalink / raw)
  To: linux-mtd

On Friday 14 February 2003 11:00 am, Thomas Gleixner wrote:
> On Friday 14 February 2003 10:46, Phil Thompson wrote:
> > How do you want it implemented?
> > Just a new ioctl that sets the selector for subsequent MEMWRITEDATA and
> > MEMREADDATA calls?
>
> Not a good idea.
>
> > IMHO it would be cleaner to replace the struct mtd_oob_buf argument to
> > MEMWRITEDATA and MEMREADDATA with a structure that included the selector
> > to use for that operation. But that would break binary compatibility. Is
> > this an option at this stage?
>
> Yep, as it's for NAND only and as there are no tools, which are usefull, I
> think we can do this and implement it in a clean way. Then we can overhaul
> nandtest utils in mtd/utils to give an example. This should include a
> "copyImageToNAND" utility with support for other filesystems too.
>
> My suggestion:
>
> struct nand_ctrl {
> 	int	oob_select;	// selector for OOB usage
> 	int	data_lenght;	// length of data
> 	int	oob_length;	// length of oob-buffer
> };

What about the other elements of struct mtd_oob_buf, ie. start and ptr?

What about a pointer to the oob buffer?

> oob_length can be 0. If its not zero it must be 8 * (data_length / 256) for
> write and 12 * (data_length / 256) for read, as nand_read_ecc appends the
> ecc_status result behind each 8 byte OOB.
> struct nand_oob_read {
> 	char	oob_data[8];
> 	int	ecc_status;
> }
>
> JFFS2 needs no oob buffer for write, but YAFFS does.

JFFS2 doesn't need one for read either does it?

> data_length for write should be checked, if it is a multiple of page size
> (mtd->oobblock), which is either 256 or 512 at the moment. Deny writing
> parts of pages.
>
> David: any complaints ?

Phil

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Initialising NAND OOB with Software ECC
  2003-02-14 11:36       ` Phil Thompson
@ 2003-02-14 13:02         ` Thomas Gleixner
  2003-02-14 13:14           ` David Woodhouse
  2003-02-14 13:25           ` Phil Thompson
  0 siblings, 2 replies; 14+ messages in thread
From: Thomas Gleixner @ 2003-02-14 13:02 UTC (permalink / raw)
  To: linux-mtd

On Friday 14 February 2003 12:36, Phil Thompson wrote:
> What about the other elements of struct mtd_oob_buf, ie. start and ptr?
Ok, I forgot start.
> What about a pointer to the oob buffer?
I see, we need data too. :)
But then we should add two pointers, so you can hold two different buffers 
from userspace.
truct nand_ctrl {
	int	start;			// startadress 
	int	oob_select;	// selector for OOB usage
 	int	data_lenght;	// length of data
 	int	oob_length;	// length of oob-buffer
	int	data_ptr;		// pointer to data buffer
	int	oob_ptr;		// pointer to oob buffer
};

> > JFFS2 needs no oob buffer for write, but YAFFS does.
> JFFS2 doesn't need one for read either does it?
No, you can just use it if you're curious or, if you want to read a complete 
image with oob-data included, which could be useful for debugging ....

-- 
Thomas
________________________________________________________________________
linutronix - competence in embedded & realtime linux
http://www.linutronix.de
mail: tglx at linutronix.de

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Initialising NAND OOB with Software ECC
  2003-02-14 13:02         ` Thomas Gleixner
@ 2003-02-14 13:14           ` David Woodhouse
  2003-02-14 13:27             ` Thomas Gleixner
  2003-02-14 13:25           ` Phil Thompson
  1 sibling, 1 reply; 14+ messages in thread
From: David Woodhouse @ 2003-02-14 13:14 UTC (permalink / raw)
  To: linux-mtd

On Fri, 2003-02-14 at 13:02, Thomas Gleixner wrote:
> But then we should add two pointers, so you can hold two different buffers 
> from userspace.

Ug. This rapidly becomes vile. How about adding a new device for the OOB
data, using pread/pwrite on that.

-- 
dwmw2

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Initialising NAND OOB with Software ECC
  2003-02-14 13:02         ` Thomas Gleixner
  2003-02-14 13:14           ` David Woodhouse
@ 2003-02-14 13:25           ` Phil Thompson
  1 sibling, 0 replies; 14+ messages in thread
From: Phil Thompson @ 2003-02-14 13:25 UTC (permalink / raw)
  To: linux-mtd

On Friday 14 February 2003 1:02 pm, Thomas Gleixner wrote:
> On Friday 14 February 2003 12:36, Phil Thompson wrote:
> > What about the other elements of struct mtd_oob_buf, ie. start and ptr?
>
> Ok, I forgot start.
>
> > What about a pointer to the oob buffer?
>
> I see, we need data too. :)
> But then we should add two pointers, so you can hold two different buffers
> from userspace.
> truct nand_ctrl {
> 	int	start;			// startadress
> 	int	oob_select;	// selector for OOB usage
>  	int	data_lenght;	// length of data
>  	int	oob_length;	// length of oob-buffer
> 	int	data_ptr;		// pointer to data buffer
> 	int	oob_ptr;		// pointer to oob buffer
> };
>
> > > JFFS2 needs no oob buffer for write, but YAFFS does.
> >
> > JFFS2 doesn't need one for read either does it?
>
> No, you can just use it if you're curious or, if you want to read a
> complete image with oob-data included, which could be useful for debugging

OK. I'll send you a patch off-list - probably early next week.

Phil

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Initialising NAND OOB with Software ECC
  2003-02-14 13:14           ` David Woodhouse
@ 2003-02-14 13:27             ` Thomas Gleixner
  2003-02-14 13:36               ` David Woodhouse
  0 siblings, 1 reply; 14+ messages in thread
From: Thomas Gleixner @ 2003-02-14 13:27 UTC (permalink / raw)
  To: linux-mtd

On Friday 14 February 2003 14:14, David Woodhouse wrote:
> On Fri, 2003-02-14 at 13:02, Thomas Gleixner wrote:
> > But then we should add two pointers, so you can hold two different
> > buffers from userspace.
>
> Ug. This rapidly becomes vile. How about adding a new device for the OOB
> data, using pread/pwrite on that.
True, but pread/pwrite does not support two pointers.

We can use one pointer, but then you have to copy things around in userspace.

-- 
Thomas
________________________________________________________________________
linutronix - competence in embedded & realtime linux
http://www.linutronix.de
mail: tglx at linutronix.de

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Initialising NAND OOB with Software ECC
  2003-02-14 13:27             ` Thomas Gleixner
@ 2003-02-14 13:36               ` David Woodhouse
  2003-02-14 13:50                 ` Thomas Gleixner
  0 siblings, 1 reply; 14+ messages in thread
From: David Woodhouse @ 2003-02-14 13:36 UTC (permalink / raw)
  To: linux-mtd

On Fri, 2003-02-14 at 13:27, Thomas Gleixner wrote:
> True, but pread/pwrite does not support two pointers.
> 
> We can use one pointer, but then you have to copy things around in userspace.

_ANYTHING_ is better than an ioctl which takes pointers to userspace
buffers. 

-- 
dwmw2

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Initialising NAND OOB with Software ECC
  2003-02-14 13:36               ` David Woodhouse
@ 2003-02-14 13:50                 ` Thomas Gleixner
  2003-02-14 13:56                   ` David Woodhouse
  0 siblings, 1 reply; 14+ messages in thread
From: Thomas Gleixner @ 2003-02-14 13:50 UTC (permalink / raw)
  To: linux-mtd

On Friday 14 February 2003 14:36, David Woodhouse wrote:
> On Fri, 2003-02-14 at 13:27, Thomas Gleixner wrote:
> > True, but pread/pwrite does not support two pointers.
> >
> > We can use one pointer, but then you have to copy things around in
> > userspace.
>
> _ANYTHING_ is better than an ioctl which takes pointers to userspace
> buffers.
True, doe you have a better idea ?

-- 
Thomas
________________________________________________________________________
linutronix - competence in embedded & realtime linux
http://www.linutronix.de
mail: tglx at linutronix.de

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Initialising NAND OOB with Software ECC
  2003-02-14 13:50                 ` Thomas Gleixner
@ 2003-02-14 13:56                   ` David Woodhouse
  2003-02-14 14:06                     ` Thomas Gleixner
  2003-02-14 14:18                     ` Phil Thompson
  0 siblings, 2 replies; 14+ messages in thread
From: David Woodhouse @ 2003-02-14 13:56 UTC (permalink / raw)
  To: linux-mtd

On Fri, 2003-02-14 at 13:50, Thomas Gleixner wrote:
> True, doe you have a better idea ?

We don't _really_ need to write both in a single atomic operation, do
we? Write to two different devices. You can always provide a library
routine to do the right thing if you really care about upsetting the
userspace programmer.

-- 
dwmw2

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Initialising NAND OOB with Software ECC
  2003-02-14 13:56                   ` David Woodhouse
@ 2003-02-14 14:06                     ` Thomas Gleixner
  2003-02-14 14:18                     ` Phil Thompson
  1 sibling, 0 replies; 14+ messages in thread
From: Thomas Gleixner @ 2003-02-14 14:06 UTC (permalink / raw)
  To: linux-mtd

On Friday 14 February 2003 14:56, David Woodhouse wrote:
> On Fri, 2003-02-14 at 13:50, Thomas Gleixner wrote:
> > True, doe you have a better idea ?
>
> We don't _really_ need to write both in a single atomic operation, do
> we? Write to two different devices. You can always provide a library
> routine to do the right thing if you really care about upsetting the
> userspace programmer.
I would like to do it in one go, as we calc ECC and put it into the supplied 
buffer, which comes either from kernel or from userspace. You should have the 
same functionality in userspace than in kernel space.
With two devices, who have two operations, which can get out of sync. Not a 
really good idea IMHO. Such a interface would be useful to test and debug 
filesystems from userspace. So you can use the same functionality as if you 
are in the kernel.

-- 
Thomas
________________________________________________________________________
linutronix - competence in embedded & realtime linux
http://www.linutronix.de
mail: tglx at linutronix.de

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Initialising NAND OOB with Software ECC
  2003-02-14 13:56                   ` David Woodhouse
  2003-02-14 14:06                     ` Thomas Gleixner
@ 2003-02-14 14:18                     ` Phil Thompson
  1 sibling, 0 replies; 14+ messages in thread
From: Phil Thompson @ 2003-02-14 14:18 UTC (permalink / raw)
  To: linux-mtd

On Friday 14 February 2003 1:56 pm, David Woodhouse wrote:
> On Fri, 2003-02-14 at 13:50, Thomas Gleixner wrote:
> > True, doe you have a better idea ?
>
> We don't _really_ need to write both in a single atomic operation, do
> we? Write to two different devices. You can always provide a library
> routine to do the right thing if you really care about upsetting the
> userspace programmer.

Given that you have to pass a pointer to the data in order to calculate the 
ECC, and you have to allow filesystems to supply some OOB data, then you 
still need 2 pointers into user space even if you separate the writes.

Phil

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2003-02-14 14:18 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2003-02-13 20:20 Initialising NAND OOB with Software ECC Phil Thompson
2003-02-14  9:05 ` Thomas Gleixner
2003-02-14  9:46   ` Phil Thompson
2003-02-14 11:00     ` Thomas Gleixner
2003-02-14 11:36       ` Phil Thompson
2003-02-14 13:02         ` Thomas Gleixner
2003-02-14 13:14           ` David Woodhouse
2003-02-14 13:27             ` Thomas Gleixner
2003-02-14 13:36               ` David Woodhouse
2003-02-14 13:50                 ` Thomas Gleixner
2003-02-14 13:56                   ` David Woodhouse
2003-02-14 14:06                     ` Thomas Gleixner
2003-02-14 14:18                     ` Phil Thompson
2003-02-14 13:25           ` Phil Thompson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox