[U-Boot] [PATCH v2 0/4] env_sf: minor refactorings
rebased patches on master fixed compiled error in 1st patch (missing variable declaration) compile tested all patches with travis
Andreas Fenkart (4): env_sf: factor out prepare_flash_device enf_sf: reuse setup_flash_device instead of open coding it env_sf: re-order error handling in single-buffer env_relocate_spec env_sf: use DIV_ROUND_UP to calculate number of sectors to erase
common/env_sf.c | 92 ++++++++++++++++++++++----------------------------------- 1 file changed, 36 insertions(+), 56 deletions(-)
copy&paste code found in single/double buffered code path
Signed-off-by: Andreas Fenkart afenkart@gmail.com Reviewed-by: Simon Glass sjg@chromium.org --- common/env_sf.c | 48 +++++++++++++++++++----------------------------- 1 file changed, 19 insertions(+), 29 deletions(-)
diff --git a/common/env_sf.c b/common/env_sf.c index 27b4d1226a..8af590a3d9 100644 --- a/common/env_sf.c +++ b/common/env_sf.c @@ -45,15 +45,11 @@ char *env_name_spec = "SPI Flash";
static struct spi_flash *env_flash;
-#if defined(CONFIG_ENV_OFFSET_REDUND) -int saveenv(void) +static int setup_flash_device(void) { - env_t env_new; - char *saved_buffer = NULL, flag = OBSOLETE_FLAG; - u32 saved_size, saved_offset, sector = 1; - int ret; #ifdef CONFIG_DM_SPI_FLASH struct udevice *new; + int ret;
/* speed and mode will be read from DT */ ret = spi_flash_probe_bus_cs(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS, @@ -76,6 +72,20 @@ int saveenv(void) } } #endif + return 0; +} + +#if defined(CONFIG_ENV_OFFSET_REDUND) +int saveenv(void) +{ + env_t env_new; + char *saved_buffer = NULL, flag = OBSOLETE_FLAG; + u32 saved_size, saved_offset, sector = 1; + int ret; + + ret = setup_flash_device(); + if (ret) + return ret;
ret = env_export(&env_new); if (ret) @@ -242,30 +252,10 @@ int saveenv(void) char *saved_buffer = NULL; int ret = 1; env_t env_new; -#ifdef CONFIG_DM_SPI_FLASH - struct udevice *new;
- /* speed and mode will be read from DT */ - ret = spi_flash_probe_bus_cs(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS, - 0, 0, &new); - if (ret) { - set_default_env("!spi_flash_probe_bus_cs() failed"); - return 1; - } - - env_flash = dev_get_uclass_priv(new); -#else - - if (!env_flash) { - env_flash = spi_flash_probe(CONFIG_ENV_SPI_BUS, - CONFIG_ENV_SPI_CS, - CONFIG_ENV_SPI_MAX_HZ, CONFIG_ENV_SPI_MODE); - if (!env_flash) { - set_default_env("!spi_flash_probe() failed"); - return 1; - } - } -#endif + ret = setup_flash_device(); + if (ret) + return ret;
/* Is the sector larger than the env (i.e. embedded) */ if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
setup_flash_device selects one of two code paths depending on the driver model being used (=CONFIG_DM_SPI_FLASH). env_relocate_spec only used the non driver-model code path. I'm unsure why, either none of the platforms that need relocation use the driver model, or - worse - the driver model is not yet usable when relocating.
Signed-off-by: Andreas Fenkart afenkart@gmail.com --- common/env_sf.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/common/env_sf.c b/common/env_sf.c index 8af590a3d9..a52fb734c8 100644 --- a/common/env_sf.c +++ b/common/env_sf.c @@ -176,12 +176,9 @@ void env_relocate_spec(void) goto out; }
- env_flash = spi_flash_probe(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS, - CONFIG_ENV_SPI_MAX_HZ, CONFIG_ENV_SPI_MODE); - if (!env_flash) { - set_default_env("!spi_flash_probe() failed"); + ret = setup_flash_device(); + if (ret) goto out; - }
ret = spi_flash_read(env_flash, CONFIG_ENV_OFFSET, CONFIG_ENV_SIZE, tmp_env1); @@ -316,10 +313,9 @@ void env_relocate_spec(void) char *buf = NULL;
buf = (char *)memalign(ARCH_DMA_MINALIGN, CONFIG_ENV_SIZE); - env_flash = spi_flash_probe(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS, - CONFIG_ENV_SPI_MAX_HZ, CONFIG_ENV_SPI_MODE); - if (!env_flash) { - set_default_env("!spi_flash_probe() failed"); + + ret = setup_flash_device(); + if (ret) { if (buf) free(buf); return;
On 8 April 2017 at 03:59, Andreas Fenkart afenkart@gmail.com wrote:
setup_flash_device selects one of two code paths depending on the driver model being used (=CONFIG_DM_SPI_FLASH). env_relocate_spec only used the non driver-model code path. I'm unsure why, either none of the platforms that need relocation use the driver model, or - worse - the driver model is not yet usable when relocating.
Signed-off-by: Andreas Fenkart afenkart@gmail.com
common/env_sf.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
this makes it easier comparable to the double-buffered version
Signed-off-by: Andreas Fenkart afenkart@gmail.com Reviewed-by: Simon Glass sjg@chromium.org --- common/env_sf.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/common/env_sf.c b/common/env_sf.c index a52fb734c8..6a1583ebec 100644 --- a/common/env_sf.c +++ b/common/env_sf.c @@ -313,29 +313,31 @@ void env_relocate_spec(void) char *buf = NULL;
buf = (char *)memalign(ARCH_DMA_MINALIGN, CONFIG_ENV_SIZE); - - ret = setup_flash_device(); - if (ret) { - if (buf) - free(buf); + if (!buf) { + set_default_env("!malloc() failed"); return; }
+ ret = setup_flash_device(); + if (ret) + goto out; + ret = spi_flash_read(env_flash, CONFIG_ENV_OFFSET, CONFIG_ENV_SIZE, buf); if (ret) { set_default_env("!spi_flash_read() failed"); - goto out; + goto err_read; }
ret = env_import(buf, 1); if (ret) gd->env_valid = 1; -out: + +err_read: spi_flash_free(env_flash); - if (buf) - free(buf); env_flash = NULL; +out: + free(buf); } #endif
On 8 April 2017 at 03:59, Andreas Fenkart afenkart@gmail.com wrote:
this makes it easier comparable to the double-buffered version
Signed-off-by: Andreas Fenkart afenkart@gmail.com Reviewed-by: Simon Glass sjg@chromium.org
common/env_sf.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
simpler to read
Signed-off-by: Andreas Fenkart afenkart@gmail.com --- common/env_sf.c | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-)
diff --git a/common/env_sf.c b/common/env_sf.c index 6a1583ebec..9944602367 100644 --- a/common/env_sf.c +++ b/common/env_sf.c @@ -80,7 +80,7 @@ int saveenv(void) { env_t env_new; char *saved_buffer = NULL, flag = OBSOLETE_FLAG; - u32 saved_size, saved_offset, sector = 1; + u32 saved_size, saved_offset, sector; int ret;
ret = setup_flash_device(); @@ -115,11 +115,7 @@ int saveenv(void) goto done; }
- if (CONFIG_ENV_SIZE > CONFIG_ENV_SECT_SIZE) { - sector = CONFIG_ENV_SIZE / CONFIG_ENV_SECT_SIZE; - if (CONFIG_ENV_SIZE % CONFIG_ENV_SECT_SIZE) - sector++; - } + sector = DIV_ROUND_UP(CONFIG_ENV_SIZE, CONFIG_ENV_SECT_SIZE);
puts("Erasing SPI flash..."); ret = spi_flash_erase(env_flash, env_new_offset, @@ -245,7 +241,7 @@ out: #else int saveenv(void) { - u32 saved_size, saved_offset, sector = 1; + u32 saved_size, saved_offset, sector; char *saved_buffer = NULL; int ret = 1; env_t env_new; @@ -268,16 +264,12 @@ int saveenv(void) goto done; }
- if (CONFIG_ENV_SIZE > CONFIG_ENV_SECT_SIZE) { - sector = CONFIG_ENV_SIZE / CONFIG_ENV_SECT_SIZE; - if (CONFIG_ENV_SIZE % CONFIG_ENV_SECT_SIZE) - sector++; - } - ret = env_export(&env_new); if (ret) goto done;
+ sector = DIV_ROUND_UP(CONFIG_ENV_SIZE, CONFIG_ENV_SECT_SIZE); + puts("Erasing SPI flash..."); ret = spi_flash_erase(env_flash, CONFIG_ENV_OFFSET, sector * CONFIG_ENV_SECT_SIZE);
On 8 April 2017 at 03:59, Andreas Fenkart afenkart@gmail.com wrote:
simpler to read
Signed-off-by: Andreas Fenkart afenkart@gmail.com
common/env_sf.c | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
On Sat, Apr 8, 2017 at 3:29 PM, Andreas Fenkart afenkart@gmail.com wrote:
rebased patches on master fixed compiled error in 1st patch (missing variable declaration) compile tested all patches with travis
Andreas Fenkart (4): env_sf: factor out prepare_flash_device enf_sf: reuse setup_flash_device instead of open coding it env_sf: re-order error handling in single-buffer env_relocate_spec env_sf: use DIV_ROUND_UP to calculate number of sectors to erase
Reviewed-by: Jagan Teki jagan@openedev.com Tested-by: Jagan Teki jagan@openedev.com
Applied to u-boot-spi/next
thanks!
participants (3)
-
Andreas Fenkart -
Jagan Teki -
Simon Glass