Patchwork mtd/gpmi : add BBT support

login
register
mail settings
Submitter Huang Shijie
Date Jan. 30, 2012, 4:36 a.m.
Message ID <1327898174-13656-1-git-send-email-b32955@freescale.com>
Download mbox | patch
Permalink /patch/138514/
State New
Headers show

Comments

Huang Shijie - Jan. 30, 2012, 4:36 a.m.
add the BBT support to the gpmi nand driver.
Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
Wolfram Sang - Jan. 30, 2012, 9:46 a.m.
On Mon, Jan 30, 2012 at 12:36:14PM +0800, Huang Shijie wrote:
> add the BBT support to the gpmi nand driver.
> Signed-off-by: Huang Shijie <b32955@freescale.com>
> ---
>  drivers/mtd/nand/gpmi-nand/gpmi-nand.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> index 493ec2f..4fbb341 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> @@ -1493,6 +1493,7 @@ static int __devinit gpmi_nfc_init(struct gpmi_nand_data *this)
>  	chip->ecc.mode		= NAND_ECC_HW;
>  	chip->ecc.size		= 1;
>  	chip->ecc.layout	= &gpmi_hw_ecclayout;
> +	chip->bbt_options	= NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB;

You are making this default. Is there a bootloader which supports that
on MXS?
Huang Shijie - Jan. 30, 2012, 10:32 a.m.
于 2012年01月30日 17:46, Wolfram Sang 写道:
> On Mon, Jan 30, 2012 at 12:36:14PM +0800, Huang Shijie wrote:
>> add the BBT support to the gpmi nand driver.
>> Signed-off-by: Huang Shijie <b32955@freescale.com>
>> ---
>>  drivers/mtd/nand/gpmi-nand/gpmi-nand.c |    1 +
>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>> index 493ec2f..4fbb341 100644
>> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>> @@ -1493,6 +1493,7 @@ static int __devinit gpmi_nfc_init(struct gpmi_nand_data *this)
>>  	chip->ecc.mode		= NAND_ECC_HW;
>>  	chip->ecc.size		= 1;
>>  	chip->ecc.layout	= &gpmi_hw_ecclayout;
>> +	chip->bbt_options	= NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB;
> You are making this default. Is there a bootloader which supports that
> on MXS?
>
The bootloader usually is burned on /dev/mtd0, while the BBT is placed
at the end
of the NAND chip.

I tested the NAND boot mode too.

What's your concern?

Huang Shijie
Wolfram Sang - Jan. 30, 2012, 10:44 a.m.
> The bootloader usually is burned on /dev/mtd0, while the BBT is placed
> at the end
> of the NAND chip.
> 
> I tested the NAND boot mode too.
> 
> What's your concern?

Bootloaders may also need to write to NAND. So, they need to share the
same bad block information with the kernel. Currently, I am not aware of
a bootloader for mxs which support BBT without OOB. Unless this has
changed meanwhile, we shouldn't make this default, because they can't
share the same information then. To be on the safe side regardings
regressions, we shouldn't make this default as well, come to think of
it.

Regards,

   Wolfram
Huang Shijie - Jan. 30, 2012, 11:29 a.m.
>> The bootloader usually is burned on /dev/mtd0, while the BBT is placed
>> at the end
>> of the NAND chip.
>>
>> I tested the NAND boot mode too.
>>
>> What's your concern?
> Bootloaders may also need to write to NAND. So, they need to share the
Do you mean the uboot may write something to the NAND?
Could you show me some more detail cases?

> same bad block information with the kernel. Currently, I am not aware of
> a bootloader for mxs which support BBT without OOB. Unless this has
> changed meanwhile, we shouldn't make this default, because they can't
The NAND_BBT_NO_OOB makes the BBT written to the NAND with the ECC enabled.
> share the same information then. To be on the safe side regardings
The kobs-ng which burns the bootloader to the NAND will also burn the
whole BBT
to the NAND too.

So I think the bootloader and the kernel share the same BBT information.

But if the bootloader can make some block bad, the BBT information
becomes different.
Does the bootloader have the feature to make some block bad?


Br
Huang Shijie




> regressions, we shouldn't make this default as well, come to think of
> it.
>
> Regards,
>
>    Wolfram
>
Wolfram Sang - Jan. 30, 2012, 11:41 a.m.
CCing Marek, he can say more about the U-Boot side probably...

> > Bootloaders may also need to write to NAND. So, they need to share the
> Do you mean the uboot may write something to the NAND?
> Could you show me some more detail cases?

U-Boot has NAND support in mainline, for barebox it is work-in-progress.
Both can and do write to NAND (kernel, rootfs), because both have
established procedures to do so (and maybe don't want to rely on
third-party tools like kobs-ng).

> > share the same information then. To be on the safe side regardings
> The kobs-ng which burns the bootloader to the NAND will also burn the
> whole BBT
> to the NAND too.

Did it always do that? Or is a newer version needed?

> But if the bootloader can make some block bad, the BBT information
> becomes different.
> Does the bootloader have the feature to make some block bad?

Sure. If you are able to write NAND, you should be able to mark blocks
bad, too :)

Thanks,

   Wolfram
Marek Vasut - Jan. 30, 2012, 12:24 p.m.
> >> The bootloader usually is burned on /dev/mtd0, while the BBT is placed
> >> at the end
> >> of the NAND chip.
> >> 
> >> I tested the NAND boot mode too.
> >> 
> >> What's your concern?
> > 
> > Bootloaders may also need to write to NAND. So, they need to share the
> 
> Do you mean the uboot may write something to the NAND?

Yes it can, mainline for certain and ... surprisingly even the one provided by 
freescale ;-)

> Could you show me some more detail cases?

Type "help" in the command prompt and look for "nand" ;-)
> 
> > same bad block information with the kernel. Currently, I am not aware of
> > a bootloader for mxs which support BBT without OOB. Unless this has
> > changed meanwhile, we shouldn't make this default, because they can't
> 
> The NAND_BBT_NO_OOB makes the BBT written to the NAND with the ECC enabled.
> 
> > share the same information then. To be on the safe side regardings

The mainline incarnation of this should be able to work with the BBT just fine. 
You can give it a go, mx28evk (the freescale kit) is supported.
> 
> The kobs-ng which burns the bootloader to the NAND will also burn the
> whole BBT
> to the NAND too.
> 
> So I think the bootloader and the kernel share the same BBT information.
> 
> But if the bootloader can make some block bad, the BBT information
> becomes different.
> Does the bootloader have the feature to make some block bad?

Sure

M
> 
> 
> Br
> Huang Shijie
> 
> > regressions, we shouldn't make this default as well, come to think of
> > it.
> > 
> > Regards,
> > 
> >    Wolfram
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Marek Vasut - Jan. 30, 2012, 12:27 p.m.
> CCing Marek, he can say more about the U-Boot side probably...

Thanks!

> 
> > > Bootloaders may also need to write to NAND. So, they need to share the
> > 
> > Do you mean the uboot may write something to the NAND?
> > Could you show me some more detail cases?
> 
> U-Boot has NAND support in mainline, for barebox it is work-in-progress.

Why don't you rather start hacking on u-boot instead of chasing behind it all 
the time ;-)

> Both can and do write to NAND (kernel, rootfs), because both have
> established procedures to do so (and maybe don't want to rely on
> third-party tools like kobs-ng).

I think the FSL U-Boot can write stuff to NAND too. You can give that a shot 
too.
> 
> > > share the same information then. To be on the safe side regardings
> > 
> > The kobs-ng which burns the bootloader to the NAND will also burn the
> > whole BBT
> > to the NAND too.
> 
> Did it always do that? Or is a newer version needed?

I can't comment on this one, I never used it. I rather got NAND working 
properly.

> 
> > But if the bootloader can make some block bad, the BBT information
> > becomes different.
> > Does the bootloader have the feature to make some block bad?
> 
> Sure. If you are able to write NAND, you should be able to mark blocks
> bad, too :)

Not really, but you prefer to do so ;-)

Thanks!

M
> 
> Thanks,
> 
>    Wolfram
Wolfram Sang - Jan. 30, 2012, 1:12 p.m.
> > U-Boot has NAND support in mainline, for barebox it is work-in-progress.
> 
> Why don't you rather start hacking on u-boot instead of chasing behind it all 
> the time ;-)

"chasing all the time behind it" is plain wrong, you should know that. I
also fed back the DMA fix I found. Besides, this is off-topic.

> > Did it always do that? Or is a newer version needed?
> 
> I can't comment on this one, I never used it. I rather got NAND working 
> properly.

Me neither. But if so, we have another regression candidate.
Wolfram Sang - Jan. 30, 2012, 1:19 p.m.
> The mainline incarnation of this should be able to work with the BBT just fine. 
> You can give it a go, mx28evk (the freescale kit) is supported.

Did you test? I can't find any NO_OOB option in the code. Not that you
lose the BBTs...
Marek Vasut - Jan. 30, 2012, 1:30 p.m.
> > > U-Boot has NAND support in mainline, for barebox it is
> > > work-in-progress.
> > 
> > Why don't you rather start hacking on u-boot instead of chasing behind it
> > all the time ;-)
> 
> "chasing all the time behind it" is plain wrong, you should know that. I
> also fed back the DMA fix I found. Besides, this is off-topic.

Hehe, just torturing you guys a bit :)
> 
> > > Did it always do that? Or is a newer version needed?
> > 
> > I can't comment on this one, I never used it. I rather got NAND working
> > properly.
> 
> Me neither. But if so, we have another regression candidate.

What regression candidate ? The version of NAND driver in u-boot should be close 
to what's in mainline Linux kernel.
Marek Vasut - Jan. 30, 2012, 1:35 p.m.
> > The mainline incarnation of this should be able to work with the BBT just
> > fine. You can give it a go, mx28evk (the freescale kit) is supported.
> 
> Did you test? I can't find any NO_OOB option in the code. Not that you
> lose the BBTs...

I didn't. Let's also Cc Fabio here, I think he can comment on the FSL part of 
stuff better than me.

Btw. aren't we duplicating stuff here? The mx28 has some builtin badblock 
protection.
Wolfram Sang - Jan. 30, 2012, 1:41 p.m.
> Hehe, just torturing you guys a bit :)

And the gain of that is? The only outcome I currently see is that I feel
less motivated to put you on CC next time.

> > > > Did it always do that? Or is a newer version needed?
> > > 
> > > I can't comment on this one, I never used it. I rather got NAND working
> > > properly.
> > 
> > Me neither. But if so, we have another regression candidate.
> 
> What regression candidate ? The version of NAND driver in u-boot should be close 
> to what's in mainline Linux kernel.

I am not talking about U-Boot here. I was wondering if somebody who uses
the FSL deployment method would need to update kobs-ng, so a proper BBT
gets written. If so, people missing the need to update would see a
regression. Which is another reason to not make BBT usage default. But I
maybe wrong here. Huang will tell.
Marek Vasut - Jan. 30, 2012, 2:33 p.m.
> > Hehe, just torturing you guys a bit :)
> 
> And the gain of that is? The only outcome I currently see is that I feel
> less motivated to put you on CC next time.
> 
> > > > > Did it always do that? Or is a newer version needed?
> > > > 
> > > > I can't comment on this one, I never used it. I rather got NAND
> > > > working properly.
> > > 
> > > Me neither. But if so, we have another regression candidate.
> > 
> > What regression candidate ? The version of NAND driver in u-boot should
> > be close to what's in mainline Linux kernel.
> 
> I am not talking about U-Boot here. I was wondering if somebody who uses
> the FSL deployment method would need to update kobs-ng, so a proper BBT
> gets written.

... if someone actually still uses kobs-ng.

> If so, people missing the need to update would see a
> regression. Which is another reason to not make BBT usage default. But I
> maybe wrong here. Huang will tell.

Yes, either him or Fabio. This is a good point actually.
Huang Shijie - Jan. 31, 2012, 8:47 a.m.
hi,
> CCing Marek, he can say more about the U-Boot side probably...
>
>>> Bootloaders may also need to write to NAND. So, they need to share the
>> Do you mean the uboot may write something to the NAND?
>> Could you show me some more detail cases?
> U-Boot has NAND support in mainline, for barebox it is work-in-progress.
> Both can and do write to NAND (kernel, rootfs), because both have
> established procedures to do so (and maybe don't want to rely on
> third-party tools like kobs-ng).
>
ok.

What's about to add a module_param() to enable/disable the BBT for GPMI?
What's your suggestion?


Br
Huang Shijie
Wolfram Sang - Jan. 31, 2012, 9:24 a.m.
> What's about to add a module_param() to enable/disable the BBT for GPMI?
> What's your suggestion?

I'd make it board specific rather than a module parameter like most
other drivers seem to do.

Thanks,

   Wolfram
Huang Shijie - Jan. 31, 2012, 9:38 a.m.
hi,
> I'd make it board specific rather than a module parameter like most
ok. I make it board specific.


thanks

Huang Shijie
> other drivers seem to do.
>
Marek Vasut - Jan. 31, 2012, 11:37 a.m.
> > What's about to add a module_param() to enable/disable the BBT for GPMI?
> > What's your suggestion?
> 
> I'd make it board specific rather than a module parameter like most
> other drivers seem to do.
> 
> Thanks,
> 
>    Wolfram

Actually it's not board specific. You can run various bootloaders or various 
bootstrap tools on any of those boards. Though pdata seems ok for the default 
case, maybe we should have a simple way to override this?

M
Wolfram Sang - Jan. 31, 2012, 11:49 a.m.
> Actually it's not board specific. You can run various bootloaders or various 
> bootstrap tools on any of those boards. Though pdata seems ok for the default 
> case, maybe we should have a simple way to override this?

You mean you want to switch to/drop the BBT if you want to? On MX23,
this can be easily a very dangerous thing to do. What's the use case?

Regards,

   Wolfram
Marek Vasut - Jan. 31, 2012, 12:21 p.m.
> > Actually it's not board specific. You can run various bootloaders or
> > various bootstrap tools on any of those boards. Though pdata seems ok
> > for the default case, maybe we should have a simple way to override
> > this?
> 
> You mean you want to switch to/drop the BBT if you want to? On MX23,
> this can be easily a very dangerous thing to do. What's the use case?

Tools that don't support writing the BBT properly.

> 
> Regards,
> 
>    Wolfram

Patch

diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index 493ec2f..4fbb341 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -1493,6 +1493,7 @@  static int __devinit gpmi_nfc_init(struct gpmi_nand_data *this)
 	chip->ecc.mode		= NAND_ECC_HW;
 	chip->ecc.size		= 1;
 	chip->ecc.layout	= &gpmi_hw_ecclayout;
+	chip->bbt_options	= NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB;
 
 	/* Allocate a temporary DMA buffer for reading ID in the nand_scan() */
 	this->bch_geometry.payload_size = 1024;