diff mbox series

[2/2] package/freescale-imx/firmware-imx: Clean up the image/target semantic

Message ID 1593012228-18959-3-git-send-email-stephane.viau@oss.nxp.com
State Changes Requested
Headers show
Series package/freescale-imx: clean-up proposal | expand

Commit Message

Stephane Viau (OSS) June 24, 2020, 3:23 p.m. UTC
The newly introduced BR2_PACKAGE_FREESCALE_IMX_NEED_xxx_FW symbols shall
be used in lieue of the SoC type when installing images or binaries on
target.

These new symbols let us define FIRMWARE_IMX_INSTALL_IMAGES_CMDS and
FIRMWARE_IMX_INSTALL_TARGET_CMDS based on platform needs rather than
SoC type.

Suggested-by: Yann E. MORIN <yann.morin.1998@free.fr>
Signed-off-by: Stephane Viau <stephane.viau@oss.nxp.com>
---
 package/freescale-imx/firmware-imx/firmware-imx.mk | 70 +++++++++++++++++-----
 1 file changed, 54 insertions(+), 16 deletions(-)

Comments

Fabio Estevam June 24, 2020, 7:57 p.m. UTC | #1
Hi Stephane,

On Wed, Jun 24, 2020 at 12:39 PM Stephane Viau
<stephane.viau@oss.nxp.com> wrote:
>
> The newly introduced BR2_PACKAGE_FREESCALE_IMX_NEED_xxx_FW symbols shall
> be used in lieue of the SoC type when installing images or binaries on
> target.
>
> These new symbols let us define FIRMWARE_IMX_INSTALL_IMAGES_CMDS and
> FIRMWARE_IMX_INSTALL_TARGET_CMDS based on platform needs rather than
> SoC type.
>
> Suggested-by: Yann E. MORIN <yann.morin.1998@free.fr>
> Signed-off-by: Stephane Viau <stephane.viau@oss.nxp.com>

This looks like a nice solution, thanks:

Reviewed-by: Fabio Estevam <festevam@gmail.com>
Thomas Petazzoni June 24, 2020, 9:04 p.m. UTC | #2
Hello,

On Wed, 24 Jun 2020 17:23:48 +0200
Stephane Viau <stephane.viau@oss.nxp.com> wrote:

>  package/freescale-imx/firmware-imx/firmware-imx.mk | 70 +++++++++++++++++-----
>  1 file changed, 54 insertions(+), 16 deletions(-)
> 
> diff --git a/package/freescale-imx/firmware-imx/firmware-imx.mk b/package/freescale-imx/firmware-imx/firmware-imx.mk
> index 6beacc0..62f97e6 100644
> --- a/package/freescale-imx/firmware-imx/firmware-imx.mk
> +++ b/package/freescale-imx/firmware-imx/firmware-imx.mk
> @@ -12,12 +12,13 @@ FIRMWARE_IMX_LICENSE = NXP Semiconductor Software License Agreement
>  FIRMWARE_IMX_LICENSE_FILES = EULA COPYING
>  FIRMWARE_IMX_REDISTRIBUTE = NO
>  
> -FIRMWARE_IMX_BLOBS = sdma vpu
> -
>  define FIRMWARE_IMX_EXTRACT_CMDS
>  	$(call FREESCALE_IMX_EXTRACT_HELPER,$(FIRMWARE_IMX_DL_DIR)/$(FIRMWARE_IMX_SOURCE))
>  endef
>  
> +# firmware-imx install images section:
> +######################################
> +

This comment is not needed.

>  ifeq ($(BR2_PACKAGE_FREESCALE_IMX_NEED_DDR_FW),y)
>  FIRMWARE_IMX_INSTALL_IMAGES = YES
>  
> @@ -71,36 +72,73 @@ define FIRMWARE_IMX_PREPARE_DDR_FW
>  		$(BINARIES_DIR)/ddr4_201810_fw.bin
>  	ln -sf $(BINARIES_DIR)/ddr4_201810_fw.bin $(BINARIES_DIR)/ddr_fw.bin
>  endef
> -endif
> +endif # DDRFW_LPDDR4 || DDRFW_DDR4
> +endif # NEED_DDR_FW
> +
> +ifeq ($(BR2_PACKAGE_FREESCALE_IMX_NEED_HDMI_FW),y)
> +FIRMWARE_IMX_INSTALL_IMAGES = YES
>  
> -ifeq ($(BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX8M),y)
>  define FIRMWARE_IMX_PREPARE_HDMI_FW
>  	cp $(@D)/firmware/hdmi/cadence/signed_hdmi_imx8m.bin \
>  		$(BINARIES_DIR)/signed_hdmi_imx8m.bin
>  endef
> -endif
> +endif # NEED_HDMI_FW
>  
>  define FIRMWARE_IMX_INSTALL_IMAGES_CMDS
>  	$(FIRMWARE_IMX_PREPARE_DDR_FW)
>  	$(FIRMWARE_IMX_PREPARE_HDMI_FW)
>  endef

I think it would be good to group that together with the
IMX_INSTALL_TARGET_CMDS definition, towards the end of the file.

> -else ifeq ($(BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX8X),y)
> -define FIRMWARE_IMX_INSTALL_TARGET_CMDS
> +
> +# firmware-imx install target section:
> +######################################
> +

Comment not really useful.



> +ifeq ($(BR2_PACKAGE_FREESCALE_IMX_NEED_EPDC_FW),y)
> +FIRMWARE_IMX_INSTALL_TARGET = YES
> +
> +define FIRMWARE_IMX_INSTALL_TARGET_EPDC_FW
> +	mkdir -p $(TARGET_DIR)/lib/firmware/imx
> +	cp -r $(@D)/firmware/epdc $(TARGET_DIR)/lib/firmware/imx
> +	mv $(TARGET_DIR)/lib/firmware/imx/epdc/epdc_ED060XH2C1.fw.nonrestricted \
> +		$(TARGET_DIR)/lib/firmware/imx/epdc/epdc_ED060XH2C1.fw
> +endef
> +endif # NEED_EPDC_FW
> +
> +ifeq ($(BR2_PACKAGE_FREESCALE_IMX_NEED_SDMA_FW),y)
> +FIRMWARE_IMX_INSTALL_TARGET = YES
> +
> +define FIRMWARE_IMX_INSTALL_TARGET_SDMA_FW
> +	mkdir -p $(TARGET_DIR)/lib/firmware/imx
> +	cp -r $(@D)/firmware/vpu $(TARGET_DIR)/lib/firmware
> +endef
> +endif # NEED_SDMA_FW
> +
> +ifeq ($(BR2_PACKAGE_FREESCALE_IMX_NEED_VPU_FW),y)

This option doesn't exist in your Config.in.

> +FIRMWARE_IMX_INSTALL_TARGET = YES
> +
> +ifeq ($(BR2_PACKAGE_FIRMWARE_VPUFW_GENERIC),y)

Are you sure this option exists ?

> +
> +define FIRMWARE_IMX_INSTALL_TARGET_VPU_FW
> +	mkdir -p $(TARGET_DIR)/lib/firmware/imx
> +	cp -r $(@D)/firmware/vpu $(TARGET_DIR)/lib/firmware
> +endef
> +
> +endif
> +ifeq ($(BR2_PACKAGE_FIRMWARE_VPUFW_IMX8),y)

And this one ?

Thanks!

Thomas
diff mbox series

Patch

diff --git a/package/freescale-imx/firmware-imx/firmware-imx.mk b/package/freescale-imx/firmware-imx/firmware-imx.mk
index 6beacc0..62f97e6 100644
--- a/package/freescale-imx/firmware-imx/firmware-imx.mk
+++ b/package/freescale-imx/firmware-imx/firmware-imx.mk
@@ -12,12 +12,13 @@  FIRMWARE_IMX_LICENSE = NXP Semiconductor Software License Agreement
 FIRMWARE_IMX_LICENSE_FILES = EULA COPYING
 FIRMWARE_IMX_REDISTRIBUTE = NO
 
-FIRMWARE_IMX_BLOBS = sdma vpu
-
 define FIRMWARE_IMX_EXTRACT_CMDS
 	$(call FREESCALE_IMX_EXTRACT_HELPER,$(FIRMWARE_IMX_DL_DIR)/$(FIRMWARE_IMX_SOURCE))
 endef
 
+# firmware-imx install images section:
+######################################
+
 ifeq ($(BR2_PACKAGE_FREESCALE_IMX_NEED_DDR_FW),y)
 FIRMWARE_IMX_INSTALL_IMAGES = YES
 
@@ -71,36 +72,73 @@  define FIRMWARE_IMX_PREPARE_DDR_FW
 		$(BINARIES_DIR)/ddr4_201810_fw.bin
 	ln -sf $(BINARIES_DIR)/ddr4_201810_fw.bin $(BINARIES_DIR)/ddr_fw.bin
 endef
-endif
+endif # DDRFW_LPDDR4 || DDRFW_DDR4
+endif # NEED_DDR_FW
+
+ifeq ($(BR2_PACKAGE_FREESCALE_IMX_NEED_HDMI_FW),y)
+FIRMWARE_IMX_INSTALL_IMAGES = YES
 
-ifeq ($(BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX8M),y)
 define FIRMWARE_IMX_PREPARE_HDMI_FW
 	cp $(@D)/firmware/hdmi/cadence/signed_hdmi_imx8m.bin \
 		$(BINARIES_DIR)/signed_hdmi_imx8m.bin
 endef
-endif
+endif # NEED_HDMI_FW
 
 define FIRMWARE_IMX_INSTALL_IMAGES_CMDS
 	$(FIRMWARE_IMX_PREPARE_DDR_FW)
 	$(FIRMWARE_IMX_PREPARE_HDMI_FW)
 endef
-else ifeq ($(BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX8X),y)
-define FIRMWARE_IMX_INSTALL_TARGET_CMDS
+
+# firmware-imx install target section:
+######################################
+
+ifeq ($(BR2_PACKAGE_FREESCALE_IMX_NEED_EPDC_FW),y)
+FIRMWARE_IMX_INSTALL_TARGET = YES
+
+define FIRMWARE_IMX_INSTALL_TARGET_EPDC_FW
+	mkdir -p $(TARGET_DIR)/lib/firmware/imx
+	cp -r $(@D)/firmware/epdc $(TARGET_DIR)/lib/firmware/imx
+	mv $(TARGET_DIR)/lib/firmware/imx/epdc/epdc_ED060XH2C1.fw.nonrestricted \
+		$(TARGET_DIR)/lib/firmware/imx/epdc/epdc_ED060XH2C1.fw
+endef
+endif # NEED_EPDC_FW
+
+ifeq ($(BR2_PACKAGE_FREESCALE_IMX_NEED_SDMA_FW),y)
+FIRMWARE_IMX_INSTALL_TARGET = YES
+
+define FIRMWARE_IMX_INSTALL_TARGET_SDMA_FW
+	mkdir -p $(TARGET_DIR)/lib/firmware/imx
+	cp -r $(@D)/firmware/vpu $(TARGET_DIR)/lib/firmware
+endef
+endif # NEED_SDMA_FW
+
+ifeq ($(BR2_PACKAGE_FREESCALE_IMX_NEED_VPU_FW),y)
+FIRMWARE_IMX_INSTALL_TARGET = YES
+
+ifeq ($(BR2_PACKAGE_FIRMWARE_VPUFW_GENERIC),y)
+
+define FIRMWARE_IMX_INSTALL_TARGET_VPU_FW
+	mkdir -p $(TARGET_DIR)/lib/firmware/imx
+	cp -r $(@D)/firmware/vpu $(TARGET_DIR)/lib/firmware
+endef
+
+endif
+ifeq ($(BR2_PACKAGE_FIRMWARE_VPUFW_IMX8),y)
+
+define FIRMWARE_IMX_INSTALL_TARGET_VPU_FW
 	$(INSTALL) -D -m 0644 $(@D)/firmware/vpu/vpu_fw_imx8_dec.bin \
 		$(TARGET_DIR)/lib/firmware/vpu/vpu_fw_imx8_dec.bin
 	$(INSTALL) -D -m 0644 $(@D)/firmware/vpu/vpu_fw_imx8_enc.bin \
 		$(TARGET_DIR)/lib/firmware/vpu/vpu_fw_imx8_enc.bin
 endef
-else
+
+endif
+endif # NEED_VPU_FW
+
 define FIRMWARE_IMX_INSTALL_TARGET_CMDS
-	mkdir -p $(TARGET_DIR)/lib/firmware/imx
-	for blobdir in $(FIRMWARE_IMX_BLOBS); do \
-		cp -r $(@D)/firmware/$${blobdir} $(TARGET_DIR)/lib/firmware; \
-	done
-	cp -r $(@D)/firmware/epdc $(TARGET_DIR)/lib/firmware/imx
-	mv $(TARGET_DIR)/lib/firmware/imx/epdc/epdc_ED060XH2C1.fw.nonrestricted \
-		$(TARGET_DIR)/lib/firmware/imx/epdc/epdc_ED060XH2C1.fw
+	$(FIRMWARE_IMX_INSTALL_TARGET_EPDC_FW)
+	$(FIRMWARE_IMX_INSTALL_TARGET_SDMA_FW)
+	$(FIRMWARE_IMX_INSTALL_TARGET_VPU_FW)
 endef
-endif
 
 $(eval $(generic-package))