Re: [PATCH] mtd: Nand Atmel: add On Flash BBT support

[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 (Битюцкий Артём)