[U-Boot] [PATCH 0/3] Serial Flash: call spi_flash_free more coherently

Some board require spi_flash_free to be called after all the accesses, in order, for instance, to restore the pin multiplexing configuration in the case where the SPI pins are multiplexed.
This patch series tries to enhance this. Patch 1 adds spi_flash_free calls to env_sf so that the SPI interface is always "cleaned up" after the env read/writes. Patch 2 adds a 'sf release' command that implicitly calls spi_flash_free and is thus the pendant of 'sf probe'. Patch 3 uses the 'sf command' for the km_arm board scripts.
Valentin Longchamp (3): env_sf: generalize call to spi_flash_free after accesses cmd_sf: add 'release' command km_arm: call 'sf release' in the newenv and update scripts
common/cmd_sf.c | 13 ++++++++++++- common/env_sf.c | 34 ++++++++++++++++------------------ include/configs/km/km_arm.h | 6 ++++-- 3 files changed, 32 insertions(+), 21 deletions(-)

Some board require spi_flash_free to be called after all the accesses, in order, for instance, to restore the pin multiplexing configuration in the case where the SPI pins are multiplexed.
This was done only in env_relocate_spec and not in saveenv. saveenv is thus changed in order to have the same behavior as env_relocate_spec.
Since the static env_flash variable will be NULL after every function call, it is thus removed and its functionality is replaced by a systematic call to spi_flash_probe at the start of both env_relocate_spec and saveenv.
Signed-off-by: Valentin Longchamp valentin.longchamp@keymile.com --- common/env_sf.c | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-)
diff --git a/common/env_sf.c b/common/env_sf.c index 9f806fb..dd0e675 100644 --- a/common/env_sf.c +++ b/common/env_sf.c @@ -41,8 +41,6 @@ DECLARE_GLOBAL_DATA_PTR;
char *env_name_spec = "SPI Flash";
-static struct spi_flash *env_flash; - #if defined(CONFIG_ENV_OFFSET_REDUND) int saveenv(void) { @@ -50,16 +48,14 @@ int saveenv(void) ssize_t len; char *res, *saved_buffer = NULL, flag = OBSOLETE_FLAG; u32 saved_size, saved_offset, sector = 1; + struct spi_flash *env_flash = NULL; int ret;
+ 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) { - 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; - } + set_default_env("!spi_flash_probe() failed"); + return 1; }
res = (char *)&env_new.data; @@ -135,6 +131,8 @@ int saveenv(void) if (saved_buffer) free(saved_buffer);
+ spi_flash_free(env_flash); + return ret; }
@@ -145,6 +143,7 @@ void env_relocate_spec(void) env_t *tmp_env1 = NULL; env_t *tmp_env2 = NULL; env_t *ep = NULL; + struct spi_flash *env_flash = NULL;
tmp_env1 = (env_t *)malloc(CONFIG_ENV_SIZE); tmp_env2 = (env_t *)malloc(CONFIG_ENV_SIZE); @@ -216,7 +215,6 @@ void env_relocate_spec(void)
err_read: spi_flash_free(env_flash); - env_flash = NULL; out: free(tmp_env1); free(tmp_env2); @@ -229,15 +227,13 @@ int saveenv(void) int ret = 1; env_t env_new; ssize_t len; + struct spi_flash *env_flash = NULL;
+ 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) { - 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; - } + set_default_env("!spi_flash_probe() failed"); + return 1; }
/* Is the sector larger than the env (i.e. embedded) */ @@ -294,12 +290,15 @@ int saveenv(void) if (saved_buffer) free(saved_buffer);
+ spi_flash_free(env_flash); + return ret; }
void env_relocate_spec(void) { char buf[CONFIG_ENV_SIZE]; + struct spi_flash *env_flash = NULL; int ret;
env_flash = spi_flash_probe(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS, @@ -321,7 +320,6 @@ void env_relocate_spec(void) gd->env_valid = 1; out: spi_flash_free(env_flash); - env_flash = NULL; } #endif

The release command is the pendant of the probe command. This command allows to call spi_flash_free from the command line. This may be necessary for some boards where sf probe does change the state of the hardware (like with some pin multiplexing changes for instance).
Signed-off-by: Valentin Longchamp valentin.longchamp@keymile.com --- common/cmd_sf.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/common/cmd_sf.c b/common/cmd_sf.c index 4af0f0a..b8ef3f4 100644 --- a/common/cmd_sf.c +++ b/common/cmd_sf.c @@ -133,6 +133,14 @@ static int do_spi_flash_probe(int argc, char * const argv[]) return 0; }
+static int do_spi_flash_release(int argc, char * const argv[]) +{ + if (flash) + spi_flash_free(flash); + flash = NULL; + + return 0; +} /** * Write a block of data to SPI flash, first checking if it is different from * what is already there. @@ -501,6 +509,8 @@ static int do_spi_flash(cmd_tbl_t *cmdtp, int flag, int argc, ret = do_spi_flash_read_write(argc, argv); else if (strcmp(cmd, "erase") == 0) ret = do_spi_flash_erase(argc, argv); + else if (strcmp(cmd, "release") == 0) + ret = do_spi_flash_release(argc, argv); #ifdef CONFIG_CMD_SF_TEST else if (!strcmp(cmd, "test")) ret = do_spi_flash_test(argc, argv); @@ -535,6 +545,7 @@ U_BOOT_CMD( "sf erase offset [+]len - erase `len' bytes from `offset'\n" " `+len' round up `len' to block size\n" "sf update addr offset len - erase and write `len' bytes from memory\n" - " at `addr' to flash at `offset'" + " at `addr' to flash at `offset'\n" + "sf release - release the current flash device" SF_TEST_HELP );

This is necessary to make sure that all the pins used for SPI access, especially the CS, are configured back to the NAND Flash interface. Otherwise, if either newenv or update are called, u-boot cannot access the NAND Flash anymore.
Signed-off-by: Valentin Longchamp valentin.longchamp@keymile.com --- include/configs/km/km_arm.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/include/configs/km/km_arm.h b/include/configs/km/km_arm.h index e0368cb..3304315 100644 --- a/include/configs/km/km_arm.h +++ b/include/configs/km/km_arm.h @@ -269,13 +269,15 @@ int get_scl(void); #define CONFIG_KM_UPDATE_UBOOT \ "update=" \ "sf probe 0;sf erase 0 +${filesize};" \ - "sf write ${load_addr_r} 0 ${filesize};\0" + "sf write ${load_addr_r} 0 ${filesize};" \ + "sf release\0"
#if defined CONFIG_KM_ENV_IS_IN_SPI_NOR #define CONFIG_KM_NEW_ENV \ "newenv=sf probe 0;" \ "sf erase " __stringify(CONFIG_ENV_OFFSET) " " \ - __stringify(CONFIG_ENV_TOTAL_SIZE)"\0" + __stringify(CONFIG_ENV_TOTAL_SIZE)";" \ + "sf release\0" #else #define CONFIG_KM_NEW_ENV \ "newenv=setenv addr 0x100000 && " \

On 09/17/2013 08:46 AM, Valentin Longchamp wrote:
Some board require spi_flash_free to be called after all the accesses, in order, for instance, to restore the pin multiplexing configuration in the case where the SPI pins are multiplexed.
This patch series tries to enhance this. Patch 1 adds spi_flash_free calls to env_sf so that the SPI interface is always "cleaned up" after the env read/writes. Patch 2 adds a 'sf release' command that implicitly calls spi_flash_free and is thus the pendant of 'sf probe'. Patch 3 uses the 'sf command' for the km_arm board scripts.
Valentin Longchamp (3): env_sf: generalize call to spi_flash_free after accesses cmd_sf: add 'release' command km_arm: call 'sf release' in the newenv and update scripts
common/cmd_sf.c | 13 ++++++++++++- common/env_sf.c | 34 ++++++++++++++++------------------ include/configs/km/km_arm.h | 6 ++++-- 3 files changed, 32 insertions(+), 21 deletions(-)
I have sent this series 4 months ago and while it is according to patchwork under review I have not received any feedback yet.
That would be nice if this goes into the next merge window since they have been floating around for 4 months now.
Thanks
Valentin

Hello Jagannadha,
On 01/21/2014 11:03 AM, Valentin Longchamp wrote:
On 09/17/2013 08:46 AM, Valentin Longchamp wrote:
Some board require spi_flash_free to be called after all the accesses, in order, for instance, to restore the pin multiplexing configuration in the case where the SPI pins are multiplexed.
This patch series tries to enhance this. Patch 1 adds spi_flash_free calls to env_sf so that the SPI interface is always "cleaned up" after the env read/writes. Patch 2 adds a 'sf release' command that implicitly calls spi_flash_free and is thus the pendant of 'sf probe'. Patch 3 uses the 'sf command' for the km_arm board scripts.
Valentin Longchamp (3): env_sf: generalize call to spi_flash_free after accesses cmd_sf: add 'release' command km_arm: call 'sf release' in the newenv and update scripts
common/cmd_sf.c | 13 ++++++++++++- common/env_sf.c | 34 ++++++++++++++++------------------ include/configs/km/km_arm.h | 6 ++++-- 3 files changed, 32 insertions(+), 21 deletions(-)
I have sent this series 4 months ago and while it is according to patchwork under review I have not received any feedback yet.
That would be nice if this goes into the next merge window since they have been floating around for 4 months now.
Here I ping again ! I have sent this series more than a year ago and you had promised me a off list a review "in a few days" at the beginning of march 2014 ... but I have seen nothing. The only thing that has happened is that the first patch of the series was rejected by you.
I don't mind that the patch was rejected. But this is a required patch for us and I want it to be mainlined, because I have to rebase it internally for every release. Without the feedback about why it was rejected, I cannot however improve it and resubmit it.
Can you please review this series and give me a feedback about it ?
Valentin

On 10 October 2014 16:53, Valentin Longchamp valentin.longchamp@keymile.com wrote:
Hello Jagannadha,
On 01/21/2014 11:03 AM, Valentin Longchamp wrote:
On 09/17/2013 08:46 AM, Valentin Longchamp wrote:
Some board require spi_flash_free to be called after all the accesses, in order, for instance, to restore the pin multiplexing configuration in the case where the SPI pins are multiplexed.
This patch series tries to enhance this. Patch 1 adds spi_flash_free calls to env_sf so that the SPI interface is always "cleaned up" after the env read/writes. Patch 2 adds a 'sf release' command that implicitly calls spi_flash_free and is thus the pendant of 'sf probe'. Patch 3 uses the 'sf command' for the km_arm board scripts.
Valentin Longchamp (3): env_sf: generalize call to spi_flash_free after accesses cmd_sf: add 'release' command km_arm: call 'sf release' in the newenv and update scripts
common/cmd_sf.c | 13 ++++++++++++- common/env_sf.c | 34 ++++++++++++++++------------------ include/configs/km/km_arm.h | 6 ++++-- 3 files changed, 32 insertions(+), 21 deletions(-)
I have sent this series 4 months ago and while it is according to patchwork under review I have not received any feedback yet.
That would be nice if this goes into the next merge window since they have been floating around for 4 months now.
Here I ping again ! I have sent this series more than a year ago and you had promised me a off list a review "in a few days" at the beginning of march 2014 ... but I have seen nothing. The only thing that has happened is that the first patch of the series was rejected by you.
I don't mind that the patch was rejected. But this is a required patch for us and I want it to be mainlined, because I have to rebase it internally for every release. Without the feedback about why it was rejected, I cannot however improve it and resubmit it.
Can you please review this series and give me a feedback about it ?
Sorry, for long run. Will give my comments soon.
thanks!
participants (2)
-
Jagan Teki
-
Valentin Longchamp