[PATCH v2] mtd: spi-nor: Enable mt35xu512aba_fixups for all mt35xx flashes

[Tudor Ambarus]

On 12/3/24 11:48 AM, Abbarapu, Venkatesh wrote:
> 
> 
>> -----Original Message-----
>> From: Tudor Ambarus <tudor.ambarus at linaro.org>
>> Sent: Tuesday, December 3, 2024 4:08 PM
>> To: Abbarapu, Venkatesh <venkatesh.abbarapu at amd.com>; u-boot at lists.denx.de;
>> j-humphreys at ti.com
>> Cc: Simek, Michal <michal.simek at amd.com>; jagan at amarulasolutions.com;
>> vigneshr at ti.com; u-kumar1 at ti.com; trini at konsulko.com; seanga2 at gmail.com;
>> caleb.connolly at linaro.org; sjg at chromium.org; william.zhang at broadcom.com;
>> stefan_b at posteo.net; quentin.schulz at cherry.de; Takahiro.Kuwano at infineon.com;
>> p-mantena at ti.com; git (AMD-Xilinx) <git at amd.com>; Ashok Reddy Soma
>> <ashok.reddy.soma at amd.com>
>> Subject: Re: [PATCH v2] mtd: spi-nor: Enable mt35xu512aba_fixups for all mt35xx
>> flashes
>>
>>
>>
>> On 11/29/24 10:22 AM, Venkatesh Yadav Abbarapu wrote:
>>> Enable mt35xu512aba_fixups for all mt35 series flashes to work in DTR
>>> mode, and return after nor->fixups is updated, otherwise
>>
>> what DTR mode, 8D-8D-8D you mean?
> Yes...correct
>>
>>> it will get overwritten with macronix_octal_fixups.
>>> This flash works in DTR mode only if CONFIG_SPI_FLASH_MT35XU is
>>> enabled and SPI_NOR_OCTAL_DTR_READ flag is set in id table.
>>>
>>> Signed-off-by: Ashok Reddy Soma <ashok.reddy.soma at amd.com>
>>> Signed-off-by: Venkatesh Yadav Abbarapu <venkatesh.abbarapu at amd.com>
>>> ---
>>> Changes in v2:
>>> - Removed the SPI_XFER_SET_DDR flag.
>>> ---
>>>  drivers/mtd/spi/spi-nor-core.c | 7 ++++++-
>>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mtd/spi/spi-nor-core.c
>>> b/drivers/mtd/spi/spi-nor-core.c index ec841fb13b..96f749f7a8 100644
>>> --- a/drivers/mtd/spi/spi-nor-core.c
>>> +++ b/drivers/mtd/spi/spi-nor-core.c
>>> @@ -4404,8 +4404,13 @@ void spi_nor_set_fixups(struct spi_nor *nor)
>>> #endif
>>>
>>>  #ifdef CONFIG_SPI_FLASH_MT35XU
>>> -	if (!strcmp(nor->info->name, "mt35xu512aba"))
>>> +	if (!strcmp(nor->info->name, "mt35xu512aba") ||
>>> +	    !strcmp(nor->info->name, "mt35xl512aba") ||
>>> +	    !strcmp(nor->info->name, "mt35xu01g") ||
>>> +	    !strcmp(nor->info->name, "mt35xu02g")) {
>>>  		nor->fixups = &mt35xu512aba_fixups;
>>> +		return;
>>> +	}
>>
>> this looks buggy. mt35xu512aba supports octal mode and I see that before your
>> patch nor->fixups was set to either mt35xu512aba_fixups or macronix_octal_fixups
>> depending on CONFIG_SPI_FLASH_MT35XU and SPI_FLASH_MACRONIX. Why
>> is it fine to remove macronix_octal_fixups for mt35xu512aba?
> https://github.com/u-boot/u-boot/blob/master/drivers/mtd/spi/spi-nor-ids.c#L360 the mt35xu512aba supports SPI_NOR_OCTAL_DTR_READ.
> Both the configs CONFIG_SPI_FLASH_MT35XU and SPI_FLASH_MACRONIX are enabled in our build and without return it will get overwritten with macronix_octal_fixups.
> 
I got that, but you didn't answer my question. Your patch changes
functionality for mt35xu512aba. If the code is wrong for mt35xu512aba,
fix that first, don't change functionality for mt35xu512aba on the fly.