On 19.09.19 15:18, Miquel Raynal wrote: > Hello, > > Schrempf Frieder <frieder.schre...@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 <pio...@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.schre...@kontron.de> > >> >> 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! >>> >>>> Signed-off-by: Piotr Sroka <pio...@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 >
