[U-Boot] [Patch V2 4/4] dm: env_sf: fix saveenv() to use driver model

[Simon Glass]

Hi,

On 15 December 2015 at 03:32, Gong Qianyu <Qianyu.Gong at freescale.com> wrote:
> It might be missed when converting spi_flash_probe() in cmd_sf.c.
>
> This commit refers to fbb099183e3a53f77a975964cdf2e73d11e565af.
>
> Signed-off-by: Gong Qianyu <Qianyu.Gong at freescale.com>
> ---
> V2:
>  - New Patch.
>
>  common/env_sf.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
>
> diff --git a/common/env_sf.c b/common/env_sf.c
> index 9409831..31d96a7 100644
> --- a/common/env_sf.c
> +++ b/common/env_sf.c
> @@ -16,6 +16,7 @@
>  #include <spi_flash.h>
>  #include <search.h>
>  #include <errno.h>
> +#include <dm/device-internal.h>
>
>  #ifndef CONFIG_ENV_SPI_BUS
>  # define CONFIG_ENV_SPI_BUS    0
> @@ -51,6 +52,29 @@ int saveenv(void)
>         char    *saved_buffer = NULL, flag = OBSOLETE_FLAG;
>         u32     saved_size, saved_offset, sector = 1;
>         int     ret;
> +#ifdef CONFIG_DM_SPI_FLASH
> +       struct udevice *new, *bus_dev;
> +       unsigned int bus = CONFIG_SF_DEFAULT_BUS;
> +       unsigned int cs = CONFIG_SF_DEFAULT_CS;
> +       unsigned int speed = CONFIG_SF_DEFAULT_SPEED;
> +       unsigned int mode = CONFIG_SF_DEFAULT_MODE;
> +
> +       /* Remove the old device, otherwise probe will just be a nop */
> +       ret = spi_find_bus_and_cs(bus, cs, &bus_dev, &new);
> +       if (!ret) {
> +               device_remove(new);
> +               device_unbind(new);
> +       }
> +       env_flash = NULL;

You should be abve to avoid the above code and just have the code below:

> +       ret = spi_flash_probe_bus_cs(bus, cs, speed, mode, &new);
> +       if (ret) {
> +               printf("Failed to initialize SPI flash at %u:%u (error %d)\n",
> +                      bus, cs, ret);
> +               return 1;
> +       }
> +
> +       env_flash = dev_get_uclass_priv(new);

The reason why 'sf probe' removes the old device is to ensure that it
is probed again, in case something has changed. Even that is suspect,
but in the env case it seems plain wrong.

However, you must always do this with driver model. Even if env_flash
is non-NULL, you must get it again (with driver model).

> +#else
>
>         if (!env_flash) {
>                 env_flash = spi_flash_probe(CONFIG_ENV_SPI_BUS,
> @@ -61,6 +85,7 @@ int saveenv(void)
>                         return 1;
>                 }
>         }
> +#endif
>
>         ret = env_export(&env_new);
>         if (ret)
> @@ -227,6 +252,29 @@ int saveenv(void)
>         char    *saved_buffer = NULL;
>         int     ret = 1;
>         env_t   env_new;
> +#ifdef CONFIG_DM_SPI_FLASH
> +       struct udevice *new, *bus_dev;
> +       unsigned int bus = CONFIG_SF_DEFAULT_BUS;
> +       unsigned int cs = CONFIG_SF_DEFAULT_CS;
> +       unsigned int speed = CONFIG_SF_DEFAULT_SPEED;
> +       unsigned int mode = CONFIG_SF_DEFAULT_MODE;
> +
> +       /* Remove the old device, otherwise probe will just be a nop */
> +       ret = spi_find_bus_and_cs(bus, cs, &bus_dev, &new);
> +       if (!ret) {
> +               device_remove(new);
> +               device_unbind(new);
> +       }
> +       env_flash = NULL;
> +       ret = spi_flash_probe_bus_cs(bus, cs, speed, mode, &new);
> +       if (ret) {
> +               printf("Failed to initialize SPI flash at %u:%u (error %d)\n",
> +                      bus, cs, ret);
> +               return 1;
> +       }
> +
> +       env_flash = dev_get_uclass_priv(new);
> +#else
>
>         if (!env_flash) {
>                 env_flash = spi_flash_probe(CONFIG_ENV_SPI_BUS,
> @@ -237,6 +285,7 @@ int saveenv(void)
>                         return 1;
>                 }
>         }
> +#endif
>
>         /* Is the sector larger than the env (i.e. embedded) */
>         if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
> --
> 2.1.0.27.g96db324
>

Regards,
Simon