| Message ID | 20181127200943.955-1-simon.k.r.goldschmidt@gmail.com |
|---|---|
| State | Superseded, archived |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
| Series | [U-Boot] sf: prevent clean_bar overwriting error codes | expand |
On Tue, 27 Nov 2018 at 13:09, Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> wrote: > > In spi_flash.c, if CONFIG_SPI_FLASH_BAR is enabled, the function > 'clean_bar' makes sure that the Bank Address Register is reset at the > end of functions using it. > > However, if this is enabled, those functions may return zero (success) > when they should return an error. This is because after e.g. > 'spi_flash_read_common' fails, the return value in 'ret' is overwritten > with the return value of 'clean_bar'. > > Fix this by changing 'clean_bar' to take the outer error code into > account and returning its own return value only if the outer error code > is 0. > > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> > --- > > drivers/mtd/spi/spi_flash.c | 21 +++++++++++++++------ > 1 file changed, 15 insertions(+), 6 deletions(-) Reviewed-by: Simon Glass <sjg@chromium.org>
Am 06.12.2018 um 02:31 schrieb Simon Glass: > On Tue, 27 Nov 2018 at 13:09, Simon Goldschmidt > <simon.k.r.goldschmidt@gmail.com> wrote: >> >> In spi_flash.c, if CONFIG_SPI_FLASH_BAR is enabled, the function >> 'clean_bar' makes sure that the Bank Address Register is reset at the >> end of functions using it. >> >> However, if this is enabled, those functions may return zero (success) >> when they should return an error. This is because after e.g. >> 'spi_flash_read_common' fails, the return value in 'ret' is overwritten >> with the return value of 'clean_bar'. >> >> Fix this by changing 'clean_bar' to take the outer error code into >> account and returning its own return value only if the outer error code >> is 0. >> >> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> >> --- >> >> drivers/mtd/spi/spi_flash.c | 21 +++++++++++++++------ >> 1 file changed, 15 insertions(+), 6 deletions(-) > > Reviewed-by: Simon Glass <sjg@chromium.org> This can be dropped due to Vignesh's series that updates SPI NOR. Simon
diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c index 0c2392f28a..b13b9f19b7 100644 --- a/drivers/mtd/spi/spi_flash.c +++ b/drivers/mtd/spi/spi_flash.c @@ -132,17 +132,26 @@ int spi_flash_cmd_get_sw_write_prot(struct spi_flash *flash) * * Otherwise, the BA24 bit may be left set and then after reset, the * ROM would read/write/erase SPL from 16 MiB * bank_sel address. + * + * Since this function is always called, it handles outer error codes + * to take care calling functions don't return 0 just because setting + * BAR succeeded. */ -static int clean_bar(struct spi_flash *flash) +static int clean_bar(struct spi_flash *flash, int outer_err) { + int ret; u8 cmd, bank_sel = 0; if (flash->bank_curr == 0) - return 0; + return outer_err; cmd = flash->bank_write_cmd; flash->bank_curr = 0; - return spi_flash_write_common(flash, &cmd, 1, &bank_sel, 1); + ret = spi_flash_write_common(flash, &cmd, 1, &bank_sel, 1); + if (outer_err) + return outer_err; + + return ret; } static int write_bar(struct spi_flash *flash, u32 offset) @@ -372,7 +381,7 @@ int spi_flash_cmd_erase_ops(struct spi_flash *flash, u32 offset, size_t len) } #ifdef CONFIG_SPI_FLASH_BAR - ret = clean_bar(flash); + ret = clean_bar(flash, ret); #endif return ret; @@ -434,7 +443,7 @@ int spi_flash_cmd_write_ops(struct spi_flash *flash, u32 offset, } #ifdef CONFIG_SPI_FLASH_BAR - ret = clean_bar(flash); + ret = clean_bar(flash, ret); #endif return ret; @@ -540,7 +549,7 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset, } #ifdef CONFIG_SPI_FLASH_BAR - ret = clean_bar(flash); + ret = clean_bar(flash, ret); #endif return log_ret(ret);
In spi_flash.c, if CONFIG_SPI_FLASH_BAR is enabled, the function 'clean_bar' makes sure that the Bank Address Register is reset at the end of functions using it. However, if this is enabled, those functions may return zero (success) when they should return an error. This is because after e.g. 'spi_flash_read_common' fails, the return value in 'ret' is overwritten with the return value of 'clean_bar'. Fix this by changing 'clean_bar' to take the outer error code into account and returning its own return value only if the outer error code is 0. Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> --- drivers/mtd/spi/spi_flash.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-)