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

[Frager, Neal]

Hi Thomas,

Thank you for the review.

> 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-fi
> rmware
> 
> 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 at 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.

I will simplify the commit log.  No problem.

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

> Spaces around = sign.

I will take out the unnecessary spaces.

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

Thank you for this!  I was trying to find a simpler solution, but when I could not think of one, I figured someone from the community would help.

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

I ultimately chose the host objcopy because you are right, the pmufw is not an ARM64 binary.
I think using $(TARGET_OBJCOPY) is a bit misleading.

> Best regards,

> Thomas
> --
> Thomas Petazzoni, co-owner and CEO, Bootlin Embedded Linux and Kernel engineering and training https://bootlin.com

Thank you for your support!

Best regards,
Neal Frager
AMD