[PATCH 3/5] env: sf: Preserve and free the previous flash

[Pratyush Yadav]

Hi Jagan,

On 14/05/20 05:41PM, Jagan Teki wrote:
> env_flash is a global flash pointer, and the probe would
> happen only if env_flash is NULL, but there is no checking
> and free the pointer if is not NULL.
> 
> So, this patch frees the env_flash if it's not NULL, and
> get the probed flash in new flash pointer and finally
> assign into env_flash.
> 
> Note: Similar approach has been followed and tested in
> cmd/sf.c
> 
> Cc: Simon Glass <sjg at chromium.org>
> Cc: Vignesh R <vigneshr at ti.com>
> Signed-off-by: Jagan Teki <jagan at amarulasolutions.com>
> ---
>  env/sf.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/env/sf.c b/env/sf.c
> index 64c57f2cdf..af59c8375c 100644
> --- a/env/sf.c
> +++ b/env/sf.c
> @@ -50,15 +50,17 @@ static int setup_flash_device(void)
>  
>  	env_flash = dev_get_uclass_priv(new);
>  #else
> +	struct spi_flash *new;
>  
> -	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) {
> -			env_set_default("spi_flash_probe() failed", 0);
> -			return -EIO;
> -		}
> +	if (env_flash)
> +		spi_flash_free(env_flash);
> +
> +	new = spi_flash_probe(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS,

Why not assign env_flash directly here? You do it in the very next 
statement anyway. I don't think the extra 'new' variable is needed.

> +			      CONFIG_ENV_SPI_MAX_HZ, CONFIG_ENV_SPI_MODE);
> +	env_flash = new;
> +	if (!new) {
> +		env_set_default("spi_flash_probe() failed", 0);
> +		return -EIO;
>  	}
>  #endif
>  	return 0;
> -- 
> 2.20.1
> 

-- 
Regards,
Pratyush Yadav
Texas Instruments India