| Message ID | 20180806143332.988-1-sr@denx.de |
|---|---|
| State | Superseded |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
| Series | [U-Boot] sf: Add SPI_FLASH_4BYTE_MODE_ONLY option to support 4-byte mode | expand |
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 >
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)
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(+)