Patchwork [U-Boot] MXS-NAND: Backport ecc.size from linux kernel

login
register
mail settings
Submitter Marek Vasut
Date Dec. 18, 2011, 5:50 p.m.
Message ID <1324230610-22050-1-git-send-email-marek.vasut@gmail.com>
Download mbox | patch
Permalink /patch/132107/
State Rejected
Headers show

Comments

Marek Vasut - Dec. 18, 2011, 5:50 p.m.
The ecc.size for mxs NAND driver is set to 1 in Linux kernel and to 512 in
U-Boot, which causes "ubi part" command malfunction due to wrong subpage size.

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
Cc: Wolfgang Denk <wd@denx.de>
Cc: Detlev Zundel <dzu@denx.de>
Cc: Stefan Roese <sr@denx.de>
Cc: Scott Wood <scottwood@freescale.com>
Cc: Veli-Pekka Peltola <veli-pekka.peltola@bluegiga.com>
---
 drivers/mtd/nand/mxs_nand.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Veli-Pekka Peltola - Dec. 19, 2011, 2:33 p.m.
Hi Marek,

On 12/18/2011 07:50 PM, Marek Vasut wrote:
> The ecc.size for mxs NAND driver is set to 1 in Linux kernel and to 512 in
> U-Boot, which causes "ubi part" command malfunction due to wrong subpage size.
[snip]

Subpage size is now reported correctly and ubifs works fine. Thanks!

Tested-by: Veli-Pekka Peltola <veli-pekka.peltola@bluegiga.com>

--
Veli-Pekka Peltola
Marek Vasut - Dec. 19, 2011, 3:05 p.m.
> Hi Marek,
> 
> On 12/18/2011 07:50 PM, Marek Vasut wrote:
> > The ecc.size for mxs NAND driver is set to 1 in Linux kernel and to 512
> > in U-Boot, which causes "ubi part" command malfunction due to wrong
> > subpage size.
> 
> [snip]
> 
> Subpage size is now reported correctly and ubifs works fine. Thanks!
> 
> Tested-by: Veli-Pekka Peltola <veli-pekka.peltola@bluegiga.com>

You're the guy doing the bluegiga mx28 board ? I think I noticed in linux 
kernel, are you for example working on mx28/device tree support too?

M
Veli-Pekka Peltola - Dec. 19, 2011, 3:30 p.m.
On 12/19/2011 05:05 PM, Marek Vasut wrote:
> You're the guy doing the bluegiga mx28 board ? I think I noticed in linux
> kernel, are you for example working on mx28/device tree support too?

Yes, I have done something but most of kernel stuff related to our board 
is done by my colleague Lauri Hintsala. Currently device tree support is 
not in our task list. I don't know if someone is working on it. Shawn 
Guo (kernel maintainer for MX28) might know.

--
Veli-Pekka Peltola
Scott Wood - Dec. 19, 2011, 8:19 p.m.
On 12/18/2011 11:50 AM, Marek Vasut wrote:
> The ecc.size for mxs NAND driver is set to 1 in Linux kernel and to 512 in
> U-Boot, which causes "ubi part" command malfunction due to wrong subpage size.
> 
> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> Cc: Wolfgang Denk <wd@denx.de>
> Cc: Detlev Zundel <dzu@denx.de>
> Cc: Stefan Roese <sr@denx.de>
> Cc: Scott Wood <scottwood@freescale.com>
> Cc: Veli-Pekka Peltola <veli-pekka.peltola@bluegiga.com>
> ---
>  drivers/mtd/nand/mxs_nand.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/mtd/nand/mxs_nand.c b/drivers/mtd/nand/mxs_nand.c
> index ce2a326..26778ac 100644
> --- a/drivers/mtd/nand/mxs_nand.c
> +++ b/drivers/mtd/nand/mxs_nand.c
> @@ -1105,7 +1105,7 @@ int board_nand_init(struct nand_chip *nand)
>  	nand->ecc.layout	= &fake_ecc_layout;
>  	nand->ecc.mode		= NAND_ECC_HW;
>  	nand->ecc.bytes		= 9;
> -	nand->ecc.size		= 512;
> +	nand->ecc.size		= 1;
>  
>  	return 0;
>  

ecc.size = 1 doesn't make sense -- this is the block size over which ecc
is calculated.

Where is this Linux driver?  I don't see mxs_nand.c in Linux.

What specifically is happening in "ubi part" with ecc.size = 512?

-Scott
Marek Vasut - Dec. 19, 2011, 8:23 p.m.
> On 12/18/2011 11:50 AM, Marek Vasut wrote:
> > The ecc.size for mxs NAND driver is set to 1 in Linux kernel and to 512
> > in U-Boot, which causes "ubi part" command malfunction due to wrong
> > subpage size.
> > 
> > Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> > Cc: Wolfgang Denk <wd@denx.de>
> > Cc: Detlev Zundel <dzu@denx.de>
> > Cc: Stefan Roese <sr@denx.de>
> > Cc: Scott Wood <scottwood@freescale.com>
> > Cc: Veli-Pekka Peltola <veli-pekka.peltola@bluegiga.com>
> > ---
> > 
> >  drivers/mtd/nand/mxs_nand.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/mxs_nand.c b/drivers/mtd/nand/mxs_nand.c
> > index ce2a326..26778ac 100644
> > --- a/drivers/mtd/nand/mxs_nand.c
> > +++ b/drivers/mtd/nand/mxs_nand.c
> > @@ -1105,7 +1105,7 @@ int board_nand_init(struct nand_chip *nand)
> > 
> >  	nand->ecc.layout	= &fake_ecc_layout;
> >  	nand->ecc.mode		= NAND_ECC_HW;
> >  	nand->ecc.bytes		= 9;
> > 
> > -	nand->ecc.size		= 512;
> > +	nand->ecc.size		= 1;
> > 
> >  	return 0;
> 
> ecc.size = 1 doesn't make sense -- this is the block size over which ecc
> is calculated.

And in a way it forces the subpage shift to be 0 ... we need to fix it in both 
later, but for this release, let's make it this way (in sync with linux).
> 
> Where is this Linux driver?  I don't see mxs_nand.c in Linux.

drivers/mtd/nand/gpmi-nand/*
> 
> What specifically is happening in "ubi part" with ecc.size = 512?

The driver doesn\t support subpage writes.
> 
> -Scott

M
Scott Wood - Dec. 19, 2011, 8:34 p.m.
On 12/19/2011 02:23 PM, Marek Vasut wrote:
>> On 12/18/2011 11:50 AM, Marek Vasut wrote:
>> What specifically is happening in "ubi part" with ecc.size = 512?
> 
> The driver doesn\t support subpage writes.

Is this meant to be a workaround for the NAND layer ignoring a driver
setting NAND_NO_SUBPAGE_WRITE?  I'd rather just fix that...

-Scott
Marek Vasut - Dec. 19, 2011, 8:56 p.m.
> On 12/19/2011 02:23 PM, Marek Vasut wrote:
> >> On 12/18/2011 11:50 AM, Marek Vasut wrote:
> >> What specifically is happening in "ubi part" with ecc.size = 512?
> > 
> > The driver doesn\t support subpage writes.
> 
> Is this meant to be a workaround for the NAND layer ignoring a driver
> setting NAND_NO_SUBPAGE_WRITE?  I'd rather just fix that...

Well it's like this in linux, it took me some time to find this out. I 
backported it, so probably.
M
Marek Vasut - Jan. 3, 2012, 9:20 p.m.
> On 12/19/2011 02:23 PM, Marek Vasut wrote:
> >> On 12/18/2011 11:50 AM, Marek Vasut wrote:
> >> What specifically is happening in "ubi part" with ecc.size = 512?
> > 
> > The driver doesn\t support subpage writes.
> 
> Is this meant to be a workaround for the NAND layer ignoring a driver
> setting NAND_NO_SUBPAGE_WRITE?  I'd rather just fix that...
> 
> -Scott

Any news here ?

M
Scott Wood - Jan. 3, 2012, 9:30 p.m.
On 01/03/2012 03:20 PM, Marek Vasut wrote:
>> On 12/19/2011 02:23 PM, Marek Vasut wrote:
>>>> On 12/18/2011 11:50 AM, Marek Vasut wrote:
>>>> What specifically is happening in "ubi part" with ecc.size = 512?
>>>
>>> The driver doesn\t support subpage writes.
>>
>> Is this meant to be a workaround for the NAND layer ignoring a driver
>> setting NAND_NO_SUBPAGE_WRITE?  I'd rather just fix that...
>>
>> -Scott
> 
> Any news here ?

I just got back from end-of-year vacation; I'll look at it soon (will
probably just remove the mask and trust the driver to not set flags that
don't make sense).  Thanks for the reminder.

-Scott
Marek Vasut - Jan. 4, 2012, 12:39 a.m.
> On 01/03/2012 03:20 PM, Marek Vasut wrote:
> >> On 12/19/2011 02:23 PM, Marek Vasut wrote:
> >>>> On 12/18/2011 11:50 AM, Marek Vasut wrote:
> >>>> What specifically is happening in "ubi part" with ecc.size = 512?
> >>> 
> >>> The driver doesn\t support subpage writes.
> >> 
> >> Is this meant to be a workaround for the NAND layer ignoring a driver
> >> setting NAND_NO_SUBPAGE_WRITE?  I'd rather just fix that...
> >> 
> >> -Scott
> > 
> > Any news here ?
> 
> I just got back from end-of-year vacation;

I hope you had good time :)

> I'll look at it soon (will
> probably just remove the mask and trust the driver to not set flags that
> don't make sense).  Thanks for the reminder.

Naw, it should be fixed in linux too! We should probably start a discussion 
there.

> 
> -Scott

Patch

diff --git a/drivers/mtd/nand/mxs_nand.c b/drivers/mtd/nand/mxs_nand.c
index ce2a326..26778ac 100644
--- a/drivers/mtd/nand/mxs_nand.c
+++ b/drivers/mtd/nand/mxs_nand.c
@@ -1105,7 +1105,7 @@  int board_nand_init(struct nand_chip *nand)
 	nand->ecc.layout	= &fake_ecc_layout;
 	nand->ecc.mode		= NAND_ECC_HW;
 	nand->ecc.bytes		= 9;
-	nand->ecc.size		= 512;
+	nand->ecc.size		= 1;
 
 	return 0;