Message ID | 20190919124139.10856-1-piotrs@cadence.com |
---|---|
State | Changes Requested |
Headers | show |
Series | - change calculating of position page containing BBM | expand |
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. "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 <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
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; 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 <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 >
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. > > 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 <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
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> > >> >> 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 <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 >
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
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;
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. This patch changes the nand_bbm_get_next_page function. It will return 0 if none of BBM flag is set and page parameter is 0. After that modification way of discovering factory bad blocks will work similar as in kernel version 5.1. Signed-off-by: Piotr Sroka <piotrs@cadence.com> --- drivers/mtd/nand/raw/nand_base.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)