Message-Id: <DEHLTTY133NE.1Z1LFJ2HCXFJI@kernel.org>
Date: Tue, 25 Nov 2025 08:41:08 +0100
From: "Michael Walle" <mwalle@...nel.org>
To: "Marc Olberding" <molberding@...dia.com>, "Miquel Raynal"
 <miquel.raynal@...tlin.com>
Cc: "Tudor Ambarus" <tudor.ambarus@...aro.org>, "Pratyush Yadav"
 <pratyush@...nel.org>, "Richard Weinberger" <richard@....at>, "Vignesh
 Raghavendra" <vigneshr@...com>, <linux-mtd@...ts.infradead.org>,
 <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mtd: spi-nor: Fix w25q01jv flags

Hi,

> This data was taken with the original patch, as sfdp didn't populate as I had provided
> the size field in the spi-nor-info table, this seems to cause us to skip any sfdp parsing
> and go down the deprecated path.

But the SFDP should still be parsed. Only if you have the SKIP_SFDP
flag set, parsing is turned off. Which kernel are you using? Which
SPI controller are you using?

> Looking at the info table, it looks like all but 4
> spi chips in the winbond table use the deprecated path. Its not clear to me however,
> why it works on Miquel's board but not mine. 
>
> /tmp/xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
> 53464450060101ff00060110800000ff84000102d00000ff03000102f000
> 00ffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffe520fbffffffff3f44eb086b083b42bbfeffffffffff
> 0000ffff40eb0c200f5210d800003602a60082ea14e2e96376337a757a75
> f7a2d55c19f74dffe970f9a5ffffffffffffffffffffffffffffffffff0a
> f0ff21ffdcff
>
> md5sum ./sfdp 
> a7b9dbf76e99a33db99e557b6676588a  ./sfdp
>
> cat /sys/kernel/debug/spi-nor/spi0.0/params
> name            (null)
> id              ef 40 21 00 00 00
> size            128 MiB
> write size      1
> page size       256
> address nbytes  4
> flags           4B_OPCODES | HAS_4BAIT | HAS_16BIT_SR | NO_READ_CR | SOFT_RESET
>
> opcodes
>  read           0x6c
>   dummy cycles  8
>  erase          0xdc
>  program        0x34
>  8D extension   none
>
> protocols
>  read           1S-1S-4S
>  write          1S-1S-4S
>  register       1S-1S-1S
>
> erase commands
>  21 (4.00 KiB) [1]

4k erases are parsed correctly as expected..

>  dc (64.0 KiB) [3]
>  c7 (128 MiB)
>
> sector map
>  region (in hex)   | erase mask | overlaid
>  ------------------+------------+----------
>  00000000-07ffffff |     [   3] | no

..but not selected. I guess you don't have
CONFIG_MTD_SPI_NOR_USE_4K_SECTORS set, do you? In that case I'm lost
why it should have been working with this patch applied. Do you see
any differences in the "params" file if the patch is applied?

>> >> My understanding (which may clearly be erroneous) is that most of these
>> >> flashes support 4K blocks but somehow don't advertise it in their SFDP
>> >> data, so every time we describe a chip we must remember to tick that
>> >> flag.
>> >
>> > Which flag? SECT_4K? I don't think that will be used at all, does
>> > it? It's only used in spi_nor_no_sfdp_init_params() which in turn is
>> > only called in spi_nor_init_params_deprecated() (or if SKIP_SFDP is
>> > set). 
>> >
>
> See my above comment about setting the size in the tables. This eventually
> gets used by spi_nor_needs_sfdp. This ends up being most of the winbond chips
> defined today.

Yeah because they were added before we switched to parsing SFDP.

> All in all, I'm a little confused what the difference is between Miquel's and my
> tests. Any direction would be appreciated, in the meantime, I'm going to continue
> debugging on my end.

What are your tests? Did you try the tests in the "minimal testing
requirements" chapter? Did you see any errors?

-michael

Powered by blists - more mailing lists