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

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

[Patrick Delaunay]

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(-)

-- 
2.7.4

[U-Boot] [PATCH 1/7] spi: update management of default speed and mode

[Patrick Delaunay]

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) {
-- 
2.7.4

[U-Boot] [PATCH 2/7] spi: add comment for spi_flash_probe_bus_cs function

[Patrick Delaunay]

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);
-- 
2.7.4

[U-Boot] [PATCH 3/7] da850evm: sf: Read default speed and mode values from DT

[Patrick Delaunay]

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)
 
-- 
2.7.4

[U-Boot] [PATCH 4/7] dfu: sf: Read default speed and mode values from DT

[Patrick Delaunay]

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);
-- 
2.7.4

[U-Boot] [PATCH 5/7] mips: mt76xx: gardena-smart-gateway: Read default speed and mode values from DT

[Patrick Delaunay]

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;
-- 
2.7.4

[U-Boot] [PATCH 6/7] env: Read default speed and mode values from DT

[Patrick Delaunay]

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);
-- 
2.7.4

[U-Boot] [PATCH 7/7] spi: remove define for SPI default SPEED and MODE

[Patrick Delaunay]

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
-- 
2.7.4

[U-Boot] [PATCH 4/7] dfu: sf: Read default speed and mode values from DT

[Lukasz Majewski]

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 at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20181210/0cbb7b5b/attachment.sig>

[U-Boot] [PATCH 6/7] env: Read default speed and mode values from DT

[Lukasz Majewski]

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,
> +				     0, 0,
>  				     &new);
>  	if (ret) {
>  		set_default_env("spi_flash_probe_bus_cs() failed",
> 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 at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20181210/0a8388d1/attachment.sig>

[U-Boot] [PATCH 1/7] spi: update management of default speed and mode

[Simon Goldschmidt]

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

[U-Boot] [PATCH 2/7] spi: add comment for spi_flash_probe_bus_cs function

[Simon Goldschmidt]

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

[U-Boot] [PATCH 7/7] spi: remove define for SPI default SPEED and MODE

[Simon Goldschmidt]

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

[U-Boot] [PATCH 6/7] env: Read default speed and mode values from DT

[Simon Goldschmidt]

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

[U-Boot] [PATCH 5/7] mips: mt76xx: gardena-smart-gateway: Read default speed and mode values from DT

[Stefan Roese]

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

Viele Grüße,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

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

[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.

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
>

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

[Simon Goldschmidt]

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

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

[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;
};

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

[U-Boot] [PATCH 1/7] spi: update management of default speed and mode

[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?


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 at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH 2/7] spi: add comment for spi_flash_probe_bus_cs function

[Petr Vorel]

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

[U-Boot] [PATCH 1/7] spi: update management of default speed and mode

[Simon Goldschmidt]

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;
>>> +       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 at lists.denx.de
>> https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH 3/7] da850evm: sf: Read default speed and mode values from DT

[Petr Vorel]

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

[U-Boot] [PATCH 4/7] dfu: sf: Read default speed and mode values from DT

[Petr Vorel]

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

[U-Boot] [PATCH 5/7] mips: mt76xx: gardena-smart-gateway: Read default speed and mode values from DT

[Petr Vorel]

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

[U-Boot] [PATCH 6/7] env: Read default speed and mode values from DT

[Petr Vorel]

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

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

[Simon Goldschmidt]

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

[U-Boot] [PATCH 7/7] spi: remove define for SPI default SPEED and MODE

[Petr Vorel]

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

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

[Petr Vorel]

Hi Patrick,

FYI this patchset breaks some machines:
https://travis-ci.org/pevik/u-boot/builds/466192406


Kind regards,
Petr

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

[Simon Goldschmidt]

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

[U-Boot] [PATCH 1/7] spi: update management of default speed and mode

[Jagan Teki]

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.

[U-Boot] [PATCH 3/7] da850evm: sf: Read default speed and mode values from DT

[Jagan Teki]

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

[U-Boot] [PATCH 6/7] env: Read default speed and mode values from DT

[Jagan Teki]

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

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

[Patrick DELAUNAY]

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

[U-Boot] [PATCH 3/7] da850evm: sf: Read default speed and mode values from DT

[Patrick DELAUNAY]

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.