[U-Boot] [PATCH] sf: Add auto detection of 4-byte mode (vs standard 3-byte mode)
Stefan Roese
sr at denx.de
Mon Oct 8 08:30:54 UTC 2018
Hi Simon,
On 05.10.2018 19:54, Simon Goldschmidt wrote:
> 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!
I've just sent out v2 of this patch with your STMicro support
added. Please give it a test and report if its okay (tested-by
tag).
Thanks,
Stefan
More information about the U-Boot
mailing list