[Buildroot] [PATCH 1/1] uboot: zynqmp: Support loading a PMU config
Luca Ceresoli
luca at lucaceresoli.net
Thu Jun 25 20:54:56 UTC 2020
Hi Brandon,
On 25/06/20 21:29, Brandon Maier wrote:
> Before now, U-Boot SPL could only load the Platform Management Unit
> (PMU) by patching the board-specific pm_cfg_obj.c file into the generic
> PMU firmware, but that then requires generating a new PMU firmware for
> every board configuration. To fix that, Luca Ceresoli added support to
> U-Boot to load the pm_cfg_obj[1].
>
> Like the PMU firmware, we need a way to pass the PMU cfg to U-Boot
> during build. U-Boot only accepts the binary format of the cfg, so we
> must convert the source file with the tool provided with U-Boot.
>
> [1] https://lucaceresoli.net/zynqmp-uboot-spl-pmufw-cfg-load/
>
> Signed-off-by: Brandon Maier <brandon.maier at rockwellcollins.com>
Thanks for your patch! It looks generally pretty good to me, but see
below for some remarks.
> ---
> boot/uboot/Config.in | 22 ++++++++++++++++++++++
> boot/uboot/uboot.mk | 17 +++++++++++++++++
> 2 files changed, 39 insertions(+)
>
> diff --git a/boot/uboot/Config.in b/boot/uboot/Config.in
> index 8cce9b1bae..5fa3b5942e 100644
> --- a/boot/uboot/Config.in
> +++ b/boot/uboot/Config.in
> @@ -458,6 +458,28 @@ config BR2_TARGET_UBOOT_ZYNQMP_PMUFW
>
> This feature requires U-Boot >= 2018.07.
>
> +config BR2_TARGET_UBOOT_ZYNQMP_PM_CFG
> + string "PMU configuration location"
> + depends on BR2_TARGET_UBOOT_BUILD_SYSTEM_KCONFIG
> + help
> + Location of a PMU configuration file.
> +
> + If not empty, Buildroot will convert the PMU configuration
> + file into a loadable blob and pass it to U-Boot. The blob gets
> + embedded into the U-Boot SPL and is used to configure the PMU
> + during board initialization.
> +
> + Unlike the PMU firmware, the PMU configuration file is unique
> + to each board configuration. A PMU configuration file can be
> + generated by building your Xilinx SDK BSP. It can be found in
> + the BSP source, for example at
> + > ./psu_cortexa53_0/libsrc/xilpm_v2_4/src/pm_cfg_obj.c
> +
> + Leave this option empty if your PMU firmware has a hard-coded
> + configuration object or you are loading it by any other means.
> +
> + This feature requires U-Boot >= v2019.10.
> +
> config BR2_TARGET_UBOOT_ZYNQMP_PSU_INIT_FILE
> string "Custom psu_init_gpl file"
> depends on BR2_TARGET_UBOOT_BUILD_SYSTEM_KCONFIG
> diff --git a/boot/uboot/uboot.mk b/boot/uboot/uboot.mk
> index 71689207e3..67d153189d 100644
> --- a/boot/uboot/uboot.mk
> +++ b/boot/uboot/uboot.mk
> @@ -293,6 +293,11 @@ define UBOOT_BUILD_CMDS
> $(if $(UBOOT_CUSTOM_DTS_PATH),
> cp -f $(UBOOT_CUSTOM_DTS_PATH) $(@D)/arch/$(UBOOT_ARCH)/dts/
> )
> + $(if $(UBOOT_ZYNQMP_PM_CFG_PATH),
> + $(UBOOT_DIR)/tools/zynqmp_pm_cfg_obj_convert.py \
> + "$(UBOOT_ZYNQMP_PM_CFG_PATH)" \
> + "$(UBOOT_ZYNQMP_PM_CFG_BIN)"
> + )
Even though this works, I find it a bit weird that you read the
UBOOT_ZYNQMP_PM_CFG_PATH variable here but you assign it below. I think
you could move the invocation of zynqmp_pm_cfg_obj_convert.py to a
pre-build hook (or a post-configure hook? hum), which in turn would
allow you to move the code inside the 'ifeq
($(BR2_TARGET_UBOOT_ZYNQMP),y)' stanza, keeping all the zynqmp-specific
code together.
> $(TARGET_CONFIGURE_OPTS) \
> $(BR2_MAKE) -C $(@D) $(UBOOT_MAKE_OPTS) \
> $(UBOOT_MAKE_TARGET)
> @@ -360,6 +365,17 @@ define UBOOT_ZYNQMP_KCONFIG_PMUFW
> $(call KCONFIG_SET_OPT,CONFIG_PMUFW_INIT_FILE,"$(UBOOT_ZYNQMP_PMUFW_PATH)")
> endef
>
> +UBOOT_ZYNQMP_PM_CFG = $(call qstrip,$(BR2_TARGET_UBOOT_ZYNQMP_PM_CFG))
> +UBOOT_ZYNQMP_PM_CFG_PATH = $(shell readlink -f $(UBOOT_ZYNQMP_PM_CFG))
I find the _PATH suffix kind of vague: all these variables contain
paths. I'd rather call it UBOOT_ZYNQMP_PM_CFG_C or
UBOOT_ZYNQMP_PM_CFG_SRC. But it's a matter of taste, not a big issue.
> +
> +ifneq ($(UBOOT_ZYNQMP_PM_CFG_PATH),)
> +UBOOT_ZYNQMP_PM_CFG_BIN = $(UBOOT_DIR)/pm_cfg_obj.bin
> +define UBOOT_ZYNQMP_KCONFIG_PM_CFG
> + $(call KCONFIG_SET_OPT,CONFIG_ZYNQMP_SPL_PM_CFG_OBJ_FILE,"$(UBOOT_ZYNQMP_PM_CFG_BIN)", \
> + $(@D)/.config)
> +endef
> +endif
> +
> UBOOT_ZYNQMP_PSU_INIT = $(call qstrip,$(BR2_TARGET_UBOOT_ZYNQMP_PSU_INIT_FILE))
> UBOOT_ZYNQMP_PSU_INIT_PATH = $(shell readlink -f $(UBOOT_ZYNQMP_PSU_INIT))
>
> @@ -422,6 +438,7 @@ endif
>
> define UBOOT_KCONFIG_FIXUP_CMDS
> $(UBOOT_ZYNQMP_KCONFIG_PMUFW)
> + $(UBOOT_ZYNQMP_KCONFIG_PM_CFG)
> $(UBOOT_ZYNQMP_KCONFIG_PSU_INIT)
> endef
>
>
--
Luca
More information about the buildroot
mailing list