Messages in this thread | | | Date | Mon, 23 Sep 2019 13:13:39 +0100 | From | Piotr Sroka <> | Subject | Re: [PATCH] - change calculating of position page containing BBM |
| |
Hello
The 09/19/2019 13:33, Schrempf Frieder wrote: >EXTERNAL MAIL > > >On 19.09.19 15:18, Miquel Raynal wrote: >> Hello, >> >> Schrempf Frieder <frieder.schrempf@kontron.de> wrote on Thu, 19 Sep >> 2019 13:15:08 +0000: >> >>> On 19.09.19 14:58, Miquel Raynal wrote: >>>> Hi Piotr, >>>> >>>> Piotr Sroka <piotrs@cadence.com> wrote on Thu, 19 Sep 2019 13:41:35 >>>> +0100: >>>> >>>>> Change calculating of position page containing BBM >>>>> >>>>> If none of BBM flags is set then function nand_bbm_get_next_page >>>>> reports EINVAL. It causes that BBM is not read at all during scanning >>>>> factory bad blocks. The result is that the BBT table is build without >>>>> checking factory BBM at all. For Micron flash memories none of this >>>>> flag is set if page size is different than 2048 bytes. >>> >>> I wonder if it wouldn't be better to fix the Micron driver instead: >>> >>> --- a/drivers/mtd/nand/raw/nand_micron.c >>> +++ b/drivers/mtd/nand/raw/nand_micron.c >>> @@ -448,6 +448,8 @@ static int micron_nand_init(struct nand_chip *chip) >>> >>> if (mtd->writesize == 2048) >>> chip->options |= NAND_BBM_FIRSTPAGE | >>> NAND_BBM_SECONDPAGE; >>> + else >>> + chip->options |= NAND_BBM_FIRSTPAGE; >> >> That's what I forgot in my last answer to this thread, I think I only >> told Piotr privately: I would like both. I think it is important to fix >> the bbm_get_next_page function but for clarity, setting the FIRSTPAGE >> flag in Micron's driver seems also pertinent. > >Indeed, that sounds reasonable. Piotr, can you send another patch with >the diff above? And by the way: thanks for fixing my code ;) > >Reviewed-by: Frieder Schrempf <frieder.schrempf@kontron.de> > Thanks Frieder and Miquel for the very quick reply. I will send next version containing Micron driver fix.
>> >>> >>> ondie = micron_supports_on_die_ecc(chip); >>> >>> >>>> >>>> "none of these flags are set" >>>> >>>>> >>>>> This patch changes the nand_bbm_get_next_page function. >>>> >>>> "Address this regression by changing the >>>> nand_bbm_get_next_page_function." >>>> >>>>> It will return 0 if none of BBM flag is set and page parameter is 0. >>>> >>>> no BBM flag is set >>>> >>>>> After that modification way of discovering factory bad blocks will work >>>>> similar as in kernel version 5.1. >>>>> >>>> >>>> Fixes + stable tags would be great! Ok I will add fixes tag and "Cc: <stable@vger.kernel.org>".
>>>> >>>>> Signed-off-by: Piotr Sroka <piotrs@cadence.com> >>>>> --- >>>>> drivers/mtd/nand/raw/nand_base.c | 8 ++++++-- >>>>> 1 file changed, 6 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c >>>>> index 5c2c30a7dffa..f64e3b6605c6 100644 >>>>> --- a/drivers/mtd/nand/raw/nand_base.c >>>>> +++ b/drivers/mtd/nand/raw/nand_base.c >>>>> @@ -292,12 +292,16 @@ int nand_bbm_get_next_page(struct nand_chip *chip, int page) >>>>> struct mtd_info *mtd = nand_to_mtd(chip); >>>>> int last_page = ((mtd->erasesize - mtd->writesize) >> >>>>> chip->page_shift) & chip->pagemask; >>>>> + unsigned int bbm_flags = NAND_BBM_FIRSTPAGE | NAND_BBM_SECONDPAGE >>>>> + | NAND_BBM_LASTPAGE; >>>>> >>>>> + if (page == 0 && !(chip->options & bbm_flags)) >>>>> + return 0; >>>>> if (page == 0 && chip->options & NAND_BBM_FIRSTPAGE) >>>>> return 0; >>>>> - else if (page <= 1 && chip->options & NAND_BBM_SECONDPAGE) >>>>> + if (page <= 1 && chip->options & NAND_BBM_SECONDPAGE) >>>>> return 1; >>>>> - else if (page <= last_page && chip->options & NAND_BBM_LASTPAGE) >>>>> + if (page <= last_page && chip->options & NAND_BBM_LASTPAGE) >>>>> return last_page; >>>>> >>>>> return -EINVAL; >>>> >>>> Lookgs good otherwise. >>>> >>>> Thanks, >>>> Miquèl >>>> >> >> Thanks, >> Miquèl >>
Thanks, Piotr
| |