Re: [PATCH] mtd: spi-nor: winbond: fix w25q128 regression

["Michael Walle" <mwalle@kernel.org>]

[-- Attachment #1.1: Type: text/plain, Size: 2303 bytes --]

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",


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/