| Message ID | 20190218104122.18788-3-frieder.schrempf@kontron.de |
|---|---|
| State | Changes Requested |
| Delegated to: | Miquel Raynal |
| Headers | show |
| Series | mtd: rawnand: Support bad block markers in first, second or last page | expand |
Hi Frieder, Schrempf Frieder <frieder.schrempf@kontron.de> wrote on Mon, 18 Feb 2019 10:42:41 +0000: > From: Frieder Schrempf <frieder.schrempf@kontron.de> > > The information about where the manufacturer puts the bad block > markers inside the bad block and in the OOB data is stored in > different places. Let's move this information to the chip struct, > as we did it for rawnand. > > Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de> > --- > drivers/mtd/nand/onenand/onenand_base.c | 5 ++++- > drivers/mtd/nand/onenand/onenand_bbt.c | 3 --- > include/linux/mtd/onenand.h | 3 +++ > 3 files changed, 7 insertions(+), 4 deletions(-) > > diff --git a/drivers/mtd/nand/onenand/onenand_base.c b/drivers/mtd/nand/onenand/onenand_base.c > index 4ca4b194e7d7..f41d76248550 100644 > --- a/drivers/mtd/nand/onenand/onenand_base.c > +++ b/drivers/mtd/nand/onenand/onenand_base.c > @@ -2458,7 +2458,7 @@ static int onenand_default_block_markbad(struct mtd_info *mtd, loff_t ofs) > bbm->bbt[block >> 2] |= 0x01 << ((block & 0x03) << 1); > > /* We write two bytes, so we don't have to mess with 16-bit access */ > - ofs += mtd->oobsize + (bbm->badblockpos & ~0x01); > + ofs += mtd->oobsize + (this->badblockpos & ~0x01); > /* FIXME : What to do when marking SLC block in partition > * with MLC erasesize? For now, it is not advisable to > * create partitions containing both SLC and MLC regions. > @@ -3967,6 +3967,9 @@ int onenand_scan(struct mtd_info *mtd, int maxchips) > if (!(this->options & ONENAND_SKIP_INITIAL_UNLOCKING)) > this->unlock_all(mtd); > > + /* Set the bad block marker position */ > + this->badblockpos = ONENAND_BADBLOCK_POS; > + > ret = this->scan_bbt(mtd); > if ((!FLEXONENAND(this)) || ret) > return ret; > diff --git a/drivers/mtd/nand/onenand/onenand_bbt.c b/drivers/mtd/nand/onenand/onenand_bbt.c > index dde20487937d..57c31c81be18 100644 > --- a/drivers/mtd/nand/onenand/onenand_bbt.c > +++ b/drivers/mtd/nand/onenand/onenand_bbt.c > @@ -190,9 +190,6 @@ static int onenand_scan_bbt(struct mtd_info *mtd, struct nand_bbt_descr *bd) > if (!bbm->bbt) > return -ENOMEM; > > - /* Set the bad block position */ > - bbm->badblockpos = ONENAND_BADBLOCK_POS; > - > /* Set erase shift */ > bbm->bbt_erase_shift = this->erase_shift; > > diff --git a/include/linux/mtd/onenand.h b/include/linux/mtd/onenand.h > index 0aaa98b219a4..e03aea7f7e61 100644 > --- a/include/linux/mtd/onenand.h > +++ b/include/linux/mtd/onenand.h > @@ -94,6 +94,7 @@ struct onenand_chip { > unsigned int technology; > unsigned int density_mask; > unsigned int options; > + int badblockpos; Any reason not to unsign this field? > > unsigned int erase_shift; > unsigned int page_shift; > @@ -188,6 +189,8 @@ struct onenand_chip { > /* Check byte access in OneNAND */ > #define ONENAND_CHECK_BYTE_ACCESS(addr) (addr & 0x1) > > +#define ONENAND_BADBLOCK_POS 0 > + > /* > * Options bits > */ Thanks, Miquèl
On 04.03.19 11:58, Miquel Raynal wrote: > Hi Frieder, > > Schrempf Frieder <frieder.schrempf@kontron.de> wrote on Mon, 18 Feb > 2019 10:42:41 +0000: > >> From: Frieder Schrempf <frieder.schrempf@kontron.de> >> >> The information about where the manufacturer puts the bad block >> markers inside the bad block and in the OOB data is stored in >> different places. Let's move this information to the chip struct, >> as we did it for rawnand. >> >> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de> >> --- >> drivers/mtd/nand/onenand/onenand_base.c | 5 ++++- >> drivers/mtd/nand/onenand/onenand_bbt.c | 3 --- >> include/linux/mtd/onenand.h | 3 +++ >> 3 files changed, 7 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/mtd/nand/onenand/onenand_base.c b/drivers/mtd/nand/onenand/onenand_base.c >> index 4ca4b194e7d7..f41d76248550 100644 >> --- a/drivers/mtd/nand/onenand/onenand_base.c >> +++ b/drivers/mtd/nand/onenand/onenand_base.c >> @@ -2458,7 +2458,7 @@ static int onenand_default_block_markbad(struct mtd_info *mtd, loff_t ofs) >> bbm->bbt[block >> 2] |= 0x01 << ((block & 0x03) << 1); >> >> /* We write two bytes, so we don't have to mess with 16-bit access */ >> - ofs += mtd->oobsize + (bbm->badblockpos & ~0x01); >> + ofs += mtd->oobsize + (this->badblockpos & ~0x01); >> /* FIXME : What to do when marking SLC block in partition >> * with MLC erasesize? For now, it is not advisable to >> * create partitions containing both SLC and MLC regions. >> @@ -3967,6 +3967,9 @@ int onenand_scan(struct mtd_info *mtd, int maxchips) >> if (!(this->options & ONENAND_SKIP_INITIAL_UNLOCKING)) >> this->unlock_all(mtd); >> >> + /* Set the bad block marker position */ >> + this->badblockpos = ONENAND_BADBLOCK_POS; >> + >> ret = this->scan_bbt(mtd); >> if ((!FLEXONENAND(this)) || ret) >> return ret; >> diff --git a/drivers/mtd/nand/onenand/onenand_bbt.c b/drivers/mtd/nand/onenand/onenand_bbt.c >> index dde20487937d..57c31c81be18 100644 >> --- a/drivers/mtd/nand/onenand/onenand_bbt.c >> +++ b/drivers/mtd/nand/onenand/onenand_bbt.c >> @@ -190,9 +190,6 @@ static int onenand_scan_bbt(struct mtd_info *mtd, struct nand_bbt_descr *bd) >> if (!bbm->bbt) >> return -ENOMEM; >> >> - /* Set the bad block position */ >> - bbm->badblockpos = ONENAND_BADBLOCK_POS; >> - >> /* Set erase shift */ >> bbm->bbt_erase_shift = this->erase_shift; >> >> diff --git a/include/linux/mtd/onenand.h b/include/linux/mtd/onenand.h >> index 0aaa98b219a4..e03aea7f7e61 100644 >> --- a/include/linux/mtd/onenand.h >> +++ b/include/linux/mtd/onenand.h >> @@ -94,6 +94,7 @@ struct onenand_chip { >> unsigned int technology; >> unsigned int density_mask; >> unsigned int options; >> + int badblockpos; > > Any reason not to unsign this field? It was signed so far, but you're right that it makes more sense to unsign it. > >> >> unsigned int erase_shift; >> unsigned int page_shift; >> @@ -188,6 +189,8 @@ struct onenand_chip { >> /* Check byte access in OneNAND */ >> #define ONENAND_CHECK_BYTE_ACCESS(addr) (addr & 0x1) >> >> +#define ONENAND_BADBLOCK_POS 0 >> + >> /* >> * Options bits >> */ > > Thanks, > Miquèl >
Hi Frieder, Schrempf Frieder <frieder.schrempf@kontron.de> wrote on Thu, 21 Mar 2019 08:47:52 +0000: > On 04.03.19 11:58, Miquel Raynal wrote: > > Hi Frieder, > > > > Schrempf Frieder <frieder.schrempf@kontron.de> wrote on Mon, 18 Feb > > 2019 10:42:41 +0000: > > > >> From: Frieder Schrempf <frieder.schrempf@kontron.de> > >> > >> The information about where the manufacturer puts the bad block > >> markers inside the bad block and in the OOB data is stored in > >> different places. Let's move this information to the chip struct, > >> as we did it for rawnand. > >> > >> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de> > >> --- > >> drivers/mtd/nand/onenand/onenand_base.c | 5 ++++- > >> drivers/mtd/nand/onenand/onenand_bbt.c | 3 --- > >> include/linux/mtd/onenand.h | 3 +++ > >> 3 files changed, 7 insertions(+), 4 deletions(-) > >> > >> diff --git a/drivers/mtd/nand/onenand/onenand_base.c b/drivers/mtd/nand/onenand/onenand_base.c > >> index 4ca4b194e7d7..f41d76248550 100644 > >> --- a/drivers/mtd/nand/onenand/onenand_base.c > >> +++ b/drivers/mtd/nand/onenand/onenand_base.c > >> @@ -2458,7 +2458,7 @@ static int onenand_default_block_markbad(struct mtd_info *mtd, loff_t ofs) > >> bbm->bbt[block >> 2] |= 0x01 << ((block & 0x03) << 1); > >> > >> /* We write two bytes, so we don't have to mess with 16-bit access */ > >> - ofs += mtd->oobsize + (bbm->badblockpos & ~0x01); > >> + ofs += mtd->oobsize + (this->badblockpos & ~0x01); > >> /* FIXME : What to do when marking SLC block in partition > >> * with MLC erasesize? For now, it is not advisable to > >> * create partitions containing both SLC and MLC regions. > >> @@ -3967,6 +3967,9 @@ int onenand_scan(struct mtd_info *mtd, int maxchips) > >> if (!(this->options & ONENAND_SKIP_INITIAL_UNLOCKING)) > >> this->unlock_all(mtd); > >> > >> + /* Set the bad block marker position */ > >> + this->badblockpos = ONENAND_BADBLOCK_POS; > >> + > >> ret = this->scan_bbt(mtd); > >> if ((!FLEXONENAND(this)) || ret) > >> return ret; > >> diff --git a/drivers/mtd/nand/onenand/onenand_bbt.c b/drivers/mtd/nand/onenand/onenand_bbt.c > >> index dde20487937d..57c31c81be18 100644 > >> --- a/drivers/mtd/nand/onenand/onenand_bbt.c > >> +++ b/drivers/mtd/nand/onenand/onenand_bbt.c > >> @@ -190,9 +190,6 @@ static int onenand_scan_bbt(struct mtd_info *mtd, struct nand_bbt_descr *bd) > >> if (!bbm->bbt) > >> return -ENOMEM; > >> > >> - /* Set the bad block position */ > >> - bbm->badblockpos = ONENAND_BADBLOCK_POS; > >> - > >> /* Set erase shift */ > >> bbm->bbt_erase_shift = this->erase_shift; > >> > >> diff --git a/include/linux/mtd/onenand.h b/include/linux/mtd/onenand.h > >> index 0aaa98b219a4..e03aea7f7e61 100644 > >> --- a/include/linux/mtd/onenand.h > >> +++ b/include/linux/mtd/onenand.h > >> @@ -94,6 +94,7 @@ struct onenand_chip { > >> unsigned int technology; > >> unsigned int density_mask; > >> unsigned int options; > >> + int badblockpos; > > > > Any reason not to unsign this field? > > It was signed so far, but you're right that it makes more sense to > unsign it. With this addressed please add my: Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com> Thanks, Miquèl
diff --git a/drivers/mtd/nand/onenand/onenand_base.c b/drivers/mtd/nand/onenand/onenand_base.c index 4ca4b194e7d7..f41d76248550 100644 --- a/drivers/mtd/nand/onenand/onenand_base.c +++ b/drivers/mtd/nand/onenand/onenand_base.c @@ -2458,7 +2458,7 @@ static int onenand_default_block_markbad(struct mtd_info *mtd, loff_t ofs) bbm->bbt[block >> 2] |= 0x01 << ((block & 0x03) << 1); /* We write two bytes, so we don't have to mess with 16-bit access */ - ofs += mtd->oobsize + (bbm->badblockpos & ~0x01); + ofs += mtd->oobsize + (this->badblockpos & ~0x01); /* FIXME : What to do when marking SLC block in partition * with MLC erasesize? For now, it is not advisable to * create partitions containing both SLC and MLC regions. @@ -3967,6 +3967,9 @@ int onenand_scan(struct mtd_info *mtd, int maxchips) if (!(this->options & ONENAND_SKIP_INITIAL_UNLOCKING)) this->unlock_all(mtd); + /* Set the bad block marker position */ + this->badblockpos = ONENAND_BADBLOCK_POS; + ret = this->scan_bbt(mtd); if ((!FLEXONENAND(this)) || ret) return ret; diff --git a/drivers/mtd/nand/onenand/onenand_bbt.c b/drivers/mtd/nand/onenand/onenand_bbt.c index dde20487937d..57c31c81be18 100644 --- a/drivers/mtd/nand/onenand/onenand_bbt.c +++ b/drivers/mtd/nand/onenand/onenand_bbt.c @@ -190,9 +190,6 @@ static int onenand_scan_bbt(struct mtd_info *mtd, struct nand_bbt_descr *bd) if (!bbm->bbt) return -ENOMEM; - /* Set the bad block position */ - bbm->badblockpos = ONENAND_BADBLOCK_POS; - /* Set erase shift */ bbm->bbt_erase_shift = this->erase_shift; diff --git a/include/linux/mtd/onenand.h b/include/linux/mtd/onenand.h index 0aaa98b219a4..e03aea7f7e61 100644 --- a/include/linux/mtd/onenand.h +++ b/include/linux/mtd/onenand.h @@ -94,6 +94,7 @@ struct onenand_chip { unsigned int technology; unsigned int density_mask; unsigned int options; + int badblockpos; unsigned int erase_shift; unsigned int page_shift; @@ -188,6 +189,8 @@ struct onenand_chip { /* Check byte access in OneNAND */ #define ONENAND_CHECK_BYTE_ACCESS(addr) (addr & 0x1) +#define ONENAND_BADBLOCK_POS 0 + /* * Options bits */