Re: [Buildroot] [PATCH v2 1/1] boot/uboot/uboot.mk: add pmufw.elf support

[Thomas Petazzoni via buildroot <buildroot@buildroot.org>]

Hello,

On Wed, 12 Oct 2022 11:54:26 -0600
Neal Frager via buildroot <buildroot@buildroot.org> wrote:

> This patch adds support for zynqmp pmufw.elf files.
> It will allow buildroot to use pmufw.elf binaries directly
> from the Xilinx git repository built by petalinux.
> 
> https://github.com/Xilinx/ubuntu-firmware/tree/v2022.1_22.04_1/xlnx-firmware
> 
> It will solve the problem of the pmufw built by the
> zynqmp-pmufw-builder where soft resets crash.  Details of the issue
> can be found here:
> 
> https://lore.kernel.org/buildroot/87ilqccu3k.fsf@dell.be.48ers.dk/
> 
> Instead of using a pmufw binary built by the zynqmp-pmufw-builder, this patch
> enables a pmufw binary built by petalinux to solve the issue.
> 
> Signed-off-by: Neal Frager <neal.frager@amd.com>

I think the whole commit log is a bit confusing. What this commit
allows is to use a PMU FW as an ELF binary in addition to allowing to
use a PMU FW provided as a raw binary.

And indeed, this will allow using the PMU FW provided by Petalinux in
addition to the ones of zynqmp-pmufw-builder, which is the motivation
for the change.

> +UBOOT_ZYNQMP_PMUFW_BASENAME=$(basename $(UBOOT_ZYNQMP_PMUFW_PATH))
> +UBOOT_ZYNQMP_PMUFW_SUFFIX=$(suffix $(UBOOT_ZYNQMP_PMUFW_PATH))

Spaces around = sign.

> +
> +ifeq ($(UBOOT_ZYNQMP_PMUFW_SUFFIX),.elf)
> +UBOOT_ZYNQMP_PMUFW_CONVERT=y

Ditto.

> +endif
> +
>  define UBOOT_ZYNQMP_KCONFIG_PMUFW
> -	$(call KCONFIG_SET_OPT,CONFIG_PMUFW_INIT_FILE,"$(UBOOT_ZYNQMP_PMUFW_PATH)")
> +	$(if $(UBOOT_ZYNQMP_PMUFW_CONVERT),
> +		objcopy -O binary -I elf32-little $(UBOOT_ZYNQMP_PMUFW_BASENAME).elf $(UBOOT_ZYNQMP_PMUFW_BASENAME).bin)

You could also write this like this:

	$(if $(filter %.elf,$(UBOOT_ZYNQMP_PMUFW_PATH)),
		objcopy -O binary -I elf32-little $(UBOOT_ZYNQMP_PMUFW_BASENAME).elf $(UBOOT_ZYNQMP_PMUFW_BASENAME).bin)

and get rid of UBOOT_ZYNQMP_PMUFW_CONVERT entirely.

I was wondering if the usage of objcopy was correct or whether
$(TARGET_OBJCOPY) would be more correct. But in fact none of them is
perfect I guess: the PMU FW is a Microblaze ELF binary, while
$(TARGET_OBJCOPY) will be an ARM64 objcopy, and plain objcopy will be
the machine native objcopy, very unlikely to be a Microblaze one :-)

So overall, I guess using objcopy is fine here.

Best regards,

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot