Message ID | 20200625192925.182477-1-brandon.maier@rockwellcollins.com |
---|---|
State | Changes Requested |
Headers | show |
Series | [1/1] uboot: zynqmp: Support loading a PMU config | expand |
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@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 > >
On Thu, Jun 25, 2020 at 3:57 PM Luca Ceresoli <luca@lucaceresoli.net> wrote: > > + $(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. I did it this way because the "dts" command was here too, but I can move it to a pre-build hook. > > > $(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. > I agree it's vague, I was trying to remain consistent with UBOOT_ZYNQMP_PSU_INIT which does it this way. > > + > > +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
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)" + ) $(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)) + +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
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@rockwellcollins.com> --- boot/uboot/Config.in | 22 ++++++++++++++++++++++ boot/uboot/uboot.mk | 17 +++++++++++++++++ 2 files changed, 39 insertions(+)