[U-Boot] [PATCH 0/7] Remove defines for SPI default speed and mode

This patch-set finish (after the last Simmon comment) and depends on the previous serie named: "Read default speed and mode values from DT"
http://patchwork.ozlabs.org/project/uboot/list/?series=76834
This patchset remove the unnecessary defines for SPI speed and mode when DM mode is used for SPI (CONFIG_DM_SPI_FLASH).
Build result is available for the serie in gihub and travis: https://github.com/patrickdelaunay/u-boot/tree/topic/spi https://travis-ci.org/patrickdelaunay/u-boot/branches
Patrick Delaunay (7): spi: update management of default speed and mode spi: add comment for spi_flash_probe_bus_cs function da850evm: sf: Read default speed and mode values from DT dfu: sf: Read default speed and mode values from DT mips: mt76xx: gardena-smart-gateway: Read default speed and mode values from DT env: Read default speed and mode values from DT spi: remove define for SPI default SPEED and MODE
board/davinci/da8xxevm/da850evm.c | 7 +++++++ board/gardena/smart-gateway-mt7688/board.c | 24 ++++++++++++++++++++---- cmd/sf.c | 10 ++++++---- common/spl/spl_spi.c | 11 ++++++----- common/splash_source.c | 11 ++++++----- drivers/dfu/dfu_sf.c | 10 ++++++++-- env/Kconfig | 4 ++-- env/sf.c | 5 ++++- include/spi_flash.h | 18 ++++++++++++++++++ 9 files changed, 77 insertions(+), 23 deletions(-)

The 2 default values for SPI mode and speed are only if CONFIG_DM_SPI_FLASH is not defined - CONFIG_SF_DEFAULT_SPEED - CONFIG_SF_DEFAULT_MODE
Inverse the logic of the test to remove these two defines.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com ---
cmd/sf.c | 10 ++++++---- common/spl/spl_spi.c | 11 ++++++----- common/splash_source.c | 11 ++++++----- 3 files changed, 18 insertions(+), 14 deletions(-)
diff --git a/cmd/sf.c b/cmd/sf.c index 84bb057..cfea545 100644 --- a/cmd/sf.c +++ b/cmd/sf.c @@ -81,16 +81,18 @@ static int do_spi_flash_probe(int argc, char * const argv[]) { 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; + /* In DM mode, defaults will be taken from DT */ + unsigned int speed = 0; + unsigned int mode = 0; char *endp; #ifdef CONFIG_DM_SPI_FLASH struct udevice *new, *bus_dev; int ret; - /* In DM mode defaults will be taken from DT */ - speed = 0, mode = 0; #else struct spi_flash *new; + + speed = CONFIG_SF_DEFAULT_SPEED; + mode = CONFIG_SF_DEFAULT_MODE; #endif
if (argc >= 2) { diff --git a/common/spl/spl_spi.c b/common/spl/spl_spi.c index b348b45..c1c1fcb 100644 --- a/common/spl/spl_spi.c +++ b/common/spl/spl_spi.c @@ -74,12 +74,13 @@ static int spl_spi_load_image(struct spl_image_info *spl_image, unsigned payload_offs = CONFIG_SYS_SPI_U_BOOT_OFFS; struct spi_flash *flash; struct image_header *header; - unsigned int max_hz = CONFIG_SF_DEFAULT_SPEED; - unsigned int spi_mode = CONFIG_SF_DEFAULT_MODE; + /* In DM mode, defaults will be taken from DT */ + unsigned int max_hz = 0; + unsigned int spi_mode = 0;
-#ifdef CONFIG_DM_SPI_FLASH - /* In DM mode defaults will be taken from DT */ - max_hz = 0, spi_mode = 0; +#ifndef CONFIG_DM_SPI_FLASH + max_hz = CONFIG_SF_DEFAULT_SPEED; + spi_mode = CONFIG_SF_DEFAULT_MODE; #endif /* * Load U-Boot image from SPI flash into RAM diff --git a/common/splash_source.c b/common/splash_source.c index 427196c..d5d5550 100644 --- a/common/splash_source.c +++ b/common/splash_source.c @@ -24,12 +24,13 @@ DECLARE_GLOBAL_DATA_PTR; static struct spi_flash *sf; static int splash_sf_read_raw(u32 bmp_load_addr, int offset, size_t read_size) { - unsigned int max_hz = CONFIG_SF_DEFAULT_SPEED; - unsigned int spi_mode = CONFIG_SF_DEFAULT_MODE; + /* In DM mode, defaults will be taken from DT */ + unsigned int max_hz = 0; + unsigned int spi_mode = 0;
-#ifdef CONFIG_DM_SPI_FLASH - /* In DM mode defaults will be taken from DT */ - max_hz = 0, spi_mode = 0; +#ifndef CONFIG_DM_SPI_FLASH + max_hz = CONFIG_SF_DEFAULT_SPEED; + spi_mode = CONFIG_SF_DEFAULT_MODE; #endif
if (!sf) {

On Mon, Dec 10, 2018 at 11:53 AM Patrick Delaunay patrick.delaunay@st.com wrote:
The 2 default values for SPI mode and speed are only if CONFIG_DM_SPI_FLASH is not defined
- CONFIG_SF_DEFAULT_SPEED
- CONFIG_SF_DEFAULT_MODE
Inverse the logic of the test to remove these two defines.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
cmd/sf.c | 10 ++++++---- common/spl/spl_spi.c | 11 ++++++----- common/splash_source.c | 11 ++++++----- 3 files changed, 18 insertions(+), 14 deletions(-)
diff --git a/cmd/sf.c b/cmd/sf.c index 84bb057..cfea545 100644 --- a/cmd/sf.c +++ b/cmd/sf.c @@ -81,16 +81,18 @@ static int do_spi_flash_probe(int argc, char * const argv[]) { 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;
/* In DM mode, defaults will be taken from DT */
unsigned int speed = 0;
unsigned int mode = 0; char *endp;
#ifdef CONFIG_DM_SPI_FLASH struct udevice *new, *bus_dev; int ret;
/* In DM mode defaults will be taken from DT */
speed = 0, mode = 0;
#else struct spi_flash *new;
speed = CONFIG_SF_DEFAULT_SPEED;
mode = CONFIG_SF_DEFAULT_MODE;
#endif
if (argc >= 2) {
diff --git a/common/spl/spl_spi.c b/common/spl/spl_spi.c index b348b45..c1c1fcb 100644 --- a/common/spl/spl_spi.c +++ b/common/spl/spl_spi.c @@ -74,12 +74,13 @@ static int spl_spi_load_image(struct spl_image_info *spl_image, unsigned payload_offs = CONFIG_SYS_SPI_U_BOOT_OFFS; struct spi_flash *flash; struct image_header *header;
unsigned int max_hz = CONFIG_SF_DEFAULT_SPEED;
unsigned int spi_mode = CONFIG_SF_DEFAULT_MODE;
/* In DM mode, defaults will be taken from DT */
unsigned int max_hz = 0;
unsigned int spi_mode = 0;
-#ifdef CONFIG_DM_SPI_FLASH
/* In DM mode defaults will be taken from DT */
max_hz = 0, spi_mode = 0;
+#ifndef CONFIG_DM_SPI_FLASH
max_hz = CONFIG_SF_DEFAULT_SPEED;
spi_mode = CONFIG_SF_DEFAULT_MODE;
#endif /* * Load U-Boot image from SPI flash into RAM diff --git a/common/splash_source.c b/common/splash_source.c index 427196c..d5d5550 100644 --- a/common/splash_source.c +++ b/common/splash_source.c @@ -24,12 +24,13 @@ DECLARE_GLOBAL_DATA_PTR; static struct spi_flash *sf; static int splash_sf_read_raw(u32 bmp_load_addr, int offset, size_t read_size) {
unsigned int max_hz = CONFIG_SF_DEFAULT_SPEED;
unsigned int spi_mode = CONFIG_SF_DEFAULT_MODE;
/* In DM mode, defaults will be taken from DT */
unsigned int max_hz = 0;
unsigned int spi_mode = 0;
-#ifdef CONFIG_DM_SPI_FLASH
/* In DM mode defaults will be taken from DT */
max_hz = 0, spi_mode = 0;
+#ifndef CONFIG_DM_SPI_FLASH
max_hz = CONFIG_SF_DEFAULT_SPEED;
spi_mode = CONFIG_SF_DEFAULT_MODE;
#endif
if (!sf) {
Reviewed-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
Regards, Simon

Hi Patrick,
On Mon, Dec 10, 2018 at 11:53 AM Patrick Delaunay patrick.delaunay@st.com wrote:
The 2 default values for SPI mode and speed are only if CONFIG_DM_SPI_FLASH is not defined
- CONFIG_SF_DEFAULT_SPEED
- CONFIG_SF_DEFAULT_MODE
Inverse the logic of the test to remove these two defines.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
Reviewed-by: Petr Vorel petr.vorel@gmail.com
cmd/sf.c | 10 ++++++---- common/spl/spl_spi.c | 11 ++++++----- common/splash_source.c | 11 ++++++-----
Patch only applies to cmd/sf.c, the other once do not apply (original patch was too old). Or am I missing something?
Kind regards, Petr
3 files changed, 18 insertions(+), 14 deletions(-)
diff --git a/cmd/sf.c b/cmd/sf.c index 84bb057..cfea545 100644 --- a/cmd/sf.c +++ b/cmd/sf.c @@ -81,16 +81,18 @@ static int do_spi_flash_probe(int argc, char * const argv[]) { 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;
/* In DM mode, defaults will be taken from DT */
unsigned int speed = 0;
unsigned int mode = 0; char *endp;
#ifdef CONFIG_DM_SPI_FLASH struct udevice *new, *bus_dev; int ret;
/* In DM mode defaults will be taken from DT */
speed = 0, mode = 0;
#else struct spi_flash *new;
speed = CONFIG_SF_DEFAULT_SPEED;
mode = CONFIG_SF_DEFAULT_MODE;
#endif
if (argc >= 2) {
diff --git a/common/spl/spl_spi.c b/common/spl/spl_spi.c index b348b45..c1c1fcb 100644 --- a/common/spl/spl_spi.c +++ b/common/spl/spl_spi.c @@ -74,12 +74,13 @@ static int spl_spi_load_image(struct spl_image_info *spl_image, unsigned payload_offs = CONFIG_SYS_SPI_U_BOOT_OFFS; struct spi_flash *flash; struct image_header *header;
unsigned int max_hz = CONFIG_SF_DEFAULT_SPEED;
unsigned int spi_mode = CONFIG_SF_DEFAULT_MODE;
/* In DM mode, defaults will be taken from DT */
unsigned int max_hz = 0;
unsigned int spi_mode = 0;
-#ifdef CONFIG_DM_SPI_FLASH
/* In DM mode defaults will be taken from DT */
max_hz = 0, spi_mode = 0;
+#ifndef CONFIG_DM_SPI_FLASH
max_hz = CONFIG_SF_DEFAULT_SPEED;
spi_mode = CONFIG_SF_DEFAULT_MODE;
#endif /* * Load U-Boot image from SPI flash into RAM diff --git a/common/splash_source.c b/common/splash_source.c index 427196c..d5d5550 100644 --- a/common/splash_source.c +++ b/common/splash_source.c @@ -24,12 +24,13 @@ DECLARE_GLOBAL_DATA_PTR; static struct spi_flash *sf; static int splash_sf_read_raw(u32 bmp_load_addr, int offset, size_t read_size) {
unsigned int max_hz = CONFIG_SF_DEFAULT_SPEED;
unsigned int spi_mode = CONFIG_SF_DEFAULT_MODE;
/* In DM mode, defaults will be taken from DT */
unsigned int max_hz = 0;
unsigned int spi_mode = 0;
-#ifdef CONFIG_DM_SPI_FLASH
/* In DM mode defaults will be taken from DT */
max_hz = 0, spi_mode = 0;
+#ifndef CONFIG_DM_SPI_FLASH
max_hz = CONFIG_SF_DEFAULT_SPEED;
spi_mode = CONFIG_SF_DEFAULT_MODE;
#endif
if (!sf) {
Reviewed-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot

Am 10.12.2018 um 21:49 schrieb Petr Vorel:
Hi Patrick,
On Mon, Dec 10, 2018 at 11:53 AM Patrick Delaunay patrick.delaunay@st.com wrote:
The 2 default values for SPI mode and speed are only if CONFIG_DM_SPI_FLASH is not defined
- CONFIG_SF_DEFAULT_SPEED
- CONFIG_SF_DEFAULT_MODE
Inverse the logic of the test to remove these two defines.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
Reviewed-by: Petr Vorel petr.vorel@gmail.com
cmd/sf.c | 10 ++++++---- common/spl/spl_spi.c | 11 ++++++----- common/splash_source.c | 11 ++++++-----
Patch only applies to cmd/sf.c, the other once do not apply (original patch was too old). Or am I missing something?
I have applied http://patchwork.ozlabs.org/project/uboot/list/?series=76834 before applying this series (as Patrick wrote in his cover letter) and it worked with current master.
Regards, Simon
Kind regards, Petr
3 files changed, 18 insertions(+), 14 deletions(-)
diff --git a/cmd/sf.c b/cmd/sf.c index 84bb057..cfea545 100644 --- a/cmd/sf.c +++ b/cmd/sf.c @@ -81,16 +81,18 @@ static int do_spi_flash_probe(int argc, char * const argv[]) { 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;
/* In DM mode, defaults will be taken from DT */
unsigned int speed = 0;
#ifdef CONFIG_DM_SPI_FLASH struct udevice *new, *bus_dev; int ret;unsigned int mode = 0; char *endp;
/* In DM mode defaults will be taken from DT */
#else struct spi_flash *new;speed = 0, mode = 0;
speed = CONFIG_SF_DEFAULT_SPEED;
#endifmode = CONFIG_SF_DEFAULT_MODE;
if (argc >= 2) {
diff --git a/common/spl/spl_spi.c b/common/spl/spl_spi.c index b348b45..c1c1fcb 100644 --- a/common/spl/spl_spi.c +++ b/common/spl/spl_spi.c @@ -74,12 +74,13 @@ static int spl_spi_load_image(struct spl_image_info *spl_image, unsigned payload_offs = CONFIG_SYS_SPI_U_BOOT_OFFS; struct spi_flash *flash; struct image_header *header;
unsigned int max_hz = CONFIG_SF_DEFAULT_SPEED;
unsigned int spi_mode = CONFIG_SF_DEFAULT_MODE;
/* In DM mode, defaults will be taken from DT */
unsigned int max_hz = 0;
unsigned int spi_mode = 0;
-#ifdef CONFIG_DM_SPI_FLASH
/* In DM mode defaults will be taken from DT */
max_hz = 0, spi_mode = 0;
+#ifndef CONFIG_DM_SPI_FLASH
max_hz = CONFIG_SF_DEFAULT_SPEED;
#endif /* * Load U-Boot image from SPI flash into RAMspi_mode = CONFIG_SF_DEFAULT_MODE;
diff --git a/common/splash_source.c b/common/splash_source.c index 427196c..d5d5550 100644 --- a/common/splash_source.c +++ b/common/splash_source.c @@ -24,12 +24,13 @@ DECLARE_GLOBAL_DATA_PTR; static struct spi_flash *sf; static int splash_sf_read_raw(u32 bmp_load_addr, int offset, size_t read_size) {
unsigned int max_hz = CONFIG_SF_DEFAULT_SPEED;
unsigned int spi_mode = CONFIG_SF_DEFAULT_MODE;
/* In DM mode, defaults will be taken from DT */
unsigned int max_hz = 0;
unsigned int spi_mode = 0;
-#ifdef CONFIG_DM_SPI_FLASH
/* In DM mode defaults will be taken from DT */
max_hz = 0, spi_mode = 0;
+#ifndef CONFIG_DM_SPI_FLASH
max_hz = CONFIG_SF_DEFAULT_SPEED;
#endifspi_mode = CONFIG_SF_DEFAULT_MODE;
if (!sf) {
Reviewed-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot

On Mon, Dec 10, 2018 at 4:23 PM Patrick Delaunay patrick.delaunay@st.com wrote:
The 2 default values for SPI mode and speed are only if CONFIG_DM_SPI_FLASH is not defined
- CONFIG_SF_DEFAULT_SPEED
- CONFIG_SF_DEFAULT_MODE
Inverse the logic of the test to remove these two defines.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
cmd/sf.c | 10 ++++++---- common/spl/spl_spi.c | 11 ++++++----- common/splash_source.c | 11 ++++++----- 3 files changed, 18 insertions(+), 14 deletions(-)
diff --git a/cmd/sf.c b/cmd/sf.c index 84bb057..cfea545 100644 --- a/cmd/sf.c +++ b/cmd/sf.c @@ -81,16 +81,18 @@ static int do_spi_flash_probe(int argc, char * const argv[]) { 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;
/* In DM mode, defaults will be taken from DT */
unsigned int speed = 0;
unsigned int mode = 0; char *endp;
#ifdef CONFIG_DM_SPI_FLASH struct udevice *new, *bus_dev; int ret;
/* In DM mode defaults will be taken from DT */
speed = 0, mode = 0;
#else struct spi_flash *new;
speed = CONFIG_SF_DEFAULT_SPEED;
mode = CONFIG_SF_DEFAULT_MODE;
Better define globally with in spi includes or some common include instead of making ifdef changes all over.

Add Kernel style documentation for spi_flash_probe_bus_cs().
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com ---
include/spi_flash.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/include/spi_flash.h b/include/spi_flash.h index e427e96..36565bb 100644 --- a/include/spi_flash.h +++ b/include/spi_flash.h @@ -180,6 +180,20 @@ int spi_flash_erase_dm(struct udevice *dev, u32 offset, size_t len); */ int spl_flash_get_sw_write_prot(struct udevice *dev);
+/** + * spi_flash_probe_bus_cs() - Find flash for selected SPI bus and chip select + * + * SPI bus probe and search flash chip for one chip select. + * + * @busnum: SPI bus identifier + * @cs: Chip select to look for + * @max_hz: Requested spi frequency, 0 = get value from platdata + * or device tree + * @spi_mode: Requested spi mode, 0 = get value from platdata + * or device tree + * @devp: Returns pointer to the flash device, if found + * @return 0 if found, -ve on error + */ int spi_flash_probe_bus_cs(unsigned int busnum, unsigned int cs, unsigned int max_hz, unsigned int spi_mode, struct udevice **devp);

On Mon, Dec 10, 2018 at 11:53 AM Patrick Delaunay patrick.delaunay@st.com wrote:
Add Kernel style documentation for spi_flash_probe_bus_cs().
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
include/spi_flash.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/include/spi_flash.h b/include/spi_flash.h index e427e96..36565bb 100644 --- a/include/spi_flash.h +++ b/include/spi_flash.h @@ -180,6 +180,20 @@ int spi_flash_erase_dm(struct udevice *dev, u32 offset, size_t len); */ int spl_flash_get_sw_write_prot(struct udevice *dev);
+/**
- spi_flash_probe_bus_cs() - Find flash for selected SPI bus and chip select
- SPI bus probe and search flash chip for one chip select.
- @busnum: SPI bus identifier
- @cs: Chip select to look for
- @max_hz: Requested spi frequency, 0 = get value from platdata
or device tree
- @spi_mode: Requested spi mode, 0 = get value from platdata
or device tree
- @devp: Returns pointer to the flash device, if found
- @return 0 if found, -ve on error
- */
int spi_flash_probe_bus_cs(unsigned int busnum, unsigned int cs, unsigned int max_hz, unsigned int spi_mode, struct udevice **devp);
Reviewed-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
Regards, Simon

Hi Patrick,
On Mon, Dec 10, 2018 at 11:53 AM Patrick Delaunay patrick.delaunay@st.com wrote:
Add Kernel style documentation for spi_flash_probe_bus_cs().
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
Reviewed-by: Petr Vorel petr.vorel@gmail.com
include/spi_flash.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
Kind regards, Petr

In case of DT boot, don't read default speed and mode for SPI from CONFIG_*, instead read from DT node.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com ---
board/davinci/da8xxevm/da850evm.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/board/davinci/da8xxevm/da850evm.c b/board/davinci/da8xxevm/da850evm.c index b0b29b3..4ef454e 100644 --- a/board/davinci/da8xxevm/da850evm.c +++ b/board/davinci/da8xxevm/da850evm.c @@ -44,8 +44,15 @@ DECLARE_GLOBAL_DATA_PTR;
#define CFG_MAC_ADDR_SPI_BUS 0 #define CFG_MAC_ADDR_SPI_CS 0 + +#ifdef CONFIG_DM_SPI_FLASH +/* In DM mode, speed and mode value will be taken from DT */ +#define CFG_MAC_ADDR_SPI_MAX_HZ 0 +#define CFG_MAC_ADDR_SPI_MODE 0 +#else #define CFG_MAC_ADDR_SPI_MAX_HZ CONFIG_SF_DEFAULT_SPEED #define CFG_MAC_ADDR_SPI_MODE SPI_MODE_3 +#endif
#define CFG_MAC_ADDR_OFFSET (flash->size - SZ_64K)

Hi Patrick,
In case of DT boot, don't read default speed and mode for SPI from CONFIG_*, instead read from DT node.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
Reviewed-by: Petr Vorel petr.vorel@gmail.com
Kind regards, Petr

+ Adam
On Mon, Dec 10, 2018 at 4:23 PM Patrick Delaunay patrick.delaunay@st.com wrote:
In case of DT boot, don't read default speed and mode for SPI from CONFIG_*, instead read from DT node.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
board/davinci/da8xxevm/da850evm.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/board/davinci/da8xxevm/da850evm.c b/board/davinci/da8xxevm/da850evm.c index b0b29b3..4ef454e 100644 --- a/board/davinci/da8xxevm/da850evm.c +++ b/board/davinci/da8xxevm/da850evm.c @@ -44,8 +44,15 @@ DECLARE_GLOBAL_DATA_PTR;
#define CFG_MAC_ADDR_SPI_BUS 0 #define CFG_MAC_ADDR_SPI_CS 0
+#ifdef CONFIG_DM_SPI_FLASH +/* In DM mode, speed and mode value will be taken from DT */ +#define CFG_MAC_ADDR_SPI_MAX_HZ 0 +#define CFG_MAC_ADDR_SPI_MODE 0 +#else #define CFG_MAC_ADDR_SPI_MAX_HZ CONFIG_SF_DEFAULT_SPEED #define CFG_MAC_ADDR_SPI_MODE SPI_MODE_3 +#endif
This board support DM_SPI_FLASH even in for SPL, so there is no need of non-dm if here.

Thanks Jagan for the reviews.
From: Jagan Teki jagan@amarulasolutions.com Sent: mercredi 12 décembre 2018 21:03
--- a/board/davinci/da8xxevm/da850evm.c +++ b/board/davinci/da8xxevm/da850evm.c
This board support DM_SPI_FLASH even in for SPL, so there is no need of non- dm if here.
I will do the update in v3
Patrick.

In case of DT boot, don't read default speed and mode for SPI from CONFIG_*, instead read from DT node.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com ---
drivers/dfu/dfu_sf.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/dfu/dfu_sf.c b/drivers/dfu/dfu_sf.c index 066e767..5e32f80 100644 --- a/drivers/dfu/dfu_sf.c +++ b/drivers/dfu/dfu_sf.c @@ -69,11 +69,17 @@ static struct spi_flash *parse_dev(char *devstr) { unsigned int bus; unsigned int cs; - unsigned int speed = CONFIG_SF_DEFAULT_SPEED; - unsigned int mode = CONFIG_SF_DEFAULT_MODE; + /* In DM mode, defaults will be taken from DT */ + unsigned int speed = 0; + unsigned int mode = 0; char *s, *endp; struct spi_flash *dev;
+#ifndef CONFIG_DM_SPI_FLASH + speed = CONFIG_SF_DEFAULT_SPEED; + mode = CONFIG_SF_DEFAULT_MODE; +#endif + s = strsep(&devstr, ":"); if (!s || !*s || (bus = simple_strtoul(s, &endp, 0), *endp)) { printf("Invalid SPI bus %s\n", s);

On Mon, 10 Dec 2018 11:52:43 +0100 Patrick Delaunay patrick.delaunay@st.com wrote:
In case of DT boot, don't read default speed and mode for SPI from CONFIG_*, instead read from DT node.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
drivers/dfu/dfu_sf.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/dfu/dfu_sf.c b/drivers/dfu/dfu_sf.c index 066e767..5e32f80 100644 --- a/drivers/dfu/dfu_sf.c +++ b/drivers/dfu/dfu_sf.c @@ -69,11 +69,17 @@ static struct spi_flash *parse_dev(char *devstr) { unsigned int bus; unsigned int cs;
- unsigned int speed = CONFIG_SF_DEFAULT_SPEED;
- unsigned int mode = CONFIG_SF_DEFAULT_MODE;
- /* In DM mode, defaults will be taken from DT */
- unsigned int speed = 0;
- unsigned int mode = 0; char *s, *endp; struct spi_flash *dev;
+#ifndef CONFIG_DM_SPI_FLASH
- speed = CONFIG_SF_DEFAULT_SPEED;
- mode = CONFIG_SF_DEFAULT_MODE;
+#endif
- s = strsep(&devstr, ":"); if (!s || !*s || (bus = simple_strtoul(s, &endp, 0), *endp))
{ printf("Invalid SPI bus %s\n", s);
Reviewed-by: Lukasz Majewski lukma@denx.de
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

Hi Patrick,
In case of DT boot, don't read default speed and mode for SPI from CONFIG_*, instead read from DT node.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
Reviewed-by: Petr Vorel petr.vorel@gmail.com
Kind regards, Petr

In case of DT boot, don't read default speed and mode for SPI from CONFIG_*, instead read from DT node.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com ---
board/gardena/smart-gateway-mt7688/board.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/board/gardena/smart-gateway-mt7688/board.c b/board/gardena/smart-gateway-mt7688/board.c index 6e11077..8a67e70 100644 --- a/board/gardena/smart-gateway-mt7688/board.c +++ b/board/gardena/smart-gateway-mt7688/board.c @@ -89,6 +89,14 @@ static void factory_data_env_config(void) u32 crc; int ret; u8 *ptr; + /* In DM mode, defaults will be taken from DT */ + unsigned int speed = 0; + unsigned int mode = 0; + +#ifndef CONFIG_DM_SPI_FLASH + speed = CONFIG_SF_DEFAULT_SPEED; + mode = CONFIG_SF_DEFAULT_MODE; +#endif
buf = malloc(FACTORY_DATA_SIZE); if (!buf) { @@ -101,8 +109,8 @@ static void factory_data_env_config(void) */ sf = spi_flash_probe(CONFIG_SF_DEFAULT_BUS, CONFIG_SF_DEFAULT_CS, - CONFIG_SF_DEFAULT_SPEED, - CONFIG_SF_DEFAULT_MODE); + speed, + mode); if (!sf) { printf("F-Data:Unable to access SPI NOR flash\n"); goto err_free; @@ -207,6 +215,14 @@ int do_fd_write(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) struct spi_flash *sf; u8 *buf; int ret = CMD_RET_FAILURE; + /* In DM mode, defaults will be taken from DT */ + unsigned int speed = 0; + unsigned int mode = 0; + +#ifndef CONFIG_DM_SPI_FLASH + speed = CONFIG_SF_DEFAULT_SPEED; + mode = CONFIG_SF_DEFAULT_MODE; +#endif
buf = malloc(FACTORY_DATA_SECT_SIZE); if (!buf) { @@ -216,8 +232,8 @@ int do_fd_write(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
sf = spi_flash_probe(CONFIG_SF_DEFAULT_BUS, CONFIG_SF_DEFAULT_CS, - CONFIG_SF_DEFAULT_SPEED, - CONFIG_SF_DEFAULT_MODE); + speed, + mode); if (!sf) { printf("F-Data:Unable to access SPI NOR flash\n"); goto err_free;

On 10.12.18 11:52, Patrick Delaunay wrote:
In case of DT boot, don't read default speed and mode for SPI from CONFIG_*, instead read from DT node.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
board/gardena/smart-gateway-mt7688/board.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/board/gardena/smart-gateway-mt7688/board.c b/board/gardena/smart-gateway-mt7688/board.c index 6e11077..8a67e70 100644 --- a/board/gardena/smart-gateway-mt7688/board.c +++ b/board/gardena/smart-gateway-mt7688/board.c @@ -89,6 +89,14 @@ static void factory_data_env_config(void) u32 crc; int ret; u8 *ptr;
- /* In DM mode, defaults will be taken from DT */
- unsigned int speed = 0;
- unsigned int mode = 0;
+#ifndef CONFIG_DM_SPI_FLASH
- speed = CONFIG_SF_DEFAULT_SPEED;
- mode = CONFIG_SF_DEFAULT_MODE;
+#endif
CONFIG_DM_SPI_FLASH is always enabled in this board. So there should be no need to handle the non-DM case at all.
Could you please change this patch and remove this unnecessary code?
Thanks, Stefan
buf = malloc(FACTORY_DATA_SIZE); if (!buf) { @@ -101,8 +109,8 @@ static void factory_data_env_config(void) */ sf = spi_flash_probe(CONFIG_SF_DEFAULT_BUS, CONFIG_SF_DEFAULT_CS,
CONFIG_SF_DEFAULT_SPEED,
CONFIG_SF_DEFAULT_MODE);
speed,
if (!sf) { printf("F-Data:Unable to access SPI NOR flash\n"); goto err_free;mode);
@@ -207,6 +215,14 @@ int do_fd_write(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) struct spi_flash *sf; u8 *buf; int ret = CMD_RET_FAILURE;
- /* In DM mode, defaults will be taken from DT */
- unsigned int speed = 0;
- unsigned int mode = 0;
+#ifndef CONFIG_DM_SPI_FLASH
- speed = CONFIG_SF_DEFAULT_SPEED;
- mode = CONFIG_SF_DEFAULT_MODE;
+#endif
buf = malloc(FACTORY_DATA_SECT_SIZE); if (!buf) { @@ -216,8 +232,8 @@ int do_fd_write(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
sf = spi_flash_probe(CONFIG_SF_DEFAULT_BUS, CONFIG_SF_DEFAULT_CS,
CONFIG_SF_DEFAULT_SPEED,
CONFIG_SF_DEFAULT_MODE);
speed,
if (!sf) { printf("F-Data:Unable to access SPI NOR flash\n"); goto err_free;mode);
Viele Grüße, Stefan

Hi Patrick,
In case of DT boot, don't read default speed and mode for SPI from CONFIG_*, instead read from DT node.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
Reviewed-by: Petr Vorel petr.vorel@gmail.com
Kind regards, Petr

In case of DT boot, don't read default speed and mode for SPI from CONFIG_*, instead read from DT node.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com ---
env/Kconfig | 4 ++-- env/sf.c | 5 ++++- 2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/env/Kconfig b/env/Kconfig index 9011109..0f760ce 100644 --- a/env/Kconfig +++ b/env/Kconfig @@ -329,11 +329,11 @@ config ENV_IS_IN_SPI_FLASH
Define the SPI bus and chip select. If not defined they will be 0.
- - CONFIG_ENV_SPI_MAX_HZ (optional): + - CONFIG_ENV_SPI_MAX_HZ (optional if !DM_SPI_FLASH):
Define the SPI max work clock. If not defined then use 1MHz.
- - CONFIG_ENV_SPI_MODE (optional): + - CONFIG_ENV_SPI_MODE (optional if !DM_SPI_FLASH):
Define the SPI work mode. If not defined then use SPI_MODE_3.
diff --git a/env/sf.c b/env/sf.c index 2e3c600..edd36f1 100644 --- a/env/sf.c +++ b/env/sf.c @@ -24,12 +24,15 @@ #ifndef CONFIG_ENV_SPI_CS # define CONFIG_ENV_SPI_CS CONFIG_SF_DEFAULT_CS #endif + +#ifndef CONFIG_DM_SPI_FLASH #ifndef CONFIG_ENV_SPI_MAX_HZ # define CONFIG_ENV_SPI_MAX_HZ CONFIG_SF_DEFAULT_SPEED #endif #ifndef CONFIG_ENV_SPI_MODE # define CONFIG_ENV_SPI_MODE CONFIG_SF_DEFAULT_MODE #endif +#endif
#ifndef CONFIG_SPL_BUILD #define CMD_SAVEENV @@ -58,7 +61,7 @@ static int setup_flash_device(void)
/* speed and mode will be read from DT */ ret = spi_flash_probe_bus_cs(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS, - CONFIG_ENV_SPI_MAX_HZ, CONFIG_ENV_SPI_MODE, + 0, 0, &new); if (ret) { set_default_env("spi_flash_probe_bus_cs() failed", 0);

On Mon, 10 Dec 2018 11:52:45 +0100 Patrick Delaunay patrick.delaunay@st.com wrote:
In case of DT boot, don't read default speed and mode for SPI from CONFIG_*, instead read from DT node.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
env/Kconfig | 4 ++-- env/sf.c | 5 ++++- 2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/env/Kconfig b/env/Kconfig index 9011109..0f760ce 100644 --- a/env/Kconfig +++ b/env/Kconfig @@ -329,11 +329,11 @@ config ENV_IS_IN_SPI_FLASH
Define the SPI bus and chip select. If not defined they
will be 0.
- CONFIG_ENV_SPI_MAX_HZ (optional):
- CONFIG_ENV_SPI_MAX_HZ (optional if !DM_SPI_FLASH):
Define the SPI max work clock. If not defined then use
1MHz.
- CONFIG_ENV_SPI_MODE (optional):
- CONFIG_ENV_SPI_MODE (optional if !DM_SPI_FLASH):
Define the SPI work mode. If not defined then use
SPI_MODE_3. diff --git a/env/sf.c b/env/sf.c index 2e3c600..edd36f1 100644 --- a/env/sf.c +++ b/env/sf.c @@ -24,12 +24,15 @@ #ifndef CONFIG_ENV_SPI_CS # define CONFIG_ENV_SPI_CS CONFIG_SF_DEFAULT_CS #endif
+#ifndef CONFIG_DM_SPI_FLASH #ifndef CONFIG_ENV_SPI_MAX_HZ # define CONFIG_ENV_SPI_MAX_HZ CONFIG_SF_DEFAULT_SPEED #endif #ifndef CONFIG_ENV_SPI_MODE # define CONFIG_ENV_SPI_MODE CONFIG_SF_DEFAULT_MODE #endif +#endif
#ifndef CONFIG_SPL_BUILD #define CMD_SAVEENV @@ -58,7 +61,7 @@ static int setup_flash_device(void)
/* speed and mode will be read from DT */ ret = spi_flash_probe_bus_cs(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS,
CONFIG_ENV_SPI_MAX_HZ,
CONFIG_ENV_SPI_MODE,
if (ret) { set_default_env("spi_flash_probe_bus_cs() failed",0, 0, &new);
0);
Reviewed-by: Lukasz Majewski lukma@denx.de
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

On Mon, Dec 10, 2018 at 11:54 AM Patrick Delaunay patrick.delaunay@st.com wrote:
In case of DT boot, don't read default speed and mode for SPI from CONFIG_*, instead read from DT node.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
env/Kconfig | 4 ++-- env/sf.c | 5 ++++- 2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/env/Kconfig b/env/Kconfig index 9011109..0f760ce 100644 --- a/env/Kconfig +++ b/env/Kconfig @@ -329,11 +329,11 @@ config ENV_IS_IN_SPI_FLASH
Define the SPI bus and chip select. If not defined they will be 0.
- CONFIG_ENV_SPI_MAX_HZ (optional):
- CONFIG_ENV_SPI_MAX_HZ (optional if !DM_SPI_FLASH): Define the SPI max work clock. If not defined then use 1MHz.
- CONFIG_ENV_SPI_MODE (optional):
- CONFIG_ENV_SPI_MODE (optional if !DM_SPI_FLASH): Define the SPI work mode. If not defined then use SPI_MODE_3.
diff --git a/env/sf.c b/env/sf.c index 2e3c600..edd36f1 100644 --- a/env/sf.c +++ b/env/sf.c @@ -24,12 +24,15 @@ #ifndef CONFIG_ENV_SPI_CS # define CONFIG_ENV_SPI_CS CONFIG_SF_DEFAULT_CS #endif
+#ifndef CONFIG_DM_SPI_FLASH #ifndef CONFIG_ENV_SPI_MAX_HZ # define CONFIG_ENV_SPI_MAX_HZ CONFIG_SF_DEFAULT_SPEED #endif #ifndef CONFIG_ENV_SPI_MODE # define CONFIG_ENV_SPI_MODE CONFIG_SF_DEFAULT_MODE #endif +#endif
#ifndef CONFIG_SPL_BUILD #define CMD_SAVEENV @@ -58,7 +61,7 @@ static int setup_flash_device(void)
/* speed and mode will be read from DT */ ret = spi_flash_probe_bus_cs(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS,
CONFIG_ENV_SPI_MAX_HZ, CONFIG_ENV_SPI_MODE,
0, 0, &new); if (ret) { set_default_env("spi_flash_probe_bus_cs() failed", 0);
Reviewed-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
Regards, Simon

Hi Patrick,
In case of DT boot, don't read default speed and mode for SPI from CONFIG_*, instead read from DT node.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
Reviewed-by: Petr Vorel petr.vorel@gmail.com
env/Kconfig | 4 ++-- env/sf.c | 5 ++++- 2 files changed, 6 insertions(+), 3 deletions(-)
...
+++ b/env/sf.c @@ -24,12 +24,15 @@ #ifndef CONFIG_ENV_SPI_CS # define CONFIG_ENV_SPI_CS CONFIG_SF_DEFAULT_CS #endif
+#ifndef CONFIG_DM_SPI_FLASH #ifndef CONFIG_ENV_SPI_MAX_HZ # define CONFIG_ENV_SPI_MAX_HZ CONFIG_SF_DEFAULT_SPEED #endif #ifndef CONFIG_ENV_SPI_MODE # define CONFIG_ENV_SPI_MODE CONFIG_SF_DEFAULT_MODE #endif +#endif
Maybe indent? (code style mix indent and not) #ifndef CONFIG_DM_SPI_FLASH # ifndef CONFIG_ENV_SPI_MAX_HZ # define CONFIG_ENV_SPI_MAX_HZ CONFIG_SF_DEFAULT_SPEED # endif # ifndef CONFIG_ENV_SPI_MODE # define CONFIG_ENV_SPI_MODE CONFIG_SF_DEFAULT_MODE # endif #endif
Kind regards, Petr

On Mon, Dec 10, 2018 at 4:23 PM Patrick Delaunay patrick.delaunay@st.com wrote:
In case of DT boot, don't read default speed and mode for SPI from CONFIG_*, instead read from DT node.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
env/Kconfig | 4 ++-- env/sf.c | 5 ++++- 2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/env/Kconfig b/env/Kconfig index 9011109..0f760ce 100644 --- a/env/Kconfig +++ b/env/Kconfig @@ -329,11 +329,11 @@ config ENV_IS_IN_SPI_FLASH
Define the SPI bus and chip select. If not defined they will be 0.
- CONFIG_ENV_SPI_MAX_HZ (optional):
- CONFIG_ENV_SPI_MAX_HZ (optional if !DM_SPI_FLASH): Define the SPI max work clock. If not defined then use 1MHz.
- CONFIG_ENV_SPI_MODE (optional):
- CONFIG_ENV_SPI_MODE (optional if !DM_SPI_FLASH): Define the SPI work mode. If not defined then use SPI_MODE_3.
diff --git a/env/sf.c b/env/sf.c index 2e3c600..edd36f1 100644 --- a/env/sf.c +++ b/env/sf.c @@ -24,12 +24,15 @@ #ifndef CONFIG_ENV_SPI_CS # define CONFIG_ENV_SPI_CS CONFIG_SF_DEFAULT_CS #endif
+#ifndef CONFIG_DM_SPI_FLASH #ifndef CONFIG_ENV_SPI_MAX_HZ # define CONFIG_ENV_SPI_MAX_HZ CONFIG_SF_DEFAULT_SPEED #endif #ifndef CONFIG_ENV_SPI_MODE # define CONFIG_ENV_SPI_MODE CONFIG_SF_DEFAULT_MODE #endif +#endif
Please try trim it for better, adding ifdef on top ifdef look, not good. see my comment on 1/7

In DM mode, the speed and mode defaults value will be taken from DT, so these defines should be never used and can be removed.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com ---
include/spi_flash.h | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/include/spi_flash.h b/include/spi_flash.h index 36565bb..c9d20a5 100644 --- a/include/spi_flash.h +++ b/include/spi_flash.h @@ -12,12 +12,16 @@ #include <dm.h> /* Because we dereference struct udevice here */ #include <linux/types.h>
+#ifndef CONFIG_DM_SPI_FLASH +/* In DM mode, speed and mode value will be taken from DT */ #ifndef CONFIG_SF_DEFAULT_SPEED # define CONFIG_SF_DEFAULT_SPEED 1000000 #endif #ifndef CONFIG_SF_DEFAULT_MODE # define CONFIG_SF_DEFAULT_MODE SPI_MODE_3 #endif +#endif + #ifndef CONFIG_SF_DEFAULT_CS # define CONFIG_SF_DEFAULT_CS 0 #endif

On Mon, Dec 10, 2018 at 11:53 AM Patrick Delaunay patrick.delaunay@st.com wrote:
In DM mode, the speed and mode defaults value will be taken from DT, so these defines should be never used and can be removed.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
include/spi_flash.h | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/include/spi_flash.h b/include/spi_flash.h index 36565bb..c9d20a5 100644 --- a/include/spi_flash.h +++ b/include/spi_flash.h @@ -12,12 +12,16 @@ #include <dm.h> /* Because we dereference struct udevice here */ #include <linux/types.h>
+#ifndef CONFIG_DM_SPI_FLASH +/* In DM mode, speed and mode value will be taken from DT */ #ifndef CONFIG_SF_DEFAULT_SPEED # define CONFIG_SF_DEFAULT_SPEED 1000000 #endif #ifndef CONFIG_SF_DEFAULT_MODE # define CONFIG_SF_DEFAULT_MODE SPI_MODE_3 #endif +#endif
#ifndef CONFIG_SF_DEFAULT_CS # define CONFIG_SF_DEFAULT_CS 0 #endif
Reviewed-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
Regards, Simon

Hi Patrick,
On Mon, Dec 10, 2018 at 11:53 AM Patrick Delaunay patrick.delaunay@st.com wrote:
In DM mode, the speed and mode defaults value will be taken from DT, so these defines should be never used and can be removed.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
Reviewed-by: Petr Vorel petr.vorel@gmail.com
include/spi_flash.h | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/include/spi_flash.h b/include/spi_flash.h index 36565bb..c9d20a5 100644 --- a/include/spi_flash.h +++ b/include/spi_flash.h @@ -12,12 +12,16 @@ #include <dm.h> /* Because we dereference struct udevice here */ #include <linux/types.h>
+#ifndef CONFIG_DM_SPI_FLASH +/* In DM mode, speed and mode value will be taken from DT */ #ifndef CONFIG_SF_DEFAULT_SPEED # define CONFIG_SF_DEFAULT_SPEED 1000000 #endif #ifndef CONFIG_SF_DEFAULT_MODE # define CONFIG_SF_DEFAULT_MODE SPI_MODE_3 #endif +#endif
Also: maybe indent preprocessor code?
Kind regards, Petr

On Mon, Dec 10, 2018 at 4:54 AM Patrick Delaunay patrick.delaunay@st.com wrote:
This patch-set finish (after the last Simmon comment) and depends on the previous serie named: "Read default speed and mode values from DT"
http://patchwork.ozlabs.org/project/uboot/list/?series=76834
This patchset remove the unnecessary defines for SPI speed and mode when DM mode is used for SPI (CONFIG_DM_SPI_FLASH).
Build result is available for the serie in gihub and travis: https://github.com/patrickdelaunay/u-boot/tree/topic/spi https://travis-ci.org/patrickdelaunay/u-boot/branches
After applying both of your patches, the da850evm no longer boots from SPI flash.
U-Boot SPL 2019.01-rc1-00754-g21fb8c41b1 (Dec 10 2018 - 09:02:28 -0600) Trying to boot from SPI Warning: SPI speed fallback to 100 kHz
[repeat in perpetuity]
U-Boot SPL 2019.01-rc1-00754-g21fb8c41b1 (Dec 10 2018 - 09:02:28 -0600) Trying to boot from SPI Warning: SPI speed fallback to 100 kHz
I then tried to apply the first patch, and it also fails booting.
I'll reply to the original series with a similar message. Looking at the comments, it looks like the code is assuming the DM_SPI_FLASH means DT when in reality, the da850evm's SPL is using platdata. I looked at the platdata structure and I didn't see entries for SPI mode or speed. However, I cannot prove this is the issue on the da850evm without diving into it a bit more.
adam
Patrick Delaunay (7): spi: update management of default speed and mode spi: add comment for spi_flash_probe_bus_cs function da850evm: sf: Read default speed and mode values from DT dfu: sf: Read default speed and mode values from DT mips: mt76xx: gardena-smart-gateway: Read default speed and mode values from DT env: Read default speed and mode values from DT spi: remove define for SPI default SPEED and MODE
board/davinci/da8xxevm/da850evm.c | 7 +++++++ board/gardena/smart-gateway-mt7688/board.c | 24 ++++++++++++++++++++---- cmd/sf.c | 10 ++++++---- common/spl/spl_spi.c | 11 ++++++----- common/splash_source.c | 11 ++++++----- drivers/dfu/dfu_sf.c | 10 ++++++++-- env/Kconfig | 4 ++-- env/sf.c | 5 ++++- include/spi_flash.h | 18 ++++++++++++++++++ 9 files changed, 77 insertions(+), 23 deletions(-)
-- 2.7.4

Am 10.12.2018 um 16:42 schrieb Adam Ford:
On Mon, Dec 10, 2018 at 4:54 AM Patrick Delaunay patrick.delaunay@st.com wrote:
This patch-set finish (after the last Simmon comment) and depends on the previous serie named: "Read default speed and mode values from DT"
http://patchwork.ozlabs.org/project/uboot/list/?series=76834
This patchset remove the unnecessary defines for SPI speed and mode when DM mode is used for SPI (CONFIG_DM_SPI_FLASH).
Build result is available for the serie in gihub and travis: https://github.com/patrickdelaunay/u-boot/tree/topic/spi https://travis-ci.org/patrickdelaunay/u-boot/branches
After applying both of your patches, the da850evm no longer boots from SPI flash.
I just did test this and it worked for me. Booting from QSPI on socfpga_socrates and loading something in U-Boot via sf probe/sf read, so: Tested-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
U-Boot SPL 2019.01-rc1-00754-g21fb8c41b1 (Dec 10 2018 - 09:02:28 -0600) Trying to boot from SPI Warning: SPI speed fallback to 100 kHz
[repeat in perpetuity]
U-Boot SPL 2019.01-rc1-00754-g21fb8c41b1 (Dec 10 2018 - 09:02:28 -0600) Trying to boot from SPI Warning: SPI speed fallback to 100 kHz
I then tried to apply the first patch, and it also fails booting.
I'll reply to the original series with a similar message. Looking at the comments, it looks like the code is assuming the DM_SPI_FLASH means DT when in reality, the da850evm's SPL is using platdata. I looked at the platdata structure and I didn't see entries for SPI mode or speed. However, I cannot prove this is the issue on the da850evm without diving into it a bit more.
Adam, from reading the da850-emv.dts and its '-u-boot.dtsi', it seems the fdtgrep indicators to keep the flash for the SPL dts are missing (compare to socfpga_cyclone5_socrates-u-boot.dtsi). In this case, the code does not find a flash node.
The same would of course apply when using platdata.
Regards, Simon
adam
Patrick Delaunay (7): spi: update management of default speed and mode spi: add comment for spi_flash_probe_bus_cs function da850evm: sf: Read default speed and mode values from DT dfu: sf: Read default speed and mode values from DT mips: mt76xx: gardena-smart-gateway: Read default speed and mode values from DT env: Read default speed and mode values from DT spi: remove define for SPI default SPEED and MODE
board/davinci/da8xxevm/da850evm.c | 7 +++++++ board/gardena/smart-gateway-mt7688/board.c | 24 ++++++++++++++++++++---- cmd/sf.c | 10 ++++++---- common/spl/spl_spi.c | 11 ++++++----- common/splash_source.c | 11 ++++++----- drivers/dfu/dfu_sf.c | 10 ++++++++-- env/Kconfig | 4 ++-- env/sf.c | 5 ++++- include/spi_flash.h | 18 ++++++++++++++++++ 9 files changed, 77 insertions(+), 23 deletions(-)
-- 2.7.4

On Mon, Dec 10, 2018 at 1:20 PM Simon Goldschmidt simon.k.r.goldschmidt@gmail.com wrote:
Am 10.12.2018 um 16:42 schrieb Adam Ford:
On Mon, Dec 10, 2018 at 4:54 AM Patrick Delaunay patrick.delaunay@st.com wrote:
This patch-set finish (after the last Simmon comment) and depends on the previous serie named: "Read default speed and mode values from DT"
http://patchwork.ozlabs.org/project/uboot/list/?series=76834
This patchset remove the unnecessary defines for SPI speed and mode when DM mode is used for SPI (CONFIG_DM_SPI_FLASH).
Build result is available for the serie in gihub and travis: https://github.com/patrickdelaunay/u-boot/tree/topic/spi https://travis-ci.org/patrickdelaunay/u-boot/branches
After applying both of your patches, the da850evm no longer boots from SPI flash.
I just did test this and it worked for me. Booting from QSPI on socfpga_socrates and loading something in U-Boot via sf probe/sf read, so: Tested-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
U-Boot SPL 2019.01-rc1-00754-g21fb8c41b1 (Dec 10 2018 - 09:02:28 -0600) Trying to boot from SPI Warning: SPI speed fallback to 100 kHz
[repeat in perpetuity]
U-Boot SPL 2019.01-rc1-00754-g21fb8c41b1 (Dec 10 2018 - 09:02:28 -0600) Trying to boot from SPI Warning: SPI speed fallback to 100 kHz
I then tried to apply the first patch, and it also fails booting.
I'll reply to the original series with a similar message. Looking at the comments, it looks like the code is assuming the DM_SPI_FLASH means DT when in reality, the da850evm's SPL is using platdata. I looked at the platdata structure and I didn't see entries for SPI mode or speed. However, I cannot prove this is the issue on the da850evm without diving into it a bit more.
Adam, from reading the da850-emv.dts and its '-u-boot.dtsi', it seems the fdtgrep indicators to keep the flash for the SPL dts are missing (compare to socfpga_cyclone5_socrates-u-boot.dtsi). In this case, the code does not find a flash node.
The same would of course apply when using platdata.
I tried modifying the -u-boot.dtsi file to look like:
&flash { compatible = "m25p64", "spi-flash"; u-boot,dm-pre-reloc; };
Unfortunately, I not get some build errors. Do you have any suggestions? I know the dtsi didn't need the "u-boot,dm-pre-reloc;" before this patch. Might there be a platdata example I can follow?
The build errors as as follows:
make[2]: 'arch/arm/dts/da850-evm.dtb' is up to date. CAT u-boot-dtb.bin COPY u-boot.bin MKIMAGE u-boot.img MKIMAGE u-boot-dtb.img DTOC C spl/dts/dt-platdata.c DTOC H include/generated/dt-structs-gen.h Traceback (most recent call last): Traceback (most recent call last): File "./tools/dtoc/dtoc", line 109, in <module> File "./tools/dtoc/dtoc", line 109, in <module> options.output) options.output) File "/home/aford/src/u-boot/tools/dtoc/dtb_platdata.py", line 564, in run_steps File "/home/aford/src/u-boot/tools/dtoc/dtb_platdata.py", line 564, in run_steps plat.scan_reg_sizes() plat.scan_reg_sizes() File "/home/aford/src/u-boot/tools/dtoc/dtb_platdata.py", line 335, in scan_reg_sizes File "/home/aford/src/u-boot/tools/dtoc/dtb_platdata.py", line 335, in scan_reg_sizes addr = fdt_util.fdt_cells_to_cpu(val[i:], reg.na) addr = fdt_util.fdt_cells_to_cpu(val[i:], reg.na) File "/home/aford/src/u-boot/tools/dtoc/fdt_util.py", line 54, in fdt_cells_to_cpu File "/home/aford/src/u-boot/tools/dtoc/fdt_util.py", line 54, in fdt_cells_to_cpu out = out << 32 | fdt32_to_cpu(val[1]) out = out << 32 | fdt32_to_cpu(val[1]) IndexErrorIndexError: : list index out of rangelist index out of range
scripts/Makefile.spl:287: recipe for target 'spl/dts/dt-platdata.c' failed make[1]: *** [spl/dts/dt-platdata.c] Error 1 make[1]: *** Waiting for unfinished jobs.... scripts/Makefile.spl:284: recipe for target 'include/generated/dt-structs-gen.h' failed make[1]: *** [include/generated/dt-structs-gen.h] Error 1 Makefile:1591: recipe for target 'spl/u-boot-spl' failed make: *** [spl/u-boot-spl] Error 2
Regards, Simon
adam
Patrick Delaunay (7): spi: update management of default speed and mode spi: add comment for spi_flash_probe_bus_cs function da850evm: sf: Read default speed and mode values from DT dfu: sf: Read default speed and mode values from DT mips: mt76xx: gardena-smart-gateway: Read default speed and mode values from DT env: Read default speed and mode values from DT spi: remove define for SPI default SPEED and MODE
board/davinci/da8xxevm/da850evm.c | 7 +++++++ board/gardena/smart-gateway-mt7688/board.c | 24 ++++++++++++++++++++---- cmd/sf.c | 10 ++++++---- common/spl/spl_spi.c | 11 ++++++----- common/splash_source.c | 11 ++++++----- drivers/dfu/dfu_sf.c | 10 ++++++++-- env/Kconfig | 4 ++-- env/sf.c | 5 ++++- include/spi_flash.h | 18 ++++++++++++++++++ 9 files changed, 77 insertions(+), 23 deletions(-)
-- 2.7.4

Am 10.12.2018 um 21:17 schrieb Adam Ford:
On Mon, Dec 10, 2018 at 1:20 PM Simon Goldschmidt simon.k.r.goldschmidt@gmail.com wrote:
Am 10.12.2018 um 16:42 schrieb Adam Ford:
On Mon, Dec 10, 2018 at 4:54 AM Patrick Delaunay patrick.delaunay@st.com wrote:
This patch-set finish (after the last Simmon comment) and depends on the previous serie named: "Read default speed and mode values from DT"
http://patchwork.ozlabs.org/project/uboot/list/?series=76834
This patchset remove the unnecessary defines for SPI speed and mode when DM mode is used for SPI (CONFIG_DM_SPI_FLASH).
Build result is available for the serie in gihub and travis: https://github.com/patrickdelaunay/u-boot/tree/topic/spi https://travis-ci.org/patrickdelaunay/u-boot/branches
After applying both of your patches, the da850evm no longer boots from SPI flash.
I just did test this and it worked for me. Booting from QSPI on socfpga_socrates and loading something in U-Boot via sf probe/sf read, so: Tested-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
U-Boot SPL 2019.01-rc1-00754-g21fb8c41b1 (Dec 10 2018 - 09:02:28 -0600) Trying to boot from SPI Warning: SPI speed fallback to 100 kHz
[repeat in perpetuity]
U-Boot SPL 2019.01-rc1-00754-g21fb8c41b1 (Dec 10 2018 - 09:02:28 -0600) Trying to boot from SPI Warning: SPI speed fallback to 100 kHz
I then tried to apply the first patch, and it also fails booting.
I'll reply to the original series with a similar message. Looking at the comments, it looks like the code is assuming the DM_SPI_FLASH means DT when in reality, the da850evm's SPL is using platdata. I looked at the platdata structure and I didn't see entries for SPI mode or speed. However, I cannot prove this is the issue on the da850evm without diving into it a bit more.
Adam, from reading the da850-emv.dts and its '-u-boot.dtsi', it seems the fdtgrep indicators to keep the flash for the SPL dts are missing (compare to socfpga_cyclone5_socrates-u-boot.dtsi). In this case, the code does not find a flash node.
The same would of course apply when using platdata.
I tried modifying the -u-boot.dtsi file to look like:
&flash { compatible = "m25p64", "spi-flash"; u-boot,dm-pre-reloc; };
That should be correct when using DTS. I have no idea about the platform data, sorry. That's a todo on my list to see if it gives me enough memory to enable FIT/signature support in my SPL, but I haven't got to compile it, yet, as cadence QSPI does not seem to support platdata, yet.
Unfortunately, I not get some build errors. Do you have any suggestions? I know the dtsi didn't need the "u-boot,dm-pre-reloc;" before this patch.
Well, that's only partly true. You need this information in SPL unless you go the hacky way of the define that Patrick wants to remove here. And the standard way to get this information into SPL is the devicetree.
I think Patrick did the right move to send this series so that we can drop the two defines and get the values from dts. Now platdata seems to be missing, so this must be solved.
Might there be a platdata example I can follow?
As written above, I don't know of any. Even without build errors, I'm not sure it would work...
Regards, Simon
The build errors as as follows:
make[2]: 'arch/arm/dts/da850-evm.dtb' is up to date. CAT u-boot-dtb.bin COPY u-boot.bin MKIMAGE u-boot.img MKIMAGE u-boot-dtb.img DTOC C spl/dts/dt-platdata.c DTOC H include/generated/dt-structs-gen.h Traceback (most recent call last): Traceback (most recent call last): File "./tools/dtoc/dtoc", line 109, in <module> File "./tools/dtoc/dtoc", line 109, in <module> options.output) options.output) File "/home/aford/src/u-boot/tools/dtoc/dtb_platdata.py", line 564, in run_steps File "/home/aford/src/u-boot/tools/dtoc/dtb_platdata.py", line 564, in run_steps plat.scan_reg_sizes() plat.scan_reg_sizes() File "/home/aford/src/u-boot/tools/dtoc/dtb_platdata.py", line 335, in scan_reg_sizes File "/home/aford/src/u-boot/tools/dtoc/dtb_platdata.py", line 335, in scan_reg_sizes addr = fdt_util.fdt_cells_to_cpu(val[i:], reg.na) addr = fdt_util.fdt_cells_to_cpu(val[i:], reg.na) File "/home/aford/src/u-boot/tools/dtoc/fdt_util.py", line 54, in fdt_cells_to_cpu File "/home/aford/src/u-boot/tools/dtoc/fdt_util.py", line 54, in fdt_cells_to_cpu out = out << 32 | fdt32_to_cpu(val[1]) out = out << 32 | fdt32_to_cpu(val[1]) IndexErrorIndexError: : list index out of rangelist index out of range
scripts/Makefile.spl:287: recipe for target 'spl/dts/dt-platdata.c' failed make[1]: *** [spl/dts/dt-platdata.c] Error 1 make[1]: *** Waiting for unfinished jobs.... scripts/Makefile.spl:284: recipe for target 'include/generated/dt-structs-gen.h' failed make[1]: *** [include/generated/dt-structs-gen.h] Error 1 Makefile:1591: recipe for target 'spl/u-boot-spl' failed make: *** [spl/u-boot-spl] Error 2
Regards, Simon
adam
Patrick Delaunay (7): spi: update management of default speed and mode spi: add comment for spi_flash_probe_bus_cs function da850evm: sf: Read default speed and mode values from DT dfu: sf: Read default speed and mode values from DT mips: mt76xx: gardena-smart-gateway: Read default speed and mode values from DT env: Read default speed and mode values from DT spi: remove define for SPI default SPEED and MODE
board/davinci/da8xxevm/da850evm.c | 7 +++++++ board/gardena/smart-gateway-mt7688/board.c | 24 ++++++++++++++++++++---- cmd/sf.c | 10 ++++++---- common/spl/spl_spi.c | 11 ++++++----- common/splash_source.c | 11 ++++++----- drivers/dfu/dfu_sf.c | 10 ++++++++-- env/Kconfig | 4 ++-- env/sf.c | 5 ++++- include/spi_flash.h | 18 ++++++++++++++++++ 9 files changed, 77 insertions(+), 23 deletions(-)
-- 2.7.4

Hi Patrick,
FYI this patchset breaks some machines: https://travis-ci.org/pevik/u-boot/builds/466192406
Kind regards, Petr

On Wed, Dec 12, 2018 at 8:51 AM Petr Vorel petr.vorel@gmail.com wrote:
Hi Patrick,
FYI this patchset breaks some machines: https://travis-ci.org/pevik/u-boot/builds/466192406
I might be wrong, but I think that build log indicates an error on a line that should not exist any more after applying this series and the one the OP says it's based on: http://patchwork.ozlabs.org/project/uboot/list/?series=76834
Regards, Simon

Hi Petr,
From: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com Sent: mercredi 12 décembre 2018 10:55 To: Petr Vorel petr.vorel@gmail.com
On Wed, Dec 12, 2018 at 8:51 AM Petr Vorel petr.vorel@gmail.com wrote:
Hi Patrick,
FYI this patchset breaks some machines: https://travis-ci.org/pevik/u-boot/builds/466192406
I might be wrong, but I think that build log indicates an error on a line that should not exist any more after applying this series and the one the OP says it's based on: http://patchwork.ozlabs.org/project/uboot/list/?series=76834
Yes I think I was not enough clear comment
The new serie depends on the previous serie named: "Read default speed and mode values from DT"
You need to apllys this patchset : http://patchwork.ozlabs.org/project/uboot/list/?series=76834
Regards, Simon
Regards, Patrick
participants (8)
-
Adam Ford
-
Jagan Teki
-
Lukasz Majewski
-
Patrick DELAUNAY
-
Patrick Delaunay
-
Petr Vorel
-
Simon Goldschmidt
-
Stefan Roese