[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