All of lore.kernel.org
 help / color / mirror / Atom feed
From: Denys Dmytriyenko <denis@denix.org>
To: afd@ti.com
Cc: Denys Dmytriyenko <denys@konsulko.com>,
	Ryan Eatmon <reatmon@ti.com>,
	meta-ti@lists.yoctoproject.org
Subject: Re: [meta-ti][master/kirkstone][PATCH 8/8] recipes-bsp: Do not use MACHINE_ARCH when package is not machine specific
Date: Wed, 25 Oct 2023 23:27:40 -0400	[thread overview]
Message-ID: <20231026032740.GD2408@denix.org> (raw)
In-Reply-To: <20231025165630.2274889-8-afd@ti.com>

On Wed, Oct 25, 2023 at 11:56:30AM -0500, Andrew Davis via lists.yoctoproject.org wrote:
> Signed-off-by: Andrew Davis <afd@ti.com>
> ---
>  meta-ti-bsp/recipes-bsp/cadence-mhdp-fw/cadence-mhdp-fw_git.bb | 2 --
>  meta-ti-bsp/recipes-bsp/cnm-wave-fw/cnm-wave-fw_git.bb         | 2 --
>  meta-ti-bsp/recipes-bsp/cpsw9g-eth-fw/cpsw9g-eth-fw_git.bb     | 1 -
>  meta-ti-bsp/recipes-bsp/goodix-fw/goodix-fw_git.bb             | 2 --
>  meta-ti-bsp/recipes-bsp/prueth-fw/prueth-fw-am65x-sr2_git.bb   | 2 --
>  meta-ti-bsp/recipes-bsp/prueth-fw/prueth-fw-am65x_git.bb       | 2 --
>  meta-ti-bsp/recipes-bsp/pruhsr-fw/pruhsr-fw-am65x-sr2_git.bb   | 2 --
>  meta-ti-bsp/recipes-bsp/prusw-fw/prusw-fw-am65x-sr2_git.bb     | 2 --
>  meta-ti-bsp/recipes-bsp/ti-img-encode-decode/vxd-dec-fw_git.bb | 2 --
>  meta-ti-bsp/recipes-bsp/vis-fw/vis_01.50.07.15.bb              | 1 -
>  meta-ti-bsp/recipes-bsp/vpdma-fw/vpdma-fw_03-2012.bb           | 1 -
>  11 files changed, 19 deletions(-)

Overall I agree and fully support the first 7 patches in this series.

But for this last one I wanted to open a discussion.

On one hand I understand the desire to make components as generic as possible 
and reduce the number of machine-specific components to a bare minimum.

But on another hand, marking the resulting package as machine-specific when it 
has a short list of compatible machines is a standard practice. The reason is 
that the list of compatible machines controls only compile time filtering, but 
doesn't have any effect on run time. And marking packages as machine specific 
helps with that. That closes the loophole of installing incompatible packages.

For example, first recipe below specifies that Cadence MHDP firmware is 
compatible with 3 J7 platforms only (or their SoC families, to be exact).
But w/o marking resulting binary package as machine-specific (therefore 
producing separate packages for those platforms), there will be a single 
generic Aarch64 package made. And there's no protection from installing 
this generic package on non-compatible platforms, like J7200 or AM65xx, 
either manullay or by pulling it into a rootfs for those incompatible 
platforms.

And you normally want to prevent this for regular components. But I guess 
this doesn't fully apply to FW images that are loaded by corresponding 
drivers anyway. Moreover, there's no compilation involved, just packaging 
the binary blob.

In that case, should we also remove COMPATIBLE_MACHINE from these firmware 
recipes?



> diff --git a/meta-ti-bsp/recipes-bsp/cadence-mhdp-fw/cadence-mhdp-fw_git.bb b/meta-ti-bsp/recipes-bsp/cadence-mhdp-fw/cadence-mhdp-fw_git.bb
> index d88bca6e..ed1c7817 100644
> --- a/meta-ti-bsp/recipes-bsp/cadence-mhdp-fw/cadence-mhdp-fw_git.bb
> +++ b/meta-ti-bsp/recipes-bsp/cadence-mhdp-fw/cadence-mhdp-fw_git.bb
> @@ -10,8 +10,6 @@ PR = "${INC_PR}.0"
>  
>  COMPATIBLE_MACHINE = "j721e|j721s2|j784s4"
>  
> -PACKAGE_ARCH = "${MACHINE_ARCH}"
> -
>  TARGET = "mhdp8546.bin"
>  
>  do_install() {
> diff --git a/meta-ti-bsp/recipes-bsp/cnm-wave-fw/cnm-wave-fw_git.bb b/meta-ti-bsp/recipes-bsp/cnm-wave-fw/cnm-wave-fw_git.bb
> index 5b1d8be1..ef7bc2ad 100644
> --- a/meta-ti-bsp/recipes-bsp/cnm-wave-fw/cnm-wave-fw_git.bb
> +++ b/meta-ti-bsp/recipes-bsp/cnm-wave-fw/cnm-wave-fw_git.bb
> @@ -12,8 +12,6 @@ PR = "${INC_PR}.1"
>  
>  COMPATIBLE_MACHINE = "j721s2|j784s4|am62axx"
>  
> -PACKAGE_ARCH = "${MACHINE_ARCH}"
> -
>  TARGET_WAVE521C = "wave521c_codec_fw.bin"
>  
>  SOURCE_WAVE521C = "wave521c_k3_codec_fw.bin"
> diff --git a/meta-ti-bsp/recipes-bsp/cpsw9g-eth-fw/cpsw9g-eth-fw_git.bb b/meta-ti-bsp/recipes-bsp/cpsw9g-eth-fw/cpsw9g-eth-fw_git.bb
> index 6e2996ce..e333d212 100755
> --- a/meta-ti-bsp/recipes-bsp/cpsw9g-eth-fw/cpsw9g-eth-fw_git.bb
> +++ b/meta-ti-bsp/recipes-bsp/cpsw9g-eth-fw/cpsw9g-eth-fw_git.bb
> @@ -4,7 +4,6 @@ LICENSE = "TI-TFL"
>  LIC_FILES_CHKSUM = "file://LICENSE.ti;md5=04ad0a015d4bb63c2b9e7b112debf3db"
>  
>  PV = "6.2+git${SRCPV}"
> -PACKAGE_ARCH = "${MACHINE_ARCH}"
>  
>  inherit update-alternatives
>  
> diff --git a/meta-ti-bsp/recipes-bsp/goodix-fw/goodix-fw_git.bb b/meta-ti-bsp/recipes-bsp/goodix-fw/goodix-fw_git.bb
> index e58f2d58..ee3a94dc 100644
> --- a/meta-ti-bsp/recipes-bsp/goodix-fw/goodix-fw_git.bb
> +++ b/meta-ti-bsp/recipes-bsp/goodix-fw/goodix-fw_git.bb
> @@ -10,8 +10,6 @@ PR = "${INC_PR}.0"
>  
>  COMPATIBLE_MACHINE = "dra7xx"
>  
> -PACKAGE_ARCH = "${MACHINE_ARCH}"
> -
>  ORIGIN = "DRA71x-RevA-GT9271_SpecDig_Config.bin"
>  TARGET = "goodix_9271_cfg.bin"
>  
> diff --git a/meta-ti-bsp/recipes-bsp/prueth-fw/prueth-fw-am65x-sr2_git.bb b/meta-ti-bsp/recipes-bsp/prueth-fw/prueth-fw-am65x-sr2_git.bb
> index 2c0736ed..4b6ef75d 100644
> --- a/meta-ti-bsp/recipes-bsp/prueth-fw/prueth-fw-am65x-sr2_git.bb
> +++ b/meta-ti-bsp/recipes-bsp/prueth-fw/prueth-fw-am65x-sr2_git.bb
> @@ -7,8 +7,6 @@ PR = "${INC_PR}.0"
>  
>  COMPATIBLE_MACHINE = "am65xx-evm|am64xx"
>  
> -PACKAGE_ARCH = "${MACHINE_ARCH}"
> -
>  TARGET = " \
>      am65x-sr2-pru0-prueth-fw.elf \
>      am65x-sr2-pru1-prueth-fw.elf \
> diff --git a/meta-ti-bsp/recipes-bsp/prueth-fw/prueth-fw-am65x_git.bb b/meta-ti-bsp/recipes-bsp/prueth-fw/prueth-fw-am65x_git.bb
> index 8b15ab7f..20b2bfb9 100644
> --- a/meta-ti-bsp/recipes-bsp/prueth-fw/prueth-fw-am65x_git.bb
> +++ b/meta-ti-bsp/recipes-bsp/prueth-fw/prueth-fw-am65x_git.bb
> @@ -7,8 +7,6 @@ PR = "${INC_PR}.0"
>  
>  COMPATIBLE_MACHINE = "am65xx"
>  
> -PACKAGE_ARCH = "${MACHINE_ARCH}"
> -
>  TARGET = " \
>      am65x-pru0-prueth-fw.elf \
>      am65x-pru1-prueth-fw.elf \
> diff --git a/meta-ti-bsp/recipes-bsp/pruhsr-fw/pruhsr-fw-am65x-sr2_git.bb b/meta-ti-bsp/recipes-bsp/pruhsr-fw/pruhsr-fw-am65x-sr2_git.bb
> index ea39d73d..bc731094 100644
> --- a/meta-ti-bsp/recipes-bsp/pruhsr-fw/pruhsr-fw-am65x-sr2_git.bb
> +++ b/meta-ti-bsp/recipes-bsp/pruhsr-fw/pruhsr-fw-am65x-sr2_git.bb
> @@ -7,8 +7,6 @@ PR = "${INC_PR}.0"
>  
>  COMPATIBLE_MACHINE = "am65xx-evm|am64xx"
>  
> -PACKAGE_ARCH = "${MACHINE_ARCH}"
> -
>  TARGET = " \
>      am65x-sr2-pru0-pruhsr-fw.elf \
>      am65x-sr2-pru1-pruhsr-fw.elf \
> diff --git a/meta-ti-bsp/recipes-bsp/prusw-fw/prusw-fw-am65x-sr2_git.bb b/meta-ti-bsp/recipes-bsp/prusw-fw/prusw-fw-am65x-sr2_git.bb
> index 63c2d311..6e296e7c 100644
> --- a/meta-ti-bsp/recipes-bsp/prusw-fw/prusw-fw-am65x-sr2_git.bb
> +++ b/meta-ti-bsp/recipes-bsp/prusw-fw/prusw-fw-am65x-sr2_git.bb
> @@ -7,8 +7,6 @@ PR = "${INC_PR}.0"
>  
>  COMPATIBLE_MACHINE = "am65xx-evm|am64xx"
>  
> -PACKAGE_ARCH = "${MACHINE_ARCH}"
> -
>  TARGET = " \
>      am65x-sr2-pru0-prusw-fw.elf \
>      am65x-sr2-pru1-prusw-fw.elf \
> diff --git a/meta-ti-bsp/recipes-bsp/ti-img-encode-decode/vxd-dec-fw_git.bb b/meta-ti-bsp/recipes-bsp/ti-img-encode-decode/vxd-dec-fw_git.bb
> index d295a1c1..74729c16 100644
> --- a/meta-ti-bsp/recipes-bsp/ti-img-encode-decode/vxd-dec-fw_git.bb
> +++ b/meta-ti-bsp/recipes-bsp/ti-img-encode-decode/vxd-dec-fw_git.bb
> @@ -9,8 +9,6 @@ PR = "${INC_PR}.0"
>  
>  COMPATIBLE_MACHINE = "j721e"
>  
> -PACKAGE_ARCH = "${MACHINE_ARCH}"
> -
>  TARGET = "pvdec_full_bin.fw"
>  
>  do_install() {
> diff --git a/meta-ti-bsp/recipes-bsp/vis-fw/vis_01.50.07.15.bb b/meta-ti-bsp/recipes-bsp/vis-fw/vis_01.50.07.15.bb
> index 7d16ae39..4ec09a70 100644
> --- a/meta-ti-bsp/recipes-bsp/vis-fw/vis_01.50.07.15.bb
> +++ b/meta-ti-bsp/recipes-bsp/vis-fw/vis_01.50.07.15.bb
> @@ -3,7 +3,6 @@ LICENSE = "TI-TSPA"
>  LIC_FILES_CHKSUM = "file://${S}/J6_VIS_DEMO_LINUX_BINARY_01.50.07.15-Manifest.html;md5=a59aa54b9470f555cf086b91dca0afa3"
>  
>  COMPATIBLE_MACHINE = "dra7xx"
> -PACKAGE_ARCH = "${MACHINE_ARCH}"
>  
>  PR = "r1"
>  
> diff --git a/meta-ti-bsp/recipes-bsp/vpdma-fw/vpdma-fw_03-2012.bb b/meta-ti-bsp/recipes-bsp/vpdma-fw/vpdma-fw_03-2012.bb
> index 2452d111..8af49577 100644
> --- a/meta-ti-bsp/recipes-bsp/vpdma-fw/vpdma-fw_03-2012.bb
> +++ b/meta-ti-bsp/recipes-bsp/vpdma-fw/vpdma-fw_03-2012.bb
> @@ -4,7 +4,6 @@ LICENSE = "TI-TSPA"
>  LIC_FILES_CHKSUM = "file://COPYING;md5=fd463c9500441ed91d07a0331baa635c"
>  
>  COMPATIBLE_MACHINE = "dra7xx"
> -PACKAGE_ARCH = "${MACHINE_ARCH}"
>  
>  SRC_URI = "http://downloads.ti.com/dsps/dsps_public_sw/glsdk/vpdma-fw/03-2012/exports/vpdma-fw_03-2012.tar.gz;protocol=http;name=dra7xx-evm"
>  SRC_URI[dra7xx-evm.md5sum] = "80176df1350c21d9efa90171789c546e"
> -- 
> 2.39.2


  reply	other threads:[~2023-10-26  3:27 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-25 16:56 [meta-ti][master/kirkstone][PATCH 1/8] conf: machine: Move IMAGE_BOOT_FILES to the SoC inc for J721s2 and J784s4 Andrew Davis
2023-10-25 16:56 ` [meta-ti][master/kirkstone][PATCH 2/8] ti-linux-fw: Make CLEANBROKEN and FILES part of common include Andrew Davis
2023-10-25 16:56 ` [meta-ti][master/kirkstone][PATCH 3/8] ti-linux-fw: Do not set source directory when including ti-linux-fw.inc Andrew Davis
2023-10-25 16:56 ` [meta-ti][master/kirkstone][PATCH 4/8] ti-sci-fw: Do not unexport CFLAGS, LDFLAGS, AS, or LD Andrew Davis
2023-10-25 16:56 ` [meta-ti][master/kirkstone][PATCH 5/8] vpdma-fw: This firmware blob does not depend on the kernel Andrew Davis
2023-10-25 16:56 ` [meta-ti][master/kirkstone][PATCH 6/8] ti-linux-fw: Add several more firmware helper lines to this common include Andrew Davis
2023-10-25 16:56 ` [meta-ti][master/kirkstone][PATCH 7/8] recipes-bsp: Do not inherit deploy in recipes that do not deploy anything Andrew Davis
2023-10-25 16:56 ` [meta-ti][master/kirkstone][PATCH 8/8] recipes-bsp: Do not use MACHINE_ARCH when package is not machine specific Andrew Davis
2023-10-26  3:27   ` Denys Dmytriyenko [this message]
2023-10-26 14:00     ` Andrew Davis
2023-11-02 14:57       ` Ryan Eatmon
2023-11-03  6:15         ` Denys Dmytriyenko
2023-11-03 16:20           ` Ryan Eatmon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231026032740.GD2408@denix.org \
    --to=denis@denix.org \
    --cc=afd@ti.com \
    --cc=denys@konsulko.com \
    --cc=meta-ti@lists.yoctoproject.org \
    --cc=reatmon@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.