[U-Boot] [PATCH v3] sf: Add auto detection of 4-byte mode (vs standard 3-byte mode)
Some SPI NOR chips only support 4-byte mode addressing. Here the default 3-byte mode does not work and leads to incorrect accesses. This patch now reads the 4-byte mode status bit (in this case in the CR register of the Macronix SPI NOR) and configures the SPI transfers accordingly.
This was noticed on the LinkIt Smart 7688 modul, which is equipped with an Macronix MX25L25635F device. But this device does *NOT* support switching to 3-byte mode via the EX4B command.
This should also work when the bootrom configures the SPI flash to 4-byte mode and runs U-Boot after this. U-Boot should dectect this mode (if the 4-byte mode detection is available for this chip) and use the correct OPs in this case.
Signed-off-by: Stefan Roese sr@denx.de Cc: Jagan Teki jagan@openedev.com Tested-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com --- v3: - Rebased on latest version (merge conflict because of new patches from Simon Glass) - Added Tested-by tag from Simon Goldschmidt
v2: - Integrated STMICRO 4-byte detection from Simon
drivers/mtd/spi/sf_internal.h | 3 +- drivers/mtd/spi/spi_flash.c | 131 ++++++++++++++++++++++++++++------ include/spi_flash.h | 5 ++ 3 files changed, 118 insertions(+), 21 deletions(-)
diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h index 4f63cacc64..eb076401d1 100644 --- a/drivers/mtd/spi/sf_internal.h +++ b/drivers/mtd/spi/sf_internal.h @@ -26,7 +26,8 @@ enum spi_nor_option_flags { };
#define SPI_FLASH_3B_ADDR_LEN 3 -#define SPI_FLASH_CMD_LEN (1 + SPI_FLASH_3B_ADDR_LEN) +#define SPI_FLASH_4B_ADDR_LEN 4 +#define SPI_FLASH_CMD_MAX_LEN (1 + SPI_FLASH_4B_ADDR_LEN) #define SPI_FLASH_16MB_BOUN 0x1000000
/* CFI Manufacture ID's */ diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c index 9230060364..b22eea2d1c 100644 --- a/drivers/mtd/spi/spi_flash.c +++ b/drivers/mtd/spi/spi_flash.c @@ -20,12 +20,19 @@
#include "sf_internal.h"
-static void spi_flash_addr(u32 addr, u8 *cmd) +static void spi_flash_addr(struct spi_flash *flash, u32 addr, u8 *cmd) { /* cmd[0] is actual command */ - cmd[1] = addr >> 16; - cmd[2] = addr >> 8; - cmd[3] = addr >> 0; + if (flash->in_4byte_mode) { + cmd[1] = addr >> 24; + cmd[2] = addr >> 16; + cmd[3] = addr >> 8; + cmd[4] = addr >> 0; + } else { + cmd[1] = addr >> 16; + cmd[2] = addr >> 8; + cmd[3] = addr >> 0; + } }
static int read_sr(struct spi_flash *flash, u8 *rs) @@ -110,6 +117,72 @@ static int write_cr(struct spi_flash *flash, u8 wc) } #endif
+#if defined(CONFIG_SPI_FLASH_MACRONIX) +static bool flash_in_4byte_mode_macronix(struct spi_flash *flash) +{ + int ret; + u8 cr; + u8 cmd; + + cmd = 0x15; /* Macronix: read configuration register RDCR */ + ret = spi_flash_read_common(flash, &cmd, 1, &cr, 1); + if (ret < 0) { + debug("SF: fail to read config register\n"); + return false; + } + + /* Return true, if 4-byte mode is enabled */ + if (cr & BIT(5)) + return true; + + return false; +} +#else +static bool flash_in_4byte_mode_macronix(struct spi_flash *flash) +{ + return false; +} +#endif + +#if defined(CONFIG_SPI_FLASH_STMICRO) +static bool flash_in_4byte_mode_stmicro(struct spi_flash *flash) +{ + int ret; + u8 fsr; + u8 cmd; + + cmd = 0x70; /* STMicro/Micron: read flag status register */ + ret = spi_flash_read_common(flash, &cmd, 1, &fsr, 1); + if (ret < 0) { + debug("SF: fail to read config register\n"); + return false; + } + + /* Return true, if 4-byte mode is enabled */ + if (fsr & BIT(0)) + return true; + + return false; +} +#else +static bool flash_in_4byte_mode_stmicro(struct spi_flash *flash) +{ + return false; +} +#endif + +static bool flash_in_4byte_mode(struct spi_flash *flash, + const struct spi_flash_info *info) +{ + if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX) + return flash_in_4byte_mode_macronix(flash); + + if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_STMICRO) + return flash_in_4byte_mode_stmicro(flash); + + return false; +} + #ifdef CONFIG_SPI_FLASH_BAR /* * This "clean_bar" is necessary in a situation when one was accessing @@ -314,7 +387,7 @@ int spi_flash_write_common(struct spi_flash *flash, const u8 *cmd, int spi_flash_cmd_erase_ops(struct spi_flash *flash, u32 offset, size_t len) { u32 erase_size, erase_addr; - u8 cmd[SPI_FLASH_CMD_LEN]; + u8 cmd[SPI_FLASH_CMD_MAX_LEN]; int ret = -1;
erase_size = flash->erase_size; @@ -344,12 +417,13 @@ int spi_flash_cmd_erase_ops(struct spi_flash *flash, u32 offset, size_t len) if (ret < 0) return ret; #endif - spi_flash_addr(erase_addr, cmd); + spi_flash_addr(flash, erase_addr, cmd);
debug("SF: erase %2x %2x %2x %2x (%x)\n", cmd[0], cmd[1], cmd[2], cmd[3], erase_addr);
- ret = spi_flash_write_common(flash, cmd, sizeof(cmd), NULL, 0); + ret = spi_flash_write_common(flash, cmd, flash->cmdlen, + NULL, 0); if (ret < 0) { debug("SF: erase failed\n"); break; @@ -373,7 +447,7 @@ int spi_flash_cmd_write_ops(struct spi_flash *flash, u32 offset, unsigned long byte_addr, page_size; u32 write_addr; size_t chunk_len, actual; - u8 cmd[SPI_FLASH_CMD_LEN]; + u8 cmd[SPI_FLASH_CMD_MAX_LEN]; int ret = -1;
page_size = flash->page_size; @@ -404,15 +478,15 @@ int spi_flash_cmd_write_ops(struct spi_flash *flash, u32 offset,
if (spi->max_write_size) chunk_len = min(chunk_len, - spi->max_write_size - sizeof(cmd)); + spi->max_write_size - flash->cmdlen);
- spi_flash_addr(write_addr, cmd); + spi_flash_addr(flash, write_addr, cmd);
debug("SF: 0x%p => cmd = { 0x%02x 0x%02x%02x%02x } chunk_len = %zu\n", buf + actual, cmd[0], cmd[1], cmd[2], cmd[3], chunk_len);
- ret = spi_flash_write_common(flash, cmd, sizeof(cmd), - buf + actual, chunk_len); + ret = spi_flash_write_common(flash, cmd, flash->cmdlen, + buf + actual, chunk_len); if (ret < 0) { debug("SF: write failed\n"); break; @@ -469,9 +543,11 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset, { struct spi_slave *spi = flash->spi; u8 cmdsz; - u32 remain_len, read_len, read_addr; + u64 remain_len; + u32 read_len, read_addr; int bank_sel = 0; int ret = 0; + int shift;
/* Handle memory-mapped SPI */ if (flash->memory_map) { @@ -487,7 +563,7 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset, return 0; }
- cmdsz = SPI_FLASH_CMD_LEN + flash->dummy_byte; + cmdsz = flash->cmdlen + flash->dummy_byte; u8 cmd[cmdsz];
cmd[0] = flash->read_cmd; @@ -504,8 +580,13 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset, return log_ret(ret); bank_sel = flash->bank_curr; #endif - remain_len = ((SPI_FLASH_16MB_BOUN << flash->shift) * - (bank_sel + 1)) - offset; + shift = flash->shift; + if (flash->in_4byte_mode) + shift += 8; + + remain_len = (((u64)SPI_FLASH_16MB_BOUN << shift) * + (bank_sel + 1)) - offset; + if (len < remain_len) read_len = len; else @@ -514,7 +595,7 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset, if (spi->max_read_size) read_len = min(read_len, spi->max_read_size);
- spi_flash_addr(read_addr, cmd); + spi_flash_addr(flash, read_addr, cmd);
ret = spi_flash_read_common(flash, cmd, cmdsz, data, read_len); if (ret < 0) { @@ -1155,6 +1236,13 @@ int spi_flash_scan(struct spi_flash *flash) write_sr(flash, sr); }
+ /* Set default value for cmd length */ + flash->cmdlen = 1 + SPI_FLASH_3B_ADDR_LEN; + if (flash_in_4byte_mode(flash, info)) { + flash->in_4byte_mode = true; + flash->cmdlen = 1 + SPI_FLASH_4B_ADDR_LEN; + } + flash->name = info->name; flash->memory_map = spi->memory_map;
@@ -1306,14 +1394,17 @@ int spi_flash_scan(struct spi_flash *flash) print_size(flash->size, ""); if (flash->memory_map) printf(", mapped at %p", flash->memory_map); + if (flash->in_4byte_mode) + printf(" (4-byte mode)"); puts("\n"); #endif
#ifndef CONFIG_SPI_FLASH_BAR - if (((flash->dual_flash == SF_SINGLE_FLASH) && - (flash->size > SPI_FLASH_16MB_BOUN)) || + if ((((flash->dual_flash == SF_SINGLE_FLASH) && + (flash->size > SPI_FLASH_16MB_BOUN)) || ((flash->dual_flash > SF_SINGLE_FLASH) && - (flash->size > SPI_FLASH_16MB_BOUN << 1))) { + (flash->size > SPI_FLASH_16MB_BOUN << 1))) && + !flash->in_4byte_mode) { puts("SF: Warning - Only lower 16MiB accessible,"); puts(" Full access #define CONFIG_SPI_FLASH_BAR\n"); } diff --git a/include/spi_flash.h b/include/spi_flash.h index 0ec98fb55d..b5bc4a85f6 100644 --- a/include/spi_flash.h +++ b/include/spi_flash.h @@ -36,6 +36,8 @@ struct spi_slave; * @dual_flash: Indicates dual flash memories - dual stacked, parallel * @shift: Flash shift useful in dual parallel * @flags: Indication of spi flash flags + * @in_4byte_mode: True if flash is detected to be in 4-byte mode + * @cmdlen: CMD length (3-byte vs 4-byte mode) * @size: Total flash size * @page_size: Write (page) size * @sector_size: Sector size @@ -69,6 +71,9 @@ struct spi_flash { u8 shift; u16 flags;
+ bool in_4byte_mode; + int cmdlen; + u32 size; u32 page_size; u32 sector_size;
Guys,
Let get back to the original thread. Since Rajat's first reply, the message id has been changed. All the comments were not captured by patchwork.
On 10/11/18 07:50, Stefan Roese wrote:
Some SPI NOR chips only support 4-byte mode addressing. Here the default 3-byte mode does not work and leads to incorrect accesses. This patch now reads the 4-byte mode status bit (in this case in the CR register of the Macronix SPI NOR) and configures the SPI transfers accordingly.
This was noticed on the LinkIt Smart 7688 modul, which is equipped with an Macronix MX25L25635F device. But this device does *NOT* support switching to 3-byte mode via the EX4B command.
This should also work when the bootrom configures the SPI flash to 4-byte mode and runs U-Boot after this. U-Boot should dectect this mode (if the 4-byte mode detection is available for this chip) and use the correct OPs in this case.
From what I read, Rajat's method is to extend the controller driver to
support read SFDP and default to 4-byte mode if supported, or overwritten by user's flag. Stefan's method is to read 4-byte status bit and doesn't change controller driver.
Is the default value of this 4-byte status bit valid and correct for all cases?
Rajat, without your patch set, does Stefan's solution work for your board?
York
Hi York,
On 25.10.18 17:49, York Sun wrote:
Guys,
Let get back to the original thread. Since Rajat's first reply, the message id has been changed. All the comments were not captured by patchwork.
I also wondered about this. Seems to have happened at some top-posting quite at the beginning of this thread.
On 10/11/18 07:50, Stefan Roese wrote:
Some SPI NOR chips only support 4-byte mode addressing. Here the default 3-byte mode does not work and leads to incorrect accesses. This patch now reads the 4-byte mode status bit (in this case in the CR register of the Macronix SPI NOR) and configures the SPI transfers accordingly.
This was noticed on the LinkIt Smart 7688 modul, which is equipped with an Macronix MX25L25635F device. But this device does *NOT* support switching to 3-byte mode via the EX4B command.
This should also work when the bootrom configures the SPI flash to 4-byte mode and runs U-Boot after this. U-Boot should dectect this mode (if the 4-byte mode detection is available for this chip) and use the correct OPs in this case.
From what I read, Rajat's method is to extend the controller driver to support read SFDP and default to 4-byte mode if supported, or overwritten by user's flag. Stefan's method is to read 4-byte status bit and doesn't change controller driver.
Yes, the last sentence it correct.
Is the default value of this 4-byte status bit valid and correct for all cases?
This 4-byte address status bit implementation is SPI NOR chip specific (vendor specific?). With Simon's help we already support 2 vendors now. Extending for other vendors / chips is fairly easy.
Thanks, Stefan
On 10/25/18 08:59, Stefan Roese wrote:
Hi York,
On 25.10.18 17:49, York Sun wrote:
Guys,
Let get back to the original thread. Since Rajat's first reply, the message id has been changed. All the comments were not captured by patchwork.
I also wondered about this. Seems to have happened at some top-posting quite at the beginning of this thread.
I am having trouble recently with office365. It seems breaking all my email threads if senders use outlook.
From what I read, Rajat's method is to extend the controller driver to support read SFDP and default to 4-byte mode if supported, or overwritten by user's flag. Stefan's method is to read 4-byte status bit and doesn't change controller driver.
Yes, the last sentence it correct.
Is the default value of this 4-byte status bit valid and correct for all cases?
This 4-byte address status bit implementation is SPI NOR chip specific (vendor specific?). With Simon's help we already support 2 vendors now. Extending for other vendors / chips is fairly easy.
Let's wait for Rajat's comment. If this method doesn't work for his boards or his controllers, he still needs to extend controller's driver feature.
York
-----Original Message----- From: York Sun Sent: Thursday, October 25, 2018 9:20 PM To: Stefan Roese sr@denx.de Cc: u-boot@lists.denx.de; Jagan Teki jagan@openedev.com; Rajat Srivastava rajat.srivastava@nxp.com; simon.k.r.goldschmidt@gmail.com; Ashish Kumar ashish.kumar@nxp.com Subject: Re: [U-Boot] [PATCH v3] sf: Add auto detection of 4-byte mode (vs standard 3-byte mode)
Guys,
Let get back to the original thread. Since Rajat's first reply, the message id has been changed. All the comments were not captured by patchwork.
On 10/11/18 07:50, Stefan Roese wrote:
Some SPI NOR chips only support 4-byte mode addressing. Here the
default
3-byte mode does not work and leads to incorrect accesses. This patch now reads the 4-byte mode status bit (in this case in the CR register of the Macronix SPI NOR) and configures the SPI transfers accordingly.
This was noticed on the LinkIt Smart 7688 modul, which is equipped with an Macronix MX25L25635F device. But this device does *NOT* support switching to 3-byte mode via the EX4B command.
This should also work when the bootrom configures the SPI flash to 4-byte mode and runs U-Boot after this. U-Boot should dectect this mode (if the 4-byte mode detection is available for this chip) and use the correct OPs in this case.
From what I read, Rajat's method is to extend the controller driver to support read SFDP and default to 4-byte mode if supported, or overwritten by user's flag. Stefan's method is to read 4-byte status bit and doesn't change controller driver.
Is the default value of this 4-byte status bit valid and correct for all cases?
Rajat, without your patch set, does Stefan's solution work for your board?
York
No. Stefan's changes are specific to his boards and is not applicable on ours. Stefan's patch is to support only certain flash that are factory strapped to work in 4-byte addressing modes only and will default to old method if such a flash is not found. The flashes on our boards (and also other vendor's board) will not work with Stefan's patch.
My patch can handle flashes with address widths of 3-byte, 4-byte or both. It also takes a more generic path (as opposed to supporting only specific flash models) by parsing SFDP standard parameters and then deciding what address width is to be used.
- Rajat
Hi Rajat,
On 26.10.18 11:59, Rajat Srivastava wrote:
<snip>
From what I read, Rajat's method is to extend the controller driver to support read SFDP and default to 4-byte mode if supported, or overwritten by user's flag. Stefan's method is to read 4-byte status bit and doesn't change controller driver.
Is the default value of this 4-byte status bit valid and correct for all cases?
Rajat, without your patch set, does Stefan's solution work for your board?
York
No. Stefan's changes are specific to his boards and is not applicable on ours. Stefan's patch is to support only certain flash that are factory strapped to work in 4-byte addressing modes only and will default to old method if such a flash is not found.
That is not 100% correct. The flash does not need to be "factory strapped" to this 4-byte mode. It needs to be configured to this 4-byte mode. And this is also the case for example, when the ROM bootloader configures the flash this way or a soft-reboot (without hard flash chip reset) reboots into U-Boot after the flash was configured to 4-byte mode in Linux.
And it supports multiple vendors and can easily be extended to support more. The only thing needed is the detection of the 3-byte vs 4-byte address mode. But please note that it currently does not actively switch from 3-byte to 4-byte mode. This could be added though if needed / wanted.
The flashes on our boards (and also other vendor's board) will not work with Stefan's patch.
My patch can handle flashes with address widths of 3-byte, 4-byte or both. It also takes a more generic path (as opposed to supporting only specific flash models) by parsing SFDP standard parameters and then deciding what address width is to be used.
I still don't see why each and every SPI controller driver needs to be extended to support this SFDP parameter reading. Can't this be handled by some generic (read common) code part in the SPI flash infrastructure?
Thanks, Stefan
Hi Stefan
-----Original Message----- From: Stefan Roese sr@denx.de Sent: Friday, October 26, 2018 3:42 PM To: Rajat Srivastava rajat.srivastava@nxp.com; York Sun york.sun@nxp.com; Jagan Teki jagan@openedev.com Cc: u-boot@lists.denx.de; simon.k.r.goldschmidt@gmail.com; Ashish Kumar ashish.kumar@nxp.com Subject: Re: [U-Boot] [PATCH v3] sf: Add auto detection of 4-byte mode (vs standard 3-byte mode)
Hi Rajat,
On 26.10.18 11:59, Rajat Srivastava wrote:
<snip>
From what I read, Rajat's method is to extend the controller driver to support read SFDP and default to 4-byte mode if supported, or overwritten by user's flag. Stefan's method is to read 4-byte status bit and doesn't change controller driver.
Is the default value of this 4-byte status bit valid and correct for all cases?
Rajat, without your patch set, does Stefan's solution work for your board?
York
No. Stefan's changes are specific to his boards and is not applicable on ours. Stefan's patch is to support only certain flash that are factory strapped to work in 4-byte addressing modes only and will default to old method if such a flash is not found.
That is not 100% correct. The flash does not need to be "factory strapped" to this 4-byte mode. It needs to be configured to this 4-byte mode. And this is also the case for example, when the ROM bootloader configures the flash this way or a soft-reboot (without hard flash chip reset) reboots into U-Boot after the flash was configured to 4-byte mode in Linux.
And it supports multiple vendors and can easily be extended to support more. The only thing needed is the detection of the 3-byte vs 4-byte address mode. But please note that it currently does not actively switch from 3-byte to 4-byte mode. This could be added though if needed / wanted.
Spansion flashes (on our boards) support both 3-byte and 4-byte addressing modes which needs active switching. What could be added to your patch is already supported in SFDP method. SFDP also provides information about page size, flash density, read/write/erase commands, etc, in addition to getting information about address width supported by flash.
The flashes on our boards (and also other vendor's board) will not work with Stefan's patch.
My patch can handle flashes with address widths of 3-byte, 4-byte or both. It also takes a more generic path (as opposed to supporting only specific flash models) by parsing SFDP standard parameters and then deciding what address width is to be used.
I still don't see why each and every SPI controller driver needs to be extended to support this SFDP parameter reading. Can't this be handled by some generic (read common) code part in the SPI flash infrastructure?
The generic (read common) code part in the SPI flash infrastructure also lands on respective SPI drivers which ultimately sends the commands to flash. Currently every SPI driver has support for basic read command (which is called after "generic read common code") but no driver has support for SFDP reading, which is what needs to be added. Also, only the drivers that want to make use of SFDP needs to extend support for SFDP parameter reading.
I am reiterating that this is how SFDP parsing has been implemented in Linux as well and this is the generic way. Driver support is absolutely required because SPI framework cannot directly send any command to any flash.
Thanks Rajat
Thanks, Stefan
On 10/26/18 03:36, Rajat Srivastava wrote:
<snip>
Spansion flashes (on our boards) support both 3-byte and 4-byte addressing modes which needs active switching. What could be added to your patch is already supported in SFDP method. SFDP also provides information about page size, flash density, read/write/erase commands, etc, in addition to getting information about address width supported by flash.
The flashes on our boards (and also other vendor's board) will not work with Stefan's patch.
My patch can handle flashes with address widths of 3-byte, 4-byte or both. It also takes a more generic path (as opposed to supporting only specific flash models) by parsing SFDP standard parameters and then deciding what address width is to be used.
I still don't see why each and every SPI controller driver needs to be extended to support this SFDP parameter reading. Can't this be handled by some generic (read common) code part in the SPI flash infrastructure?
The generic (read common) code part in the SPI flash infrastructure also lands on respective SPI drivers which ultimately sends the commands to flash. Currently every SPI driver has support for basic read command (which is called after "generic read common code") but no driver has support for SFDP reading, which is what needs to be added. Also, only the drivers that want to make use of SFDP needs to extend support for SFDP parameter reading.
I am reiterating that this is how SFDP parsing has been implemented in Linux as well and this is the generic way. Driver support is absolutely required because SPI framework cannot directly send any command to any flash.
It sounds like your difference is how to detect 4-byte addressing should be used.
Stefan's method is from the flash chip side, to check status register. The benefit is to preserve setting before U-Boot. I am not sure if it is necessary to preserve previous setting though.
Rajat's method is from the controller side, to read SFDP. It can support all flash chips I presume.
I guess Stefan's method will not determine Rajat's flash as 4-byte addressing because it indeed supports both 3- and 4-byte. To make Rajat's method work on Stefan's board, SFDP needs to be extended to the controller driver Stefan uses, is that right?
I also guess reading SFDP is controller-specific and cannot be done in a generic way. If the new feature is useful and can be gradually extended to other controllers, it sounds good to me. My guesses could wrong though.
York
participants (3)
-
Rajat Srivastava -
Stefan Roese -
York Sun