All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 1/1] uboot: zynqmp: Support loading a PMU config
@ 2020-06-25 19:29 Brandon Maier
  2020-06-25 20:54 ` Luca Ceresoli
  0 siblings, 1 reply; 3+ messages in thread
From: Brandon Maier @ 2020-06-25 19:29 UTC (permalink / raw)
  To: buildroot

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(+)

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

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [Buildroot] [PATCH 1/1] uboot: zynqmp: Support loading a PMU config
  2020-06-25 19:29 [Buildroot] [PATCH 1/1] uboot: zynqmp: Support loading a PMU config Brandon Maier
@ 2020-06-25 20:54 ` Luca Ceresoli
  2020-06-26 15:37   ` Brandon Maier
  0 siblings, 1 reply; 3+ messages in thread
From: Luca Ceresoli @ 2020-06-25 20:54 UTC (permalink / raw)
  To: buildroot

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [Buildroot] [PATCH 1/1] uboot: zynqmp: Support loading a PMU config
  2020-06-25 20:54 ` Luca Ceresoli
@ 2020-06-26 15:37   ` Brandon Maier
  0 siblings, 0 replies; 3+ messages in thread
From: Brandon Maier @ 2020-06-26 15:37 UTC (permalink / raw)
  To: buildroot

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-06-26 15:37 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-25 19:29 [Buildroot] [PATCH 1/1] uboot: zynqmp: Support loading a PMU config Brandon Maier
2020-06-25 20:54 ` Luca Ceresoli
2020-06-26 15:37   ` Brandon Maier

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.