Hi Tudor, On 10/23/2025 5:21 PM, Tudor Ambarus wrote: > > > 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, [email protected] wrote: >>>> From: Takahiro Kuwano <[email protected]> >>>> >>>> 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 <[email protected]> >>>> Signed-off-by: Takahiro Kuwano <[email protected]> >>>> --- >>>> 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. > Yeah, I will revisit to cleanup and isolation.
Thanks, Takahiro
