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 <ja...@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.
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
>
> /**
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
