[PATCH] mtd: spi-nor: Set address mode in SEMPER flash when SFDP is skipped

[Tudor Ambarus]

On 10/21/25 8:41 AM, Takahiro Kuwano wrote:
> Hi Tudor,

Hi!

> 
> On 10/21/2025 3:36 AM, Tudor Ambarus wrote:
>> Hi, Takahiro,
>>
>> On 10/20/25 8:25 AM, tkuw584924 at gmail.com wrote:
>>> From: Takahiro Kuwano <Takahiro.Kuwano at infineon.com>
>>>
>>> nor->addr_mode_nbytes is set during SFDP parse. Infineon SEMPER flash
>>> family relies on that parameter to read and write registers. To support
>>> use cases of skipping SFDP, set address mode in device specific setup()
>>> function.
>>>
>>> Tested-by: Hiroyuki Saito <Hiroyuki.Saito2 at infineon.com>
>>> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano at infineon.com>
>>> ---
>>>  drivers/mtd/spi/spi-nor-core.c | 10 ++++++++++
>>>  1 file changed, 10 insertions(+)
>>>
>>> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
>>> index 6f352c5c0e2..e382a518a34 100644
>>> --- a/drivers/mtd/spi/spi-nor-core.c
>>> +++ b/drivers/mtd/spi/spi-nor-core.c
>>> @@ -3794,6 +3794,16 @@ static int s25_s28_setup(struct spi_nor *nor, const struct flash_info *info,
>>>  #ifdef CONFIG_SPI_FLASH_BAR
>>>  	return -ENOTSUPP; /* Bank Address Register is not supported */
>>>  #endif
>>> +
>>> +	/* Setup address mode here, in case SFDP is skipped. */
>>
>> Under which conditions is the SFDP skipped?
>>
> In u-boot, SFDP is config option and some defconfig do not use it.

I'd argue this is a good idea. Maybe for the tiny duplicate version
of the driver could be, but for the main driver, it's not.

I'd also argue the tiny driver idea was ideal to start with. Instead
we should have tried to modularize SPI NOR, by vendors, static init
and SFDP.

> 
>>> +	if (!nor->addr_mode_nbytes) {
>>> +		ret = set_4byte(nor, nor->info, 1);
>>
>> why do you need this call? Isn't enough the one done in spi_nor_init()?
>>
> The addr_mode_nbytes is refereed in spansion_read_any_reg() during
> setup().

Right. So you set it twice. Can we set it just once?

I guess you can feel my disapproval about where the code is leading to if
we continue like this: unmaintainable code.

Cheers,
ta

> 
>> Cheers,
>> ta> +		if (ret)
>>> +			return ret;
>>> +
>>> +		nor->addr_mode_nbytes = 4;
>>> +	}
>>> +
>>>  	/*
>>>  	 * S25FS256T has multiple sector architecture options, with selection of
>>>  	 * count and location of 128KB and 64KB sectors. This driver supports
>