* [PATCH v3 0/3] mtd: cfi_cmdset_0002: Use chip_ready() for write on S29GL064N @ 2022-03-15 16:56 Tokunori Ikegami 2022-03-15 16:56 ` [PATCH v3 3/3] " Tokunori Ikegami 0 siblings, 1 reply; 4+ messages in thread From: Tokunori Ikegami @ 2022-03-15 16:56 UTC (permalink / raw) To: miquel.raynal Cc: linux-mtd, Tokunori Ikegami, Ahmad Fatoum, Richard Weinberger, Vignesh Raghavendra, stable As pointed out by this bug report [1], the buffered write is now broken on S29GL064N. The reason is that changed the buffered write to use chip_good instead of chip_ready. One way to solve the issue is to revert the change partially to use chip_ready for S29GL064N since the way of least surprise. [1] https://lore.kernel.org/r/b687c259-6413-26c9-d4c9-b3afa69ea124@pengutronix.de/ Fixes: dfeae1073583("mtd: cfi_cmdset_0002: Change write buffer to check correct value") Signed-off-by: Tokunori Ikegami <ikegami.t@gmail.com> Tested-by: Ahmad Fatoum <a.fatoum@pengutronix.de> Cc: Miquel Raynal <miquel.raynal@bootlin.com> Cc: Richard Weinberger <richard@nod.at> Cc: Vignesh Raghavendra <vigneshr@ti.com> Cc: linux-mtd@lists.infradead.org Cc: stable@vger.kernel.org Tokunori Ikegami (3): mtd: cfi_cmdset_0002: Add S29GL064N ID definition mtd: cfi_cmdset_0002: Move and rename chip_check/chip_ready/chip_good_for_write mtd: cfi_cmdset_0002: Use chip_ready() for write on S29GL064N drivers/mtd/chips/cfi_cmdset_0002.c | 89 +++++++++++++++-------------- 1 file changed, 47 insertions(+), 42 deletions(-) -- 2.32.0 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v3 3/3] mtd: cfi_cmdset_0002: Use chip_ready() for write on S29GL064N 2022-03-15 16:56 [PATCH v3 0/3] mtd: cfi_cmdset_0002: Use chip_ready() for write on S29GL064N Tokunori Ikegami @ 2022-03-15 16:56 ` Tokunori Ikegami 2022-03-15 18:51 ` Miquel Raynal 0 siblings, 1 reply; 4+ messages in thread From: Tokunori Ikegami @ 2022-03-15 16:56 UTC (permalink / raw) To: miquel.raynal Cc: linux-mtd, Tokunori Ikegami, Ahmad Fatoum, Richard Weinberger, Vignesh Raghavendra, stable As pointed out by this bug report [1], the buffered write is now broken on S29GL064N. The reason is that changed the buffered write to use chip_good instead of chip_ready. One way to solve the issue is to revert the change partially to use chip_ready for S29GL064N since the way of least surprise. [1] https://lore.kernel.org/r/b687c259-6413-26c9-d4c9-b3afa69ea124@pengutronix.de/ Fixes: dfeae1073583("mtd: cfi_cmdset_0002: Change write buffer to check correct value") Signed-off-by: Tokunori Ikegami <ikegami.t@gmail.com> Tested-by: Ahmad Fatoum <a.fatoum@pengutronix.de> Cc: Miquel Raynal <miquel.raynal@bootlin.com> Cc: Richard Weinberger <richard@nod.at> Cc: Vignesh Raghavendra <vigneshr@ti.com> Cc: linux-mtd@lists.infradead.org Cc: stable@vger.kernel.org --- drivers/mtd/chips/cfi_cmdset_0002.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c index 8f3f0309dc03..fa11db066c99 100644 --- a/drivers/mtd/chips/cfi_cmdset_0002.c +++ b/drivers/mtd/chips/cfi_cmdset_0002.c @@ -867,10 +867,20 @@ static int __xipram chip_good(struct map_info *map, struct flchip *chip, return chip_check(map, chip, addr, &expected); } +static bool __xipram cfi_use_chip_ready_for_write(struct map_info *map) +{ + struct cfi_private *cfi = map->fldrv_priv; + + return cfi->mfr == CFI_MFR_AMD && cfi->id == S29GL064N_MN12; +} + static int __xipram chip_good_for_write(struct map_info *map, struct flchip *chip, unsigned long addr, map_word expected) { + if (cfi_use_chip_ready_for_write(map)) + return chip_ready(map, chip, addr); + return chip_good(map, chip, addr, expected); } -- 2.32.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v3 3/3] mtd: cfi_cmdset_0002: Use chip_ready() for write on S29GL064N 2022-03-15 16:56 ` [PATCH v3 3/3] " Tokunori Ikegami @ 2022-03-15 18:51 ` Miquel Raynal 2022-03-16 16:05 ` Tokunori Ikegami 0 siblings, 1 reply; 4+ messages in thread From: Miquel Raynal @ 2022-03-15 18:51 UTC (permalink / raw) To: Tokunori Ikegami Cc: linux-mtd, Ahmad Fatoum, Richard Weinberger, Vignesh Raghavendra, stable Hi Tokunori, ikegami.t@gmail.com wrote on Wed, 16 Mar 2022 01:56:07 +0900: > As pointed out by this bug report [1], the buffered write is now broken on , buffered writes are now broken > S29GL064N. The reason is that changed the buffered write to use chip_good > instead of chip_ready. "This issue comes from a rework which switched from using chip_good() to chip_ready(), because <explain the difference here>." [please note I am just trying to understand what the root cause is, please rephrase if I'm wrong]. > One way to solve the issue is to revert the change > partially to use chip_ready for S29GL064N since the way of least surprise. s/since the way of least surprise// > > [1] https://lore.kernel.org/r/b687c259-6413-26c9-d4c9-b3afa69ea124@pengutronix.de/ > > Fixes: dfeae1073583("mtd: cfi_cmdset_0002: Change write buffer to check correct value") > Signed-off-by: Tokunori Ikegami <ikegami.t@gmail.com> > Tested-by: Ahmad Fatoum <a.fatoum@pengutronix.de> > Cc: Miquel Raynal <miquel.raynal@bootlin.com> > Cc: Richard Weinberger <richard@nod.at> > Cc: Vignesh Raghavendra <vigneshr@ti.com> > Cc: linux-mtd@lists.infradead.org I think you can get rid of all the above Cc: tags and just copy all 3 of us + the mailing list when sending your v4. > Cc: stable@vger.kernel.org > --- Please also include a Fixes/stable tag in the patch before (2/3) to explain that both patches are required in order to fix the issue and the current patch alone won't apply. You should mention that with a nice comment below the three dashes ("---") in patch 2/3 as well. > drivers/mtd/chips/cfi_cmdset_0002.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c > index 8f3f0309dc03..fa11db066c99 100644 > --- a/drivers/mtd/chips/cfi_cmdset_0002.c > +++ b/drivers/mtd/chips/cfi_cmdset_0002.c > @@ -867,10 +867,20 @@ static int __xipram chip_good(struct map_info *map, struct flchip *chip, > return chip_check(map, chip, addr, &expected); > } > > +static bool __xipram cfi_use_chip_ready_for_write(struct map_info *map) > +{ > + struct cfi_private *cfi = map->fldrv_priv; > + > + return cfi->mfr == CFI_MFR_AMD && cfi->id == S29GL064N_MN12; > +} > + > static int __xipram chip_good_for_write(struct map_info *map, > struct flchip *chip, unsigned long addr, > map_word expected) > { > + if (cfi_use_chip_ready_for_write(map)) > + return chip_ready(map, chip, addr); > + > return chip_good(map, chip, addr, expected); > } > This is much more understandable. Vignesh, perhaps it would be better to provide a way for manufacturers to overload certain callbacks instead of applying quirks like this in the code. But that will come in a second time of course. Thanks, Miquèl ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3 3/3] mtd: cfi_cmdset_0002: Use chip_ready() for write on S29GL064N 2022-03-15 18:51 ` Miquel Raynal @ 2022-03-16 16:05 ` Tokunori Ikegami 0 siblings, 0 replies; 4+ messages in thread From: Tokunori Ikegami @ 2022-03-16 16:05 UTC (permalink / raw) To: Miquel Raynal Cc: linux-mtd, Ahmad Fatoum, Richard Weinberger, Vignesh Raghavendra, stable Hi, On 2022/03/16 3:51, Miquel Raynal wrote: > Hi Tokunori, > > ikegami.t@gmail.com wrote on Wed, 16 Mar 2022 01:56:07 +0900: > >> As pointed out by this bug report [1], the buffered write is now broken on > , buffered writes are now broken > >> S29GL064N. The reason is that changed the buffered write to use chip_good >> instead of chip_ready. > "This issue comes from a rework which switched from using chip_good() > to chip_ready(), because <explain the difference here>." > > [please note I am just trying to understand what the root cause is, > please rephrase if I'm wrong]. Fixed by the version 4 patches. > >> One way to solve the issue is to revert the change >> partially to use chip_ready for S29GL064N since the way of least surprise. > s/since the way of least surprise// Fixed by the version 4 patches. > > >> [1] https://lore.kernel.org/r/b687c259-6413-26c9-d4c9-b3afa69ea124@pengutronix.de/ >> >> Fixes: dfeae1073583("mtd: cfi_cmdset_0002: Change write buffer to check correct value") >> Signed-off-by: Tokunori Ikegami <ikegami.t@gmail.com> >> Tested-by: Ahmad Fatoum <a.fatoum@pengutronix.de> >> Cc: Miquel Raynal <miquel.raynal@bootlin.com> >> Cc: Richard Weinberger <richard@nod.at> >> Cc: Vignesh Raghavendra <vigneshr@ti.com> >> Cc: linux-mtd@lists.infradead.org > I think you can get rid of all the above Cc: tags and just copy all 3 > of us + the mailing list when sending your v4. Fixed by the version 4 patches. > >> Cc: stable@vger.kernel.org >> --- > Please also include a Fixes/stable tag in the patch before (2/3) to explain > that both patches are required in order to fix the issue and the current patch alone won't apply. > > You should mention that with a nice comment below the three dashes ("---") in patch 2/3 as well. > >> drivers/mtd/chips/cfi_cmdset_0002.c | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c >> index 8f3f0309dc03..fa11db066c99 100644 >> --- a/drivers/mtd/chips/cfi_cmdset_0002.c >> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c >> @@ -867,10 +867,20 @@ static int __xipram chip_good(struct map_info *map, struct flchip *chip, >> return chip_check(map, chip, addr, &expected); >> } >> >> +static bool __xipram cfi_use_chip_ready_for_write(struct map_info *map) >> +{ >> + struct cfi_private *cfi = map->fldrv_priv; >> + >> + return cfi->mfr == CFI_MFR_AMD && cfi->id == S29GL064N_MN12; >> +} >> + >> static int __xipram chip_good_for_write(struct map_info *map, >> struct flchip *chip, unsigned long addr, >> map_word expected) >> { >> + if (cfi_use_chip_ready_for_write(map)) >> + return chip_ready(map, chip, addr); >> + >> return chip_good(map, chip, addr, expected); >> } >> > This is much more understandable. > > Vignesh, perhaps it would be better to provide a way for manufacturers > to overload certain callbacks instead of applying quirks like this in > the code. But that will come in a second time of course. > > > Thanks, > Miquèl ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-03-16 16:05 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-03-15 16:56 [PATCH v3 0/3] mtd: cfi_cmdset_0002: Use chip_ready() for write on S29GL064N Tokunori Ikegami 2022-03-15 16:56 ` [PATCH v3 3/3] " Tokunori Ikegami 2022-03-15 18:51 ` Miquel Raynal 2022-03-16 16:05 ` Tokunori Ikegami
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).