[U-Boot] [PATCH 00/13] dm: arm: zynq: Convert serial driver to driver model

[U-Boot] [PATCH 00/13] dm: arm: zynq: Convert serial driver to driver model

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

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!


Simon Glass (13):
  fdt: Add a function to look up a /chosen property
  fdt: Correct handling of alias regions
  fdtgrep: Simplify the alias generation code
  dm: serial: Deal with stdout-path with an alias
  dm: spl: Generate u-boot-spl-dtb.bin only when enabled
  dm: spl: Support device tree when BSS is in a different section
  arm: zynq: Use separate device tree instead of embedded
  arm: zynq: Drop unnecessary code in SPL board_init_f()
  arm: zynq: Support the debug UART
  dm: arm: zynq: Enable device tree control in SPL
  arm: zynq: dts: Add U-Boot device tree additions
  arm: zynq: serial: Drop non-device-tree serial driver portions
  arm: zynq: Move serial driver to driver model

 Kconfig                            |  10 ++
 arch/arm/Kconfig                   |   4 +
 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 +
 arch/arm/mach-zynq/spl.c           |  12 ++-
 arch/arm/mach-zynq/u-boot-spl.lds  |  10 +-
 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        |   6 +-
 drivers/serial/Kconfig             |   7 ++
 drivers/serial/serial-uclass.c     |  30 +++++-
 drivers/serial/serial_zynq.c       | 202 ++++++++++++++++++-------------------
 include/asm-generic/sections.h     |   1 +
 include/configs/zynq-common.h      |   6 +-
 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 -
 include/fdtdec.h                   |  11 +-
 lib/fdtdec.c                       |  22 ++--
 lib/libfdt/fdt_region.c            |   2 +-
 scripts/Makefile.spl               |   2 +
 tools/fdtgrep.c                    |  32 ++----
 42 files changed, 239 insertions(+), 168 deletions(-)

-- 
2.5.0.457.gab17608

[U-Boot] [PATCH 01/13] fdt: Add a function to look up a /chosen property

[Simon Glass]

It is sometimes useful to find a property in the chosen node. Add a function
for this.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/fdtdec.h | 11 ++++++++++-
 lib/fdtdec.c     | 15 ++++++++++-----
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/include/fdtdec.h b/include/fdtdec.h
index 0cb6fa0..2049d12 100644
--- a/include/fdtdec.h
+++ b/include/fdtdec.h
@@ -534,7 +534,16 @@ int fdtdec_get_alias_seq(const void *blob, const char *base, int node,
 			 int *seqp);
 
 /**
- * Get the offset of the given chosen node
+ * Get a property from the /chosen node
+ *
+ * @param blob		Device tree blob (if NULL, then NULL is returned)
+ * @param name		Property name to look up
+ * @return Value of property, or NULL if it does not exist
+ */
+const char *fdtdec_get_chosen_prop(const void *blob, const char *name);
+
+/**
+ * Get the offset of the given /chosen node
  *
  * This looks up a property in /chosen containing the path to another node,
  * then finds the offset of that node.
diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index 81b54f8..38888de 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -529,16 +529,21 @@ int fdtdec_get_alias_seq(const void *blob, const char *base, int offset,
 	return -ENOENT;
 }
 
-int fdtdec_get_chosen_node(const void *blob, const char *name)
+const char *fdtdec_get_chosen_prop(const void *blob, const char *name)
 {
-	const char *prop;
 	int chosen_node;
-	int len;
 
 	if (!blob)
-		return -FDT_ERR_NOTFOUND;
+		return NULL;
 	chosen_node = fdt_path_offset(blob, "/chosen");
-	prop = fdt_getprop(blob, chosen_node, name, &len);
+	return fdt_getprop(blob, chosen_node, name, NULL);
+}
+
+int fdtdec_get_chosen_node(const void *blob, const char *name)
+{
+	const char *prop;
+
+	prop = fdtdec_get_chosen_prop(blob, name);
 	if (!prop)
 		return -FDT_ERR_NOTFOUND;
 	return fdt_path_offset(blob, prop);
-- 
2.5.0.457.gab17608

[U-Boot] [PATCH 02/13] fdt: Correct handling of alias regions

[Simon Glass]

At present the last four bytes of the alias region are dropped in
the case where the last alias is included. This results in a corrupted
device tree. Fix this.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 lib/libfdt/fdt_region.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/libfdt/fdt_region.c b/lib/libfdt/fdt_region.c
index 9fea775..747d8bb 100644
--- a/lib/libfdt/fdt_region.c
+++ b/lib/libfdt/fdt_region.c
@@ -101,7 +101,7 @@ int fdt_add_alias_regions(const void *fdt, struct fdt_region *region, int count,
 			continue;
 		next = fdt_next_property_offset(fdt, offset);
 		if (next < 0)
-			next = node_end - sizeof(fdt32_t);
+			next = node_end;
 
 		if (!did_alias_header) {
 			fdt_add_region(info, base + node, 12);
-- 
2.5.0.457.gab17608

[U-Boot] [PATCH 03/13] fdtgrep: Simplify the alias generation code

[Simon Glass]

We don't need to allocate a new region list when we run out of space.
The outer function can take care of this for us.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 tools/fdtgrep.c | 32 ++++++++++----------------------
 1 file changed, 10 insertions(+), 22 deletions(-)

diff --git a/tools/fdtgrep.c b/tools/fdtgrep.c
index caaf600..67aa41a 100644
--- a/tools/fdtgrep.c
+++ b/tools/fdtgrep.c
@@ -667,28 +667,16 @@ static int fdtgrep_find_regions(const void *fdt,
 
 		new_count = fdt_add_alias_regions(fdt, region, count,
 						  max_regions, &state);
-		if (new_count > max_regions) {
-			region = malloc(new_count * sizeof(struct fdt_region));
-			if (!region) {
-				fprintf(stderr,
-					"Out of memory for %d regions\n",
-					count);
-				return -1;
-			}
-			memcpy(region, state.region,
-			       count * sizeof(struct fdt_region));
-			free(state.region);
-			new_count = fdt_add_alias_regions(fdt, region, count,
-							  max_regions, &state);
+		if (new_count <= max_regions) {
+			/*
+			* The alias regions will now be at the end of the list.
+			* Sort the regions by offset to get things into the
+			* right order
+			*/
+			count = new_count;
+			qsort(region, count, sizeof(struct fdt_region),
+			      h_cmp_region);
 		}
-
-		/*
-		 * The alias regions will now be@the end of the list. Sort
-		 * the regions by offset to get things into the right order
-		 */
-		qsort(region, new_count, sizeof(struct fdt_region),
-		      h_cmp_region);
-		count = new_count;
 	}
 
 	if (ret != -FDT_ERR_NOTFOUND)
@@ -805,7 +793,7 @@ static int do_fdtgrep(struct display_info *disp, const char *filename)
 	 * The first pass will count the regions, but if it is too many,
 	 * we do another pass to actually record them.
 	 */
-	for (i = 0; i < 2; i++) {
+	for (i = 0; i < 3; i++) {
 		region = malloc(count * sizeof(struct fdt_region));
 		if (!region) {
 			fprintf(stderr, "Out of memory for %d regions\n",
-- 
2.5.0.457.gab17608

[U-Boot] [PATCH 04/13] dm: serial: Deal with stdout-path with an alias

[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 <sjg@chromium.org>
---

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

diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
index 55011cc..842f78b 100644
--- a/drivers/serial/serial-uclass.c
+++ b/drivers/serial/serial-uclass.c
@@ -29,14 +29,34 @@ static const unsigned long baudrate_table[] = CONFIG_SYS_BAUDRATE_TABLE;
 
 static void serial_find_console_or_panic(void)
 {
+	const void *blob = gd->fdt_blob;
 	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);
+			}
+		}
 		if (node < 0)
-			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;
 			}
 		}
 	}
-	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!).
-- 
2.5.0.457.gab17608

[U-Boot] [PATCH 04/13] dm: serial: Deal with stdout-path with an alias

[Michal Simek]

On 08/29/2015 05:10 PM, Simon Glass wrote:
> 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 <sjg@chromium.org>
> ---
> 
>  drivers/serial/serial-uclass.c | 30 +++++++++++++++++++++++++-----
>  1 file changed, 25 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
> index 55011cc..842f78b 100644
> --- a/drivers/serial/serial-uclass.c
> +++ b/drivers/serial/serial-uclass.c
> @@ -29,14 +29,34 @@ static const unsigned long baudrate_table[] = CONFIG_SYS_BAUDRATE_TABLE;
>  
>  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.

>  	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);
> +			}
> +		}

And this is second.

>  		if (node < 0)

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

> -			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;
>  			}
>  		}
>  	}
> -	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!).
> 

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150831/c8a1771f/attachment.sig>

[U-Boot] [PATCH 04/13] dm: serial: Deal with stdout-path with an alias

[Simon Glass]

Hi Michal,

On 31 August 2015 at 05:13, Michal Simek <monstr@monstr.eu> wrote:
> On 08/29/2015 05:10 PM, Simon Glass wrote:
>> 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 <sjg@chromium.org>
>> ---
>>
>>  drivers/serial/serial-uclass.c | 30 +++++++++++++++++++++++++-----
>>  1 file changed, 25 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
>> index 55011cc..842f78b 100644
>> --- a/drivers/serial/serial-uclass.c
>> +++ b/drivers/serial/serial-uclass.c
>> @@ -29,14 +29,34 @@ static const unsigned long baudrate_table[] = CONFIG_SYS_BAUDRATE_TABLE;
>>
>>  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.

>
>>       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);
>> +                     }
>> +             }
>
> And this is second.
>
>>               if (node < 0)
>
> You have if (node < 0) above too which looks pretty odd.

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

>
>> -                     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;
>>                       }
>>               }
>>       }
>> -     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!).
>>

Regards,
Simon

[U-Boot] [PATCH 04/13] dm: serial: Deal with stdout-path with an alias

[Michal Simek]

On 08/31/2015 03:54 PM, Simon Glass wrote:
> Hi Michal,
> 
> On 31 August 2015 at 05:13, Michal Simek <monstr@monstr.eu> wrote:
>> On 08/29/2015 05:10 PM, Simon Glass wrote:
>>> 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 <sjg@chromium.org>
>>> ---
>>>
>>>  drivers/serial/serial-uclass.c | 30 +++++++++++++++++++++++++-----
>>>  1 file changed, 25 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
>>> index 55011cc..842f78b 100644
>>> --- a/drivers/serial/serial-uclass.c
>>> +++ b/drivers/serial/serial-uclass.c
>>> @@ -29,14 +29,34 @@ static const unsigned long baudrate_table[] = CONFIG_SYS_BAUDRATE_TABLE;
>>>
>>>  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.
> 
>>
>>>       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);
>>> +                     }
>>> +             }
>>
>> And this is second.
>>
>>>               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.

Thanks,
Michal



-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150831/f42e0d4d/attachment.sig>

[U-Boot] [PATCH 04/13] dm: serial: Deal with stdout-path with an alias

[Simon Glass]

Hi Michal,

On 31 August 2015 at 08:08, Michal Simek <monstr@monstr.eu> wrote:
> On 08/31/2015 03:54 PM, Simon Glass wrote:
>> Hi Michal,
>>
>> On 31 August 2015 at 05:13, Michal Simek <monstr@monstr.eu> wrote:
>>> On 08/29/2015 05:10 PM, Simon Glass wrote:
>>>> 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 <sjg@chromium.org>
>>>> ---
>>>>
>>>>  drivers/serial/serial-uclass.c | 30 +++++++++++++++++++++++++-----
>>>>  1 file changed, 25 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
>>>> index 55011cc..842f78b 100644
>>>> --- a/drivers/serial/serial-uclass.c
>>>> +++ b/drivers/serial/serial-uclass.c
>>>> @@ -29,14 +29,34 @@ static const unsigned long baudrate_table[] = CONFIG_SYS_BAUDRATE_TABLE;
>>>>
>>>>  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.
>>
>>>
>>>>       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);
>>>> +                     }
>>>> +             }
>>>
>>> And this is second.
>>>
>>>>               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.

OK I will add a comment.

Regards,
Simon

[U-Boot] [PATCH 05/13] dm: spl: Generate u-boot-spl-dtb.bin only when enabled

[Simon Glass]

At present this file is generated even when device tree is not enabled in
SPL. Avoid this, since this file serves no purpose in that case.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 scripts/Makefile.spl | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
index 2df93c8..dd235b9 100644
--- a/scripts/Makefile.spl
+++ b/scripts/Makefile.spl
@@ -129,7 +129,9 @@ boot.bin: $(obj)/u-boot-spl.bin
 
 ALL-y	+= $(obj)/$(SPL_BIN).bin $(obj)/$(SPL_BIN).cfg
 
+ifdef CONFIG_SPL_OF_CONTROL
 ALL-$(CONFIG_OF_SEPARATE) += $(obj)/$(SPL_BIN)-pad.bin $(obj)/$(SPL_BIN)-dtb.bin
+endif
 
 ifdef CONFIG_SAMSUNG
 ALL-y	+= $(obj)/$(BOARD)-spl.bin
-- 
2.5.0.457.gab17608

[U-Boot] [PATCH 06/13] dm: spl: Support device tree when BSS is in a different section

[Simon Glass]

At present in SPL we place the device tree immediately after BSS. This
avoids needing to copy it out of the way before BSS can be used. However on
some boards BSS is not placed with the image - e.g. it can be in RAM if
available.

Add an option to tell U-Boot that the device tree should be placed at the
end of the image binary (_image_binary_end) instead of at the end of BSS.

Note: A common reason to place BSS in RAM is to support the FAT filesystem.
We should update the code so that it does not use so much BSS.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 Kconfig                        | 10 ++++++++++
 include/asm-generic/sections.h |  1 +
 lib/fdtdec.c                   |  7 +++++--
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/Kconfig b/Kconfig
index 05a34f7..819fa90 100644
--- a/Kconfig
+++ b/Kconfig
@@ -132,6 +132,16 @@ config SPL_STACK_R_ADDR
 	  Specify the address in SDRAM for the SPL stack. This will be set up
 	  before board_init_r() is called.
 
+config SPL_SEPARATE_BSS
+	depends on SPL
+	bool "BSS section is in a different memory region from text"
+	help
+	  Some platforms need a large BSS region in SPL and can provide this
+	  because RAM is already set up. In this case BSS can be moved to RAM.
+	  This option should then be enabled so that the correct device tree
+	  location is used. Normally we put the device tree at the end of BSS
+	  but with this option enabled, it goes at _image_binary_end.
+
 config TPL
 	bool
 	depends on SPL && SUPPORT_TPL
diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
index 458952f..328bc62 100644
--- a/include/asm-generic/sections.h
+++ b/include/asm-generic/sections.h
@@ -71,6 +71,7 @@ extern char __bss_start[];
 extern char __bss_end[];
 extern char __image_copy_start[];
 extern char __image_copy_end[];
+extern char _image_binary_end[];
 extern char __rel_dyn_start[];
 extern char __rel_dyn_end[];
 
diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index 38888de..e8684a4 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -1150,8 +1150,11 @@ int fdtdec_setup(void)
 	gd->fdt_blob = __dtb_dt_begin;
 # elif defined CONFIG_OF_SEPARATE
 #  ifdef CONFIG_SPL_BUILD
-	/* FDT is at end of BSS */
-	gd->fdt_blob = (ulong *)&__bss_end;
+	/* FDT is at end of BSS unless it is in a different memory region */
+	if (IS_ENABLED(CONFIG_SPL_SEPARATE_BSS))
+		gd->fdt_blob = (ulong *)&_image_binary_end;
+	else
+		gd->fdt_blob = (ulong *)&__bss_end;
 #  else
 	/* FDT is at end of image */
 	gd->fdt_blob = (ulong *)&_end;
-- 
2.5.0.457.gab17608

[U-Boot] [PATCH 07/13] arm: zynq: Use separate device tree instead of embedded

[Simon Glass]

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 <sjg@chromium.org>
---

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

diff --git a/configs/zynq_microzed_defconfig b/configs/zynq_microzed_defconfig
index e9c3209..9d51540 100644
--- a/configs/zynq_microzed_defconfig
+++ b/configs/zynq_microzed_defconfig
@@ -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
diff --git a/configs/zynq_picozed_defconfig b/configs/zynq_picozed_defconfig
index f2b71e9..3a42efb 100644
--- a/configs/zynq_picozed_defconfig
+++ b/configs/zynq_picozed_defconfig
@@ -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
diff --git a/configs/zynq_zc702_defconfig b/configs/zynq_zc702_defconfig
index 0abb7a8..6faf928 100644
--- a/configs/zynq_zc702_defconfig
+++ b/configs/zynq_zc702_defconfig
@@ -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
diff --git a/configs/zynq_zc706_defconfig b/configs/zynq_zc706_defconfig
index d67f507..d6559b5 100644
--- a/configs/zynq_zc706_defconfig
+++ b/configs/zynq_zc706_defconfig
@@ -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
diff --git a/configs/zynq_zc70x_defconfig b/configs/zynq_zc70x_defconfig
index 37c249f..49c987a 100644
--- a/configs/zynq_zc70x_defconfig
+++ b/configs/zynq_zc70x_defconfig
@@ -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
diff --git a/configs/zynq_zc770_xm010_defconfig b/configs/zynq_zc770_xm010_defconfig
index 0e826bb..4bfb9cc 100644
--- a/configs/zynq_zc770_xm010_defconfig
+++ b/configs/zynq_zc770_xm010_defconfig
@@ -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
diff --git a/configs/zynq_zc770_xm011_defconfig b/configs/zynq_zc770_xm011_defconfig
index 46d043b..2a61fe3 100644
--- a/configs/zynq_zc770_xm011_defconfig
+++ b/configs/zynq_zc770_xm011_defconfig
@@ -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
diff --git a/configs/zynq_zc770_xm012_defconfig b/configs/zynq_zc770_xm012_defconfig
index 34d479f..eb98a39 100644
--- a/configs/zynq_zc770_xm012_defconfig
+++ b/configs/zynq_zc770_xm012_defconfig
@@ -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
diff --git a/configs/zynq_zc770_xm013_defconfig b/configs/zynq_zc770_xm013_defconfig
index c59599f..8d65c05 100644
--- a/configs/zynq_zc770_xm013_defconfig
+++ b/configs/zynq_zc770_xm013_defconfig
@@ -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
diff --git a/configs/zynq_zed_defconfig b/configs/zynq_zed_defconfig
index 886b4a5..13bef36 100644
--- a/configs/zynq_zed_defconfig
+++ b/configs/zynq_zed_defconfig
@@ -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
diff --git a/configs/zynq_zybo_defconfig b/configs/zynq_zybo_defconfig
index 77b9409..fc251dc 100644
--- a/configs/zynq_zybo_defconfig
+++ b/configs/zynq_zybo_defconfig
@@ -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
diff --git a/include/configs/zynq-common.h b/include/configs/zynq-common.h
index e7ab50a..aa4785f 100644
--- a/include/configs/zynq-common.h
+++ b/include/configs/zynq-common.h
@@ -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
 
 /* Disable dcache for SPL just for sure */
-- 
2.5.0.457.gab17608

[U-Boot] [PATCH 07/13] arm: zynq: Use separate device tree instead of embedded

[Michal Simek]

On 08/29/2015 05:10 PM, Simon Glass wrote:
> 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.

> 
> 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 <sjg@chromium.org>
> ---
> 
>  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(-)
> 
> diff --git a/configs/zynq_microzed_defconfig b/configs/zynq_microzed_defconfig
> index e9c3209..9d51540 100644
> --- a/configs/zynq_microzed_defconfig
> +++ b/configs/zynq_microzed_defconfig
> @@ -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
> diff --git a/configs/zynq_picozed_defconfig b/configs/zynq_picozed_defconfig
> index f2b71e9..3a42efb 100644
> --- a/configs/zynq_picozed_defconfig
> +++ b/configs/zynq_picozed_defconfig
> @@ -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
> diff --git a/configs/zynq_zc702_defconfig b/configs/zynq_zc702_defconfig
> index 0abb7a8..6faf928 100644
> --- a/configs/zynq_zc702_defconfig
> +++ b/configs/zynq_zc702_defconfig
> @@ -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
> diff --git a/configs/zynq_zc706_defconfig b/configs/zynq_zc706_defconfig
> index d67f507..d6559b5 100644
> --- a/configs/zynq_zc706_defconfig
> +++ b/configs/zynq_zc706_defconfig
> @@ -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
> diff --git a/configs/zynq_zc70x_defconfig b/configs/zynq_zc70x_defconfig
> index 37c249f..49c987a 100644
> --- a/configs/zynq_zc70x_defconfig
> +++ b/configs/zynq_zc70x_defconfig
> @@ -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
> diff --git a/configs/zynq_zc770_xm010_defconfig b/configs/zynq_zc770_xm010_defconfig
> index 0e826bb..4bfb9cc 100644
> --- a/configs/zynq_zc770_xm010_defconfig
> +++ b/configs/zynq_zc770_xm010_defconfig
> @@ -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
> diff --git a/configs/zynq_zc770_xm011_defconfig b/configs/zynq_zc770_xm011_defconfig
> index 46d043b..2a61fe3 100644
> --- a/configs/zynq_zc770_xm011_defconfig
> +++ b/configs/zynq_zc770_xm011_defconfig
> @@ -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
> diff --git a/configs/zynq_zc770_xm012_defconfig b/configs/zynq_zc770_xm012_defconfig
> index 34d479f..eb98a39 100644
> --- a/configs/zynq_zc770_xm012_defconfig
> +++ b/configs/zynq_zc770_xm012_defconfig
> @@ -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
> diff --git a/configs/zynq_zc770_xm013_defconfig b/configs/zynq_zc770_xm013_defconfig
> index c59599f..8d65c05 100644
> --- a/configs/zynq_zc770_xm013_defconfig
> +++ b/configs/zynq_zc770_xm013_defconfig
> @@ -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
> diff --git a/configs/zynq_zed_defconfig b/configs/zynq_zed_defconfig
> index 886b4a5..13bef36 100644
> --- a/configs/zynq_zed_defconfig
> +++ b/configs/zynq_zed_defconfig
> @@ -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
> diff --git a/configs/zynq_zybo_defconfig b/configs/zynq_zybo_defconfig
> index 77b9409..fc251dc 100644
> --- a/configs/zynq_zybo_defconfig
> +++ b/configs/zynq_zybo_defconfig
> @@ -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
> diff --git a/include/configs/zynq-common.h b/include/configs/zynq-common.h
> index e7ab50a..aa4785f 100644
> --- a/include/configs/zynq-common.h
> +++ b/include/configs/zynq-common.h
> @@ -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
>  
>  /* Disable dcache for SPL just for sure */
> 

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

Thanks,
Michal



-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150831/cbb14eaf/attachment.sig>

[U-Boot] [PATCH 07/13] arm: zynq: Use separate device tree instead of embedded

[Simon Glass]

Hi Michal,

On 31 August 2015 at 05:24, Michal Simek <monstr@monstr.eu> wrote:
> On 08/29/2015 05:10 PM, Simon Glass wrote:
>> 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.

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.

>
>>
>> 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 <sjg@chromium.org>
>> ---
>>
>>  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(-)
>>
>> diff --git a/configs/zynq_microzed_defconfig b/configs/zynq_microzed_defconfig
>> index e9c3209..9d51540 100644
>> --- a/configs/zynq_microzed_defconfig
>> +++ b/configs/zynq_microzed_defconfig
>> @@ -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
>> diff --git a/configs/zynq_picozed_defconfig b/configs/zynq_picozed_defconfig
>> index f2b71e9..3a42efb 100644
>> --- a/configs/zynq_picozed_defconfig
>> +++ b/configs/zynq_picozed_defconfig
>> @@ -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
>> diff --git a/configs/zynq_zc702_defconfig b/configs/zynq_zc702_defconfig
>> index 0abb7a8..6faf928 100644
>> --- a/configs/zynq_zc702_defconfig
>> +++ b/configs/zynq_zc702_defconfig
>> @@ -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
>> diff --git a/configs/zynq_zc706_defconfig b/configs/zynq_zc706_defconfig
>> index d67f507..d6559b5 100644
>> --- a/configs/zynq_zc706_defconfig
>> +++ b/configs/zynq_zc706_defconfig
>> @@ -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
>> diff --git a/configs/zynq_zc70x_defconfig b/configs/zynq_zc70x_defconfig
>> index 37c249f..49c987a 100644
>> --- a/configs/zynq_zc70x_defconfig
>> +++ b/configs/zynq_zc70x_defconfig
>> @@ -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
>> diff --git a/configs/zynq_zc770_xm010_defconfig b/configs/zynq_zc770_xm010_defconfig
>> index 0e826bb..4bfb9cc 100644
>> --- a/configs/zynq_zc770_xm010_defconfig
>> +++ b/configs/zynq_zc770_xm010_defconfig
>> @@ -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
>> diff --git a/configs/zynq_zc770_xm011_defconfig b/configs/zynq_zc770_xm011_defconfig
>> index 46d043b..2a61fe3 100644
>> --- a/configs/zynq_zc770_xm011_defconfig
>> +++ b/configs/zynq_zc770_xm011_defconfig
>> @@ -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
>> diff --git a/configs/zynq_zc770_xm012_defconfig b/configs/zynq_zc770_xm012_defconfig
>> index 34d479f..eb98a39 100644
>> --- a/configs/zynq_zc770_xm012_defconfig
>> +++ b/configs/zynq_zc770_xm012_defconfig
>> @@ -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
>> diff --git a/configs/zynq_zc770_xm013_defconfig b/configs/zynq_zc770_xm013_defconfig
>> index c59599f..8d65c05 100644
>> --- a/configs/zynq_zc770_xm013_defconfig
>> +++ b/configs/zynq_zc770_xm013_defconfig
>> @@ -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
>> diff --git a/configs/zynq_zed_defconfig b/configs/zynq_zed_defconfig
>> index 886b4a5..13bef36 100644
>> --- a/configs/zynq_zed_defconfig
>> +++ b/configs/zynq_zed_defconfig
>> @@ -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
>> diff --git a/configs/zynq_zybo_defconfig b/configs/zynq_zybo_defconfig
>> index 77b9409..fc251dc 100644
>> --- a/configs/zynq_zybo_defconfig
>> +++ b/configs/zynq_zybo_defconfig
>> @@ -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
>> diff --git a/include/configs/zynq-common.h b/include/configs/zynq-common.h
>> index e7ab50a..aa4785f 100644
>> --- a/include/configs/zynq-common.h
>> +++ b/include/configs/zynq-common.h
>> @@ -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
>>
>>  /* 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?

Regards,
Simon

[U-Boot] [PATCH 07/13] arm: zynq: Use separate device tree instead of embedded

[Michal Simek]

On 08/31/2015 03:54 PM, Simon Glass wrote:
> Hi Michal,
> 
> On 31 August 2015 at 05:24, Michal Simek <monstr@monstr.eu> wrote:
>> On 08/29/2015 05:10 PM, Simon Glass wrote:
>>> 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.
> 
> 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.

...

>>> diff --git a/include/configs/zynq-common.h b/include/configs/zynq-common.h
>>> index e7ab50a..aa4785f 100644
>>> --- a/include/configs/zynq-common.h
>>> +++ b/include/configs/zynq-common.h
>>> @@ -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
>>>
>>>  /* 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.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150831/3886acfe/attachment.sig>

[U-Boot] [PATCH 07/13] arm: zynq: Use separate device tree instead of embedded

[Simon Glass]

Hi Michal,

On 31 August 2015 at 08:07, Michal Simek <monstr@monstr.eu> wrote:
> On 08/31/2015 03:54 PM, Simon Glass wrote:
>> Hi Michal,
>>
>> On 31 August 2015 at 05:24, Michal Simek <monstr@monstr.eu> wrote:
>>> On 08/29/2015 05:10 PM, Simon Glass wrote:
>>>> 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.
>>
>> 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.

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

>
> ...
>
>>>> diff --git a/include/configs/zynq-common.h b/include/configs/zynq-common.h
>>>> index e7ab50a..aa4785f 100644
>>>> --- a/include/configs/zynq-common.h
>>>> +++ b/include/configs/zynq-common.h
>>>> @@ -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
>>>>
>>>>  /* 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.

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

Regards,
Simon

[U-Boot] [PATCH 07/13] arm: zynq: Use separate device tree instead of embedded

[Michal Simek]

Hi Simon,

On 09/01/2015 01:12 AM, Simon Glass wrote:
> Hi Michal,
> 
> On 31 August 2015 at 08:07, Michal Simek <monstr@monstr.eu> wrote:
>> On 08/31/2015 03:54 PM, Simon Glass wrote:
>>> Hi Michal,
>>>
>>> On 31 August 2015 at 05:24, Michal Simek <monstr@monstr.eu> wrote:
>>>> On 08/29/2015 05:10 PM, Simon Glass wrote:
>>>>> 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.
>>>
>>> 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.


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.

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

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.


>>
>> ...
>>
>>>>> diff --git a/include/configs/zynq-common.h b/include/configs/zynq-common.h
>>>>> index e7ab50a..aa4785f 100644
>>>>> --- a/include/configs/zynq-common.h
>>>>> +++ b/include/configs/zynq-common.h
>>>>> @@ -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
>>>>>
>>>>>  /* 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.
> 
> 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.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150901/27565187/attachment.sig>

[U-Boot] [PATCH 07/13] arm: zynq: Use separate device tree instead of embedded

[Simon Glass]

Hi Michal,

On 1 September 2015 at 07:12, Michal Simek <monstr@monstr.eu> wrote:
> Hi Simon,
>
> On 09/01/2015 01:12 AM, Simon Glass wrote:
>> Hi Michal,
>>
>> On 31 August 2015 at 08:07, Michal Simek <monstr@monstr.eu> wrote:
>>> On 08/31/2015 03:54 PM, Simon Glass wrote:
>>>> Hi Michal,
>>>>
>>>> On 31 August 2015 at 05:24, Michal Simek <monstr@monstr.eu> wrote:
>>>>> On 08/29/2015 05:10 PM, Simon Glass wrote:
>>>>>> 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.
>>>>
>>>> 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.
>
>
> 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.

Mainline boards should follow best practice, since downstream boards
(and potentially other mainline boards) always copy from them.

It seems like this is painful for Zynq....

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

I see. Well adding u-boot-dtb.elf seems reasonable to me although I'm
not sure how you would do it in practice. It seems a bit tricky to
tack a binary file onto an ELF. With the efi-x86 target it does this
by putting a u-boot-dtb.bin payload inside an ELF image. I'm not sure
if that would work for you.

>
>
>>>
>>> ...
>>>
>>>>>> diff --git a/include/configs/zynq-common.h b/include/configs/zynq-common.h
>>>>>> index e7ab50a..aa4785f 100644
>>>>>> --- a/include/configs/zynq-common.h
>>>>>> +++ b/include/configs/zynq-common.h
>>>>>> @@ -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
>>>>>>
>>>>>>  /* 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.
>>
>> 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.

OK so really we should just change it to u-boot-dtb.img.

Regards,
Simon

[U-Boot] [PATCH 08/13] arm: zynq: Drop unnecessary code in SPL board_init_f()

[Simon Glass]

Move to the new way of starting up SPL. Clearing of BSS and calling
board_init_r() is now handled by crt0.S.

Also tidy up the header include order.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 arch/arm/mach-zynq/spl.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/arch/arm/mach-zynq/spl.c b/arch/arm/mach-zynq/spl.c
index 7bdac3b..1805455 100644
--- a/arch/arm/mach-zynq/spl.c
+++ b/arch/arm/mach-zynq/spl.c
@@ -7,8 +7,8 @@
 #include <spl.h>
 
 #include <asm/io.h>
-#include <asm/arch/hardware.h>
 #include <asm/spl.h>
+#include <asm/arch/hardware.h>
 #include <asm/arch/sys_proto.h>
 
 DECLARE_GLOBAL_DATA_PTR;
@@ -17,11 +17,7 @@ void board_init_f(ulong dummy)
 {
 	ps7_init();
 
-	/* Clear the BSS. */
-	memset(__bss_start, 0, __bss_end - __bss_start);
-
 	arch_cpu_init();
-	board_init_r(NULL, 0);
 }
 
 #ifdef CONFIG_SPL_BOARD_INIT
-- 
2.5.0.457.gab17608

[U-Boot] [PATCH 09/13] arm: zynq: Support the debug UART

[Simon Glass]

Add support for the debug UART to assist with early debugging. Enable it
for Zybo as an example.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 arch/arm/mach-zynq/spl.c     |  6 ++++
 configs/zynq_zybo_defconfig  |  4 +++
 drivers/serial/Kconfig       |  7 ++++
 drivers/serial/serial_zynq.c | 77 +++++++++++++++++++++++++++++++++++---------
 4 files changed, 78 insertions(+), 16 deletions(-)

diff --git a/arch/arm/mach-zynq/spl.c b/arch/arm/mach-zynq/spl.c
index 1805455..723019d 100644
--- a/arch/arm/mach-zynq/spl.c
+++ b/arch/arm/mach-zynq/spl.c
@@ -4,6 +4,7 @@
  * SPDX-License-Identifier:	GPL-2.0+
  */
 #include <common.h>
+#include <debug_uart.h>
 #include <spl.h>
 
 #include <asm/io.h>
@@ -18,6 +19,11 @@ void board_init_f(ulong dummy)
 	ps7_init();
 
 	arch_cpu_init();
+	/*
+	 * The debug UART can be used from this point:
+	 * debug_uart_init();
+	 * printch('x');
+	 */
 }
 
 #ifdef CONFIG_SPL_BOARD_INIT
diff --git a/configs/zynq_zybo_defconfig b/configs/zynq_zybo_defconfig
index fc251dc..b7531d6 100644
--- a/configs/zynq_zybo_defconfig
+++ b/configs/zynq_zybo_defconfig
@@ -11,3 +11,7 @@ CONFIG_FIT_SIGNATURE=y
 # CONFIG_CMD_SETEXPR is not set
 CONFIG_OF_SEPARATE=y
 CONFIG_NET_RANDOM_ETHADDR=y
+CONFIG_DEBUG_UART=y
+CONFIG_DEBUG_UART_ZYNQ=y
+CONFIG_DEBUG_UART_BASE=0xe0001000
+CONFIG_DEBUG_UART_CLOCK=50000000
diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
index 4f6a3b8..2d4c96a 100644
--- a/drivers/serial/Kconfig
+++ b/drivers/serial/Kconfig
@@ -77,6 +77,13 @@ config DEBUG_UART_S5P
 	  will need to provide parameters to make this work. The driver will
 	  be available until the real driver-model serial is running.
 
+config DEBUG_UART_ZYNQ
+	bool "Xilinx Zynq"
+	help
+	  Select this to enable a debug UART using the serial_s5p driver. You
+	  will need to provide parameters to make this work. The driver will
+	  be available until the real driver-model serial is running.
+
 endchoice
 
 config DEBUG_UART_BASE
diff --git a/drivers/serial/serial_zynq.c b/drivers/serial/serial_zynq.c
index 9d84290..c4304dd 100644
--- a/drivers/serial/serial_zynq.c
+++ b/drivers/serial/serial_zynq.c
@@ -6,6 +6,7 @@
  */
 
 #include <common.h>
+#include <errno.h>
 #include <fdtdec.h>
 #include <watchdog.h>
 #include <asm/io.h>
@@ -43,20 +44,16 @@ static struct uart_zynq *uart_zynq_ports[2] = {
 };
 
 /* Set up the baud rate in gd struct */
-static void uart_zynq_serial_setbrg(const int port)
+static void _uart_zynq_serial_setbrg(struct uart_zynq *regs,
+				     unsigned long clock, unsigned long baud)
 {
 	/* Calculation results. */
 	unsigned int calc_bauderror, bdiv, bgen;
 	unsigned long calc_baud = 0;
-	unsigned long baud;
-	unsigned long clock = get_uart_clk(port);
-	struct uart_zynq *regs = uart_zynq_ports[port];
 
 	/* Covering case where input clock is so slow */
-	if (clock < 1000000 && gd->baudrate > 4800)
-		gd->baudrate = 4800;
-
-	baud = gd->baudrate;
+	if (clock < 1000000 && baud > 4800)
+		baud = 4800;
 
 	/*                master clock
 	 * Baud rate = ------------------
@@ -87,36 +84,59 @@ static void uart_zynq_serial_setbrg(const int port)
 	writel(bgen, &regs->baud_rate_gen);
 }
 
-/* Initialize the UART, with...some settings. */
-static int uart_zynq_serial_init(const int port)
+/* 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];
 
-	if (!regs)
-		return -1;
+	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)
+{
 	/* RX/TX enabled & reset */
 	writel(ZYNQ_UART_CR_TX_EN | ZYNQ_UART_CR_RX_EN | ZYNQ_UART_CR_TXRST | \
 					ZYNQ_UART_CR_RXRST, &regs->control);
 	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)
+		return -EAGAIN;
+
+	writel(c, &regs->tx_rx_fifo);
+
+	return 0;
+}
+
 static void uart_zynq_serial_putc(const char c, const int port)
 {
 	struct uart_zynq *regs = uart_zynq_ports[port];
 
-	while ((readl(&regs->channel_sts) & ZYNQ_UART_SR_TXFULL) != 0)
+	while (_uart_zynq_serial_putc(regs, c) == -EAGAIN)
 		WATCHDOG_RESET();
 
 	if (c == '\n') {
-		writel('\r', &regs->tx_rx_fifo);
-		while ((readl(&regs->channel_sts) & ZYNQ_UART_SR_TXFULL) != 0)
+		while (_uart_zynq_serial_putc(regs, '\r') == -EAGAIN)
 			WATCHDOG_RESET();
 	}
-	writel(c, &regs->tx_rx_fifo);
 }
 
 static void uart_zynq_serial_puts(const char *s, const int port)
@@ -218,3 +238,28 @@ void zynq_serial_initialize(void)
 	serial_register(&uart_zynq_serial0_device);
 	serial_register(&uart_zynq_serial1_device);
 }
+
+#ifdef CONFIG_DEBUG_UART_ZYNQ
+
+#include <debug_uart.h>
+
+void debug_uart_init(void)
+{
+	struct uart_zynq *regs = (struct uart_zynq *)CONFIG_DEBUG_UART_BASE;
+
+	_uart_zynq_serial_init(regs);
+	_uart_zynq_serial_setbrg(regs, CONFIG_DEBUG_UART_CLOCK,
+				 CONFIG_BAUDRATE);
+}
+
+static inline void _debug_uart_putc(int ch)
+{
+	struct uart_zynq *regs = (struct uart_zynq *)CONFIG_DEBUG_UART_BASE;
+
+	while (_uart_zynq_serial_putc(regs, ch) == -EAGAIN)
+		WATCHDOG_RESET();
+}
+
+DEBUG_UART_FUNCS
+
+#endif
-- 
2.5.0.457.gab17608

[U-Boot] [PATCH 10/13] dm: arm: zynq: Enable device tree control in SPL

[Simon Glass]

Move to using device tree control in SPL so that we can use the same driver
code in both SPL and U-Boot proper.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 arch/arm/Kconfig                  |  3 +++
 arch/arm/mach-zynq/u-boot-spl.lds | 10 +++++++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index a99ae28..a82306e 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -683,9 +683,12 @@ config ARCH_ZYNQ
 	select CPU_V7
 	select SUPPORT_SPL
 	select OF_CONTROL
+	select SPL_OF_CONTROL
 	select DM
+	select SPL_DM
 	select DM_SPI
 	select DM_SPI_FLASH
+	select SPL_SEPARATE_BSS
 
 config ARCH_ZYNQMP
 	bool "Support Xilinx ZynqMP Platform"
diff --git a/arch/arm/mach-zynq/u-boot-spl.lds b/arch/arm/mach-zynq/u-boot-spl.lds
index 0f2f756..ecdf6a0 100644
--- a/arch/arm/mach-zynq/u-boot-spl.lds
+++ b/arch/arm/mach-zynq/u-boot-spl.lds
@@ -38,10 +38,18 @@ SECTIONS
 	} > .sram
 
 	. = ALIGN(4);
+#ifdef CONFIG_SPL_DM
+	.u_boot_list : {
+		KEEP(*(SORT(.u_boot_list_*_driver_*)));
+		KEEP(*(SORT(.u_boot_list_*_uclass_*)));
+	} > .sram
+
+	. = ALIGN(4);
+#endif
 
 	. = .;
 
-	__image_copy_end = .;
+	_image_binary_end = .;
 
 	_end = .;
 
-- 
2.5.0.457.gab17608

[U-Boot] [PATCH 11/13] arm: zynq: dts: Add U-Boot device tree additions

[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 <sjg@chromium.org>
---

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

diff --git a/arch/arm/dts/zynq-7000.dtsi b/arch/arm/dts/zynq-7000.dtsi
index 0b62cb0..12614f2 100644
--- a/arch/arm/dts/zynq-7000.dtsi
+++ b/arch/arm/dts/zynq-7000.dtsi
@@ -54,6 +54,7 @@
 	};
 
 	amba: amba {
+		u-boot,dm-pre-reloc;
 		compatible = "simple-bus";
 		#address-cells = <1>;
 		#size-cells = <1>;
diff --git a/arch/arm/dts/zynq-microzed.dts b/arch/arm/dts/zynq-microzed.dts
index c373a2c..5dff18e6 100644
--- a/arch/arm/dts/zynq-microzed.dts
+++ b/arch/arm/dts/zynq-microzed.dts
@@ -21,3 +21,8 @@
 		reg = <0 0x40000000>;
 	};
 };
+
+&uart1 {
+	u-boot,dm-pre-reloc;
+	status = "okay";
+};
diff --git a/arch/arm/dts/zynq-picozed.dts b/arch/arm/dts/zynq-picozed.dts
index 686b98f..3408df8 100644
--- a/arch/arm/dts/zynq-picozed.dts
+++ b/arch/arm/dts/zynq-picozed.dts
@@ -21,3 +21,8 @@
 		reg = <0 0x40000000>;
 	};
 };
+
+&uart1 {
+	u-boot,dm-pre-reloc;
+	status = "okay";
+};
diff --git a/arch/arm/dts/zynq-zc702.dts b/arch/arm/dts/zynq-zc702.dts
index 6691a8d..4fcef4b 100644
--- a/arch/arm/dts/zynq-zc702.dts
+++ b/arch/arm/dts/zynq-zc702.dts
@@ -375,6 +375,7 @@
 };
 
 &uart1 {
+	u-boot,dm-pre-reloc;
 	status = "okay";
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_uart1_default>;
diff --git a/arch/arm/dts/zynq-zc706.dts b/arch/arm/dts/zynq-zc706.dts
index cf7bce4..8198917 100644
--- a/arch/arm/dts/zynq-zc706.dts
+++ b/arch/arm/dts/zynq-zc706.dts
@@ -296,6 +296,7 @@
 };
 
 &uart1 {
+	u-boot,dm-pre-reloc;
 	status = "okay";
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_uart1_default>;
diff --git a/arch/arm/dts/zynq-zc770-xm010.dts b/arch/arm/dts/zynq-zc770-xm010.dts
index 680f24c..e2473d5 100644
--- a/arch/arm/dts/zynq-zc770-xm010.dts
+++ b/arch/arm/dts/zynq-zc770-xm010.dts
@@ -83,6 +83,7 @@
 };
 
 &uart1 {
+	u-boot,dm-pre-reloc;
 	status = "okay";
 };
 
diff --git a/arch/arm/dts/zynq-zc770-xm011.dts b/arch/arm/dts/zynq-zc770-xm011.dts
index f73c0dd..77e3bb0 100644
--- a/arch/arm/dts/zynq-zc770-xm011.dts
+++ b/arch/arm/dts/zynq-zc770-xm011.dts
@@ -55,6 +55,7 @@
 };
 
 &uart1 {
+	u-boot,dm-pre-reloc;
 	status = "okay";
 };
 
diff --git a/arch/arm/dts/zynq-zc770-xm012.dts b/arch/arm/dts/zynq-zc770-xm012.dts
index 4289e31..3e1769a 100644
--- a/arch/arm/dts/zynq-zc770-xm012.dts
+++ b/arch/arm/dts/zynq-zc770-xm012.dts
@@ -62,5 +62,6 @@
 };
 
 &uart1 {
+	u-boot,dm-pre-reloc;
 	status = "okay";
 };
diff --git a/arch/arm/dts/zynq-zc770-xm013.dts b/arch/arm/dts/zynq-zc770-xm013.dts
index 5124cdc..288e248 100644
--- a/arch/arm/dts/zynq-zc770-xm013.dts
+++ b/arch/arm/dts/zynq-zc770-xm013.dts
@@ -75,5 +75,6 @@
 };
 
 &uart0 {
+	u-boot,dm-pre-reloc;
 	status = "okay";
 };
diff --git a/arch/arm/dts/zynq-zed.dts b/arch/arm/dts/zynq-zed.dts
index 5762576..81c64a6 100644
--- a/arch/arm/dts/zynq-zed.dts
+++ b/arch/arm/dts/zynq-zed.dts
@@ -53,6 +53,7 @@
 };
 
 &uart1 {
+	u-boot,dm-pre-reloc;
 	status = "okay";
 };
 
diff --git a/arch/arm/dts/zynq-zybo.dts b/arch/arm/dts/zynq-zybo.dts
index 10f7815..dcfc00e 100644
--- a/arch/arm/dts/zynq-zybo.dts
+++ b/arch/arm/dts/zynq-zybo.dts
@@ -49,5 +49,6 @@
 };
 
 &uart1 {
+	u-boot,dm-pre-reloc;
 	status = "okay";
 };
-- 
2.5.0.457.gab17608

[U-Boot] [PATCH 11/13] arm: zynq: dts: Add U-Boot device tree additions

[Masahiro Yamada]

Simon,


2015-08-30 0:10 GMT+09:00 Simon Glass <sjg@chromium.org>:
> 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 <sjg@chromium.org>
> ---
>
>  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(+)
>
> diff --git a/arch/arm/dts/zynq-7000.dtsi b/arch/arm/dts/zynq-7000.dtsi
> index 0b62cb0..12614f2 100644
> --- a/arch/arm/dts/zynq-7000.dtsi
> +++ b/arch/arm/dts/zynq-7000.dtsi
> @@ -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?




-- 
Best Regards
Masahiro Yamada

[U-Boot] [PATCH 11/13] arm: zynq: dts: Add U-Boot device tree additions

[Simon Glass]

Hi Masahiro,

On 31 August 2015 at 04:01, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> Simon,
>
>
> 2015-08-30 0:10 GMT+09:00 Simon Glass <sjg@chromium.org>:
>> 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 <sjg@chromium.org>
>> ---
>>
>>  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(+)
>>
>> diff --git a/arch/arm/dts/zynq-7000.dtsi b/arch/arm/dts/zynq-7000.dtsi
>> index 0b62cb0..12614f2 100644
>> --- a/arch/arm/dts/zynq-7000.dtsi
>> +++ b/arch/arm/dts/zynq-7000.dtsi
>> @@ -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.

That should not happen unless you put the property in each child.

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

Yes, that is wrong. I remember noticing this at the time but not
having it as a parameter so I wasn't sure how to best regenerate it.

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

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.

Regards,
Simon

[U-Boot] [PATCH 11/13] arm: zynq: dts: Add U-Boot device tree additions

[Masahiro Yamada]

Simon,


2015-08-31 22:54 GMT+09:00 Simon Glass <sjg@chromium.org>:
>>
>>
>> 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.

Sounds good to me.
Are you willing to do this?



For now, I am planing to increase CONFIG_SYS_MALLOC_F_LEN
to work around the "All or Nothing" logic of the simple-bus binding.

http://patchwork.ozlabs.org/patch/512847/




-- 
Best Regards
Masahiro Yamada

[U-Boot] [PATCH 11/13] arm: zynq: dts: Add U-Boot device tree additions

[Michal Simek]

On 08/29/2015 05:10 PM, Simon Glass wrote:
> 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 <sjg@chromium.org>
> ---
> 
>  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(+)
> 
> diff --git a/arch/arm/dts/zynq-7000.dtsi b/arch/arm/dts/zynq-7000.dtsi
> index 0b62cb0..12614f2 100644
> --- a/arch/arm/dts/zynq-7000.dtsi
> +++ b/arch/arm/dts/zynq-7000.dtsi
> @@ -54,6 +54,7 @@
>  	};
>  
>  	amba: amba {
> +		u-boot,dm-pre-reloc;
>  		compatible = "simple-bus";
>  		#address-cells = <1>;
>  		#size-cells = <1>;
> diff --git a/arch/arm/dts/zynq-microzed.dts b/arch/arm/dts/zynq-microzed.dts
> index c373a2c..5dff18e6 100644
> --- a/arch/arm/dts/zynq-microzed.dts
> +++ b/arch/arm/dts/zynq-microzed.dts
> @@ -21,3 +21,8 @@
>  		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?

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150831/fce90296/attachment.sig>

[U-Boot] [PATCH 11/13] arm: zynq: dts: Add U-Boot device tree additions

[Simon Glass]

Hi Michal,

On 31 August 2015 at 05:30, Michal Simek <monstr@monstr.eu> wrote:
> On 08/29/2015 05:10 PM, Simon Glass wrote:
>> 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 <sjg@chromium.org>
>> ---
>>
>>  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(+)
>>
>> diff --git a/arch/arm/dts/zynq-7000.dtsi b/arch/arm/dts/zynq-7000.dtsi
>> index 0b62cb0..12614f2 100644
>> --- a/arch/arm/dts/zynq-7000.dtsi
>> +++ b/arch/arm/dts/zynq-7000.dtsi
>> @@ -54,6 +54,7 @@
>>       };
>>
>>       amba: amba {
>> +             u-boot,dm-pre-reloc;
>>               compatible = "simple-bus";
>>               #address-cells = <1>;
>>               #size-cells = <1>;
>> diff --git a/arch/arm/dts/zynq-microzed.dts b/arch/arm/dts/zynq-microzed.dts
>> index c373a2c..5dff18e6 100644
>> --- a/arch/arm/dts/zynq-microzed.dts
>> +++ b/arch/arm/dts/zynq-microzed.dts
>> @@ -21,3 +21,8 @@
>>               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.

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

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

Regards,
Simon

[U-Boot] [PATCH 11/13] arm: zynq: dts: Add U-Boot device tree additions

[Michal Simek]

On 08/31/2015 03:54 PM, Simon Glass wrote:
> Hi Michal,
> 
> On 31 August 2015 at 05:30, Michal Simek <monstr@monstr.eu> wrote:
>> On 08/29/2015 05:10 PM, Simon Glass wrote:
>>> 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 <sjg@chromium.org>
>>> ---
>>>
>>>  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(+)
>>>
>>> diff --git a/arch/arm/dts/zynq-7000.dtsi b/arch/arm/dts/zynq-7000.dtsi
>>> index 0b62cb0..12614f2 100644
>>> --- a/arch/arm/dts/zynq-7000.dtsi
>>> +++ b/arch/arm/dts/zynq-7000.dtsi
>>> @@ -54,6 +54,7 @@
>>>       };
>>>
>>>       amba: amba {
>>> +             u-boot,dm-pre-reloc;
>>>               compatible = "simple-bus";
>>>               #address-cells = <1>;
>>>               #size-cells = <1>;
>>> diff --git a/arch/arm/dts/zynq-microzed.dts b/arch/arm/dts/zynq-microzed.dts
>>> index c373a2c..5dff18e6 100644
>>> --- a/arch/arm/dts/zynq-microzed.dts
>>> +++ b/arch/arm/dts/zynq-microzed.dts
>>> @@ -21,3 +21,8 @@
>>>               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.
> 
>>
>> 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. :-)

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


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

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150831/9d04c0af/attachment.sig>

[U-Boot] [PATCH 11/13] arm: zynq: dts: Add U-Boot device tree additions

[Simon Glass]

Hi Michal,

On 31 August 2015 at 08:16, Michal Simek <monstr@monstr.eu> wrote:
> On 08/31/2015 03:54 PM, Simon Glass wrote:
>> Hi Michal,
>>
>> On 31 August 2015 at 05:30, Michal Simek <monstr@monstr.eu> wrote:
>>> On 08/29/2015 05:10 PM, Simon Glass wrote:
>>>> 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 <sjg@chromium.org>
>>>> ---
>>>>
>>>>  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(+)
>>>>
>>>> diff --git a/arch/arm/dts/zynq-7000.dtsi b/arch/arm/dts/zynq-7000.dtsi
>>>> index 0b62cb0..12614f2 100644
>>>> --- a/arch/arm/dts/zynq-7000.dtsi
>>>> +++ b/arch/arm/dts/zynq-7000.dtsi
>>>> @@ -54,6 +54,7 @@
>>>>       };
>>>>
>>>>       amba: amba {
>>>> +             u-boot,dm-pre-reloc;
>>>>               compatible = "simple-bus";
>>>>               #address-cells = <1>;
>>>>               #size-cells = <1>;
>>>> diff --git a/arch/arm/dts/zynq-microzed.dts b/arch/arm/dts/zynq-microzed.dts
>>>> index c373a2c..5dff18e6 100644
>>>> --- a/arch/arm/dts/zynq-microzed.dts
>>>> +++ b/arch/arm/dts/zynq-microzed.dts
>>>> @@ -21,3 +21,8 @@
>>>>               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.
>>
>>>
>>> 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. :-)
>
>>
>>> 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.

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?

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

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.

Regards,
Simon

[U-Boot] [PATCH 11/13] arm: zynq: dts: Add U-Boot device tree additions

[Michal Simek]

On 09/01/2015 01:13 AM, Simon Glass wrote:
> Hi Michal,
> 
> On 31 August 2015 at 08:16, Michal Simek <monstr@monstr.eu> wrote:
>> On 08/31/2015 03:54 PM, Simon Glass wrote:
>>> Hi Michal,
>>>
>>> On 31 August 2015 at 05:30, Michal Simek <monstr@monstr.eu> wrote:
>>>> On 08/29/2015 05:10 PM, Simon Glass wrote:
>>>>> 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 <sjg@chromium.org>
>>>>> ---
>>>>>
>>>>>  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(+)
>>>>>
>>>>> diff --git a/arch/arm/dts/zynq-7000.dtsi b/arch/arm/dts/zynq-7000.dtsi
>>>>> index 0b62cb0..12614f2 100644
>>>>> --- a/arch/arm/dts/zynq-7000.dtsi
>>>>> +++ b/arch/arm/dts/zynq-7000.dtsi
>>>>> @@ -54,6 +54,7 @@
>>>>>       };
>>>>>
>>>>>       amba: amba {
>>>>> +             u-boot,dm-pre-reloc;
>>>>>               compatible = "simple-bus";
>>>>>               #address-cells = <1>;
>>>>>               #size-cells = <1>;
>>>>> diff --git a/arch/arm/dts/zynq-microzed.dts b/arch/arm/dts/zynq-microzed.dts
>>>>> index c373a2c..5dff18e6 100644
>>>>> --- a/arch/arm/dts/zynq-microzed.dts
>>>>> +++ b/arch/arm/dts/zynq-microzed.dts
>>>>> @@ -21,3 +21,8 @@
>>>>>               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.
>>>
>>>>
>>>> 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. :-)
>>
>>>
>>>> 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.
> 
> In this case wouldn't it need to list the simple-bus also?

yes for zc702 case

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


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

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.

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

Not now. But you can use qemu platform. I will look at it to see what
there is. But I have confirmation that you should be able to run u-boot
there.

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150901/7fe48cab/attachment.sig>

[U-Boot] [PATCH 11/13] arm: zynq: dts: Add U-Boot device tree additions

[Masahiro Yamada]

Hi.


2015-09-02 0:41 GMT+09:00 Michal Simek <monstr@monstr.eu>:

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





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




-- 
Best Regards
Masahiro Yamada

[U-Boot] [PATCH 11/13] arm: zynq: dts: Add U-Boot device tree additions

[Simon Glass]

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

Hi,

On 1 September 2015 at 10:19, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> Hi.
>
>
> 2015-09-02 0:41 GMT+09:00 Michal Simek <monstr@monstr.eu>:
>
>>>>
>>>>>
>>>>>> 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.
>>>
>>> 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;
>
>
>
>
>
>>
>>>
>>> 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?
>>
>> 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.

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?

If we go this way, who is going to write the patch??

Regards,
Simon

[U-Boot] [PATCH 11/13] arm: zynq: dts: Add U-Boot device tree additions

[Michal Simek]

On 09/02/2015 04:49 AM, Simon Glass wrote:
> +Tom and a few others who may have an opinion.
> 
> Hi,
> 
> On 1 September 2015 at 10:19, Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
>> Hi.
>>
>>
>> 2015-09-02 0:41 GMT+09:00 Michal Simek <monstr@monstr.eu>:
>>
>>>>>
>>>>>>
>>>>>>> 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.
>>>>
>>>> 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;
>>
>>
>>
>>
>>
>>>
>>>>
>>>> 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?
>>>
>>> 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.
> 
> 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

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150903/f390e91e/attachment.sig>

[U-Boot] [PATCH 11/13] arm: zynq: dts: Add U-Boot device tree additions

[Simon Glass]

Hi Michal,

On 3 September 2015 at 05:35, Michal Simek <monstr@monstr.eu> wrote:
> On 09/02/2015 04:49 AM, Simon Glass wrote:
>> +Tom and a few others who may have an opinion.
>>
>> Hi,
>>
>> On 1 September 2015 at 10:19, Masahiro Yamada
>> <yamada.masahiro@socionext.com> wrote:
>>> Hi.
>>>
>>>
>>> 2015-09-02 0:41 GMT+09:00 Michal Simek <monstr@monstr.eu>:
>>>
>>>>>>
>>>>>>>
>>>>>>>> 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.
>>>>>
>>>>> 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;
>>>
>>>
>>>
>>>
>>>
>>>>
>>>>>
>>>>> 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?
>>>>
>>>> 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.
>>
>> 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?

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?

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

Regards,
Simon

[U-Boot] [PATCH 11/13] arm: zynq: dts: Add U-Boot device tree additions

[Michal Simek]

On 09/04/2015 02:22 AM, Simon Glass wrote:
> Hi Michal,
> 
> On 3 September 2015 at 05:35, Michal Simek <monstr@monstr.eu> wrote:
>> On 09/02/2015 04:49 AM, Simon Glass wrote:
>>> +Tom and a few others who may have an opinion.
>>>
>>> Hi,
>>>
>>> On 1 September 2015 at 10:19, Masahiro Yamada
>>> <yamada.masahiro@socionext.com> wrote:
>>>> Hi.
>>>>
>>>>
>>>> 2015-09-02 0:41 GMT+09:00 Michal Simek <monstr@monstr.eu>:
>>>>
>>>>>>>
>>>>>>>>
>>>>>>>>> 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.
>>>>>>
>>>>>> 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;
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>>
>>>>>>
>>>>>> 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?
>>>>>
>>>>> 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.
>>>
>>> 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?
> 
> 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?

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.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150904/29548541/attachment.sig>

[U-Boot] [PATCH 11/13] arm: zynq: dts: Add U-Boot device tree additions

[Simon Glass]

Hi Michal,

On 4 September 2015 at 00:04, Michal Simek <monstr@monstr.eu> wrote:
> On 09/04/2015 02:22 AM, Simon Glass wrote:
>> Hi Michal,
>>
>> On 3 September 2015 at 05:35, Michal Simek <monstr@monstr.eu> wrote:
>>> On 09/02/2015 04:49 AM, Simon Glass wrote:
>>>> +Tom and a few others who may have an opinion.
>>>>
>>>> Hi,
>>>>
>>>> On 1 September 2015 at 10:19, Masahiro Yamada
>>>> <yamada.masahiro@socionext.com> wrote:
>>>>> Hi.
>>>>>
>>>>>
>>>>> 2015-09-02 0:41 GMT+09:00 Michal Simek <monstr@monstr.eu>:
>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>>> 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.
>>>>>>>
>>>>>>> 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;
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>>>
>>>>>>> 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?
>>>>>>
>>>>>> 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.
>>>>
>>>> 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?
>>
>> 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?
>
> 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.

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?

Regards,
Simon

[U-Boot] [PATCH 11/13] arm: zynq: dts: Add U-Boot device tree additions

[Michal Simek]

Hi Simon,

<cut some previous parts>

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

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

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

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150904/68e9112f/attachment.sig>

[U-Boot] [PATCH 11/13] arm: zynq: dts: Add U-Boot device tree additions

[Simon Glass]

Hi Michal,

On Friday, 4 September 2015, Michal Simek <monstr@monstr.eu> wrote:
>
> Hi Simon,
>
> <cut some previous parts>
>
> >>>>>>>>
> >>>>>>>> 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?
> >>>>>>>
> >>>>>>> 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.
> >>>>>
> >>>>> 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?
> >>>
> >>> 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?
> >>
> >> 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.
> >
> > 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.


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

>
> 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.
>
>
> > 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?
>
> (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?
>
>
> 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.
>
>
> 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.

Regards,
Simon

[U-Boot] [PATCH 11/13] arm: zynq: dts: Add U-Boot device tree additions

[Michal Simek]

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 <sjg@chromium.org>:

> Hi Michal,
>
> On Friday, 4 September 2015, Michal Simek <monstr@monstr.eu> wrote:
> >
> > Hi Simon,
> >
> > <cut some previous parts>
> >
> > >>>>>>>>
> > >>>>>>>> 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?
> > >>>>>>>
> > >>>>>>> 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.
> > >>>>>
> > >>>>> 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?
> > >>>
> > >>> 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?
> > >>
> > >> 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.
> > >
> > > 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.
>
>
> 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.



>
> >
> > 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.
> >
> >
> > > 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?
> >
> > (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?
>

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.




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

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.

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform

[U-Boot] [PATCH 11/13] arm: zynq: dts: Add U-Boot device tree additions

[Tom Rini]

On Sat, Sep 19, 2015 at 03:07:54AM +0200, Michal Simek wrote:
> 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 <sjg@chromium.org>:
[snip]
> > 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.

I think that with ELC-E coming up and that once again there will be a DT
BoF, we (the U-Boot community) need to show up there and in turn see
how many folks we can also get to show up in our mini-summit.  I've
already bugged Pantelis about this a few times.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150919/85fef963/attachment.sig>

[U-Boot] [PATCH 11/13] arm: zynq: dts: Add U-Boot device tree additions

[Simon Glass]

Hi Michal,

On 18 September 2015 at 19:07, Michal Simek <monstr@monstr.eu> wrote:
> Hi Simon,
>
> first of all sorry for late reply. I am traveling and I have pretty busy
> time now.

That's OK, I don't think there is any particular hurry.

>
> 2015-09-09 20:07 GMT+02:00 Simon Glass <sjg@chromium.org>:
>>
>> Hi Michal,
>>
>> On Friday, 4 September 2015, Michal Simek <monstr@monstr.eu> wrote:
>> >
>> > Hi Simon,
>> >
>> > <cut some previous parts>
>> >
>> > >>>>>>>>
>> > >>>>>>>> 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?
>> > >>>>>>>
>> > >>>>>>> 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.
>> > >>>>>
>> > >>>>> 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?
>> > >>>
>> > >>> 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?
>> > >>
>> > >> 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.
>> > >
>> > > 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.
>>
>>
>> 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.

Normally the .dts file will specify the UART which means that it will
often need rewriting with this scheme.

The potential for stuff-ups and confusion is there. With the current
scheme at least marking can be kept as a property of the device (where
arguably it belongs) rather than having to be recreated for each
board.

Perhaps we could allow multiple properties in /chosen and scan them
all (u-boot,dm-pre-reloc-soc, u-boot,dm-pre-reloc-pinctrl, etc.)?

>
>
>>
>>
>> >
>> > 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.
>> >
>> >
>> > > 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?
>> >
>> > (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?
>
>
> 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.

OK, by all means. But please cc the U-Boot mailing list.

I'm not tied to the current solution at all. It does work and
something like it is necessary. It allows SOC people to set things up
so that it is relatively easy for people to write now .dts files.

But I'd like to change it once someone has figured out the
implications of the change:

- Impact on maintainability of .dts/.dtsi files
- Mechanism for implementation of device tree subset feature
(currently fdtgrep / SPL)
- Code changes required in U-Boot
- Maintaining the efficiency (the algorithm above would require many
scans of the DT)
- A patch :-)

I feel in general that there is some conflict between hard-nosed
efficiency and perceived beauty in device tree bindings. This doesn't
seem to matter much in Linux, but in U-Boot it is more important. I'd
like to err on the side of keeping things simple and fast, bringing in
new functionality as needed (perhaps behind an option) rather than
requiring a lump of 10KB of parsing code just to get get a serial
driver running.

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

Well it does follow device tree guidelines - it has a' u-boot,' prefix
just like all the properties which have a 'linux,' prefix. So I think
it is sane from that point of view. It's just that it's a pain to add
it to several nodes, and not obvious that another approach (a list of
nodes somewhere) is better.

Yes I'd really like the 'DT guys' to get involved in this sort of
thing, including contributing patches to U-Boot in this area. It takes
a lot of energy to go through these discussions and having something
on the inside track who can figure these things out and help build out
the infrastructure would be a big help.

As Tom suggests we could try to figure this out at ELCE? I can go to
the BoF session if there is time to discuss it. So far the topics seem
to be quite 'high-end' :-) But we could talk about it at the U-Boot
one.

Regards,
Simon

[U-Boot] [PATCH 11/13] arm: zynq: dts: Add U-Boot device tree additions

[Michal Simek]

On 08/29/2015 05:10 PM, Simon Glass wrote:
> 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 <sjg@chromium.org>
> ---
> 
>  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(+)
> 
> diff --git a/arch/arm/dts/zynq-7000.dtsi b/arch/arm/dts/zynq-7000.dtsi
> index 0b62cb0..12614f2 100644
> --- a/arch/arm/dts/zynq-7000.dtsi
> +++ b/arch/arm/dts/zynq-7000.dtsi
> @@ -54,6 +54,7 @@
>  	};
>  
>  	amba: amba {
> +		u-boot,dm-pre-reloc;

One more point here IRC the first line in every node should be
compatible string not any property that's why this should be move below.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150901/cf9fadcb/attachment.sig>

[U-Boot] [PATCH 12/13] arm: zynq: serial: Drop non-device-tree serial driver portions

[Simon Glass]

Since we use device tree in SPL also, we can drop this code.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/serial/serial_zynq.c | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/drivers/serial/serial_zynq.c b/drivers/serial/serial_zynq.c
index c4304dd..c39c1a9 100644
--- a/drivers/serial/serial_zynq.c
+++ b/drivers/serial/serial_zynq.c
@@ -195,7 +195,6 @@ DECLARE_PSSERIAL_FUNCTIONS(1);
 static struct serial_device uart_zynq_serial1_device =
 	INIT_PSSERIAL_STRUCTURE(1, "ttyPS1");
 
-#if CONFIG_IS_ENABLED(OF_CONTROL)
 __weak struct serial_device *default_serial_console(void)
 {
 	const void *blob = gd->fdt_blob;
@@ -218,20 +217,6 @@ __weak struct serial_device *default_serial_console(void)
 
 	return NULL;
 }
-#else
-__weak struct serial_device *default_serial_console(void)
-{
-#if defined(CONFIG_ZYNQ_SERIAL_UART0)
-	if (uart_zynq_ports[0])
-		return &uart_zynq_serial0_device;
-#endif
-#if defined(CONFIG_ZYNQ_SERIAL_UART1)
-	if (uart_zynq_ports[1])
-		return &uart_zynq_serial1_device;
-#endif
-	return NULL;
-}
-#endif
 
 void zynq_serial_initialize(void)
 {
-- 
2.5.0.457.gab17608

[U-Boot] [PATCH 13/13] arm: zynq: Move serial driver to driver model

[Simon Glass]

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

Signed-off-by: Simon Glass <sjg@chromium.org>
---

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

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index a82306e..7705e03 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -687,6 +687,7 @@ config ARCH_ZYNQ
 	select DM
 	select SPL_DM
 	select DM_SPI
+	select DM_SERIAL
 	select DM_SPI_FLASH
 	select SPL_SEPARATE_BSS
 
diff --git a/drivers/serial/serial_zynq.c b/drivers/serial/serial_zynq.c
index c39c1a9..7069afe 100644
--- a/drivers/serial/serial_zynq.c
+++ b/drivers/serial/serial_zynq.c
@@ -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);
-}
+static const struct dm_serial_ops zynq_serial_ops = {
+	.putc = zynq_serial_putc,
+	.pending = zynq_serial_pending,
+	.getc = zynq_serial_getc,
+	.setbrg = zynq_serial_setbrg,
+};
+
+static const struct udevice_id zynq_serial_ids[] = {
+	{ .compatible = "xlnx,xuartps" },
+	{ }
+};
+
+U_BOOT_DRIVER(serial_s5p) = {
+	.name	= "serial_zynq",
+	.id	= UCLASS_SERIAL,
+	.of_match = zynq_serial_ids,
+	.ofdata_to_platdata = zynq_serial_ofdata_to_platdata,
+	.priv_auto_alloc_size = sizeof(struct zynq_uart_priv),
+	.probe = zynq_serial_probe,
+	.ops	= &zynq_serial_ops,
+	.flags = DM_FLAG_PRE_RELOC,
+};
 
 #ifdef CONFIG_DEBUG_UART_ZYNQ
 
diff --git a/include/configs/zynq_microzed.h b/include/configs/zynq_microzed.h
index 549a664..b5ffafb 100644
--- a/include/configs/zynq_microzed.h
+++ b/include/configs/zynq_microzed.h
@@ -12,7 +12,6 @@
 
 #define CONFIG_SYS_SDRAM_SIZE		(1024 * 1024 * 1024)
 
-#define CONFIG_ZYNQ_SERIAL_UART1
 #define CONFIG_ZYNQ_GEM0
 #define CONFIG_ZYNQ_GEM_PHY_ADDR0	0
 
diff --git a/include/configs/zynq_picozed.h b/include/configs/zynq_picozed.h
index d116e05..ffc73bd 100644
--- a/include/configs/zynq_picozed.h
+++ b/include/configs/zynq_picozed.h
@@ -12,7 +12,6 @@
 
 #define CONFIG_SYS_SDRAM_SIZE		(1024 * 1024 * 1024)
 
-#define CONFIG_ZYNQ_SERIAL_UART1
 #define CONFIG_ZYNQ_GEM0
 #define CONFIG_ZYNQ_GEM_PHY_ADDR0	0
 
diff --git a/include/configs/zynq_zc70x.h b/include/configs/zynq_zc70x.h
index b659054..468a6bc 100644
--- a/include/configs/zynq_zc70x.h
+++ b/include/configs/zynq_zc70x.h
@@ -12,7 +12,6 @@
 
 #define CONFIG_SYS_SDRAM_SIZE		(1024 * 1024 * 1024)
 
-#define CONFIG_ZYNQ_SERIAL_UART1
 #define CONFIG_ZYNQ_GEM0
 #define CONFIG_ZYNQ_GEM_PHY_ADDR0	7
 
diff --git a/include/configs/zynq_zc770.h b/include/configs/zynq_zc770.h
index 7a1b872..63224dd 100644
--- a/include/configs/zynq_zc770.h
+++ b/include/configs/zynq_zc770.h
@@ -15,26 +15,20 @@
 #define CONFIG_SYS_NO_FLASH
 
 #if defined(CONFIG_ZC770_XM010)
-# define CONFIG_ZYNQ_SERIAL_UART1
 # define CONFIG_ZYNQ_GEM0
 # define CONFIG_ZYNQ_GEM_PHY_ADDR0	7
 # define CONFIG_ZYNQ_SDHCI0
 # define CONFIG_ZYNQ_SPI
 
 #elif defined(CONFIG_ZC770_XM011)
-# define CONFIG_ZYNQ_SERIAL_UART1
 
 #elif defined(CONFIG_ZC770_XM012)
-# define CONFIG_ZYNQ_SERIAL_UART1
 # undef CONFIG_SYS_NO_FLASH
 
 #elif defined(CONFIG_ZC770_XM013)
-# define CONFIG_ZYNQ_SERIAL_UART0
 # define CONFIG_ZYNQ_GEM1
 # define CONFIG_ZYNQ_GEM_PHY_ADDR1	7
 
-#else
-# define CONFIG_ZYNQ_SERIAL_UART0
 #endif
 
 #include <configs/zynq-common.h>
diff --git a/include/configs/zynq_zed.h b/include/configs/zynq_zed.h
index 946de95..6ec6117 100644
--- a/include/configs/zynq_zed.h
+++ b/include/configs/zynq_zed.h
@@ -12,7 +12,6 @@
 
 #define CONFIG_SYS_SDRAM_SIZE		(512 * 1024 * 1024)
 
-#define CONFIG_ZYNQ_SERIAL_UART1
 #define CONFIG_ZYNQ_GEM0
 #define CONFIG_ZYNQ_GEM_PHY_ADDR0	0
 
diff --git a/include/configs/zynq_zybo.h b/include/configs/zynq_zybo.h
index 191f2a5..e2270cd 100644
--- a/include/configs/zynq_zybo.h
+++ b/include/configs/zynq_zybo.h
@@ -13,7 +13,6 @@
 
 #define CONFIG_SYS_SDRAM_SIZE (512 * 1024 * 1024)
 
-#define CONFIG_ZYNQ_SERIAL_UART1
 #define CONFIG_ZYNQ_GEM0
 #define CONFIG_ZYNQ_GEM_PHY_ADDR0	0
 
-- 
2.5.0.457.gab17608

[U-Boot] [PATCH 13/13] arm: zynq: Move serial driver to driver model

[Michal Simek]

On 08/29/2015 05:10 PM, Simon Glass wrote:
> Update this driver to use driver model and change all users.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  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.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150831/e3f53253/attachment.sig>

[U-Boot] [PATCH 13/13] arm: zynq: Move serial driver to driver model

[Simon Glass]

Hi Michal,

On 31 August 2015 at 05:33, Michal Simek <monstr@monstr.eu> wrote:
> On 08/29/2015 05:10 PM, Simon Glass wrote:
>> Update this driver to use driver model and change all users.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>>  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.

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

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

Regards,
Simon

[U-Boot] [PATCH 13/13] arm: zynq: Move serial driver to driver model

[Michal Simek]

On 08/31/2015 03:54 PM, Simon Glass wrote:
> Hi Michal,
> 
> On 31 August 2015 at 05:33, Michal Simek <monstr@monstr.eu> wrote:
>> On 08/29/2015 05:10 PM, Simon Glass wrote:
>>> Update this driver to use driver model and change all users.
>>>
>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>> ---
>>>
>>>  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.
> 
> See my note in the cover letter. ZynqMP does not build for me and I
> cannot see any device tree files in U-Boot.

I missed that. Look my reply to your cover letter.

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

ok. I expect you have tested this on the real hw. I will allocate some
time for it this week to do some more testing on that.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150831/a79e190d/attachment.sig>

[U-Boot] [PATCH 13/13] arm: zynq: Move serial driver to driver model

[Michal Simek]

On 08/29/2015 05:10 PM, Simon Glass wrote:
> Update this driver to use driver model and change all users.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  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(-)
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index a82306e..7705e03 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -687,6 +687,7 @@ config ARCH_ZYNQ
>  	select DM
>  	select SPL_DM
>  	select DM_SPI
> +	select DM_SERIAL

Can you please keep DM_SPI and DM_SPI_FLASH together?

>  	select DM_SPI_FLASH
>  	select SPL_SEPARATE_BSS
>  
> diff --git a/drivers/serial/serial_zynq.c b/drivers/serial/serial_zynq.c
> index c39c1a9..7069afe 100644
> --- a/drivers/serial/serial_zynq.c
> +++ b/drivers/serial/serial_zynq.c
> @@ -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);
> -}
> +static const struct dm_serial_ops zynq_serial_ops = {
> +	.putc = zynq_serial_putc,
> +	.pending = zynq_serial_pending,
> +	.getc = zynq_serial_getc,
> +	.setbrg = zynq_serial_setbrg,
> +};
> +
> +static const struct udevice_id zynq_serial_ids[] = {
> +	{ .compatible = "xlnx,xuartps" },

I would prefer to use
{ .compatible = "cdns,uart-r1p8" },
instead.
Or both.

The reason is that xlnx,xuartps compatible string is deprecated.
We switch to cadence compatible string.


> +	{ }
> +};
> +
> +U_BOOT_DRIVER(serial_s5p) = {

serial_zynq here

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150901/81fd229e/attachment.sig>

[U-Boot] [PATCH 00/13] dm: arm: zynq: Convert serial driver to driver model

[Michal Simek]

On 08/29/2015 05:10 PM, Simon Glass wrote:
> 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!

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

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150831/1f2756fb/attachment.sig>

[U-Boot] [PATCH 00/13] dm: arm: zynq: Convert serial driver to driver model

[Simon Glass]

Hi Michal,

On 31 August 2015 at 08:11, Michal Simek <monstr@monstr.eu> wrote:
> On 08/29/2015 05:10 PM, Simon Glass wrote:
>> 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!
>
> 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
+      ((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

Regards,
Simon

[U-Boot] [PATCH 00/13] dm: arm: zynq: Convert serial driver to driver model

[Michal Simek]

On 09/01/2015 01:12 AM, Simon Glass wrote:
> Hi Michal,
> 
> On 31 August 2015 at 08:11, Michal Simek <monstr@monstr.eu> wrote:
>> On 08/29/2015 05:10 PM, Simon Glass wrote:
>>> 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!
>>
>> 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
> +      ((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

gem and usb warnings but as you see it is building. Gem warnings were
reported to Tom and that DWC3 were also reported to Marek. We will work
on fixing them.

I will look at qemu and will send you manual how you can run it.

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150901/2df6bc1d/attachment.sig>

[U-Boot] [PATCH 00/13] dm: arm: zynq: Convert serial driver to driver model

[Michal Simek]

On 09/01/2015 01:12 AM, Simon Glass wrote:
> Hi Michal,
> 
> On 31 August 2015 at 08:11, Michal Simek <monstr@monstr.eu> wrote:
>> On 08/29/2015 05:10 PM, Simon Glass wrote:
>>> 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!
>>
>> 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
> +      ((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
> 

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.

Thanks,
Michal




-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150901/b81b1e30/attachment.sig>

[U-Boot] [PATCH 00/13] dm: arm: zynq: Convert serial driver to driver model

[Michal Simek]

On 09/01/2015 07:41 PM, Michal Simek wrote:
> On 09/01/2015 01:12 AM, Simon Glass wrote:
>> Hi Michal,
>>
>> On 31 August 2015 at 08:11, Michal Simek <monstr@monstr.eu> wrote:
>>> On 08/29/2015 05:10 PM, Simon Glass wrote:
>>>> 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!
>>>
>>> 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
>> +      ((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
>>
> 
> 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.

One more thing I forget is that for that get up and running I had to
cherry-pick your reverted patch
"fdt: Fix fdtdec_get_addr_size() for 64-bit"
(sha1: 6fd4307bd55ba0e216770bdf362d2ce5f37a2311) because zynqmp is using
address-cells 2 and size cells 1 which is not covered in the current HEAD.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150901/c961ba05/attachment.sig>

[U-Boot] [PATCH 00/13] dm: arm: zynq: Convert serial driver to driver model

[Simon Glass]

Hi Michael,

On 1 September 2015 at 11:50, Michal Simek <monstr@monstr.eu> wrote:
> On 09/01/2015 07:41 PM, Michal Simek wrote:
>> On 09/01/2015 01:12 AM, Simon Glass wrote:
>>> Hi Michal,
>>>
>>> On 31 August 2015 at 08:11, Michal Simek <monstr@monstr.eu> wrote:
>>>> On 08/29/2015 05:10 PM, Simon Glass wrote:
>>>>> 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!
>>>>
>>>> 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
>>> +      ((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
>>>
>>
>> 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.
>
> One more thing I forget is that for that get up and running I had to
> cherry-pick your reverted patch
> "fdt: Fix fdtdec_get_addr_size() for 64-bit"
> (sha1: 6fd4307bd55ba0e216770bdf362d2ce5f37a2311) because zynqmp is using
> address-cells 2 and size cells 1 which is not covered in the current HEAD.

OK thanks for the info.

Do you have patches to move zynqmp to device tree? That would be a
prerequisite for any patches I send.

Regards,
Simon

[U-Boot] [PATCH 00/13] dm: arm: zynq: Convert serial driver to driver model

[Michal Simek]

On 09/02/2015 04:05 PM, Simon Glass wrote:
> Hi Michael,
> 
> On 1 September 2015 at 11:50, Michal Simek <monstr@monstr.eu> wrote:
>> On 09/01/2015 07:41 PM, Michal Simek wrote:
>>> On 09/01/2015 01:12 AM, Simon Glass wrote:
>>>> Hi Michal,
>>>>
>>>> On 31 August 2015 at 08:11, Michal Simek <monstr@monstr.eu> wrote:
>>>>> On 08/29/2015 05:10 PM, Simon Glass wrote:
>>>>>> 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!
>>>>>
>>>>> 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
>>>> +      ((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
>>>>
>>>
>>> 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.
>>
>> One more thing I forget is that for that get up and running I had to
>> cherry-pick your reverted patch
>> "fdt: Fix fdtdec_get_addr_size() for 64-bit"
>> (sha1: 6fd4307bd55ba0e216770bdf362d2ce5f37a2311) because zynqmp is using
>> address-cells 2 and size cells 1 which is not covered in the current HEAD.
> 
> OK thanks for the info.
> 
> Do you have patches to move zynqmp to device tree? That would be a
> prerequisite for any patches I send.

Did you see my previous email I sent? There is link to the repo with
that patches.

Thanks,
Michal



-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150904/5a4fb133/attachment.sig>

[U-Boot] [PATCH 00/13] dm: arm: zynq: Convert serial driver to driver model

[Simon Glass]

Hi Michal,

On 3 September 2015 at 23:53, Michal Simek <monstr@monstr.eu> wrote:
>
> On 09/02/2015 04:05 PM, Simon Glass wrote:
> > Hi Michael,
> >
> > On 1 September 2015 at 11:50, Michal Simek <monstr@monstr.eu> wrote:
> >> On 09/01/2015 07:41 PM, Michal Simek wrote:
> >>> On 09/01/2015 01:12 AM, Simon Glass wrote:
> >>>> Hi Michal,
> >>>>
> >>>> On 31 August 2015 at 08:11, Michal Simek <monstr@monstr.eu> wrote:
> >>>>> On 08/29/2015 05:10 PM, Simon Glass wrote:
> >>>>>> 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!
> >>>>>
> >>>>> 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
> >>>> +      ((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
> >>>>
> >>>
> >>> 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.
> >>
> >> One more thing I forget is that for that get up and running I had to
> >> cherry-pick your reverted patch
> >> "fdt: Fix fdtdec_get_addr_size() for 64-bit"
> >> (sha1: 6fd4307bd55ba0e216770bdf362d2ce5f37a2311) because zynqmp is using
> >> address-cells 2 and size cells 1 which is not covered in the current HEAD.
> >
> > OK thanks for the info.
> >
> > Do you have patches to move zynqmp to device tree? That would be a
> > prerequisite for any patches I send.
>
> Did you see my previous email I sent? There is link to the repo with
> that patches.

Yes I see. That works for me, thanks.

I've pushed an updated tree to u-boot-dm/zynq-working. Please take a
look and see what you think. I'll send out patches once I hear from
you.

Unfortunately the revert you need is not the way I want to go. Instead
I'd like to apply this patch:

http://patchwork.ozlabs.org/patch/504918/

It breaks on zynqmp, so if you can figure out why that would be very
helpful. It seems to work OK on other platforms I have tested.

Regards,
Simon