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

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

[Andreas Fenkart]

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 | 91 ++++++++++++++++++++++-----------------------------------
 1 file changed, 35 insertions(+), 56 deletions(-)

-- 
2.10.1

[U-Boot] [PATCH 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 <andreas.fenkart@digitalstrom.com>
---
 common/env_sf.c | 47 ++++++++++++++++++-----------------------------
 1 file changed, 18 insertions(+), 29 deletions(-)

diff --git a/common/env_sf.c b/common/env_sf.c
index c53200f..5126762 100644
--- a/common/env_sf.c
+++ b/common/env_sf.c
@@ -45,13 +45,8 @@ 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;
 
@@ -76,6 +71,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 +251,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.10.1

[U-Boot] [PATCH 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 <andreas.fenkart@digitalstrom.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 5126762..ba9ac8a 100644
--- a/common/env_sf.c
+++ b/common/env_sf.c
@@ -175,12 +175,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);
@@ -315,10 +312,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.10.1

[U-Boot] [PATCH 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 <andreas.fenkart@digitalstrom.com>
---
 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 ba9ac8a..8a3de63 100644
--- a/common/env_sf.c
+++ b/common/env_sf.c
@@ -312,29 +312,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.10.1

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

[Andreas Fenkart]

simpler, needs less thinking when reading the code

Signed-off-by: Andreas Fenkart <andreas.fenkart@digitalstrom.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 8a3de63..0434bb8 100644
--- a/common/env_sf.c
+++ b/common/env_sf.c
@@ -79,7 +79,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();
@@ -114,11 +114,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,
@@ -244,7 +240,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;
@@ -267,16 +263,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.10.1

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

[Simon Glass]

On 28 November 2016 at 03:01, Andreas Fenkart
<andreas.fenkart@digitalstrom.com> wrote:
> copy&paste code found in single/double buffered code path
>
> Signed-off-by: Andreas Fenkart <andreas.fenkart@digitalstrom.com>
> ---
>  common/env_sf.c | 47 ++++++++++++++++++-----------------------------
>  1 file changed, 18 insertions(+), 29 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

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

[Simon Glass]

On 28 November 2016 at 03:01, Andreas Fenkart
<andreas.fenkart@digitalstrom.com> wrote:
> this makes it easier comparable to the double-buffered version
>
> Signed-off-by: Andreas Fenkart <andreas.fenkart@digitalstrom.com>
> ---
>  common/env_sf.c | 20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

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

[Tom Rini]

On Mon, Nov 28, 2016 at 11:01:14AM +0100, Andreas Fenkart wrote:

> copy&paste code found in single/double buffered code path
> 
> Signed-off-by: Andreas Fenkart <andreas.fenkart@digitalstrom.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Please test build this series on all platforms (perhaps via travis-ci)
as this breaks on ls1012afrdm_qspi and others, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161202/4651ce7a/attachment.sig>