[U-Boot] [Patch V2 4/4] dm: env_sf: fix saveenv() to use driver model
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) {
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
-----Original Message----- From: sjg@google.com [mailto:sjg@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@freescale.com; yao.yuan@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 ...
-----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@freescale.com; yao.yuan@freescale.com; Jagan Teki Subject: RE: [Patch V2 4/4] dm: env_sf: fix saveenv() to use driver model
-----Original Message----- From: sjg@google.com [mailto:sjg@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@freescale.com; yao.yuan@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
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@freescale.com; yao.yuan@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@freescale.com; yao.yuan@freescale.com; Jagan Teki Subject: RE: [Patch V2 4/4] dm: env_sf: fix saveenv() to use driver model
-----Original Message----- From: sjg@google.com [mailto:sjg@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@freescale.com; yao.yuan@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
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
-----Original Message----- From: sjg@google.com [mailto:sjg@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@freescale.com; yao.yuan@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
participants (3)
-
Gong Qianyu -
Qianyu Gong -
Simon Glass