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

Abdellatif El Khlifi
2022-08-05T11:15:52Z

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@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@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.

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.
- 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).

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

Cheers,
Abdellatif