mtd: nand: cleanups to BBT
Subsystems (3)
cover mtd: nand: cleanups to BBT
A few random BBT cleanups, some of which I've had sitting around for a while. There will probably be some more in the near future.
Brian Norris (4):
mtd: nand: add accessors, macros for in-memory BBT
mtd: nand: refactor chip->block_markbad interface
mtd: nand: hide in-memory BBT implementation details
mtd: nand: remove NAND_BBT_SCANEMPTY
Documentation/DocBook/mtdnand.tmpl | 2 - drivers/mtd/nand/docg4.c | 6 - drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 44 +++----- drivers/mtd/nand/nand_base.c | 113 ++++++++++--------- drivers/mtd/nand/nand_bbt.c | 194 +++++++++++++++++---------------- drivers/mtd/nand/omap2.c | 2 +- drivers/mtd/nand/sm_common.c | 9 +- drivers/mtd/onenand/onenand_bbt.c | 1 - include/linux/mtd/bbm.h | 2 - include/linux/mtd/nand.h | 2 +- 10 files changed, 185 insertions(+), 190 deletions(-)
Signature · 1 line
1.8.3.2
PATCH 1/4 mtd: nand: add accessors, macros for in-memory BBT
There is an abundance of magic numbers and complicated shifting/masking logic in the in-memory BBT code, and due to the complicated shifting/masking, we often store the block number multiplied by 2. Together, these features make the code unnecessary complex and hard to read.
This patch adds macros to represent the 00b, 01b, 10b, and 11b memory-BBT magic numbers, as well as two accessor functions for reading and marking the memory-BBT bitfield for a given block.
Signed-off-by Brian Norris <[email protected]>
--- drivers/mtd/nand/nand_bbt.c | 132 ++++++++++++++++++++++++-------------------- 1 file changed, 72 insertions(+), 60 deletions(-)
drivers/mtd/nand/nand_bbt.c +72 -60
@@ -71,6 +71,28 @@#include <linux/export.h> #include <linux/string.h> +#define BBT_BLOCK_GOOD 0x00 +#define BBT_BLOCK_WORN 0x01 +#define BBT_BLOCK_RESERVED 0x02 +#define BBT_BLOCK_FACTORY_BAD 0x03 + +#define BBT_ENTRY_MASK 0x03 +#define BBT_ENTRY_SHIFT 2 + +static inline uint8_t bbt_get_entry (struct nand_chip *chip,int block) +{ +uint8_t entry = chip->bbt[block >>BBT_ENTRY_SHIFT ]; + entry >>= (block &BBT_ENTRY_MASK ) *2 ; +return entry &BBT_ENTRY_MASK ; +} + +static inline void bbt_mark_entry (struct nand_chip *chip,int block, +uint8_t mark) +{ +uint8_t msk = (mark &BBT_ENTRY_MASK ) << ((block &BBT_ENTRY_MASK ) *2 ); + chip->bbt[block >>BBT_ENTRY_SHIFT ] |= msk; +} +static int check_pattern_no_oob (uint8_t *buf,struct nand_bbt_descr *td) {if (memcmp (buf, td->pattern, td->len))
@@ -159,7 +181,7 @@ static u32 add_marker_len(struct nand_bbt_descr *td)* @page: the starting page * @num: the number of bbt descriptors to read * @td: the bbt describtion table -* @offs: offset in the memory table +* @offs: block number offset in the table * * Read the bad block table starting from page. */
@@ -209,14 +231,16 @@ static int read_bbt(struct mtd_info *mtd, uint8_t *buf, int page, int num,/* Analyse data */ for (i =0 ; i < len; i++) {uint8_t dat = buf[i]; -for (j =0 ; j <8 ; j += bits, act +=2 ) { +for (j =0 ; j <8 ; j += bits, act++) {uint8_t tmp = (dat >> j) & msk;if (tmp == msk)continue ;if (reserved_block_code && (tmp == reserved_block_code)) {pr_info ("nand_read_bbt: reserved block at 0x%012llx\n" , - (loff_t)((offs <<2 ) + (act >>1 )) << this->bbt_erase_shift); - this->bbt[offs + (act >>3 )] |=0x2 << (act &0x06 ); + (loff_t)(offs + act) << + this->bbt_erase_shift); +bbt_mark_entry (this, offs + act, +BBT_BLOCK_RESERVED ); mtd->ecc_stats.bbtblocks++;continue ; }
@@ -225,12 +249,15 @@ static int read_bbt(struct mtd_info *mtd, uint8_t *buf, int page, int num,* move this message to pr_debug. */ pr_info ("nand_read_bbt: bad block at 0x%012llx\n" , - (loff_t)((offs <<2 ) + (act >>1 )) << this->bbt_erase_shift); + (loff_t)(offs + act) << + this->bbt_erase_shift);/* Factory marked bad or worn out? */ if (tmp ==0 ) - this->bbt[offs + (act >>3 )] |=0x3 << (act &0x06 ); +bbt_mark_entry (this, offs + act, +BBT_BLOCK_FACTORY_BAD );else - this->bbt[offs + (act >>3 )] |=0x1 << (act &0x06 ); +bbt_mark_entry (this, offs + act, +BBT_BLOCK_WORN ); mtd->ecc_stats.badblocks++; } }
@@ -265,7 +292,7 @@ static int read_abs_bbt(struct mtd_info *mtd, uint8_t *buf, struct nand_bbt_desc td, offs);if (res)return res; - offs += this->chipsize >> (this->bbt_erase_shift +2 ); + offs += this->chipsize >> this->bbt_erase_shift; } }else { res = read_bbt(mtd, buf, td->pages[0],
@@ -489,11 +516,7 @@ static int create_bbt(struct mtd_info *mtd, uint8_t *buf, }if (chip ==-1 ) { -/* -* Note that numblocks is 2 * (real numblocks) here, see i+=2 -* below as it makes shifting and masking less painful -*/ - numblocks = mtd->size >> (this->bbt_erase_shift -1 ); + numblocks = mtd->size >> this->bbt_erase_shift; startblock =0 ; from =0 ; }else {
@@ -502,16 +525,16 @@ static int create_bbt(struct mtd_info *mtd, uint8_t *buf, chip + 1, this->numchips);return -EINVAL ; } - numblocks = this->chipsize >> (this->bbt_erase_shift -1 ); + numblocks = this->chipsize >> this->bbt_erase_shift; startblock = chip * numblocks; numblocks += startblock; - from = (loff_t )startblock << (this->bbt_erase_shift -1 ); + from = (loff_t )startblock << this->bbt_erase_shift; }if (this->bbt_options &NAND_BBT_SCANLASTPAGE ) from += mtd->erasesize - (mtd->writesize * numpages); -for (i = startblock; i < numblocks;) { +for (i = startblock; i < numblocks; i++) {int ret;BUG_ON (bd->options &NAND_BBT_NO_OOB );
@@ -526,13 +549,12 @@ static int create_bbt(struct mtd_info *mtd, uint8_t *buf,return ret;if (ret) { - this->bbt[i >>3 ] |=0x03 << (i &0x6 ); +bbt_mark_entry (this, i,BBT_BLOCK_FACTORY_BAD );pr_warn ("Bad eraseblock %d at 0x%012llx\n" , - i >>1 , (unsigned long long )from); + i, (unsigned long long )from); mtd->ecc_stats.badblocks++; } - i +=2 ; from += (1 << this->bbt_erase_shift); }return 0 ;
@@ -655,9 +677,9 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf, {struct nand_chip *this = mtd->priv;struct erase_info einfo; -int i, j, res, chip =0 ; +int i, res, chip =0 ;int bits, startblock, dir, page, offs, numblocks, sft, sftmsk; -int nrchips, bbtoffs, pageoffs, ooboffs; +int nrchips, pageoffs, ooboffs;uint8_t msk[4 ];uint8_t rcode = td->reserved_block_code;size_t retlen, len =0 ;
@@ -713,10 +735,9 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,for (i =0 ; i < td->maxblocks; i++) {int block = startblock + dir * i;/* Check, if the block is bad */ -switch ((this->bbt[block >>2 ] >> - (2 * (block &0x03 ))) &0x03 ) { -case 0x01 : -case 0x03 : +switch (bbt_get_entry (this, block)) { +case BBT_BLOCK_WORN : +case BBT_BLOCK_FACTORY_BAD :continue ; } page = block <<
@@ -748,8 +769,6 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,default :return -EINVAL ; } - bbtoffs = chip * (numblocks >>2 ); - to = ((loff_t )page) << this->page_shift;/* Must we save the block contents? */
@@ -814,16 +833,12 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf, buf[ooboffs + td->veroffs] = td->version[chip];/* Walk through the memory table */ -for (i =0 ; i < numblocks;) { +for (i =0 ; i < numblocks; i++) {uint8_t dat; - dat = this->bbt[bbtoffs + (i >>2 )]; -for (j =0 ; j <4 ; j++, i++) { -int sftcnt = (i << (3 - sft)) & sftmsk; -/* Do not store the reserved bbt blocks! */ - buf[offs + (i >> sft)] &= - ~(msk[dat &0x03 ] << sftcnt); - dat >>=2 ; - } +int sftcnt = (i << (3 - sft)) & sftmsk; + dat =bbt_get_entry (this, chip * numblocks + i); +/* Do not store the reserved bbt blocks! */ + buf[offs + (i >> sft)] &= ~(msk[dat] << sftcnt); }memset (&einfo,0 ,sizeof (einfo));
@@ -1009,7 +1024,7 @@ static void mark_bbt_region(struct mtd_info *mtd, struct nand_bbt_descr *td) {struct nand_chip *this = mtd->priv;int i, j, chips, block, nrblocks, update; -uint8_t oldval, newval; +uint8_t oldval;/* Do we have a bbt per chip? */ if (td->options &NAND_BBT_PERCHIP ) {
@@ -1026,12 +1041,12 @@ static void mark_bbt_region(struct mtd_info *mtd, struct nand_bbt_descr *td)if (td->pages[i] ==-1 )continue ; block = td->pages[i] >> (this->bbt_erase_shift - this->page_shift); - block <<=1 ; - oldval = this->bbt[(block >>3 )]; - newval = oldval | (0x2 << (block &0x06 )); - this->bbt[(block >>3 )] = newval; -if ((oldval != newval) && td->reserved_block_code) -nand_update_bbt (mtd, (loff_t )block << (this->bbt_erase_shift -1 )); + oldval =bbt_get_entry (this, block); +bbt_mark_entry (this, block,BBT_BLOCK_RESERVED ); +if ((oldval !=BBT_BLOCK_RESERVED ) && + td->reserved_block_code) +nand_update_bbt (mtd, (loff_t )block << + this->bbt_erase_shift);continue ; } update =0 ;
@@ -1039,14 +1054,12 @@ static void mark_bbt_region(struct mtd_info *mtd, struct nand_bbt_descr *td) block = ((i +1 ) * nrblocks) - td->maxblocks; else block = i * nrblocks; - block <<=1 ;for (j =0 ; j < td->maxblocks; j++) { - oldval = this->bbt[(block >>3 )]; - newval = oldval | (0x2 << (block &0x06 )); - this->bbt[(block >>3 )] = newval; -if (oldval != newval) + oldval =bbt_get_entry (this, block); +bbt_mark_entry (this, block,BBT_BLOCK_RESERVED ); +if (oldval !=BBT_BLOCK_RESERVED ) update =1 ; - block +=2 ; + block++; }/* * If we want reserved blocks to be recorded to flash, and some
@@ -1054,7 +1067,8 @@ static void mark_bbt_region(struct mtd_info *mtd, struct nand_bbt_descr *td)* bbts. This should only happen once. */ if (update && td->reserved_block_code) -nand_update_bbt (mtd, (loff_t)(block -2 ) << (this->bbt_erase_shift -1 )); +nand_update_bbt (mtd, (loff_t)(block -1 ) << + this->bbt_erase_shift); } }
@@ -1356,23 +1370,21 @@ int nand_default_bbt(struct mtd_info *mtd)int nand_isbad_bbt (struct mtd_info *mtd,loff_t offs,int allowbbt) {struct nand_chip *this = mtd->priv; -int block; -uint8_t res; +int block, res; -/* Get block number * 2 */ - block = (int )(offs >> (this->bbt_erase_shift -1 )); - res = (this->bbt[block >>3 ] >> (block &0x06 )) &0x03 ; + block = (int )(offs >> this->bbt_erase_shift); + res =bbt_get_entry (this, block);pr_debug ("nand_isbad_bbt(): bbt info for offs 0x%08x: " "(block %d) 0x%02x\n" , - (unsigned int )offs, block >>1 , res); + (unsigned int )offs, block, res); -switch ((int )res) { -case 0x00 : +switch (res) { +case BBT_BLOCK_GOOD :return 0 ; -case 0x01 : +case BBT_BLOCK_WORN :return 1 ; -case 0x02 : +case BBT_BLOCK_RESERVED :return allowbbt ?0 :1 ; }return 1 ;
Signature · 1 line
1.8.3.2
Re: [PATCH 1/4] mtd: nand: add accessors, macros for in-memory BBT
Hi Brian,
Here's my attempt at reviewing this patchset, given the recent discussion about needing more MTD reviewing.
I don't have enough experience with the NAND core to say anything about this, but I noticed you're doing a bunch of related but not necessarily tied changes.
IMHO, splitting this patch further might make reviewing a lot easier, see below.
Quoted 21 lines · Brian Norris
There is an abundance of magic numbers and complicated shifting/masking logic in the in-memory BBT code, and due to the complicated shifting/masking, we often store the block number multiplied by 2. Together, these features make the code unnecessary complex and hard to read.
This patch adds macros to represent the 00b, 01b, 10b, and 11b memory-BBT magic numbers, as well as two accessor functions for reading and marking the memory-BBT bitfield for a given block.
Signed-off-by Brian Norris <[email protected]>--- drivers/mtd/nand/nand_bbt.c | 132 ++++++++++++++++++++++++-------------------- 1 file changed, 72 insertions(+), 60 deletions(-)drivers/mtd/nand/nand_bbt.c +0
-0@@ -71,6 +71,28 @@#include <linux/export.h>
drivers/mtd/nand/nand_bbt.c line 81
Quoted 10 lines
drivers/mtd/nand/nand_bbt.c +8
-0#include <linux/string.h> +#define BBT_BLOCK_GOOD 0x00 +#define BBT_BLOCK_WORN 0x01 +#define BBT_BLOCK_RESERVED 0x02 +#define BBT_BLOCK_FACTORY_BAD 0x03 + +#define BBT_ENTRY_MASK 0x03 +#define BBT_ENTRY_SHIFT 2 +
You can have one patch to change all the magic numbers into this nice macros.
[...]
drivers/mtd/nand/nand_bbt.c static int read_bbt(struct mtd_info *mtd, uint8_t *buf, int page, int num, · line 243
Quoted 13 lines
drivers/mtd/nand/nand_bbt.c +5
-3-for (j =0 ; j <8 ; j += bits, act +=2 ) { +for (j =0 ; j <8 ; j += bits, act++) {uint8_t tmp = (dat >> j) & msk;if (tmp == msk)continue ;if (reserved_block_code && (tmp == reserved_block_code)) {pr_info ("nand_read_bbt: reserved block at 0x%012llx\n" , - (loff_t)((offs <<2 ) + (act >>1 )) << this->bbt_erase_shift); - this->bbt[offs + (act >>3 )] |=0x2 << (act &0x06 ); + (loff_t)(offs + act) << + this->bbt_erase_shift); +bbt_mark_entry (this, offs + act, +BBT_BLOCK_RESERVED );
And then another patch to use the new accesors.
[...]
drivers/mtd/nand/nand_bbt.c int nand_default_bbt(struct mtd_info *mtd) · line 1362
Quoted 8 lines
int
nand_isbad_bbt(struct mtd_info *mtd, loff_t offs, int allowbbt){drivers/mtd/nand/nand_bbt.c +1
-3struct nand_chip *this = mtd->priv; -int block; -uint8_t res; +int block, res; -/* Get block number * 2 */
And then a third patch to make the block * 2 -> block change.
drivers/mtd/nand/nand_bbt.c int nand_default_bbt(struct mtd_info *mtd) · line 1382
Quoted 13 lines
drivers/mtd/nand/nand_bbt.c +4
-5- block = (int )(offs >> (this->bbt_erase_shift -1 )); - res = (this->bbt[block >>3 ] >> (block &0x06 )) &0x03 ; + block = (int )(offs >> this->bbt_erase_shift); + res =bbt_get_entry (this, block);pr_debug ("nand_isbad_bbt(): bbt info for offs 0x%08x: " "(block %d) 0x%02x\n" , - (unsigned int )offs, block >>1 , res); + (unsigned int )offs, block, res); -switch ((int )res) { -case 0x00 : +switch (res) {
Mmm.. and then if you are really paranoid (like me) you can make the uint8_t -> int type change in another patch.
Quoted 13 lines
drivers/mtd/nand/nand_bbt.c +3
-2+case BBT_BLOCK_GOOD :return 0 ; -case 0x01 : +case BBT_BLOCK_WORN :return 1 ; -case 0x02 : +case BBT_BLOCK_RESERVED :return allowbbt ?0 :1 ; }return 1 ;Signature · 1 line
1.8.3.2
Hope this is of any help!
Signature · 3 lines
Ezequiel García, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com
Re: [PATCH 1/4] mtd: nand: add accessors, macros for in-memory BBT
Hi Ezequiel,
Quoted 9 lines · Ezequiel Garcia
Hi Brian,
Here's my attempt at reviewing this patchset, given the recent discussion about needing more MTD reviewing.
I don't have enough experience with the NAND core to say anything about this, but I noticed you're doing a bunch of related but not necessarily tied changes.
IMHO, splitting this patch further might make reviewing a lot easier, see below.
At first I disagreed, but after re-reading my work, I agree in part. The original code was quite ugly, but that doesn't excuse me for not making a little effort to make it easier to review.
Quoted 21 lines · Brian Norris
Quoted 6 lines
drivers/mtd/nand/nand_bbt.c +0
-0@@ -71,6 +71,28 @@#include <linux/export.h> drivers/mtd/nand/nand_bbt.c line 81
Quoted 10 lines
drivers/mtd/nand/nand_bbt.c +8
-0#include <linux/string.h> +#define BBT_BLOCK_GOOD 0x00 +#define BBT_BLOCK_WORN 0x01 +#define BBT_BLOCK_RESERVED 0x02 +#define BBT_BLOCK_FACTORY_BAD 0x03 + +#define BBT_ENTRY_MASK 0x03 +#define BBT_ENTRY_SHIFT 2 +You can have one patch to change all the magic numbers into this nice macros.
Quoted 3 lines
[...]
And then another patch to use the new accesors.
I plan to still lump the macros + accessors together, since most of the shifting and masking ugliness is very intertwined. It's easier (IMO) to just reason about a hunk like this:
[combined hunk]
- this->bbt[offs + (act >> 3)] |= 0x2 << (act & 0x06);
+ bbt_mark_entry(this, (offs << 2) + (act >> 1), BBT_BLOCK_RESERVED);
Than the next two (as if they are split to 2 patches):
[hunk for patch 1]
- this->bbt[offs + (act >> 3)] |= 0x2 << (act & 0x06);
+ this->bbt[offs + ((act >> 1) >> BBT_ENTRY_SHIFT)] |=
+ BBT_BLOCK_RESERVED << ((act >> 1) & BBT_ENTRY_MASK);
[hunk for patch 2]
- this->bbt[offs + ((act >> 1) >> BBT_ENTRY_SHIFT)] |=
- BBT_BLOCK_RESERVED << ((act >> 1) & BBT_ENTRY_MASK);
+ bbt_mark_entry(this, (offs << 2) + (act >> 1), BBT_BLOCK_RESERVED);
But I can split again, if the next series is still hard to review.
Quoted 11 lines
[...]
drivers/mtd/nand/nand_bbt.c int nand_default_bbt(struct mtd_info *mtd) · line 1362
Quoted 8 lines
int nand_isbad_bbt(struct mtd_info *mtd, loff_t offs, int allowbbt) { struct nand_chip *this = mtd->priv;drivers/mtd/nand/nand_bbt.c +1
-3-int block; -uint8_t res; +int block, res; -/* Get block number * 2 */ And then a third patch to make the block * 2 -> block change.
Yes, this one can be kept pretty separate, and it makes the intermediate changes easier to read.
Quoted 10 lines
Quoted 10 lines
drivers/mtd/nand/nand_bbt.c +3
-3- block = (int )(offs >> (this->bbt_erase_shift -1 )); - res = (this->bbt[block >>3 ] >> (block &0x06 )) &0x03 ; + block = (int )(offs >> this->bbt_erase_shift); + res =bbt_get_entry (this, block);pr_debug ("nand_isbad_bbt(): bbt info for offs 0x%08x: " "(block %d) 0x%02x\n" , - (unsigned int )offs, block >>1 , res); + (unsigned int )offs, block, res);
Quoted 6 lines
drivers/mtd/nand/nand_bbt.c int nand_default_bbt(struct mtd_info *mtd) · line 1382
Quoted 3 lines
drivers/mtd/nand/nand_bbt.c +1
-2-switch ((int )res) { -case 0x00 : +switch (res) {Mmm.. and then if you are really paranoid (like me) you can make the
uint8_t-> int type change in another patch.
I'll consider it, but I don't consider myself quite that paranoid ;)
Quoted 4 lines
Quoted 4 lines
drivers/mtd/nand/nand_bbt.c +2
-1+case BBT_BLOCK_GOOD :return 0 ; -case 0x01 : +case BBT_BLOCK_WORN :
Quoted 8 lines
drivers/mtd/nand/nand_bbt.c int nand_default_bbt(struct mtd_info *mtd) · line 1390
Quoted 6 lines
return 1;
drivers/mtd/nand/nand_bbt.c +1
-1-case 0x02 : +case BBT_BLOCK_RESERVED :return allowbbt ?0 :1 ; }return 1 ;Hope this is of any help!
Yes, thanks for the review! I'll send a split v2 series, and I hope it will be more reviewable.
Brian
PATCH 2/4 mtd: nand: refactor chip->block_markbad interface
In reply to Brian NorrisThe chip->block_markbad pointer should really only be responsible for writing a bad block marker for new bad blocks. It should not take care of BBT-related functionality, nor should it handle bookkeeping of bad block stats.
This patch refactors the 3 users of the block_markbad interface (plus the default nand_base implementation) so that the common code is kept in nand_block_markbad_lowlevel(). It removes some inconsistencies between the various implementations and should allow for more centralized improvements in the future.
Because gpmi-nand no longer needs the nand_update_bbt() function, let's stop exporting it as well.
Signed-off-by Brian Norris <[email protected]>
Cc Huang Shijie <[email protected]>
--- drivers/mtd/nand/docg4.c | 6 --- drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 44 +++++++---------- drivers/mtd/nand/nand_base.c | 87 ++++++++++++++++++++-------------- drivers/mtd/nand/nand_bbt.c | 1 - drivers/mtd/nand/sm_common.c | 9 ++-- 5 files changed, 72 insertions(+), 75 deletions(-)
drivers/mtd/nand/docg4.c +0 -6
@@ -1093,7 +1093,6 @@ static int docg4_block_markbad(struct mtd_info *mtd, loff_t ofs)struct nand_chip *nand = mtd->priv;struct docg4_priv *doc = nand->priv;struct nand_bbt_descr *bbtd = nand->badblock_pattern; -int block = (int )(ofs >> nand->bbt_erase_shift);int page = (int )(ofs >> nand->page_shift);uint32_t g4_addr =mtd_to_docg4_address (page,0 );
@@ -1108,9 +1107,6 @@ static int docg4_block_markbad(struct mtd_info *mtd, loff_t ofs)if (buf ==NULL )return -ENOMEM ; -/* update bbt in memory */ - nand->bbt[block /4 ] |=0x01 << ((block &0x03 ) *2 ); -/* write bit-wise negation of pattern to oob buffer */ memset (nand->oob_poi,0xff , mtd->oobsize);for (i =0 ; i < bbtd->len; i++)
@@ -1120,8 +1116,6 @@ static int docg4_block_markbad(struct mtd_info *mtd, loff_t ofs)write_page_prologue (mtd, g4_addr);docg4_write_page (mtd, nand, buf,1 ); ret =pageprog (mtd); -if (!ret) - mtd->ecc_stats.badblocks++;kfree (buf);
drivers/mtd/nand/gpmi-nand/gpmi-nand.c +16 -28
59 lines not on this page · raw message
drivers/mtd/nand/nand_base.c +52 -35
124 lines not on this page · raw message
drivers/mtd/nand/nand_bbt.c +0 -1
@@ -1392,4 +1392,3 @@ int nand_isbad_bbt(struct mtd_info *mtd, loff_t offs, int allowbbt)EXPORT_SYMBOL (nand_scan_bbt);EXPORT_SYMBOL (nand_default_bbt); -EXPORT_SYMBOL_GPL (nand_update_bbt);
drivers/mtd/nand/sm_common.c +4 -5
@@ -42,7 +42,7 @@ static int sm_block_markbad(struct mtd_info *mtd, loff_t ofs) {struct mtd_oob_ops ops;struct sm_oob oob; -int ret, error =0 ; +int ret;memset (&oob,-1 ,SM_OOB_SIZE ); oob.block_status =0x0F ;
@@ -61,11 +61,10 @@ static int sm_block_markbad(struct mtd_info *mtd, loff_t ofs)printk (KERN_NOTICE "sm_common: can't mark sector at %i as bad\n" , (int )ofs); - error = -EIO ; - }else - mtd->ecc_stats.badblocks++; +return -EIO ; + } -return error; +return 0 ; }static struct nand_flash_dev nand_smartmedia_flash_ids[] = {
Signature · 1 line
1.8.3.2
Re: [PATCH 2/4] mtd: nand: refactor chip->block_markbad interface
Quoted 4 lines · Brian Norris
The
chip->block_markbadpointer should really only be responsible for writing a bad block marker for new bad blocks. It should not take care of BBT-related functionality, nor should it handle bookkeeping of bad block stats.
yes.
the gpmi driver should not do the BBT-related jobs.
Quoted 8 lines
This patch refactors the 3 users of the
block_markbadinterface (plus the defaultnand_baseimplementation) so that the common code is kept innand_block_markbad_lowlevel(). It removes some inconsistencies between the various implementations and should allow for more centralized improvements in the future.Because gpmi-nand no longer needs the
nand_update_bbt()function, let's stop exporting it as well.
for the gpmi-nand part:
Acked-by Huang Shijie <[email protected]>
thanks
Huang Shijie
PATCH 3/4 mtd: nand: hide in-memory BBT implementation details
In reply to Brian Norrisnand_base.c shouldn't have to know the implementation details of nand_bbt's in-memory BBT. Specifically, nand_base shouldn't perform the bit masking and shifting to isolate a BBT entry.
Instead, just move some of the BBT code into a new nand_markbad_bbt() interface. This interface allows external users (i.e., nand_base) to mark a single block as bad in the BBT. Then nand_bbt will take care of modifying the in-memory BBT and updating the flash-based BBT (if applicable).
Signed-off-by Brian Norris <[email protected]>
--- drivers/mtd/nand/nand_base.c | 32 +++++++++++--------------------- drivers/mtd/nand/nand_bbt.c | 28 ++++++++++++++++++++++++++-- include/linux/mtd/nand.h | 2 +- 3 files changed, 38 insertions(+), 24 deletions(-)
drivers/mtd/nand/nand_base.c +11 -21
@@ -374,22 +374,20 @@ static int nand_default_block_markbad(struct mtd_info *mtd, loff_t ofs)* block table(s) and/or marker(s)). We only allow the hardware driver to * specify how to write bad block markers to OOB (chip->block_markbad). * -* We try operations in the following order, according to our bbt_options -* (NAND_BBT_NO_OOB_BBM and NAND_BBT_USE_FLASH): +* We try operations in the following order: * (1) erase the affected block, to allow OOB marker to be written cleanly -* (2) update in-memory BBT -* (3) write bad block marker to OOB area of affected block -* (4) update flash-based BBT -* Note that we retain the first error encountered in (3) or (4), finish the +* (2) write bad block marker to OOB area of affected block (unless flag +* NAND_BBT_NO_OOB_BBM is present) +* (3) update the BBT +* Note that we retain the first error encountered in (2) or (3), finish the * procedures, and dump the error in the end. */ static int nand_block_markbad_lowlevel (struct mtd_info *mtd,loff_t ofs) {struct nand_chip *chip = mtd->priv; -int block, res, ret =0 ; -int write_oob = !(chip->bbt_options &NAND_BBT_NO_OOB_BBM ); +int res, ret =0 ; -if (write_oob) { +if (!(chip->bbt_options &NAND_BBT_NO_OOB_BBM )) {struct erase_info einfo;/* Attempt erase before marking OOB */
@@ -398,24 +396,16 @@ static int nand_block_markbad_lowlevel(struct mtd_info *mtd, loff_t ofs) einfo.addr = ofs; einfo.len =1 << chip->phys_erase_shift;nand_erase_nand (mtd, &einfo,0 ); - } - -/* Get block number */ - block = (int )(ofs >> chip->bbt_erase_shift); -/* Mark block bad in memory-based BBT */ -if (chip->bbt) - chip->bbt[block >>2 ] |=0x01 << ((block &0x03 ) <<1 ); -/* Write bad block marker to OOB */ -if (write_oob) { +/* Write bad block marker to OOB */ nand_get_device (mtd,FL_WRITING ); ret = chip->block_markbad (mtd, ofs);nand_release_device (mtd); } -/* Update flash-based bad block table */ -if (chip->bbt_options &NAND_BBT_USE_FLASH ) { - res =nand_update_bbt (mtd, ofs); +/* Mark block bad in BBT */ +if (chip->bbt) { + res =nand_markbad_bbt (mtd, ofs);if (!ret) ret = res; }
drivers/mtd/nand/nand_bbt.c +26 -2
@@ -79,6 +79,8 @@#define BBT_ENTRY_MASK 0x03#define BBT_ENTRY_SHIFT 2 +static int nand_update_bbt (struct mtd_info *mtd,loff_t offs); +static inline uint8_t bbt_get_entry (struct nand_chip *chip,int block) {uint8_t entry = chip->bbt[block >>BBT_ENTRY_SHIFT ];
@@ -1194,13 +1196,13 @@ int nand_scan_bbt(struct mtd_info *mtd, struct nand_bbt_descr *bd) }/** -* nand_update_bbt - [NAND Interface] update bad block table(s) +* nand_update_bbt - update bad block table(s) * @mtd: MTD device structure * @offs: the offset of the newly marked block * * The function updates the bad block table(s). */ -int nand_update_bbt (struct mtd_info *mtd,loff_t offs) +static int nand_update_bbt (struct mtd_info *mtd,loff_t offs) {struct nand_chip *this = mtd->priv;int len, res =0 ;
@@ -1390,5 +1392,27 @@ int nand_isbad_bbt(struct mtd_info *mtd, loff_t offs, int allowbbt)return 1 ; } +/** +* nand_markbad_bbt - [NAND Interface] Mark a block bad in the BBT +* @mtd: MTD device structure +* @offs: offset of the bad block +*/ +int nand_markbad_bbt (struct mtd_info *mtd,loff_t offs) +{ +struct nand_chip *this = mtd->priv; +int block, ret =0 ; + + block = (int )(offs >> this->bbt_erase_shift); + +/* Mark bad block in memory */ +bbt_mark_entry (this, block,BBT_BLOCK_WORN ); + +/* Update flash-based bad block table */ +if (this->bbt_options &NAND_BBT_USE_FLASH ) + ret =nand_update_bbt (mtd, offs); + +return ret; +} +EXPORT_SYMBOL (nand_scan_bbt);EXPORT_SYMBOL (nand_default_bbt);
include/linux/mtd/nand.h +1 -1
@@ -628,8 +628,8 @@ extern struct nand_flash_dev nand_flash_ids[];extern struct nand_manufacturers nand_manuf_ids[];extern int nand_scan_bbt (struct mtd_info *mtd,struct nand_bbt_descr *bd); -extern int nand_update_bbt (struct mtd_info *mtd,loff_t offs);extern int nand_default_bbt (struct mtd_info *mtd); +extern int nand_markbad_bbt (struct mtd_info *mtd,loff_t offs);extern int nand_isbad_bbt (struct mtd_info *mtd,loff_t offs,int allowbbt);extern int nand_erase_nand (struct mtd_info *mtd,struct erase_info *instr,int allowbbt);
Signature · 1 line
1.8.3.2
PATCH 4/4 mtd: nand: remove NAND_BBT_SCANEMPTY
In reply to Brian NorrisNAND_BBT_SCANEMPTY is a strange, badly-supported option with omap as its single remaining user.
NAND_BBT_SCANEMPTY was likely used by accident in omap2[1]. And anyway, omap2 doesn't scan the chip for bad blocks (courtesy of NAND_SKIP_BBTSCAN), and so its use of this option is irrelevant.
This patch drops the NAND_BBT_SCANEMPTY option.
[1] http://lists.infradead.org/pipermail/linux-mtd/2012-July/042902.html
Signed-off-by Brian Norris <[email protected]>
Cc Ivan Djelic <[email protected]>
--- Documentation/DocBook/mtdnand.tmpl | 2 -- drivers/mtd/nand/nand_bbt.c | 33 +++++---------------------------- drivers/mtd/nand/omap2.c | 2 +- drivers/mtd/onenand/onenand_bbt.c | 1 - include/linux/mtd/bbm.h | 2 -- 5 files changed, 6 insertions(+), 34 deletions(-)
Documentation/DocBook/mtdnand.tmpl +0 -2
@@ -1224,8 +1224,6 @@ in this page</entry> #define NAND_BBT_CREATE 0x00000200 /* Search good / bad pattern through all pages of a block */ #define NAND_BBT_SCANALLPAGES 0x00000400 -/* Scan block empty during good / bad block scan */ -#define NAND_BBT_SCANEMPTY 0x00000800 /* Write bbt if neccecary */ #define NAND_BBT_WRITE 0x00001000 /* Read and write back block contents when writing bbt */
drivers/mtd/nand/nand_bbt.c +5 -28
@@ -110,33 +110,17 @@ static int check_pattern_no_oob(uint8_t *buf, struct nand_bbt_descr *td)* @td: search pattern descriptor * * Check for a pattern at the given place. Used to search bad block tables and -* good / bad block identifiers. If the SCAN_EMPTY option is set then check, if -* all bytes except the pattern area contain 0xff. +* good / bad block identifiers. */ static int check_pattern (uint8_t *buf,int len,int paglen,struct nand_bbt_descr *td) { -int end =0 ; -uint8_t *p = buf; -if (td->options &NAND_BBT_NO_OOB )return check_pattern_no_oob (buf, td); - end = paglen + td->offs; -if (td->options &NAND_BBT_SCANEMPTY ) -if (memchr_inv (p,0xff , end)) -return -1 ; - p += end; -/* Compare the pattern */ -if (memcmp (p, td->pattern, td->len)) +if (memcmp (buf + paglen + td->offs, td->pattern, td->len))return -1 ; -if (td->options &NAND_BBT_SCANEMPTY ) { - p += td->len; - end += td->len; -if (memchr_inv (p,0xff , len - end)) -return -1 ; - }return 0 ; }
@@ -507,15 +491,9 @@ static int create_bbt(struct mtd_info *mtd, uint8_t *buf,else numpages =1 ; -if (!(bd->options &NAND_BBT_SCANEMPTY )) { -/* We need only read few bytes from the OOB area */ - scanlen =0 ; - readlen = bd->len; - }else { -/* Full page content should be read */ - scanlen = mtd->writesize + mtd->oobsize; - readlen = numpages * mtd->writesize; - } +/* We need only read few bytes from the OOB area */ + scanlen =0 ; + readlen = bd->len;if (chip ==-1 ) { numblocks = mtd->size >> this->bbt_erase_shift;
@@ -882,7 +860,6 @@ static inline int nand_memory_bbt(struct mtd_info *mtd, struct nand_bbt_descr *b {struct nand_chip *this = mtd->priv; - bd->options &= ~NAND_BBT_SCANEMPTY ;return create_bbt (mtd, this->buffers->databuf, bd,-1 ); }
drivers/mtd/nand/omap2.c +1 -1
@@ -159,7 +159,7 @@ static struct nand_ecclayout omap_oobinfo;*/ static uint8_t scan_ff_pattern[] = {0xff };static struct nand_bbt_descr bb_descrip_flashbased = { - .options =NAND_BBT_SCANEMPTY |NAND_BBT_SCANALLPAGES , + .options =NAND_BBT_SCANALLPAGES , .offs =0 , .len =1 , .pattern = scan_ff_pattern,
drivers/mtd/onenand/onenand_bbt.c +0 -1
@@ -133,7 +133,6 @@ static inline int onenand_memory_bbt (struct mtd_info *mtd, struct nand_bbt_desc {struct onenand_chip *this = mtd->priv; - bd->options &= ~NAND_BBT_SCANEMPTY ;return create_bbt (mtd, this->page_buf, bd,-1 ); }
include/linux/mtd/bbm.h +0 -2
@@ -93,8 +93,6 @@ struct nand_bbt_descr {#define NAND_BBT_CREATE_EMPTY 0x00000400/* Search good / bad pattern through all pages of a block */ #define NAND_BBT_SCANALLPAGES 0x00000800 -/* Scan block empty during good / bad block scan */ -#define NAND_BBT_SCANEMPTY 0x00001000/* Write bbt if neccecary */ #define NAND_BBT_WRITE 0x00002000/* Read and write back block contents when writing bbt */
Signature · 1 line
1.8.3.2
v2 cover mtd: nand: cleanups to BBT
In reply to Brian NorrisHi all,
This is v2 of my NAND BBT cleanup. The net diff is exactly the same as v1, but it splits up a few logical changes.
v1 -> v2:
Split up first patch into 3 patches, according to Ezequiel's comments. Hopefully this makes it easier to read and review.
Brian Norris (6):
mtd: nand: add accessors, macros for in-memory BBT
mtd: nand: remove multiplied-by-2 block logic
mtd: nand: eliminate cast
mtd: nand: refactor chip->block_markbad interface
mtd: nand: hide in-memory BBT implementation details
mtd: nand: remove NAND_BBT_SCANEMPTY
Documentation/DocBook/mtdnand.tmpl | 2 - drivers/mtd/nand/docg4.c | 6 - drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 44 +++----- drivers/mtd/nand/nand_base.c | 113 ++++++++++--------- drivers/mtd/nand/nand_bbt.c | 194 +++++++++++++++++--------------- drivers/mtd/nand/omap2.c | 2 +- drivers/mtd/nand/sm_common.c | 9 +- drivers/mtd/onenand/onenand_bbt.c | 1 - include/linux/mtd/bbm.h | 2 - include/linux/mtd/nand.h | 2 +- 10 files changed, 185 insertions(+), 190 deletions(-)
Signature · 1 line
1.7.9.5
PATCH v2 1/6 mtd: nand: add accessors, macros for in-memory BBT
There is an abundance of magic numbers and complicated shifting/masking logic in the in-memory BBT code which makes the code unnecessary complex and hard to read.
This patch adds macros to represent the 00b, 01b, 10b, and 11b memory-BBT magic numbers, as well as two accessor functions for reading and marking the memory-BBT bitfield for a given block.
Signed-off-by Brian Norris <[email protected]>
--- drivers/mtd/nand/nand_bbt.c | 70 +++++++++++++++++++++++++++++-------------- 1 file changed, 48 insertions(+), 22 deletions(-)
drivers/mtd/nand/nand_bbt.c +48 -22
@@ -71,6 +71,28 @@#include <linux/export.h> #include <linux/string.h> +#define BBT_BLOCK_GOOD 0x00 +#define BBT_BLOCK_WORN 0x01 +#define BBT_BLOCK_RESERVED 0x02 +#define BBT_BLOCK_FACTORY_BAD 0x03 + +#define BBT_ENTRY_MASK 0x03 +#define BBT_ENTRY_SHIFT 2 + +static inline uint8_t bbt_get_entry (struct nand_chip *chip,int block) +{ +uint8_t entry = chip->bbt[block >>BBT_ENTRY_SHIFT ]; + entry >>= (block &BBT_ENTRY_MASK ) *2 ; +return entry &BBT_ENTRY_MASK ; +} + +static inline void bbt_mark_entry (struct nand_chip *chip,int block, +uint8_t mark) +{ +uint8_t msk = (mark &BBT_ENTRY_MASK ) << ((block &BBT_ENTRY_MASK ) *2 ); + chip->bbt[block >>BBT_ENTRY_SHIFT ] |= msk; +} +static int check_pattern_no_oob (uint8_t *buf,struct nand_bbt_descr *td) {if (memcmp (buf, td->pattern, td->len))
@@ -216,7 +238,9 @@ static int read_bbt(struct mtd_info *mtd, uint8_t *buf, int page, int num,if (reserved_block_code && (tmp == reserved_block_code)) {pr_info ("nand_read_bbt: reserved block at 0x%012llx\n" , (loff_t)((offs <<2 ) + (act >>1 )) << this->bbt_erase_shift); - this->bbt[offs + (act >>3 )] |=0x2 << (act &0x06 ); +bbt_mark_entry (this, (offs <<2 ) + + (act >>1 ), +BBT_BLOCK_RESERVED ); mtd->ecc_stats.bbtblocks++;continue ; }
@@ -228,9 +252,13 @@ static int read_bbt(struct mtd_info *mtd, uint8_t *buf, int page, int num, (loff_t)((offs << 2) + (act >> 1)) << this->bbt_erase_shift);/* Factory marked bad or worn out? */ if (tmp ==0 ) - this->bbt[offs + (act >>3 )] |=0x3 << (act &0x06 ); +bbt_mark_entry (this, (offs <<2 ) + + (act >>1 ), +BBT_BLOCK_FACTORY_BAD );else - this->bbt[offs + (act >>3 )] |=0x1 << (act &0x06 ); +bbt_mark_entry (this, (offs <<2 ) + + (act >>1 ), +BBT_BLOCK_WORN ); mtd->ecc_stats.badblocks++; } }
@@ -526,7 +554,7 @@ static int create_bbt(struct mtd_info *mtd, uint8_t *buf,return ret;if (ret) { - this->bbt[i >>3 ] |=0x03 << (i &0x6 ); +bbt_mark_entry (this, i >>1 ,BBT_BLOCK_FACTORY_BAD );pr_warn ("Bad eraseblock %d at 0x%012llx\n" , i >>1 , (unsigned long long )from); mtd->ecc_stats.badblocks++;
@@ -713,10 +741,9 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,for (i =0 ; i < td->maxblocks; i++) {int block = startblock + dir * i;/* Check, if the block is bad */ -switch ((this->bbt[block >>2 ] >> - (2 * (block &0x03 ))) &0x03 ) { -case 0x01 : -case 0x03 : +switch (bbt_get_entry (this, block)) { +case BBT_BLOCK_WORN : +case BBT_BLOCK_FACTORY_BAD :continue ; } page = block <<
@@ -816,7 +843,7 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,/* Walk through the memory table */ for (i =0 ; i < numblocks;) {uint8_t dat; - dat = this->bbt[bbtoffs + (i >>2 )]; + dat =bbt_get_entry (this, (bbtoffs <<2 ) + i);for (j =0 ; j <4 ; j++, i++) {int sftcnt = (i << (3 - sft)) & sftmsk;/* Do not store the reserved bbt blocks! */
@@ -1009,7 +1036,7 @@ static void mark_bbt_region(struct mtd_info *mtd, struct nand_bbt_descr *td) {struct nand_chip *this = mtd->priv;int i, j, chips, block, nrblocks, update; -uint8_t oldval, newval; +uint8_t oldval;/* Do we have a bbt per chip? */ if (td->options &NAND_BBT_PERCHIP ) {
@@ -1027,10 +1054,10 @@ static void mark_bbt_region(struct mtd_info *mtd, struct nand_bbt_descr *td)continue ; block = td->pages[i] >> (this->bbt_erase_shift - this->page_shift); block <<=1 ; - oldval = this->bbt[(block >>3 )]; - newval = oldval | (0x2 << (block &0x06 )); - this->bbt[(block >>3 )] = newval; -if ((oldval != newval) && td->reserved_block_code) + oldval =bbt_get_entry (this, block >>1 ); +bbt_mark_entry (this, block >>1 ,BBT_BLOCK_RESERVED ); +if ((oldval !=BBT_BLOCK_RESERVED ) && + td->reserved_block_code)nand_update_bbt (mtd, (loff_t )block << (this->bbt_erase_shift -1 ));continue ; }
@@ -1041,10 +1068,9 @@ static void mark_bbt_region(struct mtd_info *mtd, struct nand_bbt_descr *td) block = i * nrblocks; block <<=1 ;for (j =0 ; j < td->maxblocks; j++) { - oldval = this->bbt[(block >>3 )]; - newval = oldval | (0x2 << (block &0x06 )); - this->bbt[(block >>3 )] = newval; -if (oldval != newval) + oldval =bbt_get_entry (this, block >>1 ); +bbt_mark_entry (this, block >>1 ,BBT_BLOCK_RESERVED ); +if (oldval !=BBT_BLOCK_RESERVED ) update =1 ; block +=2 ; }
@@ -1361,18 +1387,18 @@ int nand_isbad_bbt(struct mtd_info *mtd, loff_t offs, int allowbbt)/* Get block number * 2 */ block = (int )(offs >> (this->bbt_erase_shift -1 )); - res = (this->bbt[block >>3 ] >> (block &0x06 )) &0x03 ; + res =bbt_get_entry (this, block >>1 );pr_debug ("nand_isbad_bbt(): bbt info for offs 0x%08x: " "(block %d) 0x%02x\n" , (unsigned int )offs, block >>1 , res);switch ((int )res) { -case 0x00 : +case BBT_BLOCK_GOOD :return 0 ; -case 0x01 : +case BBT_BLOCK_WORN :return 1 ; -case 0x02 : +case BBT_BLOCK_RESERVED :return allowbbt ?0 :1 ; }return 1 ;
Signature · 1 line
1.7.9.5
PATCH v2 2/6 mtd: nand: remove multiplied-by-2 block logic
In reply to Brian NorrisThe previous patch makes the following comment obsolete:
/* * Note that numblocks is 2 * (real numblocks) here, see i+=2 * below as it makes shifting and masking less painful */
I don't think it ever could have been "less painful" to have to shift an extra bit (or 2, or 3) at various points in nand_bbt.c (and even outside, since we leak our in-memory format). But now it is certainly more painful, since we have nice macros and functions to retrieve the relevant portions of the BBT.
This patch removes any points where the block number is doubled/halved/otherwise-shifted, instead representing the block number in its most natural form: as the actual block number.
Signed-off-by Brian Norris <[email protected]>
--- drivers/mtd/nand/nand_bbt.c | 83 ++++++++++++++++++------------------------- 1 file changed, 35 insertions(+), 48 deletions(-)
drivers/mtd/nand/nand_bbt.c +35 -48
@@ -181,7 +181,7 @@ static u32 add_marker_len(struct nand_bbt_descr *td)* @page: the starting page * @num: the number of bbt descriptors to read * @td: the bbt describtion table -* @offs: offset in the memory table +* @offs: block number offset in the table * * Read the bad block table starting from page. */
@@ -231,15 +231,15 @@ static int read_bbt(struct mtd_info *mtd, uint8_t *buf, int page, int num,/* Analyse data */ for (i =0 ; i < len; i++) {uint8_t dat = buf[i]; -for (j =0 ; j <8 ; j += bits, act +=2 ) { +for (j =0 ; j <8 ; j += bits, act++) {uint8_t tmp = (dat >> j) & msk;if (tmp == msk)continue ;if (reserved_block_code && (tmp == reserved_block_code)) {pr_info ("nand_read_bbt: reserved block at 0x%012llx\n" , - (loff_t)((offs <<2 ) + (act >>1 )) << this->bbt_erase_shift); -bbt_mark_entry (this, (offs <<2 ) + - (act >>1 ), + (loff_t)(offs + act) << + this->bbt_erase_shift); +bbt_mark_entry (this, offs + act,BBT_BLOCK_RESERVED ); mtd->ecc_stats.bbtblocks++;continue ;
@@ -249,15 +249,14 @@ static int read_bbt(struct mtd_info *mtd, uint8_t *buf, int page, int num,* move this message to pr_debug. */ pr_info ("nand_read_bbt: bad block at 0x%012llx\n" , - (loff_t)((offs <<2 ) + (act >>1 )) << this->bbt_erase_shift); + (loff_t)(offs + act) << + this->bbt_erase_shift);/* Factory marked bad or worn out? */ if (tmp ==0 ) -bbt_mark_entry (this, (offs <<2 ) + - (act >>1 ), +bbt_mark_entry (this, offs + act,BBT_BLOCK_FACTORY_BAD );else -bbt_mark_entry (this, (offs <<2 ) + - (act >>1 ), +bbt_mark_entry (this, offs + act,BBT_BLOCK_WORN ); mtd->ecc_stats.badblocks++; }
@@ -293,7 +292,7 @@ static int read_abs_bbt(struct mtd_info *mtd, uint8_t *buf, struct nand_bbt_desc td, offs);if (res)return res; - offs += this->chipsize >> (this->bbt_erase_shift +2 ); + offs += this->chipsize >> this->bbt_erase_shift; } }else { res = read_bbt(mtd, buf, td->pages[0],
@@ -517,11 +516,7 @@ static int create_bbt(struct mtd_info *mtd, uint8_t *buf, }if (chip ==-1 ) { -/* -* Note that numblocks is 2 * (real numblocks) here, see i+=2 -* below as it makes shifting and masking less painful -*/ - numblocks = mtd->size >> (this->bbt_erase_shift -1 ); + numblocks = mtd->size >> this->bbt_erase_shift; startblock =0 ; from =0 ; }else {
@@ -530,16 +525,16 @@ static int create_bbt(struct mtd_info *mtd, uint8_t *buf, chip + 1, this->numchips);return -EINVAL ; } - numblocks = this->chipsize >> (this->bbt_erase_shift -1 ); + numblocks = this->chipsize >> this->bbt_erase_shift; startblock = chip * numblocks; numblocks += startblock; - from = (loff_t )startblock << (this->bbt_erase_shift -1 ); + from = (loff_t )startblock << this->bbt_erase_shift; }if (this->bbt_options &NAND_BBT_SCANLASTPAGE ) from += mtd->erasesize - (mtd->writesize * numpages); -for (i = startblock; i < numblocks;) { +for (i = startblock; i < numblocks; i++) {int ret;BUG_ON (bd->options &NAND_BBT_NO_OOB );
@@ -554,13 +549,12 @@ static int create_bbt(struct mtd_info *mtd, uint8_t *buf,return ret;if (ret) { -bbt_mark_entry (this, i >>1 ,BBT_BLOCK_FACTORY_BAD ); +bbt_mark_entry (this, i,BBT_BLOCK_FACTORY_BAD );pr_warn ("Bad eraseblock %d at 0x%012llx\n" , - i >>1 , (unsigned long long )from); + i, (unsigned long long )from); mtd->ecc_stats.badblocks++; } - i +=2 ; from += (1 << this->bbt_erase_shift); }return 0 ;
@@ -683,9 +677,9 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf, {struct nand_chip *this = mtd->priv;struct erase_info einfo; -int i, j, res, chip =0 ; +int i, res, chip =0 ;int bits, startblock, dir, page, offs, numblocks, sft, sftmsk; -int nrchips, bbtoffs, pageoffs, ooboffs; +int nrchips, pageoffs, ooboffs;uint8_t msk[4 ];uint8_t rcode = td->reserved_block_code;size_t retlen, len =0 ;
@@ -775,8 +769,6 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,default :return -EINVAL ; } - bbtoffs = chip * (numblocks >>2 ); - to = ((loff_t )page) << this->page_shift;/* Must we save the block contents? */
@@ -841,16 +833,12 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf, buf[ooboffs + td->veroffs] = td->version[chip];/* Walk through the memory table */ -for (i =0 ; i < numblocks;) { +for (i =0 ; i < numblocks; i++) {uint8_t dat; - dat =bbt_get_entry (this, (bbtoffs <<2 ) + i); -for (j =0 ; j <4 ; j++, i++) { -int sftcnt = (i << (3 - sft)) & sftmsk; -/* Do not store the reserved bbt blocks! */ - buf[offs + (i >> sft)] &= - ~(msk[dat &0x03 ] << sftcnt); - dat >>=2 ; - } +int sftcnt = (i << (3 - sft)) & sftmsk; + dat =bbt_get_entry (this, chip * numblocks + i); +/* Do not store the reserved bbt blocks! */ + buf[offs + (i >> sft)] &= ~(msk[dat] << sftcnt); }memset (&einfo,0 ,sizeof (einfo));
@@ -1053,12 +1041,12 @@ static void mark_bbt_region(struct mtd_info *mtd, struct nand_bbt_descr *td)if (td->pages[i] ==-1 )continue ; block = td->pages[i] >> (this->bbt_erase_shift - this->page_shift); - block <<=1 ; - oldval =bbt_get_entry (this, block >>1 ); -bbt_mark_entry (this, block >>1 ,BBT_BLOCK_RESERVED ); + oldval =bbt_get_entry (this, block); +bbt_mark_entry (this, block,BBT_BLOCK_RESERVED );if ((oldval !=BBT_BLOCK_RESERVED ) && td->reserved_block_code) -nand_update_bbt (mtd, (loff_t )block << (this->bbt_erase_shift -1 )); +nand_update_bbt (mtd, (loff_t )block << + this->bbt_erase_shift);continue ; } update =0 ;
@@ -1066,13 +1054,12 @@ static void mark_bbt_region(struct mtd_info *mtd, struct nand_bbt_descr *td) block = ((i +1 ) * nrblocks) - td->maxblocks; else block = i * nrblocks; - block <<=1 ;for (j =0 ; j < td->maxblocks; j++) { - oldval =bbt_get_entry (this, block >>1 ); -bbt_mark_entry (this, block >>1 ,BBT_BLOCK_RESERVED ); + oldval =bbt_get_entry (this, block); +bbt_mark_entry (this, block,BBT_BLOCK_RESERVED );if (oldval !=BBT_BLOCK_RESERVED ) update =1 ; - block +=2 ; + block++; }/* * If we want reserved blocks to be recorded to flash, and some
@@ -1080,7 +1067,8 @@ static void mark_bbt_region(struct mtd_info *mtd, struct nand_bbt_descr *td)* bbts. This should only happen once. */ if (update && td->reserved_block_code) -nand_update_bbt (mtd, (loff_t)(block -2 ) << (this->bbt_erase_shift -1 )); +nand_update_bbt (mtd, (loff_t)(block -1 ) << + this->bbt_erase_shift); } }
@@ -1385,13 +1373,12 @@ int nand_isbad_bbt(struct mtd_info *mtd, loff_t offs, int allowbbt)int block;uint8_t res; -/* Get block number * 2 */ - block = (int )(offs >> (this->bbt_erase_shift -1 )); - res =bbt_get_entry (this, block >>1 ); + block = (int )(offs >> this->bbt_erase_shift); + res =bbt_get_entry (this, block);pr_debug ("nand_isbad_bbt(): bbt info for offs 0x%08x: " "(block %d) 0x%02x\n" , - (unsigned int )offs, block >>1 , res); + (unsigned int )offs, block, res);switch ((int )res) {case BBT_BLOCK_GOOD :
Signature · 1 line
1.7.9.5
PATCH v2 3/6 mtd: nand: eliminate cast
In reply to Brian NorrisJust make 'res' an int.
Signed-off-by Brian Norris <[email protected]>
--- drivers/mtd/nand/nand_bbt.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
drivers/mtd/nand/nand_bbt.c +2 -3
@@ -1370,8 +1370,7 @@ int nand_default_bbt(struct mtd_info *mtd)int nand_isbad_bbt (struct mtd_info *mtd,loff_t offs,int allowbbt) {struct nand_chip *this = mtd->priv; -int block; -uint8_t res; +int block, res; block = (int )(offs >> this->bbt_erase_shift); res =bbt_get_entry (this, block);
@@ -1380,7 +1379,7 @@ int nand_isbad_bbt(struct mtd_info *mtd, loff_t offs, int allowbbt)"(block %d) 0x%02x\n" , (unsigned int)offs, block, res); -switch ((int )res) { +switch (res) {case BBT_BLOCK_GOOD :return 0 ;case BBT_BLOCK_WORN :
Signature · 1 line
1.7.9.5
PATCH v2 4/6 mtd: nand: refactor chip->block_markbad interface
In reply to Brian NorrisThe chip->block_markbad pointer should really only be responsible for writing a bad block marker for new bad blocks. It should not take care of BBT-related functionality, nor should it handle bookkeeping of bad block stats.
This patch refactors the 3 users of the block_markbad interface (plus the default nand_base implementation) so that the common code is kept in nand_block_markbad_lowlevel(). It removes some inconsistencies between the various implementations and should allow for more centralized improvements in the future.
Because gpmi-nand no longer needs the nand_update_bbt() function, let's stop exporting it as well.
Signed-off-by Brian Norris <[email protected]>
--- drivers/mtd/nand/docg4.c | 6 --- drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 44 ++++++---------- drivers/mtd/nand/nand_base.c | 87 +++++++++++++++++++------------- drivers/mtd/nand/nand_bbt.c | 1 - drivers/mtd/nand/sm_common.c | 9 ++-- 5 files changed, 72 insertions(+), 75 deletions(-)
drivers/mtd/nand/docg4.c +0 -6
@@ -1093,7 +1093,6 @@ static int docg4_block_markbad(struct mtd_info *mtd, loff_t ofs)struct nand_chip *nand = mtd->priv;struct docg4_priv *doc = nand->priv;struct nand_bbt_descr *bbtd = nand->badblock_pattern; -int block = (int )(ofs >> nand->bbt_erase_shift);int page = (int )(ofs >> nand->page_shift);uint32_t g4_addr =mtd_to_docg4_address (page,0 );
@@ -1108,9 +1107,6 @@ static int docg4_block_markbad(struct mtd_info *mtd, loff_t ofs)if (buf ==NULL )return -ENOMEM ; -/* update bbt in memory */ - nand->bbt[block /4 ] |=0x01 << ((block &0x03 ) *2 ); -/* write bit-wise negation of pattern to oob buffer */ memset (nand->oob_poi,0xff , mtd->oobsize);for (i =0 ; i < bbtd->len; i++)
@@ -1120,8 +1116,6 @@ static int docg4_block_markbad(struct mtd_info *mtd, loff_t ofs)write_page_prologue (mtd, g4_addr);docg4_write_page (mtd, nand, buf,1 ); ret =pageprog (mtd); -if (!ret) - mtd->ecc_stats.badblocks++;kfree (buf);
drivers/mtd/nand/gpmi-nand/gpmi-nand.c +16 -28
59 lines not on this page · raw message
drivers/mtd/nand/nand_base.c +52 -35
124 lines not on this page · raw message
drivers/mtd/nand/nand_bbt.c +0 -1
@@ -1392,4 +1392,3 @@ int nand_isbad_bbt(struct mtd_info *mtd, loff_t offs, int allowbbt)EXPORT_SYMBOL (nand_scan_bbt);EXPORT_SYMBOL (nand_default_bbt); -EXPORT_SYMBOL_GPL (nand_update_bbt);
drivers/mtd/nand/sm_common.c +4 -5
@@ -42,7 +42,7 @@ static int sm_block_markbad(struct mtd_info *mtd, loff_t ofs) {struct mtd_oob_ops ops;struct sm_oob oob; -int ret, error =0 ; +int ret;memset (&oob,-1 ,SM_OOB_SIZE ); oob.block_status =0x0F ;
@@ -61,11 +61,10 @@ static int sm_block_markbad(struct mtd_info *mtd, loff_t ofs)printk (KERN_NOTICE "sm_common: can't mark sector at %i as bad\n" , (int )ofs); - error = -EIO ; - }else - mtd->ecc_stats.badblocks++; +return -EIO ; + } -return error; +return 0 ; }static struct nand_flash_dev nand_smartmedia_flash_ids[] = {
Signature · 1 line
1.7.9.5
PATCH v2 5/6 mtd: nand: hide in-memory BBT implementation details
In reply to Brian Norrisnand_base.c shouldn't have to know the implementation details of nand_bbt's in-memory BBT. Specifically, nand_base shouldn't perform the bit masking and shifting to isolate a BBT entry.
Instead, just move some of the BBT code into a new nand_markbad_bbt() interface. This interface allows external users (i.e., nand_base) to mark a single block as bad in the BBT. Then nand_bbt will take care of modifying the in-memory BBT and updating the flash-based BBT (if applicable).
Signed-off-by Brian Norris <[email protected]>
--- drivers/mtd/nand/nand_base.c | 32 +++++++++++--------------------- drivers/mtd/nand/nand_bbt.c | 28 ++++++++++++++++++++++++++-- include/linux/mtd/nand.h | 2 +- 3 files changed, 38 insertions(+), 24 deletions(-)
drivers/mtd/nand/nand_base.c +11 -21
@@ -374,22 +374,20 @@ static int nand_default_block_markbad(struct mtd_info *mtd, loff_t ofs)* block table(s) and/or marker(s)). We only allow the hardware driver to * specify how to write bad block markers to OOB (chip->block_markbad). * -* We try operations in the following order, according to our bbt_options -* (NAND_BBT_NO_OOB_BBM and NAND_BBT_USE_FLASH): +* We try operations in the following order: * (1) erase the affected block, to allow OOB marker to be written cleanly -* (2) update in-memory BBT -* (3) write bad block marker to OOB area of affected block -* (4) update flash-based BBT -* Note that we retain the first error encountered in (3) or (4), finish the +* (2) write bad block marker to OOB area of affected block (unless flag +* NAND_BBT_NO_OOB_BBM is present) +* (3) update the BBT +* Note that we retain the first error encountered in (2) or (3), finish the * procedures, and dump the error in the end. */ static int nand_block_markbad_lowlevel (struct mtd_info *mtd,loff_t ofs) {struct nand_chip *chip = mtd->priv; -int block, res, ret =0 ; -int write_oob = !(chip->bbt_options &NAND_BBT_NO_OOB_BBM ); +int res, ret =0 ; -if (write_oob) { +if (!(chip->bbt_options &NAND_BBT_NO_OOB_BBM )) {struct erase_info einfo;/* Attempt erase before marking OOB */
@@ -398,24 +396,16 @@ static int nand_block_markbad_lowlevel(struct mtd_info *mtd, loff_t ofs) einfo.addr = ofs; einfo.len =1 << chip->phys_erase_shift;nand_erase_nand (mtd, &einfo,0 ); - } - -/* Get block number */ - block = (int )(ofs >> chip->bbt_erase_shift); -/* Mark block bad in memory-based BBT */ -if (chip->bbt) - chip->bbt[block >>2 ] |=0x01 << ((block &0x03 ) <<1 ); -/* Write bad block marker to OOB */ -if (write_oob) { +/* Write bad block marker to OOB */ nand_get_device (mtd,FL_WRITING ); ret = chip->block_markbad (mtd, ofs);nand_release_device (mtd); } -/* Update flash-based bad block table */ -if (chip->bbt_options &NAND_BBT_USE_FLASH ) { - res =nand_update_bbt (mtd, ofs); +/* Mark block bad in BBT */ +if (chip->bbt) { + res =nand_markbad_bbt (mtd, ofs);if (!ret) ret = res; }
drivers/mtd/nand/nand_bbt.c +26 -2
@@ -79,6 +79,8 @@#define BBT_ENTRY_MASK 0x03#define BBT_ENTRY_SHIFT 2 +static int nand_update_bbt (struct mtd_info *mtd,loff_t offs); +static inline uint8_t bbt_get_entry (struct nand_chip *chip,int block) {uint8_t entry = chip->bbt[block >>BBT_ENTRY_SHIFT ];
@@ -1194,13 +1196,13 @@ int nand_scan_bbt(struct mtd_info *mtd, struct nand_bbt_descr *bd) }/** -* nand_update_bbt - [NAND Interface] update bad block table(s) +* nand_update_bbt - update bad block table(s) * @mtd: MTD device structure * @offs: the offset of the newly marked block * * The function updates the bad block table(s). */ -int nand_update_bbt (struct mtd_info *mtd,loff_t offs) +static int nand_update_bbt (struct mtd_info *mtd,loff_t offs) {struct nand_chip *this = mtd->priv;int len, res =0 ;
@@ -1390,5 +1392,27 @@ int nand_isbad_bbt(struct mtd_info *mtd, loff_t offs, int allowbbt)return 1 ; } +/** +* nand_markbad_bbt - [NAND Interface] Mark a block bad in the BBT +* @mtd: MTD device structure +* @offs: offset of the bad block +*/ +int nand_markbad_bbt (struct mtd_info *mtd,loff_t offs) +{ +struct nand_chip *this = mtd->priv; +int block, ret =0 ; + + block = (int )(offs >> this->bbt_erase_shift); + +/* Mark bad block in memory */ +bbt_mark_entry (this, block,BBT_BLOCK_WORN ); + +/* Update flash-based bad block table */ +if (this->bbt_options &NAND_BBT_USE_FLASH ) + ret =nand_update_bbt (mtd, offs); + +return ret; +} +EXPORT_SYMBOL (nand_scan_bbt);EXPORT_SYMBOL (nand_default_bbt);
include/linux/mtd/nand.h +1 -1
@@ -628,8 +628,8 @@ extern struct nand_flash_dev nand_flash_ids[];extern struct nand_manufacturers nand_manuf_ids[];extern int nand_scan_bbt (struct mtd_info *mtd,struct nand_bbt_descr *bd); -extern int nand_update_bbt (struct mtd_info *mtd,loff_t offs);extern int nand_default_bbt (struct mtd_info *mtd); +extern int nand_markbad_bbt (struct mtd_info *mtd,loff_t offs);extern int nand_isbad_bbt (struct mtd_info *mtd,loff_t offs,int allowbbt);extern int nand_erase_nand (struct mtd_info *mtd,struct erase_info *instr,int allowbbt);
Signature · 1 line
1.7.9.5
PATCH v2 6/6 mtd: nand: remove NAND_BBT_SCANEMPTY
In reply to Brian NorrisNAND_BBT_SCANEMPTY is a strange, badly-supported option with omap as its single remaining user.
NAND_BBT_SCANEMPTY was likely used by accident in omap2[1]. And anyway, omap2 doesn't scan the chip for bad blocks (courtesy of NAND_SKIP_BBTSCAN), and so its use of this option is irrelevant.
This patch drops the NAND_BBT_SCANEMPTY option.
[1] http://lists.infradead.org/pipermail/linux-mtd/2012-July/042902.html
Signed-off-by Brian Norris <[email protected]>
Cc Ivan Djelic <[email protected]>
--- Documentation/DocBook/mtdnand.tmpl | 2 -- drivers/mtd/nand/nand_bbt.c | 33 +++++---------------------------- drivers/mtd/nand/omap2.c | 2 +- drivers/mtd/onenand/onenand_bbt.c | 1 - include/linux/mtd/bbm.h | 2 -- 5 files changed, 6 insertions(+), 34 deletions(-)
Documentation/DocBook/mtdnand.tmpl +0 -2
@@ -1224,8 +1224,6 @@ in this page</entry> #define NAND_BBT_CREATE 0x00000200 /* Search good / bad pattern through all pages of a block */ #define NAND_BBT_SCANALLPAGES 0x00000400 -/* Scan block empty during good / bad block scan */ -#define NAND_BBT_SCANEMPTY 0x00000800 /* Write bbt if neccecary */ #define NAND_BBT_WRITE 0x00001000 /* Read and write back block contents when writing bbt */
drivers/mtd/nand/nand_bbt.c +5 -28
@@ -110,33 +110,17 @@ static int check_pattern_no_oob(uint8_t *buf, struct nand_bbt_descr *td)* @td: search pattern descriptor * * Check for a pattern at the given place. Used to search bad block tables and -* good / bad block identifiers. If the SCAN_EMPTY option is set then check, if -* all bytes except the pattern area contain 0xff. +* good / bad block identifiers. */ static int check_pattern (uint8_t *buf,int len,int paglen,struct nand_bbt_descr *td) { -int end =0 ; -uint8_t *p = buf; -if (td->options &NAND_BBT_NO_OOB )return check_pattern_no_oob (buf, td); - end = paglen + td->offs; -if (td->options &NAND_BBT_SCANEMPTY ) -if (memchr_inv (p,0xff , end)) -return -1 ; - p += end; -/* Compare the pattern */ -if (memcmp (p, td->pattern, td->len)) +if (memcmp (buf + paglen + td->offs, td->pattern, td->len))return -1 ; -if (td->options &NAND_BBT_SCANEMPTY ) { - p += td->len; - end += td->len; -if (memchr_inv (p,0xff , len - end)) -return -1 ; - }return 0 ; }
@@ -507,15 +491,9 @@ static int create_bbt(struct mtd_info *mtd, uint8_t *buf,else numpages =1 ; -if (!(bd->options &NAND_BBT_SCANEMPTY )) { -/* We need only read few bytes from the OOB area */ - scanlen =0 ; - readlen = bd->len; - }else { -/* Full page content should be read */ - scanlen = mtd->writesize + mtd->oobsize; - readlen = numpages * mtd->writesize; - } +/* We need only read few bytes from the OOB area */ + scanlen =0 ; + readlen = bd->len;if (chip ==-1 ) { numblocks = mtd->size >> this->bbt_erase_shift;
@@ -882,7 +860,6 @@ static inline int nand_memory_bbt(struct mtd_info *mtd, struct nand_bbt_descr *b {struct nand_chip *this = mtd->priv; - bd->options &= ~NAND_BBT_SCANEMPTY ;return create_bbt (mtd, this->buffers->databuf, bd,-1 ); }
drivers/mtd/nand/omap2.c +1 -1
@@ -159,7 +159,7 @@ static struct nand_ecclayout omap_oobinfo;*/ static uint8_t scan_ff_pattern[] = {0xff };static struct nand_bbt_descr bb_descrip_flashbased = { - .options =NAND_BBT_SCANEMPTY |NAND_BBT_SCANALLPAGES , + .options =NAND_BBT_SCANALLPAGES , .offs =0 , .len =1 , .pattern = scan_ff_pattern,
drivers/mtd/onenand/onenand_bbt.c +0 -1
@@ -133,7 +133,6 @@ static inline int onenand_memory_bbt (struct mtd_info *mtd, struct nand_bbt_desc {struct onenand_chip *this = mtd->priv; - bd->options &= ~NAND_BBT_SCANEMPTY ;return create_bbt (mtd, this->page_buf, bd,-1 ); }
include/linux/mtd/bbm.h +0 -2
@@ -93,8 +93,6 @@ struct nand_bbt_descr {#define NAND_BBT_CREATE_EMPTY 0x00000400/* Search good / bad pattern through all pages of a block */ #define NAND_BBT_SCANALLPAGES 0x00000800 -/* Scan block empty during good / bad block scan */ -#define NAND_BBT_SCANEMPTY 0x00001000/* Write bbt if neccecary */ #define NAND_BBT_WRITE 0x00002000/* Read and write back block contents when writing bbt */
Signature · 1 line
1.7.9.5
Artem Bityutskiy
AppliedQuoted 8 lines · Brian Norris
Hi all,
This is v2 of my NAND BBT cleanup. The net diff is exactly the same as v1, but it splits up a few logical changes.
v1 -> v2:
Split up first patch into 3 patches, according to Ezequiel's comments. Hopefully this makes it easier to read and review.
Pushed to l2-mtd.git, thanks!
Signature · 2 lines
Best Regards, Artem Bityutskiy