dm: arm: zynq: Convert serial driver to driver model

Sometimes stdout-path contains a UART alias along with speed, etc. For example:

	stdout-path = "serial0:115200n8";

Add support for decoding this.

Signed-off-by Simon Glass <[email protected]>

---

 drivers/serial/serial-uclass.c | 30 +++++++++++++++++++++++++-----
 1 file changed, 25 insertions(+), 5 deletions(-)

drivers/serial/serial-uclass.c +0 -0

index 55011cc..842f78b 100644

drivers/serial/serial-uclass.c +11 -2

 	struct udevice *dev;
 	int node;
 
-	if (CONFIG_IS_ENABLED(OF_CONTROL) && gd->fdt_blob) {
+	if (CONFIG_IS_ENABLED(OF_CONTROL) && blob) {
 		/* Check for a chosen console */
-		node = fdtdec_get_chosen_node(gd->fdt_blob, "stdout-path");
+		node = fdtdec_get_chosen_node(blob, "stdout-path");
+		if (node < 0) {
+			const char *str, *p, *name;
+
+			/*
+			 * Deal with things like
+			 *	stdout-path = "serial0:115200n8";
+			 *
+			 * We need to look up the alias and then follow it to
+			 * the correct node.

drivers/serial/serial-uclass.c +2 -2

-			node = fdt_path_offset(gd->fdt_blob, "console");
+			node = fdt_path_offset(blob, "console");
 		if (!uclass_get_device_by_of_offset(UCLASS_SERIAL, node,
 						    &dev)) {
 			gd->cur_serial_dev = dev;

@@ -48,14 +68,14 @@ static void serial_find_console_or_panic(void)
 		* bind it anyway.
 		*/
 		if (node > 0 &&
-		    !lists_bind_fdt(gd->dm_root, gd->fdt_blob, node, &dev)) {
+		    !lists_bind_fdt(gd->dm_root, blob, node, &dev)) {
 			if (!device_probe(dev)) {

Quoted 19 lines · Simon Glass

Sometimes stdout-path contains a UART alias along with speed, etc. For example:

      stdout-path = "serial0:115200n8";

Add support for decoding this.

Signed-off-by Simon Glass <[email protected]>

---

 drivers/serial/serial-uclass.c | 30 +++++++++++++++++++++++++-----
 1 file changed, 25 insertions(+), 5 deletions(-)

drivers/serial/serial-uclass.c +0 -0

index 55011cc..842f78b 100644
@@ -29,14 +29,34 @@ static const unsigned long baudrate_table[] = CONFIG_SYS_BAUDRATE_TABLE;
 

drivers/serial/serial-uclass.c static const unsigned long baudrate_table[] = CONFIG_SYS_BAUDRATE_TABLE; · line 32

Quoted 3 lines

drivers/serial/serial-uclass.c +1 -0

 static void serial_find_console_or_panic(void)
 {
+     const void *blob = gd->fdt_blob;

This is one change and should be separated from the rest.

Quoted 18 lines

drivers/serial/serial-uclass.c +11 -2

      struct udevice *dev;
      int node;
 
-     if (CONFIG_IS_ENABLED(OF_CONTROL) && gd->fdt_blob) {
+     if (CONFIG_IS_ENABLED(OF_CONTROL) && blob) {
              /* Check for a chosen console */
-             node = fdtdec_get_chosen_node(gd->fdt_blob, "stdout-path");
+             node = fdtdec_get_chosen_node(blob, "stdout-path");
+             if (node < 0) {
+                     const char *str, *p, *name;
+
+                     /*
+                      * Deal with things like
+                      *      stdout-path = "serial0:115200n8";
+                      *
+                      * We need to look up the alias and then follow it to
+                      * the correct node.

drivers/serial/serial-uclass.c static const unsigned long baudrate_table[] = CONFIG_SYS_BAUDRATE_TABLE; · line 57

Quoted 10 lines

drivers/serial/serial-uclass.c +10 -0

+                      */
+                     str = fdtdec_get_chosen_prop(blob, "stdout-path");
+                     if (str) {
+                             p = strchr(str, ':');
+                             name = fdt_get_alias_namelen(blob, str,
+                                             p ? p - str : strlen(str));
+                             if (name)
+                                     node = fdt_path_offset(blob, name);
+                     }
+             }

And this is second.

drivers/serial/serial-uclass.c static const unsigned long baudrate_table[] = CONFIG_SYS_BAUDRATE_TABLE; · line 58

Quoted 1 line

drivers/serial/serial-uclass.c +0 -0

              if (node < 0)

You have if (node < 0) above too which looks pretty odd.

Quoted 16 lines

drivers/serial/serial-uclass.c +2 -2

-                     node = fdt_path_offset(gd->fdt_blob, "console");
+                     node = fdt_path_offset(blob, "console");
              if (!uclass_get_device_by_of_offset(UCLASS_SERIAL, node,
                                                  &dev)) {
                      gd->cur_serial_dev = dev;

@@ -48,14 +68,14 @@ static void serial_find_console_or_panic(void)
              * bind it anyway.
              */
              if (node > 0 &&
-                 !lists_bind_fdt(gd->dm_root, gd->fdt_blob, node, &dev)) {
+                 !lists_bind_fdt(gd->dm_root, blob, node, &dev)) {
                      if (!device_probe(dev)) {
                              gd->cur_serial_dev = dev;
                              return;
                      }

drivers/serial/serial-uclass.c static void serial_find_console_or_panic(void) · line 81

Quoted 8 lines

drivers/serial/serial-uclass.c +1 -1

              }
      }
-     if (!SPL_BUILD || !CONFIG_IS_ENABLED(OF_CONTROL) || !gd->fdt_blob) {
+     if (!SPL_BUILD || !CONFIG_IS_ENABLED(OF_CONTROL) || !blob) {
              /*
              * Try to use CONFIG_CONS_INDEX if available (it is numbered
              * from 1!).

Hi Michal,

Quoted 20 lines · Michal Simek

Quoted 19 lines · Simon Glass

Sometimes stdout-path contains a UART alias along with speed, etc. For example:

      stdout-path = "serial0:115200n8";

Add support for decoding this.

Signed-off-by Simon Glass <[email protected]>

---

 drivers/serial/serial-uclass.c | 30 +++++++++++++++++++++++++-----
 1 file changed, 25 insertions(+), 5 deletions(-)

drivers/serial/serial-uclass.c +0 -0

index 55011cc..842f78b 100644
@@ -29,14 +29,34 @@ static const unsigned long baudrate_table[] = CONFIG_SYS_BAUDRATE_TABLE;
 

Quoted 5 lines

drivers/serial/serial-uclass.c static const unsigned long baudrate_table[] = CONFIG_SYS_BAUDRATE_TABLE; · line 32

Quoted 3 lines

drivers/serial/serial-uclass.c +1 -0

 static void serial_find_console_or_panic(void)
 {
+     const void *blob = gd->fdt_blob;

This is one change and should be separated from the rest.

Yes I should do that.

Quoted 29 lines

Quoted 28 lines

drivers/serial/serial-uclass.c +21 -2

      struct udevice *dev;
      int node;
 
-     if (CONFIG_IS_ENABLED(OF_CONTROL) && gd->fdt_blob) {
+     if (CONFIG_IS_ENABLED(OF_CONTROL) && blob) {
              /* Check for a chosen console */
-             node = fdtdec_get_chosen_node(gd->fdt_blob, "stdout-path");
+             node = fdtdec_get_chosen_node(blob, "stdout-path");
+             if (node < 0) {
+                     const char *str, *p, *name;
+
+                     /*
+                      * Deal with things like
+                      *      stdout-path = "serial0:115200n8";
+                      *
+                      * We need to look up the alias and then follow it to
+                      * the correct node.
+                      */
+                     str = fdtdec_get_chosen_prop(blob, "stdout-path");
+                     if (str) {
+                             p = strchr(str, ':');
+                             name = fdt_get_alias_namelen(blob, str,
+                                             p ? p - str : strlen(str));
+                             if (name)
+                                     node = fdt_path_offset(blob, name);
+                     }
+             }

Quoted 5 lines

And this is second.

drivers/serial/serial-uclass.c static const unsigned long baudrate_table[] = CONFIG_SYS_BAUDRATE_TABLE; · line 58

Quoted 1 line

drivers/serial/serial-uclass.c +0 -0

              if (node < 0)

You have if (node < 0) above too which looks pretty odd.

OK, but that's what I want to check...

Quoted 62 lines · Simon Glass

Hi Michal,

Quoted 20 lines · Michal Simek

Quoted 19 lines · Simon Glass

Sometimes stdout-path contains a UART alias along with speed, etc. For example:

      stdout-path = "serial0:115200n8";

Add support for decoding this.

Signed-off-by Simon Glass <[email protected]>

---

 drivers/serial/serial-uclass.c | 30 +++++++++++++++++++++++++-----
 1 file changed, 25 insertions(+), 5 deletions(-)

drivers/serial/serial-uclass.c +0 -0

index 55011cc..842f78b 100644
@@ -29,14 +29,34 @@ static const unsigned long baudrate_table[] = CONFIG_SYS_BAUDRATE_TABLE;
 

Quoted 5 lines

drivers/serial/serial-uclass.c static const unsigned long baudrate_table[] = CONFIG_SYS_BAUDRATE_TABLE; · line 32

Quoted 3 lines

drivers/serial/serial-uclass.c +1 -0

 static void serial_find_console_or_panic(void)
 {
+     const void *blob = gd->fdt_blob;

This is one change and should be separated from the rest.

Yes I should do that.

Quoted 31 lines

Quoted 18 lines

drivers/serial/serial-uclass.c +11 -2

      struct udevice *dev;
      int node;
 
-     if (CONFIG_IS_ENABLED(OF_CONTROL) && gd->fdt_blob) {
+     if (CONFIG_IS_ENABLED(OF_CONTROL) && blob) {
              /* Check for a chosen console */
-             node = fdtdec_get_chosen_node(gd->fdt_blob, "stdout-path");
+             node = fdtdec_get_chosen_node(blob, "stdout-path");
+             if (node < 0) {
+                     const char *str, *p, *name;
+
+                     /*
+                      * Deal with things like
+                      *      stdout-path = "serial0:115200n8";
+                      *
+                      * We need to look up the alias and then follow it to
+                      * the correct node.

drivers/serial/serial-uclass.c static const unsigned long baudrate_table[] = CONFIG_SYS_BAUDRATE_TABLE; · line 57

Quoted 10 lines

drivers/serial/serial-uclass.c +10 -0

+                      */
+                     str = fdtdec_get_chosen_prop(blob, "stdout-path");
+                     if (str) {
+                             p = strchr(str, ':');
+                             name = fdt_get_alias_namelen(blob, str,
+                                             p ? p - str : strlen(str));
+                             if (name)
+                                     node = fdt_path_offset(blob, name);
+                     }
+             }

And this is second.

Quoted 5 lines

Quoted 3 lines

drivers/serial/serial-uclass.c static const unsigned long baudrate_table[] = CONFIG_SYS_BAUDRATE_TABLE; · line 58

Quoted 1 line

drivers/serial/serial-uclass.c +0 -0

              if (node < 0)

You have if (node < 0) above too which looks pretty odd.

OK, but that's what I want to check...

I believe you but then I would expect you have just one if (node < 0) in the code not two.

Production boards should not use CONFIG_OF_EMBED. Fix this for the Zybo boards.

The image to use now becomes u-boot-dtb.bin.

For example, the .bif file should contain a line like:

[load = 0x04000000,startup=0x04000000]/path/to/u-boot-dtb.bin

instead of:

[load = 0x04000000,startup=0x04000000]/path/to/u-boot.bin

When device tree is enabled we need to load u-boot-dtb.img. Change the settings so that SPL does the right thing.

Signed-off-by Simon Glass <[email protected]>

---

 configs/zynq_microzed_defconfig    | 2 +-
 configs/zynq_picozed_defconfig     | 2 +-
 configs/zynq_zc702_defconfig       | 2 +-
 configs/zynq_zc706_defconfig       | 2 +-
 configs/zynq_zc70x_defconfig       | 2 +-
 configs/zynq_zc770_xm010_defconfig | 2 +-
 configs/zynq_zc770_xm011_defconfig | 2 +-
 configs/zynq_zc770_xm012_defconfig | 2 +-
 configs/zynq_zc770_xm013_defconfig | 2 +-
 configs/zynq_zed_defconfig         | 2 +-
 configs/zynq_zybo_defconfig        | 2 +-
 include/configs/zynq-common.h      | 6 +++++-
 12 files changed, 16 insertions(+), 12 deletions(-)

configs/zynq_microzed_defconfig +1 -1

index e9c3209..9d51540 100644
@@ -9,5 +9,5 @@ CONFIG_FIT_SIGNATURE=y
 # CONFIG_CMD_IMLS is not set
 # CONFIG_CMD_FLASH is not set
 # CONFIG_CMD_SETEXPR is not set
-CONFIG_OF_EMBED=y
+CONFIG_OF_SEPARATE=y
 CONFIG_NET_RANDOM_ETHADDR=y

configs/zynq_picozed_defconfig +1 -1

index f2b71e9..3a42efb 100644
@@ -6,5 +6,5 @@ CONFIG_SPL=y
 # CONFIG_CMD_IMLS is not set
 # CONFIG_CMD_FLASH is not set
 # CONFIG_CMD_SETEXPR is not set
-CONFIG_OF_EMBED=y
+CONFIG_OF_SEPARATE=y
 CONFIG_NET_RANDOM_ETHADDR=y

configs/zynq_zc702_defconfig +1 -1

index 0abb7a8..6faf928 100644
@@ -8,5 +8,5 @@ CONFIG_FIT_SIGNATURE=y
 # CONFIG_CMD_IMLS is not set
 # CONFIG_CMD_FLASH is not set
 # CONFIG_CMD_SETEXPR is not set
-CONFIG_OF_EMBED=y
+CONFIG_OF_SEPARATE=y
 CONFIG_NET_RANDOM_ETHADDR=y

configs/zynq_zc706_defconfig +1 -1

index d67f507..d6559b5 100644
@@ -9,5 +9,5 @@ CONFIG_FIT_SIGNATURE=y
 # CONFIG_CMD_IMLS is not set
 # CONFIG_CMD_FLASH is not set
 # CONFIG_CMD_SETEXPR is not set
-CONFIG_OF_EMBED=y
+CONFIG_OF_SEPARATE=y
 CONFIG_NET_RANDOM_ETHADDR=y

configs/zynq_zc70x_defconfig +1 -1

index 37c249f..49c987a 100644
@@ -9,5 +9,5 @@ CONFIG_FIT_SIGNATURE=y
 # CONFIG_CMD_IMLS is not set
 # CONFIG_CMD_FLASH is not set
 # CONFIG_CMD_SETEXPR is not set
-CONFIG_OF_EMBED=y
+CONFIG_OF_SEPARATE=y
 CONFIG_NET_RANDOM_ETHADDR=y

configs/zynq_zc770_xm010_defconfig +1 -1

index 0e826bb..4bfb9cc 100644
@@ -10,6 +10,6 @@ CONFIG_SYS_EXTRA_OPTIONS="ZC770_XM010"
 # CONFIG_CMD_IMLS is not set
 # CONFIG_CMD_FLASH is not set
 # CONFIG_CMD_SETEXPR is not set
-CONFIG_OF_EMBED=y
+CONFIG_OF_SEPARATE=y
 CONFIG_NET_RANDOM_ETHADDR=y
 CONFIG_SPI_FLASH=y

configs/zynq_zc770_xm011_defconfig +1 -1

index 46d043b..2a61fe3 100644
@@ -10,5 +10,5 @@ CONFIG_SYS_EXTRA_OPTIONS="ZC770_XM011"
 # CONFIG_CMD_IMLS is not set
 # CONFIG_CMD_FLASH is not set
 # CONFIG_CMD_SETEXPR is not set
-CONFIG_OF_EMBED=y
+CONFIG_OF_SEPARATE=y
 CONFIG_NET_RANDOM_ETHADDR=y

configs/zynq_zc770_xm012_defconfig +1 -1

index 34d479f..eb98a39 100644
@@ -8,5 +8,5 @@ CONFIG_FIT_VERBOSE=y
 CONFIG_FIT_SIGNATURE=y
 CONFIG_SYS_EXTRA_OPTIONS="ZC770_XM012"
 # CONFIG_CMD_SETEXPR is not set
-CONFIG_OF_EMBED=y
+CONFIG_OF_SEPARATE=y
 CONFIG_NET_RANDOM_ETHADDR=y

configs/zynq_zc770_xm013_defconfig +1 -1

index c59599f..8d65c05 100644
@@ -10,5 +10,5 @@ CONFIG_SYS_EXTRA_OPTIONS="ZC770_XM013"
 # CONFIG_CMD_IMLS is not set
 # CONFIG_CMD_FLASH is not set
 # CONFIG_CMD_SETEXPR is not set
-CONFIG_OF_EMBED=y
+CONFIG_OF_SEPARATE=y
 CONFIG_NET_RANDOM_ETHADDR=y

configs/zynq_zed_defconfig +1 -1

index 886b4a5..13bef36 100644
@@ -9,5 +9,5 @@ CONFIG_FIT_SIGNATURE=y
 # CONFIG_CMD_IMLS is not set
 # CONFIG_CMD_FLASH is not set
 # CONFIG_CMD_SETEXPR is not set
-CONFIG_OF_EMBED=y
+CONFIG_OF_SEPARATE=y
 CONFIG_NET_RANDOM_ETHADDR=y

configs/zynq_zybo_defconfig +1 -1

index 77b9409..fc251dc 100644
@@ -9,5 +9,5 @@ CONFIG_FIT_SIGNATURE=y
 # CONFIG_CMD_IMLS is not set
 # CONFIG_CMD_FLASH is not set
 # CONFIG_CMD_SETEXPR is not set
-CONFIG_OF_EMBED=y
+CONFIG_OF_SEPARATE=y
 CONFIG_NET_RANDOM_ETHADDR=y

include/configs/zynq-common.h +0 -0

index e7ab50a..aa4785f 100644
@@ -319,7 +319,11 @@
 #define CONFIG_SYS_MMCSD_FS_BOOT_PARTITION     1
 #define CONFIG_SPL_LIBDISK_SUPPORT

Quoted 2 lines · Simon Glass

Production boards should not use CONFIG_OF_EMBED. Fix this for the Zybo boards.

Zynq boards?

As you see I have enabled OF_EMBED some weeks ago.
zynq: Make CONFIG_OF_EMBED default case 98b532b42079a7ffd617ce0330d6778288b7c535

What's the reason not to use CONFIG_OF_EMBED for production boards? Strictly speaking none of these boards are production one. I would label them more as refence boards, development boards.

Quoted 170 lines

The image to use now becomes u-boot-dtb.bin.

For example, the .bif file should contain a line like:

[load = 0x04000000,startup=0x04000000]/path/to/u-boot-dtb.bin

instead of:

[load = 0x04000000,startup=0x04000000]/path/to/u-boot.bin

When device tree is enabled we need to load u-boot-dtb.img. Change the settings so that SPL does the right thing.

Signed-off-by Simon Glass <[email protected]>

---

 configs/zynq_microzed_defconfig    | 2 +-
 configs/zynq_picozed_defconfig     | 2 +-
 configs/zynq_zc702_defconfig       | 2 +-
 configs/zynq_zc706_defconfig       | 2 +-
 configs/zynq_zc70x_defconfig       | 2 +-
 configs/zynq_zc770_xm010_defconfig | 2 +-
 configs/zynq_zc770_xm011_defconfig | 2 +-
 configs/zynq_zc770_xm012_defconfig | 2 +-
 configs/zynq_zc770_xm013_defconfig | 2 +-
 configs/zynq_zed_defconfig         | 2 +-
 configs/zynq_zybo_defconfig        | 2 +-
 include/configs/zynq-common.h      | 6 +++++-
 12 files changed, 16 insertions(+), 12 deletions(-)

configs/zynq_microzed_defconfig +1 -1

index e9c3209..9d51540 100644
@@ -9,5 +9,5 @@ CONFIG_FIT_SIGNATURE=y
 # CONFIG_CMD_IMLS is not set
 # CONFIG_CMD_FLASH is not set
 # CONFIG_CMD_SETEXPR is not set
-CONFIG_OF_EMBED=y
+CONFIG_OF_SEPARATE=y
 CONFIG_NET_RANDOM_ETHADDR=y

configs/zynq_picozed_defconfig +1 -1

index f2b71e9..3a42efb 100644
@@ -6,5 +6,5 @@ CONFIG_SPL=y
 # CONFIG_CMD_IMLS is not set
 # CONFIG_CMD_FLASH is not set
 # CONFIG_CMD_SETEXPR is not set
-CONFIG_OF_EMBED=y
+CONFIG_OF_SEPARATE=y
 CONFIG_NET_RANDOM_ETHADDR=y

configs/zynq_zc702_defconfig +1 -1

index 0abb7a8..6faf928 100644
@@ -8,5 +8,5 @@ CONFIG_FIT_SIGNATURE=y
 # CONFIG_CMD_IMLS is not set
 # CONFIG_CMD_FLASH is not set
 # CONFIG_CMD_SETEXPR is not set
-CONFIG_OF_EMBED=y
+CONFIG_OF_SEPARATE=y
 CONFIG_NET_RANDOM_ETHADDR=y

configs/zynq_zc706_defconfig +1 -1

index d67f507..d6559b5 100644
@@ -9,5 +9,5 @@ CONFIG_FIT_SIGNATURE=y
 # CONFIG_CMD_IMLS is not set
 # CONFIG_CMD_FLASH is not set
 # CONFIG_CMD_SETEXPR is not set
-CONFIG_OF_EMBED=y
+CONFIG_OF_SEPARATE=y
 CONFIG_NET_RANDOM_ETHADDR=y

configs/zynq_zc70x_defconfig +1 -1

index 37c249f..49c987a 100644
@@ -9,5 +9,5 @@ CONFIG_FIT_SIGNATURE=y
 # CONFIG_CMD_IMLS is not set
 # CONFIG_CMD_FLASH is not set
 # CONFIG_CMD_SETEXPR is not set
-CONFIG_OF_EMBED=y
+CONFIG_OF_SEPARATE=y
 CONFIG_NET_RANDOM_ETHADDR=y

configs/zynq_zc770_xm010_defconfig +1 -1

index 0e826bb..4bfb9cc 100644
@@ -10,6 +10,6 @@ CONFIG_SYS_EXTRA_OPTIONS="ZC770_XM010"
 # CONFIG_CMD_IMLS is not set
 # CONFIG_CMD_FLASH is not set
 # CONFIG_CMD_SETEXPR is not set
-CONFIG_OF_EMBED=y
+CONFIG_OF_SEPARATE=y
 CONFIG_NET_RANDOM_ETHADDR=y
 CONFIG_SPI_FLASH=y

configs/zynq_zc770_xm011_defconfig +1 -1

index 46d043b..2a61fe3 100644
@@ -10,5 +10,5 @@ CONFIG_SYS_EXTRA_OPTIONS="ZC770_XM011"
 # CONFIG_CMD_IMLS is not set
 # CONFIG_CMD_FLASH is not set
 # CONFIG_CMD_SETEXPR is not set
-CONFIG_OF_EMBED=y
+CONFIG_OF_SEPARATE=y
 CONFIG_NET_RANDOM_ETHADDR=y

configs/zynq_zc770_xm012_defconfig +1 -1

index 34d479f..eb98a39 100644
@@ -8,5 +8,5 @@ CONFIG_FIT_VERBOSE=y
 CONFIG_FIT_SIGNATURE=y
 CONFIG_SYS_EXTRA_OPTIONS="ZC770_XM012"
 # CONFIG_CMD_SETEXPR is not set
-CONFIG_OF_EMBED=y
+CONFIG_OF_SEPARATE=y
 CONFIG_NET_RANDOM_ETHADDR=y

configs/zynq_zc770_xm013_defconfig +1 -1

index c59599f..8d65c05 100644
@@ -10,5 +10,5 @@ CONFIG_SYS_EXTRA_OPTIONS="ZC770_XM013"
 # CONFIG_CMD_IMLS is not set
 # CONFIG_CMD_FLASH is not set
 # CONFIG_CMD_SETEXPR is not set
-CONFIG_OF_EMBED=y
+CONFIG_OF_SEPARATE=y
 CONFIG_NET_RANDOM_ETHADDR=y

configs/zynq_zed_defconfig +1 -1

index 886b4a5..13bef36 100644
@@ -9,5 +9,5 @@ CONFIG_FIT_SIGNATURE=y
 # CONFIG_CMD_IMLS is not set
 # CONFIG_CMD_FLASH is not set
 # CONFIG_CMD_SETEXPR is not set
-CONFIG_OF_EMBED=y
+CONFIG_OF_SEPARATE=y
 CONFIG_NET_RANDOM_ETHADDR=y

configs/zynq_zybo_defconfig +1 -1

index 77b9409..fc251dc 100644
@@ -9,5 +9,5 @@ CONFIG_FIT_SIGNATURE=y
 # CONFIG_CMD_IMLS is not set
 # CONFIG_CMD_FLASH is not set
 # CONFIG_CMD_SETEXPR is not set
-CONFIG_OF_EMBED=y
+CONFIG_OF_SEPARATE=y
 CONFIG_NET_RANDOM_ETHADDR=y

include/configs/zynq-common.h +5 -1

index e7ab50a..aa4785f 100644
@@ -319,7 +319,11 @@
 #define CONFIG_SYS_MMCSD_FS_BOOT_PARTITION     1
 #define CONFIG_SPL_LIBDISK_SUPPORT
 #define CONFIG_SPL_FAT_SUPPORT
-#define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME     "u-boot.img"
+#ifdef CONFIG_OF_CONTROL
+# define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME     "u-boot-dtb.img"
+#else
+# define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME     "u-boot.img"
+#endif
 #endif
 

include/configs/zynq-common.h line 329

Quoted 2 lines

include/configs/zynq-common.h +0 -0

 /* Disable dcache for SPL just for sure */

this was removed by Masahiro long time ago.
kconfig: move CONFIG_OF_* to Kconfig
sha1: 783e6a72b8278d59854ced41a4696c9a14abbb0b

Hi Michal,

Quoted 10 lines · Michal Simek

Quoted 2 lines · Simon Glass

Production boards should not use CONFIG_OF_EMBED. Fix this for the Zybo boards.

Zynq boards?

As you see I have enabled OF_EMBED some weeks ago.
zynq: Make CONFIG_OF_EMBED default case 98b532b42079a7ffd617ce0330d6778288b7c535

Quoted 3 lines

What's the reason not to use CONFIG_OF_EMBED for production boards? Strictly speaking none of these boards are production one. I would label them more as refence boards, development boards.

Well I mean that we should not have boards in the tree that use this option, It has been a long-standing convention since device tree support was added - see README.fdt-control. I'll send a patch to add this note to Kconfig.

Quoted 16 lines

Quoted 16 lines

include/configs/zynq-common.h +5 -1

index e7ab50a..aa4785f 100644
@@ -319,7 +319,11 @@
 #define CONFIG_SYS_MMCSD_FS_BOOT_PARTITION     1
 #define CONFIG_SPL_LIBDISK_SUPPORT
 #define CONFIG_SPL_FAT_SUPPORT
-#define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME     "u-boot.img"
+#ifdef CONFIG_OF_CONTROL
+# define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME     "u-boot-dtb.img"
+#else
+# define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME     "u-boot.img"
+#endif
 #endif
 

Quoted 6 lines

include/configs/zynq-common.h line 329

Quoted 2 lines

include/configs/zynq-common.h +0 -0

 /* Disable dcache for SPL just for sure */

this was removed by Masahiro long time ago.
kconfig: move CONFIG_OF_* to Kconfig
sha1: 783e6a72b8278d59854ced41a4696c9a14abbb0b

What was moved?

Quoted 17 lines · Simon Glass

Hi Michal,

Quoted 13 lines · Michal Simek

Quoted 2 lines · Simon Glass

Production boards should not use CONFIG_OF_EMBED. Fix this for the Zybo boards.

Zynq boards?

As you see I have enabled OF_EMBED some weeks ago.
zynq: Make CONFIG_OF_EMBED default case 98b532b42079a7ffd617ce0330d6778288b7c535

What's the reason not to use CONFIG_OF_EMBED for production boards? Strictly speaking none of these boards are production one. I would label them more as refence boards, development boards.

Quoted 4 lines

Well I mean that we should not have boards in the tree that use this option, It has been a long-standing convention since device tree support was added - see README.fdt-control. I'll send a patch to add this note to Kconfig.

ok then do we want to support this option? If this shouldn't be enabled in any config in the tree? (Note: someone has to enable that and keep building u-boot to ensure that this feature still work)

The reason why I have enabled that was that using u-boot-dtb.img is breaking all users because they have to start to change a lot of things. That's why having OF_EMBED enabled was less painful.
Users which use these boards just don't recognize any change when this feature is enabled.

...

Quoted 23 lines

Quoted 22 lines

Quoted 8 lines

include/configs/zynq-common.h +0 -0

index e7ab50a..aa4785f 100644
@@ -319,7 +319,11 @@
 #define CONFIG_SYS_MMCSD_FS_BOOT_PARTITION     1
 #define CONFIG_SPL_LIBDISK_SUPPORT

include/configs/zynq-common.h line 329

Quoted 10 lines

include/configs/zynq-common.h +5 -1

 #define CONFIG_SPL_FAT_SUPPORT
-#define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME     "u-boot.img"
+#ifdef CONFIG_OF_CONTROL
+# define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME     "u-boot-dtb.img"
+#else
+# define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME     "u-boot.img"
+#endif
 #endif
 
 /* Disable dcache for SPL just for sure */

this was removed by Masahiro long time ago.
kconfig: move CONFIG_OF_* to Kconfig
sha1: 783e6a72b8278d59854ced41a4696c9a14abbb0b

Quoted 1 line

What was moved?

Using different names for u-boot image.

Hi Michal,

Quoted 18 lines · Michal Simek

Quoted 17 lines · Simon Glass

Hi Michal,

Quoted 13 lines · Michal Simek

Quoted 2 lines · Simon Glass

Production boards should not use CONFIG_OF_EMBED. Fix this for the Zybo boards.

Zynq boards?

As you see I have enabled OF_EMBED some weeks ago.
zynq: Make CONFIG_OF_EMBED default case 98b532b42079a7ffd617ce0330d6778288b7c535

What's the reason not to use CONFIG_OF_EMBED for production boards? Strictly speaking none of these boards are production one. I would label them more as refence boards, development boards.

Quoted 8 lines

Quoted 4 lines

Well I mean that we should not have boards in the tree that use this option, It has been a long-standing convention since device tree support was added - see README.fdt-control. I'll send a patch to add this note to Kconfig.

ok then do we want to support this option? If this shouldn't be enabled in any config in the tree? (Note: someone has to enable that and keep building u-boot to ensure that this feature still work)

It's a bit like disabling relocation. It is useful when you want to load an elf image onto your board with a debugger, without having to worry about getting the device tree loaded also. It is normally possible to load u-boot-dtb.bin and then load the symbols from u-boot separately, but that's the reasoning.

Quoted 7 lines

The reason why I have enabled that was that using u-boot-dtb.img is breaking all users because they have to start to change a lot of things. That's why having OF_EMBED enabled was less painful.
Users which use these boards just don't recognize any change when this feature is enabled.

I see. Well perhaps you could enable the debug UART / do something else so that a message will be printed in this case for all boards?

We could perhaps move to a different scheme:

u-boot.bin - contains U-Boot and device tree(s) u-boot-nodtb.bin - contains plain binary without device tree

and perhaps this behaviour could even be optional (thus you could select it for Zynq). This has come up before. Tegra uses u-boot-nodtb-tegra.bin although it is for a different purpose. See commits 9972db5c and 984df4ec for the reasoning. I suspect that could perhaps be dropped but I am not sure.

Quoted 26 lines

...

Quoted 23 lines

Quoted 22 lines

Quoted 8 lines

include/configs/zynq-common.h +0 -0

index e7ab50a..aa4785f 100644
@@ -319,7 +319,11 @@
 #define CONFIG_SYS_MMCSD_FS_BOOT_PARTITION     1
 #define CONFIG_SPL_LIBDISK_SUPPORT

include/configs/zynq-common.h line 329

Quoted 10 lines

include/configs/zynq-common.h +5 -1

 #define CONFIG_SPL_FAT_SUPPORT
-#define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME     "u-boot.img"
+#ifdef CONFIG_OF_CONTROL
+# define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME     "u-boot-dtb.img"
+#else
+# define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME     "u-boot.img"
+#endif
 #endif
 
 /* Disable dcache for SPL just for sure */

this was removed by Masahiro long time ago.
kconfig: move CONFIG_OF_* to Kconfig
sha1: 783e6a72b8278d59854ced41a4696c9a14abbb0b

Quoted 3 lines

Quoted 1 line

What was moved?

Using different names for u-boot image.

OK. So are you saying my patch needs to change in this area?

Hi Simon,

Quoted 30 lines · Simon Glass

Hi Michal,

Quoted 18 lines · Michal Simek

Quoted 17 lines · Simon Glass

Hi Michal,

Quoted 13 lines · Michal Simek

Quoted 2 lines · Simon Glass

Production boards should not use CONFIG_OF_EMBED. Fix this for the Zybo boards.

Zynq boards?

As you see I have enabled OF_EMBED some weeks ago.
zynq: Make CONFIG_OF_EMBED default case 98b532b42079a7ffd617ce0330d6778288b7c535

What's the reason not to use CONFIG_OF_EMBED for production boards? Strictly speaking none of these boards are production one. I would label them more as refence boards, development boards.

Quoted 8 lines

Quoted 4 lines

Well I mean that we should not have boards in the tree that use this option, It has been a long-standing convention since device tree support was added - see README.fdt-control. I'll send a patch to add this note to Kconfig.

ok then do we want to support this option? If this shouldn't be enabled in any config in the tree? (Note: someone has to enable that and keep building u-boot to ensure that this feature still work)

Quoted 5 lines

It's a bit like disabling relocation. It is useful when you want to load an elf image onto your board with a debugger, without having to worry about getting the device tree loaded also. It is normally possible to load u-boot-dtb.bin and then load the symbols from u-boot separately, but that's the reasoning.

The related problem is that in your commit message you are using correct line for bif if you use FSBL. But the problem is with the all software around it and users around.
u-boot-dtb.bin is binary file and you need to keep record where to load it. If you have elf file you have entry point there.
It means that this change breaks all current users which is something what is pretty problematic.
I can live without disabling OF_EMBED because I will use it in xilinx tree but it is just pain which you should be aware of.

And still I can't see the problem to have OF_EMBED enabled for these zynq boards because all of them are not recommended for production.

Quoted 14 lines

Quoted 8 lines

The reason why I have enabled that was that using u-boot-dtb.img is breaking all users because they have to start to change a lot of things. That's why having OF_EMBED enabled was less painful.
Users which use these boards just don't recognize any change when this feature is enabled.

I see. Well perhaps you could enable the debug UART / do something else so that a message will be printed in this case for all boards?

We could perhaps move to a different scheme:

Quoted 8 lines

u-boot.bin - contains U-Boot and device tree(s) u-boot-nodtb.bin - contains plain binary without device tree

and perhaps this behaviour could even be optional (thus you could select it for Zynq). This has come up before. Tegra uses u-boot-nodtb-tegra.bin although it is for a different purpose. See commits 9972db5c and 984df4ec for the reasoning. I suspect that could perhaps be dropped but I am not sure.

The problem is more with using bin formats which lack information about entry point.

You are using this line and it is completely valid. [load = 0x04000000,startup=0x04000000]/path/to/u-boot.bin but most of users are just using
u-boot.elf
and they don't need to care about addresses at all.

I am fully aware that this is around for a long time but it will be the best to have something like u-boot-dtb.elf or u-boot.elf with DTB inside. As we discussed above u-boot.elf with DTB inside is when OF_EMBED is enabled.

Quoted 32 lines

Quoted 31 lines

...

Quoted 16 lines

Quoted 16 lines

Quoted 16 lines

include/configs/zynq-common.h +5 -1

index e7ab50a..aa4785f 100644
@@ -319,7 +319,11 @@
 #define CONFIG_SYS_MMCSD_FS_BOOT_PARTITION     1
 #define CONFIG_SPL_LIBDISK_SUPPORT
 #define CONFIG_SPL_FAT_SUPPORT
-#define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME     "u-boot.img"
+#ifdef CONFIG_OF_CONTROL
+# define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME     "u-boot-dtb.img"
+#else
+# define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME     "u-boot.img"
+#endif
 #endif
 

Quoted 8 lines

Quoted 6 lines

include/configs/zynq-common.h line 329

Quoted 2 lines

include/configs/zynq-common.h +0 -0

 /* Disable dcache for SPL just for sure */

this was removed by Masahiro long time ago.
kconfig: move CONFIG_OF_* to Kconfig
sha1: 783e6a72b8278d59854ced41a4696c9a14abbb0b

What was moved?

Using different names for u-boot image.

Quoted 1 line

OK. So are you saying my patch needs to change in this area?

What I am saying is that we patched config to use just one name and currently you have started to use two again.
OF_CONTROL will be enabled by default that's why only one name should be enough.
Or even better just move this name to Kconfig and setup one default value.

We need to mark some device tree nodes so that they are available before relocation. This enables driver model to find these automatically. In the case of SPL it ensures that these nodes will be retained in SPL.

Signed-off-by Simon Glass <[email protected]>

---

 arch/arm/dts/zynq-7000.dtsi       | 1 +
 arch/arm/dts/zynq-microzed.dts    | 5 +++++
 arch/arm/dts/zynq-picozed.dts     | 5 +++++
 arch/arm/dts/zynq-zc702.dts       | 1 +
 arch/arm/dts/zynq-zc706.dts       | 1 +
 arch/arm/dts/zynq-zc770-xm010.dts | 1 +
 arch/arm/dts/zynq-zc770-xm011.dts | 1 +
 arch/arm/dts/zynq-zc770-xm012.dts | 1 +
 arch/arm/dts/zynq-zc770-xm013.dts | 1 +
 arch/arm/dts/zynq-zed.dts         | 1 +
 arch/arm/dts/zynq-zybo.dts        | 1 +
 11 files changed, 19 insertions(+)

arch/arm/dts/zynq-7000.dtsi +0 -0

index 0b62cb0..12614f2 100644

Simon,

2015-08-30 0:10 GMT+09:00 Simon Glass <[email protected]>:

Quoted 25 lines

We need to mark some device tree nodes so that they are available before relocation. This enables driver model to find these automatically. In the case of SPL it ensures that these nodes will be retained in SPL.

Signed-off-by Simon Glass <[email protected]>

---

 arch/arm/dts/zynq-7000.dtsi       | 1 +
 arch/arm/dts/zynq-microzed.dts    | 5 +++++
 arch/arm/dts/zynq-picozed.dts     | 5 +++++
 arch/arm/dts/zynq-zc702.dts       | 1 +
 arch/arm/dts/zynq-zc706.dts       | 1 +
 arch/arm/dts/zynq-zc770-xm010.dts | 1 +
 arch/arm/dts/zynq-zc770-xm011.dts | 1 +
 arch/arm/dts/zynq-zc770-xm012.dts | 1 +
 arch/arm/dts/zynq-zc770-xm013.dts | 1 +
 arch/arm/dts/zynq-zed.dts         | 1 +
 arch/arm/dts/zynq-zybo.dts        | 1 +
 11 files changed, 19 insertions(+)

arch/arm/dts/zynq-7000.dtsi +0 -0

index 0b62cb0..12614f2 100644

arch/arm/dts/zynq-7000.dtsi line 60

Quoted 7 lines

arch/arm/dts/zynq-7000.dtsi +1 -0

@@ -54,6 +54,7 @@
        };
 
        amba: amba {
+               u-boot,dm-pre-reloc;
                compatible = "simple-bus";
                #address-cells = <1>;
                #size-cells = <1>;

You are adding "u-boot,dm-pre-reloc" to a simple-bus node.

With this commit, all the children are bound before relocation.

simple_bus_post_bind() calls dm_scan_fdt_node() with false for 'pre_reloc_only'. I guess, this implementation is a problem.

Currently, 'pre_reloc_only' can be specified for the first-level nodes.

For example, assume node structure like this:

    amba  {
                       uart0 {

             };

             pinctrl {

             };

                       usb {

             };

             eth {

             };
       };

Please tell me:
how to bind "uart0" and "pinctrl", but not "usb", "eth" before relocation.

Any idea to propagate 'pre_reloc_only' downwards?

Quoted 3 lines

Any idea to propagate 'pre_reloc_only' downwards?

Checking the GD_FLG_RELOC is the only thing I can think of. We don't want to add parameter to the bind() method I think.

We need to mark some device tree nodes so that they are available before relocation. This enables driver model to find these automatically. In the case of SPL it ensures that these nodes will be retained in SPL.

Signed-off-by Simon Glass <[email protected]>

---

 arch/arm/dts/zynq-7000.dtsi       | 1 +
 arch/arm/dts/zynq-microzed.dts    | 5 +++++
 arch/arm/dts/zynq-picozed.dts     | 5 +++++
 arch/arm/dts/zynq-zc702.dts       | 1 +
 arch/arm/dts/zynq-zc706.dts       | 1 +
 arch/arm/dts/zynq-zc770-xm010.dts | 1 +
 arch/arm/dts/zynq-zc770-xm011.dts | 1 +
 arch/arm/dts/zynq-zc770-xm012.dts | 1 +
 arch/arm/dts/zynq-zc770-xm013.dts | 1 +
 arch/arm/dts/zynq-zed.dts         | 1 +
 arch/arm/dts/zynq-zybo.dts        | 1 +
 11 files changed, 19 insertions(+)

arch/arm/dts/zynq-7000.dtsi +1 -0

index 0b62cb0..12614f2 100644
@@ -54,6 +54,7 @@
 	};
 
 	amba: amba {
+		u-boot,dm-pre-reloc;
 		compatible = "simple-bus";
 		#address-cells = <1>;
 		#size-cells = <1>;

arch/arm/dts/zynq-microzed.dts +0 -0

index c373a2c..5dff18e6 100644

Quoted 37 lines · Simon Glass

We need to mark some device tree nodes so that they are available before relocation. This enables driver model to find these automatically. In the case of SPL it ensures that these nodes will be retained in SPL.

Signed-off-by Simon Glass <[email protected]>

---

 arch/arm/dts/zynq-7000.dtsi       | 1 +
 arch/arm/dts/zynq-microzed.dts    | 5 +++++
 arch/arm/dts/zynq-picozed.dts     | 5 +++++
 arch/arm/dts/zynq-zc702.dts       | 1 +
 arch/arm/dts/zynq-zc706.dts       | 1 +
 arch/arm/dts/zynq-zc770-xm010.dts | 1 +
 arch/arm/dts/zynq-zc770-xm011.dts | 1 +
 arch/arm/dts/zynq-zc770-xm012.dts | 1 +
 arch/arm/dts/zynq-zc770-xm013.dts | 1 +
 arch/arm/dts/zynq-zed.dts         | 1 +
 arch/arm/dts/zynq-zybo.dts        | 1 +
 11 files changed, 19 insertions(+)

arch/arm/dts/zynq-7000.dtsi +1 -0

index 0b62cb0..12614f2 100644
@@ -54,6 +54,7 @@
      };
 
      amba: amba {
+             u-boot,dm-pre-reloc;
              compatible = "simple-bus";
              #address-cells = <1>;
              #size-cells = <1>;

arch/arm/dts/zynq-microzed.dts +0 -0

index c373a2c..5dff18e6 100644
@@ -21,3 +21,8 @@

arch/arm/dts/zynq-microzed.dts line 26

Quoted 6 lines

arch/arm/dts/zynq-microzed.dts +3 -0

              reg = <0 0x40000000>;
      };
 };
+
+&uart1 {
+     u-boot,dm-pre-reloc;

Was this reviewed on DT mailing list?

TBH adding this to every node seems to me a lot of work.

Why not just add one more uboot property to chosen with list of IPs which needs to be relocated?

Hi Michal,

Quoted 38 lines · Michal Simek

Quoted 37 lines · Simon Glass

We need to mark some device tree nodes so that they are available before relocation. This enables driver model to find these automatically. In the case of SPL it ensures that these nodes will be retained in SPL.

Signed-off-by Simon Glass <[email protected]>

---

 arch/arm/dts/zynq-7000.dtsi       | 1 +
 arch/arm/dts/zynq-microzed.dts    | 5 +++++
 arch/arm/dts/zynq-picozed.dts     | 5 +++++
 arch/arm/dts/zynq-zc702.dts       | 1 +
 arch/arm/dts/zynq-zc706.dts       | 1 +
 arch/arm/dts/zynq-zc770-xm010.dts | 1 +
 arch/arm/dts/zynq-zc770-xm011.dts | 1 +
 arch/arm/dts/zynq-zc770-xm012.dts | 1 +
 arch/arm/dts/zynq-zc770-xm013.dts | 1 +
 arch/arm/dts/zynq-zed.dts         | 1 +
 arch/arm/dts/zynq-zybo.dts        | 1 +
 11 files changed, 19 insertions(+)

arch/arm/dts/zynq-7000.dtsi +1 -0

index 0b62cb0..12614f2 100644
@@ -54,6 +54,7 @@
      };
 
      amba: amba {
+             u-boot,dm-pre-reloc;
              compatible = "simple-bus";
              #address-cells = <1>;
              #size-cells = <1>;

arch/arm/dts/zynq-microzed.dts +0 -0

index c373a2c..5dff18e6 100644
@@ -21,3 +21,8 @@

Quoted 8 lines

arch/arm/dts/zynq-microzed.dts line 26

Quoted 6 lines

arch/arm/dts/zynq-microzed.dts +3 -0

              reg = <0 0x40000000>;
      };
 };
+
+&uart1 {
+     u-boot,dm-pre-reloc;

Was this reviewed on DT mailing list?

There is a thread going at present for Raspberry Pi but it had not yielded much light last time I looked.

Quoted 1 line

Quoted 1 line

TBH adding this to every node seems to me a lot of work.

This i how it works at present. Typically we only have a UART and that is not necessary since U-Boot can force-bind this. But when the UART is not in the root node we have to add something.

Quoted 3 lines

Why not just add one more uboot property to chosen with list of IPs which needs to be relocated?

You mean a list of devices needed before relocation?

If you like you could look at working up a patch for this. I'm certainly interested in other ideas. It does need to be efficient.

Quoted 54 lines · Simon Glass

Hi Michal,

Quoted 38 lines · Michal Simek

Quoted 37 lines · Simon Glass

We need to mark some device tree nodes so that they are available before relocation. This enables driver model to find these automatically. In the case of SPL it ensures that these nodes will be retained in SPL.

Signed-off-by Simon Glass <[email protected]>

---

 arch/arm/dts/zynq-7000.dtsi       | 1 +
 arch/arm/dts/zynq-microzed.dts    | 5 +++++
 arch/arm/dts/zynq-picozed.dts     | 5 +++++
 arch/arm/dts/zynq-zc702.dts       | 1 +
 arch/arm/dts/zynq-zc706.dts       | 1 +
 arch/arm/dts/zynq-zc770-xm010.dts | 1 +
 arch/arm/dts/zynq-zc770-xm011.dts | 1 +
 arch/arm/dts/zynq-zc770-xm012.dts | 1 +
 arch/arm/dts/zynq-zc770-xm013.dts | 1 +
 arch/arm/dts/zynq-zed.dts         | 1 +
 arch/arm/dts/zynq-zybo.dts        | 1 +
 11 files changed, 19 insertions(+)

arch/arm/dts/zynq-7000.dtsi +1 -0

index 0b62cb0..12614f2 100644
@@ -54,6 +54,7 @@
      };
 
      amba: amba {
+             u-boot,dm-pre-reloc;
              compatible = "simple-bus";
              #address-cells = <1>;
              #size-cells = <1>;

arch/arm/dts/zynq-microzed.dts +0 -0

index c373a2c..5dff18e6 100644
@@ -21,3 +21,8 @@

Quoted 8 lines

arch/arm/dts/zynq-microzed.dts line 26

Quoted 6 lines

arch/arm/dts/zynq-microzed.dts +3 -0

              reg = <0 0x40000000>;
      };
 };
+
+&uart1 {
+     u-boot,dm-pre-reloc;

Was this reviewed on DT mailing list?

There is a thread going at present for Raspberry Pi but it had not yielded much light last time I looked.

Quoted 1 line

Quoted 5 lines

Quoted 1 line

TBH adding this to every node seems to me a lot of work.

This i how it works at present. Typically we only have a UART and that is not necessary since U-Boot can force-bind this. But when the UART is not in the root node we have to add something.

It is partially problem with DT mess that we have platforms with and without bus. :-)

Quoted 5 lines

Quoted 3 lines

Why not just add one more uboot property to chosen with list of IPs which needs to be relocated?

You mean a list of devices needed before relocation?

I mean something like this:

chosen {
        u-boot,dm-pre-reloc = <&uart1 ...>;
}

And then just go through this list. I expect that you are looking for that property anyway.

Quoted 4 lines

If you like you could look at working up a patch for this. I'm certainly interested in other ideas. It does need to be efficient.

I will test this series and will look at it in more details soon.

Hi Michal,

Quoted 72 lines · Michal Simek

Quoted 54 lines · Simon Glass

Hi Michal,

Quoted 38 lines · Michal Simek

Quoted 37 lines · Simon Glass

We need to mark some device tree nodes so that they are available before relocation. This enables driver model to find these automatically. In the case of SPL it ensures that these nodes will be retained in SPL.

Signed-off-by Simon Glass <[email protected]>

---

 arch/arm/dts/zynq-7000.dtsi       | 1 +
 arch/arm/dts/zynq-microzed.dts    | 5 +++++
 arch/arm/dts/zynq-picozed.dts     | 5 +++++
 arch/arm/dts/zynq-zc702.dts       | 1 +
 arch/arm/dts/zynq-zc706.dts       | 1 +
 arch/arm/dts/zynq-zc770-xm010.dts | 1 +
 arch/arm/dts/zynq-zc770-xm011.dts | 1 +
 arch/arm/dts/zynq-zc770-xm012.dts | 1 +
 arch/arm/dts/zynq-zc770-xm013.dts | 1 +
 arch/arm/dts/zynq-zed.dts         | 1 +
 arch/arm/dts/zynq-zybo.dts        | 1 +
 11 files changed, 19 insertions(+)

arch/arm/dts/zynq-7000.dtsi +1 -0

index 0b62cb0..12614f2 100644
@@ -54,6 +54,7 @@
      };
 
      amba: amba {
+             u-boot,dm-pre-reloc;
              compatible = "simple-bus";
              #address-cells = <1>;
              #size-cells = <1>;

arch/arm/dts/zynq-microzed.dts +0 -0

index c373a2c..5dff18e6 100644
@@ -21,3 +21,8 @@

Quoted 8 lines

arch/arm/dts/zynq-microzed.dts line 26

Quoted 6 lines

arch/arm/dts/zynq-microzed.dts +3 -0

              reg = <0 0x40000000>;
      };
 };
+
+&uart1 {
+     u-boot,dm-pre-reloc;

Was this reviewed on DT mailing list?

There is a thread going at present for Raspberry Pi but it had not yielded much light last time I looked.

Quoted 1 line

Quoted 5 lines

Quoted 1 line

TBH adding this to every node seems to me a lot of work.

This i how it works at present. Typically we only have a UART and that is not necessary since U-Boot can force-bind this. But when the UART is not in the root node we have to add something.

It is partially problem with DT mess that we have platforms with and without bus. :-)

Quoted 5 lines

Quoted 3 lines

Why not just add one more uboot property to chosen with list of IPs which needs to be relocated?

You mean a list of devices needed before relocation?

I mean something like this:

Quoted 6 lines

chosen {
        u-boot,dm-pre-reloc = <&uart1 ...>;
}

And then just go through this list. I expect that you are looking for that property anyway.

In this case wouldn't it need to list the simple-bus also?

We also use this with fdtgrep to remove nodes not needed for SPL. So we would have to come up with a tool to make that work. At present 'fdtgrep -p u-boot,dm-pre-reloc' picks out all the nodes we want (it finds nodes with that property).

I'm actually not sure that this approach is any easier/better. What are the advantages?

Quoted 5 lines

Quoted 4 lines

If you like you could look at working up a patch for this. I'm certainly interested in other ideas. It does need to be efficient.

Quoted 1 line

I will test this series and will look at it in more details soon.

Thanks.

BTW are there zynqmp dev boards available at reasonable cost? I did this Zynq series because I discovered some old patches that were not applied and decided to update then. It's a really interesting platform
- FPGA, etc.

Quoted 13 lines

Quoted 13 lines

Quoted 6 lines

Quoted 4 lines

Why not just add one more uboot property to chosen with list of IPs which needs to be relocated?

You mean a list of devices needed before relocation?

I mean something like this:

chosen {
        u-boot,dm-pre-reloc = <&uart1 ...>;
}

Quoted 4 lines

Quoted 2 lines

And then just go through this list. I expect that you are looking for that property anyway.

In this case wouldn't it need to list the simple-bus also?

yes for zc702 case

u-boot,dm-pre-reloc = <&amba &uart1>;

Quoted 7 lines

We also use this with fdtgrep to remove nodes not needed for SPL. So we would have to come up with a tool to make that work. At present 'fdtgrep -p u-boot,dm-pre-reloc' picks out all the nodes we want (it finds nodes with that property).

Quoted 2 lines

I'm actually not sure that this approach is any easier/better. What are the advantages?

The question is if current solution which you are using is fully compatible with binding. Adding bootloader property to the HW node doesn't look like a best solution.
On the other hand chosen node is the location where OS specific properties are coming that's why I have suggested to use it.

Hi.

2015-09-02 0:41 GMT+09:00 Michal Simek <[email protected]>:

Quoted 13 lines

Quoted 13 lines

Quoted 13 lines

Quoted 6 lines

Quoted 4 lines

Why not just add one more uboot property to chosen with list of IPs which needs to be relocated?

You mean a list of devices needed before relocation?

I mean something like this:

chosen {
        u-boot,dm-pre-reloc = <&uart1 ...>;
}

Quoted 8 lines

Quoted 4 lines

Quoted 2 lines

And then just go through this list. I expect that you are looking for that property anyway.

In this case wouldn't it need to list the simple-bus also?

yes for zc702 case

u-boot,dm-pre-reloc = <&amba &uart1>;

I think this should be

u-boot,dm-pre-reloc = &amba, &uart1;

<&label> is used for phandle, while &label is replaced with a string standing for the full path for the node.

For example, stdout-path takes that:

stdout-path = &serial0;

Quoted 7 lines

Quoted 7 lines

We also use this with fdtgrep to remove nodes not needed for SPL. So we would have to come up with a tool to make that work. At present 'fdtgrep -p u-boot,dm-pre-reloc' picks out all the nodes we want (it finds nodes with that property).

Quoted 8 lines

Quoted 2 lines

I'm actually not sure that this approach is any easier/better. What are the advantages?

The question is if current solution which you are using is fully compatible with binding. Adding bootloader property to the HW node doesn't look like a best solution.
On the other hand chosen node is the location where OS specific properties are coming that's why I have suggested to use it.

I like Michal's idea.
We need some work for fdtgrep, but I believe it is worthwhile.

From Michal's recent patches (http://patchwork.ozlabs.org/patch/498609/),
I guess he is tackling on syncing device trees between Linux and U-boot, and this is right thing to do.

Inserting the U-boot specific property here and there makes it difficult.

+Tom and a few others who may have an opinion.

Hi,

Quoted 69 lines · Masahiro Yamada

Hi.

2015-09-02 0:41 GMT+09:00 Michal Simek <[email protected]>:

Quoted 13 lines

Quoted 13 lines

Quoted 13 lines

Quoted 6 lines

Quoted 4 lines

Why not just add one more uboot property to chosen with list of IPs which needs to be relocated?

You mean a list of devices needed before relocation?

I mean something like this:

chosen {
        u-boot,dm-pre-reloc = <&uart1 ...>;
}

Quoted 8 lines

Quoted 4 lines

Quoted 2 lines

And then just go through this list. I expect that you are looking for that property anyway.

In this case wouldn't it need to list the simple-bus also?

yes for zc702 case

u-boot,dm-pre-reloc = <&amba &uart1>;

I think this should be

u-boot,dm-pre-reloc = &amba, &uart1;

<&label> is used for phandle, while &label is replaced with a string standing for the full path for the node.

For example, stdout-path takes that:

stdout-path = &serial0;

Quoted 7 lines

Quoted 7 lines

We also use this with fdtgrep to remove nodes not needed for SPL. So we would have to come up with a tool to make that work. At present 'fdtgrep -p u-boot,dm-pre-reloc' picks out all the nodes we want (it finds nodes with that property).

Quoted 8 lines

Quoted 2 lines

I'm actually not sure that this approach is any easier/better. What are the advantages?

The question is if current solution which you are using is fully compatible with binding. Adding bootloader property to the HW node doesn't look like a best solution.
On the other hand chosen node is the location where OS specific properties are coming that's why I have suggested to use it.

I like Michal's idea.
We need some work for fdtgrep, but I believe it is worthwhile.

From Michal's recent patches (http://patchwork.ozlabs.org/patch/498609/),
I guess he is tackling on syncing device trees between Linux and U-boot, and this is right thing to do.

Quoted 2 lines

Inserting the U-boot specific property here and there makes it difficult.

Yes it is attractive.

With this scheme we cannot put the properties into .dtsi (i.e. make them common for the soc). Is there a way around that or would we just have to live with it?

Quoted 75 lines · Simon Glass

+Tom and a few others who may have an opinion.

Hi,

Quoted 69 lines · Masahiro Yamada

Hi.

2015-09-02 0:41 GMT+09:00 Michal Simek <[email protected]>:

Quoted 13 lines

Quoted 13 lines

Quoted 13 lines

Quoted 6 lines

Quoted 4 lines

Why not just add one more uboot property to chosen with list of IPs which needs to be relocated?

You mean a list of devices needed before relocation?

I mean something like this:

chosen {
        u-boot,dm-pre-reloc = <&uart1 ...>;
}

Quoted 8 lines

Quoted 4 lines

Quoted 2 lines

And then just go through this list. I expect that you are looking for that property anyway.

In this case wouldn't it need to list the simple-bus also?

yes for zc702 case

u-boot,dm-pre-reloc = <&amba &uart1>;

I think this should be

u-boot,dm-pre-reloc = &amba, &uart1;

<&label> is used for phandle, while &label is replaced with a string standing for the full path for the node.

For example, stdout-path takes that:

stdout-path = &serial0;

Quoted 7 lines

Quoted 7 lines

We also use this with fdtgrep to remove nodes not needed for SPL. So we would have to come up with a tool to make that work. At present 'fdtgrep -p u-boot,dm-pre-reloc' picks out all the nodes we want (it finds nodes with that property).

Quoted 8 lines

Quoted 2 lines

I'm actually not sure that this approach is any easier/better. What are the advantages?

The question is if current solution which you are using is fully compatible with binding. Adding bootloader property to the HW node doesn't look like a best solution.
On the other hand chosen node is the location where OS specific properties are coming that's why I have suggested to use it.

I like Michal's idea.
We need some work for fdtgrep, but I believe it is worthwhile.

From Michal's recent patches (http://patchwork.ozlabs.org/patch/498609/),
I guess he is tackling on syncing device trees between Linux and U-boot, and this is right thing to do.

Quoted 8 lines

Quoted 2 lines

Inserting the U-boot specific property here and there makes it difficult.

Yes it is attractive.

With this scheme we cannot put the properties into .dtsi (i.e. make them common for the soc). Is there a way around that or would we just have to live with it?

Why not? You can add chosen node to dtsi without any problem which should be shared for all boards. The only one question remains which is what exactly you need to add to dtsi.
One example is Zynq. We have 2 serial PS IPs. Which one you want to add? Both?

System can have 0, 1 or 2 uarts or uart on any other IP in PL.

Thanks,
Michal

Hi Michal,

Quoted 85 lines · Michal Simek

Quoted 75 lines · Simon Glass

+Tom and a few others who may have an opinion.

Hi,

Quoted 69 lines · Masahiro Yamada

Hi.

2015-09-02 0:41 GMT+09:00 Michal Simek <[email protected]>:

Quoted 13 lines

Quoted 13 lines

Quoted 13 lines

Quoted 6 lines

Quoted 4 lines

Why not just add one more uboot property to chosen with list of IPs which needs to be relocated?

You mean a list of devices needed before relocation?

I mean something like this:

chosen {
        u-boot,dm-pre-reloc = <&uart1 ...>;
}

Quoted 8 lines

Quoted 4 lines

Quoted 2 lines

And then just go through this list. I expect that you are looking for that property anyway.

In this case wouldn't it need to list the simple-bus also?

yes for zc702 case

u-boot,dm-pre-reloc = <&amba &uart1>;

I think this should be

u-boot,dm-pre-reloc = &amba, &uart1;

<&label> is used for phandle, while &label is replaced with a string standing for the full path for the node.

For example, stdout-path takes that:

stdout-path = &serial0;

Quoted 7 lines

Quoted 7 lines

We also use this with fdtgrep to remove nodes not needed for SPL. So we would have to come up with a tool to make that work. At present 'fdtgrep -p u-boot,dm-pre-reloc' picks out all the nodes we want (it finds nodes with that property).

Quoted 8 lines

Quoted 2 lines

I'm actually not sure that this approach is any easier/better. What are the advantages?

The question is if current solution which you are using is fully compatible with binding. Adding bootloader property to the HW node doesn't look like a best solution.
On the other hand chosen node is the location where OS specific properties are coming that's why I have suggested to use it.

I like Michal's idea.
We need some work for fdtgrep, but I believe it is worthwhile.

From Michal's recent patches (http://patchwork.ozlabs.org/patch/498609/),
I guess he is tackling on syncing device trees between Linux and U-boot, and this is right thing to do.

Quoted 8 lines

Quoted 2 lines

Inserting the U-boot specific property here and there makes it difficult.

Yes it is attractive.

With this scheme we cannot put the properties into .dtsi (i.e. make them common for the soc). Is there a way around that or would we just have to live with it?

Quoted 5 lines

Why not? You can add chosen node to dtsi without any problem which should be shared for all boards. The only one question remains which is what exactly you need to add to dtsi.
One example is Zynq. We have 2 serial PS IPs. Which one you want to add? Both?

If you have something like this:

soc {
   uart0 {
   };
   uart1 {
   };
}

in the common .dtsi then you can currently put the marker in the soc node. I'm not sure how you do that with your scheme. If we put it in the .dtsi then the .dts will override it.

Does this matter?

Quoted 103 lines · Simon Glass

Hi Michal,

Quoted 85 lines · Michal Simek

Quoted 75 lines · Simon Glass

+Tom and a few others who may have an opinion.

Hi,

Quoted 69 lines · Masahiro Yamada

Hi.

2015-09-02 0:41 GMT+09:00 Michal Simek <[email protected]>:

Quoted 13 lines

Quoted 13 lines

Quoted 13 lines

Quoted 6 lines

Quoted 4 lines

Why not just add one more uboot property to chosen with list of IPs which needs to be relocated?

You mean a list of devices needed before relocation?

I mean something like this:

chosen {
        u-boot,dm-pre-reloc = <&uart1 ...>;
}

Quoted 8 lines

Quoted 4 lines

Quoted 2 lines

And then just go through this list. I expect that you are looking for that property anyway.

In this case wouldn't it need to list the simple-bus also?

yes for zc702 case

u-boot,dm-pre-reloc = <&amba &uart1>;

I think this should be

u-boot,dm-pre-reloc = &amba, &uart1;

<&label> is used for phandle, while &label is replaced with a string standing for the full path for the node.

For example, stdout-path takes that:

stdout-path = &serial0;

Quoted 7 lines

Quoted 7 lines

We also use this with fdtgrep to remove nodes not needed for SPL. So we would have to come up with a tool to make that work. At present 'fdtgrep -p u-boot,dm-pre-reloc' picks out all the nodes we want (it finds nodes with that property).

Quoted 8 lines

Quoted 2 lines

I'm actually not sure that this approach is any easier/better. What are the advantages?

The question is if current solution which you are using is fully compatible with binding. Adding bootloader property to the HW node doesn't look like a best solution.
On the other hand chosen node is the location where OS specific properties are coming that's why I have suggested to use it.

I like Michal's idea.
We need some work for fdtgrep, but I believe it is worthwhile.

From Michal's recent patches (http://patchwork.ozlabs.org/patch/498609/),
I guess he is tackling on syncing device trees between Linux and U-boot, and this is right thing to do.

Quoted 8 lines

Quoted 2 lines

Inserting the U-boot specific property here and there makes it difficult.

Yes it is attractive.

With this scheme we cannot put the properties into .dtsi (i.e. make them common for the soc). Is there a way around that or would we just have to live with it?

Quoted 5 lines

Why not? You can add chosen node to dtsi without any problem which should be shared for all boards. The only one question remains which is what exactly you need to add to dtsi.
One example is Zynq. We have 2 serial PS IPs. Which one you want to add? Both?

If you have something like this:

soc {
   uart0 {
   };
   uart1 {
   };
}

Quoted 5 lines

in the common .dtsi then you can currently put the marker in the soc node. I'm not sure how you do that with your scheme. If we put it in the .dtsi then the .dts will override it.

Does this matter?

As far as I understand DTSI is shared across all boards. And DTS can overwrite DTSI at any time.

Let me extend this to make it more clear

soc: soc {
    uart0: uart at XXX {
    };
    uart1: uart at XXX {
    };

}

In DTSI I would put probably this to show everybody what needs to be there.

chosen {
        u-boot,dm-pre-reloc = &soc &uart0 &uart1;
}

And in DTS if you have only one uart DTS will overwrite it.

chosen {
        u-boot,dm-pre-reloc = &soc &uart0;
}

Or the next option is to make code smarter and detect that uart1 is disabled that's why it is not used and only chosen from DTSI should be enough.

Quoted 75 lines

Quoted 47 lines

Quoted 32 lines

Quoted 23 lines

Quoted 23 lines

Quoted 6 lines

Quoted 6 lines

We also use this with fdtgrep to remove nodes not needed for SPL. So we would have to come up with a tool to make that work. At present 'fdtgrep -p u-boot,dm-pre-reloc' picks out all the nodes we want (it finds nodes with that property).

Quoted 8 lines

Quoted 2 lines

I'm actually not sure that this approach is any easier/better. What are the advantages?

The question is if current solution which you are using is fully compatible with binding. Adding bootloader property to the HW node doesn't look like a best solution.
On the other hand chosen node is the location where OS specific properties are coming that's why I have suggested to use it.

I like Michal's idea.
We need some work for fdtgrep, but I believe it is worthwhile.

From Michal's recent patches (http://patchwork.ozlabs.org/patch/498609/),
I guess he is tackling on syncing device trees between Linux and U-boot, and this is right thing to do.

Quoted 8 lines

Quoted 2 lines

Inserting the U-boot specific property here and there makes it difficult.

Yes it is attractive.

With this scheme we cannot put the properties into .dtsi (i.e. make them common for the soc). Is there a way around that or would we just have to live with it?

Quoted 5 lines

Why not? You can add chosen node to dtsi without any problem which should be shared for all boards. The only one question remains which is what exactly you need to add to dtsi.
One example is Zynq. We have 2 serial PS IPs. Which one you want to add? Both?

If you have something like this:

soc {
   uart0 {
   };
   uart1 {
   };
}

Quoted 5 lines

in the common .dtsi then you can currently put the marker in the soc node. I'm not sure how you do that with your scheme. If we put it in the .dtsi then the .dts will override it.

Does this matter?

As far as I understand DTSI is shared across all boards. And DTS can overwrite DTSI at any time.

Let me extend this to make it more clear

soc: soc {
    uart0: uart at XXX {
    };
    uart1: uart at XXX {
    };

}

In DTSI I would put probably this to show everybody what needs to be there.

chosen {
        u-boot,dm-pre-reloc = &soc &uart0 &uart1;
}

And in DTS if you have only one uart DTS will overwrite it.

chosen {
        u-boot,dm-pre-reloc = &soc &uart0;
}

Quoted 3 lines

Or the next option is to make code smarter and detect that uart1 is disabled that's why it is not used and only chosen from DTSI should be enough.

Yes I see - you just overwrite the property in the main file. If it includes pinctrl nodes, gpio nodes, etc. then you would have to add those also.

For mainline Rockchip this means that each board would have to have something like:

 chosen {
         u-boot,dm-pre-reloc = &mmc0, &serial0, &dmc, &power, &syscon0

&syscon1 &pinctrl &gpio3 &gpio8 &leds &led_work &led_power;
}

That seems a bit unwieldy to me. What do you think?

Hi Simon,

<cut some previous parts>

Quoted 79 lines

Quoted 78 lines

Quoted 47 lines

Quoted 32 lines

Quoted 23 lines

Quoted 23 lines

Quoted 6 lines

Quoted 6 lines

We also use this with fdtgrep to remove nodes not needed for SPL. So we would have to come up with a tool to make that work. At present 'fdtgrep -p u-boot,dm-pre-reloc' picks out all the nodes we want (it finds nodes with that property).

Quoted 8 lines

Quoted 2 lines

I'm actually not sure that this approach is any easier/better. What are the advantages?

The question is if current solution which you are using is fully compatible with binding. Adding bootloader property to the HW node doesn't look like a best solution.
On the other hand chosen node is the location where OS specific properties are coming that's why I have suggested to use it.

I like Michal's idea.
We need some work for fdtgrep, but I believe it is worthwhile.

From Michal's recent patches (http://patchwork.ozlabs.org/patch/498609/),
I guess he is tackling on syncing device trees between Linux and U-boot, and this is right thing to do.

Quoted 8 lines

Quoted 2 lines

Inserting the U-boot specific property here and there makes it difficult.

Yes it is attractive.

With this scheme we cannot put the properties into .dtsi (i.e. make them common for the soc). Is there a way around that or would we just have to live with it?

Quoted 5 lines

Why not? You can add chosen node to dtsi without any problem which should be shared for all boards. The only one question remains which is what exactly you need to add to dtsi.
One example is Zynq. We have 2 serial PS IPs. Which one you want to add? Both?

If you have something like this:

soc {
   uart0 {
   };
   uart1 {
   };
}

Quoted 5 lines

in the common .dtsi then you can currently put the marker in the soc node. I'm not sure how you do that with your scheme. If we put it in the .dtsi then the .dts will override it.

Does this matter?

As far as I understand DTSI is shared across all boards. And DTS can overwrite DTSI at any time.

Let me extend this to make it more clear

soc: soc {
    uart0: uart at XXX {
    };
    uart1: uart at XXX {
    };

}

In DTSI I would put probably this to show everybody what needs to be there.

chosen {
        u-boot,dm-pre-reloc = &soc &uart0 &uart1;
}

And in DTS if you have only one uart DTS will overwrite it.

chosen {
        u-boot,dm-pre-reloc = &soc &uart0;
}

Or the next option is to make code smarter and detect that uart1 is disabled that's why it is not used and only chosen from DTSI should be enough.

Quoted 3 lines

Yes I see - you just overwrite the property in the main file. If it includes pinctrl nodes, gpio nodes, etc. then you would have to add those also.

Right - DTSI is most like a pattern to follow and DTS is doing that platform specific stuff. It means in DTSI you can have guidance what to use.

Also in algorithm can be checking if that IP has status okay or disabled and do relocation or not.

Quoted 10 lines

For mainline Rockchip this means that each board would have to have something like:

 chosen {
         u-boot,dm-pre-reloc = &mmc0, &serial0, &dmc, &power, &syscon0

&syscon1 &pinctrl &gpio3 &gpio8 &leds &led_work &led_power;
}

Quoted 1 line

That seems a bit unwieldy to me. What do you think?

(note: without commas)
I think that this solution is compatible with the binding and it is better than adding bootloader specific property to nodes which I have never seen before. Chosen node was used for that maybe even from beginning.

Regarding technical aspects - I have never checked if there is any line length limitation on parameter side which can be problematic. On the other hand if yes phandles should be used.

Regarding compactness of this solution. From one line you can see what will be pre relocated which looks pretty compact to me and you can easy check if something is missing.

Hi Michal,

Quoted 7 lines · Michal Simek

Hi Simon,

<cut some previous parts>

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

We also use this with fdtgrep to remove nodes not needed for

SPL. So

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

we would have to come up with a tool to make that work. At

present

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

'fdtgrep -p u-boot,dm-pre-reloc' picks out all the nodes we

want (it

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

finds nodes with that property).

I'm actually not sure that this approach is any easier/better.

What

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 1 line

are the advantages?

The question is if current solution which you are using is fully compatible with binding. Adding bootloader property to the HW

node

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Quoted 3 lines

doesn't look like a best solution.
On the other hand chosen node is the location where OS specific properties are coming that's why I have suggested to use it.

I like Michal's idea.
We need some work for fdtgrep, but I believe it is worthwhile.

From Michal's recent patches (

http://patchwork.ozlabs.org/patch/498609/),

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

I guess he is tackling on syncing device trees between Linux and

U-boot,

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Quoted 4 lines

and this is right thing to do.

Inserting the U-boot specific property here and there makes it difficult.

Yes it is attractive.

With this scheme we cannot put the properties into .dtsi (i.e. make them common for the soc). Is there a way around that or would we

just

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 1 line

have to live with it?

Why not? You can add chosen node to dtsi without any problem which should be shared for all boards. The only one question remains

which is

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

what exactly you need to add to dtsi.
One example is Zynq. We have 2 serial PS IPs. Which one you want to

add?

Quoted 29 lines

Quoted 29 lines

Quoted 29 lines

Quoted 11 lines

Quoted 1 line

Both?

If you have something like this:

soc {
   uart0 {
   };
   uart1 {
   };
}

Quoted 5 lines

in the common .dtsi then you can currently put the marker in the soc node. I'm not sure how you do that with your scheme. If we put it in the .dtsi then the .dts will override it.

Does this matter?

As far as I understand DTSI is shared across all boards. And DTS can overwrite DTSI at any time.

Let me extend this to make it more clear

soc: soc {
    uart0: uart at XXX {
    };
    uart1: uart at XXX {
    };

}

Quoted 1 line

Quoted 1 line

Quoted 1 line

In DTSI I would put probably this to show everybody what needs to be

there.

Quoted 17 lines

Quoted 16 lines

Quoted 4 lines

chosen {
        u-boot,dm-pre-reloc = &soc &uart0 &uart1;
}

Quoted 8 lines

And in DTS if you have only one uart DTS will overwrite it.

chosen {
        u-boot,dm-pre-reloc = &soc &uart0;
}

Or the next option is to make code smarter and detect that uart1 is disabled that's why it is not used and only chosen from DTSI should be enough.

Yes I see - you just overwrite the property in the main file. If it includes pinctrl nodes, gpio nodes, etc. then you would have to add those also.

Quoted 3 lines

Right - DTSI is most like a pattern to follow and DTS is doing that platform specific stuff. It means in DTSI you can have guidance what to use.

Do you mean we should add a comment to the .dtsi to tell you want you need in your .dts? Is this improving anything?

Quoted 4 lines

Also in algorithm can be checking if that IP has status okay or disabled and do relocation or not.

Can you please rephrase that? I don't understand.

Quoted 10 lines

Quoted 10 lines

For mainline Rockchip this means that each board would have to have something like:

 chosen {
         u-boot,dm-pre-reloc = &mmc0, &serial0, &dmc, &power, &syscon0

&syscon1 &pinctrl &gpio3 &gpio8 &leds &led_work &led_power;
}

Quoted 6 lines

Quoted 1 line

That seems a bit unwieldy to me. What do you think?

(note: without commas)
I think that this solution is compatible with the binding and it is better than adding bootloader specific property to nodes which I have never seen before. Chosen node was used for that maybe even from

beginning.

There are Linux-specific properties, so I really don't see the difference. Don't forget that we are using device tree to control our boot loader, so it would be unusual to not see at least some U-Boot-specific properties.

My understanding of /chosen is that it is for the OS. But in any case it would still be a binding change, wouldn't it? What makes you think that this scheme would be more acceptable as a binding change?

Quoted 5 lines

Regarding technical aspects - I have never checked if there is any line length limitation on parameter side which can be problematic. On the other hand if yes phandles should be used.

No there is not limit I know of. It should be fine.

Quoted 5 lines

Regarding compactness of this solution. From one line you can see what will be pre relocated which looks pretty compact to me and you can easy check if something is missing.

Yes it is nice to have this in once place. But it's not really in one place as you need to reference nodes from elsewhere, and update all .dts files if something in the .dtsi changes. Granted that shouldn't happen often.

One advantage is that this copes with boards that have a common .dtsi but different drivers needed before relocation - e.g. one board might want to use a serial port attached to a /soc node, another might want to use something on /pci. But I have not seen this happen yet.

So overall i can see benefits and drawbacks. I'm on the fence.

Hi Simon,

first of all sorry for late reply. I am traveling and I have pretty busy time now.

2015-09-09 20:07 GMT+02:00 Simon Glass <[email protected]>:

Quoted 9 lines

There are Linux-specific properties, so I really don't see the difference. Don't forget that we are using device tree to control our boot loader, so it would be unusual to not see at least some U-Boot-specific properties.

My understanding of /chosen is that it is for the OS. But in any case it would still be a binding change, wouldn't it? What makes you think that this scheme would be more acceptable as a binding change?

I think we should move this discussion to device-tree mailing list. I simple just don't think that OS/bootloader property in the device node is a good idea.
Maybe there is better solution but i think adding OS/Bootloader to chosen is just better option
then added this to every node.
Not sure if sequence matter or not but via one property you can also control it.

Hi Simon,

first of all sorry for late reply. I am traveling and I have pretty busy time now.

2015-09-09 20:07 GMT+02:00 Simon Glass <[email protected]>:

Quoted 105 lines

Hi Michal,

Quoted 101 lines · Michal Simek

Hi Simon,

<cut some previous parts>

Quoted 95 lines

Quoted 83 lines

Quoted 58 lines

Quoted 41 lines

Quoted 33 lines

Quoted 24 lines

Quoted 13 lines

Quoted 9 lines

We also use this with fdtgrep to remove nodes not needed for SPL. So
we would have to come up with a tool to make that work. At present
'fdtgrep -p u-boot,dm-pre-reloc' picks out all the nodes we want (it
finds nodes with that property).

Quoted 3 lines

I'm actually not sure that this approach is any easier/better. What
are the advantages?

Quoted 6 lines

The question is if current solution which you are using is fully compatible with binding. Adding bootloader property to the HW node
doesn't look like a best solution.
On the other hand chosen node is the location where OS specific properties are coming that's why I have suggested to use it.

I like Michal's idea.
We need some work for fdtgrep, but I believe it is worthwhile.

Quoted 8 lines

From Michal's recent patches
(http://patchwork.ozlabs.org/patch/498609/),
I guess he is tackling on syncing device trees between Linux and U-boot,
and this is right thing to do.

Inserting the U-boot specific property here and there makes it difficult.

Quoted 7 lines

Yes it is attractive.

With this scheme we cannot put the properties into .dtsi (i.e. make
them common for the soc). Is there a way around that or would we just
have to live with it?

Quoted 7 lines

Why not? You can add chosen node to dtsi without any problem which should be shared for all boards. The only one question remains which is
what exactly you need to add to dtsi.
One example is Zynq. We have 2 serial PS IPs. Which one you want to add?
Both?

If you have something like this:

soc {
   uart0 {
   };
   uart1 {
   };
}

Quoted 5 lines

in the common .dtsi then you can currently put the marker in the soc node. I'm not sure how you do that with your scheme. If we put it in the .dtsi then the .dts will override it.

Does this matter?

As far as I understand DTSI is shared across all boards. And DTS can overwrite DTSI at any time.

Let me extend this to make it more clear

soc: soc {
    uart0: uart at XXX {
    };
    uart1: uart at XXX {
    };

}

In DTSI I would put probably this to show everybody what needs to be there.

chosen {
        u-boot,dm-pre-reloc = &soc &uart0 &uart1;
}

And in DTS if you have only one uart DTS will overwrite it.

Quoted 8 lines

chosen {
        u-boot,dm-pre-reloc = &soc &uart0;
}

Or the next option is to make code smarter and detect that uart1 is disabled that's why it is not used and only chosen from DTSI should be
enough.

Yes I see - you just overwrite the property in the main file. If it includes pinctrl nodes, gpio nodes, etc. then you would have to add those also.

Quoted 7 lines

Quoted 3 lines

Right - DTSI is most like a pattern to follow and DTS is doing that platform specific stuff. It means in DTSI you can have guidance what to use.

Do you mean we should add a comment to the .dtsi to tell you want you need in your .dts? Is this improving anything?

what I think it will be the best to add that line to DTSI with all IPs which should be pre relocated.
Then algorithm should be like this

for_each_IP_in_property {
if (status=ok)
relocate
else
ignore it
}

If you have others nodes in DTS then you have to rewrite it.

Quoted 29 lines

Quoted 7 lines

Also in algorithm can be checking if that IP has status okay or disabled and do relocation or not.

Can you please rephrase that? I don't understand.

Quoted 10 lines

Quoted 10 lines

For mainline Rockchip this means that each board would have to have something like:

 chosen {
         u-boot,dm-pre-reloc = &mmc0, &serial0, &dmc, &power, &syscon0

&syscon1 &pinctrl &gpio3 &gpio8 &leds &led_work &led_power;
}

Quoted 7 lines

Quoted 1 line

That seems a bit unwieldy to me. What do you think?

(note: without commas)
I think that this solution is compatible with the binding and it is better than adding bootloader specific property to nodes which I have never seen before. Chosen node was used for that maybe even from beginning.

Quoted 8 lines

There are Linux-specific properties, so I really don't see the difference. Don't forget that we are using device tree to control our boot loader, so it would be unusual to not see at least some U-Boot-specific properties.

My understanding of /chosen is that it is for the OS. But in any case it would still be a binding change, wouldn't it? What makes you think that this scheme would be more acceptable as a binding change?

I think we should move this discussion to device-tree mailing list. I simple just don't think that OS/bootloader property in the device node is a good idea.
Maybe there is better solution but i think adding OS/Bootloader to chosen is just better option
then added this to every node.
Not sure if sequence matter or not but via one property you can also control it.

Quoted 24 lines

Quoted 9 lines

Regarding technical aspects - I have never checked if there is any line length limitation on parameter side which can be problematic. On the other hand if yes phandles should be used.

No there is not limit I know of. It should be fine.

Quoted 5 lines

Regarding compactness of this solution. From one line you can see what will be pre relocated which looks pretty compact to me and you can easy check if something is missing.

Yes it is nice to have this in once place. But it's not really in one place as you need to reference nodes from elsewhere, and update all .dts files if something in the .dtsi changes. Granted that shouldn't happen often.

Quoted 6 lines

One advantage is that this copes with boards that have a common .dtsi but different drivers needed before relocation - e.g. one board might want to use a serial port attached to a /soc node, another might want to use something on /pci. But I have not seen this happen yet.

So overall i can see benefits and drawbacks. I'm on the fence.

TBH there should be any ACK from DT guys before this u-boot property was used.
From that discussion should come how this can be used.
In past I have some experience with using incorrect binding in private drivers and it ends up
in disaster that's why I want to make sure before we start to use this that it was properly review.

We need to mark some device tree nodes so that they are available before relocation. This enables driver model to find these automatically. In the case of SPL it ensures that these nodes will be retained in SPL.

Signed-off-by Simon Glass <[email protected]>

---

 arch/arm/dts/zynq-7000.dtsi       | 1 +
 arch/arm/dts/zynq-microzed.dts    | 5 +++++
 arch/arm/dts/zynq-picozed.dts     | 5 +++++
 arch/arm/dts/zynq-zc702.dts       | 1 +
 arch/arm/dts/zynq-zc706.dts       | 1 +
 arch/arm/dts/zynq-zc770-xm010.dts | 1 +
 arch/arm/dts/zynq-zc770-xm011.dts | 1 +
 arch/arm/dts/zynq-zc770-xm012.dts | 1 +
 arch/arm/dts/zynq-zc770-xm013.dts | 1 +
 arch/arm/dts/zynq-zed.dts         | 1 +
 arch/arm/dts/zynq-zybo.dts        | 1 +
 11 files changed, 19 insertions(+)

arch/arm/dts/zynq-7000.dtsi +0 -0

index 0b62cb0..12614f2 100644

Update this driver to use driver model and change all users.

Signed-off-by Simon Glass <[email protected]>

---

 arch/arm/Kconfig                |   1 +
 drivers/serial/serial_zynq.c    | 162 ++++++++++++++++------------------------
 include/configs/zynq_microzed.h |   1 -
 include/configs/zynq_picozed.h  |   1 -
 include/configs/zynq_zc70x.h    |   1 -
 include/configs/zynq_zc770.h    |   6 --
 include/configs/zynq_zed.h      |   1 -
 include/configs/zynq_zybo.h     |   1 -
 8 files changed, 65 insertions(+), 109 deletions(-)

Quoted 5 lines · Simon Glass

Update this driver to use driver model and change all users.

Signed-off-by Simon Glass <[email protected]>

---

Quoted 10 lines

 arch/arm/Kconfig                |   1 +
 drivers/serial/serial_zynq.c    | 162 ++++++++++++++++------------------------
 include/configs/zynq_microzed.h |   1 -
 include/configs/zynq_picozed.h  |   1 -
 include/configs/zynq_zc70x.h    |   1 -
 include/configs/zynq_zc770.h    |   6 --
 include/configs/zynq_zed.h      |   1 -
 include/configs/zynq_zybo.h     |   1 -
 8 files changed, 65 insertions(+), 109 deletions(-)

Unfortunately you forget to move ZynqMP target to DM. I have done it in our internal tree and it is probably the right time to send that patches out to cover it. Definitely they have to come to the tree first.

I will test all others patches but TBH I have moved serial driver but I didn't resolved that SPL case but it is wonderful that you have probably fixed it. I am looking for to see what will be sizes with and without DM.

Hi Michal,

Quoted 17 lines · Michal Simek

Quoted 15 lines · Simon Glass

Update this driver to use driver model and change all users.

Signed-off-by Simon Glass <[email protected]>

---

 arch/arm/Kconfig                |   1 +
 drivers/serial/serial_zynq.c    | 162 ++++++++++++++++------------------------
 include/configs/zynq_microzed.h |   1 -
 include/configs/zynq_picozed.h  |   1 -
 include/configs/zynq_zc70x.h    |   1 -
 include/configs/zynq_zc770.h    |   6 --
 include/configs/zynq_zed.h      |   1 -
 include/configs/zynq_zybo.h     |   1 -
 8 files changed, 65 insertions(+), 109 deletions(-)

Quoted 3 lines

Unfortunately you forget to move ZynqMP target to DM. I have done it in our internal tree and it is probably the right time to send that patches out to cover it. Definitely they have to come to the tree first.

See my note in the cover letter. ZynqMP does not build for me and I cannot see any device tree files in U-Boot.

Quoted 5 lines

I will test all others patches but TBH I have moved serial driver but I didn't resolved that SPL case but it is wonderful that you have probably fixed it. I am looking for to see what will be sizes with and without DM.

Yes, it will be larger. Likely there is some optimisation that can be done - I put some effort into this on Rockchip. Once we have a reasonable selection of boards using this in SPL we should review it carefully and see how to improve code size / performance. Fundamentally (in terms of architectural design) I think it is reasonable, but it is worth putting effort in to achieve the best possible result.

Update this driver to use driver model and change all users.

Signed-off-by Simon Glass <[email protected]>

---

 arch/arm/Kconfig                |   1 +
 drivers/serial/serial_zynq.c    | 162 ++++++++++++++++------------------------
 include/configs/zynq_microzed.h |   1 -
 include/configs/zynq_picozed.h  |   1 -
 include/configs/zynq_zc70x.h    |   1 -
 include/configs/zynq_zc770.h    |   6 --
 include/configs/zynq_zed.h      |   1 -
 include/configs/zynq_zybo.h     |   1 -
 8 files changed, 65 insertions(+), 109 deletions(-)

arch/arm/Kconfig +0 -0

index a82306e..7705e03 100644

arch/arm/Kconfig +0 -0

 	select DM_SPI_FLASH
 	select SPL_SEPARATE_BSS
 

drivers/serial/serial_zynq.c +42 -97

index c39c1a9..7069afe 100644
@@ -6,6 +6,8 @@
  */
 
 #include <common.h>
+#include <debug_uart.h>
+#include <dm.h>
 #include <errno.h>
 #include <fdtdec.h>
 #include <watchdog.h>

@@ -18,6 +20,7 @@
 DECLARE_GLOBAL_DATA_PTR;
 
 #define ZYNQ_UART_SR_TXFULL	0x00000010 /* TX FIFO full */
+#define ZYNQ_UART_SR_TXACTIVE	(1 << 11)  /* TX active */
 #define ZYNQ_UART_SR_RXEMPTY	0x00000002 /* RX FIFO empty */
 
 #define ZYNQ_UART_CR_TX_EN	0x00000010 /* TX enabled */

@@ -38,9 +41,8 @@ struct uart_zynq {
 	u32 baud_rate_divider; /* 0x34 - Baud Rate Divider [7:0] */
 };
 
-static struct uart_zynq *uart_zynq_ports[2] = {
-	[0] = (struct uart_zynq *)ZYNQ_SERIAL_BASEADDR0,
-	[1] = (struct uart_zynq *)ZYNQ_SERIAL_BASEADDR1,
+struct zynq_uart_priv {
+	struct uart_zynq *regs;
 };
 
 /* Set up the baud rate in gd struct */

@@ -84,15 +86,6 @@ static void _uart_zynq_serial_setbrg(struct uart_zynq *regs,
 	writel(bgen, &regs->baud_rate_gen);
 }
 
-/* Set up the baud rate in gd struct */
-static void uart_zynq_serial_setbrg(const int port)
-{
-	unsigned long clock = get_uart_clk(port);
-	struct uart_zynq *regs = uart_zynq_ports[port];
-
-	return _uart_zynq_serial_setbrg(regs, clock, gd->baudrate);
-}
-
 /* Initialize the UART, with...some settings. */
 static void _uart_zynq_serial_init(struct uart_zynq *regs)
 {

@@ -102,20 +95,6 @@ static void _uart_zynq_serial_init(struct uart_zynq *regs)
 	writel(ZYNQ_UART_MR_PARITY_NONE, &regs->mode); /* 8 bit, no parity */
 }
 
-/* Initialize the UART, with...some settings. */
-static int uart_zynq_serial_init(const int port)
-{
-	struct uart_zynq *regs = uart_zynq_ports[port];
-
-	if (!regs)
-		return -1;
-
-	_uart_zynq_serial_init(regs);
-	uart_zynq_serial_setbrg(port);
-
-	return 0;
-}
-
 static int _uart_zynq_serial_putc(struct uart_zynq *regs, const char c)
 {
 	if (readl(&regs->channel_sts) & ZYNQ_UART_SR_TXFULL)

@@ -126,103 +105,90 @@ static int _uart_zynq_serial_putc(struct uart_zynq *regs, const char c)
 	return 0;
 }
 
-static void uart_zynq_serial_putc(const char c, const int port)
+int zynq_serial_setbrg(struct udevice *dev, int baudrate)
 {
-	struct uart_zynq *regs = uart_zynq_ports[port];
+	struct zynq_uart_priv *priv = dev_get_priv(dev);
+	unsigned long clock = get_uart_clk(0);
 
-	while (_uart_zynq_serial_putc(regs, c) == -EAGAIN)
-		WATCHDOG_RESET();
+	_uart_zynq_serial_setbrg(priv->regs, clock, baudrate);
 
-	if (c == '\n') {
-		while (_uart_zynq_serial_putc(regs, '\r') == -EAGAIN)
-			WATCHDOG_RESET();
-	}
+	return 0;
 }
 
-static void uart_zynq_serial_puts(const char *s, const int port)
+static int zynq_serial_probe(struct udevice *dev)
 {
-	while (*s)
-		uart_zynq_serial_putc(*s++, port);
-}
+	struct zynq_uart_priv *priv = dev_get_priv(dev);
 
-static int uart_zynq_serial_tstc(const int port)
-{
-	struct uart_zynq *regs = uart_zynq_ports[port];
+	_uart_zynq_serial_init(priv->regs);
 
-	return (readl(&regs->channel_sts) & ZYNQ_UART_SR_RXEMPTY) == 0;
+	return 0;
 }
 
-static int uart_zynq_serial_getc(const int port)
+static int zynq_serial_getc(struct udevice *dev)
 {
-	struct uart_zynq *regs = uart_zynq_ports[port];
+	struct zynq_uart_priv *priv = dev_get_priv(dev);
+	struct uart_zynq *regs = priv->regs;
+
+	if (readl(&regs->channel_sts) & ZYNQ_UART_SR_RXEMPTY)
+		return -EAGAIN;
 
-	while (!uart_zynq_serial_tstc(port))
-		WATCHDOG_RESET();
 	return readl(&regs->tx_rx_fifo);
 }
 
-/* Multi serial device functions */
-#define DECLARE_PSSERIAL_FUNCTIONS(port) \
-	static int uart_zynq##port##_init(void) \
-				{ return uart_zynq_serial_init(port); } \
-	static void uart_zynq##port##_setbrg(void) \
-				{ return uart_zynq_serial_setbrg(port); } \
-	static int uart_zynq##port##_getc(void) \
-				{ return uart_zynq_serial_getc(port); } \
-	static int uart_zynq##port##_tstc(void) \
-				{ return uart_zynq_serial_tstc(port); } \
-	static void uart_zynq##port##_putc(const char c) \
-				{ uart_zynq_serial_putc(c, port); } \
-	static void uart_zynq##port##_puts(const char *s) \
-				{ uart_zynq_serial_puts(s, port); }
-
-/* Serial device descriptor */
-#define INIT_PSSERIAL_STRUCTURE(port, __name) {	\
-	  .name   = __name,			\
-	  .start  = uart_zynq##port##_init,	\
-	  .stop   = NULL,			\
-	  .setbrg = uart_zynq##port##_setbrg,	\
-	  .getc   = uart_zynq##port##_getc,	\
-	  .tstc   = uart_zynq##port##_tstc,	\
-	  .putc   = uart_zynq##port##_putc,	\
-	  .puts   = uart_zynq##port##_puts,	\
-}
+static int zynq_serial_putc(struct udevice *dev, const char ch)
+{
+	struct zynq_uart_priv *priv = dev_get_priv(dev);
 
-DECLARE_PSSERIAL_FUNCTIONS(0);
-static struct serial_device uart_zynq_serial0_device =
-	INIT_PSSERIAL_STRUCTURE(0, "ttyPS0");
-DECLARE_PSSERIAL_FUNCTIONS(1);
-static struct serial_device uart_zynq_serial1_device =
-	INIT_PSSERIAL_STRUCTURE(1, "ttyPS1");
+	return _uart_zynq_serial_putc(priv->regs, ch);
+}
 
-__weak struct serial_device *default_serial_console(void)
+static int zynq_serial_pending(struct udevice *dev, bool input)
 {
-	const void *blob = gd->fdt_blob;
-	int node;
-	unsigned int base_addr;
+	struct zynq_uart_priv *priv = dev_get_priv(dev);
+	struct uart_zynq *regs = priv->regs;
 
-	node = fdt_path_offset(blob, "serial0");
-	if (node < 0)
-		return NULL;
+	if (input)
+		return !(readl(&regs->channel_sts) & ZYNQ_UART_SR_RXEMPTY);
+	else
+		return !!(readl(&regs->channel_sts) & ZYNQ_UART_SR_TXACTIVE);
+}
 
-	base_addr = fdtdec_get_addr(blob, node, "reg");
-	if (base_addr == FDT_ADDR_T_NONE)
-		return NULL;
+static int zynq_serial_ofdata_to_platdata(struct udevice *dev)
+{
+	struct zynq_uart_priv *priv = dev_get_priv(dev);
+	fdt_addr_t addr;
 
-	if (base_addr == ZYNQ_SERIAL_BASEADDR0)
-		return &uart_zynq_serial0_device;
+	addr = fdtdec_get_addr(gd->fdt_blob, dev->of_offset, "reg");
+	if (addr == FDT_ADDR_T_NONE)
+		return -EINVAL;
 
-	if (base_addr == ZYNQ_SERIAL_BASEADDR1)
-		return &uart_zynq_serial1_device;
+	priv->regs = (struct uart_zynq *)addr;
 
-	return NULL;
+	return 0;
 }
 
-void zynq_serial_initialize(void)
-{
-	serial_register(&uart_zynq_serial0_device);
-	serial_register(&uart_zynq_serial1_device);

This series updates the Zynq serial driver to use driver model. Along the way several problems are fixed:

- Support for /chosen/stdout-path using an alias
- Fix to fdtgrep which is currently breaking alias building
- Avoid building u-boot-spl-dtb.bin when it is not requested
- Deal with boards which have BSS in SDRAM

For zynq this series makes a few changes:
- Use the new SPL init procedure, which just involves a few tweaks for zynq
- Add debug UART support
- Move to using a separate device tree instead of embedded

Only zybo has been tested. Testing on other zynq boards is welcome. They are all set up roughly the same so I expect only minor problems.

For some reason zynqmp does not have a device tree, so this series does not work on that. But that board fails to build on mainline for me anyway so perhaps nothing is lost. I don't have a board to test with so help on this would be appreciated!

Quoted 16 lines · Simon Glass

This series updates the Zynq serial driver to use driver model. Along the way several problems are fixed:

- Support for /chosen/stdout-path using an alias
- Fix to fdtgrep which is currently breaking alias building
- Avoid building u-boot-spl-dtb.bin when it is not requested
- Deal with boards which have BSS in SDRAM

For zynq this series makes a few changes:
- Use the new SPL init procedure, which just involves a few tweaks for zynq
- Add debug UART support
- Move to using a separate device tree instead of embedded

Only zybo has been tested. Testing on other zynq boards is welcome. They are all set up roughly the same so I expect only minor problems.

Quoted 4 lines

For some reason zynqmp does not have a device tree, so this series does not work on that. But that board fails to build on mainline for me anyway so perhaps nothing is lost. I don't have a board to test with so help on this would be appreciated!

I have no problem to build zynqmp on my PC. Can you please c&p error which you are getting?

Hi Michal,

Quoted 17 lines · Michal Simek

Quoted 16 lines · Simon Glass

This series updates the Zynq serial driver to use driver model. Along the way several problems are fixed:

- Support for /chosen/stdout-path using an alias
- Fix to fdtgrep which is currently breaking alias building
- Avoid building u-boot-spl-dtb.bin when it is not requested
- Deal with boards which have BSS in SDRAM

For zynq this series makes a few changes:
- Use the new SPL init procedure, which just involves a few tweaks for zynq
- Add debug UART support
- Move to using a separate device tree instead of embedded

Only zybo has been tested. Testing on other zynq boards is welcome. They are all set up roughly the same so I expect only minor problems.

Quoted 7 lines

Quoted 4 lines

For some reason zynqmp does not have a device tree, so this series does not work on that. But that board fails to build on mainline for me anyway so perhaps nothing is lost. I don't have a board to test with so help on this would be appreciated!

I have no problem to build zynqmp on my PC. Can you please c&p error which you are getting?

I get this:

$ buildman zynqmp
Building current source for 1 boards (1 thread, 32 jobs per thread)
   aarch64:  +   xilinx_zynqmp_ep

(quoted diff) +31 -0

+      ((u32)(priv->rxbuffers) +
+       ^
+   writel((u32)priv->rx_bd, &regs->rxqbase);
+          ^
+arch/arm/include/asm/io.h:146:34: note: in definition of macro 'writel'
+ #define writel(v,c) ({ u32 __v = v; __iowmb(); __arch_putl(__v,c); __v; })
+                                  ^
+  writel((u32)priv->tx_bd, &regs->txqbase);
+         ^
+  priv->tx_bd->addr = (u32)ptr;
+                      ^
+  addr = (u32) ptr;
+  addr = (u32)priv->rxbuffers;
+   net_process_received_packet((u8 *)addr, frame_len);
+                               ^
+  priv->rx_bd = (struct emac_bd *)((u32)bd_space + BD_SEPRN_SPACE);
+                                   ^
+                ^
+  dwc3_flush_cache((int)trb, sizeof(*trb));
+                   ^
+  dma_unmap_single((void *)dwc->scratch_addr, dwc->nr_scratch *
+  dma_unmap_single((void *)req->dma, req->length,
+    dwc3_flush_cache((int)trb, sizeof(*trb));
+                     ^
+   dwc3_flush_cache((int)dwc->ep0_bounce, DWC3_EP0_BOUNCE_SIZE);
+                    ^
+  dwc->regs = (int *)(dwc3_dev->base + DWC3_GLOBALS_REGS_START);
+              ^
+  debug("%s: dev->in_req->length:%d to_cpy:%d\n", __func__,
+  ^
+    dwc3_flush_cache((int)evt->buf, evt->length);

w+drivers/net/zynq_gem.c: In function 'zynq_gem_init':
w+drivers/net/zynq_gem.c:330:7: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
w+In file included from drivers/net/zynq_gem.c:19:0:
w+drivers/net/zynq_gem.c:336:10: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/net/zynq_gem.c: In function 'zynq_gem_send':
w+drivers/net/zynq_gem.c:399:9: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/net/zynq_gem.c:404:22: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/net/zynq_gem.c:409:9: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/net/zynq_gem.c:414:9: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/net/zynq_gem.c: In function 'zynq_gem_recv':
w+drivers/net/zynq_gem.c:454:31: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] w+drivers/net/zynq_gem.c: In function 'zynq_gem_initialize':
w+drivers/net/zynq_gem.c:533:35: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/net/zynq_gem.c:533:16: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] w+drivers/usb/dwc3/ep0.c: In function 'dwc3_ep0_start_trans':
w+drivers/usb/dwc3/ep0.c:85:19: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/usb/dwc3/core.c: In function 'dwc3_setup_scratch_buffers':
w+drivers/usb/dwc3/core.c:284:19: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] w+drivers/usb/dwc3/core.c: In function 'dwc3_free_scratch_buffers':
w+drivers/usb/dwc3/core.c:299:19: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] w+drivers/usb/gadget/udc/udc-core.c: In function 'usb_gadget_unmap_request':
w+drivers/usb/gadget/udc/udc-core.c:68:19: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] w+drivers/usb/dwc3/ep0.c: In function 'dwc3_ep0_complete_data':
w+drivers/usb/dwc3/ep0.c:793:19: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/usb/dwc3/ep0.c:824:21: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/usb/dwc3/ep0.c:834:20: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/usb/dwc3/core.c: In function 'dwc3_uboot_init':
w+drivers/usb/dwc3/core.c:632:14: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] w+drivers/usb/dwc3/gadget.c: In function 'dwc3_prepare_one_trb':
w+drivers/usb/dwc3/gadget.c:775:19: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/usb/dwc3/gadget.c: In function 'dwc3_cleanup_done_reqs':
w+drivers/usb/dwc3/gadget.c:1772:19: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]

w+drivers/usb/gadget/f_thor.c: In function 'thor_tx_data':
w+drivers/usb/gadget/f_thor.c:568:2: warning: format '%d' expects argument of type 'int', but argument 4 has type 'long unsigned int' [-Wformat=]
w+drivers/usb/dwc3/gadget.c: In function 'dwc3_gadget_uboot_handle_interrupt':
w+drivers/usb/dwc3/gadget.c:2673:21: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
0 1 0 /1 xilinx_zynqmp_ep

Hi Michal,

Quoted 17 lines · Michal Simek

Quoted 16 lines · Simon Glass

This series updates the Zynq serial driver to use driver model. Along the way several problems are fixed:

- Support for /chosen/stdout-path using an alias
- Fix to fdtgrep which is currently breaking alias building
- Avoid building u-boot-spl-dtb.bin when it is not requested
- Deal with boards which have BSS in SDRAM

For zynq this series makes a few changes:
- Use the new SPL init procedure, which just involves a few tweaks for zynq
- Add debug UART support
- Move to using a separate device tree instead of embedded

Only zybo has been tested. Testing on other zynq boards is welcome. They are all set up roughly the same so I expect only minor problems.

Quoted 7 lines

Quoted 4 lines

For some reason zynqmp does not have a device tree, so this series does not work on that. But that board fails to build on mainline for me anyway so perhaps nothing is lost. I don't have a board to test with so help on this would be appreciated!

I have no problem to build zynqmp on my PC. Can you please c&p error which you are getting?

I get this:

$ buildman zynqmp
Building current source for 1 boards (1 thread, 32 jobs per thread)
   aarch64:  +   xilinx_zynqmp_ep

(quoted diff) +31 -0

+      ((u32)(priv->rxbuffers) +
+       ^
+   writel((u32)priv->rx_bd, &regs->rxqbase);
+          ^
+arch/arm/include/asm/io.h:146:34: note: in definition of macro 'writel'
+ #define writel(v,c) ({ u32 __v = v; __iowmb(); __arch_putl(__v,c); __v; })
+                                  ^
+  writel((u32)priv->tx_bd, &regs->txqbase);
+         ^
+  priv->tx_bd->addr = (u32)ptr;
+                      ^
+  addr = (u32) ptr;
+  addr = (u32)priv->rxbuffers;
+   net_process_received_packet((u8 *)addr, frame_len);
+                               ^
+  priv->rx_bd = (struct emac_bd *)((u32)bd_space + BD_SEPRN_SPACE);
+                                   ^
+                ^
+  dwc3_flush_cache((int)trb, sizeof(*trb));
+                   ^
+  dma_unmap_single((void *)dwc->scratch_addr, dwc->nr_scratch *
+  dma_unmap_single((void *)req->dma, req->length,
+    dwc3_flush_cache((int)trb, sizeof(*trb));
+                     ^
+   dwc3_flush_cache((int)dwc->ep0_bounce, DWC3_EP0_BOUNCE_SIZE);
+                    ^
+  dwc->regs = (int *)(dwc3_dev->base + DWC3_GLOBALS_REGS_START);
+              ^
+  debug("%s: dev->in_req->length:%d to_cpy:%d\n", __func__,
+  ^
+    dwc3_flush_cache((int)evt->buf, evt->length);

w+drivers/net/zynq_gem.c: In function 'zynq_gem_init':
w+drivers/net/zynq_gem.c:330:7: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
w+In file included from drivers/net/zynq_gem.c:19:0:
w+drivers/net/zynq_gem.c:336:10: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/net/zynq_gem.c: In function 'zynq_gem_send':
w+drivers/net/zynq_gem.c:399:9: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/net/zynq_gem.c:404:22: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/net/zynq_gem.c:409:9: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/net/zynq_gem.c:414:9: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/net/zynq_gem.c: In function 'zynq_gem_recv':
w+drivers/net/zynq_gem.c:454:31: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] w+drivers/net/zynq_gem.c: In function 'zynq_gem_initialize':
w+drivers/net/zynq_gem.c:533:35: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/net/zynq_gem.c:533:16: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] w+drivers/usb/dwc3/ep0.c: In function 'dwc3_ep0_start_trans':
w+drivers/usb/dwc3/ep0.c:85:19: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/usb/dwc3/core.c: In function 'dwc3_setup_scratch_buffers':
w+drivers/usb/dwc3/core.c:284:19: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] w+drivers/usb/dwc3/core.c: In function 'dwc3_free_scratch_buffers':
w+drivers/usb/dwc3/core.c:299:19: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] w+drivers/usb/gadget/udc/udc-core.c: In function 'usb_gadget_unmap_request':
w+drivers/usb/gadget/udc/udc-core.c:68:19: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] w+drivers/usb/dwc3/ep0.c: In function 'dwc3_ep0_complete_data':
w+drivers/usb/dwc3/ep0.c:793:19: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/usb/dwc3/ep0.c:824:21: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/usb/dwc3/ep0.c:834:20: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/usb/dwc3/core.c: In function 'dwc3_uboot_init':
w+drivers/usb/dwc3/core.c:632:14: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] w+drivers/usb/dwc3/gadget.c: In function 'dwc3_prepare_one_trb':
w+drivers/usb/dwc3/gadget.c:775:19: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/usb/dwc3/gadget.c: In function 'dwc3_cleanup_done_reqs':
w+drivers/usb/dwc3/gadget.c:1772:19: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]

w+drivers/usb/gadget/f_thor.c: In function 'thor_tx_data':
w+drivers/usb/gadget/f_thor.c:568:2: warning: format '%d' expects argument of type 'int', but argument 4 has type 'long unsigned int' [-Wformat=]
w+drivers/usb/dwc3/gadget.c: In function 'dwc3_gadget_uboot_handle_interrupt':
w+drivers/usb/dwc3/gadget.c:2673:21: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
0 1 0 /1 xilinx_zynqmp_ep

Quoted 115 lines · Simon Glass

Hi Michal,

Quoted 17 lines · Michal Simek

Quoted 16 lines · Simon Glass

This series updates the Zynq serial driver to use driver model. Along the way several problems are fixed:

- Support for /chosen/stdout-path using an alias
- Fix to fdtgrep which is currently breaking alias building
- Avoid building u-boot-spl-dtb.bin when it is not requested
- Deal with boards which have BSS in SDRAM

For zynq this series makes a few changes:
- Use the new SPL init procedure, which just involves a few tweaks for zynq
- Add debug UART support
- Move to using a separate device tree instead of embedded

Only zybo has been tested. Testing on other zynq boards is welcome. They are all set up roughly the same so I expect only minor problems.

Quoted 7 lines

Quoted 4 lines

For some reason zynqmp does not have a device tree, so this series does not work on that. But that board fails to build on mainline for me anyway so perhaps nothing is lost. I don't have a board to test with so help on this would be appreciated!

I have no problem to build zynqmp on my PC. Can you please c&p error which you are getting?

I get this:

$ buildman zynqmp
Building current source for 1 boards (1 thread, 32 jobs per thread)
   aarch64:  +   xilinx_zynqmp_ep

(quoted diff) +31 -0

+      ((u32)(priv->rxbuffers) +
+       ^
+   writel((u32)priv->rx_bd, &regs->rxqbase);
+          ^
+arch/arm/include/asm/io.h:146:34: note: in definition of macro 'writel'
+ #define writel(v,c) ({ u32 __v = v; __iowmb(); __arch_putl(__v,c); __v; })
+                                  ^
+  writel((u32)priv->tx_bd, &regs->txqbase);
+         ^
+  priv->tx_bd->addr = (u32)ptr;
+                      ^
+  addr = (u32) ptr;
+  addr = (u32)priv->rxbuffers;
+   net_process_received_packet((u8 *)addr, frame_len);
+                               ^
+  priv->rx_bd = (struct emac_bd *)((u32)bd_space + BD_SEPRN_SPACE);
+                                   ^
+                ^
+  dwc3_flush_cache((int)trb, sizeof(*trb));
+                   ^
+  dma_unmap_single((void *)dwc->scratch_addr, dwc->nr_scratch *
+  dma_unmap_single((void *)req->dma, req->length,
+    dwc3_flush_cache((int)trb, sizeof(*trb));
+                     ^
+   dwc3_flush_cache((int)dwc->ep0_bounce, DWC3_EP0_BOUNCE_SIZE);
+                    ^
+  dwc->regs = (int *)(dwc3_dev->base + DWC3_GLOBALS_REGS_START);
+              ^
+  debug("%s: dev->in_req->length:%d to_cpy:%d\n", __func__,
+  ^
+    dwc3_flush_cache((int)evt->buf, evt->length);

w+drivers/net/zynq_gem.c: In function 'zynq_gem_init':
w+drivers/net/zynq_gem.c:330:7: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
w+In file included from drivers/net/zynq_gem.c:19:0:
w+drivers/net/zynq_gem.c:336:10: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/net/zynq_gem.c: In function 'zynq_gem_send':
w+drivers/net/zynq_gem.c:399:9: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/net/zynq_gem.c:404:22: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/net/zynq_gem.c:409:9: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/net/zynq_gem.c:414:9: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/net/zynq_gem.c: In function 'zynq_gem_recv':
w+drivers/net/zynq_gem.c:454:31: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] w+drivers/net/zynq_gem.c: In function 'zynq_gem_initialize':
w+drivers/net/zynq_gem.c:533:35: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/net/zynq_gem.c:533:16: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] w+drivers/usb/dwc3/ep0.c: In function 'dwc3_ep0_start_trans':
w+drivers/usb/dwc3/ep0.c:85:19: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/usb/dwc3/core.c: In function 'dwc3_setup_scratch_buffers':
w+drivers/usb/dwc3/core.c:284:19: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] w+drivers/usb/dwc3/core.c: In function 'dwc3_free_scratch_buffers':
w+drivers/usb/dwc3/core.c:299:19: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] w+drivers/usb/gadget/udc/udc-core.c: In function 'usb_gadget_unmap_request':
w+drivers/usb/gadget/udc/udc-core.c:68:19: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] w+drivers/usb/dwc3/ep0.c: In function 'dwc3_ep0_complete_data':
w+drivers/usb/dwc3/ep0.c:793:19: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/usb/dwc3/ep0.c:824:21: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/usb/dwc3/ep0.c:834:20: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/usb/dwc3/core.c: In function 'dwc3_uboot_init':
w+drivers/usb/dwc3/core.c:632:14: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] w+drivers/usb/dwc3/gadget.c: In function 'dwc3_prepare_one_trb':
w+drivers/usb/dwc3/gadget.c:775:19: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/usb/dwc3/gadget.c: In function 'dwc3_cleanup_done_reqs':
w+drivers/usb/dwc3/gadget.c:1772:19: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]

Quoted 9 lines

w+drivers/usb/gadget/f_thor.c: In function 'thor_tx_data':
w+drivers/usb/gadget/f_thor.c:568:2: warning: format '%d' expects argument of type 'int', but argument 4 has type 'long unsigned int' [-Wformat=]
w+drivers/usb/dwc3/gadget.c: In function 'dwc3_gadget_uboot_handle_interrupt':
w+drivers/usb/dwc3/gadget.c:2673:21: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
0 1 0 /1 xilinx_zynqmp_ep

Here are step for running qemu.

git clone git://git.qemu.org/qemu.git
cd qemu
git submodule update --init pixman
./configure --target-list=aarch64-softmmu --disable-werror

./aarch64-softmmu/qemu-system-aarch64 -M xlnx-ep108 -display none -kernel /mnt/disk/u-boot/u-boot.elf -m 8000000 -nographic -serial mon:stdio

Please try and you will see.

I have ZynqMP Serial DM. Will send patches tmr.

Quoted 138 lines · Michal Simek

Quoted 115 lines · Simon Glass

Hi Michal,

Quoted 17 lines · Michal Simek

Quoted 16 lines · Simon Glass

This series updates the Zynq serial driver to use driver model. Along the way several problems are fixed:

- Support for /chosen/stdout-path using an alias
- Fix to fdtgrep which is currently breaking alias building
- Avoid building u-boot-spl-dtb.bin when it is not requested
- Deal with boards which have BSS in SDRAM

For zynq this series makes a few changes:
- Use the new SPL init procedure, which just involves a few tweaks for zynq
- Add debug UART support
- Move to using a separate device tree instead of embedded

Only zybo has been tested. Testing on other zynq boards is welcome. They are all set up roughly the same so I expect only minor problems.

Quoted 7 lines

Quoted 4 lines

For some reason zynqmp does not have a device tree, so this series does not work on that. But that board fails to build on mainline for me anyway so perhaps nothing is lost. I don't have a board to test with so help on this would be appreciated!

I have no problem to build zynqmp on my PC. Can you please c&p error which you are getting?

I get this:

$ buildman zynqmp
Building current source for 1 boards (1 thread, 32 jobs per thread)
   aarch64:  +   xilinx_zynqmp_ep

(quoted diff) +31 -0

+      ((u32)(priv->rxbuffers) +
+       ^
+   writel((u32)priv->rx_bd, &regs->rxqbase);
+          ^
+arch/arm/include/asm/io.h:146:34: note: in definition of macro 'writel'
+ #define writel(v,c) ({ u32 __v = v; __iowmb(); __arch_putl(__v,c); __v; })
+                                  ^
+  writel((u32)priv->tx_bd, &regs->txqbase);
+         ^
+  priv->tx_bd->addr = (u32)ptr;
+                      ^
+  addr = (u32) ptr;
+  addr = (u32)priv->rxbuffers;
+   net_process_received_packet((u8 *)addr, frame_len);
+                               ^
+  priv->rx_bd = (struct emac_bd *)((u32)bd_space + BD_SEPRN_SPACE);
+                                   ^
+                ^
+  dwc3_flush_cache((int)trb, sizeof(*trb));
+                   ^
+  dma_unmap_single((void *)dwc->scratch_addr, dwc->nr_scratch *
+  dma_unmap_single((void *)req->dma, req->length,
+    dwc3_flush_cache((int)trb, sizeof(*trb));
+                     ^
+   dwc3_flush_cache((int)dwc->ep0_bounce, DWC3_EP0_BOUNCE_SIZE);
+                    ^
+  dwc->regs = (int *)(dwc3_dev->base + DWC3_GLOBALS_REGS_START);
+              ^
+  debug("%s: dev->in_req->length:%d to_cpy:%d\n", __func__,
+  ^
+    dwc3_flush_cache((int)evt->buf, evt->length);

w+drivers/net/zynq_gem.c: In function 'zynq_gem_init':
w+drivers/net/zynq_gem.c:330:7: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
w+In file included from drivers/net/zynq_gem.c:19:0:
w+drivers/net/zynq_gem.c:336:10: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/net/zynq_gem.c: In function 'zynq_gem_send':
w+drivers/net/zynq_gem.c:399:9: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/net/zynq_gem.c:404:22: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/net/zynq_gem.c:409:9: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/net/zynq_gem.c:414:9: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/net/zynq_gem.c: In function 'zynq_gem_recv':
w+drivers/net/zynq_gem.c:454:31: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] w+drivers/net/zynq_gem.c: In function 'zynq_gem_initialize':
w+drivers/net/zynq_gem.c:533:35: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/net/zynq_gem.c:533:16: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] w+drivers/usb/dwc3/ep0.c: In function 'dwc3_ep0_start_trans':
w+drivers/usb/dwc3/ep0.c:85:19: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/usb/dwc3/core.c: In function 'dwc3_setup_scratch_buffers':
w+drivers/usb/dwc3/core.c:284:19: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] w+drivers/usb/dwc3/core.c: In function 'dwc3_free_scratch_buffers':
w+drivers/usb/dwc3/core.c:299:19: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] w+drivers/usb/gadget/udc/udc-core.c: In function 'usb_gadget_unmap_request':
w+drivers/usb/gadget/udc/udc-core.c:68:19: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] w+drivers/usb/dwc3/ep0.c: In function 'dwc3_ep0_complete_data':
w+drivers/usb/dwc3/ep0.c:793:19: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/usb/dwc3/ep0.c:824:21: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/usb/dwc3/ep0.c:834:20: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/usb/dwc3/core.c: In function 'dwc3_uboot_init':
w+drivers/usb/dwc3/core.c:632:14: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] w+drivers/usb/dwc3/gadget.c: In function 'dwc3_prepare_one_trb':
w+drivers/usb/dwc3/gadget.c:775:19: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] w+drivers/usb/dwc3/gadget.c: In function 'dwc3_cleanup_done_reqs':
w+drivers/usb/dwc3/gadget.c:1772:19: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]

Quoted 9 lines

w+drivers/usb/gadget/f_thor.c: In function 'thor_tx_data':
w+drivers/usb/gadget/f_thor.c:568:2: warning: format '%d' expects argument of type 'int', but argument 4 has type 'long unsigned int' [-Wformat=]
w+drivers/usb/dwc3/gadget.c: In function 'dwc3_gadget_uboot_handle_interrupt':
w+drivers/usb/dwc3/gadget.c:2673:21: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
0 1 0 /1 xilinx_zynqmp_ep

Here are step for running qemu.

git clone git://git.qemu.org/qemu.git
cd qemu
git submodule update --init pixman
./configure --target-list=aarch64-softmmu --disable-werror

./aarch64-softmmu/qemu-system-aarch64 -M xlnx-ep108 -display none -kernel /mnt/disk/u-boot/u-boot.elf -m 8000000 -nographic -serial mon:stdio

Please try and you will see.