All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [Patch V2 4/4] dm: env_sf: fix saveenv() to use driver model
@ 2015-12-15 10:32 Gong Qianyu
  2015-12-19  2:51 ` Simon Glass
  2016-01-08  3:34 ` Simon Glass
  0 siblings, 2 replies; 7+ messages in thread
From: Gong Qianyu @ 2015-12-15 10:32 UTC (permalink / raw)
  To: u-boot

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@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;
+	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,
@@ -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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [U-Boot] [Patch V2 4/4] dm: env_sf: fix saveenv() to use driver model
  2015-12-15 10:32 [U-Boot] [Patch V2 4/4] dm: env_sf: fix saveenv() to use driver model Gong Qianyu
@ 2015-12-19  2:51 ` Simon Glass
  2015-12-21  5:25   ` Qianyu Gong
  2015-12-21  6:01   ` Qianyu Gong
  2016-01-08  3:34 ` Simon Glass
  1 sibling, 2 replies; 7+ messages in thread
From: Simon Glass @ 2015-12-19  2:51 UTC (permalink / raw)
  To: u-boot

Hi Gong,

On 15 December 2015 at 03:32, Gong Qianyu <Qianyu.Gong@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@freescale.com>
> ---
> V2:
>  - New Patch.
>
>  common/env_sf.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)

The 'saveenv' command seems to work OK for me with driver model. Can
you please explain what problem this patch solves?

Regards,
Simon

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [U-Boot] [Patch V2 4/4] dm: env_sf: fix saveenv() to use driver model
  2015-12-19  2:51 ` Simon Glass
@ 2015-12-21  5:25   ` Qianyu Gong
  2015-12-21  6:01   ` Qianyu Gong
  1 sibling, 0 replies; 7+ messages in thread
From: Qianyu Gong @ 2015-12-21  5:25 UTC (permalink / raw)
  To: u-boot



> -----Original Message-----
> From: sjg at google.com [mailto:sjg at google.com] On Behalf Of Simon Glass
> Sent: Saturday, December 19, 2015 10:51 AM
> To: Gong Qianyu
> Cc: U-Boot Mailing List; Mingkai Hu; R58495 at freescale.com;
> yao.yuan at freescale.com; Jagan Teki
> Subject: Re: [Patch V2 4/4] dm: env_sf: fix saveenv() to use driver model
> 
> Hi Gong,
> 
> On 15 December 2015 at 03:32, Gong Qianyu <Qianyu.Gong@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@freescale.com>
> > ---
> > V2:
> >  - New Patch.
> >
> >  common/env_sf.c | 49
> > +++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 49 insertions(+)
> 
> The 'saveenv' command seems to work OK for me with driver model. Can you
> please explain what problem this patch solves?
> 
> Regards,
> Simon

Hi Simon,

The saveenv() always keeps the latest 'sf probe' value of env_flash. So if I run 'sf probe' 
a few more times till it returns a different value, saveenv() will then get into sync abort.

Actually the problem I met is:
=> saveenv
Saving Environment to SPI Flash...
SF: Detected S25FL128S_64K with page size 256 Bytes, erase size 64 KiB, total 16MiB
Erasing SPI flash...Writing to SPI flash...done
=> sf probe
SF: Detected S25FL128S_64K with page size 256 Bytes, erase size 64 KiB, total 16MiB
=> saveenv
Saving Environment to SPI Flash...
"Synchronous Abort" handler, esr 0x02000000
ELR:     fffa8520
LR:      fff6fd28
x0 : 00000000ffe44850 x1 : 0000000000102000
x2 : 000000000000e000 x3 : 00000000ffe6a670
x4 : 00000000fffa8520 x5 : 00000000fffa89c0
x6 : 00000000fffa8340 x7 : 0000000000000053
x8 : 0000000000000000 x9 : 000000000000000c
x10: 00000000ffe6a660 x11: 00000000fff90000
x12: 000000000000000f x13: 0000000040000000
x14: 0000000000200000 x15: 00000000fff490d0
x16: 00000000fff4984c x17: 0000000000000064
x18: 00000000ffe43da0 x19: 00000000fffb7000
x20: 00000000ffe6a670 x21: 00000000fffb7000
x22: 00000000fffb7000 x23: 0000000000000001
x24: 00000000fffb6db0 x25: 0000000000000000
x26: 0000000000000000 x27: 0000000000000000
x28: 0000000000000000 x29: 00000000ffe3f4d0

Resetting CPU ...

resetting ...

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [U-Boot] [Patch V2 4/4] dm: env_sf: fix saveenv() to use driver model
  2015-12-19  2:51 ` Simon Glass
  2015-12-21  5:25   ` Qianyu Gong
@ 2015-12-21  6:01   ` Qianyu Gong
  2016-01-04  9:44     ` Qianyu Gong
  1 sibling, 1 reply; 7+ messages in thread
From: Qianyu Gong @ 2015-12-21  6:01 UTC (permalink / raw)
  To: u-boot



> -----Original Message-----
> From: Qianyu Gong
> Sent: Monday, December 21, 2015 1:25 PM
> To: 'Simon Glass'; Gong Qianyu
> Cc: U-Boot Mailing List; Mingkai Hu; R58495 at freescale.com;
> yao.yuan at freescale.com; Jagan Teki
> Subject: RE: [Patch V2 4/4] dm: env_sf: fix saveenv() to use driver model
> 
> 
> 
> > -----Original Message-----
> > From: sjg at google.com [mailto:sjg at google.com] On Behalf Of Simon Glass
> > Sent: Saturday, December 19, 2015 10:51 AM
> > To: Gong Qianyu
> > Cc: U-Boot Mailing List; Mingkai Hu; R58495 at freescale.com;
> > yao.yuan at freescale.com; Jagan Teki
> > Subject: Re: [Patch V2 4/4] dm: env_sf: fix saveenv() to use driver
> > model
> >
> > Hi Gong,
> >
> > On 15 December 2015 at 03:32, Gong Qianyu <Qianyu.Gong@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@freescale.com>
> > > ---
> > > V2:
> > >  - New Patch.
> > >
> > >  common/env_sf.c | 49
> > > +++++++++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 49 insertions(+)
> >
> > The 'saveenv' command seems to work OK for me with driver model. Can
> > you please explain what problem this patch solves?
> >
> > Regards,
> > Simon
> 
> Hi Simon,
> 
> The saveenv() always keeps the latest 'sf probe' value of env_flash. So
> if I run 'sf probe'
> a few more times till it returns a different value, saveenv() will then
> get into sync abort.
> 
> Actually the problem I met is:
> => saveenv
> Saving Environment to SPI Flash...
> SF: Detected S25FL128S_64K with page size 256 Bytes, erase size 64 KiB,
> total 16MiB Erasing SPI flash...Writing to SPI flash...done => sf probe
> SF: Detected S25FL128S_64K with page size 256 Bytes, erase size 64 KiB,
> total 16MiB => saveenv Saving Environment to SPI Flash...
> "Synchronous Abort" handler, esr 0x02000000
> ELR:     fffa8520
> LR:      fff6fd28
> x0 : 00000000ffe44850 x1 : 0000000000102000
> x2 : 000000000000e000 x3 : 00000000ffe6a670
> x4 : 00000000fffa8520 x5 : 00000000fffa89c0
> x6 : 00000000fffa8340 x7 : 0000000000000053
> x8 : 0000000000000000 x9 : 000000000000000c
> x10: 00000000ffe6a660 x11: 00000000fff90000
> x12: 000000000000000f x13: 0000000040000000
> x14: 0000000000200000 x15: 00000000fff490d0
> x16: 00000000fff4984c x17: 0000000000000064
> x18: 00000000ffe43da0 x19: 00000000fffb7000
> x20: 00000000ffe6a670 x21: 00000000fffb7000
> x22: 00000000fffb7000 x23: 0000000000000001
> x24: 00000000fffb6db0 x25: 0000000000000000
> x26: 0000000000000000 x27: 0000000000000000
> x28: 0000000000000000 x29: 00000000ffe3f4d0
> 
> Resetting CPU ...
> 
> resetting ...
> 
> 

This patch makes 'saveenv' probes the flash every time like 'sf probe'. Maybe there is a better way
to solve this. I'm also confused why the current 'sf probe' may return different 
values for the same SPI flash(the same cs and bus)..Isn't it freed cleanly?

Regards,
Qianyu

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [U-Boot] [Patch V2 4/4] dm: env_sf: fix saveenv() to use driver model
  2015-12-21  6:01   ` Qianyu Gong
@ 2016-01-04  9:44     ` Qianyu Gong
  0 siblings, 0 replies; 7+ messages in thread
From: Qianyu Gong @ 2016-01-04  9:44 UTC (permalink / raw)
  To: u-boot

Hi Jagan and Simon,

Do you have any comments on the patch set? 
Or could you please help to merge them? Thank you.

Regards,
Qianyu

> -----Original Message-----
> From: Qianyu Gong
> Sent: Monday, December 21, 2015 2:01 PM
> To: Simon Glass <sjg@chromium.org>
> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Mingkai Hu
> <Mingkai.Hu@freescale.com>; R58495 at freescale.com; yao.yuan at freescale.com;
> Jagan Teki <jteki@openedev.com>
> Subject: RE: [Patch V2 4/4] dm: env_sf: fix saveenv() to use driver model
> 
> 
> 
> > -----Original Message-----
> > From: Qianyu Gong
> > Sent: Monday, December 21, 2015 1:25 PM
> > To: 'Simon Glass'; Gong Qianyu
> > Cc: U-Boot Mailing List; Mingkai Hu; R58495 at freescale.com;
> > yao.yuan at freescale.com; Jagan Teki
> > Subject: RE: [Patch V2 4/4] dm: env_sf: fix saveenv() to use driver
> > model
> >
> >
> >
> > > -----Original Message-----
> > > From: sjg at google.com [mailto:sjg at google.com] On Behalf Of Simon
> > > Glass
> > > Sent: Saturday, December 19, 2015 10:51 AM
> > > To: Gong Qianyu
> > > Cc: U-Boot Mailing List; Mingkai Hu; R58495 at freescale.com;
> > > yao.yuan at freescale.com; Jagan Teki
> > > Subject: Re: [Patch V2 4/4] dm: env_sf: fix saveenv() to use driver
> > > model
> > >
> > > Hi Gong,
> > >
> > > On 15 December 2015 at 03:32, Gong Qianyu
> > > <Qianyu.Gong@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@freescale.com>
> > > > ---
> > > > V2:
> > > >  - New Patch.
> > > >
> > > >  common/env_sf.c | 49
> > > > +++++++++++++++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 49 insertions(+)
> > >
> > > The 'saveenv' command seems to work OK for me with driver model. Can
> > > you please explain what problem this patch solves?
> > >
> > > Regards,
> > > Simon
> >
> > Hi Simon,
> >
> > The saveenv() always keeps the latest 'sf probe' value of env_flash.
> > So if I run 'sf probe'
> > a few more times till it returns a different value, saveenv() will
> > then get into sync abort.
> >
> > Actually the problem I met is:
> > => saveenv
> > Saving Environment to SPI Flash...
> > SF: Detected S25FL128S_64K with page size 256 Bytes, erase size 64
> > KiB, total 16MiB Erasing SPI flash...Writing to SPI flash...done => sf
> > probe
> > SF: Detected S25FL128S_64K with page size 256 Bytes, erase size 64
> > KiB, total 16MiB => saveenv Saving Environment to SPI Flash...
> > "Synchronous Abort" handler, esr 0x02000000
> > ELR:     fffa8520
> > LR:      fff6fd28
> > x0 : 00000000ffe44850 x1 : 0000000000102000
> > x2 : 000000000000e000 x3 : 00000000ffe6a670
> > x4 : 00000000fffa8520 x5 : 00000000fffa89c0
> > x6 : 00000000fffa8340 x7 : 0000000000000053
> > x8 : 0000000000000000 x9 : 000000000000000c
> > x10: 00000000ffe6a660 x11: 00000000fff90000
> > x12: 000000000000000f x13: 0000000040000000
> > x14: 0000000000200000 x15: 00000000fff490d0
> > x16: 00000000fff4984c x17: 0000000000000064
> > x18: 00000000ffe43da0 x19: 00000000fffb7000
> > x20: 00000000ffe6a670 x21: 00000000fffb7000
> > x22: 00000000fffb7000 x23: 0000000000000001
> > x24: 00000000fffb6db0 x25: 0000000000000000
> > x26: 0000000000000000 x27: 0000000000000000
> > x28: 0000000000000000 x29: 00000000ffe3f4d0
> >
> > Resetting CPU ...
> >
> > resetting ...
> >
> >
> 
> This patch makes 'saveenv' probes the flash every time like 'sf probe'. Maybe there
> is a better way to solve this. I'm also confused why the current 'sf probe' may
> return different values for the same SPI flash(the same cs and bus)..Isn't it freed
> cleanly?
> 
> Regards,
> Qianyu

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [U-Boot] [Patch V2 4/4] dm: env_sf: fix saveenv() to use driver model
  2015-12-15 10:32 [U-Boot] [Patch V2 4/4] dm: env_sf: fix saveenv() to use driver model Gong Qianyu
  2015-12-19  2:51 ` Simon Glass
@ 2016-01-08  3:34 ` Simon Glass
  2016-01-08  7:01   ` Qianyu Gong
  1 sibling, 1 reply; 7+ messages in thread
From: Simon Glass @ 2016-01-08  3:34 UTC (permalink / raw)
  To: u-boot

Hi,

On 15 December 2015 at 03:32, Gong Qianyu <Qianyu.Gong@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@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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [U-Boot] [Patch V2 4/4] dm: env_sf: fix saveenv() to use driver model
  2016-01-08  3:34 ` Simon Glass
@ 2016-01-08  7:01   ` Qianyu Gong
  0 siblings, 0 replies; 7+ messages in thread
From: Qianyu Gong @ 2016-01-08  7:01 UTC (permalink / raw)
  To: u-boot

> -----Original Message-----
> From: sjg at google.com [mailto:sjg at google.com] On Behalf Of Simon Glass
> Sent: Friday, January 08, 2016 11:34 AM
> To: Gong Qianyu <Qianyu.Gong@freescale.com>
> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Mingkai Hu
> <Mingkai.Hu@freescale.com>; R58495 at freescale.com; yao.yuan at freescale.com;
> Jagan Teki <jteki@openedev.com>
> Subject: Re: [Patch V2 4/4] dm: env_sf: fix saveenv() to use driver model
> 
> Hi,
> 
> On 15 December 2015 at 03:32, Gong Qianyu <Qianyu.Gong@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@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).
> 
Hi Simon,

Ok, I see. Thanks for the explanation. 
I'll send a new version of it then.

Regards,
Qianyu

> > +#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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2016-01-08  7:01 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-15 10:32 [U-Boot] [Patch V2 4/4] dm: env_sf: fix saveenv() to use driver model Gong Qianyu
2015-12-19  2:51 ` Simon Glass
2015-12-21  5:25   ` Qianyu Gong
2015-12-21  6:01   ` Qianyu Gong
2016-01-04  9:44     ` Qianyu Gong
2016-01-08  3:34 ` Simon Glass
2016-01-08  7:01   ` Qianyu Gong

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.