[U-Boot] [PATCH v10 20/27] mtd: spi-nor: Add 4-byte addresswidth support

[Cyrille Pitchen]

Le 28/12/2017 à 09:06, Cyrille Pitchen a écrit :
> Hi Jagan,
> 
> Le 28/12/2017 à 07:12, Jagan Teki a écrit :
>> Add 4-byte address supports, so-that SPI-NOR chips
>> has > 16MiB should accessible.
>>
>> Signed-off-by: Jagan Teki <jagan at amarulasolutions.com>
>> ---
>>  drivers/mtd/spi-nor/m25p80.c  |  1 +
>>  drivers/mtd/spi-nor/spi-nor.c | 38 ++++++++++++++++++++++++++++++++++++++
>>  include/linux/mtd/spi-nor.h   |  6 +++++-
>>  3 files changed, 44 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/spi-nor/m25p80.c b/drivers/mtd/spi-nor/m25p80.c
>> index 5465921..7af6f59 100644
>> --- a/drivers/mtd/spi-nor/m25p80.c
>> +++ b/drivers/mtd/spi-nor/m25p80.c
>> @@ -35,6 +35,7 @@ static void m25p_addr2cmd(struct spi_nor *nor, unsigned int addr, u8 *cmd)
>>  	cmd[1] = addr >> (nor->addr_width * 8 -  8);
>>  	cmd[2] = addr >> (nor->addr_width * 8 - 16);
>>  	cmd[3] = addr >> (nor->addr_width * 8 - 24);
>> +	cmd[4] = addr >> (nor->addr_width * 8 - 32);
>>  }
>>  
>>  static int m25p_cmdsz(struct spi_nor *nor)
>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>> index 8bf9e67..e0085cf 100644
>> --- a/drivers/mtd/spi-nor/spi-nor.c
>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>> @@ -632,6 +632,38 @@ static int stm_unlock(struct udevice *dev, loff_t ofs, uint64_t len)
>>  }
>>  #endif
>>  
>> +/* Enable/disable 4-byte addressing mode. */
>> +static int set_4byte(struct udevice *dev, const struct spi_nor_info *info,
>> +		     int enable)
>> +{
>> +	struct spi_nor *nor = spi_nor_get_spi_nor_dev(dev);
>> +	const struct spi_nor_ops *ops = spi_nor_get_ops(dev);
>> +	int status;
>> +	bool need_wren = false;
>> +	u8 cmd;
>> +
>> +	switch (JEDEC_MFR(info)) {
>> +	case SNOR_MFR_MICRON:
>> +		/* Some Micron need WREN command; all will accept it */
>> +		need_wren = true;
>> +	case SNOR_MFR_MACRONIX:
>> +	case SNOR_MFR_WINBOND:
>> +		if (need_wren)
>> +			write_enable(dev);
>> +
>> +		cmd = enable ? SNOR_OP_EN4B : SNOR_OP_EX4B;
>> +		status = ops->write_reg(dev, cmd, NULL, 0);
>> +		if (need_wren)
>> +			write_disable(dev);
>> +
>> +		return status;
>> +	default:
>> +		/* Spansion style */
>> +		nor->cmd_buf[0] = enable << 7;
>> +		return ops->write_reg(dev, SNOR_OP_BRWR, nor->cmd_buf, 1);
>> +	}
>> +}
>> +
>>  #ifdef CONFIG_SPI_NOR_MACRONIX
>>  static int macronix_quad_enable(struct udevice *dev)
>>  {
>> @@ -873,6 +905,12 @@ int spi_nor_scan(struct spi_nor *nor)
>>  	}
>>  
>>  	nor->addr_width = 3;
>> +	if (mtd->size > SNOR_16MB_BOUN) {
>> +		nor->addr_width = 4;
>> +		ret = set_4byte(nor->dev, info, true);
>> +		if (ret)
>> +			goto err;
>> +	}
>>
> 
> This is a bad idea: you make the SPI NOR memory enter its 4-byte address
> mode, which is statefull. Then, once in Linux for instance, the memory
> would still be in its 4-byte address mode but expected to be in its reset
> state, hence in 3-byte address mode.
> 
> At this point, this might not be an issue yet because spi-nor under linux
> is likely to use the 4-byte address instruction set if available, which is
> stateless by the way, so whatever the memory is in its 3-byte or in 4-byte
> address mode, the op codes of the 4-byte address instruction set are always
> expected to be followed by a 4-byte address. So Linux won't notice that the
> SPI NOR memory is in its 4-byte mode but not in its 3-byte mode as expected.
> 
> However, if a spurious reboot occurs at the CPU side after the SPI NOR
> memory has entered its 4-byte address mode, either still in u-boot or
> already in linux or whatever running code, the SPI NOR memory, likely not
> reset on its side, would still be in its 4-byte address mode, whereas many
> earlier boot-loaders would expect the memory to be in it's reset state, ie
> in its 3-byte address mode. Hence those boot-loaders, running before u-boot,
> would not be able to read data (load u-boot) from the SPI NOR memory:
> the boot would fail!
> 
> Examples of those early boot-loaders are the ROM Codes of many Atmel
> SAMA5Dx SoCs but I'm pretty sure this issue also applies to other
> manufacturers based on patches I've seen on the linux-mtd mailing list to
> add the SPI_NOR_4B_OPCODES flag to some memory parts:
> https://patchwork.ozlabs.org/patch/750305/
> 
> Think about a watchdog resetting the CPU for instance.
> 
> 
> TL;DR
> You should avoid making the SPI NOR memory enter its statefull 4-byte
> address mode but consider using the stateless 4-byte address instruction
> set instead, when possible.
> 
> Almost all recent SPI NOR memories with sizes > 16MiB support the 4-byte
> address mode.

I meant the 4-byte address instruction set ;)
> 
> Best regards,
> 
> Cyrille
>   
>>  	/* Dummy cycles for read */
>>  	switch (nor->read_opcode) {
>> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
>> index e1688e2..fc4a649 100644
>> --- a/include/linux/mtd/spi-nor.h
>> +++ b/include/linux/mtd/spi-nor.h
>> @@ -63,6 +63,10 @@
>>  #define SNOR_OP_BP		0x02	/* Byte program */
>>  #define SNOR_OP_AAI_WP		0xad	/* Auto addr increment word program */
>>  
>> +/* Used for Macronix and Winbond flashes. */
>> +#define SNOR_OP_EN4B		0xb7    /* Enter 4-byte mode */
>> +#define SNOR_OP_EX4B		0xe9    /* Exit 4-byte mode */
>> +
>>  /* Status Register bits. */
>>  #define SR_WIP			BIT(0)	/* Write in progress */
>>  #define SR_WEL			BIT(1)	/* Write enable latch */
>> @@ -84,7 +88,7 @@
>>  /* Flash timeout values */
>>  #define SNOR_READY_WAIT_PROG	(2 * CONFIG_SYS_HZ)
>>  #define SNOR_READY_WAIT_ERASE	(5 * CONFIG_SYS_HZ)
>> -#define SNOR_MAX_CMD_SIZE	4
>> +#define SNOR_MAX_CMD_SIZE	6
>>  #define SNOR_16MB_BOUN		0x1000000
>>  
>>  /**
>>
>