> -----Original Message----- > From: Tudor Ambarus <[email protected]> > Sent: Tuesday, December 3, 2024 6:37 PM > To: Abbarapu, Venkatesh <[email protected]>; [email protected]; > [email protected] > Cc: Simek, Michal <[email protected]>; [email protected]; > [email protected]; [email protected]; [email protected]; [email protected]; > [email protected]; [email protected]; [email protected]; > [email protected]; [email protected]; [email protected]; > [email protected]; git (AMD-Xilinx) <[email protected]>; Ashok Reddy Soma > <[email protected]> > 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 <[email protected]> > >> Sent: Tuesday, December 3, 2024 4:08 PM > >> To: Abbarapu, Venkatesh <[email protected]>; > >> [email protected]; [email protected] > >> Cc: Simek, Michal <[email protected]>; [email protected]; > >> [email protected]; [email protected]; [email protected]; > >> [email protected]; [email protected]; [email protected]; > >> [email protected]; [email protected]; > >> [email protected]; [email protected]; > >> [email protected]; git (AMD-Xilinx) <[email protected]>; Ashok Reddy Soma > >> <[email protected]> > >> 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 <[email protected]> > >>> Signed-off-by: Venkatesh Yadav Abbarapu <[email protected]> > >>> --- > >>> 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
