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

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

[Valentin Longchamp]

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

-- 
1.8.0.1

[U-Boot] [PATCH 1/3] env_sf: generalize call to spi_flash_free after accesses

[Valentin Longchamp]

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

[U-Boot] [PATCH 2/3] cmd_sf: add 'release' command

[Valentin Longchamp]

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

[U-Boot] [PATCH 3/3] km_arm: call 'sf release' in the newenv and update scripts

[Valentin Longchamp]

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 && "				\
-- 
1.8.0.1

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

[Valentin Longchamp]

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

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

[Valentin Longchamp]

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

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

[Jagan Teki]

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!
-- 
Jagan.