> -----Original Message----- > From: Vignesh Raghavendra <[email protected]> > Sent: Friday, July 19, 2019 11:41 PM > To: Ashish Kumar <[email protected]>; Jagan Teki > <[email protected]>; [email protected] > Cc: U-Boot-Denx <[email protected]> > 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 <[email protected]> > >>>>>> --- > >>>>>> 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& > ;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 _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
