[U-Boot] [PATCH v2 0/4] env_sf: minor refactorings

[U-Boot] [PATCH v2 0/4] env_sf: minor refactorings

[Andreas Fenkart]

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(-)

-- 
2.11.0

[U-Boot] [PATCH v2 1/4] env_sf: factor out prepare_flash_device

[Andreas Fenkart]

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) {
-- 
2.11.0

[U-Boot] [PATCH v2 2/4] enf_sf: reuse setup_flash_device instead of open coding it

[Andreas Fenkart]

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;
-- 
2.11.0

[U-Boot] [PATCH v2 3/4] env_sf: re-order error handling in single-buffer env_relocate_spec

[Andreas Fenkart]

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
 
-- 
2.11.0

[U-Boot] [PATCH v2 4/4] env_sf: use DIV_ROUND_UP to calculate number of sectors to erase

[Andreas Fenkart]

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);
-- 
2.11.0

[U-Boot] [PATCH v2 2/4] enf_sf: reuse setup_flash_device instead of open coding it

[Simon Glass]

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>

[U-Boot] [PATCH v2 3/4] env_sf: re-order error handling in single-buffer env_relocate_spec

[Simon Glass]

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>

[U-Boot] [PATCH v2 4/4] env_sf: use DIV_ROUND_UP to calculate number of sectors to erase

[Simon Glass]

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>

[U-Boot] [PATCH v2 0/4] env_sf: minor refactorings

[Jagan Teki]

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!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.