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

[Abbarapu, Venkatesh]

> -----Original Message-----
> From: Tudor Ambarus <tudor.ambarus at linaro.org>
> Sent: Tuesday, December 3, 2024 6:37 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 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.
I missed this change https://github.com/u-boot/u-boot/blob/master/drivers/mtd/spi/spi-nor-core.c#L4412 (mtd: spi-nor: Check nor->info before setting macronix_octal_fixups). Will update the patch.

Thanks
Venkatesh