Patchwork [v3] mtd/gpmi : add BBT support to gpmi nand driver

login
register
mail settings
Submitter Huang Shijie
Date Jan. 31, 2012, 11:11 a.m.
Message ID <1328008316-27258-1-git-send-email-b32955@freescale.com>
Download mbox | patch
Permalink /patch/138763/
State New
Headers show

Comments

Huang Shijie - Jan. 31, 2012, 11:11 a.m.
Add a new field to gpmi_nand_platform_data{}.
Make the BBT support to board specific.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c |    2 ++
 include/linux/mtd/gpmi-nand.h          |    2 ++
 2 files changed, 4 insertions(+), 0 deletions(-)
Wolfram Sang - Jan. 31, 2012, 12:10 p.m.
On Tue, Jan 31, 2012 at 07:11:56PM +0800, Huang Shijie wrote:
> Add a new field to gpmi_nand_platform_data{}.
> Make the BBT support to board specific.
> 
> Signed-off-by: Huang Shijie <b32955@freescale.com>

Reviewed-by: Wolfram Sang <w.sang@pengutronix.de>

I'd think you won't be able to mark blocks bad, though. Will send a
patch in a minute. Please test.

> ---
>  drivers/mtd/nand/gpmi-nand/gpmi-nand.c |    2 ++
>  include/linux/mtd/gpmi-nand.h          |    2 ++
>  2 files changed, 4 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..4d369df 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> @@ -1493,6 +1493,8 @@ 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;
> +	if (pdata->enable_bbt)
> +		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;
> diff --git a/include/linux/mtd/gpmi-nand.h b/include/linux/mtd/gpmi-nand.h
> index 69b6dbf..04ab366 100644
> --- a/include/linux/mtd/gpmi-nand.h
> +++ b/include/linux/mtd/gpmi-nand.h
> @@ -51,6 +51,7 @@
>   * @partitions:              An optional pointer to an array of partition
>   *                           descriptions.
>   * @partition_count:         The number of elements in the partitions array.
> + * @enable_bbt:              Enable the BBT or not.
>   */
>  struct gpmi_nand_platform_data {
>  	/* SoC hardware information. */
> @@ -64,5 +65,6 @@ struct gpmi_nand_platform_data {
>  	/* Medium information. */
>  	struct		mtd_partition *partitions;
>  	unsigned	partition_count;
> +	unsigned int	enable_bbt:1;
>  };
>  #endif
> -- 
> 1.7.0.4
> 
>
Bityutskiy, Artem - Feb. 2, 2012, 11:51 a.m.
On Tue, 2012-01-31 at 13:10 +0100, Wolfram Sang wrote:
> On Tue, Jan 31, 2012 at 07:11:56PM +0800, Huang Shijie wrote:
> > Add a new field to gpmi_nand_platform_data{}.
> > Make the BBT support to board specific.
> > 
> > Signed-off-by: Huang Shijie <b32955@freescale.com>
> 
> Reviewed-by: Wolfram Sang <w.sang@pengutronix.de>
> 
> I'd think you won't be able to mark blocks bad, though. Will send a
> patch in a minute. Please test.

Sorry, I have not read this and the previous related long threads, but
from the few first e-mails I got the impression that you are not aware
of the patch from Brian Norris where he modifies MTD BBT code so that it
will by default write bad block markers _both_ to the BBT and to OOB. It
sounds like you should hold on with this patch and wait when Brian's
patch gets in. Then you probably may just use the first version of this
patch.

The patch I referred to:
http://patchwork.ozlabs.org/patch/137154/

HTH.
Lothar Waßmann - Feb. 2, 2012, 1:29 p.m.
Hi,

Bityutskiy, Artem writes:
> On Tue, 2012-01-31 at 13:10 +0100, Wolfram Sang wrote:
> > On Tue, Jan 31, 2012 at 07:11:56PM +0800, Huang Shijie wrote:
> > > Add a new field to gpmi_nand_platform_data{}.
> > > Make the BBT support to board specific.
> > > 
> > > Signed-off-by: Huang Shijie <b32955@freescale.com>
> > 
> > Reviewed-by: Wolfram Sang <w.sang@pengutronix.de>
> > 
> > I'd think you won't be able to mark blocks bad, though. Will send a
> > patch in a minute. Please test.
> 
> Sorry, I have not read this and the previous related long threads, but
> from the few first e-mails I got the impression that you are not aware
> of the patch from Brian Norris where he modifies MTD BBT code so that it
> will by default write bad block markers _both_ to the BBT and to OOB. It
> sounds like you should hold on with this patch and wait when Brian's
> patch gets in. Then you probably may just use the first version of this
> patch.
> 
IMO it's just silly trying to write anything, even a bad block marker,
to a known bad block in NAND flash.

The bad block markers in particular bytes in the OOB area are provided
by the manufacturer to identify "Factory Bad Blocks". That and no
more!
I don't know why everybody is trying to interpret them as a general
bad block indicator.

To keep track of bad blocks you need at least one good block that can
be reliably written and read (AKA flash based BBT). The only function
of the "Bad Block Markers" is to provide an initial list of blocks
that should be listed in the BBT.


Lothar Waßmann
Ricard Wanderlof - Feb. 2, 2012, 2:01 p.m.
On Thu, 2 Feb 2012, Lothar Waßmann wrote:

> Hi,

> IMO it's just silly trying to write anything, even a bad block marker,to 
> a known bad block in NAND flash.
>
> The bad block markers in particular bytes in the OOB area are providedby 
> the manufacturer to identify "Factory Bad Blocks". That and no more! I 
> don't know why everybody is trying to interpret them as a general bad 
> block indicator.

Why not? It's just another byte within the block.

It depends on what you mean by the concept 'bad block'. In the data sheets 
manufacturers usually state that 'additional bad blocks can develop over 
time', and this basically means that a block becomes worn out so that they 
don't meet the specifications regarding data retention etc, but the block 
is not defective in the sense that there's been a destructive change in 
the silicon structure.

In contrast, factory bad blocks can be both blocks with physical defects 
preventing their use, as well as blocks which don't meet the 
specifications. In my experience with 1 GB SLC flashes, the latter type of 
'bad' dominates, so that in many cases it is possible to erase the bad 
block markers on factory bad blocks and use them just as any other block. 
Since they were marked bad from the factory it is not a wise idea and 
should definitely not be used for production but can be very handy for 
'unbricking' a device where something has gone wrong with the flash 
management and all blocks have been marked as 'bad' with no information as 
to whether they were factory bad or not.

(In fact in a test concluded by NASA the factory marked bad blocks did not 
seem to correlate with the blocks having the worst characteristics; I 
think the relevant document is 
http://trs-new.jpl.nasa.gov/dspace/bitstream/2014/40761/1/08-07.pdf but 
I'm not completely sure).

The Flash chip reports an error on a write or erase operation if the 
operation does not complete successfully within certain time constraints, 
which are grounds for marking a block as unusable, i.e. bad, but usually 
the block is worn out long before the on-chip write/erase algorithms 
report errors. Hence a block should really be marked 'bad' while it still 
is 'good', in a sense.

/Ricard
Lothar Waßmann - Feb. 2, 2012, 2:43 p.m.
Hi,

Ricard Wanderlof writes:
> 
> On Thu, 2 Feb 2012, Lothar Waßmann wrote:
> 
> > Hi,
> 
> > IMO it's just silly trying to write anything, even a bad block marker,to 
> > a known bad block in NAND flash.
> >
> > The bad block markers in particular bytes in the OOB area are providedby 
> > the manufacturer to identify "Factory Bad Blocks". That and no more! I 
> > don't know why everybody is trying to interpret them as a general bad 
> > block indicator.
> 
> Why not? It's just another byte within the block.
> 
But on a block that is known to have failed erasure or programming you
cannot rely on being able to program anything reliably. Thus it is
vain to try storing the info that the block is bad within that block
itself.

> The Flash chip reports an error on a write or erase operation if the 
> operation does not complete successfully within certain time constraints, 
> which are grounds for marking a block as unusable, i.e. bad, but usually 
> the block is worn out long before the on-chip write/erase algorithms 
> report errors. Hence a block should really be marked 'bad' while it still 
> is 'good', in a sense.
> 
How do you know that a worn out block will still retain the 'This
block is bad' information?


Lothar Waßmann

Patch

diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index 493ec2f..4d369df 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -1493,6 +1493,8 @@  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;
+	if (pdata->enable_bbt)
+		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;
diff --git a/include/linux/mtd/gpmi-nand.h b/include/linux/mtd/gpmi-nand.h
index 69b6dbf..04ab366 100644
--- a/include/linux/mtd/gpmi-nand.h
+++ b/include/linux/mtd/gpmi-nand.h
@@ -51,6 +51,7 @@ 
  * @partitions:              An optional pointer to an array of partition
  *                           descriptions.
  * @partition_count:         The number of elements in the partitions array.
+ * @enable_bbt:              Enable the BBT or not.
  */
 struct gpmi_nand_platform_data {
 	/* SoC hardware information. */
@@ -64,5 +65,6 @@  struct gpmi_nand_platform_data {
 	/* Medium information. */
 	struct		mtd_partition *partitions;
 	unsigned	partition_count;
+	unsigned int	enable_bbt:1;
 };
 #endif