Patchwork [1/1] MTD mtdchar: add missing initializer on raw write

login
register
mail settings
Submitter Peter Wippich
Date June 6, 2011, 1:50 p.m.
Message ID <alpine.LSU.2.00.1106061541210.26001@redbean.intranet.gw-instruments.de>
Download mbox | patch
Permalink /patch/98917/
State New
Headers show

Comments

Peter Wippich - June 6, 2011, 1:50 p.m.
On writes in MODE_RAW the mtd_oob_ops struct is not sufficiently 
initialized which may cause nandwrite to fail. With this patch
it is possible to write raw nand/oob data without additional ECC
(either for testing or when some sectors need different oob layout
e.g. bootloader) like
nandwrite  -n -r -o  /dev/mtd0 <myfile>

Signed-off-by: Peter Wippich <pewi@gw-instruments.de>
Artem Bityutskiy - June 6, 2011, 3:28 p.m.
On Mon, 2011-06-06 at 15:50 +0200, Peter Wippich wrote:
> On writes in MODE_RAW the mtd_oob_ops struct is not sufficiently 
> initialized which may cause nandwrite to fail. With this patch
> it is possible to write raw nand/oob data without additional ECC
> (either for testing or when some sectors need different oob layout
> e.g. bootloader) like
> nandwrite  -n -r -o  /dev/mtd0 <myfile>
> 
> Signed-off-by: Peter Wippich <pewi@gw-instruments.de>

Looks good, thanks. We also need to add this patch to the stable tree.

Ricard, does it solve the issue you reported here

http://lists.infradead.org/pipermail/linux-mtd/2011-March/034516.html

?

> 
> diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
> index 3f92731..797a34a 100644
> --- a/drivers/mtd/mtdchar.c
> +++ b/drivers/mtd/mtdchar.c
> @@ -321,6 +321,7 @@ static ssize_t mtd_write(struct file *file, const char __user *buf, size_t count
>                          ops.datbuf = kbuf;
>                          ops.oobbuf = NULL;
>                          ops.len = len;
> +                       ops.ooboffs = 0;
> 
>                          ret = mtd->write_oob(mtd, *ppos, &ops);
>                          retlen = ops.retlen;
>
Artem Bityutskiy - June 6, 2011, 3:33 p.m.
On Mon, 2011-06-06 at 15:50 +0200, Peter Wippich wrote:
> On writes in MODE_RAW the mtd_oob_ops struct is not sufficiently 
> initialized which may cause nandwrite to fail. With this patch
> it is possible to write raw nand/oob data without additional ECC
> (either for testing or when some sectors need different oob layout
> e.g. bootloader) like
> nandwrite  -n -r -o  /dev/mtd0 <myfile>
> 
> Signed-off-by: Peter Wippich <pewi@gw-instruments.de>

Pushed your patch to the l2-mtd-2.6.git tree, thanks. It did not apply,
but I amended it. I guess it is against some old kernel tree. Also, I've
added "Cc: stable@kernel.org" to make sure it reaches the -stable tree.

http://git.infradead.org/users/dedekind/l2-mtd-2.6.git/commit/87d5d1460c4e13636406a613f5740ef5e5cdbbfa

A "Tested-by" would be great!
Peter Wippich - June 6, 2011, 3:39 p.m.
Hi Artem,

On Mon, 6 Jun 2011, Artem Bityutskiy wrote:

> On Mon, 2011-06-06 at 15:50 +0200, Peter Wippich wrote:
>> On writes in MODE_RAW the mtd_oob_ops struct is not sufficiently
>> initialized which may cause nandwrite to fail. With this patch
>> it is possible to write raw nand/oob data without additional ECC
>> (either for testing or when some sectors need different oob layout
>> e.g. bootloader) like
>> nandwrite  -n -r -o  /dev/mtd0 <myfile>
>>
>> Signed-off-by: Peter Wippich <pewi@gw-instruments.de>
>
> Pushed your patch to the l2-mtd-2.6.git tree, thanks. It did not apply,
> but I amended it. I guess it is against some old kernel tree. Also, I've
> added "Cc: stable@kernel.org" to make sure it reaches the -stable tree.
>
> http://git.infradead.org/users/dedekind/l2-mtd-2.6.git/commit/87d5d1460c4e13636406a613f5740ef5e5cdbbfa
>
> A "Tested-by" would be great!
>
>

thanks. Thought it was again linux-next. May be a messed up something.
Next time I'll add a tested by .......

Cheers,

Peter
Artem Bityutskiy - June 6, 2011, 3:44 p.m.
On Mon, 2011-06-06 at 17:39 +0200, Peter Wippich wrote:
> Next time I'll add a tested by .......

I meant a tested-by from Ricard, as an independent person :-)
Peter Wippich - June 6, 2011, 3:49 p.m.
On Mon, 6 Jun 2011, Artem Bityutskiy wrote:

> On Mon, 2011-06-06 at 17:39 +0200, Peter Wippich wrote:
>> Next time I'll add a tested by .......
>
> I meant a tested-by from Ricard, as an independent person :-)

Yep, makes sense :-).
Ricard Wanderlof - June 7, 2011, 9:28 a.m.
On Mon, 6 Jun 2011, Artem Bityutskiy wrote:

> On Mon, 2011-06-06 at 15:50 +0200, Peter Wippich wrote:
> On writes in MODE_RAW the mtd_oob_ops struct is not sufficiently 
> initialized which may cause nandwrite to fail. With this patch
> it is possible to write raw nand/oob data without additional ECC
> (either for testing or when some sectors need different oob layout
> e.g. bootloader) like
> nandwrite  -n -r -o  /dev/mtd0 <myfile>
> 
> Signed-off-by: Peter Wippich <pewi@gw-instruments.de>

Looks good, thanks. We also need to add this patch to the stable tree.

Ricard, does it solve the issue you reported here

http://lists.infradead.org/pipermail/linux-mtd/2011-March/034516.html

Yes, it does, it seems to work beautifully. The error messages I reported 
in the post are gone, nandwrite reports success, and the resulting data in 
the flash appears to be ok. Thanks Peter for finding the root cause and 
the elegantly trivial patch.

Tested-by: Ricard Wanderlof <ricardw@axis.com>

/Ricard

> 
> diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
> index 3f92731..797a34a 100644
> --- a/drivers/mtd/mtdchar.c
> +++ b/drivers/mtd/mtdchar.c
> @@ -321,6 +321,7 @@ static ssize_t mtd_write(struct file *file, const char __user *buf, size_t count
>                          ops.datbuf = kbuf;
>                          ops.oobbuf = NULL;
>                          ops.len = len;
> +                       ops.ooboffs = 0;
>
>                          ret = mtd->write_oob(mtd, *ppos, &ops);
>                          retlen = ops.retlen;
>
Artem Bityutskiy - June 7, 2011, 9:38 a.m.
On Tue, 2011-06-07 at 11:28 +0200, Ricard Wanderlof wrote:
> Tested-by: Ricard Wanderlof <ricardw@axis.com>

Thank you!

Patch

diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
index 3f92731..797a34a 100644
--- a/drivers/mtd/mtdchar.c
+++ b/drivers/mtd/mtdchar.c
@@ -321,6 +321,7 @@  static ssize_t mtd_write(struct file *file, const char __user *buf, size_t count
                         ops.datbuf = kbuf;
                         ops.oobbuf = NULL;
                         ops.len = len;
+                       ops.ooboffs = 0;

                         ret = mtd->write_oob(mtd, *ppos, &ops);
                         retlen = ops.retlen;