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

[Jagan Teki]

On Fri, May 15, 2020 at 12:54 PM Pratyush Yadav <p.yadav at ti.com> wrote:
>
> 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.

If flash pointer is used free it, before probing a new flash and
storing it in flash. In this scenario it requires local new pointer,
it was known observation we have faced in cmd/sf.c

Jagan.