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

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@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) }, { INFO("s25fl512s_256k", 0x010220, 0x4d00, 256 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) }, { INFO("s25fl512s_64k", 0x010220, 0x4d01, 64 * 1024, 1024, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) }, { INFO("s25fl512s_512k", 0x010220, 0x4f00, 256 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },

+ Vignesh
On Wed, Jul 17, 2019 at 11:46 AM Ashish Kumar Ashish.Kumar@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@nxp.com
v3:
- Add version info, rebase to top.
- Re-word commit message.
v2:
- Adding more description in commit msg.
- 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.

-----Original Message----- From: Jagan Teki jagan@amarulasolutions.com Sent: Thursday, July 18, 2019 3:59 PM To: Ashish Kumar ashish.kumar@nxp.com; Vignesh R vigneshr@ti.com Cc: U-Boot-Denx u-boot@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@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@nxp.com
v3:
- Add version info, rebase to top.
- Re-word commit message.
v2:
- Adding more description in commit msg.
- 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().
There is patch to unset CONFIG_SPI_FLASH_BAR, for boards that are using 4-byte addressing mode with this flash http://patchwork.ozlabs.org/patch/1108287/
Regards Ashish

On Thu, Jul 18, 2019 at 4:15 PM Ashish Kumar ashish.kumar@nxp.com wrote:
-----Original Message----- From: Jagan Teki jagan@amarulasolutions.com Sent: Thursday, July 18, 2019 3:59 PM To: Ashish Kumar ashish.kumar@nxp.com; Vignesh R vigneshr@ti.com Cc: U-Boot-Denx u-boot@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@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@nxp.com
v3:
- Add version info, rebase to top.
- Re-word commit message.
v2:
- Adding more description in commit msg.
- 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().
There is patch to unset CONFIG_SPI_FLASH_BAR, for boards that are using 4-byte addressing mode with this flash http://patchwork.ozlabs.org/patch/1108287/
Thanks for the information.
Applied to u-boot-spi/master

On 18/07/19 4:29 PM, Jagan Teki wrote:
On Thu, Jul 18, 2019 at 4:15 PM Ashish Kumar ashish.kumar@nxp.com wrote:
-----Original Message----- From: Jagan Teki jagan@amarulasolutions.com Sent: Thursday, July 18, 2019 3:59 PM To: Ashish Kumar ashish.kumar@nxp.com; Vignesh R vigneshr@ti.com Cc: U-Boot-Denx u-boot@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@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@nxp.com
v3:
- Add version info, rebase to top.
- Re-word commit message.
v2:
- Adding more description in commit msg.
- 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) 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://patchwork.ozlabs.org/patch/1108287/
Thanks for the information.
Applied to u-boot-spi/master

-----Original Message----- From: Vignesh Raghavendra vigneshr@ti.com Sent: Thursday, July 18, 2019 5:37 PM To: Jagan Teki jagan@amarulasolutions.com; Ashish Kumar ashish.kumar@nxp.com Cc: U-Boot-Denx u-boot@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@nxp.com
wrote:
-----Original Message----- From: Jagan Teki jagan@amarulasolutions.com Sent: Thursday, July 18, 2019 3:59 PM To: Ashish Kumar ashish.kumar@nxp.com; Vignesh R
Cc: U-Boot-Denx u-boot@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
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@nxp.com
v3:
- Add version info, rebase to top.
- Re-word commit message.
v2:
- Adding more description in commit msg.
- 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&a...
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

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@nxp.com
v3:
- Add version info, rebase to top.
- Re-word commit message.
v2:
- Adding more description in commit msg.
- 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,
- 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?
- 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://www.jedec.org/standards-documents/docs/jep-137b)
SPI NOR flashes from Cypress/Spansion reads 0x1 for Manufacturer ID. This is based on JEDEC standard https://www.jedec.org/standards-documents/docs/jep-106ab
- 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.
Regards Vignesh
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

-----Original Message----- From: Vignesh Raghavendra vigneshr@ti.com Sent: Friday, July 19, 2019 11:41 PM To: Ashish Kumar ashish.kumar@nxp.com; Jagan Teki jagan@amarulasolutions.com; hs@denx.de Cc: U-Boot-Denx u-boot@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@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,
- 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?
- 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
- 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

On Mon, Jul 22, 2019 at 11:11 AM Ashish Kumar ashish.kumar@nxp.com wrote:
-----Original Message----- From: Vignesh Raghavendra vigneshr@ti.com Sent: Friday, July 19, 2019 11:41 PM To: Ashish Kumar ashish.kumar@nxp.com; Jagan Teki jagan@amarulasolutions.com; hs@denx.de Cc: U-Boot-Denx u-boot@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@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,
- 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?
- 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
- 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.
Will drop anyway.
participants (4)
-
Ashish Kumar
-
Ashish Kumar
-
Jagan Teki
-
Vignesh Raghavendra