[U-Boot] [PATCH] sf: Add auto detection of 4-byte mode (vs standard 3-byte mode)
Simon Goldschmidt
simon.k.r.goldschmidt at gmail.com
Fri Oct 5 17:54:13 UTC 2018
On 04.10.2018 17:16, 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
S/dectect/detect/
> 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 at denx.de>
> Cc: Jagan Teki <jagan at openedev.com>
> Cc: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>
I've successfully tested this on my socfpga_cyclone5 board. But I had to
implement the detection for 'stmicro' flash. See below.
> ---
> Please note that this patch replaces this one:
>
> [PATCH] sf: Add SPI_FLASH_4BYTE_MODE_ONLY option to support 4-byte mode
>
> which introduced a CONFIG option to enable this 4-byte mode. In discussion
> with Simon we agreed, that an auto detection would be much better. Hence
> this new version now.
>
> drivers/mtd/spi/sf_internal.h | 3 +-
> drivers/mtd/spi/spi_flash.c | 94 +++++++++++++++++++++++++++--------
> include/spi_flash.h | 5 ++
> 3 files changed, 81 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 c159124259..357b4f2422 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,33 @@ static int write_cr(struct spi_flash *flash, u8 wc)
> }
> #endif
>
> +#if defined(CONFIG_SPI_FLASH_MACRONIX)
> +static bool flash_in_4byte_mode(struct spi_flash *flash)
I renamed this function 'flash_in_4byte_mode_macronix()' as the current
generic function name gives me name clashes when compiling with MACRONIX
and STMICRO enabled, see below.
> +{
> + 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
Here, I've added this function for STMICRO:
#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;
}
printf("flash_in_4byte_mode_stmicro(): fsr: 0x%02x\n", fsr);
/* Return true, if 4-byte mode is enabled */
if (fsr & BIT(0)) {
return true;
}
return false;
}
#endif
> +static bool flash_in_4byte_mode(struct spi_flash *flash)
> +{
> + return false;
> +}
> +#endif
> +
> #ifdef CONFIG_SPI_FLASH_BAR
> /*
> * This "clean_bar" is necessary in a situation when one was accessing
> @@ -314,7 +348,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 +378,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 +408,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 +439,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 +504,11 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
> {
> struct spi_slave *spi = flash->spi;
> u8 *cmd, cmdsz;
> - u32 remain_len, read_len, read_addr;
> + u64 remain_len;
> + u32 read_len, read_addr;
> int bank_sel = 0;
> int ret = -1;
> + int shift;
>
> /* Handle memory-mapped SPI */
> if (flash->memory_map) {
> @@ -487,7 +524,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;
> cmd = calloc(1, cmdsz);
> if (!cmd) {
> debug("SF: Failed to allocate cmd\n");
> @@ -508,8 +545,13 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
> return 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
> @@ -518,7 +560,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) {
> @@ -1160,6 +1202,15 @@ 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 (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX) {
> + if (flash_in_4byte_mode(flash)) {
> + flash->in_4byte_mode = true;
> + flash->cmdlen = 1 + SPI_FLASH_4B_ADDR_LEN;
> + }
> + }
And here, I inserted:
#if defined(CONFIG_SPI_FLASH_STMICRO)
if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_STMICRO) {
if (flash_in_4byte_mode_stmicro(flash)) {
flash->in_4byte_mode = true;
flash->cmdlen = 1 + SPI_FLASH_4B_ADDR_LEN;
}
}
#endif
After that, it works fine. I'd probably always call
'flash_in_4byte_mode()' and implement the vendor check in that function
to make 'spi_flash_scan()' easier to read...
I'd be happy if you could integrate this into your patch as a v2!
Simon
> +
> flash->name = info->name;
> flash->memory_map = spi->memory_map;
>
> @@ -1311,14 +1362,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;
More information about the U-Boot
mailing list