[PATCH 0/6] introduce Arm FF-A support

[Simon Glass]

Hi Abdellatif,

On Fri, 5 Aug 2022 at 05:15, Abdellatif El Khlifi
<abdellatif.elkhlifi at arm.com> wrote:
>
> On Mon, Aug 01, 2022 at 09:08:00PM -0600, Simon Glass wrote:
> > Hi Sudeep,
> >
> > On Mon, 1 Aug 2022 at 13:28, Sudeep Holla <sudeep.holla at arm.com> wrote:
> > >
> > > On Mon, Aug 01, 2022 at 01:13:23PM -0600, Simon Glass wrote:
> > > > On Wed, 13 Apr 2022 at 10:46, Tom Rini <trini at konsulko.com> wrote:
> > > > >
> > > > > How is it both discoverable and doesn't have a device tree node, in the
> > > > > kernel?
> > > >
> > > > Also, if it is discoverable, we can still use U-Boot to discover it
> > > > and then pass the info to Linux in the DT.
> > > >
> > >
> > > Why ? Linux can discover the presence of the feature with a simple SMCCC
> > > based query. We don't need any DT bindings for this particular feature.
> > > Not sure if you are talking in general or in the context of $subject
> > > feature in the kernel.
> >
> > Oh well my understanding of Open Firmware was that the firmware did
> > the probing and passed the info to the OS.
> >
> > >
> > > > I am seeing several series which don't have 'proper' DT bindings in
> > > > Linux. First I heard it was for legacy reasons, but now I am hearing
> > > > something different. For U-Boot, we really do need to have DT bindings
> > > > for devices. All this ad-hoc creation of stuff makes things hard to
> > > > discover, adds to code size and makes things like of-platdata
> > > > impossible.
> > > >
> > >
> > > I may not have the complete picture here. If you are saying that every
> > > feature in the u-boot needs DT for some reason, then that's U-boot's
> > > limitation or restriction. But just the presence of node means nothing
> >
> > Yes it is something I am becoming more and more concerned about.
> >
> > > until the corresponding feature is queried and confirmed to be present
> > > in the firmware. That is very important as we can't skip the query stage
> > > just because of presence of some DT node for this.
> > >
> > > > Furthermore, if the bindings affect U-Boot, then the U-Boot project
> > > > should have a say in what is being done there, not just be downstream
> > > > of all such changes.
> > > >
> > >
> > > I still think you talking about some issue in general and it doesn't
> > > apply in this case. The new firmware interfaces are designed to be
> > > discoverable which is the main advantage over any non discoverable
> > > hardware and/or firmware interface. One main advantage I see is that we
> > > don't need any DT bindings which makes the firmware upgrades must simpler
> > > as the users can query the interface and know exactly what they need
> > > instead of relying on DT node which may get stale if not updated with the
> > > firmware update. I am not sure if whatever I am writing here is relevant
> > > to what your concerns are as I think I haven't understood them fully.
> >
> > I'm not sure either. In particular I'm not sure why it would be easier
> > to update whatever the FF-A software is than to update the DT, since
> > presumably they are both in the firmware.
> >
> > I am talking about an issue in general and the same issue in
> > particular with this series.
> >
> > Can I suggest resending this series with a change log for each patch.
> > Also please try to avoid #ifdefs and make sure to include
> > documentation in doc/ including how this relates to the UEFI
> > firmware-update effort that ARM/Linaro is undertaking. Also, what
> > happened to the tests / sandbox driver?
> >
> > Regards,
> > Simon
>
> Hi Simon,
>
> OK, the next patchset update gonna include:
>
> - A change log for each patch
> - Less #ifdefs
> - A detailed readme under doc/
>
> The full history of what each patchset version brings are listed in v3 cover letter: please see [1]. It also contains a note about sandbox, saying that updating sandbox driver and tests is work in progress.

Thanks for the email. OK, let's get that work completed before this goes in.

Also, if you use patman (for both U-Boot and Linux) it does the change
logs for you and combines them all into the cover letter too, among
other things.

>
> A note regarding the use of #ifdefs:
>
> In some cases we need to use them. For example:
>
> - Including the driver header which exports APIs and data structures only relevant when FF-A is enabled. This needs to be decided at build time. Some platforms don't want to build FF-A code at all.

I think it is fine in header files when needed, as you say. We should
try to avoid them in C files.

> - At initcall level like done for other busses. an #ifdef is used to add an initcall for bus discovery. When FF-A is not needed, the discovery function will not be build at all (saves memory).

We'll need to discuss this as driver model handles devices in U-Boot.
Will wait to see what you come up with.

>
> [1] https://lore.kernel.org/all/20220801172053.20163-1-abdellatif.elkhlifi@arm.com/
Regards,
Simon