[PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver

[Abdellatif El Khlifi]

On Thu, Jan 19, 2023 at 04:56:09PM +0000, Sudeep Holla wrote:
> Hi Simon,
> 
> (sorry we just crossed the emails)
> 
> On Thu, Jan 19, 2023 at 09:41:12AM -0700, Simon Glass wrote:
> >
> > Can you add a DT node for the 'FF-A SW interfaces' and attach some
> > sort of top-level driver to that? Perhaps simple-bus, or your own
> > thing? You don't need to add compatible strings for subnodes (devices
> > that are discoverable within that).
> >
> 
> Thanks for putting this nicely. I just wrote the same thing probably in
> not so simpler way. But I agree with you as Abdellatif last email talks
> more around sub-nodes or child device nodes (devices that are discoverable
> within that)

Hi Sudeep,

A gentle reminder about what has been said in my last reply:

I didn't talk about sub-nodes or child nodes. The FF-A driver in u-boot doesn't do that.

In this patchset, only 1 device is created: The FF-A bus device (arm_ffa).

The discovery done by the driver queries all the discovery interfaces described in Chapter 8 of the spec, not just FFA_VERSION.

The discovery interfaces are queried in this order:

    FFA_VERSION.
    FFA_ID_GET.
    FFA_FEATURES.
    FFA_PARTITION_INFO_GET.

Setup interfaces are used during discovery as well.

If one of these interfaces fail, the discovery fails.

The discovery from this patchset point of view is the discovery process involving the interfaces above.

Cheers

> 
> > If you don't want to submit the compatible string to Linux, I will do
> > it. If it has to have a 'u-boot,' prefix then so be it, but I don't
> > see why that is necessary, since Linux can ignore it if it likes.
> >
> > We have been talking about this for far too long, IMO. Would you like
> > me to send a patch? It is something like this:
> >
> > ff-a {
> >     compatible = "arm,ff-a";
> > };
> >
> 
> Makes sense if DT node is the only way. It should be as simple as this
> and presence of this must not imply presence of FF-A feature on the
> platform. The driver must check using FFA_VERSION
> 
> > >
> > > Discovering means gathering information about the FF-A framework such as:
> > > the FF-A version, supported features, secure partitions number and attributes.
> > >
> > > Please refer to the following paragraphs for more details: [2], [3], [4], [5]
> > >
> > > The core driver provided by this patchset implements the Setup and discovery interfaces
> > > in addition to direct messaging.
> > >
> > > The driver provides ffa_bus_discover() API that allows to discover the FF-A bus
> > > as described by the spec and in the FF-A driver readme [6].
> > >
> > > We expect and highly recommend FF-A users to always discover the FF-A bus using ffa_bus_discover() API.
> > >
> > > A use case is provided which is the EFI MM communication [7].
> > >
> > > ffa_bus_discover() does the following:
> > >
> > > - creates, binds and probes the arm_ffa device
> > > - at probe level, discovery FF-A interfaces are called to try to discover the FF-A framework
> > > - when all discovery interfaces succeed, probing is successful and FF-A bus is ready to use
> > > - if one of the discovery interfaces fails, the arm_ffa device is removed from the DM and
> > >   FF-A bus can not be used
> >
> > This is not how things are supposed to work in U-Boot. Please read the
> > documentation which is here:
> >
> > https://u-boot.readthedocs.io/en/latest/develop/driver-model/index.html
> >
> > So referencing above:
> >
> > 1, No, the binding of the ff-a device should happen automatically from the DT
> > 2. probing ff-a causes the other devices to be bound (as with PCI,
> > USB, every other bus in U-Boot)
> > 3. Yes
> > 4. No, you must not unbind it. It just sits there unprobed and cannot
> > be used. We might want to have a command that looks at what is wrong
> > with it. Probing the device should produce an error, as with every
> > other device in U-Boot
> >
> 
> Agreed on all points(assuming DT way here). Especially the last point,
> there is no point is rolling back if one partion/device initialisation
> fails or is not bound.
> 
> --
> Regards,
> Sudeep