| Message ID | 20240610074809.2180535-1-mwalle@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | mtd: spi-nor: winbond: fix w25q128 regression | expand |
Am 10.06.2024 um 09:48 schrieb Michael Walle: > Commit 83e824a4a595 ("mtd: spi-nor: Correct flags for Winbond w25q128") > removed the flags for non-SFDP devices. It was assumed that it wasn't in > use anymore. This turned out to be wrong. Add the no_sfdp_flags as > well as the size again. > > Reported-by: e9hack <e9hack@gmail.com> Reported-by: Hartmut Birr <e9hack@gmail.com> > Fixes: 83e824a4a595 ("mtd: spi-nor: Correct flags for Winbond w25q128") > Signed-off-by: Michael Walle <mwalle@kernel.org> > --- > Hartmut, Linus, could you please test it on your boards? Also, do > you have a real name we should put in the Reported-by tag? I cannot test it. I'm using OpenWRT on both routers. OpenWRT uses linux 6.6.32. The differences are to big. Regards, Hartmut > > This will also need a manual backport to the stable kernels due to > the new syntax. But that should be straight forward. > --- > drivers/mtd/spi-nor/winbond.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/mtd/spi-nor/winbond.c b/drivers/mtd/spi-nor/winbond.c > index ca67bf2c46c3..6b6dec6f8faf 100644 > --- a/drivers/mtd/spi-nor/winbond.c > +++ b/drivers/mtd/spi-nor/winbond.c > @@ -105,7 +105,9 @@ static const struct flash_info winbond_nor_parts[] = { > }, { > .id = SNOR_ID(0xef, 0x40, 0x18), > .name = "w25q128", > + .size = SZ_16M, > .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB, > + .no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ, > }, { > .id = SNOR_ID(0xef, 0x40, 0x19), > .name = "w25q256",
On Mon, Jun 10, 2024 at 9:48 AM Michael Walle <mwalle@kernel.org> wrote: > Commit 83e824a4a595 ("mtd: spi-nor: Correct flags for Winbond w25q128") > removed the flags for non-SFDP devices. It was assumed that it wasn't in > use anymore. This turned out to be wrong. Add the no_sfdp_flags as > well as the size again. > > Reported-by: e9hack <e9hack@gmail.com> > Fixes: 83e824a4a595 ("mtd: spi-nor: Correct flags for Winbond w25q128") > Signed-off-by: Michael Walle <mwalle@kernel.org> > --- > Hartmut, Linus, could you please test it on your boards? Also, do > you have a real name we should put in the Reported-by tag? I'm unable to bring mainline Linux back up on this device, so sadly I can't test it right now. But it sure looks fine to me: Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
On 6/10/24 8:48 AM, Michael Walle wrote: > Commit 83e824a4a595 ("mtd: spi-nor: Correct flags for Winbond w25q128") That commit did: - { "w25q128", INFO(0xef4018, 0, 64 * 1024, 256) - NO_SFDP_FLAGS(SECT_4K) }, + { "w25q128", INFO(0xef4018, 0, 0, 0) + PARSE_SFDP + FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB) }, > removed the flags for non-SFDP devices. It was assumed that it wasn't in > use anymore. This turned out to be wrong. Add the no_sfdp_flags as > well as the size again. > > Reported-by: e9hack <e9hack@gmail.com> > Fixes: 83e824a4a595 ("mtd: spi-nor: Correct flags for Winbond w25q128") > Signed-off-by: Michael Walle <mwalle@kernel.org> > --- > Hartmut, Linus, could you please test it on your boards? Also, do > you have a real name we should put in the Reported-by tag? > > This will also need a manual backport to the stable kernels due to > the new syntax. But that should be straight forward. > --- > drivers/mtd/spi-nor/winbond.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/mtd/spi-nor/winbond.c b/drivers/mtd/spi-nor/winbond.c > index ca67bf2c46c3..6b6dec6f8faf 100644 > --- a/drivers/mtd/spi-nor/winbond.c > +++ b/drivers/mtd/spi-nor/winbond.c > @@ -105,7 +105,9 @@ static const struct flash_info winbond_nor_parts[] = { > }, { > .id = SNOR_ID(0xef, 0x40, 0x18), > .name = "w25q128", > + .size = SZ_16M, > .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB, > + .no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ, and here you add dual and quad to trigger SFDP parsing I guess. All fine if the old flash supports dual and quad read. But please update the commit message describing the intention. With that ACK. Would be good to have this merged soon. > }, { > .id = SNOR_ID(0xef, 0x40, 0x19), > .name = "w25q256",
Hi Tudor, On Tue Jun 18, 2024 at 12:33 PM CEST, Tudor Ambarus wrote: > On 6/10/24 8:48 AM, Michael Walle wrote: > > Commit 83e824a4a595 ("mtd: spi-nor: Correct flags for Winbond w25q128") > > That commit did: > - { "w25q128", INFO(0xef4018, 0, 64 * 1024, 256) > - NO_SFDP_FLAGS(SECT_4K) }, > + { "w25q128", INFO(0xef4018, 0, 0, 0) > + PARSE_SFDP > + FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB) }, > > > removed the flags for non-SFDP devices. It was assumed that it wasn't in > > use anymore. This turned out to be wrong. Add the no_sfdp_flags as > > well as the size again. > > > > Reported-by: e9hack <e9hack@gmail.com> > > Fixes: 83e824a4a595 ("mtd: spi-nor: Correct flags for Winbond w25q128") > > Signed-off-by: Michael Walle <mwalle@kernel.org> > > --- > > Hartmut, Linus, could you please test it on your boards? Also, do > > you have a real name we should put in the Reported-by tag? > > > > This will also need a manual backport to the stable kernels due to > > the new syntax. But that should be straight forward. > > --- > > drivers/mtd/spi-nor/winbond.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/mtd/spi-nor/winbond.c b/drivers/mtd/spi-nor/winbond.c > > index ca67bf2c46c3..6b6dec6f8faf 100644 > > --- a/drivers/mtd/spi-nor/winbond.c > > +++ b/drivers/mtd/spi-nor/winbond.c > > @@ -105,7 +105,9 @@ static const struct flash_info winbond_nor_parts[] = { > > }, { > > .id = SNOR_ID(0xef, 0x40, 0x18), > > .name = "w25q128", > > + .size = SZ_16M, > > .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB, > > + .no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ, > > and here you add dual and quad to trigger SFDP parsing I guess. All fine > if the old flash supports dual and quad read. But please update the > commit message describing the intention. With that ACK. Would be good to > have this merged soon. Right. It's not because it will trigger the SFDP parsing, but because that what was tested by Esben. We're lucky that this will trigger the SFDP parsing ;) I'll explain that in more detail and add a Link: to the bug report mail. -michael > > > }, { > > .id = SNOR_ID(0xef, 0x40, 0x19), > > .name = "w25q256",
diff --git a/drivers/mtd/spi-nor/winbond.c b/drivers/mtd/spi-nor/winbond.c index ca67bf2c46c3..6b6dec6f8faf 100644 --- a/drivers/mtd/spi-nor/winbond.c +++ b/drivers/mtd/spi-nor/winbond.c @@ -105,7 +105,9 @@ static const struct flash_info winbond_nor_parts[] = { }, { .id = SNOR_ID(0xef, 0x40, 0x18), .name = "w25q128", + .size = SZ_16M, .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB, + .no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ, }, { .id = SNOR_ID(0xef, 0x40, 0x19), .name = "w25q256",
Commit 83e824a4a595 ("mtd: spi-nor: Correct flags for Winbond w25q128") removed the flags for non-SFDP devices. It was assumed that it wasn't in use anymore. This turned out to be wrong. Add the no_sfdp_flags as well as the size again. Reported-by: e9hack <e9hack@gmail.com> Fixes: 83e824a4a595 ("mtd: spi-nor: Correct flags for Winbond w25q128") Signed-off-by: Michael Walle <mwalle@kernel.org> --- Hartmut, Linus, could you please test it on your boards? Also, do you have a real name we should put in the Reported-by tag? This will also need a manual backport to the stable kernels due to the new syntax. But that should be straight forward. --- drivers/mtd/spi-nor/winbond.c | 2 ++ 1 file changed, 2 insertions(+)