From: Schrempf Frieder <frieder.schrempf@kontron.de>
To: Boris Brezillon <bbrezillon@kernel.org>
Cc: "miquel.raynal@bootlin.com" <miquel.raynal@bootlin.com>,
"richard@nod.at" <richard@nod.at>,
Kyungmin Park <kyungmin.park@samsung.com>,
David Woodhouse <dwmw2@infradead.org>,
Brian Norris <computersforpeace@gmail.com>,
"Marek Vasut" <marek.vasut@gmail.com>,
"linux-mtd@lists.infradead.org" <linux-mtd@lists.infradead.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 1/5] mtd: nand: Always store info about bad block markers in chip struct
Date: Wed, 30 Jan 2019 13:37:57 +0000 [thread overview]
Message-ID: <872cfbb0-f0dd-df25-4835-bb0cca3044cc@kontron.de> (raw)
In-Reply-To: <20190130142838.5590f8b4@bbrezillon>
Hi Boris,
Thanks for reviewing.
On 30.01.19 14:28, Boris Brezillon wrote:
> On Wed, 30 Jan 2019 13:01:44 +0000
> Schrempf Frieder <frieder.schrempf@kontron.de> wrote:
>
>> 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 nand_chip.options
>> and nand_chip.badblockpos.
>>
>> As this chip-specific information is not directly related to the
>> bad block table (BBT), we also rename the flags to NAND_BBM_*.
>>
>> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
>> ---
>> drivers/mtd/nand/onenand/onenand_base.c | 2 +-
>> drivers/mtd/nand/onenand/onenand_bbt.c | 2 +-
>> drivers/mtd/nand/raw/nand_amd.c | 2 +-
>> drivers/mtd/nand/raw/nand_base.c | 12 ++++++------
>> drivers/mtd/nand/raw/nand_bbt.c | 4 ++--
>> drivers/mtd/nand/raw/nand_esmt.c | 2 +-
>> drivers/mtd/nand/raw/nand_hynix.c | 4 ++--
>> drivers/mtd/nand/raw/nand_macronix.c | 2 +-
>> drivers/mtd/nand/raw/nand_micron.c | 2 +-
>> drivers/mtd/nand/raw/nand_samsung.c | 4 ++--
>> drivers/mtd/nand/raw/nand_toshiba.c | 2 +-
>> drivers/mtd/nand/raw/sh_flctl.c | 4 ++--
>> include/linux/mtd/bbm.h | 14 +-------------
>> include/linux/mtd/rawnand.h | 15 +++++++++++++++
>
> Might be better to split that in 2 patches:
> - update onenand logic
> - update rawnand logic
Ok
>
>> 14 files changed, 37 insertions(+), 34 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/onenand/onenand_base.c b/drivers/mtd/nand/onenand/onenand_base.c
>> index 4ca4b194e7d7..d6701b8f031f 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);
>
> Looks like the ->badblockpos field does not exist in struct
> onenand_chip, which means you didn't compile test this part ;-).
Hm, indeed. I forgot to enable onenand in my config. I will add the
field to the onenand_chip struct, to be consistent with rawnand.
>
>> /* 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.
>> diff --git a/drivers/mtd/nand/onenand/onenand_bbt.c b/drivers/mtd/nand/onenand/onenand_bbt.c
>> index dde20487937d..880b0abd36c8 100644
>> --- a/drivers/mtd/nand/onenand/onenand_bbt.c
>> +++ b/drivers/mtd/nand/onenand/onenand_bbt.c
>> @@ -191,7 +191,7 @@ static int onenand_scan_bbt(struct mtd_info *mtd, struct nand_bbt_descr *bd)
>> return -ENOMEM;
>>
>> /* Set the bad block position */
>> - bbm->badblockpos = ONENAND_BADBLOCK_POS;
>> + this->badblockpos = NAND_BBM_POS_ONENAND;
>
> This should be done in onenand_base.c not onenand_bbt.c.
Ok.
>
>>
>> /* Set erase shift */
>> bbm->bbt_erase_shift = this->erase_shift;
>
> ...
>
>> diff --git a/drivers/mtd/nand/raw/nand_bbt.c b/drivers/mtd/nand/raw/nand_bbt.c
>> index 1b722fe9213c..862eaa3a0552 100644
>> --- a/drivers/mtd/nand/raw/nand_bbt.c
>> +++ b/drivers/mtd/nand/raw/nand_bbt.c
>> @@ -466,7 +466,7 @@ static int create_bbt(struct nand_chip *this, uint8_t *buf,
>>
>> pr_info("Scanning device for bad blocks\n");
>>
>> - if (bd->options & NAND_BBT_SCAN2NDPAGE)
>> + if (this->options & NAND_BBM_SECONDPAGE)
>> numpages = 2;
>> else
>> numpages = 1;
>> @@ -487,7 +487,7 @@ static int create_bbt(struct nand_chip *this, uint8_t *buf,
>> from = (loff_t)startblock << this->bbt_erase_shift;
>> }
>>
>> - if (this->bbt_options & NAND_BBT_SCANLASTPAGE)
>> + if (this->bbt_options & NAND_BBM_LASTPAGE)
>
> You're not checking the right field here:
>
> if (this->options & NAND_BBM_LASTPAGE)
I will fix this.
Regards,
Frieder
>
>> from += mtd->erasesize - (mtd->writesize * numpages);
>>
>> for (i = startblock; i < numblocks; i++) {
next prev parent reply other threads:[~2019-01-30 13:38 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-30 13:01 [PATCH v3 0/5] mtd: rawnand: Support bad block markers in first, second or last page Schrempf Frieder
2019-01-30 13:01 ` [PATCH v3 1/5] mtd: nand: Always store info about bad block markers in chip struct Schrempf Frieder
2019-01-30 13:28 ` Boris Brezillon
2019-01-30 13:37 ` Schrempf Frieder [this message]
2019-01-30 13:01 ` [PATCH v3 2/5] mtd: nand: Make flags for bad block marker position more granular Schrempf Frieder
2019-01-30 13:38 ` Boris Brezillon
2019-01-30 13:01 ` [PATCH v3 3/5] mtd: rawnand: Support bad block markers in first, second or last page Schrempf Frieder
2019-01-30 13:01 ` [PATCH v3 4/5] mtd: rawnand: ESMT: Also use the last page for bad block markers Schrempf Frieder
2019-01-30 13:01 ` [PATCH v3 5/5] mtd: rawnand: AMD: " Schrempf Frieder
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=872cfbb0-f0dd-df25-4835-bb0cca3044cc@kontron.de \
--to=frieder.schrempf@kontron.de \
--cc=bbrezillon@kernel.org \
--cc=computersforpeace@gmail.com \
--cc=dwmw2@infradead.org \
--cc=kyungmin.park@samsung.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mtd@lists.infradead.org \
--cc=marek.vasut@gmail.com \
--cc=miquel.raynal@bootlin.com \
--cc=richard@nod.at \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).