[U-Boot] [PATCH] sf: Add SPI_FLASH_4BYTE_MODE_ONLY option to support 4-byte mode

Some SPI NOR chips only support 4-byte mode addressing. Here the default 3-byte mode does not work and leads to incorrect accesses. Setting this option enables the use of such SPI NOR chips, that only support this 4-byte mode.
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.
Signed-off-by: Stefan Roese sr@denx.de Cc: Jagan Teki jagan@openedev.com --- drivers/mtd/spi/Kconfig | 9 +++++++++ drivers/mtd/spi/sf_internal.h | 5 +++++ drivers/mtd/spi/spi_flash.c | 7 +++++++ 3 files changed, 21 insertions(+)
diff --git a/drivers/mtd/spi/Kconfig b/drivers/mtd/spi/Kconfig index 4484cf8195..5738cd66e8 100644 --- a/drivers/mtd/spi/Kconfig +++ b/drivers/mtd/spi/Kconfig @@ -49,6 +49,15 @@ config SF_DUAL_FLASH Enable this option to support two flash memories connected to a single controller. Currently Xilinx Zynq qspi supports this.
+config SPI_FLASH_4BYTE_MODE_ONLY + bool "SPI 4-byte mode only supported" + depends on SPI_FLASH + help + Some SPI NOR chips only support 4-byte mode addressing. Here + the default 3-byte mode does not work and leads to incorrect + accesses. Setting this option enables the use of such SPI + NOR chips, that only support this 4-byte mode. + if SPI_FLASH
config SPI_FLASH_ATMEL diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h index 4f63cacc64..78be6e442f 100644 --- a/drivers/mtd/spi/sf_internal.h +++ b/drivers/mtd/spi/sf_internal.h @@ -26,7 +26,12 @@ enum spi_nor_option_flags { };
#define SPI_FLASH_3B_ADDR_LEN 3 +#define SPI_FLASH_4B_ADDR_LEN 4 +#ifdef CONFIG_SPI_FLASH_4BYTE_MODE_ONLY +#define SPI_FLASH_CMD_LEN (1 + SPI_FLASH_4B_ADDR_LEN) +#else #define SPI_FLASH_CMD_LEN (1 + SPI_FLASH_3B_ADDR_LEN) +#endif #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..3b26d8ca88 100644 --- a/drivers/mtd/spi/spi_flash.c +++ b/drivers/mtd/spi/spi_flash.c @@ -23,9 +23,16 @@ static void spi_flash_addr(u32 addr, u8 *cmd) { /* cmd[0] is actual command */ +#ifdef CONFIG_SPI_FLASH_4BYTE_MODE_ONLY + 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; +#endif }
static int read_sr(struct spi_flash *flash, u8 *rs)

Stefan Roese sr@denx.de schrieb am Mo., 6. Aug. 2018, 16:34:
Some SPI NOR chips only support 4-byte mode addressing. Here the default 3-byte mode does not work and leads to incorrect accesses. Setting this option enables the use of such SPI NOR chips, that only support this 4-byte mode.
I think it would make more sense to enable 4-byte mode or 4-byte opcodes on all chips with more than 16 mbyte rather than having to select at compile time.
Simon
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.
Signed-off-by: Stefan Roese sr@denx.de Cc: Jagan Teki jagan@openedev.com
drivers/mtd/spi/Kconfig | 9 +++++++++ drivers/mtd/spi/sf_internal.h | 5 +++++ drivers/mtd/spi/spi_flash.c | 7 +++++++ 3 files changed, 21 insertions(+)
diff --git a/drivers/mtd/spi/Kconfig b/drivers/mtd/spi/Kconfig index 4484cf8195..5738cd66e8 100644 --- a/drivers/mtd/spi/Kconfig +++ b/drivers/mtd/spi/Kconfig @@ -49,6 +49,15 @@ config SF_DUAL_FLASH Enable this option to support two flash memories connected to a single controller. Currently Xilinx Zynq qspi supports this.
+config SPI_FLASH_4BYTE_MODE_ONLY
bool "SPI 4-byte mode only supported"
depends on SPI_FLASH
help
Some SPI NOR chips only support 4-byte mode addressing. Here
the default 3-byte mode does not work and leads to incorrect
accesses. Setting this option enables the use of such SPI
NOR chips, that only support this 4-byte mode.
if SPI_FLASH
config SPI_FLASH_ATMEL diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h index 4f63cacc64..78be6e442f 100644 --- a/drivers/mtd/spi/sf_internal.h +++ b/drivers/mtd/spi/sf_internal.h @@ -26,7 +26,12 @@ enum spi_nor_option_flags { };
#define SPI_FLASH_3B_ADDR_LEN 3 +#define SPI_FLASH_4B_ADDR_LEN 4 +#ifdef CONFIG_SPI_FLASH_4BYTE_MODE_ONLY +#define SPI_FLASH_CMD_LEN (1 + SPI_FLASH_4B_ADDR_LEN) +#else #define SPI_FLASH_CMD_LEN (1 + SPI_FLASH_3B_ADDR_LEN) +#endif #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..3b26d8ca88 100644 --- a/drivers/mtd/spi/spi_flash.c +++ b/drivers/mtd/spi/spi_flash.c @@ -23,9 +23,16 @@ static void spi_flash_addr(u32 addr, u8 *cmd) { /* cmd[0] is actual command */ +#ifdef CONFIG_SPI_FLASH_4BYTE_MODE_ONLY
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; +#endif }
static int read_sr(struct spi_flash *flash, u8 *rs)
2.18.0
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot

Hi Simon,
On 06.08.2018 17:15, Simon Goldschmidt wrote:
Stefan Roese <sr@denx.de mailto:sr@denx.de> schrieb am Mo., 6. Aug. 2018, 16:34:
Some SPI NOR chips only support 4-byte mode addressing. Here the default 3-byte mode does not work and leads to incorrect accesses. Setting this option enables the use of such SPI NOR chips, that only support this 4-byte mode.
I think it would make more sense to enable 4-byte mode or 4-byte opcodes on all chips with more than 16 mbyte rather than having to select at compile time.
We need to be careful here. As setting the chip into 4-byte mode unconditionally (for bigger devices) will very likely cause boot problems with internal bootROMs expecting 3-byte mode.
As noted in the commit message, this is a very special case, where the SPI NOR only supports 4-byte opcodes / mode.
Another idea would be to check the 3-byte / 4-byte mode of the SPI NOR device upon SPI NOR driver loading and use the selected mode accordingly. This could be done without compile time options but it would not help in general for users with bigger SPI NOR devices that support both modes.
Thanks, Stefan

Stefan Roese sr@denx.de schrieb am Mo., 6. Aug. 2018, 17:23:
Hi Simon,
On 06.08.2018 17:15, Simon Goldschmidt wrote:
Stefan Roese <sr@denx.de mailto:sr@denx.de> schrieb am Mo., 6. Aug. 2018, 16:34:
Some SPI NOR chips only support 4-byte mode addressing. Here the
default
3-byte mode does not work and leads to incorrect accesses. Setting
this
option enables the use of such SPI NOR chips, that only support this 4-byte mode.
I think it would make more sense to enable 4-byte mode or 4-byte opcodes on all chips with more than 16 mbyte rather than having to select at compile time.
We need to be careful here. As setting the chip into 4-byte mode unconditionally (for bigger devices) will very likely cause boot problems with internal bootROMs expecting 3-byte mode.
I have a similar problem on socfpga where Linux 4.9 sets the chip into 4-byte mode and SPL cannot use it on warm reboot. However, the bootROM does not run on warm reboot on this platform.
I just think it would be good to solve these related problems in a more generic way.
Simon
As noted in the commit message, this is a very special case, where the SPI NOR only supports 4-byte opcodes / mode.
Another idea would be to check the 3-byte / 4-byte mode of the SPI NOR device upon SPI NOR driver loading and use the selected mode accordingly. This could be done without compile time options but it would not help in general for users with bigger SPI NOR devices that support both modes.
Thanks, Stefan

On 06.08.2018 17:27, Simon Goldschmidt wrote:
Stefan Roese <sr@denx.de mailto:sr@denx.de> schrieb am Mo., 6. Aug. 2018, 17:23:
Hi Simon, On 06.08.2018 17:15, Simon Goldschmidt wrote: > > > Stefan Roese <sr@denx.de <mailto:sr@denx.de> <mailto:sr@denx.de <mailto:sr@denx.de>>> schrieb am Mo., 6. Aug. > 2018, 16:34: > > Some SPI NOR chips only support 4-byte mode addressing. Here the default > 3-byte mode does not work and leads to incorrect accesses. Setting this > option enables the use of such SPI NOR chips, that only support this > 4-byte mode. > > > I think it would make more sense to enable 4-byte mode or 4-byte opcodes > on all chips with more than 16 mbyte rather than having to select at > compile time. We need to be careful here. As setting the chip into 4-byte mode unconditionally (for bigger devices) will very likely cause boot problems with internal bootROMs expecting 3-byte mode.
I have a similar problem on socfpga where Linux 4.9 sets the chip into 4-byte mode and SPL cannot use it on warm reboot. However, the bootROM does not run on warm reboot on this platform.
So in your "warm reboot" case, my option b) below would help, right?
b) Another idea would be to check the 3-byte / 4-byte mode of the SPI NOR device upon SPI NOR driver loading and use the selected mode accordingly. This could be done without compile time options but it would not help in general for users with bigger SPI NOR devices that support both modes.
Thanks, Stefan

Stefan Roese sr@denx.de schrieb am Mo., 6. Aug. 2018, 17:29:
On 06.08.2018 17:27, Simon Goldschmidt wrote:
Stefan Roese <sr@denx.de mailto:sr@denx.de> schrieb am Mo., 6. Aug. 2018, 17:23:
Hi Simon, On 06.08.2018 17:15, Simon Goldschmidt wrote: > > > Stefan Roese <sr@denx.de <mailto:sr@denx.de> <mailto:sr@denx.de <mailto:sr@denx.de>>> schrieb am Mo., 6. Aug. > 2018, 16:34: > > Some SPI NOR chips only support 4-byte mode addressing. Here the default > 3-byte mode does not work and leads to incorrect accesses. Setting this > option enables the use of such SPI NOR chips, that only support this > 4-byte mode. > > > I think it would make more sense to enable 4-byte mode or 4-byte opcodes > on all chips with more than 16 mbyte rather than having to select
at
> compile time. We need to be careful here. As setting the chip into 4-byte mode unconditionally (for bigger devices) will very likely cause boot problems with internal bootROMs expecting 3-byte mode.
I have a similar problem on socfpga where Linux 4.9 sets the chip into 4-byte mode and SPL cannot use it on warm reboot. However, the bootROM does not run on warm reboot on this platform.
So in your "warm reboot" case, my option b) below would help, right?
Yes, I guess it would.
An even better idea would be to use stateless 4-byte opcodes. That's what Linux does in more recent versions. But that would be a larger patch, I guess. And with Jagan's bigger rework in his queue, I can't tell if this is a good time to do so.
b) Another idea would be to check the 3-byte / 4-byte mode of the SPI NOR device upon SPI NOR driver loading and use the selected mode accordingly. This could be done without compile time options but it would not help in general for users with bigger SPI NOR devices that support both modes.
Thanks, Stefan

On 06.08.2018 17:33, Simon Goldschmidt wrote:
Stefan Roese <sr@denx.de mailto:sr@denx.de> schrieb am Mo., 6. Aug. 2018, 17:29:
On 06.08.2018 17:27, Simon Goldschmidt wrote: > > > Stefan Roese <sr@denx.de <mailto:sr@denx.de> <mailto:sr@denx.de <mailto:sr@denx.de>>> schrieb am Mo., 6. Aug. > 2018, 17:23: > > Hi Simon, > > On 06.08.2018 17:15, Simon Goldschmidt wrote: > > > > > > Stefan Roese <sr@denx.de <mailto:sr@denx.de> <mailto:sr@denx.de <mailto:sr@denx.de>> <mailto:sr@denx.de <mailto:sr@denx.de> > <mailto:sr@denx.de <mailto:sr@denx.de>>>> schrieb am Mo., 6. Aug. > > 2018, 16:34: > > > > Some SPI NOR chips only support 4-byte mode addressing. Here > the default > > 3-byte mode does not work and leads to incorrect accesses. > Setting this > > option enables the use of such SPI NOR chips, that only > support this > > 4-byte mode. > > > > > > I think it would make more sense to enable 4-byte mode or 4-byte > opcodes > > on all chips with more than 16 mbyte rather than having to select at > > compile time. > > We need to be careful here. As setting the chip into 4-byte mode > unconditionally (for bigger devices) will very likely cause boot > problems with internal bootROMs expecting 3-byte mode. > > > I have a similar problem on socfpga where Linux 4.9 sets the chip into > 4-byte mode and SPL cannot use it on warm reboot. However, the bootROM > does not run on warm reboot on this platform. So in your "warm reboot" case, my option b) below would help, right?
Yes, I guess it would.
An even better idea would be to use stateless 4-byte opcodes. That's what Linux does in more recent versions. But that would be a larger patch, I guess. And with Jagan's bigger rework in his queue, I can't tell if this is a good time to do so.
Yes, this is definitely out of scope for me, reworking this 4-byte support to this stateless version. Sorry. But if we agree that the option b) is helpful and harmless (which I think it is), then I can rework this patch and re-send a version which should also work for you - as described below.
b) Another idea would be to check the 3-byte / 4-byte mode of the SPI NOR device upon SPI NOR driver loading and use the selected mode accordingly. This could be done without compile time options but it would not help in general for users with bigger SPI NOR devices that support both modes. Thanks, Stefan
Thanks, Stefan

Stefan Roese sr@denx.de schrieb am Mo., 6. Aug. 2018, 17:36:
On 06.08.2018 17:33, Simon Goldschmidt wrote:
Stefan Roese <sr@denx.de mailto:sr@denx.de> schrieb am Mo., 6. Aug. 2018, 17:29:
On 06.08.2018 17:27, Simon Goldschmidt wrote: > > > Stefan Roese <sr@denx.de <mailto:sr@denx.de> <mailto:sr@denx.de <mailto:sr@denx.de>>> schrieb am Mo., 6. Aug. > 2018, 17:23: > > Hi Simon, > > On 06.08.2018 17:15, Simon Goldschmidt wrote: > > > > > > Stefan Roese <sr@denx.de <mailto:sr@denx.de> <mailto:sr@denx.de <mailto:sr@denx.de>> <mailto:sr@denx.de <mailto:sr@denx.de> > <mailto:sr@denx.de <mailto:sr@denx.de>>>> schrieb am Mo., 6.
Aug.
> > 2018, 16:34: > > > > Some SPI NOR chips only support 4-byte mode addressing. Here > the default > > 3-byte mode does not work and leads to incorrect
accesses.
> Setting this > > option enables the use of such SPI NOR chips, that only > support this > > 4-byte mode. > > > > > > I think it would make more sense to enable 4-byte mode or 4-byte > opcodes > > on all chips with more than 16 mbyte rather than having to select at > > compile time. > > We need to be careful here. As setting the chip into 4-byte
mode
> unconditionally (for bigger devices) will very likely cause
boot
> problems with internal bootROMs expecting 3-byte mode. > > > I have a similar problem on socfpga where Linux 4.9 sets the chip into > 4-byte mode and SPL cannot use it on warm reboot. However, the bootROM > does not run on warm reboot on this platform. So in your "warm reboot" case, my option b) below would help, right?
Yes, I guess it would.
An even better idea would be to use stateless 4-byte opcodes. That's what Linux does in more recent versions. But that would be a larger patch, I guess. And with Jagan's bigger rework in his queue, I can't tell if this is a good time to do so.
Yes, this is definitely out of scope for me, reworking this 4-byte support to this stateless version.
Right. For me too.
Sorry. But if we agree that the
option b) is helpful and harmless (which I think it is), then I can rework this patch and re-send a version which should also work for you - as described below.
That would be cool. I'd test this on my socrates board.
Thanks, Simon
b) Another idea would be to check the 3-byte / 4-byte mode of the SPI NOR device upon SPI NOR driver loading and use the selected mode accordingly. This could be done without compile time options but it would not help in general for users with bigger SPI NOR devices that support both modes. Thanks, Stefan
Thanks, Stefan
participants (2)
-
Simon Goldschmidt
-
Stefan Roese