[U-Boot] [PATCH] arm64: zynqmp: fail SPL build if no psu_init found

[U-Boot] [PATCH] arm64: zynqmp: fail SPL build if no psu_init found

[Jean-Francois Dagenais]

When doing the SPL build, not finding the psu_init files yields an
unuseable spl binary. Time is wasted as the build is carried through dev
ops all the way to trying to boot the DUD build.

A failure immediately alerts developers that something is wrong in the
configuration of the u-boot tree, thus allowing quicker recovery.

Adding an extra test in this section made the logic a bit too clunky so
the layout of the tests was refactored for the reader's benefit.

Signed-off-by: Jean-Francois Dagenais <jeff.dagenais@gmail.com>
---
 board/xilinx/zynqmp/Makefile | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/board/xilinx/zynqmp/Makefile b/board/xilinx/zynqmp/Makefile
index 2bf0375..78cb550 100644
--- a/board/xilinx/zynqmp/Makefile
+++ b/board/xilinx/zynqmp/Makefile
@@ -7,22 +7,38 @@
 
 obj-y	:= zynqmp.o
 
-hw-platform-y :=$(shell echo $(CONFIG_DEFAULT_DEVICE_TREE))
+ifeq ($(CONFIG_SPL_BUILD), y)
 
+# use the devicetree name as a hint for the psu_init file dir name
+hw-platform-y :=$(shell echo $(CONFIG_DEFAULT_DEVICE_TREE))
 init-objs := $(if $(wildcard $(srctree)/$(src)/$(hw-platform-y)/psu_init_gpl.c),\
 	$(hw-platform-y)/psu_init_gpl.o)
 
+# if couldn't find psu_init files where they *should*, be...
 ifeq ($(init-objs),)
-ifneq ($(wildcard $(srctree)/$(src)/psu_init_gpl.c),)
-init-objs := psu_init_gpl.o
-$(if $(CONFIG_SPL_BUILD),\
-$(warning Put custom psu_init_gpl.c/h to board/xilinx/zynqmp/custom_hw_platform/))
+# then fallback to this current dir, if file exists
+init-objs := $(if $(wildcard $(srctree)/$(src)/psu_init_gpl.c), psu_init_gpl.o)
 endif
+
+# if init-objs still empty, then abort
+ifeq ($(init-objs),)
+# define a \n equivalent:
+define n
+
+
+endef
+
+$(error Could not locate psu_init_gpl.c/h files, tried: $n\
+	$(srctree)/$(src)/$(hw-platform-y)/psu_init_gpl.c and $n\
+	$(srctree)/$(src)/psu_init_gpl.c)
 endif
 
-obj-$(CONFIG_ZYNQ_SDHCI) += tap_delays.o
 obj-$(CONFIG_SPL_BUILD) += $(init-objs)
 
+endif # if SPL_BUILD
+
+obj-$(CONFIG_ZYNQ_SDHCI) += tap_delays.o
+
 # Suppress "warning: function declaration isn't a prototype"
 CFLAGS_REMOVE_psu_init_gpl.o := -Wstrict-prototypes
 
-- 
2.1.4

[U-Boot] [PATCH] arm64: zynqmp: fail SPL build if no psu_init found

[Michal Simek]

Hi,

On 3.4.2017 02:51, Jean-Francois Dagenais wrote:
> When doing the SPL build, not finding the psu_init files yields an
> unuseable spl binary. Time is wasted as the build is carried through dev
> ops all the way to trying to boot the DUD build.
> 
> A failure immediately alerts developers that something is wrong in the
> configuration of the u-boot tree, thus allowing quicker recovery.
> 
> Adding an extra test in this section made the logic a bit too clunky so
> the layout of the tests was refactored for the reader's benefit.
> 
> Signed-off-by: Jean-Francois Dagenais <jeff.dagenais@gmail.com>
> ---
>  board/xilinx/zynqmp/Makefile | 28 ++++++++++++++++++++++------
>  1 file changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/board/xilinx/zynqmp/Makefile b/board/xilinx/zynqmp/Makefile
> index 2bf0375..78cb550 100644
> --- a/board/xilinx/zynqmp/Makefile
> +++ b/board/xilinx/zynqmp/Makefile
> @@ -7,22 +7,38 @@
>  
>  obj-y	:= zynqmp.o
>  
> -hw-platform-y :=$(shell echo $(CONFIG_DEFAULT_DEVICE_TREE))
> +ifeq ($(CONFIG_SPL_BUILD), y)
>  
> +# use the devicetree name as a hint for the psu_init file dir name
> +hw-platform-y :=$(shell echo $(CONFIG_DEFAULT_DEVICE_TREE))
>  init-objs := $(if $(wildcard $(srctree)/$(src)/$(hw-platform-y)/psu_init_gpl.c),\
>  	$(hw-platform-y)/psu_init_gpl.o)
>  
> +# if couldn't find psu_init files where they *should*, be...
>  ifeq ($(init-objs),)
> -ifneq ($(wildcard $(srctree)/$(src)/psu_init_gpl.c),)
> -init-objs := psu_init_gpl.o
> -$(if $(CONFIG_SPL_BUILD),\
> -$(warning Put custom psu_init_gpl.c/h to board/xilinx/zynqmp/custom_hw_platform/))
> +# then fallback to this current dir, if file exists
> +init-objs := $(if $(wildcard $(srctree)/$(src)/psu_init_gpl.c), psu_init_gpl.o)
>  endif
> +
> +# if init-objs still empty, then abort
> +ifeq ($(init-objs),)
> +# define a \n equivalent:
> +define n
> +
> +
> +endef
> +
> +$(error Could not locate psu_init_gpl.c/h files, tried: $n\
> +	$(srctree)/$(src)/$(hw-platform-y)/psu_init_gpl.c and $n\
> +	$(srctree)/$(src)/psu_init_gpl.c)
>  endif
>  
> -obj-$(CONFIG_ZYNQ_SDHCI) += tap_delays.o
>  obj-$(CONFIG_SPL_BUILD) += $(init-objs)
>  
> +endif # if SPL_BUILD
> +
> +obj-$(CONFIG_ZYNQ_SDHCI) += tap_delays.o
> +
>  # Suppress "warning: function declaration isn't a prototype"
>  CFLAGS_REMOVE_psu_init_gpl.o := -Wstrict-prototypes
>  
> 

This is not only one way how to configure system. You can use
psu_init*.tcl and configure chip through jtag and then you don't need
this .c/.h files.
You should be able to see warning already about it if you miss that files.

Thanks,
Michal

[U-Boot] [PATCH] arm64: zynqmp: fail SPL build if no psu_init found

[Jean-Francois Dagenais]

> On Apr 3, 2017, at 02:55, Michal Simek <michal.simek@xilinx.com> wrote:
> 
> This is not only one way how to configure system. You can use
> psu_init*.tcl and configure chip through jtag and then you don't need
> this .c/.h files.
> You should be able to see warning already about it if you miss that files.

Mmh. I had thought of that, but it seemed like this scenario might have been a
minority issue. I guess this is a very yocto-centric problem. Warnings emitted
inside builds that bitbake spawns are only visible in a log file for each steps
(configure, compile, etc.) inside each recipe's work dir and so they are
completely invisible to the operator that invokes bitbake.

Perhaps I should come up with a strategy to emerge this particular warning in
the u-boot-xlnx_%.bb recipe?

Thoughts?

[U-Boot] [PATCH] arm64: zynqmp: fail SPL build if no psu_init found

[Michal Simek]

On 3.4.2017 13:38, Jean-Francois Dagenais wrote:
> 
>> On Apr 3, 2017, at 02:55, Michal Simek <michal.simek@xilinx.com> wrote:
>>
>> This is not only one way how to configure system. You can use
>> psu_init*.tcl and configure chip through jtag and then you don't need
>> this .c/.h files.
>> You should be able to see warning already about it if you miss that files.
> 
> Mmh. I had thought of that, but it seemed like this scenario might have been a
> minority issue. I guess this is a very yocto-centric problem. Warnings emitted
> inside builds that bitbake spawns are only visible in a log file for each steps
> (configure, compile, etc.) inside each recipe's work dir and so they are
> completely invisible to the operator that invokes bitbake.
> 
> Perhaps I should come up with a strategy to emerge this particular warning in
> the u-boot-xlnx_%.bb recipe?
> 
> Thoughts?

No idea but u-boot does it IMHO right.

M

[U-Boot] [PATCH] arm64: zynqmp: fail SPL build if no psu_init found

[Nathan Rossi]

On 4 April 2017 at 17:20, Michal Simek <michal.simek@xilinx.com> wrote:
> On 3.4.2017 13:38, Jean-Francois Dagenais wrote:
>>
>>> On Apr 3, 2017, at 02:55, Michal Simek <michal.simek@xilinx.com> wrote:
>>>
>>> This is not only one way how to configure system. You can use
>>> psu_init*.tcl and configure chip through jtag and then you don't need
>>> this .c/.h files.
>>> You should be able to see warning already about it if you miss that files.
>>
>> Mmh. I had thought of that, but it seemed like this scenario might have been a
>> minority issue. I guess this is a very yocto-centric problem. Warnings emitted
>> inside builds that bitbake spawns are only visible in a log file for each steps
>> (configure, compile, etc.) inside each recipe's work dir and so they are
>> completely invisible to the operator that invokes bitbake.
>>
>> Perhaps I should come up with a strategy to emerge this particular warning in
>> the u-boot-xlnx_%.bb recipe?
>>
>> Thoughts?

The meta-xilinx layer has some logic to cause an error when these
files are not available (currently only for zynq, this should work
fine for the zynqmp use case as well). Specifically if nothing is
providing the platform init files it will cause a dependency chain
error. However due to u-boot source providing the platform init for
some boards there is a list of u-boot configs that don't trigger the
error on purpose.

This could be extended to check that the symbols (the ps*_init ones)
are compiled and included in the output u-boot-spl elf as a sanity
check. Which would be more complete than just validating the files are
there.

Regards,
Nathan