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

From: Luca Ceresoli via buildroot <buildroot@buildroot.org>
To: "Frager, Neal" <neal.frager@amd.com>
Cc: "yann.morin@orange.com" <yann.morin@orange.com>,
	"buildroot@buildroot.org" <buildroot@buildroot.org>,
	"Erkiaga Elorza, Ibai" <ibai.erkiaga-elorza@amd.com>,
	"brandon.maier@collins.com" <brandon.maier@collins.com>,
	"ju.o@free.fr" <ju.o@free.fr>,
	"thomas.petazzoni@bootlin.com" <thomas.petazzoni@bootlin.com>,
	"romain.naour@smile.fr" <romain.naour@smile.fr>,
	"Simek, Michal" <michal.simek@amd.com>,
	"romain.naour@gmail.com" <romain.naour@gmail.com>
Subject: Re: [Buildroot] [PATCH v3 1/1] boot/xilinx-embeddedsw: only allow apps for device family
Date: Wed, 13 Aug 2025 15:31:32 +0200	[thread overview]
Message-ID: <20250813153132.5942d8f7@booty> (raw)
In-Reply-To: <SA1PR12MB56150000BDD0EC7C783A9CEFF02AA@SA1PR12MB5615.namprd12.prod.outlook.com>

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