[U-Boot] [PATCH 00/13] dm: arm: zynq: Convert serial driver to driver model
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(-)
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);
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);
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 at 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",
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!).
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");@@ -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
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
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
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
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
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;
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 */
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
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
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
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
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
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
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
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, ®s->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, ®s->control); writel(ZYNQ_UART_MR_PARITY_NONE, ®s->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(®s->channel_sts) & ZYNQ_UART_SR_TXFULL) + return -EAGAIN; + + writel(c, ®s->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(®s->channel_sts) & ZYNQ_UART_SR_TXFULL) != 0) + while (_uart_zynq_serial_putc(regs, c) == -EAGAIN) WATCHDOG_RESET();
if (c == '\n') { - writel('\r', ®s->tx_rx_fifo); - while ((readl(®s->channel_sts) & ZYNQ_UART_SR_TXFULL) != 0) + while (_uart_zynq_serial_putc(regs, '\r') == -EAGAIN) WATCHDOG_RESET(); } - writel(c, ®s->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
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 = .;
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"; };
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?
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
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/
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;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
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
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
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
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
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.
+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
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
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
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@XXX { }; uart1: uart@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
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@XXX { }; uart1: uart@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
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@XXX { }; uart1: uart@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
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@XXX { }; uart1: uart@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
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@XXX { }; uart1: uart@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
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.
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@XXX { }; uart1: uart@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
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
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) {
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, ®s->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, ®s->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(®s->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(®s->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(®s->channel_sts) & ZYNQ_UART_SR_RXEMPTY) + return -EAGAIN;
- while (!uart_zynq_serial_tstc(port)) - WATCHDOG_RESET(); return readl(®s->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(®s->channel_sts) & ZYNQ_UART_SR_RXEMPTY); + else + return !!(readl(®s->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
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
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
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
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, ®s->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, ®s->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(®s->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(®s->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(®s->channel_sts) & ZYNQ_UART_SR_RXEMPTY)
return -EAGAIN;
- while (!uart_zynq_serial_tstc(port))
WATCHDOG_RESET();}
-/* 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(®s->channel_sts) & ZYNQ_UART_SR_RXEMPTY);- else
return !!(readl(®s->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
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
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, ®s->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, ®s->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
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, ®s->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, ®s->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
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, ®s->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, ®s->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
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, ®s->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, ®s->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
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, ®s->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, ®s->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
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, ®s->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, ®s->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
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, ®s->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, ®s->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
participants (4)
-
Masahiro Yamada -
Michal Simek -
Simon Glass -
Tom Rini