[U-Boot] [EXT] Re: [Patch V3] drivers: mtd :spi: Enable 4B opcodes for SPANSION s25fl512s

Ashish Kumar ashish.kumar at nxp.com
Thu Jul 18 14:27:30 UTC 2019



> -----Original Message-----
> From: Vignesh Raghavendra <vigneshr at ti.com>
> Sent: Thursday, July 18, 2019 5:37 PM
> To: Jagan Teki <jagan at amarulasolutions.com>; Ashish Kumar
> <ashish.kumar at nxp.com>
> Cc: U-Boot-Denx <u-boot at lists.denx.de>
> Subject: Re: [EXT] Re: [Patch V3] drivers: mtd :spi: Enable 4B opcodes for
> SPANSION s25fl512s
> 
> Caution: EXT Email
> 
> On 18/07/19 4:29 PM, Jagan Teki wrote:
> > On Thu, Jul 18, 2019 at 4:15 PM Ashish Kumar <ashish.kumar at nxp.com>
> wrote:
> >>
> >>
> >>
> >>> -----Original Message-----
> >>> From: Jagan Teki <jagan at amarulasolutions.com>
> >>> Sent: Thursday, July 18, 2019 3:59 PM
> >>> To: Ashish Kumar <ashish.kumar at nxp.com>; Vignesh R
> <vigneshr at ti.com>
> >>> Cc: U-Boot-Denx <u-boot at lists.denx.de>
> >>> Subject: [EXT] Re: [Patch V3] drivers: mtd :spi: Enable 4B opcodes
> >>> for SPANSION s25fl512s
> >>>
> >>> Caution: EXT Email
> >>>
> >>> + Vignesh
> >>>
> >>> On Wed, Jul 17, 2019 at 11:46 AM Ashish Kumar
> <Ashish.Kumar at nxp.com>
> >>> wrote:
> >>>>
> >>>> s25fs512s and s25fl512s which has same JEDEC ID but only varies in
> >>>> operating volatge so s25fs512s shares same command set as
> mentioned
> >>>> below:
> >>>> – Serial Command subset and footprint compatible with S25FL-A,
> >>>> S25FL-K, S25FL-P, and S25FL-S SPI families – Multi I/O Command
> >>>> subset and footprint compatible with S25FL-P, and S25FL-S SPI
> >>>> families
> >>>>
> >>>> Signed-off-by: Ashish Kumar <Ashish.Kumar at nxp.com>
> >>>> ---
> >>>> v3:
> >>>> 1. Add version info, rebase to top.
> >>>> 2. Re-word commit message.
> >>>> v2:
> >>>> 1. Adding more description in commit msg.
> >>>> 2. consolidating "" and  "" in single patch.
> >>>>
> >>>>  drivers/mtd/spi/spi-nor-ids.c | 2 +-
> >>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/mtd/spi/spi-nor-ids.c
> >>>> b/drivers/mtd/spi/spi-nor-ids.c index 3217fc6..a992966 100644
> >>>> --- a/drivers/mtd/spi/spi-nor-ids.c
> >>>> +++ b/drivers/mtd/spi/spi-nor-ids.c
> >>>> @@ -181,7 +181,7 @@ const struct flash_info spi_nor_ids[] = {
> >>>>         { INFO("s25sl064p",  0x010216, 0x4d00,  64 * 1024, 128,
> >>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> >>>>         { INFO("s25fl256s0", 0x010219, 0x4d00, 256 * 1024, 128, USE_CLSR)
> },
> >>>>         { INFO("s25fl256s1", 0x010219, 0x4d01,  64 * 1024, 512,
> >>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
> >>>> -       { INFO6("s25fl512s",  0x010220, 0x4d0081, 256 * 1024, 256,
> >>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
> >>>> +       { INFO6("s25fl512s",  0x010220, 0x4d0081, 256 * 1024, 256,
> >>>> + SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR |
> >>>> + SPI_NOR_4B_OPCODES) },
> >>>
> >>> I didn't find any diff b/w this with respect to v1 patch, seems like
> >>> Vignesh commented some issue? any update on that.
> >>
> >> Hi Jagan, Vignesh,
> >>
> >> I had updated commit message.
> >>
> >> Wrt Vignesh's comment:
> >> To me it seems not valid. This Flash can be used in extended 3-byte
> addressing mode as well as 4-byte addressing mode.
> >> Adding SPI_NOR_4B_OPCODE, will make use of 4-byte addressing mode
> with CONFIG_SPI_FLASH_BAR being __not__ set.
> >> By adding SPI_NOR_4B_OPCODES code flow will via
> spi_nor_set_4byte_opcodes() in place of set_4byte().
> >>
> 
> This does not seem right. Here is the code snippet from spi-nor-
> core.c::spi_nor_scan():
> 
> #ifndef CONFIG_SPI_FLASH_BAR
>                 /* enable 4-byte addressing if the device exceeds 16MiB */
>                 nor->addr_width = 4;
>                 if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
>                     info->flags & SPI_NOR_4B_OPCODES)
Hi Vignesh,

1. It seems old, but at least one of the flash(spansion) on my NXP/Freescale board reported manufacturing ID as 02h in place of 01h as result I had to add this flags, I will try to trace that particular board. Reading the flow again it seems this patch is not required due to presence of || operator as long manufacturer ID is reported correctly.
if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
                     info->flags & SPI_NOR_4B_OPCODES)
2. The very first link on google [1] leads to an APP note from spansion suggesting manufacturing id as 02h, is there a possibility of such flashes in circulation, or am I referring something incorrect ? I will double check this with spansion FAE. 
Snippet from APP note named: "AN98488 - Quick Guide to Common Flash Interface"
>>>>>
The Cypress Manufacturer ID for flash devices is 0002h (historically the AMD ID is
0002h and the Fujitsu ID is 0004h; the former Spansion ID is 0002h)."
<<<<<
3. s25fl064l, s25fl128l flash id has SPI_NOR_4B_OPCODES, what is the use of this flag here then ?

[1]: https://www.google.com/url?sa=t&rct=j&q=&esrc=s&source=web&cd=1&cad=rja&uact=8&ved=2ahUKEwjHpY3U0b7jAhVEWX0KHeqSAQkQFjAAegQIBBAB&url=http%3A%2F%2Fwww.cypress.com%2Ffile%2F195291%2Fdownload&usg=AOvVaw0zPEXOjP80J8XVrwmB7TBS

Regards
Ashish 
>                         spi_nor_set_4byte_opcodes(nor, info); #else
>         /* Configure the BAR - discover bank cmds and read current bank */
>         nor->addr_width = 3;
>         ret = read_bar(nor, info);
>         if (ret < 0)
>                 return ret;
> #endif
> 
> 
> So as long as CONFIG_SPI_FLASH_BAR is not defined and flash is Spansion
> flash
> spi_nor_set_4byte_opcodes() is always called irrespective of
> SPI_NOR_4B_OPCODES
> 
> Also from spi_nor_init():,
> 
> 
>         if (nor->addr_width == 4 &&
>             (JEDEC_MFR(nor->info) != SNOR_MFR_SPANSION) &&
>             !(nor->info->flags & SPI_NOR_4B_OPCODES)) {
>                 /*
>                  * If the RESET# pin isn't hooked up properly, or the system
>                  * otherwise doesn't perform a reset command in the boot
>                  * sequence, it's impossible to 100% protect against unexpected
>                  * reboots (e.g., crashes). Warn the user (or hopefully, system
>                  * designer) that this is bad.
>                  */
>                 if (nor->flags & SNOR_F_BROKEN_RESET)
>                         printf("enabling reset hack; may not recover from unexpected
> reboots\n");
>                 set_4byte(nor, nor->info, 1);
>         }
> 
> So set_4byte() is not called for Spansion flashes. So I don't see need for this
> patch.
> 
> 
> >> There is patch to unset CONFIG_SPI_FLASH_BAR, for boards that are
> >> using 4-byte addressing mode with this flash
> >> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatc
> >>
> hwork.ozlabs.org%2Fpatch%2F1108287%2F&data=02%7C01%7Cashish.k
> umar
> >>
> %40nxp.com%7C035eff480dc046fe37c108d70b78630e%7C686ea1d3bc2b4c6f
> a92cd
> >>
> 99c5c301635%7C0%7C0%7C636990484001673615&sdata=dsD80nUUtmJ
> yOoxGvi
> >> iDFaOx1SKPXzI6bzXXpH630i8%3D&reserved=0
> >
> > Thanks for the information.
> >
> > Applied to u-boot-spi/master
> >
> 
> --
> Regards
> Vignesh


More information about the U-Boot mailing list