[Buildroot] [PATCH v3 1/1] boot/xilinx-embeddedsw: only allow apps for device family

[Buildroot] [PATCH v3 1/1] boot/xilinx-embeddedsw: only allow apps for device family

[Neal Frager via buildroot]

This patch adds an architecture cpu dependency to each application to make
sure that users can only build applications which are applicable to their
target device family.

The versal_plm and versal_psmfw applications are specific to versal devices
which are based on BR2_cortex_a72.

The zynqmp_pmufw application is specific to zynqmp devices which are based on
BR2_cortex_a53.

Signed-off-by: Neal Frager <neal.frager@amd.com>
---
V1->V2:
- Replaced new family variant config option with an architecture cpu
  dependency, so no new configs are needed.
- Updated patch title and commit message accordingly.
V2->V3:
- Changed package dependency to only appear if the cpu is BR2_cortex_a53
  or BR2_cortex_a72. This way, the xilinx-embeddedsw package will not
  appear with zero application options if another BR2_aarch64 cpu is
  selected.
---
 boot/xilinx-embeddedsw/Config.in | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/boot/xilinx-embeddedsw/Config.in b/boot/xilinx-embeddedsw/Config.in
index 0dd6433608..96e9ca2fb0 100644
--- a/boot/xilinx-embeddedsw/Config.in
+++ b/boot/xilinx-embeddedsw/Config.in
@@ -5,7 +5,7 @@ comment "xilinx-embeddedsw needs a bare metal toolchain for tuple microblazeel-b
 
 menuconfig BR2_TARGET_XILINX_EMBEDDEDSW
 	bool "xilinx-embeddedsw"
-	depends on BR2_aarch64
+	depends on BR2_cortex_a53 || BR2_cortex_a72
 	depends on BR2_TOOLCHAIN_BARE_METAL_BUILDROOT
 	help
 	  Build boot firmware applications from source for Xilinx
@@ -30,6 +30,7 @@ config BR2_TARGET_XILINX_EMBEDDEDSW_VERSION
 
 config BR2_TARGET_XILINX_EMBEDDEDSW_VERSAL_PLM
 	bool "versal plm"
+	depends on BR2_cortex_a72
 	help
 	  Build versal plm application from Xilinx/embeddedsw repo.
 	  If selected, the xilinx-prebuilt package will not install
@@ -40,6 +41,7 @@ config BR2_TARGET_XILINX_EMBEDDEDSW_VERSAL_PLM
 
 config BR2_TARGET_XILINX_EMBEDDEDSW_VERSAL_PSMFW
 	bool "versal psmfw"
+	depends on BR2_cortex_a72
 	help
 	  Build versal psmfw application from Xilinx/embeddedsw repo.
 	  If selected, the xilinx-prebuilt package will not install
@@ -50,6 +52,7 @@ config BR2_TARGET_XILINX_EMBEDDEDSW_VERSAL_PSMFW
 
 config BR2_TARGET_XILINX_EMBEDDEDSW_ZYNQMP_PMUFW
 	bool "zynqmp pmufw"
+	depends on BR2_cortex_a53
 	help
 	  Build zynqmp pmufw application from Xilinx/embeddedsw repo.
 	  If selected, the xilinx-prebuilt package will not install
-- 
2.25.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3 1/1] boot/xilinx-embeddedsw: only allow apps for device family

[Luca Ceresoli via buildroot]

Hello Neal,

On Wed, 13 Aug 2025 09:43:54 +0100
Neal Frager <neal.frager@amd.com> wrote:

> This patch adds an architecture cpu dependency to each application to make
> sure that users can only build applications which are applicable to their
> target device family.
> 
> The versal_plm and versal_psmfw applications are specific to versal devices
> which are based on BR2_cortex_a72.
> 
> The zynqmp_pmufw application is specific to zynqmp devices which are based on
> BR2_cortex_a53.
> 
> Signed-off-by: Neal Frager <neal.frager@amd.com>
> ---
> V1->V2:
> - Replaced new family variant config option with an architecture cpu
>   dependency, so no new configs are needed.
> - Updated patch title and commit message accordingly.
> V2->V3:
> - Changed package dependency to only appear if the cpu is BR2_cortex_a53
>   or BR2_cortex_a72. This way, the xilinx-embeddedsw package will not
>   appear with zero application options if another BR2_aarch64 cpu is
>   selected.
> ---
>  boot/xilinx-embeddedsw/Config.in | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/boot/xilinx-embeddedsw/Config.in b/boot/xilinx-embeddedsw/Config.in
> index 0dd6433608..96e9ca2fb0 100644
> --- a/boot/xilinx-embeddedsw/Config.in
> +++ b/boot/xilinx-embeddedsw/Config.in
> @@ -5,7 +5,7 @@ comment "xilinx-embeddedsw needs a bare metal toolchain for tuple microblazeel-b
>  
>  menuconfig BR2_TARGET_XILINX_EMBEDDEDSW
>  	bool "xilinx-embeddedsw"
> -	depends on BR2_aarch64
> +	depends on BR2_cortex_a53 || BR2_cortex_a72

The same change should apply to the 'comment' line above, no?

BTW the example I gave in v2 was touching the comment line and not the
the menuconfig line, so each of us changed one place and forgot about
the other one. :-)

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3 1/1] boot/xilinx-embeddedsw: only allow apps for device family

[Frager, Neal via buildroot]

[AMD Official Use Only - AMD Internal Distribution Only]

Hi Luca,

> This patch adds an architecture cpu dependency to each application to make
> sure that users can only build applications which are applicable to their
> target device family.
>
> The versal_plm and versal_psmfw applications are specific to versal devices
> which are based on BR2_cortex_a72.
>
> The zynqmp_pmufw application is specific to zynqmp devices which are based on
> BR2_cortex_a53.
>
> Signed-off-by: Neal Frager <neal.frager@amd.com>
> ---
> V1->V2:
> - Replaced new family variant config option with an architecture cpu
>   dependency, so no new configs are needed.
> - Updated patch title and commit message accordingly.
> V2->V3:
> - Changed package dependency to only appear if the cpu is BR2_cortex_a53
>   or BR2_cortex_a72. This way, the xilinx-embeddedsw package will not
>   appear with zero application options if another BR2_aarch64 cpu is
>   selected.
> ---
>  boot/xilinx-embeddedsw/Config.in | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/boot/xilinx-embeddedsw/Config.in b/boot/xilinx-embeddedsw/Config.in
> index 0dd6433608..96e9ca2fb0 100644
> --- a/boot/xilinx-embeddedsw/Config.in
> +++ b/boot/xilinx-embeddedsw/Config.in
> @@ -5,7 +5,7 @@ comment "xilinx-embeddedsw needs a bare metal toolchain for tuple microblazeel-b
>
>  menuconfig BR2_TARGET_XILINX_EMBEDDEDSW
>       bool "xilinx-embeddedsw"
> -     depends on BR2_aarch64
> +     depends on BR2_cortex_a53 || BR2_cortex_a72

> The same change should apply to the 'comment' line above, no?

> BTW the example I gave in v2 was touching the comment line and not the
> the menuconfig line, so each of us changed one place and forgot about
> the other one. :-)

Yes, of course! :)
I will fix this in v4.

Best regards,
Neal Frager
AMD

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3 1/1] boot/xilinx-embeddedsw: only allow apps for device family

[yann.morin]

Neal, All,

On 2025-08-13 09:43 +0100, Neal Frager via buildroot spake thusly:
> This patch adds an architecture cpu dependency to each application to make
> sure that users can only build applications which are applicable to their
> target device family.
> 
> The versal_plm and versal_psmfw applications are specific to versal devices
> which are based on BR2_cortex_a72.
> 
> The zynqmp_pmufw application is specific to zynqmp devices which are based on
> BR2_cortex_a53.
> 
> Signed-off-by: Neal Frager <neal.frager@amd.com>
> ---
> V1->V2:
> - Replaced new family variant config option with an architecture cpu
>   dependency, so no new configs are needed.
> - Updated patch title and commit message accordingly.
> V2->V3:
> - Changed package dependency to only appear if the cpu is BR2_cortex_a53
>   or BR2_cortex_a72. This way, the xilinx-embeddedsw package will not
>   appear with zero application options if another BR2_aarch64 cpu is
>   selected.
> ---
>  boot/xilinx-embeddedsw/Config.in | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/boot/xilinx-embeddedsw/Config.in b/boot/xilinx-embeddedsw/Config.in
> index 0dd6433608..96e9ca2fb0 100644
> --- a/boot/xilinx-embeddedsw/Config.in
> +++ b/boot/xilinx-embeddedsw/Config.in
> @@ -5,7 +5,7 @@ comment "xilinx-embeddedsw needs a bare metal toolchain for tuple microblazeel-b
>  
>  menuconfig BR2_TARGET_XILINX_EMBEDDEDSW
>  	bool "xilinx-embeddedsw"
> -	depends on BR2_aarch64
> +	depends on BR2_cortex_a53 || BR2_cortex_a72

Note that (BR2_cortex_a53 || BR2_cortex_a72) is not a subset of
BR2_aarch64; they can each be select when the target is BR2_arm,
BR2_armeb or BR2_aarch64_be.

So if xilinx-embeddedsw really is AArch64-LE, then you need to keep
the dependency on BR2_aarch64. If it's really just AArch64, then you can
relax it with a dependency on BR2_ARCH_IS_64 (plus the individual CPUs,
of course).

Regards,
Yann E. MORIN.

>  	depends on BR2_TOOLCHAIN_BARE_METAL_BUILDROOT
>  	help
>  	  Build boot firmware applications from source for Xilinx
> @@ -30,6 +30,7 @@ config BR2_TARGET_XILINX_EMBEDDEDSW_VERSION
>  
>  config BR2_TARGET_XILINX_EMBEDDEDSW_VERSAL_PLM
>  	bool "versal plm"
> +	depends on BR2_cortex_a72
>  	help
>  	  Build versal plm application from Xilinx/embeddedsw repo.
>  	  If selected, the xilinx-prebuilt package will not install
> @@ -40,6 +41,7 @@ config BR2_TARGET_XILINX_EMBEDDEDSW_VERSAL_PLM
>  
>  config BR2_TARGET_XILINX_EMBEDDEDSW_VERSAL_PSMFW
>  	bool "versal psmfw"
> +	depends on BR2_cortex_a72
>  	help
>  	  Build versal psmfw application from Xilinx/embeddedsw repo.
>  	  If selected, the xilinx-prebuilt package will not install
> @@ -50,6 +52,7 @@ config BR2_TARGET_XILINX_EMBEDDEDSW_VERSAL_PSMFW
>  
>  config BR2_TARGET_XILINX_EMBEDDEDSW_ZYNQMP_PMUFW
>  	bool "zynqmp pmufw"
> +	depends on BR2_cortex_a53
>  	help
>  	  Build zynqmp pmufw application from Xilinx/embeddedsw repo.
>  	  If selected, the xilinx-prebuilt package will not install
> -- 
> 2.25.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 

.-----------------.--------------------:̅̅ ̅̅̅̅ ̅̅̅̅ ̅̅̅̅ ̅̅̅̅ ̅̅̅̅ ̅̅̅̅ ̅̅̅̅_̅ ̅̅̅̅ ̅̅̅̅ ̅̅̅̅ ̅:------------------.
|  Yann E. MORIN  | Real-Time Embedded |    __/ )   | /"\ ASCII RIBBON |
|                 | Software  Designer |  _/ - /'   | \ / CAMPAIGN     |
| +33 638.411.245 '--------------------: (_    `--, |  X  AGAINST      |
| yann.morin (at) orange.com           |_="    ,--' | / \ HTML MAIL    |
'--------------------------------------:______/_____:------------------'

____________________________________________________________________________________________________________
Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3 1/1] boot/xilinx-embeddedsw: only allow apps for device family

[Luca Ceresoli via buildroot]

Hi Yann,

On Wed, 13 Aug 2025 11:20:59 +0200
yann.morin@orange.com wrote:

> Neal, All,
> 
> On 2025-08-13 09:43 +0100, Neal Frager via buildroot spake thusly:
> > This patch adds an architecture cpu dependency to each application to make
> > sure that users can only build applications which are applicable to their
> > target device family.
> > 
> > The versal_plm and versal_psmfw applications are specific to versal devices
> > which are based on BR2_cortex_a72.
> > 
> > The zynqmp_pmufw application is specific to zynqmp devices which are based on
> > BR2_cortex_a53.
> > 
> > Signed-off-by: Neal Frager <neal.frager@amd.com>
> > ---
> > V1->V2:
> > - Replaced new family variant config option with an architecture cpu
> >   dependency, so no new configs are needed.
> > - Updated patch title and commit message accordingly.
> > V2->V3:
> > - Changed package dependency to only appear if the cpu is BR2_cortex_a53
> >   or BR2_cortex_a72. This way, the xilinx-embeddedsw package will not
> >   appear with zero application options if another BR2_aarch64 cpu is
> >   selected.
> > ---
> >  boot/xilinx-embeddedsw/Config.in | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/boot/xilinx-embeddedsw/Config.in b/boot/xilinx-embeddedsw/Config.in
> > index 0dd6433608..96e9ca2fb0 100644
> > --- a/boot/xilinx-embeddedsw/Config.in
> > +++ b/boot/xilinx-embeddedsw/Config.in
> > @@ -5,7 +5,7 @@ comment "xilinx-embeddedsw needs a bare metal toolchain for tuple microblazeel-b
> >  
> >  menuconfig BR2_TARGET_XILINX_EMBEDDEDSW
> >  	bool "xilinx-embeddedsw"
> > -	depends on BR2_aarch64
> > +	depends on BR2_cortex_a53 || BR2_cortex_a72  
> 
> Note that (BR2_cortex_a53 || BR2_cortex_a72) is not a subset of
> BR2_aarch64; they can each be select when the target is BR2_arm,
> BR2_armeb or BR2_aarch64_be.

Ah, thanks for pointing out! I was indeed assuming that (a53 || a72) is
a subset of aarch64, or maybe (aarch64 || aarch64_be).

> So if xilinx-embeddedsw really is AArch64-LE, then you need to keep
> the dependency on BR2_aarch64. If it's really just AArch64, then you can
> relax it with a dependency on BR2_ARCH_IS_64 (plus the individual CPUs,
> of course).

AFAIK all the impacted Xilinx SoCs are aarch64, this that would become:

  depends on BR2_aarch64 && (BR2_cortex_a53 || BR2_cortex_a72)

But I'll let Neal confirm or correct me.

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3 1/1] boot/xilinx-embeddedsw: only allow apps for device family

[Frager, Neal via buildroot]

[AMD Official Use Only - AMD Internal Distribution Only]

Hi Yann, Luca,

> Neal, All,
>
> On 2025-08-13 09:43 +0100, Neal Frager via buildroot spake thusly:
> > This patch adds an architecture cpu dependency to each application to make
> > sure that users can only build applications which are applicable to their
> > target device family.
> >
> > The versal_plm and versal_psmfw applications are specific to versal devices
> > which are based on BR2_cortex_a72.
> >
> > The zynqmp_pmufw application is specific to zynqmp devices which are based on
> > BR2_cortex_a53.
> >
> > Signed-off-by: Neal Frager <neal.frager@amd.com>
> > ---
> > V1->V2:
> > - Replaced new family variant config option with an architecture cpu
> >   dependency, so no new configs are needed.
> > - Updated patch title and commit message accordingly.
> > V2->V3:
> > - Changed package dependency to only appear if the cpu is BR2_cortex_a53
> >   or BR2_cortex_a72. This way, the xilinx-embeddedsw package will not
> >   appear with zero application options if another BR2_aarch64 cpu is
> >   selected.
> > ---
> >  boot/xilinx-embeddedsw/Config.in | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/boot/xilinx-embeddedsw/Config.in b/boot/xilinx-embeddedsw/Config.in
> > index 0dd6433608..96e9ca2fb0 100644
> > --- a/boot/xilinx-embeddedsw/Config.in
> > +++ b/boot/xilinx-embeddedsw/Config.in
> > @@ -5,7 +5,7 @@ comment "xilinx-embeddedsw needs a bare metal toolchain for tuple microblazeel-b
> >
> >  menuconfig BR2_TARGET_XILINX_EMBEDDEDSW
> >     bool "xilinx-embeddedsw"
> > -   depends on BR2_aarch64
> > +   depends on BR2_cortex_a53 || BR2_cortex_a72
>
> Note that (BR2_cortex_a53 || BR2_cortex_a72) is not a subset of
> BR2_aarch64; they can each be select when the target is BR2_arm,
> BR2_armeb or BR2_aarch64_be.

> Ah, thanks for pointing out! I was indeed assuming that (a53 || a72) is
> a subset of aarch64, or maybe (aarch64 || aarch64_be).

> So if xilinx-embeddedsw really is AArch64-LE, then you need to keep
> the dependency on BR2_aarch64. If it's really just AArch64, then you can
> relax it with a dependency on BR2_ARCH_IS_64 (plus the individual CPUs,
> of course).

> AFAIK all the impacted Xilinx SoCs are aarch64, this that would become:

>  depends on BR2_aarch64 && (BR2_cortex_a53 || BR2_cortex_a72)

> But I'll let Neal confirm or correct me.

Yes, all Xilinx SoCs using this package are aarch64, and I expect all future
SoCs will also be aarch64 (or whatever might come next one day from ARM).

I will thus change the dependency to the following:

depends on BR2_aarch64
depends on BR2_cortex_a53 || BR2_cortex_a72

I think it looks easier to understand as two lines.

Best regards,
Neal Frager
AMD
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3 1/1] boot/xilinx-embeddedsw: only allow apps for device family

[yann.morin]

Neal, All,

On 2025-08-13 09:44 +0000, Frager, Neal spake thusly:
> > On 2025-08-13 09:43 +0100, Neal Frager via buildroot spake thusly:
[--SNIP--]
> > > -   depends on BR2_aarch64
> > > +   depends on BR2_cortex_a53 || BR2_cortex_a72
> > Note that (BR2_cortex_a53 || BR2_cortex_a72) is not a subset of
> > BR2_aarch64; they can each be select when the target is BR2_arm,
> > BR2_armeb or BR2_aarch64_be.
[--SNIP--]
> I will thus change the dependency to the following:
> depends on BR2_aarch64
> depends on BR2_cortex_a53 || BR2_cortex_a72
> I think it looks easier to understand as two lines.

When the dependency is not tricvial, I think it is good to introduce an
intermediate symbol:

    config BR2_TARGET_XILINX_EMBEDDEDSW_ARCH_SUPPORTS
        bool
        default y
        # All Xilinx SoCs using this package are aarch64 only
        depends on BR2_aarch64
        # only CPUs with corresponding firmwares:
        depends on BR2_cortex_a53 || BR2_cortex_a72  # only CPUs with

... and then use that in the main symbol:

    config BR2_TARGET_XILINX_EMBEDDEDSW
        bool "xilinx-embeddedsw"
        depends on BR2_TARGET_XILINX_EMBEDDEDSW_ARCH_SUPPORTS

Regards,
Yann E. MORIN.

-- 

.-----------------.--------------------:̅̅ ̅̅̅̅ ̅̅̅̅ ̅̅̅̅ ̅̅̅̅ ̅̅̅̅ ̅̅̅̅ ̅̅̅̅_̅ ̅̅̅̅ ̅̅̅̅ ̅̅̅̅ ̅:------------------.
|  Yann E. MORIN  | Real-Time Embedded |    __/ )   | /"\ ASCII RIBBON |
|                 | Software  Designer |  _/ - /'   | \ / CAMPAIGN     |
| +33 638.411.245 '--------------------: (_    `--, |  X  AGAINST      |
| yann.morin (at) orange.com           |_="    ,--' | / \ HTML MAIL    |
'--------------------------------------:______/_____:------------------'

____________________________________________________________________________________________________________
Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3 1/1] boot/xilinx-embeddedsw: only allow apps for device family

[Frager, Neal via buildroot]

[AMD Official Use Only - AMD Internal Distribution Only]

Hi Yann,

> [--SNIP--]
> > > -   depends on BR2_aarch64
> > > +   depends on BR2_cortex_a53 || BR2_cortex_a72
> > Note that (BR2_cortex_a53 || BR2_cortex_a72) is not a subset of
> > BR2_aarch64; they can each be select when the target is BR2_arm,
> > BR2_armeb or BR2_aarch64_be.
[--SNIP--]
> I will thus change the dependency to the following:
> depends on BR2_aarch64
> depends on BR2_cortex_a53 || BR2_cortex_a72
> I think it looks easier to understand as two lines.

> When the dependency is not tricvial, I think it is good to introduce an
> intermediate symbol:

>    config BR2_TARGET_XILINX_EMBEDDEDSW_ARCH_SUPPORTS
>        bool
>        default y
>        # All Xilinx SoCs using this package are aarch64 only
>        depends on BR2_aarch64
>        # only CPUs with corresponding firmwares:
>        depends on BR2_cortex_a53 || BR2_cortex_a72  # only CPUs with

> ... and then use that in the main symbol:

>    config BR2_TARGET_XILINX_EMBEDDEDSW
>        bool "xilinx-embeddedsw"
>        depends on BR2_TARGET_XILINX_EMBEDDEDSW_ARCH_SUPPORTS

I see your point, but don't you think adding another symbol is still
a bit overkill?  The dependencies are still only 3 lines and are rather
straight forward.

menuconfig BR2_TARGET_XILINX_EMBEDDEDSW
        bool "xilinx-embeddedsw"
        depends on BR2_aarch64
        depends on BR2_cortex_a53 || BR2_cortex_a72
        depends on BR2_TOOLCHAIN_BARE_METAL_BUILDROOT

I could add some comments, if you want, but I am not sure if we should create
another symbol just yet.

Best regards,
Neal Frager
AMD
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3 1/1] boot/xilinx-embeddedsw: only allow apps for device family

[yann.morin]

Neal, All,

On 2025-08-13 10:28 +0000, Frager, Neal spake thusly:
[--SNIP--]
> > When the dependency is not tricvial, I think it is good to introduce an
> > intermediate symbol:
> >    config BR2_TARGET_XILINX_EMBEDDEDSW_ARCH_SUPPORTS
[--SNIP--]
> I see your point, but don't you think adding another symbol is still
> a bit overkill?  The dependencies are still only 3 lines and are rather
> straight forward.

This would not be unheard of, there already are a few packages with very
few arch dependencies, but that still have a dedicated symbol, see for
example (some are more interesting, as they do have comments for each
dependency they carry):
  - bayer2rgb-neon
  - bitcoin
  - bpftrace
  - dpdk
  - host-flutter-sdk-bin
  - (host-)gdb
  - gtkiostream
  - ... and so on...

Note that we usually do that for libraries, so that it is easier to
inherit their dependencies in selecting packages, but of the above,
only two are libraries.

But maintainers will have to decide what they want. ;-)

Regards,
Yann E. MORIN.

-- 

.-----------------.--------------------:̅̅ ̅̅̅̅ ̅̅̅̅ ̅̅̅̅ ̅̅̅̅ ̅̅̅̅ ̅̅̅̅ ̅̅̅̅_̅ ̅̅̅̅ ̅̅̅̅ ̅̅̅̅ ̅:------------------.
|  Yann E. MORIN  | Real-Time Embedded |    __/ )   | /"\ ASCII RIBBON |
|                 | Software  Designer |  _/ - /'   | \ / CAMPAIGN     |
| +33 638.411.245 '--------------------: (_    `--, |  X  AGAINST      |
| yann.morin (at) orange.com           |_="    ,--' | / \ HTML MAIL    |
'--------------------------------------:______/_____:------------------'

____________________________________________________________________________________________________________
Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3 1/1] boot/xilinx-embeddedsw: only allow apps for device family

[Frager, Neal via buildroot]

[AMD Official Use Only - AMD Internal Distribution Only]

Hi Yann,

[--SNIP--]
> > When the dependency is not tricvial, I think it is good to introduce an
> > intermediate symbol:
> >    config BR2_TARGET_XILINX_EMBEDDEDSW_ARCH_SUPPORTS
[--SNIP--]
> I see your point, but don't you think adding another symbol is still
> a bit overkill?  The dependencies are still only 3 lines and are rather
> straight forward.

> This would not be unheard of, there already are a few packages with very
> few arch dependencies, but that still have a dedicated symbol, see for
> example (some are more interesting, as they do have comments for each
> dependency they carry):
>  - bayer2rgb-neon
>  - bitcoin
>  - bpftrace
>  - dpdk
>  - host-flutter-sdk-bin
>  - (host-)gdb
>  - gtkiostream
>  - ... and so on...

> Note that we usually do that for libraries, so that it is easier to
> inherit their dependencies in selecting packages, but of the above,
> only two are libraries.

> But maintainers will have to decide what they want. ;-)

In the end, I changed my mind on the issue since there are two places where
the arch dependency is used. The BR2_TARGET_XILINX_EMBEDDEDSW as well as the
comment about the dependency needs.

To avoid having the two be out of sync, it makes sense to create a
symbol BR2_TARGET_XILINX_EMBEDDEDSW_ARCH_SUPPORTS that keeps all the arch
dependencies in one place. This way, when I add BR2_cortex_a78 in the near
future, I will only have one place to add it instead of two.

Thanks for your help!

Best regards,
Neal Frager
AMD
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3 1/1] boot/xilinx-embeddedsw: only allow apps for device family

[Luca Ceresoli via buildroot]

Hello Neal,

On Wed, 13 Aug 2025 11:46:24 +0000
"Frager, Neal" <neal.frager@amd.com> wrote:

> [AMD Official Use Only - AMD Internal Distribution Only]
> 
> Hi Yann,
> 
> [--SNIP--]
> > > When the dependency is not tricvial, I think it is good to introduce an
> > > intermediate symbol:
> > >    config BR2_TARGET_XILINX_EMBEDDEDSW_ARCH_SUPPORTS  
> [--SNIP--]
> > I see your point, but don't you think adding another symbol is still
> > a bit overkill?  The dependencies are still only 3 lines and are rather
> > straight forward.  
> 
> > This would not be unheard of, there already are a few packages with very
> > few arch dependencies, but that still have a dedicated symbol, see for
> > example (some are more interesting, as they do have comments for each
> > dependency they carry):
> >  - bayer2rgb-neon
> >  - bitcoin
> >  - bpftrace
> >  - dpdk
> >  - host-flutter-sdk-bin
> >  - (host-)gdb
> >  - gtkiostream
> >  - ... and so on...  
> 
> > Note that we usually do that for libraries, so that it is easier to
> > inherit their dependencies in selecting packages, but of the above,
> > only two are libraries.  
> 
> > But maintainers will have to decide what they want. ;-)  
> 
> In the end, I changed my mind on the issue since there are two places where
> the arch dependency is used. The BR2_TARGET_XILINX_EMBEDDEDSW as well as the
> comment about the dependency needs.
> 
> To avoid having the two be out of sync, it makes sense to create a
> symbol BR2_TARGET_XILINX_EMBEDDEDSW_ARCH_SUPPORTS that keeps all the arch
> dependencies in one place. This way, when I add BR2_cortex_a78 in the near
> future, I will only have one place to add it instead of two.

I agree this is a good idea.

However the same logic can apply to other Xilinx firmware and related
packages, such as xilinx-prebuilt. Thus I'd rather create a single
symbol that can be reused for both packages, and it should probably be
in boot/Config.in. It should be also named without "EMBEDDEDSW", e.g.
BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS.

Side note: I personally don't recommend sending new iterations so
quickly, without letting the discussion settle, it produces a lot of
noise. I'd wait at least a few days after the latest e-mail in the
discussion before sending a new version.

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3 1/1] boot/xilinx-embeddedsw: only allow apps for device family

[Frager, Neal via buildroot]

[AMD Official Use Only - AMD Internal Distribution Only]

Hi Luca,

>
> Hi Yann,
>
> [--SNIP--]
> > > When the dependency is not tricvial, I think it is good to introduce an
> > > intermediate symbol:
> > >    config BR2_TARGET_XILINX_EMBEDDEDSW_ARCH_SUPPORTS
> [--SNIP--]
> > I see your point, but don't you think adding another symbol is still
> > a bit overkill?  The dependencies are still only 3 lines and are rather
> > straight forward.
>
> > This would not be unheard of, there already are a few packages with very
> > few arch dependencies, but that still have a dedicated symbol, see for
> > example (some are more interesting, as they do have comments for each
> > dependency they carry):
> >  - bayer2rgb-neon
> >  - bitcoin
> >  - bpftrace
> >  - dpdk
> >  - host-flutter-sdk-bin
> >  - (host-)gdb
> >  - gtkiostream
> >  - ... and so on...
>
> > Note that we usually do that for libraries, so that it is easier to
> > inherit their dependencies in selecting packages, but of the above,
> > only two are libraries.
>
> > But maintainers will have to decide what they want. ;-)
>
> In the end, I changed my mind on the issue since there are two places where
> the arch dependency is used. The BR2_TARGET_XILINX_EMBEDDEDSW as well as the
> comment about the dependency needs.
>
> To avoid having the two be out of sync, it makes sense to create a
> symbol BR2_TARGET_XILINX_EMBEDDEDSW_ARCH_SUPPORTS that keeps all the arch
> dependencies in one place. This way, when I add BR2_cortex_a78 in the near
> future, I will only have one place to add it instead of two.

> I agree this is a good idea.

> However the same logic can apply to other Xilinx firmware and related
> packages, such as xilinx-prebuilt. Thus I'd rather create a single
> symbol that can be reused for both packages, and it should probably be
> in boot/Config.in. It should be also named without "EMBEDDEDSW", e.g.
> BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS.

> Side note: I personally don't recommend sending new iterations so
> quickly, without letting the discussion settle, it produces a lot of
> noise. I'd wait at least a few days after the latest e-mail in the
> discussion before sending a new version.

Yes, you are right that we can create a global symbol in boot/Config.in called:
BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS

This symbol could be used for both xilinx-embeddedsw and xilinx-prebuilt.

Not only would this enable us to only show the xilinx-prebuilt package as an
option when one of the correct cpus is selected, but we could also use the
BR2_cortex_a53 and BR2_cortex_a72 to limit the available choices in the
family variant selector. This would prevent selecting BR2_cortex_a72 and
having a zynqmp or kria xilinx-prebuilt family selection or vice versa.

It would not change the need for having the family and board information
because the xilinx-prebuilt treats zynqmp and kria boards differently due to
the directory names where the pmufw.elf files are located even though these
are both based on BR2_cortex_a53.

But at least creating a BR2_TARGET_XILINX_ARCH_SUPPORTS would put the arch
dependencies of Xilinx components in one place. We could even update the
uboot package BR2_TARGET_UBOOT_ZYNQMP symbol to be dependent on
BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS instead of BR2_aarch64 as well.

The only exception would be the zynq platform. The BR2_TARGET_UBOOT_ZYNQ
symbol would stay dependent on BR2_arm because it is the only family that is
not an aarch64. I suppose I should add a BR2_cortex_a9 dependency to
BR2_TARGET_UBOOT_ZYNQ as well, so that it limits the option appearing even
more.

https://patchwork.ozlabs.org/project/buildroot/patch/20250619153922.891360-1-neal.frager@amd.com/

Thanks for the constructive discussion!

Best regards,
Neal Frager
AMD
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3 1/1] boot/xilinx-embeddedsw: only allow apps for device family

[Luca Ceresoli via buildroot]

On Wed, 13 Aug 2025 14:22:39 +0000
"Frager, Neal" <neal.frager@amd.com> wrote:

> [AMD Official Use Only - AMD Internal Distribution Only]
> 
> Hi Luca,
> 
> >
> > Hi Yann,
> >
> > [--SNIP--]  
> > > > When the dependency is not tricvial, I think it is good to introduce an
> > > > intermediate symbol:
> > > >    config BR2_TARGET_XILINX_EMBEDDEDSW_ARCH_SUPPORTS  
> > [--SNIP--]  
> > > I see your point, but don't you think adding another symbol is still
> > > a bit overkill?  The dependencies are still only 3 lines and are rather
> > > straight forward.  
> >  
> > > This would not be unheard of, there already are a few packages with very
> > > few arch dependencies, but that still have a dedicated symbol, see for
> > > example (some are more interesting, as they do have comments for each
> > > dependency they carry):
> > >  - bayer2rgb-neon
> > >  - bitcoin
> > >  - bpftrace
> > >  - dpdk
> > >  - host-flutter-sdk-bin
> > >  - (host-)gdb
> > >  - gtkiostream
> > >  - ... and so on...  
> >  
> > > Note that we usually do that for libraries, so that it is easier to
> > > inherit their dependencies in selecting packages, but of the above,
> > > only two are libraries.  
> >  
> > > But maintainers will have to decide what they want. ;-)  
> >
> > In the end, I changed my mind on the issue since there are two places where
> > the arch dependency is used. The BR2_TARGET_XILINX_EMBEDDEDSW as well as the
> > comment about the dependency needs.
> >
> > To avoid having the two be out of sync, it makes sense to create a
> > symbol BR2_TARGET_XILINX_EMBEDDEDSW_ARCH_SUPPORTS that keeps all the arch
> > dependencies in one place. This way, when I add BR2_cortex_a78 in the near
> > future, I will only have one place to add it instead of two.  
> 
> > I agree this is a good idea.  
> 
> > However the same logic can apply to other Xilinx firmware and related
> > packages, such as xilinx-prebuilt. Thus I'd rather create a single
> > symbol that can be reused for both packages, and it should probably be
> > in boot/Config.in. It should be also named without "EMBEDDEDSW", e.g.
> > BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS.  
> 
> > Side note: I personally don't recommend sending new iterations so
> > quickly, without letting the discussion settle, it produces a lot of
> > noise. I'd wait at least a few days after the latest e-mail in the
> > discussion before sending a new version.  
> 
> Yes, you are right that we can create a global symbol in boot/Config.in called:
> BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS
> 
> This symbol could be used for both xilinx-embeddedsw and xilinx-prebuilt.
> 
> Not only would this enable us to only show the xilinx-prebuilt package as an
> option when one of the correct cpus is selected, but we could also use the
> BR2_cortex_a53 and BR2_cortex_a72 to limit the available choices in the
> family variant selector. This would prevent selecting BR2_cortex_a72 and
> having a zynqmp or kria xilinx-prebuilt family selection or vice versa.
> 
> It would not change the need for having the family and board information
> because the xilinx-prebuilt treats zynqmp and kria boards differently due to
> the directory names where the pmufw.elf files are located even though these
> are both based on BR2_cortex_a53.
> 
> But at least creating a BR2_TARGET_XILINX_ARCH_SUPPORTS would put the arch
> dependencies of Xilinx components in one place. We could even update the
> uboot package BR2_TARGET_UBOOT_ZYNQMP symbol to be dependent on
> BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS instead of BR2_aarch64 as well.

Sure.

> The only exception would be the zynq platform. The BR2_TARGET_UBOOT_ZYNQ
> symbol would stay dependent on BR2_arm because it is the only family that is
> not an aarch64. I suppose I should add a BR2_cortex_a9 dependency to
> BR2_TARGET_UBOOT_ZYNQ as well, so that it limits the option appearing even
> more.
> 
> https://patchwork.ozlabs.org/project/buildroot/patch/20250619153922.891360-1-neal.frager@amd.com/

That's true as well.

If you do all of there, please keep them as separate commits in the
series to be easier to review:

 - add BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS
 - use BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS in xilinx-embeddedsw
 - use BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS in xilinx-prebuilt
 - use BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS in uboot

And the zynq7 changes are already a separate series, so I guess thay
can stay separate.

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3 1/1] boot/xilinx-embeddedsw: only allow apps for device family

[Frager, Neal via buildroot]

[AMD Official Use Only - AMD Internal Distribution Only]

Hi Luca,

>
> Hi Luca,
>
> >
> > Hi Yann,
> >
> > [--SNIP--]
> > > > When the dependency is not tricvial, I think it is good to introduce an
> > > > intermediate symbol:
> > > >    config BR2_TARGET_XILINX_EMBEDDEDSW_ARCH_SUPPORTS
> > [--SNIP--]
> > > I see your point, but don't you think adding another symbol is still
> > > a bit overkill?  The dependencies are still only 3 lines and are rather
> > > straight forward.
> >
> > > This would not be unheard of, there already are a few packages with very
> > > few arch dependencies, but that still have a dedicated symbol, see for
> > > example (some are more interesting, as they do have comments for each
> > > dependency they carry):
> > >  - bayer2rgb-neon
> > >  - bitcoin
> > >  - bpftrace
> > >  - dpdk
> > >  - host-flutter-sdk-bin
> > >  - (host-)gdb
> > >  - gtkiostream
> > >  - ... and so on...
> >
> > > Note that we usually do that for libraries, so that it is easier to
> > > inherit their dependencies in selecting packages, but of the above,
> > > only two are libraries.
> >
> > > But maintainers will have to decide what they want. ;-)
> >
> > In the end, I changed my mind on the issue since there are two places where
> > the arch dependency is used. The BR2_TARGET_XILINX_EMBEDDEDSW as well as the
> > comment about the dependency needs.
> >
> > To avoid having the two be out of sync, it makes sense to create a
> > symbol BR2_TARGET_XILINX_EMBEDDEDSW_ARCH_SUPPORTS that keeps all the arch
> > dependencies in one place. This way, when I add BR2_cortex_a78 in the near
> > future, I will only have one place to add it instead of two.
>
> > I agree this is a good idea.
>
> > However the same logic can apply to other Xilinx firmware and related
> > packages, such as xilinx-prebuilt. Thus I'd rather create a single
> > symbol that can be reused for both packages, and it should probably be
> > in boot/Config.in. It should be also named without "EMBEDDEDSW", e.g.
> > BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS.
>
> > Side note: I personally don't recommend sending new iterations so
> > quickly, without letting the discussion settle, it produces a lot of
> > noise. I'd wait at least a few days after the latest e-mail in the
> > discussion before sending a new version.
>
> Yes, you are right that we can create a global symbol in boot/Config.in called:
> BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS
>
> This symbol could be used for both xilinx-embeddedsw and xilinx-prebuilt.
>
> Not only would this enable us to only show the xilinx-prebuilt package as an
> option when one of the correct cpus is selected, but we could also use the
> BR2_cortex_a53 and BR2_cortex_a72 to limit the available choices in the
> family variant selector. This would prevent selecting BR2_cortex_a72 and
> having a zynqmp or kria xilinx-prebuilt family selection or vice versa.
>
> It would not change the need for having the family and board information
> because the xilinx-prebuilt treats zynqmp and kria boards differently due to
> the directory names where the pmufw.elf files are located even though these
> are both based on BR2_cortex_a53.
>
> But at least creating a BR2_TARGET_XILINX_ARCH_SUPPORTS would put the arch
> dependencies of Xilinx components in one place. We could even update the
> uboot package BR2_TARGET_UBOOT_ZYNQMP symbol to be dependent on
> BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS instead of BR2_aarch64 as well.

> Sure.

> The only exception would be the zynq platform. The BR2_TARGET_UBOOT_ZYNQ
> symbol would stay dependent on BR2_arm because it is the only family that is
> not an aarch64. I suppose I should add a BR2_cortex_a9 dependency to
> BR2_TARGET_UBOOT_ZYNQ as well, so that it limits the option appearing even
> more.
>
> https://patchwork.ozlabs.org/project/buildroot/patch/20250619153922.891360-1-neal.frager@amd.com/

> That's true as well.

> If you do all of there, please keep them as separate commits in the
> series to be easier to review:

> - add BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS
> - use BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS in xilinx-embeddedsw
> - use BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS in xilinx-prebuilt
> - use BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS in uboot

I will separate the patches to the individual Config.in files, but to
simplify the usage of BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS, I plan to
use it right away in the first patch by doing the following in boot/Config.in.

+if BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS
 source "boot/xilinx-embeddedsw/Config.in"
 source "boot/xilinx-prebuilt/Config.in"
+endif

In this way, I can simply remove the BR2_aarch64 dependencies in each of the
subsequent package patches.

> And the zynq7 changes are already a separate series, so I guess thay
> can stay separate.

I agree. I will add a second patch to the zynq7 series to add a BR2_cortex_a9
dependency to the BR2_TARGET_UBOOT_ZYNQ symbol.

Best regards,
Neal Frager
AMD
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3 1/1] boot/xilinx-embeddedsw: only allow apps for device family

[Luca Ceresoli via buildroot]

On Wed, 13 Aug 2025 15:28:00 +0000
"Frager, Neal" <neal.frager@amd.com> wrote:

> [AMD Official Use Only - AMD Internal Distribution Only]
> 
> Hi Luca,
> 
> >
> > Hi Luca,
> >  
> > >
> > > Hi Yann,
> > >
> > > [--SNIP--]  
> > > > > When the dependency is not tricvial, I think it is good to introduce an
> > > > > intermediate symbol:
> > > > >    config BR2_TARGET_XILINX_EMBEDDEDSW_ARCH_SUPPORTS  
> > > [--SNIP--]  
> > > > I see your point, but don't you think adding another symbol is still
> > > > a bit overkill?  The dependencies are still only 3 lines and are rather
> > > > straight forward.  
> > >  
> > > > This would not be unheard of, there already are a few packages with very
> > > > few arch dependencies, but that still have a dedicated symbol, see for
> > > > example (some are more interesting, as they do have comments for each
> > > > dependency they carry):
> > > >  - bayer2rgb-neon
> > > >  - bitcoin
> > > >  - bpftrace
> > > >  - dpdk
> > > >  - host-flutter-sdk-bin
> > > >  - (host-)gdb
> > > >  - gtkiostream
> > > >  - ... and so on...  
> > >  
> > > > Note that we usually do that for libraries, so that it is easier to
> > > > inherit their dependencies in selecting packages, but of the above,
> > > > only two are libraries.  
> > >  
> > > > But maintainers will have to decide what they want. ;-)  
> > >
> > > In the end, I changed my mind on the issue since there are two places where
> > > the arch dependency is used. The BR2_TARGET_XILINX_EMBEDDEDSW as well as the
> > > comment about the dependency needs.
> > >
> > > To avoid having the two be out of sync, it makes sense to create a
> > > symbol BR2_TARGET_XILINX_EMBEDDEDSW_ARCH_SUPPORTS that keeps all the arch
> > > dependencies in one place. This way, when I add BR2_cortex_a78 in the near
> > > future, I will only have one place to add it instead of two.  
> >  
> > > I agree this is a good idea.  
> >  
> > > However the same logic can apply to other Xilinx firmware and related
> > > packages, such as xilinx-prebuilt. Thus I'd rather create a single
> > > symbol that can be reused for both packages, and it should probably be
> > > in boot/Config.in. It should be also named without "EMBEDDEDSW", e.g.
> > > BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS.  
> >  
> > > Side note: I personally don't recommend sending new iterations so
> > > quickly, without letting the discussion settle, it produces a lot of
> > > noise. I'd wait at least a few days after the latest e-mail in the
> > > discussion before sending a new version.  
> >
> > Yes, you are right that we can create a global symbol in boot/Config.in called:
> > BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS
> >
> > This symbol could be used for both xilinx-embeddedsw and xilinx-prebuilt.
> >
> > Not only would this enable us to only show the xilinx-prebuilt package as an
> > option when one of the correct cpus is selected, but we could also use the
> > BR2_cortex_a53 and BR2_cortex_a72 to limit the available choices in the
> > family variant selector. This would prevent selecting BR2_cortex_a72 and
> > having a zynqmp or kria xilinx-prebuilt family selection or vice versa.
> >
> > It would not change the need for having the family and board information
> > because the xilinx-prebuilt treats zynqmp and kria boards differently due to
> > the directory names where the pmufw.elf files are located even though these
> > are both based on BR2_cortex_a53.
> >
> > But at least creating a BR2_TARGET_XILINX_ARCH_SUPPORTS would put the arch
> > dependencies of Xilinx components in one place. We could even update the
> > uboot package BR2_TARGET_UBOOT_ZYNQMP symbol to be dependent on
> > BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS instead of BR2_aarch64 as well.  
> 
> > Sure.  
> 
> > The only exception would be the zynq platform. The BR2_TARGET_UBOOT_ZYNQ
> > symbol would stay dependent on BR2_arm because it is the only family that is
> > not an aarch64. I suppose I should add a BR2_cortex_a9 dependency to
> > BR2_TARGET_UBOOT_ZYNQ as well, so that it limits the option appearing even
> > more.
> >
> > https://patchwork.ozlabs.org/project/buildroot/patch/20250619153922.891360-1-neal.frager@amd.com/  
> 
> > That's true as well.  
> 
> > If you do all of there, please keep them as separate commits in the
> > series to be easier to review:  
> 
> > - add BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS
> > - use BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS in xilinx-embeddedsw
> > - use BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS in xilinx-prebuilt
> > - use BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS in uboot  
> 
> I will separate the patches to the individual Config.in files, but to
> simplify the usage of BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS, I plan to
> use it right away in the first patch by doing the following in boot/Config.in.
> 
> +if BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS
>  source "boot/xilinx-embeddedsw/Config.in"
>  source "boot/xilinx-prebuilt/Config.in"
> +endif

Aah, yes, if it is that simple I guess a single patch will be fine.

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3 1/1] boot/xilinx-embeddedsw: only allow apps for device family

[Frager, Neal via buildroot]

[AMD Official Use Only - AMD Internal Distribution Only]

Hi Luca, Yann,

>
> Hi Luca,
>
> >
> > Hi Luca,
> >
> > >
> > > Hi Yann,
> > >
> > > [--SNIP--]
> > > > > When the dependency is not tricvial, I think it is good to introduce an
> > > > > intermediate symbol:
> > > > >    config BR2_TARGET_XILINX_EMBEDDEDSW_ARCH_SUPPORTS
> > > [--SNIP--]
> > > > I see your point, but don't you think adding another symbol is still
> > > > a bit overkill?  The dependencies are still only 3 lines and are rather
> > > > straight forward.
> > >
> > > > This would not be unheard of, there already are a few packages with very
> > > > few arch dependencies, but that still have a dedicated symbol, see for
> > > > example (some are more interesting, as they do have comments for each
> > > > dependency they carry):
> > > >  - bayer2rgb-neon
> > > >  - bitcoin
> > > >  - bpftrace
> > > >  - dpdk
> > > >  - host-flutter-sdk-bin
> > > >  - (host-)gdb
> > > >  - gtkiostream
> > > >  - ... and so on...
> > >
> > > > Note that we usually do that for libraries, so that it is easier to
> > > > inherit their dependencies in selecting packages, but of the above,
> > > > only two are libraries.
> > >
> > > > But maintainers will have to decide what they want. ;-)
> > >
> > > In the end, I changed my mind on the issue since there are two places where
> > > the arch dependency is used. The BR2_TARGET_XILINX_EMBEDDEDSW as well as the
> > > comment about the dependency needs.
> > >
> > > To avoid having the two be out of sync, it makes sense to create a
> > > symbol BR2_TARGET_XILINX_EMBEDDEDSW_ARCH_SUPPORTS that keeps all the arch
> > > dependencies in one place. This way, when I add BR2_cortex_a78 in the near
> > > future, I will only have one place to add it instead of two.
> >
> > > I agree this is a good idea.
> >
> > > However the same logic can apply to other Xilinx firmware and related
> > > packages, such as xilinx-prebuilt. Thus I'd rather create a single
> > > symbol that can be reused for both packages, and it should probably be
> > > in boot/Config.in. It should be also named without "EMBEDDEDSW", e.g.
> > > BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS.
> >
> > > Side note: I personally don't recommend sending new iterations so
> > > quickly, without letting the discussion settle, it produces a lot of
> > > noise. I'd wait at least a few days after the latest e-mail in the
> > > discussion before sending a new version.
> >
> > Yes, you are right that we can create a global symbol in boot/Config.in called:
> > BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS
> >
> > This symbol could be used for both xilinx-embeddedsw and xilinx-prebuilt.
> >
> > Not only would this enable us to only show the xilinx-prebuilt package as an
> > option when one of the correct cpus is selected, but we could also use the
> > BR2_cortex_a53 and BR2_cortex_a72 to limit the available choices in the
> > family variant selector. This would prevent selecting BR2_cortex_a72 and
> > having a zynqmp or kria xilinx-prebuilt family selection or vice versa.
> >
> > It would not change the need for having the family and board information
> > because the xilinx-prebuilt treats zynqmp and kria boards differently due to
> > the directory names where the pmufw.elf files are located even though these
> > are both based on BR2_cortex_a53.
> >
> > But at least creating a BR2_TARGET_XILINX_ARCH_SUPPORTS would put the arch
> > dependencies of Xilinx components in one place. We could even update the
> > uboot package BR2_TARGET_UBOOT_ZYNQMP symbol to be dependent on
> > BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS instead of BR2_aarch64 as well.
>
> > Sure.
>
> > The only exception would be the zynq platform. The BR2_TARGET_UBOOT_ZYNQ
> > symbol would stay dependent on BR2_arm because it is the only family that is
> > not an aarch64. I suppose I should add a BR2_cortex_a9 dependency to
> > BR2_TARGET_UBOOT_ZYNQ as well, so that it limits the option appearing even
> > more.
> >
> > https://patchwork.ozlabs.org/project/buildroot/patch/20250619153922.891360-1-neal.frager@amd.com/
>
> > That's true as well.
>
> > If you do all of there, please keep them as separate commits in the
> > series to be easier to review:
>
> > - add BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS
> > - use BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS in xilinx-embeddedsw
> > - use BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS in xilinx-prebuilt
> > - use BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS in uboot
>
> I will separate the patches to the individual Config.in files, but to
> simplify the usage of BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS, I plan to
> use it right away in the first patch by doing the following in boot/Config.in.
>
> +if BR2_TARGET_XILINX_FIRMWARE_ARCH_SUPPORTS
>  source "boot/xilinx-embeddedsw/Config.in"
>  source "boot/xilinx-prebuilt/Config.in"
> +endif

> Aah, yes, if it is that simple I guess a single patch will be fine.

Could we move this discussion to v6?

I have created a patch series which I believe is much nicer that covers all
the points we have discussed.

Could we start a new discussion reviewing from the v6 patch series?
https://patchwork.ozlabs.org/project/buildroot/list/?series=469156

Best regards,
Neal Frager
AMD
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot