Patchwork mtd: Nand Atmel: add On Flash BBT support

2009/5/27 Artem Bityutskiy <dedekind@infradead.org>:
> On Tue, 2009-05-26 at 13:55 +0200, simon polette wrote:
>> 2009/5/26 simon polette <spolette@gmail.com>:
>> > 2009/5/26 simon polette <spolette@gmail.com>:
>> >> 2009/5/26 Artem Bityutskiy <dedekind@infradead.org>:
>> >>> On Tue, 2009-05-26 at 11:40 +0200, simon polette wrote:
>> >>>> 2009/5/26 Artem Bityutskiy <dedekind@infradead.org>:
>> >>>> > On Mon, 2009-05-25 at 17:44 +0200, simon polette wrote:
>> >>>> >> +config       MTD_NAND_ATMEL_FLASH_BBT
>> >>>> >> +     bool "Use On-Flash Bad Block Table"
>> >>>> >> +     depends on MTD_NAND_ATMEL
>> >>>> >> +     help
>> >>>> >> +       This enables the On-Flash BBT, which mean that the bad blocks
>> >>>> >> +       will be scanned one time then the BBT will be stored
>> >>>> >> +       in flash, so scanning Nand flash for bad blocks will be no more
>> >>>> >> +       necessary for the next boots.
>> >>>> >> +
>> >>>> >
>> >>>> > I do not think you need a config option for this. It should be
>> >>>> > a module parameter instead.
>> >>>> >
>> >>>> > --
>> >>>> > Best regards,
>> >>>> > Artem Bityutskiy (Битюцкий Артём)
>> >>>> >
>> >>>> >
>> >>>>
>> >>>> Yes, good idea, but do you think that I can keep a config option to
>> >>>> define the default state of that param, it means doing something like
>> >>>> :
>> >>>> #ifdef CONFIG_MTD_NAND_ATMEL_FLASH_BBT
>> >>>> static int use_on_flash_bbt = 1;
>> >>>> #else
>> >>>> static int use_on_flash_bbt = 0;
>> >>>> #endif
>> >>>> module_param(use_on_flash_bbt, int, 0);
>> >>>
>> >>> I think it is generally bad idea if each nand driver will
>> >>> introduce a separate config option for this kind of stuff.
>> >>>
>> >>> You may always boot your kernel with something like
>> >>> atmel_nand.on_flash_bbt=1 in the kernel parameters.
>> >>>
>> >>> --
>> >>> Best regards,
>> >>> Artem Bityutskiy (Битюцкий Артём)
>> >>>
>> >>>
>> >>
>> >> Ok, I asked this question cause it's what have been done in the nand
>> >> diskonchip driver.
>> >> I'll send you a new patch soon.
>> >>
>> >> Best regards,
>> >>
>> >> Simon Polette
>> >> Adeneo - Adetelgroup
>> >>
>> >
>> > So here is the new patch :
>> >
>> > Signed-off-by: Simon Polette <spolette@adetelgroup.com>
>> > ---
>> >  drivers/mtd/nand/atmel_nand.c |    9 +++++++++
>> >  1 files changed, 9 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
>> > index 47a33ce..e113594 100644
>> > --- a/drivers/mtd/nand/atmel_nand.c
>> > +++ b/drivers/mtd/nand/atmel_nand.c
>> > @@ -24,6 +24,7 @@
>> >
>> >  #include <linux/slab.h>
>> >  #include <linux/module.h>
>> > +#include <linux/moduleparam.h>
>> >  #include <linux/platform_device.h>
>> >  #include <linux/mtd/mtd.h>
>> >  #include <linux/mtd/nand.h>Signed-off-by: Simon Polette
>> > <spolette@adetelgroup.com>
>> > @@ -47,6 +48,9 @@
>> >  #define no_ecc         0
>> >  #endif
>> >
>> > +static int on_flash_bbt = 0;
>> > +module_param(on_flash_bbt, int, 0);
>> > +
>> >  /* Register access macros */
>> >  #define ecc_readl(add, reg)                            \
>> >        __raw_readl(add + ATMEL_ECC_##reg)
>> > @@ -465,6 +469,11 @@ static int __init atmel_nand_probe(struct
>> > platform_device *pdev)
>> >                }
>> >        }
>> >
>> > +       if (on_flash_bbt) {
>> > +               printk("atmel_nand: Use On Flash BBT\n");
>> > +               nand_chip->options |= NAND_USE_FLASH_BBT;
>> > +       }
>> > +
>> >        /* first scan to find the device and get the page size */
>> >        if (nand_scan_ident(mtd, 1)) {
>> >                res = -ENXIO;
>> > --
>> > 1.6.0.4
>> >
>> > --
>> > Best regards,
>> >
>> > Simon Polette
>> > Adeneo - Adetelgroup
>> >
>> A line have been mistakenly inserted into the patch, probably by a
>> middle click, sorry for the inconvenience.
>>
>>
>> Signed-off-by: Simon Polette <spolette@adetelgroup.com>
>> ---
>>  drivers/mtd/nand/atmel_nand.c |    9 +++++++++
>>  1 files changed, 9 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
>> index 47a33ce..e113594 100644
>> --- a/drivers/mtd/nand/atmel_nand.c
>> +++ b/drivers/mtd/nand/atmel_nand.c
>> @@ -24,6 +24,7 @@
>>
>>  #include <linux/slab.h>
>>  #include <linux/module.h>
>> +#include <linux/moduleparam.h>
>>  #include <linux/platform_device.h>
>>  #include <linux/mtd/mtd.h>
>>  #include <linux/mtd/nand.h>
>> @@ -47,6 +48,9 @@
>>  #define no_ecc               0
>>  #endif
>>
>> +static int on_flash_bbt = 0;
>> +module_param(on_flash_bbt, int, 0);
>> +
>>  /* Register access macros */
>>  #define ecc_readl(add, reg)                          \
>>       __raw_readl(add + ATMEL_ECC_##reg)
>> @@ -465,6 +469,11 @@ static int __init atmel_nand_probe(struct
>> platform_device *pdev)
>>               }
>>       }
>>
>> +     if (on_flash_bbt) {
>> +             printk("atmel_nand: Use On Flash BBT\n");
>> +             nand_chip->options |= NAND_USE_FLASH_BBT;
>> +     }
>
> Is it please also possible to add KERN_INFO to the 'printk()'
> please. And also add it to the already existing 'printk()' in
> the driver: printk("No SmartMedia card inserted.\n");
>
> --
> Best regards,
> Artem Bityutskiy (Битюцкий Артём)
>
>
Hoping the patch is correct now :

On Wed, 2009-05-27 at 12:00 +0200, simon polette wrote:
> Hoping the patch is correct now :

Did you that test this driver with on-flash BBT? Does it
work? Did you try marking/un-marking blocks bad? I'm just
curious.

I added your patch to my l2-mtd-2.6.git.

2009/5/27 Artem Bityutskiy <dedekind@infradead.org>:
> On Wed, 2009-05-27 at 12:00 +0200, simon polette wrote:
>> Hoping the patch is correct now :
>
> Did you that test this driver with on-flash BBT? Does it
> work? Did you try marking/un-marking blocks bad? I'm just
> curious.
>
> I added your patch to my l2-mtd-2.6.git.
>
> --
> Best regards,
> Artem Bityutskiy (Битюцкий Артём)
>
>

Yes I test it on a AT91SAM9261EK board and it work find, but I didn't
try to mark blocks as bad.

Best regards.