| Message ID | 20220811122011.716045-1-bram.vlerick@openpixelsystems.org |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | package/freescale-imx/firmware-imx: allow lpddr4 firmware version override | expand |
Hello Blam, On Thu, 11 Aug 2022 14:20:11 +0200 Bram Vlerick <bram.vlerick@openpixelsystems.org> wrote: > Add support for selecting a specific version of the LPDDR4 firmware > version. Variscite's imx8mp-var-dart SOM requires the "_202006" version > of the lpddr4_pmu_train_* binaries. > > Signed-off-by: Bram Vlerick <bram.vlerick@openpixelsystems.org> Thanks a lot for your patch. Looks good overall. A few comments below, though. > package/freescale-imx/firmware-imx/Config.in | 10 ++++++++++ > package/freescale-imx/firmware-imx/firmware-imx.mk | 12 ++++++++++-- > 2 files changed, 20 insertions(+), 2 deletions(-) > > diff --git a/package/freescale-imx/firmware-imx/Config.in b/package/freescale-imx/firmware-imx/Config.in > index 06c4d8036c..f894d87250 100644 > --- a/package/freescale-imx/firmware-imx/Config.in > +++ b/package/freescale-imx/firmware-imx/Config.in > @@ -82,6 +82,16 @@ config BR2_PACKAGE_FIRMWARE_IMX_DDR3 > > endchoice # DDR training FW > > +if BR2_PACKAGE_FIRMWARE_IMX_LPDDR4 > + > +config BR2_PACKAGE_FIRMWARE_IMX_LPDDR4_VERSION > + string "LPDDR4 Version" > + help > + Use a specific version of the lpddr4_pmu_train_* binaries such > + as "202006". > + > +endif # BR2_PACKAGE_FIRMWARE_IMX_LPDDR4 Looking at the list of firmware files: ddr3_dmem_1d_201810.bin ddr3_dmem_1d.bin ddr3_imem_1d_201810.bin ddr3_imem_1d.bin ddr4_dmem_1d_201810.bin ddr4_dmem_1d_202006.bin ddr4_dmem_1d.bin ddr4_dmem_2d_201810.bin ddr4_dmem_2d_202006.bin ddr4_dmem_2d.bin ddr4_imem_1d_201810.bin ddr4_imem_1d_202006.bin ddr4_imem_1d.bin ddr4_imem_2d_201810.bin ddr4_imem_2d_202006.bin ddr4_imem_2d.bin lpddr4_pmu_train_1d_dmem_201904.bin lpddr4_pmu_train_1d_dmem_202006.bin lpddr4_pmu_train_1d_dmem.bin lpddr4_pmu_train_1d_imem_201904.bin lpddr4_pmu_train_1d_imem_202006.bin lpddr4_pmu_train_1d_imem.bin lpddr4_pmu_train_2d_dmem_201904.bin lpddr4_pmu_train_2d_dmem_202006.bin lpddr4_pmu_train_2d_dmem.bin lpddr4_pmu_train_2d_imem_201904.bin lpddr4_pmu_train_2d_imem_202006.bin lpddr4_pmu_train_2d_imem.bin it's not only LPDDR4 firmware files that can have versions, but also DDR3 and DDR4 ones. So can we instead have: config BR2_PACKAGE_FIRMWARE_IMX_DDR_VERSION string "DDR3/DDR4/LPDDR4 firmware version" help ... In the help text, perhaps state "leave empty for the default version", or something like that. > diff --git a/package/freescale-imx/firmware-imx/firmware-imx.mk b/package/freescale-imx/firmware-imx/firmware-imx.mk > index 47c21585aa..40aa84e162 100644 > --- a/package/freescale-imx/firmware-imx/firmware-imx.mk > +++ b/package/freescale-imx/firmware-imx/firmware-imx.mk > @@ -38,15 +38,23 @@ endef > > ifeq ($(BR2_PACKAGE_FIRMWARE_IMX_LPDDR4),y) > FIRMWARE_IMX_DDRFW_DIR = $(@D)/firmware/ddr/synopsys > +ifneq ($(BR2_PACKAGE_FIRMWARE_IMX_LPDDR4_VERSION), "") > + FIRMWARE_IMX_LPDDR4_FW_VERSION = _$(BR2_PACKAGE_FIRMWARE_IMX_LPDDR4_VERSION) Use qstrip, and don't indent variables, so: FIRMWARE_IMX_DDR_VERSION = $(call qstrip,$(BR2_PACKAGE_FIRMWARE_IMX_DDR_VERSION)) ifneq ($(FIRMWARE_IMX_DDR_VERSION),) FIRMWARE_IMX_DDR_VERSION_SUFFIX = _$(FIRMWARE_IMX_DDR_VERSION) endif and use $(FIRMWARE_IMX_DDR_VERSION_SUFFIX) Thanks a lot! Thomas
Hi Thomas, I'm currently adding the suggested changes. With regards to the suggestion below. The DDR3 and DDR4 already have a "hardcoded" version selected. What would be the prefered approach, adding a default value in the firmware-imx.mk for both the DDR3 and DDR4 cases. Or would it be better to update the individual defconfig and add the BR2_PACKAGE_FIRMWARE_IMX_DDR_VERSION variable to the configs? On Thu, Aug 11, 2022 at 10:41:44PM +0200, Thomas Petazzoni wrote: > it's not only LPDDR4 firmware files that can have versions, but also > DDR3 and DDR4 ones. > > So can we instead have: > > config BR2_PACKAGE_FIRMWARE_IMX_DDR_VERSION > string "DDR3/DDR4/LPDDR4 firmware version" > help > ... > > In the help text, perhaps state "leave empty for the default version", > or something like that. > Kind regards, Bram
Hello Bram, On Fri, 12 Aug 2022 10:51:24 +0200 Bram Vlerick <bram.vlerick@openpixelsystems.org> wrote: > I'm currently adding the suggested changes. With regards to the suggestion below. > The DDR3 and DDR4 already have a "hardcoded" version selected. What would be the > prefered approach, adding a default value in the firmware-imx.mk for both the DDR3 > and DDR4 cases. Or would it be better to update the individual defconfig and add > the BR2_PACKAGE_FIRMWARE_IMX_DDR_VERSION variable to the configs? Good point. Then in order to preserve backward compatibility, I would suggest: config BR2_PACKAGE_FIRMWARE_IMX_DDR_VERSION string "DDR3/DDR4/LPDDR4 firmware version" # Needed for backward compatibility, the package used to # unconditionally use HW version 201810 on DDR3/DDR4 before the # FW version was made configurable default "201810" if BR2_PACKAGE_FIRMWARE_IMX_DDR3 || BR2_PACKAGE_FIRMWARE_IMX_DDR4 help ... Best regards, Thomas
diff --git a/package/freescale-imx/firmware-imx/Config.in b/package/freescale-imx/firmware-imx/Config.in index 06c4d8036c..f894d87250 100644 --- a/package/freescale-imx/firmware-imx/Config.in +++ b/package/freescale-imx/firmware-imx/Config.in @@ -82,6 +82,16 @@ config BR2_PACKAGE_FIRMWARE_IMX_DDR3 endchoice # DDR training FW +if BR2_PACKAGE_FIRMWARE_IMX_LPDDR4 + +config BR2_PACKAGE_FIRMWARE_IMX_LPDDR4_VERSION + string "LPDDR4 Version" + help + Use a specific version of the lpddr4_pmu_train_* binaries such + as "202006". + +endif # BR2_PACKAGE_FIRMWARE_IMX_LPDDR4 + config BR2_PACKAGE_FIRMWARE_IMX_IMEM_LEN hex "(LP)DDR IMEM padding length" default 0x8000 diff --git a/package/freescale-imx/firmware-imx/firmware-imx.mk b/package/freescale-imx/firmware-imx/firmware-imx.mk index 47c21585aa..40aa84e162 100644 --- a/package/freescale-imx/firmware-imx/firmware-imx.mk +++ b/package/freescale-imx/firmware-imx/firmware-imx.mk @@ -38,15 +38,23 @@ endef ifeq ($(BR2_PACKAGE_FIRMWARE_IMX_LPDDR4),y) FIRMWARE_IMX_DDRFW_DIR = $(@D)/firmware/ddr/synopsys +ifneq ($(BR2_PACKAGE_FIRMWARE_IMX_LPDDR4_VERSION), "") + FIRMWARE_IMX_LPDDR4_FW_VERSION = _$(BR2_PACKAGE_FIRMWARE_IMX_LPDDR4_VERSION) +endif define FIRMWARE_IMX_INSTALL_IMAGE_DDR_FW # Create padded versions of lpddr4_pmu_* and generate lpddr4_pmu_train_fw.bin. # lpddr4_pmu_train_fw.bin is needed when generating imx8-boot-sd.bin # which is done in post-image script. + @echo lpddr4_version: $(FIRMWARE_IMX_LPDDR4_FW_VERSION) $(call FIRMWARE_IMX_PREPARE_DDR_FW, \ - lpddr4_pmu_train_1d_imem,lpddr4_pmu_train_1d_dmem,lpddr4_pmu_train_1d_fw) + lpddr4_pmu_train_1d_imem$(FIRMWARE_IMX_LPDDR4_FW_VERSION), + lpddr4_pmu_train_1d_dmem$(FIRMWARE_IMX_LPDDR4_FW_VERSION), + lpddr4_pmu_train_1d_fw) $(call FIRMWARE_IMX_PREPARE_DDR_FW, \ - lpddr4_pmu_train_2d_imem,lpddr4_pmu_train_2d_dmem,lpddr4_pmu_train_2d_fw) + lpddr4_pmu_train_2d_imem$(FIRMWARE_IMX_LPDDR4_FW_VERSION), + lpddr4_pmu_train_2d_dmem$(FIRMWARE_IMX_LPDDR4_FW_VERSION), + lpddr4_pmu_train_2d_fw) cat $(FIRMWARE_IMX_DDRFW_DIR)/lpddr4_pmu_train_1d_fw.bin \ $(FIRMWARE_IMX_DDRFW_DIR)/lpddr4_pmu_train_2d_fw.bin > \ $(BINARIES_DIR)/lpddr4_pmu_train_fw.bin
Add support for selecting a specific version of the LPDDR4 firmware version. Variscite's imx8mp-var-dart SOM requires the "_202006" version of the lpddr4_pmu_train_* binaries. Signed-off-by: Bram Vlerick <bram.vlerick@openpixelsystems.org> --- package/freescale-imx/firmware-imx/Config.in | 10 ++++++++++ package/freescale-imx/firmware-imx/firmware-imx.mk | 12 ++++++++++-- 2 files changed, 20 insertions(+), 2 deletions(-)