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

Ashish Kumar ashish.kumar at nxp.com
Mon Jul 22 05:41:34 UTC 2019 


> -----Original Message-----
> From: Vignesh Raghavendra <vigneshr at ti.com>
> Sent: Friday, July 19, 2019 11:41 PM
> To: Ashish Kumar <ashish.kumar at nxp.com>; Jagan Teki
> <jagan at amarulasolutions.com>; hs at denx.de
> 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
> 
> >>>>>> 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)
> 
> So its just a quirky flash with different manf ID? If manuf ID reads different
> then how does it match s25fl512s entry in the spi-nor-ids?
> 
> > 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)."
> > <<<<<
> 
> Cypress seems to use different ID for CFI NOR flash (on Parallel NOR interface
> or HyperBus interface). Thats a different standard (JEDEC CFI ID codes
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.j
> edec.org%2Fstandards-documents%2Fdocs%2Fjep-
> 137b&data=02%7C01%7Cashish.kumar%40nxp.com%7Cd21e92a1fe4541
> 8eae2608d70c7486b4%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
> C636991566928021371&sdata=1i7GEVXSIk2i5%2F%2FXuhXdan1%2BByRt
> yNZrVltINAka1sI%3D&reserved=0)
> 
> SPI NOR flashes from Cypress/Spansion reads 0x1 for Manufacturer ID.
> This is based on JEDEC standard
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.j
> edec.org%2Fstandards-documents%2Fdocs%2Fjep-
> 106ab&data=02%7C01%7Cashish.kumar%40nxp.com%7Cd21e92a1fe454
> 18eae2608d70c7486b4%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%
> 7C636991566928021371&sdata=OA1ivCTeCO%2BIQsYFEZc5dMxEcETSIIt9
> yZu%2FU%2FWQBfI%3D&reserved=0
> 
> > 3. s25fl064l, s25fl128l flash id has SPI_NOR_4B_OPCODES, what is the use
> of this flag here then ?
> >
> 
> That was an overlook when those were added to Linux spi-nor code and
> became part of U-Boot during syncing of framework.
> 
> I am not saying that adding SPI_NOR_4B_OPCODES is wrong, because
> Spansion flashes does support stateless 4 byte addressing opcodes. But I am
> saying that it should not be needed in the first place (as you can see from
> code snippets). If you ever need to add SPI_NOR_4B_OPCODES to spansion
> flashes (with manf ID 0x1) then I think there is a bug somewhere in the code
> which needs to be debugged and understood.
Hi Vignesh ,

Agreed.
Should I send a negative patch to remove my patch for spansion flash.
Or it will be simply drop from spi-master by Jagan.

Regards
Ashish 
> 
> Regards
> Vignesh
> 
> > [1]:
> >
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.
> >
> google.com%2Furl%3Fsa%3Dt%26rct%3Dj%26q%3D%26esrc%3Ds%26source%
> 3Dweb%2
> >
> 6cd%3D1%26cad%3Drja%26uact%3D8%26ved%3D2ahUKEwjHpY3U0b7jAhVE
> WX0KHeqSAQ
> >
> kQFjAAegQIBBAB%26url%3Dhttp%253A%252F%252Fwww.cypress.com%252Ffi
> le%252
> >
> F195291%252Fdownload%26usg%3DAOvVaw0zPEXOjP80J8XVrwmB7TBS&amp
> ;data=02%
> >
> 7C01%7Cashish.kumar%40nxp.com%7Cd21e92a1fe45418eae2608d70c7486b4
> %7C686
> >
> ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636991566928021371&
> sdata=M
> >
> GPbuuu%2FmSWvgbJqs8H23r6hzjs0nm8kelQBby7%2F1Kk%3D&reserved
> =0
> >
> > 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 http://patc
> >>>>
> >>
> 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