[U-Boot] [PATCH 0/5] Clear error flags & initialization of Spansion

This patchset updates serial flash initialization and clear error flags for Spansion SPI flash.
Testing these patches has been done on S25FL128S and High speed SPI Controller that do switch back & forth between different modes (legacy, dual, quad).
* spi_flash_scan: Since Spansion has BP# bits & QEB on the same register, preserving the original values is recommended. Otherwise, unstable behavior may occur. e.g. if the last mode used by the SPI controller was Quad read, while the new scan should use legacy. In such a case, garbage bytes may be read instead.
* clear_sr: A new function that is used to clear status register-1 of Spansion when write or erase error occur. Clearing SR is recommended to avoid problems with the next operation(s), such as Timeout! while it is already caused by the last operation but not the current one.
TODO: If applicable, the clear_sr() should be done for the other flash vendors too.
Ahmed Samir Khalil (5): mtd: spi: Define CLSR command mtd: spi_flash: Support clearing status register mtd: spi_flash: Clear SR if write/erase error is flagged mtd: spi_flash: Clear SR error flags in case of failed write mtd: spi_flash: preserve Spansion's original value during reboot
drivers/mtd/spi/sf_internal.h | 3 +++ drivers/mtd/spi/spi_flash.c | 30 +++++++++++++++++++++++++++++- 2 files changed, 32 insertions(+), 1 deletion(-)

Define clear status register command to be used in error handling.
Signed-off-by: Ahmed S. Khalil engkhalil86@gmail.com --- drivers/mtd/spi/sf_internal.h | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h index 839cdbe..42f2b20 100644 --- a/drivers/mtd/spi/sf_internal.h +++ b/drivers/mtd/spi/sf_internal.h @@ -71,6 +71,9 @@ enum spi_nor_option_flags { # define CMD_EXTNADDR_RDEAR 0xC8 #endif
+/* Register access commands */ +#define CMD_CLEAR_STATUS 0x30 + /* Common status */ #define STATUS_WIP BIT(0) #define STATUS_QEB_WINSPAN BIT(1)

A function to clear status register-1 after error flag(s) being triggered.
Signed-off-by: Ahmed S. Khalil engkhalil86@gmail.com --- drivers/mtd/spi/spi_flash.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c index 34f6888..52dcb84 100644 --- a/drivers/mtd/spi/spi_flash.c +++ b/drivers/mtd/spi/spi_flash.c @@ -110,6 +110,27 @@ static int write_cr(struct spi_flash *flash, u8 wc)
return 0; } + +/* + * Clear status register-1 + * + * TODO: Check validity for the other flash vendors. + */ +static int clear_sr(struct spi_flash *flash) +{ + struct spi_slave *spi = flash->spi; + u8 cmd, buf; + int ret; + + cmd = CMD_CLEAR_STATUS; + ret = spi_flash_cmd_write(spi, cmd, 1, buf, 1); + if (ret < 0) { + debug("SF: fail to clear status register\n"); + return ret; + } + + return ret; +} #endif
#ifdef CONFIG_SPI_FLASH_BAR

On Sun, Oct 1, 2017 at 6:50 AM, Ahmed Samir Khalil engkhalil86@gmail.com wrote:
A function to clear status register-1 after error flag(s) being triggered.
Signed-off-by: Ahmed S. Khalil engkhalil86@gmail.com
drivers/mtd/spi/spi_flash.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c index 34f6888..52dcb84 100644 --- a/drivers/mtd/spi/spi_flash.c +++ b/drivers/mtd/spi/spi_flash.c @@ -110,6 +110,27 @@ static int write_cr(struct spi_flash *flash, u8 wc)
return 0;
}
+/*
- Clear status register-1
- TODO: Check validity for the other flash vendors.
- */
+static int clear_sr(struct spi_flash *flash) +{
struct spi_slave *spi = flash->spi;
u8 cmd, buf;
int ret;
cmd = CMD_CLEAR_STATUS;
ret = spi_flash_cmd_write(spi, cmd, 1, buf, 1);
This is wrong, we should have clear status value to clear the status, and why we need this operation? doesn't mentioned on commit message? and all these 5 patches does same job better to squash as one resend.
thanks!

In case of write or erase error, flags are being triggered into status register-1. The flags should be cleared, otherwise they may lead to unstable behavior for the following operation(s).
Signed-off-by: Ahmed S. Khalil engkhalil86@gmail.com --- drivers/mtd/spi/spi_flash.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c index 52dcb84..6f54e10 100644 --- a/drivers/mtd/spi/spi_flash.c +++ b/drivers/mtd/spi/spi_flash.c @@ -353,6 +353,9 @@ int spi_flash_cmd_erase_ops(struct spi_flash *flash, u32 offset, size_t len) ret = spi_flash_write_common(flash, cmd, sizeof(cmd), NULL, 0); if (ret < 0) { debug("SF: erase failed\n"); +#ifdef CONFIG_SPI_FLASH_SPANSION + clear_sr(flash); +#endif break; }

If writing fail, error that is flagged into status register-1 should be cleared. Otherwise, next operation(s) may be done incorrectly/incomplete.
Signed-off-by: Ahmed S. Khalil engkhalil86@gmail.com --- drivers/mtd/spi/spi_flash.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c index 6f54e10..b6b56fe 100644 --- a/drivers/mtd/spi/spi_flash.c +++ b/drivers/mtd/spi/spi_flash.c @@ -415,6 +415,9 @@ int spi_flash_cmd_write_ops(struct spi_flash *flash, u32 offset, buf + actual, chunk_len); if (ret < 0) { debug("SF: write failed\n"); +#ifdef CONFIG_SPI_FLASH_SPANSION + clear_sr(flash); +#endif break; }

Spansion flash has QEB in the same status register as well. Therefore, preserve the original values while rebooting. Otherwise, some SPI controllers may be unable to access the flash correctly e.g. receive garbage bytes instead.
Signed-off-by: Ahmed S. Khalil engkhalil86@gmail.com --- drivers/mtd/spi/spi_flash.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c index b6b56fe..2e603ed 100644 --- a/drivers/mtd/spi/spi_flash.c +++ b/drivers/mtd/spi/spi_flash.c @@ -984,7 +984,8 @@ int spi_flash_scan(struct spi_flash *flash) */ if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_ATMEL || (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_SST) || - (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX)) { + (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX) || + (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_SPANSION)) { u8 sr = 0;
if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX) {
participants (2)
-
Ahmed Samir Khalil
-
Jagan Teki