Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Wed, Jan 25, 2023 at 04:44:34PM +, Abdellatif El Khlifi wrote:
[...]
> Future SW features using SMC can be discovered by arm_smccc as well.
> We can document this approach in U-Boot/Linux so future developments
> will follow this approach.
>
OK as discussed in private, you must not need any new binding. Also looking
(a very quick glance) at the U-Boot and in particular
"Commit 2fbe47b7e771 ("firmware: psci: bind arm smccc features when
discovered")"
It looks like you have most of what you need already. Etienne has already
added mechanism for SMCCC discovery. You just need to extend things from
there if you need to support other features using SMCCC and discoverable
via SMCCC. Hope that helps.
--
Regards,
Sudeep
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Wed, Jan 25, 2023 at 10:00:58AM -0600, Rob Herring wrote: > On Tue, Jan 24, 2023 at 9:56 AM Abdellatif El Khlifi > wrote: > > > > On Mon, Jan 23, 2023 at 09:32:28AM -0700, Simon Glass wrote: > > > Hi Rob, > > > > > > On Mon, 23 Jan 2023 at 08:13, Rob Herring wrote: > > > > > > > > On Fri, Jan 20, 2023 at 4:04 PM Simon Glass wrote: > > > > > > > > > > Hi Rob, > > > > > > > > > > On Thu, 19 Jan 2023 at 11:11, Rob Herring wrote: > > > > > > > > > > > > On Thu, Jan 19, 2023 at 10:41 AM Simon Glass > > > > > > wrote: > > > > > > > > > > > > > > Hi Abdellatif, > > > > > > > > > > > > > > On Thu, 19 Jan 2023 at 09:32, Abdellatif El Khlifi > > > > > > > wrote: > > > > > > > > > > > > > > > > On Wed, Jan 18, 2023 at 08:59:32AM -0500, Tom Rini wrote: > > > > > > > > > On Wed, Jan 18, 2023 at 01:46:54PM +, Sudeep Holla wrote: > > > > > > > > > > On Wed, Jan 18, 2023 at 12:49 PM Tom Rini > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I guess the problem comes down to, can we have one > > > > > > > > > > > discovery method that > > > > > > > > > > > everyone shares, or do we have to let everyone invent a > > > > > > > > > > > new discovery > > > > > > > > > > > method every time? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > No one needs to invent any discovery method every time if > > > > > > > > > > the firmware > > > > > > > > > > specification > > > > > > > > > > provides one and as Rob mentioned many times in the thread, > > > > > > > > > > all new firmware > > > > > > > > > > specification must provide one and we are trying to make > > > > > > > > > > sure that is > > > > > > > > > > the case with all new > > > > > > > > > > specs from Arm. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > FF-A, Op-tee, U-Boot, coreboot, barebox (and > > > > > > > > > > > everyone else I'm unintentionally forgetting) could just > > > > > > > > > > > discover these > > > > > > > > > > > things via device tree. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I leave that to the individual projects to decide and agree > > > > > > > > > > but > > > > > > > > > > fundamentally if > > > > > > > > > > the specification provides a way to discover, not sure why > > > > > > > > > > we are even > > > > > > > > > > discussing > > > > > > > > > > an alternative method here. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Or, we could all write our own code to perform > > > > > > > > > > > the discovery. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > For what reason ? I can understand if there is no discovery > > > > > > > > > > mechanism but > > > > > > > > > > that's not the > > > > > > > > > > case in $subject. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > And when RISC-V comes along with similar functionality, > > > > > > > > > > > we could probe their device tree and see they've > > > > > > > > > > > implemented the same > > > > > > > > > > > concept, but a little differently, but still have the > > > > > > > > > > > discovery portion > > > > > > > > > > > be in the device tree. To which it sounds like your > > > > > > > > > > > answer is "not in > > > > > > > > > > > the device tree". > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I see U-boot seem to have made a decision to create DT node > > > > > > > > > > for each and > > > > > > > > > > everything > > > > > > > > > > that needs to be added to DM which seems bit unfortunate > > > > > > > > > > but I don't > > > > > > > > > > understand the > > > > > > > > > > history/motive/background for it but I respect the decision > > > > > > > > > > if it is > > > > > > > > > > already made. > > > > > > > > > > > > > > > > > > > > These firmware interfaces are standard on all Arm platforms > > > > > > > > > > and can be > > > > > > > > > > discovered > > > > > > > > > > based on PSCI/SMCCC. Not using the same and use DT node > > > > > > > > > > needs unnecessary > > > > > > > > > > addition of DT nodes for all the f/w i/f on all the > > > > > > > > > > platforms that need the > > > > > > > > > > support when > > > > > > > > > > one can be just discovered. > > > > > > > > > > > > > > > > > > > > Sorry for the sudden appearance on this thread, I was > > > > > > > > > > avoiding getting into > > > > > > > > > > this but thought > > > > > > > > > > I will at least express my opinion and also the way the > > > > > > > > > > firmware > > > > > > > > > > specifications from Arm is > > > > > > > > > > expected to be evolved from now on. With that I will leave > > > > > > > > > > it to you and > > > > > > > > > > other U-boot > > > > > > > > > > maintainers and the community in general to decide the > > > > > > > > > > right course in this > > > > > > > > > > case. > > > > > > > > > > > > > > > > > > To be clear, if the position is that "this is what everyone > > > > > > > > > else will > > > > > > > > > use, really" then yes, w
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Tue, Jan 24, 2023 at 9:56 AM Abdellatif El Khlifi wrote: > > On Mon, Jan 23, 2023 at 09:32:28AM -0700, Simon Glass wrote: > > Hi Rob, > > > > On Mon, 23 Jan 2023 at 08:13, Rob Herring wrote: > > > > > > On Fri, Jan 20, 2023 at 4:04 PM Simon Glass wrote: > > > > > > > > Hi Rob, > > > > > > > > On Thu, 19 Jan 2023 at 11:11, Rob Herring wrote: > > > > > > > > > > On Thu, Jan 19, 2023 at 10:41 AM Simon Glass > > > > > wrote: > > > > > > > > > > > > Hi Abdellatif, > > > > > > > > > > > > On Thu, 19 Jan 2023 at 09:32, Abdellatif El Khlifi > > > > > > wrote: > > > > > > > > > > > > > > On Wed, Jan 18, 2023 at 08:59:32AM -0500, Tom Rini wrote: > > > > > > > > On Wed, Jan 18, 2023 at 01:46:54PM +, Sudeep Holla wrote: > > > > > > > > > On Wed, Jan 18, 2023 at 12:49 PM Tom Rini > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I guess the problem comes down to, can we have one > > > > > > > > > > discovery method that > > > > > > > > > > everyone shares, or do we have to let everyone invent a new > > > > > > > > > > discovery > > > > > > > > > > method every time? > > > > > > > > > > > > > > > > > > > > > > > > > > > No one needs to invent any discovery method every time if the > > > > > > > > > firmware > > > > > > > > > specification > > > > > > > > > provides one and as Rob mentioned many times in the thread, > > > > > > > > > all new firmware > > > > > > > > > specification must provide one and we are trying to make sure > > > > > > > > > that is > > > > > > > > > the case with all new > > > > > > > > > specs from Arm. > > > > > > > > > > > > > > > > > > > > > > > > > > > > FF-A, Op-tee, U-Boot, coreboot, barebox (and > > > > > > > > > > everyone else I'm unintentionally forgetting) could just > > > > > > > > > > discover these > > > > > > > > > > things via device tree. > > > > > > > > > > > > > > > > > > > > > > > > > > > I leave that to the individual projects to decide and agree > > > > > > > > > but > > > > > > > > > fundamentally if > > > > > > > > > the specification provides a way to discover, not sure why we > > > > > > > > > are even > > > > > > > > > discussing > > > > > > > > > an alternative method here. > > > > > > > > > > > > > > > > > > > > > > > > > > > > Or, we could all write our own code to perform > > > > > > > > > > the discovery. > > > > > > > > > > > > > > > > > > > > > > > > > > > For what reason ? I can understand if there is no discovery > > > > > > > > > mechanism but > > > > > > > > > that's not the > > > > > > > > > case in $subject. > > > > > > > > > > > > > > > > > > > > > > > > > > > > And when RISC-V comes along with similar functionality, > > > > > > > > > > we could probe their device tree and see they've > > > > > > > > > > implemented the same > > > > > > > > > > concept, but a little differently, but still have the > > > > > > > > > > discovery portion > > > > > > > > > > be in the device tree. To which it sounds like your answer > > > > > > > > > > is "not in > > > > > > > > > > the device tree". > > > > > > > > > > > > > > > > > > > > > > > > > > > > I see U-boot seem to have made a decision to create DT node > > > > > > > > > for each and > > > > > > > > > everything > > > > > > > > > that needs to be added to DM which seems bit unfortunate but > > > > > > > > > I don't > > > > > > > > > understand the > > > > > > > > > history/motive/background for it but I respect the decision > > > > > > > > > if it is > > > > > > > > > already made. > > > > > > > > > > > > > > > > > > These firmware interfaces are standard on all Arm platforms > > > > > > > > > and can be > > > > > > > > > discovered > > > > > > > > > based on PSCI/SMCCC. Not using the same and use DT node needs > > > > > > > > > unnecessary > > > > > > > > > addition of DT nodes for all the f/w i/f on all the platforms > > > > > > > > > that need the > > > > > > > > > support when > > > > > > > > > one can be just discovered. > > > > > > > > > > > > > > > > > > Sorry for the sudden appearance on this thread, I was > > > > > > > > > avoiding getting into > > > > > > > > > this but thought > > > > > > > > > I will at least express my opinion and also the way the > > > > > > > > > firmware > > > > > > > > > specifications from Arm is > > > > > > > > > expected to be evolved from now on. With that I will leave it > > > > > > > > > to you and > > > > > > > > > other U-boot > > > > > > > > > maintainers and the community in general to decide the right > > > > > > > > > course in this > > > > > > > > > case. > > > > > > > > > > > > > > > > To be clear, if the position is that "this is what everyone > > > > > > > > else will > > > > > > > > use, really" then yes, we'll follow this in U-Boot. > > > > > > > > > > > > > > Hi Simon, Tom, > > > > > > > > > > > > > > The FF-A transport is a SW bus and is not associated to any HW > > > > > > > peripheral or > > > > > > > undiscoverable base address. > > > > > > > > > > > > > > There is only 1 way of discoverin
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Wed, Jan 25, 2023 at 10:55:11AM +, Abdellatif El Khlifi wrote:
> Hi Simon, Rob, Sudeep
>
> I'm tweaking my previous suggestion regarding the creation of a new
> arm_smccc root node..
>
> This node is the parent of all SW features using SMC calls like FF-A, PSCI,
> optee, ...
>
> However, no child node is created in the DT. The SW features are seen as
> architecture features and can be discovered by querying through SMC calls.
>
> So, when probed the arm_smccc device driver tries to discover the SW
> features (FF-A, PSCI, optee, ...) by calling xxx_discover() API for each
> feature ( e.g: ffa_discover() ).
>
> The xxx_discover() creates U-Boot device(s) for the feature and set
> arm_smccc as the parent.
>
> "dm tree" command should show:
>
> SMCCC
> +--PSCI
> +--TRNG
> +--FF-A
> +--SCMI (sometimes)
> +--OP-TEE
> +--...Whatever Arm comes up with next...
>
> As Sudeep suggested, we only need this node:
>
> firmware {
> arm_smccc {
> compatible = "arm,smccc-1.2";
> };
> };
>
> What do you think guys ? Are you happy with this approach ?
>
This looks good to me. No objections binding per se.
However if you look at Rob's earlier comments, due to the way all these
firmware specs evolved and hence the corresponding binding, though PSCI
uses SMCCC like all other specs in the above list/tree, the PSCI binding
was introduced along with its implementation in the kernel. Also the
fact that until SMCCC v1.1, there was no version query feature in SMCCC
and PSCI_FEATURE was used instead. It is handled in the same way in the
kernel today.
Not sure if that is the reason Rob suggested just using PSCI node like
we do in the kernel today.
As I mentioned I am not against your suggestion as it would be the best
way to start if we were doing it today, but we have legacy to handle.
Let us see what is Rob's opinion. If we decide not to use PSCI like in
the kernel and add this new SMCCC, we may have to handle that in the kernel
and also preferably add checks in DTC to not have PSCI node if SMCCC is
present or something on that lines.
--
Regards,
Sudeep
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Wed, Jan 25, 2023 at 07:48:17AM +, Sudeep Holla wrote:
> On Tue, Jan 24, 2023 at 03:56:19PM +, Abdellatif El Khlifi wrote:
> >
> > I'd like to suggest the creation of a root node called arm_smccc as shown
> > below.
> >
>
> Not sure if U-Boot already supports the existing binding of PSCI and OPTEE.
>
> > This node is the parent of all nodes using SMC calls like FF-A, PSCI,
> > optee, ...
> >
>
> Well if you want to start with the clean slate for all new platforms, then
> it is a good way. But you don't need any of the child nodes as they can all
> be queried and discovered.
>
> > The first child to be integrated in the arm_smccc root node is FF-A.
> > Hopefully, in the future the other features such as PSCI could be integrated
> > under arm_smccc as well.
> >
>
> You don't need FF-A or any other node if the presence of SMCCC node indicates
> the support of SMCCC v1.1+(need to go back and check to confirm this).
> You can query the presence of all the feature. There is no need to have a
> node for each of the feature supported. That was the whole discussion of this
> thread IIUC.
>
> --
> Regards,
> Sudeep
Hi Simon, Rob, Sudeep
I'm tweaking my previous suggestion regarding the creation of a new arm_smccc
root node.
This node is the parent of all SW features using SMC calls like FF-A, PSCI,
optee, ...
However, no child node is created in the DT. The SW features are seen as
architecture features
and can be discovered by querying through SMC calls.
So, when probed the arm_smccc device driver tries to discover the SW features
(FF-A, PSCI, optee, ...)
by calling xxx_discover() API for each feature ( e.g: ffa_discover() ).
The xxx_discover() creates U-Boot device(s) for the feature and set arm_smccc
as the parent.
"dm tree" command should show:
SMCCC
+--PSCI
+--TRNG
+--FF-A
+--SCMI (sometimes)
+--OP-TEE
+--...Whatever Arm comes up with next...
As Sudeep suggested, we only need this node:
firmware {
arm_smccc {
compatible = "arm,smccc-1.2";
};
};
What do you think guys ? Are you happy with this approach ?
Kind regards,
Abdellatif
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Tue, Jan 24, 2023 at 03:56:19PM +, Abdellatif El Khlifi wrote: > > I'd like to suggest the creation of a root node called arm_smccc as shown > below. > Not sure if U-Boot already supports the existing binding of PSCI and OPTEE. > This node is the parent of all nodes using SMC calls like FF-A, PSCI, optee, > ... > Well if you want to start with the clean slate for all new platforms, then it is a good way. But you don't need any of the child nodes as they can all be queried and discovered. > The first child to be integrated in the arm_smccc root node is FF-A. > Hopefully, in the future the other features such as PSCI could be integrated > under arm_smccc as well. > You don't need FF-A or any other node if the presence of SMCCC node indicates the support of SMCCC v1.1+(need to go back and check to confirm this). You can query the presence of all the feature. There is no need to have a node for each of the feature supported. That was the whole discussion of this thread IIUC. -- Regards, Sudeep
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Tue, Jan 24, 2023 at 03:44:53PM -0700, Simon Glass wrote: > Hi Sudeep, > > On Tue, 24 Jan 2023 at 04:30, Sudeep Holla wrote: > > > > Hi Simon, > > [...] > > Not really. I think I have not conveyed the setup details properly. > > Let me try to explain with 2 hopefully simple examples: > > > > 1. Consider a system with a power controller running its own firmware and > >OS(or any other application software including U-Boot) interacting with > >it to get the required power resource access/modification. > > > >One such thing is CPU operating points(OPP). Typically on old systems, > >these are listing in the DT and OSPM drives them completely. Now on the > >newer systems like the one I am presenting, ideally we need to get the > >list of OPP from the f/w at runtime and use the information to drive it. > > > >Suppose we have these information statically built into DT, then if the > >firmware is upgraded and there is change in the OPP set or if the f/w > >detects some configuration(bios/similar) that requires change in the > >OPP set presented to the OSPM(or any other users), then the static info > >built in the DT is stale or wrong. And hence the use of DT in such a > >configuation or system is not correct unless DT is populated on the fly > >interacting with the firmware before DT is consumed by the users. > > > >This was one example which I was referring when I said I don't want to > >use the DT info over live firmware information. > > So you need to rebuild the firmware component to update the > information? That's not nice. > Well, I understand the firmware component itself can fetch these data from the DT, but not all firmware are built with DT support to start with(for simple reasons like they need to run with very limited ram in orders of few kB). Even if they did, they might have independent DT blob with the data required for the firmware to function. The holy grail of system DT is yet to see the reality IMO. And this rebuild is very much needed during development and updating DT for the data makes no sense to me when the Linux kernel can query the same through standard interfaces defined by the firmware. There can be no argument to no use the interface to fetch the data runtime as it will be most accurate than any other data. DT may have more data or data that firmware decided not to support runtime for various reasons(I think I mentioned couple earlier). So, strict no to static data from the DT and even if some boot code updates it, I trust getting it from the firmware if and when possible. The interface has been added considering all these. It is agnostic of firmware designs and implementation. Assuming DT support in the firmware just makes these unnecessarily hard. > These parameters need to be factored out of the code. The best way to > do that is to put them in the DT. The DT should configure all of > firmware. It should not be necessary to rebuild all the code to update > a few parameters in the build. > Completely agreed in terms of design, but in reality not all implementations want to follow that for reasons of their own. > Also these parameters should not be 'buried' in the code. I'd really > like you to consider the difference between code and data, with the > (configurable) data being in DT. > Again agreed, but this is something we are enforcing in the Linux kernel and U-Boot. It is not universal and IMO it may never be. The firmware interfaces are defined to cope up with these and they are bound to stay. DT can never replace it. Further like ACPI, these firmware interface don't just provide static data, it also supports run-time. So there is some advantage and are here to stay. Preferring DT data for sake of uniformity and simplicity are not going to sway the wind away from these interface to the DT. I am definitely not for it for all the reasons I have already mentioned. > > > > 2. Now the FF-A case, if the list of partitions on the system is listed in > >the DT and there is change in that list due to various reasons(firmware > >upgrade, reconfiguration, resource disabled for a specific reason,...etc) > >then the list will be stale when presented to the user(OSPM/Linux/U-Boot) > >if it is not updated on the fly before the DT data is consumed. > > U-Boot can set this up in the DT if that is what you want, but I think > you are confusing two things: > > - what is available to use (i.e. drivers or code that is compiled in) > - what we actually want to use and how each piece if configured (this > should be handled at runtime) > No I understand all these and advantages you have been mentioning. But there is also other side of the story and that's what I have been trying to group as firmware interface. > Imagine your code is a Linux kernel. We don't hard-code these sorts of > settings in Linux. > Yes definitely and I agree as mentioned quite a few times above. But that doesn't change a bit as already me
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
Hi Abdellatif, On Tue, 24 Jan 2023 at 08:56, Abdellatif El Khlifi wrote: > > On Mon, Jan 23, 2023 at 09:32:28AM -0700, Simon Glass wrote: > > Hi Rob, > > > > On Mon, 23 Jan 2023 at 08:13, Rob Herring wrote: > > > > > > On Fri, Jan 20, 2023 at 4:04 PM Simon Glass wrote: > > > > > > > > Hi Rob, > > > > > > > > On Thu, 19 Jan 2023 at 11:11, Rob Herring wrote: > > > > > > > > > > On Thu, Jan 19, 2023 at 10:41 AM Simon Glass > > > > > wrote: > > > > > > > > > > > > Hi Abdellatif, > > > > > > > > > > > > On Thu, 19 Jan 2023 at 09:32, Abdellatif El Khlifi > > > > > > wrote: > > > > > > > > > > > > > > On Wed, Jan 18, 2023 at 08:59:32AM -0500, Tom Rini wrote: > > > > > > > > On Wed, Jan 18, 2023 at 01:46:54PM +, Sudeep Holla wrote: > > > > > > > > > On Wed, Jan 18, 2023 at 12:49 PM Tom Rini > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I guess the problem comes down to, can we have one > > > > > > > > > > discovery method that > > > > > > > > > > everyone shares, or do we have to let everyone invent a new > > > > > > > > > > discovery > > > > > > > > > > method every time? > > > > > > > > > > > > > > > > > > > > > > > > > > > No one needs to invent any discovery method every time if the > > > > > > > > > firmware > > > > > > > > > specification > > > > > > > > > provides one and as Rob mentioned many times in the thread, > > > > > > > > > all new firmware > > > > > > > > > specification must provide one and we are trying to make sure > > > > > > > > > that is > > > > > > > > > the case with all new > > > > > > > > > specs from Arm. > > > > > > > > > > > > > > > > > > > > > > > > > > > > FF-A, Op-tee, U-Boot, coreboot, barebox (and > > > > > > > > > > everyone else I'm unintentionally forgetting) could just > > > > > > > > > > discover these > > > > > > > > > > things via device tree. > > > > > > > > > > > > > > > > > > > > > > > > > > > I leave that to the individual projects to decide and agree > > > > > > > > > but > > > > > > > > > fundamentally if > > > > > > > > > the specification provides a way to discover, not sure why we > > > > > > > > > are even > > > > > > > > > discussing > > > > > > > > > an alternative method here. > > > > > > > > > > > > > > > > > > > > > > > > > > > > Or, we could all write our own code to perform > > > > > > > > > > the discovery. > > > > > > > > > > > > > > > > > > > > > > > > > > > For what reason ? I can understand if there is no discovery > > > > > > > > > mechanism but > > > > > > > > > that's not the > > > > > > > > > case in $subject. > > > > > > > > > > > > > > > > > > > > > > > > > > > > And when RISC-V comes along with similar functionality, > > > > > > > > > > we could probe their device tree and see they've > > > > > > > > > > implemented the same > > > > > > > > > > concept, but a little differently, but still have the > > > > > > > > > > discovery portion > > > > > > > > > > be in the device tree. To which it sounds like your answer > > > > > > > > > > is "not in > > > > > > > > > > the device tree". > > > > > > > > > > > > > > > > > > > > > > > > > > > > I see U-boot seem to have made a decision to create DT node > > > > > > > > > for each and > > > > > > > > > everything > > > > > > > > > that needs to be added to DM which seems bit unfortunate but > > > > > > > > > I don't > > > > > > > > > understand the > > > > > > > > > history/motive/background for it but I respect the decision > > > > > > > > > if it is > > > > > > > > > already made. > > > > > > > > > > > > > > > > > > These firmware interfaces are standard on all Arm platforms > > > > > > > > > and can be > > > > > > > > > discovered > > > > > > > > > based on PSCI/SMCCC. Not using the same and use DT node needs > > > > > > > > > unnecessary > > > > > > > > > addition of DT nodes for all the f/w i/f on all the platforms > > > > > > > > > that need the > > > > > > > > > support when > > > > > > > > > one can be just discovered. > > > > > > > > > > > > > > > > > > Sorry for the sudden appearance on this thread, I was > > > > > > > > > avoiding getting into > > > > > > > > > this but thought > > > > > > > > > I will at least express my opinion and also the way the > > > > > > > > > firmware > > > > > > > > > specifications from Arm is > > > > > > > > > expected to be evolved from now on. With that I will leave it > > > > > > > > > to you and > > > > > > > > > other U-boot > > > > > > > > > maintainers and the community in general to decide the right > > > > > > > > > course in this > > > > > > > > > case. > > > > > > > > > > > > > > > > To be clear, if the position is that "this is what everyone > > > > > > > > else will > > > > > > > > use, really" then yes, we'll follow this in U-Boot. > > > > > > > > > > > > > > Hi Simon, Tom, > > > > > > > > > > > > > > The FF-A transport is a SW bus and is not associated to any HW > > > > > > > peripheral or > > > > > > > undiscoverable base address. > > > > > > > > > > > > > > There is only 1 way
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
Hi Sudeep, On Tue, 24 Jan 2023 at 04:30, Sudeep Holla wrote: > > Hi Simon, > > On Mon, Jan 23, 2023 at 09:32:19AM -0700, Simon Glass wrote: > > Hi Sudeep, > > > > I'd like to see DT defined across firmware and OS, not just be a Linux > > thing. > > Fair enough. > > > It is a better approach than having little fiefdoms everywhere > > with their own config mechanisms. > > > > Agreed. > > > It seems that you have lots of build-time config in the ARM > > components. > > Not really. I think I have not conveyed the setup details properly. > Let me try to explain with 2 hopefully simple examples: > > 1. Consider a system with a power controller running its own firmware and >OS(or any other application software including U-Boot) interacting with >it to get the required power resource access/modification. > >One such thing is CPU operating points(OPP). Typically on old systems, >these are listing in the DT and OSPM drives them completely. Now on the >newer systems like the one I am presenting, ideally we need to get the >list of OPP from the f/w at runtime and use the information to drive it. > >Suppose we have these information statically built into DT, then if the >firmware is upgraded and there is change in the OPP set or if the f/w >detects some configuration(bios/similar) that requires change in the >OPP set presented to the OSPM(or any other users), then the static info >built in the DT is stale or wrong. And hence the use of DT in such a >configuation or system is not correct unless DT is populated on the fly >interacting with the firmware before DT is consumed by the users. > >This was one example which I was referring when I said I don't want to >use the DT info over live firmware information. So you need to rebuild the firmware component to update the information? That's not nice. These parameters need to be factored out of the code. The best way to do that is to put them in the DT. The DT should configure all of firmware. It should not be necessary to rebuild all the code to update a few parameters in the build. Also these parameters should not be 'buried' in the code. I'd really like you to consider the difference between code and data, with the (configurable) data being in DT. > > 2. Now the FF-A case, if the list of partitions on the system is listed in >the DT and there is change in that list due to various reasons(firmware >upgrade, reconfiguration, resource disabled for a specific reason,...etc) >then the list will be stale when presented to the user(OSPM/Linux/U-Boot) >if it is not updated on the fly before the DT data is consumed. U-Boot can set this up in the DT if that is what you want, but I think you are confusing two things: - what is available to use (i.e. drivers or code that is compiled in) - what we actually want to use and how each piece if configured (this should be handled at runtime) Imagine your code is a Linux kernel. We don't hard-code these sorts of settings in Linux. > > Since in both the cases the firmware provides interface to the users to get > the information at runtime, I am against using the DT as source of > information. Well we should figure out a time or place to talk about this, as I believe this is heading down the wrong path. As firmware gets more complex, we need a unified configuration mechanism and proper visibility into each component and what it is doing. They need to act as a unified whole, not as a collection of blobs that might conflict. Regards, Simon
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Mon, Jan 23, 2023 at 09:32:28AM -0700, Simon Glass wrote: > Hi Rob, > > On Mon, 23 Jan 2023 at 08:13, Rob Herring wrote: > > > > On Fri, Jan 20, 2023 at 4:04 PM Simon Glass wrote: > > > > > > Hi Rob, > > > > > > On Thu, 19 Jan 2023 at 11:11, Rob Herring wrote: > > > > > > > > On Thu, Jan 19, 2023 at 10:41 AM Simon Glass wrote: > > > > > > > > > > Hi Abdellatif, > > > > > > > > > > On Thu, 19 Jan 2023 at 09:32, Abdellatif El Khlifi > > > > > wrote: > > > > > > > > > > > > On Wed, Jan 18, 2023 at 08:59:32AM -0500, Tom Rini wrote: > > > > > > > On Wed, Jan 18, 2023 at 01:46:54PM +, Sudeep Holla wrote: > > > > > > > > On Wed, Jan 18, 2023 at 12:49 PM Tom Rini > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > I guess the problem comes down to, can we have one discovery > > > > > > > > > method that > > > > > > > > > everyone shares, or do we have to let everyone invent a new > > > > > > > > > discovery > > > > > > > > > method every time? > > > > > > > > > > > > > > > > > > > > > > > > No one needs to invent any discovery method every time if the > > > > > > > > firmware > > > > > > > > specification > > > > > > > > provides one and as Rob mentioned many times in the thread, all > > > > > > > > new firmware > > > > > > > > specification must provide one and we are trying to make sure > > > > > > > > that is > > > > > > > > the case with all new > > > > > > > > specs from Arm. > > > > > > > > > > > > > > > > > > > > > > > > > FF-A, Op-tee, U-Boot, coreboot, barebox (and > > > > > > > > > everyone else I'm unintentionally forgetting) could just > > > > > > > > > discover these > > > > > > > > > things via device tree. > > > > > > > > > > > > > > > > > > > > > > > > I leave that to the individual projects to decide and agree but > > > > > > > > fundamentally if > > > > > > > > the specification provides a way to discover, not sure why we > > > > > > > > are even > > > > > > > > discussing > > > > > > > > an alternative method here. > > > > > > > > > > > > > > > > > > > > > > > > > Or, we could all write our own code to perform > > > > > > > > > the discovery. > > > > > > > > > > > > > > > > > > > > > > > > For what reason ? I can understand if there is no discovery > > > > > > > > mechanism but > > > > > > > > that's not the > > > > > > > > case in $subject. > > > > > > > > > > > > > > > > > > > > > > > > > And when RISC-V comes along with similar functionality, > > > > > > > > > we could probe their device tree and see they've implemented > > > > > > > > > the same > > > > > > > > > concept, but a little differently, but still have the > > > > > > > > > discovery portion > > > > > > > > > be in the device tree. To which it sounds like your answer is > > > > > > > > > "not in > > > > > > > > > the device tree". > > > > > > > > > > > > > > > > > > > > > > > > > I see U-boot seem to have made a decision to create DT node for > > > > > > > > each and > > > > > > > > everything > > > > > > > > that needs to be added to DM which seems bit unfortunate but I > > > > > > > > don't > > > > > > > > understand the > > > > > > > > history/motive/background for it but I respect the decision if > > > > > > > > it is > > > > > > > > already made. > > > > > > > > > > > > > > > > These firmware interfaces are standard on all Arm platforms and > > > > > > > > can be > > > > > > > > discovered > > > > > > > > based on PSCI/SMCCC. Not using the same and use DT node needs > > > > > > > > unnecessary > > > > > > > > addition of DT nodes for all the f/w i/f on all the platforms > > > > > > > > that need the > > > > > > > > support when > > > > > > > > one can be just discovered. > > > > > > > > > > > > > > > > Sorry for the sudden appearance on this thread, I was avoiding > > > > > > > > getting into > > > > > > > > this but thought > > > > > > > > I will at least express my opinion and also the way the firmware > > > > > > > > specifications from Arm is > > > > > > > > expected to be evolved from now on. With that I will leave it > > > > > > > > to you and > > > > > > > > other U-boot > > > > > > > > maintainers and the community in general to decide the right > > > > > > > > course in this > > > > > > > > case. > > > > > > > > > > > > > > To be clear, if the position is that "this is what everyone else > > > > > > > will > > > > > > > use, really" then yes, we'll follow this in U-Boot. > > > > > > > > > > > > Hi Simon, Tom, > > > > > > > > > > > > The FF-A transport is a SW bus and is not associated to any HW > > > > > > peripheral or > > > > > > undiscoverable base address. > > > > > > > > > > > > There is only 1 way of discovering the FF-A bus and it's through > > > > > > the FF-A SW > > > > > > interfaces. The FF-A spec [1] describes this in details. > > > > > > > > > > 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 fo
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
Hi Simon, On Mon, Jan 23, 2023 at 09:32:19AM -0700, Simon Glass wrote: > Hi Sudeep, > > I'd like to see DT defined across firmware and OS, not just be a Linux > thing. Fair enough. > It is a better approach than having little fiefdoms everywhere > with their own config mechanisms. > Agreed. > It seems that you have lots of build-time config in the ARM > components. Not really. I think I have not conveyed the setup details properly. Let me try to explain with 2 hopefully simple examples: 1. Consider a system with a power controller running its own firmware and OS(or any other application software including U-Boot) interacting with it to get the required power resource access/modification. One such thing is CPU operating points(OPP). Typically on old systems, these are listing in the DT and OSPM drives them completely. Now on the newer systems like the one I am presenting, ideally we need to get the list of OPP from the f/w at runtime and use the information to drive it. Suppose we have these information statically built into DT, then if the firmware is upgraded and there is change in the OPP set or if the f/w detects some configuration(bios/similar) that requires change in the OPP set presented to the OSPM(or any other users), then the static info built in the DT is stale or wrong. And hence the use of DT in such a configuation or system is not correct unless DT is populated on the fly interacting with the firmware before DT is consumed by the users. This was one example which I was referring when I said I don't want to use the DT info over live firmware information. 2. Now the FF-A case, if the list of partitions on the system is listed in the DT and there is change in that list due to various reasons(firmware upgrade, reconfiguration, resource disabled for a specific reason,...etc) then the list will be stale when presented to the user(OSPM/Linux/U-Boot) if it is not updated on the fly before the DT data is consumed. Since in both the cases the firmware provides interface to the users to get the information at runtime, I am against using the DT as source of information. -- Regards, Sudeep
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
Hi Rob, On Mon, 23 Jan 2023 at 08:13, Rob Herring wrote: > > On Fri, Jan 20, 2023 at 4:04 PM Simon Glass wrote: > > > > Hi Rob, > > > > On Thu, 19 Jan 2023 at 11:11, Rob Herring wrote: > > > > > > On Thu, Jan 19, 2023 at 10:41 AM Simon Glass wrote: > > > > > > > > Hi Abdellatif, > > > > > > > > On Thu, 19 Jan 2023 at 09:32, Abdellatif El Khlifi > > > > wrote: > > > > > > > > > > On Wed, Jan 18, 2023 at 08:59:32AM -0500, Tom Rini wrote: > > > > > > On Wed, Jan 18, 2023 at 01:46:54PM +, Sudeep Holla wrote: > > > > > > > On Wed, Jan 18, 2023 at 12:49 PM Tom Rini > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > I guess the problem comes down to, can we have one discovery > > > > > > > > method that > > > > > > > > everyone shares, or do we have to let everyone invent a new > > > > > > > > discovery > > > > > > > > method every time? > > > > > > > > > > > > > > > > > > > > > No one needs to invent any discovery method every time if the > > > > > > > firmware > > > > > > > specification > > > > > > > provides one and as Rob mentioned many times in the thread, all > > > > > > > new firmware > > > > > > > specification must provide one and we are trying to make sure > > > > > > > that is > > > > > > > the case with all new > > > > > > > specs from Arm. > > > > > > > > > > > > > > > > > > > > > > FF-A, Op-tee, U-Boot, coreboot, barebox (and > > > > > > > > everyone else I'm unintentionally forgetting) could just > > > > > > > > discover these > > > > > > > > things via device tree. > > > > > > > > > > > > > > > > > > > > > I leave that to the individual projects to decide and agree but > > > > > > > fundamentally if > > > > > > > the specification provides a way to discover, not sure why we are > > > > > > > even > > > > > > > discussing > > > > > > > an alternative method here. > > > > > > > > > > > > > > > > > > > > > > Or, we could all write our own code to perform > > > > > > > > the discovery. > > > > > > > > > > > > > > > > > > > > > For what reason ? I can understand if there is no discovery > > > > > > > mechanism but > > > > > > > that's not the > > > > > > > case in $subject. > > > > > > > > > > > > > > > > > > > > > > And when RISC-V comes along with similar functionality, > > > > > > > > we could probe their device tree and see they've implemented > > > > > > > > the same > > > > > > > > concept, but a little differently, but still have the discovery > > > > > > > > portion > > > > > > > > be in the device tree. To which it sounds like your answer is > > > > > > > > "not in > > > > > > > > the device tree". > > > > > > > > > > > > > > > > > > > > > > I see U-boot seem to have made a decision to create DT node for > > > > > > > each and > > > > > > > everything > > > > > > > that needs to be added to DM which seems bit unfortunate but I > > > > > > > don't > > > > > > > understand the > > > > > > > history/motive/background for it but I respect the decision if it > > > > > > > is > > > > > > > already made. > > > > > > > > > > > > > > These firmware interfaces are standard on all Arm platforms and > > > > > > > can be > > > > > > > discovered > > > > > > > based on PSCI/SMCCC. Not using the same and use DT node needs > > > > > > > unnecessary > > > > > > > addition of DT nodes for all the f/w i/f on all the platforms > > > > > > > that need the > > > > > > > support when > > > > > > > one can be just discovered. > > > > > > > > > > > > > > Sorry for the sudden appearance on this thread, I was avoiding > > > > > > > getting into > > > > > > > this but thought > > > > > > > I will at least express my opinion and also the way the firmware > > > > > > > specifications from Arm is > > > > > > > expected to be evolved from now on. With that I will leave it to > > > > > > > you and > > > > > > > other U-boot > > > > > > > maintainers and the community in general to decide the right > > > > > > > course in this > > > > > > > case. > > > > > > > > > > > > To be clear, if the position is that "this is what everyone else > > > > > > will > > > > > > use, really" then yes, we'll follow this in U-Boot. > > > > > > > > > > Hi Simon, Tom, > > > > > > > > > > The FF-A transport is a SW bus and is not associated to any HW > > > > > peripheral or > > > > > undiscoverable base address. > > > > > > > > > > There is only 1 way of discovering the FF-A bus and it's through the > > > > > FF-A SW > > > > > interfaces. The FF-A spec [1] describes this in details. > > > > > > > > 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). > > > > > > We already have that. It's just called 'arm,psci'. FF-A is not the > > > top-level thing. SMCCC is. That's unfortunately called PSCI in DT > > > because SMCCC grew out of PSCI. Evolution is ugly... > > > > > > It's like this: > > > >
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
Hi Sudeep, On Fri, 20 Jan 2023 at 04:17, Sudeep Holla wrote: > > On Thu, Jan 19, 2023 at 10:22:07AM -0700, Simon Glass wrote: > > Hi Sudeep, > > > > On Thu, 19 Jan 2023 at 10:09, Sudeep Holla wrote: > > > > > > On Thu, Jan 19, 2023 at 11:57:44AM -0500, Tom Rini wrote: > > > > > > > > But it's also true that at run-time, within U-Boot, we can modify the > > > > device tree we have, with live tree yes? So, the whole series in > > > > question here can be done without modifying the base DT and getting in > > > > to the further discussions that doing so entails. The assertion is that > > > > the software discoverable bus here is sufficient to not need DT, so, OK, > > > > lets go. > > > > > > OK, may be I am not up-to-date on the U-Boot. IIUC, the modifications > > > done in the DT by U-Boot is mostly for consumption by the next stage > > > loader/OS and not for self-consumption. But if it is for self consumption, > > > then good. It helps especially for the subnodes(as Simon referred) or the > > > partitions that can be discovered at run-time using FF-A interface. > > > > It's really just dodging the issue though, because you need a > > compatible string and you might as well add it to the DT in the source > > as do it at runtime. > > > > Well that is one of the argument assuming DT node is a must. But adding > DT node requirement when it is not needed can be seen an issue itself if > one has to play devil's advocate here. > > > > > > > As mentioned I am not again DT, it is just not needed and especially > > > for subnodes it could result in inconsistency b/w what is in DT and > > > what the firmware provides. As mentioned in previous response, having a > > > simple node that Simon provided as example earlier is fine by me if that > > > is the only option to make progress as I just feel it is redundant and > > > one can say not scalable(but that is debatable again 😄). > > > > Gosh, how many of these things are you going to add? I believe the > > inconsistency argument is dealt with by the bind/probe explanation. It > > may be redundant in Linux but I doubt it would hurt there either. > > > > Not really. Currently the number of partitions is static and all of them > are bundled into on or few binaries. But we could have dynamic partitions > in the future. More over it is pain to update the DT for each possible > configuration especially during development where you want to experiment > things around. Having to deal with the DT for every small change in the > config is just annoying. Yes one could have universal list of partitions > and defer it to be dealt at probe/bind, but not sure if it is possible > to have such a universal list during development. We may have it handy > for production but we also want to ease development here. > > In short, I have had quite a lot of issues with DT and firmware > inconsistency due to duplication of same data at the beginning and the > things diverged. So I am simply not ready to go back there and I am sure > quite a few in the kernel community have their fingers bitten because of > such inconsistency created by *unnecessary static addition of data* to > the DT. So, let me be clear, I wouldn't use the information from the > DT if I can get more accurate one dynamically from the firmware in the > kernel driver. I'd like to see DT defined across firmware and OS, not just be a Linux thing. It is a better approach than having little fiefdoms everywhere with their own config mechanisms. It seems that you have lots of build-time config in the ARM components. I suggest looking at how you can move this to runtime config, if you are not planning to upstream the functionality to U-Boot. Regards, Simon
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Fri, Jan 20, 2023 at 4:04 PM Simon Glass wrote: > > Hi Rob, > > On Thu, 19 Jan 2023 at 11:11, Rob Herring wrote: > > > > On Thu, Jan 19, 2023 at 10:41 AM Simon Glass wrote: > > > > > > Hi Abdellatif, > > > > > > On Thu, 19 Jan 2023 at 09:32, Abdellatif El Khlifi > > > wrote: > > > > > > > > On Wed, Jan 18, 2023 at 08:59:32AM -0500, Tom Rini wrote: > > > > > On Wed, Jan 18, 2023 at 01:46:54PM +, Sudeep Holla wrote: > > > > > > On Wed, Jan 18, 2023 at 12:49 PM Tom Rini > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > I guess the problem comes down to, can we have one discovery > > > > > > > method that > > > > > > > everyone shares, or do we have to let everyone invent a new > > > > > > > discovery > > > > > > > method every time? > > > > > > > > > > > > > > > > > > No one needs to invent any discovery method every time if the > > > > > > firmware > > > > > > specification > > > > > > provides one and as Rob mentioned many times in the thread, all new > > > > > > firmware > > > > > > specification must provide one and we are trying to make sure that > > > > > > is > > > > > > the case with all new > > > > > > specs from Arm. > > > > > > > > > > > > > > > > > > > FF-A, Op-tee, U-Boot, coreboot, barebox (and > > > > > > > everyone else I'm unintentionally forgetting) could just discover > > > > > > > these > > > > > > > things via device tree. > > > > > > > > > > > > > > > > > > I leave that to the individual projects to decide and agree but > > > > > > fundamentally if > > > > > > the specification provides a way to discover, not sure why we are > > > > > > even > > > > > > discussing > > > > > > an alternative method here. > > > > > > > > > > > > > > > > > > > Or, we could all write our own code to perform > > > > > > > the discovery. > > > > > > > > > > > > > > > > > > For what reason ? I can understand if there is no discovery > > > > > > mechanism but > > > > > > that's not the > > > > > > case in $subject. > > > > > > > > > > > > > > > > > > > And when RISC-V comes along with similar functionality, > > > > > > > we could probe their device tree and see they've implemented the > > > > > > > same > > > > > > > concept, but a little differently, but still have the discovery > > > > > > > portion > > > > > > > be in the device tree. To which it sounds like your answer is > > > > > > > "not in > > > > > > > the device tree". > > > > > > > > > > > > > > > > > > > I see U-boot seem to have made a decision to create DT node for > > > > > > each and > > > > > > everything > > > > > > that needs to be added to DM which seems bit unfortunate but I don't > > > > > > understand the > > > > > > history/motive/background for it but I respect the decision if it is > > > > > > already made. > > > > > > > > > > > > These firmware interfaces are standard on all Arm platforms and can > > > > > > be > > > > > > discovered > > > > > > based on PSCI/SMCCC. Not using the same and use DT node needs > > > > > > unnecessary > > > > > > addition of DT nodes for all the f/w i/f on all the platforms that > > > > > > need the > > > > > > support when > > > > > > one can be just discovered. > > > > > > > > > > > > Sorry for the sudden appearance on this thread, I was avoiding > > > > > > getting into > > > > > > this but thought > > > > > > I will at least express my opinion and also the way the firmware > > > > > > specifications from Arm is > > > > > > expected to be evolved from now on. With that I will leave it to > > > > > > you and > > > > > > other U-boot > > > > > > maintainers and the community in general to decide the right course > > > > > > in this > > > > > > case. > > > > > > > > > > To be clear, if the position is that "this is what everyone else will > > > > > use, really" then yes, we'll follow this in U-Boot. > > > > > > > > Hi Simon, Tom, > > > > > > > > The FF-A transport is a SW bus and is not associated to any HW > > > > peripheral or > > > > undiscoverable base address. > > > > > > > > There is only 1 way of discovering the FF-A bus and it's through the > > > > FF-A SW > > > > interfaces. The FF-A spec [1] describes this in details. > > > > > > 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). > > > > We already have that. It's just called 'arm,psci'. FF-A is not the > > top-level thing. SMCCC is. That's unfortunately called PSCI in DT > > because SMCCC grew out of PSCI. Evolution is ugly... > > > > It's like this: > > > > SMCCC > > +--PSCI > > +--TRNG > > +--FF-A > > +--SCMI (sometimes) > > +--OP-TEE > > +--...Whatever Arm comes up with next... > > OK well that sounds OK. > > So what is the problem here? We have an SMCCC top-level thing in the > DT and everything else can be bound from that, right? Are people on > this thread not aware of this...or am I s
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
Hi Rob, On Thu, 19 Jan 2023 at 11:11, Rob Herring wrote: > > On Thu, Jan 19, 2023 at 10:41 AM Simon Glass wrote: > > > > Hi Abdellatif, > > > > On Thu, 19 Jan 2023 at 09:32, Abdellatif El Khlifi > > wrote: > > > > > > On Wed, Jan 18, 2023 at 08:59:32AM -0500, Tom Rini wrote: > > > > On Wed, Jan 18, 2023 at 01:46:54PM +, Sudeep Holla wrote: > > > > > On Wed, Jan 18, 2023 at 12:49 PM Tom Rini wrote: > > > > > > > > > > > > > > > > > I guess the problem comes down to, can we have one discovery method > > > > > > that > > > > > > everyone shares, or do we have to let everyone invent a new > > > > > > discovery > > > > > > method every time? > > > > > > > > > > > > > > > No one needs to invent any discovery method every time if the firmware > > > > > specification > > > > > provides one and as Rob mentioned many times in the thread, all new > > > > > firmware > > > > > specification must provide one and we are trying to make sure that is > > > > > the case with all new > > > > > specs from Arm. > > > > > > > > > > > > > > > > FF-A, Op-tee, U-Boot, coreboot, barebox (and > > > > > > everyone else I'm unintentionally forgetting) could just discover > > > > > > these > > > > > > things via device tree. > > > > > > > > > > > > > > > I leave that to the individual projects to decide and agree but > > > > > fundamentally if > > > > > the specification provides a way to discover, not sure why we are even > > > > > discussing > > > > > an alternative method here. > > > > > > > > > > > > > > > > Or, we could all write our own code to perform > > > > > > the discovery. > > > > > > > > > > > > > > > For what reason ? I can understand if there is no discovery mechanism > > > > > but > > > > > that's not the > > > > > case in $subject. > > > > > > > > > > > > > > > > And when RISC-V comes along with similar functionality, > > > > > > we could probe their device tree and see they've implemented the > > > > > > same > > > > > > concept, but a little differently, but still have the discovery > > > > > > portion > > > > > > be in the device tree. To which it sounds like your answer is "not > > > > > > in > > > > > > the device tree". > > > > > > > > > > > > > > > > I see U-boot seem to have made a decision to create DT node for each > > > > > and > > > > > everything > > > > > that needs to be added to DM which seems bit unfortunate but I don't > > > > > understand the > > > > > history/motive/background for it but I respect the decision if it is > > > > > already made. > > > > > > > > > > These firmware interfaces are standard on all Arm platforms and can be > > > > > discovered > > > > > based on PSCI/SMCCC. Not using the same and use DT node needs > > > > > unnecessary > > > > > addition of DT nodes for all the f/w i/f on all the platforms that > > > > > need the > > > > > support when > > > > > one can be just discovered. > > > > > > > > > > Sorry for the sudden appearance on this thread, I was avoiding > > > > > getting into > > > > > this but thought > > > > > I will at least express my opinion and also the way the firmware > > > > > specifications from Arm is > > > > > expected to be evolved from now on. With that I will leave it to you > > > > > and > > > > > other U-boot > > > > > maintainers and the community in general to decide the right course > > > > > in this > > > > > case. > > > > > > > > To be clear, if the position is that "this is what everyone else will > > > > use, really" then yes, we'll follow this in U-Boot. > > > > > > Hi Simon, Tom, > > > > > > The FF-A transport is a SW bus and is not associated to any HW peripheral > > > or > > > undiscoverable base address. > > > > > > There is only 1 way of discovering the FF-A bus and it's through the FF-A > > > SW > > > interfaces. The FF-A spec [1] describes this in details. > > > > 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). > > We already have that. It's just called 'arm,psci'. FF-A is not the > top-level thing. SMCCC is. That's unfortunately called PSCI in DT > because SMCCC grew out of PSCI. Evolution is ugly... > > It's like this: > > SMCCC > +--PSCI > +--TRNG > +--FF-A > +--SCMI (sometimes) > +--OP-TEE > +--...Whatever Arm comes up with next... OK well that sounds OK. So what is the problem here? We have an SMCCC top-level thing in the DT and everything else can be bound from that, right? Are people on this thread not aware of this...or am I still missing something? Can you point to the SMCCC driver in U-Boot? Is this bind_smccc_features(), i.w.c. it looks like it does what I want...why does this thread exist? So confused...:-) Regards, Simon
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Thu, Jan 19, 2023 at 10:22:07AM -0700, Simon Glass wrote: > Hi Sudeep, > > On Thu, 19 Jan 2023 at 10:09, Sudeep Holla wrote: > > > > On Thu, Jan 19, 2023 at 11:57:44AM -0500, Tom Rini wrote: > > > > > > But it's also true that at run-time, within U-Boot, we can modify the > > > device tree we have, with live tree yes? So, the whole series in > > > question here can be done without modifying the base DT and getting in > > > to the further discussions that doing so entails. The assertion is that > > > the software discoverable bus here is sufficient to not need DT, so, OK, > > > lets go. > > > > OK, may be I am not up-to-date on the U-Boot. IIUC, the modifications > > done in the DT by U-Boot is mostly for consumption by the next stage > > loader/OS and not for self-consumption. But if it is for self consumption, > > then good. It helps especially for the subnodes(as Simon referred) or the > > partitions that can be discovered at run-time using FF-A interface. > > It's really just dodging the issue though, because you need a > compatible string and you might as well add it to the DT in the source > as do it at runtime. > Well that is one of the argument assuming DT node is a must. But adding DT node requirement when it is not needed can be seen an issue itself if one has to play devil's advocate here. > > > > As mentioned I am not again DT, it is just not needed and especially > > for subnodes it could result in inconsistency b/w what is in DT and > > what the firmware provides. As mentioned in previous response, having a > > simple node that Simon provided as example earlier is fine by me if that > > is the only option to make progress as I just feel it is redundant and > > one can say not scalable(but that is debatable again 😄). > > Gosh, how many of these things are you going to add? I believe the > inconsistency argument is dealt with by the bind/probe explanation. It > may be redundant in Linux but I doubt it would hurt there either. > Not really. Currently the number of partitions is static and all of them are bundled into on or few binaries. But we could have dynamic partitions in the future. More over it is pain to update the DT for each possible configuration especially during development where you want to experiment things around. Having to deal with the DT for every small change in the config is just annoying. Yes one could have universal list of partitions and defer it to be dealt at probe/bind, but not sure if it is possible to have such a universal list during development. We may have it handy for production but we also want to ease development here. In short, I have had quite a lot of issues with DT and firmware inconsistency due to duplication of same data at the beginning and the things diverged. So I am simply not ready to go back there and I am sure quite a few in the kernel community have their fingers bitten because of such inconsistency created by *unnecessary static addition of data* to the DT. So, let me be clear, I wouldn't use the information from the DT if I can get more accurate one dynamically from the firmware in the kernel driver. -- Regards, Sudeep
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
Hi Simon, On Thu, Jan 19, 2023 at 11:04:16AM -0700, Simon Glass wrote: [...] > > Well we already have the code, right...? > Correct, that what we would like to see. > The entire device tree is optional. We could just use platform data or > even C function calls to set up devices. We could call a C function > for each board which builds a device tree early in boot. Etc... As I > remember it, no one wanted to use DT and it was only Linus Torvalds' > refusal to accept another ARM pull request that made people move away > from platform data and other ad-hoc mechanisms. I'll note that U-Boot > adopted DT on ARM in 2011, the same year as Linux [1] [2]. Since then > huge strides have been made in many ways, e.g. loads of useful > bindings and Rob Herring's validation stuff. > OK here is where I differ from your understanding. DT for mainly to replace platform specific data. And though firmware is *platform specific*, the firmware interface itself is not and can be compared to architecture features. Do we define all Arm architecture specific features in the DT, the answer is clearly no, unless there is something missed in the architecture specification and too late to add anything to support platforms in the wild. Similarly, the some of these standard firmware interfaces are to avoid such platform description in DT or ACPI. There are self discoverable like some of the architecture features. It is just in software rather than the hardware. > IMO the problem here is exactly because of the point I mentioned. I > suspect the reason no one wants to add a compatible string for the > top-level FF-A device is that it will be rejected by Linux and they > don't want another case of PTSD (I am willing to take that on if it > helps). I'm sorry that we are in this situation, but it is not going > to be fixed by ignoring the problem. It is causing all sorts of > work-arounds [3], is bad for project interoperability (and therefore > the open source industry as a whole), wastes a huge amount of time in > discussion and gives device tree a bad name[4]. It really, really > needs to stop. I am not sure if that is the reason for ACPI adoption TBH. IMO both have their own advantage and disadvantage. The idea is to try to minimise the deviation between the two(not on a wider scope though). The firmware interfaces like FF-A avoids the needs for any description of the feature in DT or ACPI all together and hence avoids any such deviation problems. > > Can we all agree to work together on this and see device tree as a > shared technology and resource, across firmware and OS? Could this be > the last thread on this topic? > With the mention of ACPI now I just can't think how do you propose to use DT as shared tech/ resource across f/w and OS when ACPI is used. For sure if something is not discoverable, they need to be described in DT and hence in ACPI too, but FF-A is not one such feature and I have no justification to add it in ACPI other than "DT has it, so lets add even if it is unnecessary". In summary, look at FF-A as software architecture feature that can be discovered and not as a platform specific feature that needs platform specific data from DT or ACPI. -- Regards, Sudeep
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Thu, Jan 19, 2023 at 12:11:34PM -0600, Rob Herring wrote: > On Thu, Jan 19, 2023 at 10:41 AM 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). > > We already have that. It's just called 'arm,psci'. FF-A is not the > top-level thing. SMCCC is. That's unfortunately called PSCI in DT > because SMCCC grew out of PSCI. Evolution is ugly... > > It's like this: > > SMCCC > +--PSCI > +--TRNG > +--FF-A > +--SCMI (sometimes) > +--OP-TEE > +--...Whatever Arm comes up with next... > Thanks Rob for the nice description. Hi Simon, Though SMCCC and PSCI are kind of swapped in reality like Rob mentioned while referring to the ugly evolution, we are trying to stick with PSCI node and discovery other features dynamically in the kernel. Hope the reasons for not defining the extra unnecessary bindings for FF-A is clear now. There are couple of other bindings IIRC, one being for the OPTEE for example which again could have been avoid if we had better idea on how these SMCCC and its users would have evolved. -- Regards, Sudeep
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Thu, Jan 19, 2023 at 04:56:09PM +, 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
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Thu, Jan 19, 2023 at 10:41 AM Simon Glass wrote: > > Hi Abdellatif, > > On Thu, 19 Jan 2023 at 09:32, Abdellatif El Khlifi > wrote: > > > > On Wed, Jan 18, 2023 at 08:59:32AM -0500, Tom Rini wrote: > > > On Wed, Jan 18, 2023 at 01:46:54PM +, Sudeep Holla wrote: > > > > On Wed, Jan 18, 2023 at 12:49 PM Tom Rini wrote: > > > > > > > > > > > > > > I guess the problem comes down to, can we have one discovery method > > > > > that > > > > > everyone shares, or do we have to let everyone invent a new discovery > > > > > method every time? > > > > > > > > > > > > No one needs to invent any discovery method every time if the firmware > > > > specification > > > > provides one and as Rob mentioned many times in the thread, all new > > > > firmware > > > > specification must provide one and we are trying to make sure that is > > > > the case with all new > > > > specs from Arm. > > > > > > > > > > > > > FF-A, Op-tee, U-Boot, coreboot, barebox (and > > > > > everyone else I'm unintentionally forgetting) could just discover > > > > > these > > > > > things via device tree. > > > > > > > > > > > > I leave that to the individual projects to decide and agree but > > > > fundamentally if > > > > the specification provides a way to discover, not sure why we are even > > > > discussing > > > > an alternative method here. > > > > > > > > > > > > > Or, we could all write our own code to perform > > > > > the discovery. > > > > > > > > > > > > For what reason ? I can understand if there is no discovery mechanism > > > > but > > > > that's not the > > > > case in $subject. > > > > > > > > > > > > > And when RISC-V comes along with similar functionality, > > > > > we could probe their device tree and see they've implemented the same > > > > > concept, but a little differently, but still have the discovery > > > > > portion > > > > > be in the device tree. To which it sounds like your answer is "not in > > > > > the device tree". > > > > > > > > > > > > > I see U-boot seem to have made a decision to create DT node for each and > > > > everything > > > > that needs to be added to DM which seems bit unfortunate but I don't > > > > understand the > > > > history/motive/background for it but I respect the decision if it is > > > > already made. > > > > > > > > These firmware interfaces are standard on all Arm platforms and can be > > > > discovered > > > > based on PSCI/SMCCC. Not using the same and use DT node needs > > > > unnecessary > > > > addition of DT nodes for all the f/w i/f on all the platforms that need > > > > the > > > > support when > > > > one can be just discovered. > > > > > > > > Sorry for the sudden appearance on this thread, I was avoiding getting > > > > into > > > > this but thought > > > > I will at least express my opinion and also the way the firmware > > > > specifications from Arm is > > > > expected to be evolved from now on. With that I will leave it to you and > > > > other U-boot > > > > maintainers and the community in general to decide the right course in > > > > this > > > > case. > > > > > > To be clear, if the position is that "this is what everyone else will > > > use, really" then yes, we'll follow this in U-Boot. > > > > Hi Simon, Tom, > > > > The FF-A transport is a SW bus and is not associated to any HW peripheral or > > undiscoverable base address. > > > > There is only 1 way of discovering the FF-A bus and it's through the FF-A SW > > interfaces. The FF-A spec [1] describes this in details. > > 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). We already have that. It's just called 'arm,psci'. FF-A is not the top-level thing. SMCCC is. That's unfortunately called PSCI in DT because SMCCC grew out of PSCI. Evolution is ugly... It's like this: SMCCC +--PSCI +--TRNG +--FF-A +--SCMI (sometimes) +--OP-TEE +--...Whatever Arm comes up with next... Rob
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
Hi Tom, On Thu, 19 Jan 2023 at 10:24, Tom Rini wrote: > > On Thu, Jan 19, 2023 at 10:21:07AM -0700, Simon Glass wrote: > > Hi Tom, > > > > On Thu, 19 Jan 2023 at 09:57, Tom Rini wrote: > > > > > > On Thu, Jan 19, 2023 at 09:54:29AM -0700, Simon Glass wrote: > > > > Hi Sudeep, > > > > > > > > On Thu, 19 Jan 2023 at 09:46, Sudeep Holla wrote: > > > > > > > > > > Hi Abdellatif, > > > > > > > > > > On Thu, Jan 19, 2023 at 04:31:57PM +, Abdellatif El Khlifi wrote: > > > > > > > > > > > > Hi Simon, Tom, > > > > > > > > > > > > The FF-A transport is a SW bus and is not associated to any HW > > > > > > peripheral or > > > > > > undiscoverable base address. > > > > > > > > > > > > There is only 1 way of discovering the FF-A bus and it's through > > > > > > the FF-A SW > > > > > > interfaces. The FF-A spec [1] describes this in details. > > > > > > > > > > > > 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. > > > > > > > > > > > > > > > > Thanks for the details. But IIRC this discussion is not about the > > > > > FF-A bus > > > > > and device(partitions) discovery, but the support for FF-A itself. The > > > > > discussion is about where to have a device node to represent the > > > > > existence of > > > > > FF-A support on a platform. If we are talking about individual > > > > > partitions > > > > > (devices) in the device tree, then that is pure stupidity as it goes > > > > > out > > > > > of since with the firmware the moment a partition is added or removed > > > > > in > > > > > the firmware. > > > > > > > > > > IIUC, the whole discussion was around whether to use FFA_VERSION as > > > > > the > > > > > discovery mechanism for existence of FF-A support on a platform or you > > > > > have a device node to specify the same. > > > > > > > > No, with respect, that is not quite the situation here. > > > > > > > > > > > > > > Just to be clear, even if it is decided to add a device node, the > > > > > FFA_VERSION must be used to detect the presence of FF-A support and > > > > > return error otherwise. DT node presence is just to satisfy the design > > > > > and must be treated as no auto-confirmation for the presence of FF-A > > > > > support. We are just arguing the device node presence is just > > > > > redundant, > > > > > but as mentioned before it is up to U-Boot community to make a call on > > > > > what is best. > > > > > > > > U-Boot driver model design already supports this. You can have a > > > > device that binds (from DT) but will not probe because it is not > > > > present / wrong version. Perhaps this was missed in the conversion to > > > > Linux: > > > > > > > > https://u-boot.readthedocs.io/en/latest/develop/driver-model/design.html#driver-lifecycle > > > > > > > > So there is nothing clever needed here at all and anything you do just > > > > adds confusion and bad precedent. > > > > > > But it's also true that at run-time, within U-Boot, we can modify the > > > device tree we have, with live tree yes? So, the whole series in > > > question here can be done without modifying the base DT and getting in > > > to the further discussions that doing so entails. The assertion is that > > > the software discoverable bus here is sufficient to not need DT, so, OK, > > > lets go. > > > > One of the reasons that I find this all so frustrating is that it is > > circular logic: > > > > 1. Device-tree bindings are controlled by Linux; U-Boot cannot upstream > > bindings > > 2. We can only have upstreamed bindings in any device tree > > > > We have invented a whole u-boot.dtsi feature in U-Boot to hold > > modifications from Linux. Board vendors have been suffering with this > > for years. > > > > It is not fair and it really needs to stop. I am doing what I can to > > upstream some basic U-Boot bindings and I hope that will work and can > > lead to a healthier relationship here. > > Yes, but this is a problem outside of that scope. The argument here is > that one does not need a device tree node to work. So lets see just how > clean and nice the code can be without what you and I have been > insisting would lead to the cleanest result. Well we already have the code, right...? The entire device tree is optional. We could just use platform data or even C fun
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Thu, Jan 19, 2023 at 10:21:07AM -0700, Simon Glass wrote: > Hi Tom, > > On Thu, 19 Jan 2023 at 09:57, Tom Rini wrote: > > > > On Thu, Jan 19, 2023 at 09:54:29AM -0700, Simon Glass wrote: > > > Hi Sudeep, > > > > > > On Thu, 19 Jan 2023 at 09:46, Sudeep Holla wrote: > > > > > > > > Hi Abdellatif, > > > > > > > > On Thu, Jan 19, 2023 at 04:31:57PM +, Abdellatif El Khlifi wrote: > > > > > > > > > > Hi Simon, Tom, > > > > > > > > > > The FF-A transport is a SW bus and is not associated to any HW > > > > > peripheral or > > > > > undiscoverable base address. > > > > > > > > > > There is only 1 way of discovering the FF-A bus and it's through the > > > > > FF-A SW > > > > > interfaces. The FF-A spec [1] describes this in details. > > > > > > > > > > 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. > > > > > > > > > > > > > Thanks for the details. But IIRC this discussion is not about the FF-A > > > > bus > > > > and device(partitions) discovery, but the support for FF-A itself. The > > > > discussion is about where to have a device node to represent the > > > > existence of > > > > FF-A support on a platform. If we are talking about individual > > > > partitions > > > > (devices) in the device tree, then that is pure stupidity as it goes out > > > > of since with the firmware the moment a partition is added or removed in > > > > the firmware. > > > > > > > > IIUC, the whole discussion was around whether to use FFA_VERSION as the > > > > discovery mechanism for existence of FF-A support on a platform or you > > > > have a device node to specify the same. > > > > > > No, with respect, that is not quite the situation here. > > > > > > > > > > > Just to be clear, even if it is decided to add a device node, the > > > > FFA_VERSION must be used to detect the presence of FF-A support and > > > > return error otherwise. DT node presence is just to satisfy the design > > > > and must be treated as no auto-confirmation for the presence of FF-A > > > > support. We are just arguing the device node presence is just redundant, > > > > but as mentioned before it is up to U-Boot community to make a call on > > > > what is best. > > > > > > U-Boot driver model design already supports this. You can have a > > > device that binds (from DT) but will not probe because it is not > > > present / wrong version. Perhaps this was missed in the conversion to > > > Linux: > > > > > > https://u-boot.readthedocs.io/en/latest/develop/driver-model/design.html#driver-lifecycle > > > > > > So there is nothing clever needed here at all and anything you do just > > > adds confusion and bad precedent. > > > > But it's also true that at run-time, within U-Boot, we can modify the > > device tree we have, with live tree yes? So, the whole series in > > question here can be done without modifying the base DT and getting in > > to the further discussions that doing so entails. The assertion is that > > the software discoverable bus here is sufficient to not need DT, so, OK, > > lets go. > > One of the reasons that I find this all so frustrating is that it is > circular logic: > > 1. Device-tree bindings are controlled by Linux; U-Boot cannot upstream > bindings > 2. We can only have upstreamed bindings in any device tree > > We have invented a whole u-boot.dtsi feature in U-Boot to hold > modifications from Linux. Board vendors have been suffering with this > for years. > > It is not fair and it really needs to stop. I am doing what I can to > upstream some basic U-Boot bindings and I hope that will work and can > lead to a healthier relationship here. Yes, but this is a problem outside of that scope. The argument here is that one does not need a device tree node to work. So lets see just how clean and nice the code can be without what you and I have been insisting would lead to the cleanest result. -- Tom signature.asc Description: PGP signature
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
Hi Sudeep, On Thu, 19 Jan 2023 at 10:09, Sudeep Holla wrote: > > On Thu, Jan 19, 2023 at 11:57:44AM -0500, Tom Rini wrote: > > > > But it's also true that at run-time, within U-Boot, we can modify the > > device tree we have, with live tree yes? So, the whole series in > > question here can be done without modifying the base DT and getting in > > to the further discussions that doing so entails. The assertion is that > > the software discoverable bus here is sufficient to not need DT, so, OK, > > lets go. > > OK, may be I am not up-to-date on the U-Boot. IIUC, the modifications > done in the DT by U-Boot is mostly for consumption by the next stage > loader/OS and not for self-consumption. But if it is for self consumption, > then good. It helps especially for the subnodes(as Simon referred) or the > partitions that can be discovered at run-time using FF-A interface. It's really just dodging the issue though, because you need a compatible string and you might as well add it to the DT in the source as do it at runtime. > > As mentioned I am not again DT, it is just not needed and especially > for subnodes it could result in inconsistency b/w what is in DT and > what the firmware provides. As mentioned in previous response, having a > simple node that Simon provided as example earlier is fine by me if that > is the only option to make progress as I just feel it is redundant and > one can say not scalable(but that is debatable again 😄). Gosh, how many of these things are you going to add? I believe the inconsistency argument is dealt with by the bind/probe explanation. It may be redundant in Linux but I doubt it would hurt there either. > > In short, I am not concerned about having simple node, just don't like > to see entire FF-A bus enumerated in DT as subnodes for reasons mentioned > already. Fair enough, and that is agreed on my side. I'll note that with PCI we sometimes do add nodes in order to provide parameters to the driver, there being no other sensible way to do this in U-Boot, for example: https://github.com/u-boot/u-boot/blob/master/arch/x86/dts/chromebook_coral.dts#L183 Regards, Simon
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Thu, Jan 19, 2023 at 05:09:45PM +, Sudeep Holla wrote: > On Thu, Jan 19, 2023 at 11:57:44AM -0500, Tom Rini wrote: > > > > But it's also true that at run-time, within U-Boot, we can modify the > > device tree we have, with live tree yes? So, the whole series in > > question here can be done without modifying the base DT and getting in > > to the further discussions that doing so entails. The assertion is that > > the software discoverable bus here is sufficient to not need DT, so, OK, > > lets go. > > OK, may be I am not up-to-date on the U-Boot. IIUC, the modifications > done in the DT by U-Boot is mostly for consumption by the next stage > loader/OS and not for self-consumption. But if it is for self consumption, > then good. It helps especially for the subnodes(as Simon referred) or the > partitions that can be discovered at run-time using FF-A interface. > > As mentioned I am not again DT, it is just not needed and especially > for subnodes it could result in inconsistency b/w what is in DT and > what the firmware provides. As mentioned in previous response, having a > simple node that Simon provided as example earlier is fine by me if that > is the only option to make progress as I just feel it is redundant and > one can say not scalable(but that is debatable again 😄). > > In short, I am not concerned about having simple node, just don't like > to see entire FF-A bus enumerated in DT as subnodes for reasons mentioned > already. So there's two parts to this discussion. The first of which has been, do Simon and I agree with the direction of defining a software discoverable bus rather than using device tree to describe it. To which the answer is no, and also neither camp seems likely to convince the other. The second part of the discussion is what can we practically do about it. To which the answer is that I believe the live tree support (see https://u-boot.readthedocs.io/en/latest/develop/driver-model/livetree.html) should allow for the FF-A bus support to be integrated with U-Boot, without having to add something to the base device tree. And we can evaluate down the line if our fears (or at least mine) were unfounded or not. -- Tom signature.asc Description: PGP signature
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
Hi Tom, On Thu, 19 Jan 2023 at 09:57, Tom Rini wrote: > > On Thu, Jan 19, 2023 at 09:54:29AM -0700, Simon Glass wrote: > > Hi Sudeep, > > > > On Thu, 19 Jan 2023 at 09:46, Sudeep Holla wrote: > > > > > > Hi Abdellatif, > > > > > > On Thu, Jan 19, 2023 at 04:31:57PM +, Abdellatif El Khlifi wrote: > > > > > > > > Hi Simon, Tom, > > > > > > > > The FF-A transport is a SW bus and is not associated to any HW > > > > peripheral or > > > > undiscoverable base address. > > > > > > > > There is only 1 way of discovering the FF-A bus and it's through the > > > > FF-A SW > > > > interfaces. The FF-A spec [1] describes this in details. > > > > > > > > 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. > > > > > > > > > > Thanks for the details. But IIRC this discussion is not about the FF-A bus > > > and device(partitions) discovery, but the support for FF-A itself. The > > > discussion is about where to have a device node to represent the > > > existence of > > > FF-A support on a platform. If we are talking about individual partitions > > > (devices) in the device tree, then that is pure stupidity as it goes out > > > of since with the firmware the moment a partition is added or removed in > > > the firmware. > > > > > > IIUC, the whole discussion was around whether to use FFA_VERSION as the > > > discovery mechanism for existence of FF-A support on a platform or you > > > have a device node to specify the same. > > > > No, with respect, that is not quite the situation here. > > > > > > > > Just to be clear, even if it is decided to add a device node, the > > > FFA_VERSION must be used to detect the presence of FF-A support and > > > return error otherwise. DT node presence is just to satisfy the design > > > and must be treated as no auto-confirmation for the presence of FF-A > > > support. We are just arguing the device node presence is just redundant, > > > but as mentioned before it is up to U-Boot community to make a call on > > > what is best. > > > > U-Boot driver model design already supports this. You can have a > > device that binds (from DT) but will not probe because it is not > > present / wrong version. Perhaps this was missed in the conversion to > > Linux: > > > > https://u-boot.readthedocs.io/en/latest/develop/driver-model/design.html#driver-lifecycle > > > > So there is nothing clever needed here at all and anything you do just > > adds confusion and bad precedent. > > But it's also true that at run-time, within U-Boot, we can modify the > device tree we have, with live tree yes? So, the whole series in > question here can be done without modifying the base DT and getting in > to the further discussions that doing so entails. The assertion is that > the software discoverable bus here is sufficient to not need DT, so, OK, > lets go. One of the reasons that I find this all so frustrating is that it is circular logic: 1. Device-tree bindings are controlled by Linux; U-Boot cannot upstream bindings 2. We can only have upstreamed bindings in any device tree We have invented a whole u-boot.dtsi feature in U-Boot to hold modifications from Linux. Board vendors have been suffering with this for years. It is not fair and it really needs to stop. I am doing what I can to upstream some basic U-Boot bindings and I hope that will work and can lead to a healthier relationship here. Regards, Simon
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Thu, Jan 19, 2023 at 11:57:44AM -0500, Tom Rini wrote: > > But it's also true that at run-time, within U-Boot, we can modify the > device tree we have, with live tree yes? So, the whole series in > question here can be done without modifying the base DT and getting in > to the further discussions that doing so entails. The assertion is that > the software discoverable bus here is sufficient to not need DT, so, OK, > lets go. OK, may be I am not up-to-date on the U-Boot. IIUC, the modifications done in the DT by U-Boot is mostly for consumption by the next stage loader/OS and not for self-consumption. But if it is for self consumption, then good. It helps especially for the subnodes(as Simon referred) or the partitions that can be discovered at run-time using FF-A interface. As mentioned I am not again DT, it is just not needed and especially for subnodes it could result in inconsistency b/w what is in DT and what the firmware provides. As mentioned in previous response, having a simple node that Simon provided as example earlier is fine by me if that is the only option to make progress as I just feel it is redundant and one can say not scalable(but that is debatable again 😄). In short, I am not concerned about having simple node, just don't like to see entire FF-A bus enumerated in DT as subnodes for reasons mentioned already. -- Regards, Sudeep
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Thu, Jan 19, 2023 at 09:54:29AM -0700, Simon Glass wrote: > Hi Sudeep, > > On Thu, 19 Jan 2023 at 09:46, Sudeep Holla wrote: > > > > Thanks for the details. But IIRC this discussion is not about the FF-A bus > > and device(partitions) discovery, but the support for FF-A itself. The > > discussion is about where to have a device node to represent the existence > > of > > FF-A support on a platform. If we are talking about individual partitions > > (devices) in the device tree, then that is pure stupidity as it goes out > > of since with the firmware the moment a partition is added or removed in > > the firmware. > > > > IIUC, the whole discussion was around whether to use FFA_VERSION as the > > discovery mechanism for existence of FF-A support on a platform or you > > have a device node to specify the same. > > No, with respect, that is not quite the situation here. > Hmm, not sure what you mean by that. Based on your earlier response, I thought we are in agreement but you sound to differ here. Am I missing something ? > > > > Just to be clear, even if it is decided to add a device node, the > > FFA_VERSION must be used to detect the presence of FF-A support and > > return error otherwise. DT node presence is just to satisfy the design > > and must be treated as no auto-confirmation for the presence of FF-A > > support. We are just arguing the device node presence is just redundant, > > but as mentioned before it is up to U-Boot community to make a call on > > what is best. > > U-Boot driver model design already supports this. You can have a > device that binds (from DT) but will not probe because it is not > present / wrong version. Perhaps this was missed in the conversion to > Linux: > > https://u-boot.readthedocs.io/en/latest/develop/driver-model/design.html#driver-lifecycle > > So there is nothing clever needed here at all and anything you do just > adds confusion and bad precedent. > OK, I will give that a read. -- Regards, Sudeep
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Thu, Jan 19, 2023 at 09:54:29AM -0700, Simon Glass wrote: > Hi Sudeep, > > On Thu, 19 Jan 2023 at 09:46, Sudeep Holla wrote: > > > > Hi Abdellatif, > > > > On Thu, Jan 19, 2023 at 04:31:57PM +, Abdellatif El Khlifi wrote: > > > > > > Hi Simon, Tom, > > > > > > The FF-A transport is a SW bus and is not associated to any HW peripheral > > > or > > > undiscoverable base address. > > > > > > There is only 1 way of discovering the FF-A bus and it's through the FF-A > > > SW > > > interfaces. The FF-A spec [1] describes this in details. > > > > > > 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. > > > > > > > Thanks for the details. But IIRC this discussion is not about the FF-A bus > > and device(partitions) discovery, but the support for FF-A itself. The > > discussion is about where to have a device node to represent the existence > > of > > FF-A support on a platform. If we are talking about individual partitions > > (devices) in the device tree, then that is pure stupidity as it goes out > > of since with the firmware the moment a partition is added or removed in > > the firmware. > > > > IIUC, the whole discussion was around whether to use FFA_VERSION as the > > discovery mechanism for existence of FF-A support on a platform or you > > have a device node to specify the same. > > No, with respect, that is not quite the situation here. > > > > > Just to be clear, even if it is decided to add a device node, the > > FFA_VERSION must be used to detect the presence of FF-A support and > > return error otherwise. DT node presence is just to satisfy the design > > and must be treated as no auto-confirmation for the presence of FF-A > > support. We are just arguing the device node presence is just redundant, > > but as mentioned before it is up to U-Boot community to make a call on > > what is best. > > U-Boot driver model design already supports this. You can have a > device that binds (from DT) but will not probe because it is not > present / wrong version. Perhaps this was missed in the conversion to > Linux: > > https://u-boot.readthedocs.io/en/latest/develop/driver-model/design.html#driver-lifecycle > > So there is nothing clever needed here at all and anything you do just > adds confusion and bad precedent. But it's also true that at run-time, within U-Boot, we can modify the device tree we have, with live tree yes? So, the whole series in question here can be done without modifying the base DT and getting in to the further discussions that doing so entails. The assertion is that the software discoverable bus here is sufficient to not need DT, so, OK, lets go. -- Tom signature.asc Description: PGP signature
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
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)
> 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
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
Hi Sudeep, On Thu, 19 Jan 2023 at 09:46, Sudeep Holla wrote: > > Hi Abdellatif, > > On Thu, Jan 19, 2023 at 04:31:57PM +, Abdellatif El Khlifi wrote: > > > > Hi Simon, Tom, > > > > The FF-A transport is a SW bus and is not associated to any HW peripheral or > > undiscoverable base address. > > > > There is only 1 way of discovering the FF-A bus and it's through the FF-A SW > > interfaces. The FF-A spec [1] describes this in details. > > > > 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. > > > > Thanks for the details. But IIRC this discussion is not about the FF-A bus > and device(partitions) discovery, but the support for FF-A itself. The > discussion is about where to have a device node to represent the existence of > FF-A support on a platform. If we are talking about individual partitions > (devices) in the device tree, then that is pure stupidity as it goes out > of since with the firmware the moment a partition is added or removed in > the firmware. > > IIUC, the whole discussion was around whether to use FFA_VERSION as the > discovery mechanism for existence of FF-A support on a platform or you > have a device node to specify the same. No, with respect, that is not quite the situation here. > > Just to be clear, even if it is decided to add a device node, the > FFA_VERSION must be used to detect the presence of FF-A support and > return error otherwise. DT node presence is just to satisfy the design > and must be treated as no auto-confirmation for the presence of FF-A > support. We are just arguing the device node presence is just redundant, > but as mentioned before it is up to U-Boot community to make a call on > what is best. U-Boot driver model design already supports this. You can have a device that binds (from DT) but will not probe because it is not present / wrong version. Perhaps this was missed in the conversion to Linux: https://u-boot.readthedocs.io/en/latest/develop/driver-model/design.html#driver-lifecycle So there is nothing clever needed here at all and anything you do just adds confusion and bad precedent. Regards, Simon
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
Hi Abdellatif, On Thu, Jan 19, 2023 at 04:31:57PM +, Abdellatif El Khlifi wrote: > > Hi Simon, Tom, > > The FF-A transport is a SW bus and is not associated to any HW peripheral or > undiscoverable base address. > > There is only 1 way of discovering the FF-A bus and it's through the FF-A SW > interfaces. The FF-A spec [1] describes this in details. > > 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. > Thanks for the details. But IIRC this discussion is not about the FF-A bus and device(partitions) discovery, but the support for FF-A itself. The discussion is about where to have a device node to represent the existence of FF-A support on a platform. If we are talking about individual partitions (devices) in the device tree, then that is pure stupidity as it goes out of since with the firmware the moment a partition is added or removed in the firmware. IIUC, the whole discussion was around whether to use FFA_VERSION as the discovery mechanism for existence of FF-A support on a platform or you have a device node to specify the same. Just to be clear, even if it is decided to add a device node, the FFA_VERSION must be used to detect the presence of FF-A support and return error otherwise. DT node presence is just to satisfy the design and must be treated as no auto-confirmation for the presence of FF-A support. We are just arguing the device node presence is just redundant, but as mentioned before it is up to U-Boot community to make a call on what is best. -- Regards, Sudeep
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Thu, Jan 19, 2023 at 09:41:12AM -0700, Simon Glass wrote:
> Hi Abdellatif,
>
> On Thu, 19 Jan 2023 at 09:32, Abdellatif El Khlifi
> wrote:
> >
> > On Wed, Jan 18, 2023 at 08:59:32AM -0500, Tom Rini wrote:
> > > On Wed, Jan 18, 2023 at 01:46:54PM +, Sudeep Holla wrote:
> > > > On Wed, Jan 18, 2023 at 12:49 PM Tom Rini wrote:
> > > >
> > > > >
> > > > > I guess the problem comes down to, can we have one discovery method
> > > > > that
> > > > > everyone shares, or do we have to let everyone invent a new discovery
> > > > > method every time?
> > > >
> > > >
> > > > No one needs to invent any discovery method every time if the firmware
> > > > specification
> > > > provides one and as Rob mentioned many times in the thread, all new
> > > > firmware
> > > > specification must provide one and we are trying to make sure that is
> > > > the case with all new
> > > > specs from Arm.
> > > >
> > > >
> > > > > FF-A, Op-tee, U-Boot, coreboot, barebox (and
> > > > > everyone else I'm unintentionally forgetting) could just discover
> > > > > these
> > > > > things via device tree.
> > > >
> > > >
> > > > I leave that to the individual projects to decide and agree but
> > > > fundamentally if
> > > > the specification provides a way to discover, not sure why we are even
> > > > discussing
> > > > an alternative method here.
> > > >
> > > >
> > > > > Or, we could all write our own code to perform
> > > > > the discovery.
> > > >
> > > >
> > > > For what reason ? I can understand if there is no discovery mechanism
> > > > but
> > > > that's not the
> > > > case in $subject.
> > > >
> > > >
> > > > > And when RISC-V comes along with similar functionality,
> > > > > we could probe their device tree and see they've implemented the same
> > > > > concept, but a little differently, but still have the discovery
> > > > > portion
> > > > > be in the device tree. To which it sounds like your answer is "not in
> > > > > the device tree".
> > > > >
> > > >
> > > > I see U-boot seem to have made a decision to create DT node for each and
> > > > everything
> > > > that needs to be added to DM which seems bit unfortunate but I don't
> > > > understand the
> > > > history/motive/background for it but I respect the decision if it is
> > > > already made.
> > > >
> > > > These firmware interfaces are standard on all Arm platforms and can be
> > > > discovered
> > > > based on PSCI/SMCCC. Not using the same and use DT node needs
> > > > unnecessary
> > > > addition of DT nodes for all the f/w i/f on all the platforms that need
> > > > the
> > > > support when
> > > > one can be just discovered.
> > > >
> > > > Sorry for the sudden appearance on this thread, I was avoiding getting
> > > > into
> > > > this but thought
> > > > I will at least express my opinion and also the way the firmware
> > > > specifications from Arm is
> > > > expected to be evolved from now on. With that I will leave it to you and
> > > > other U-boot
> > > > maintainers and the community in general to decide the right course in
> > > > this
> > > > case.
> > >
> > > To be clear, if the position is that "this is what everyone else will
> > > use, really" then yes, we'll follow this in U-Boot.
> >
> > Hi Simon, Tom,
> >
> > The FF-A transport is a SW bus and is not associated to any HW peripheral or
> > undiscoverable base address.
> >
> > There is only 1 way of discovering the FF-A bus and it's through the FF-A SW
> > interfaces. The FF-A spec [1] describes this in details.
>
> 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).
>
> 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";
> };
No, we don't need a DT node here. Everyone else is insisting that we can
solve the problems without it. So, lets go ahead and prove it. The
approach they're describing can be integrated without a device tree
node, in to the rest of the framework we have.
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
Hi Abdellatif,
On Thu, 19 Jan 2023 at 09:32, Abdellatif El Khlifi
wrote:
>
> On Wed, Jan 18, 2023 at 08:59:32AM -0500, Tom Rini wrote:
> > On Wed, Jan 18, 2023 at 01:46:54PM +, Sudeep Holla wrote:
> > > On Wed, Jan 18, 2023 at 12:49 PM Tom Rini wrote:
> > >
> > > >
> > > > I guess the problem comes down to, can we have one discovery method that
> > > > everyone shares, or do we have to let everyone invent a new discovery
> > > > method every time?
> > >
> > >
> > > No one needs to invent any discovery method every time if the firmware
> > > specification
> > > provides one and as Rob mentioned many times in the thread, all new
> > > firmware
> > > specification must provide one and we are trying to make sure that is
> > > the case with all new
> > > specs from Arm.
> > >
> > >
> > > > FF-A, Op-tee, U-Boot, coreboot, barebox (and
> > > > everyone else I'm unintentionally forgetting) could just discover these
> > > > things via device tree.
> > >
> > >
> > > I leave that to the individual projects to decide and agree but
> > > fundamentally if
> > > the specification provides a way to discover, not sure why we are even
> > > discussing
> > > an alternative method here.
> > >
> > >
> > > > Or, we could all write our own code to perform
> > > > the discovery.
> > >
> > >
> > > For what reason ? I can understand if there is no discovery mechanism but
> > > that's not the
> > > case in $subject.
> > >
> > >
> > > > And when RISC-V comes along with similar functionality,
> > > > we could probe their device tree and see they've implemented the same
> > > > concept, but a little differently, but still have the discovery portion
> > > > be in the device tree. To which it sounds like your answer is "not in
> > > > the device tree".
> > > >
> > >
> > > I see U-boot seem to have made a decision to create DT node for each and
> > > everything
> > > that needs to be added to DM which seems bit unfortunate but I don't
> > > understand the
> > > history/motive/background for it but I respect the decision if it is
> > > already made.
> > >
> > > These firmware interfaces are standard on all Arm platforms and can be
> > > discovered
> > > based on PSCI/SMCCC. Not using the same and use DT node needs unnecessary
> > > addition of DT nodes for all the f/w i/f on all the platforms that need
> > > the
> > > support when
> > > one can be just discovered.
> > >
> > > Sorry for the sudden appearance on this thread, I was avoiding getting
> > > into
> > > this but thought
> > > I will at least express my opinion and also the way the firmware
> > > specifications from Arm is
> > > expected to be evolved from now on. With that I will leave it to you and
> > > other U-boot
> > > maintainers and the community in general to decide the right course in
> > > this
> > > case.
> >
> > To be clear, if the position is that "this is what everyone else will
> > use, really" then yes, we'll follow this in U-Boot.
>
> Hi Simon, Tom,
>
> The FF-A transport is a SW bus and is not associated to any HW peripheral or
> undiscoverable base address.
>
> There is only 1 way of discovering the FF-A bus and it's through the FF-A SW
> interfaces. The FF-A spec [1] describes this in details.
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).
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";
};
>
> 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
docum
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Thu, Jan 19, 2023 at 04:31:57PM +, Abdellatif El Khlifi wrote: > On Wed, Jan 18, 2023 at 08:59:32AM -0500, Tom Rini wrote: > > On Wed, Jan 18, 2023 at 01:46:54PM +, Sudeep Holla wrote: > > > On Wed, Jan 18, 2023 at 12:49 PM Tom Rini wrote: > > > > > > > > > > > I guess the problem comes down to, can we have one discovery method that > > > > everyone shares, or do we have to let everyone invent a new discovery > > > > method every time? > > > > > > > > > No one needs to invent any discovery method every time if the firmware > > > specification > > > provides one and as Rob mentioned many times in the thread, all new > > > firmware > > > specification must provide one and we are trying to make sure that is > > > the case with all new > > > specs from Arm. > > > > > > > > > > FF-A, Op-tee, U-Boot, coreboot, barebox (and > > > > everyone else I'm unintentionally forgetting) could just discover these > > > > things via device tree. > > > > > > > > > I leave that to the individual projects to decide and agree but > > > fundamentally if > > > the specification provides a way to discover, not sure why we are even > > > discussing > > > an alternative method here. > > > > > > > > > > Or, we could all write our own code to perform > > > > the discovery. > > > > > > > > > For what reason ? I can understand if there is no discovery mechanism but > > > that's not the > > > case in $subject. > > > > > > > > > > And when RISC-V comes along with similar functionality, > > > > we could probe their device tree and see they've implemented the same > > > > concept, but a little differently, but still have the discovery portion > > > > be in the device tree. To which it sounds like your answer is "not in > > > > the device tree". > > > > > > > > > > I see U-boot seem to have made a decision to create DT node for each and > > > everything > > > that needs to be added to DM which seems bit unfortunate but I don't > > > understand the > > > history/motive/background for it but I respect the decision if it is > > > already made. > > > > > > These firmware interfaces are standard on all Arm platforms and can be > > > discovered > > > based on PSCI/SMCCC. Not using the same and use DT node needs unnecessary > > > addition of DT nodes for all the f/w i/f on all the platforms that need > > > the > > > support when > > > one can be just discovered. > > > > > > Sorry for the sudden appearance on this thread, I was avoiding getting > > > into > > > this but thought > > > I will at least express my opinion and also the way the firmware > > > specifications from Arm is > > > expected to be evolved from now on. With that I will leave it to you and > > > other U-boot > > > maintainers and the community in general to decide the right course in > > > this > > > case. > > > > To be clear, if the position is that "this is what everyone else will > > use, really" then yes, we'll follow this in U-Boot. > > Hi Simon, Tom, > > The FF-A transport is a SW bus and is not associated to any HW peripheral or > undiscoverable base address. > > There is only 1 way of discovering the FF-A bus and it's through the FF-A SW > interfaces. The FF-A spec [1] describes this in details. > > 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 > > > Cheers > Abdellatif > > [1]: FF-A spec version 1.0, > https://developer.arm.com/documentation/den0077/latest/ > > [2] 2.8 Partition identification and discovery > > All FF-A components can discover the identities and properties of other > partitions through the FFA_PARTITION_INFO_GET > interface. Once discovered, the IDs must be used in the messaging > interfaces to identify the target of a message. > > [3] 4.2.2.2 Buffer discovery and setup > > This version of the Framework enables discovery and setup of RX/TX buffer > pairs between FF-A components as > follows. > > ... > > 2. An endpoint could allocate the b
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Wed, Jan 18, 2023 at 08:59:32AM -0500, Tom Rini wrote: > On Wed, Jan 18, 2023 at 01:46:54PM +, Sudeep Holla wrote: > > On Wed, Jan 18, 2023 at 12:49 PM Tom Rini wrote: > > > > > > > > I guess the problem comes down to, can we have one discovery method that > > > everyone shares, or do we have to let everyone invent a new discovery > > > method every time? > > > > > > No one needs to invent any discovery method every time if the firmware > > specification > > provides one and as Rob mentioned many times in the thread, all new firmware > > specification must provide one and we are trying to make sure that is > > the case with all new > > specs from Arm. > > > > > > > FF-A, Op-tee, U-Boot, coreboot, barebox (and > > > everyone else I'm unintentionally forgetting) could just discover these > > > things via device tree. > > > > > > I leave that to the individual projects to decide and agree but > > fundamentally if > > the specification provides a way to discover, not sure why we are even > > discussing > > an alternative method here. > > > > > > > Or, we could all write our own code to perform > > > the discovery. > > > > > > For what reason ? I can understand if there is no discovery mechanism but > > that's not the > > case in $subject. > > > > > > > And when RISC-V comes along with similar functionality, > > > we could probe their device tree and see they've implemented the same > > > concept, but a little differently, but still have the discovery portion > > > be in the device tree. To which it sounds like your answer is "not in > > > the device tree". > > > > > > > I see U-boot seem to have made a decision to create DT node for each and > > everything > > that needs to be added to DM which seems bit unfortunate but I don't > > understand the > > history/motive/background for it but I respect the decision if it is > > already made. > > > > These firmware interfaces are standard on all Arm platforms and can be > > discovered > > based on PSCI/SMCCC. Not using the same and use DT node needs unnecessary > > addition of DT nodes for all the f/w i/f on all the platforms that need the > > support when > > one can be just discovered. > > > > Sorry for the sudden appearance on this thread, I was avoiding getting into > > this but thought > > I will at least express my opinion and also the way the firmware > > specifications from Arm is > > expected to be evolved from now on. With that I will leave it to you and > > other U-boot > > maintainers and the community in general to decide the right course in this > > case. > > To be clear, if the position is that "this is what everyone else will > use, really" then yes, we'll follow this in U-Boot. Hi Simon, Tom, The FF-A transport is a SW bus and is not associated to any HW peripheral or undiscoverable base address. There is only 1 way of discovering the FF-A bus and it's through the FF-A SW interfaces. The FF-A spec [1] describes this in details. 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 Cheers Abdellatif [1]: FF-A spec version 1.0, https://developer.arm.com/documentation/den0077/latest/ [2] 2.8 Partition identification and discovery All FF-A components can discover the identities and properties of other partitions through the FFA_PARTITION_INFO_GET interface. Once discovered, the IDs must be used in the messaging interfaces to identify the target of a message. [3] 4.2.2.2 Buffer discovery and setup This version of the Framework enables discovery and setup of RX/TX buffer pairs between FF-A components as follows. ... 2. An endpoint could allocate the buffer pair and use the FFA_RXTX_MAP interface to map it with the Hypervisor or SPM as applicable. [4] 4.2.2.3 Buffer attributes An endpoint must discover the minimum size and alignment boundary for the RX/TX buffers by passing the function ID of the FFA_RXTX_MAP ABI as input in the FFA_FEATURES interface (see 8.2 FFA_FEATURES). [5] 6.1.1 Commo
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Wed, Jan 18, 2023 at 08:59:32AM -0500, Tom Rini wrote: > To be clear, if the position is that "this is what everyone else will > use, really" then yes, we'll follow this in U-Boot. Well, that's difficult question to provide an assertive answer TBH. I don't know all the projects using FF-A for example. I wasn't even aware of U-Boot plans for it even being part of the same company. So lots of things happen outside the company, in different community like this and mainly without even Arm or interested party's involvement. I guess you understand what I am trying to say here. I or even Arm as a company can only provide suggestions when requested or when interested developers like me happen to stumble across discussions like this. I can never give you assurance that "everyone else will follow our suggestion" unfortunately :(. -- Regards, Sudeep
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Wed, Jan 18, 2023 at 01:46:54PM +, Sudeep Holla wrote: > On Wed, Jan 18, 2023 at 12:49 PM Tom Rini wrote: > > > > > I guess the problem comes down to, can we have one discovery method that > > everyone shares, or do we have to let everyone invent a new discovery > > method every time? > > > No one needs to invent any discovery method every time if the firmware > specification > provides one and as Rob mentioned many times in the thread, all new firmware > specification must provide one and we are trying to make sure that is > the case with all new > specs from Arm. > > > > FF-A, Op-tee, U-Boot, coreboot, barebox (and > > everyone else I'm unintentionally forgetting) could just discover these > > things via device tree. > > > I leave that to the individual projects to decide and agree but > fundamentally if > the specification provides a way to discover, not sure why we are even > discussing > an alternative method here. > > > > Or, we could all write our own code to perform > > the discovery. > > > For what reason ? I can understand if there is no discovery mechanism but > that's not the > case in $subject. > > > > And when RISC-V comes along with similar functionality, > > we could probe their device tree and see they've implemented the same > > concept, but a little differently, but still have the discovery portion > > be in the device tree. To which it sounds like your answer is "not in > > the device tree". > > > > I see U-boot seem to have made a decision to create DT node for each and > everything > that needs to be added to DM which seems bit unfortunate but I don't > understand the > history/motive/background for it but I respect the decision if it is > already made. > > These firmware interfaces are standard on all Arm platforms and can be > discovered > based on PSCI/SMCCC. Not using the same and use DT node needs unnecessary > addition of DT nodes for all the f/w i/f on all the platforms that need the > support when > one can be just discovered. > > Sorry for the sudden appearance on this thread, I was avoiding getting into > this but thought > I will at least express my opinion and also the way the firmware > specifications from Arm is > expected to be evolved from now on. With that I will leave it to you and > other U-boot > maintainers and the community in general to decide the right course in this > case. To be clear, if the position is that "this is what everyone else will use, really" then yes, we'll follow this in U-Boot. -- Tom signature.asc Description: PGP signature
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Wed, Jan 18, 2023 at 12:49 PM Tom Rini wrote: > > I guess the problem comes down to, can we have one discovery method that > everyone shares, or do we have to let everyone invent a new discovery > method every time? No one needs to invent any discovery method every time if the firmware specification provides one and as Rob mentioned many times in the thread, all new firmware specification must provide one and we are trying to make sure that is the case with all new specs from Arm. > FF-A, Op-tee, U-Boot, coreboot, barebox (and > everyone else I'm unintentionally forgetting) could just discover these > things via device tree. I leave that to the individual projects to decide and agree but fundamentally if the specification provides a way to discover, not sure why we are even discussing an alternative method here. > Or, we could all write our own code to perform > the discovery. For what reason ? I can understand if there is no discovery mechanism but that's not the case in $subject. > And when RISC-V comes along with similar functionality, > we could probe their device tree and see they've implemented the same > concept, but a little differently, but still have the discovery portion > be in the device tree. To which it sounds like your answer is "not in > the device tree". > I see U-boot seem to have made a decision to create DT node for each and everything that needs to be added to DM which seems bit unfortunate but I don't understand the history/motive/background for it but I respect the decision if it is already made. These firmware interfaces are standard on all Arm platforms and can be discovered based on PSCI/SMCCC. Not using the same and use DT node needs unnecessary addition of DT nodes for all the f/w i/f on all the platforms that need the support when one can be just discovered. Sorry for the sudden appearance on this thread, I was avoiding getting into this but thought I will at least express my opinion and also the way the firmware specifications from Arm is expected to be evolved from now on. With that I will leave it to you and other U-boot maintainers and the community in general to decide the right course in this case.
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Tue, Jan 17, 2023 at 08:51:51PM -0600, Rob Herring wrote:
> On Thu, Jan 12, 2023 at 5:43 PM Simon Glass wrote:
> >
> > Hi Rob,
> >
> > On Wed, 11 Jan 2023 at 19:10, Rob Herring wrote:
> > >
> > > On Mon, Dec 19, 2022 at 1:21 PM Simon Glass wrote:
> > > >
> > > > Hi Abdellatif,
> > > >
> > > > On Mon, 19 Dec 2022 at 04:12, Abdellatif El Khlifi
> > > > wrote:
> > > > >
> > > > > On Mon, Dec 05, 2022 at 09:49:30AM -0600, Rob Herring wrote:
> > > > > > On Sun, Dec 4, 2022 at 1:22 PM Simon Glass
> > > > > > wrote:
> > > > > > >
> > > > > > > Hi Rob,
> > > > > > >
> > > > > > > On Tue, 29 Nov 2022 at 05:22, Rob Herring wrote:
> > > > > > > >
> > > > > > > > On Fri, Nov 25, 2022 at 3:18 PM Simon Glass
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > Hi Abdellatif,
> > > > > > > > >
> > > > > > > > > On Thu, 24 Nov 2022 at 06:21, Abdellatif El Khlifi
> > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > On Tue, Nov 22, 2022 at 07:09:16PM -0700, Simon Glass wrote:
> > > > > > > > > > > should be called 'priov' and should beHi Abdellatif,
> > > > > > > > > > >
> > > > > > > > >
> > > > > > > > > [..]
> > > > > > > > >
> > > > > > > > > > > > +/**
> > > > > > > > > > > > + * ffa_device_get - create, bind and probe the arm_ffa
> > > > > > > > > > > > device
> > > > > > > > > > > > + * @pdev: the address of a device pointer (to be
> > > > > > > > > > > > filled when the arm_ffa bus device is created
> > > > > > > > > > > > + * successfully)
> > > > > > > > > > > > + *
> > > > > > > > > > > > + * This function makes sure the arm_ffa device is
> > > > > > > > > > > > + * created, bound to this driver, probed and ready to
> > > > > > > > > > > > use.
> > > > > > > > > > > > + * Arm FF-A transport is implemented through a single
> > > > > > > > > > > > U-Boot
> > > > > > > > > > > > + * device managing the FF-A bus (arm_ffa).
> > > > > > > > > > > > + *
> > > > > > > > > > > > + * Return:
> > > > > > > > > > > > + *
> > > > > > > > > > > > + * 0 on success. Otherwise, failure
> > > > > > > > > > > > + */
> > > > > > > > > > > > +int ffa_device_get(struct udevice **pdev)
> > > > > > > > > > > > +{
> > > > > > > > > > > > + int ret;
> > > > > > > > > > > > + struct udevice *dev = NULL;
> > > > > > > > > > > > +
> > > > > > > > > > > > + ret = device_bind(dm_root(),
> > > > > > > > > > > > DM_DRIVER_GET(arm_ffa), FFA_DRV_NAME, NULL,
> > > > > > > > > > > > ofnode_null(),
> > > > > > > > > > > > + &dev);
> > > > > > > > > > >
> > > > > > > > > > > Please add a DT binding. Even if only temporary, we need
> > > > > > > > > > > something for this.
> > > > > > > > > >
> > > > > > > > > > Thanks for the feedback. I'm happy to address all the
> > > > > > > > > > comments.
> > > > > > > > > >
> > > > > > > > > > Regarding DT binding and FF-A discovery. We agreed with
> > > > > > > > > > Linaro and Rob Herring
> > > > > > > > > > about the following:
> > > > > > > > > >
> > > > > > > > > > - DT is only for what we failed to make discoverable. For
> > > > > > > > > > hardware, we're stuck
> > > > > > > > > > with it. We shouldn't repeat that for software
> > > > > > > > > > interfaces. This approach is
> > > > > > > > > > already applied in the FF-A kernel driver which comes
> > > > > > > > > > with no DT support and
> > > > > > > > > > discovers the bus with bus_register() API [1].
> > > > > > > > >
> > > > > > > > > This may be the UEFI view, but it is not how U-Boot works.
> > > > > > > > > This is not something we are 'stuck' with. It is how we
> > > > > > > > > define what is present on a device. This is how the PCI bus
> > > > > > > > > works in U-Boot. It is best practice in U-Boot to use the
> > > > > > > > > device tree to make this things visible and configurable.
> > > > > > > > > Unlike with Linux there is no other way to provide
> > > > > > > > > configuration needed by these devices.
> > > > > > > >
> > > > > > > > Where do you get UEFI out of this?
> > > > > > >
> > > > > > > I assume it was UEFI as there was no discussion about this in
> > > > > > > U-Boot.
> > > > > > > Which firmware project was consulted about this?
> > > > > > >
> > > > > > > >
> > > > > > > > It is the discoverability of hardware that is fixed (and we are
> > > > > > > > stuck
> > > > > > > > with). We can't change hardware. The disoverability may be PCI
> > > > > > > > VID/PID, USB device descriptors, or nothing. We only use DT
> > > > > > > > when those
> > > > > > > > are not sufficient. For a software interface, there is no
> > > > > > > > reason to
> > > > > > > > make them non-discoverable as the interface can be fixed (at
> > > > > > > > least for
> > > > > > > > new things like FF-A).
> > > > > > >
> > > > > > > Here I am talking about the controller itself, the top-level node
> > > > > > > in
> > > > > > > the device tree. For PCI this is a device tree node and it should
> > > > > > > be
> > > > > > > the same here. So I am no
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Tue, Jan 17, 2023 at 8:04 AM Tom Rini wrote:
>
> On Mon, Jan 16, 2023 at 01:23:53PM +, Abdellatif El Khlifi wrote:
> > On Fri, Jan 13, 2023 at 11:00:28AM -0700, Simon Glass wrote:
> > > Hi Abdellatif,
> > >
> > > On Fri, 13 Jan 2023 at 03:44, Abdellatif El Khlifi
> > > wrote:
> > > >
> > > > On Thu, Jan 12, 2023 at 04:43:32PM -0700, Simon Glass wrote:
> > > > > Hi Rob,
> > > > >
> > > > > On Wed, 11 Jan 2023 at 19:10, Rob Herring wrote:
> > > > > >
> > > > > > On Mon, Dec 19, 2022 at 1:21 PM Simon Glass
> > > > > > wrote:
> > > > > > >
> > > > > > > Hi Abdellatif,
> > > > > > >
> > > > > > > On Mon, 19 Dec 2022 at 04:12, Abdellatif El Khlifi
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > On Mon, Dec 05, 2022 at 09:49:30AM -0600, Rob Herring wrote:
> > > > > > > > > On Sun, Dec 4, 2022 at 1:22 PM Simon Glass
> > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > Hi Rob,
> > > > > > > > > >
> > > > > > > > > > On Tue, 29 Nov 2022 at 05:22, Rob Herring
> > > > > > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > > On Fri, Nov 25, 2022 at 3:18 PM Simon Glass
> > > > > > > > > > > wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > Hi Abdellatif,
> > > > > > > > > > > >
> > > > > > > > > > > > On Thu, 24 Nov 2022 at 06:21, Abdellatif El Khlifi
> > > > > > > > > > > > wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > On Tue, Nov 22, 2022 at 07:09:16PM -0700, Simon Glass
> > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > > should be called 'priov' and should beHi
> > > > > > > > > > > > > > Abdellatif,
> > > > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > [..]
> > > > > > > > > > > >
> > > > > > > > > > > > > > > +/**
> > > > > > > > > > > > > > > + * ffa_device_get - create, bind and probe the
> > > > > > > > > > > > > > > arm_ffa device
> > > > > > > > > > > > > > > + * @pdev: the address of a device pointer (to be
> > > > > > > > > > > > > > > filled when the arm_ffa bus device is created
> > > > > > > > > > > > > > > + * successfully)
> > > > > > > > > > > > > > > + *
> > > > > > > > > > > > > > > + * This function makes sure the arm_ffa device is
> > > > > > > > > > > > > > > + * created, bound to this driver, probed and
> > > > > > > > > > > > > > > ready to use.
> > > > > > > > > > > > > > > + * Arm FF-A transport is implemented through a
> > > > > > > > > > > > > > > single U-Boot
> > > > > > > > > > > > > > > + * device managing the FF-A bus (arm_ffa).
> > > > > > > > > > > > > > > + *
> > > > > > > > > > > > > > > + * Return:
> > > > > > > > > > > > > > > + *
> > > > > > > > > > > > > > > + * 0 on success. Otherwise, failure
> > > > > > > > > > > > > > > + */
> > > > > > > > > > > > > > > +int ffa_device_get(struct udevice **pdev)
> > > > > > > > > > > > > > > +{
> > > > > > > > > > > > > > > + int ret;
> > > > > > > > > > > > > > > + struct udevice *dev = NULL;
> > > > > > > > > > > > > > > +
> > > > > > > > > > > > > > > + ret = device_bind(dm_root(),
> > > > > > > > > > > > > > > DM_DRIVER_GET(arm_ffa), FFA_DRV_NAME, NULL,
> > > > > > > > > > > > > > > ofnode_null(),
> > > > > > > > > > > > > > > + &dev);
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Please add a DT binding. Even if only temporary, we
> > > > > > > > > > > > > > need something for this.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Thanks for the feedback. I'm happy to address all the
> > > > > > > > > > > > > comments.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Regarding DT binding and FF-A discovery. We agreed
> > > > > > > > > > > > > with Linaro and Rob Herring
> > > > > > > > > > > > > about the following:
> > > > > > > > > > > > >
> > > > > > > > > > > > > - DT is only for what we failed to make discoverable.
> > > > > > > > > > > > > For hardware, we're stuck
> > > > > > > > > > > > > with it. We shouldn't repeat that for software
> > > > > > > > > > > > > interfaces. This approach is
> > > > > > > > > > > > > already applied in the FF-A kernel driver which
> > > > > > > > > > > > > comes with no DT support and
> > > > > > > > > > > > > discovers the bus with bus_register() API [1].
> > > > > > > > > > > >
> > > > > > > > > > > > This may be the UEFI view, but it is not how U-Boot
> > > > > > > > > > > > works. This is not something we are 'stuck' with. It is
> > > > > > > > > > > > how we define what is present on a device. This is how
> > > > > > > > > > > > the PCI bus works in U-Boot. It is best practice in
> > > > > > > > > > > > U-Boot to use the device tree to make this things
> > > > > > > > > > > > visible and configurable. Unlike with Linux there is no
> > > > > > > > > > > > other way to provide configuration needed by these
> > > > > > > > > > > > devices.
> > > > > > > > > > >
> > > > > > > > > > > Where do you get UEFI out of this?
> > > > > > > > > >
> > > > > > > > > > I assume it was UEFI as there was no discussion about this
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Thu, Jan 12, 2023 at 5:43 PM Simon Glass wrote:
>
> Hi Rob,
>
> On Wed, 11 Jan 2023 at 19:10, Rob Herring wrote:
> >
> > On Mon, Dec 19, 2022 at 1:21 PM Simon Glass wrote:
> > >
> > > Hi Abdellatif,
> > >
> > > On Mon, 19 Dec 2022 at 04:12, Abdellatif El Khlifi
> > > wrote:
> > > >
> > > > On Mon, Dec 05, 2022 at 09:49:30AM -0600, Rob Herring wrote:
> > > > > On Sun, Dec 4, 2022 at 1:22 PM Simon Glass wrote:
> > > > > >
> > > > > > Hi Rob,
> > > > > >
> > > > > > On Tue, 29 Nov 2022 at 05:22, Rob Herring wrote:
> > > > > > >
> > > > > > > On Fri, Nov 25, 2022 at 3:18 PM Simon Glass
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > Hi Abdellatif,
> > > > > > > >
> > > > > > > > On Thu, 24 Nov 2022 at 06:21, Abdellatif El Khlifi
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > On Tue, Nov 22, 2022 at 07:09:16PM -0700, Simon Glass wrote:
> > > > > > > > > > should be called 'priov' and should beHi Abdellatif,
> > > > > > > > > >
> > > > > > > >
> > > > > > > > [..]
> > > > > > > >
> > > > > > > > > > > +/**
> > > > > > > > > > > + * ffa_device_get - create, bind and probe the arm_ffa
> > > > > > > > > > > device
> > > > > > > > > > > + * @pdev: the address of a device pointer (to be filled
> > > > > > > > > > > when the arm_ffa bus device is created
> > > > > > > > > > > + * successfully)
> > > > > > > > > > > + *
> > > > > > > > > > > + * This function makes sure the arm_ffa device is
> > > > > > > > > > > + * created, bound to this driver, probed and ready to
> > > > > > > > > > > use.
> > > > > > > > > > > + * Arm FF-A transport is implemented through a single
> > > > > > > > > > > U-Boot
> > > > > > > > > > > + * device managing the FF-A bus (arm_ffa).
> > > > > > > > > > > + *
> > > > > > > > > > > + * Return:
> > > > > > > > > > > + *
> > > > > > > > > > > + * 0 on success. Otherwise, failure
> > > > > > > > > > > + */
> > > > > > > > > > > +int ffa_device_get(struct udevice **pdev)
> > > > > > > > > > > +{
> > > > > > > > > > > + int ret;
> > > > > > > > > > > + struct udevice *dev = NULL;
> > > > > > > > > > > +
> > > > > > > > > > > + ret = device_bind(dm_root(),
> > > > > > > > > > > DM_DRIVER_GET(arm_ffa), FFA_DRV_NAME, NULL, ofnode_null(),
> > > > > > > > > > > + &dev);
> > > > > > > > > >
> > > > > > > > > > Please add a DT binding. Even if only temporary, we need
> > > > > > > > > > something for this.
> > > > > > > > >
> > > > > > > > > Thanks for the feedback. I'm happy to address all the
> > > > > > > > > comments.
> > > > > > > > >
> > > > > > > > > Regarding DT binding and FF-A discovery. We agreed with
> > > > > > > > > Linaro and Rob Herring
> > > > > > > > > about the following:
> > > > > > > > >
> > > > > > > > > - DT is only for what we failed to make discoverable. For
> > > > > > > > > hardware, we're stuck
> > > > > > > > > with it. We shouldn't repeat that for software interfaces.
> > > > > > > > > This approach is
> > > > > > > > > already applied in the FF-A kernel driver which comes with
> > > > > > > > > no DT support and
> > > > > > > > > discovers the bus with bus_register() API [1].
> > > > > > > >
> > > > > > > > This may be the UEFI view, but it is not how U-Boot works. This
> > > > > > > > is not something we are 'stuck' with. It is how we define what
> > > > > > > > is present on a device. This is how the PCI bus works in
> > > > > > > > U-Boot. It is best practice in U-Boot to use the device tree to
> > > > > > > > make this things visible and configurable. Unlike with Linux
> > > > > > > > there is no other way to provide configuration needed by these
> > > > > > > > devices.
> > > > > > >
> > > > > > > Where do you get UEFI out of this?
> > > > > >
> > > > > > I assume it was UEFI as there was no discussion about this in
> > > > > > U-Boot.
> > > > > > Which firmware project was consulted about this?
> > > > > >
> > > > > > >
> > > > > > > It is the discoverability of hardware that is fixed (and we are
> > > > > > > stuck
> > > > > > > with). We can't change hardware. The disoverability may be PCI
> > > > > > > VID/PID, USB device descriptors, or nothing. We only use DT when
> > > > > > > those
> > > > > > > are not sufficient. For a software interface, there is no reason
> > > > > > > to
> > > > > > > make them non-discoverable as the interface can be fixed (at
> > > > > > > least for
> > > > > > > new things like FF-A).
> > > > > >
> > > > > > Here I am talking about the controller itself, the top-level node in
> > > > > > the device tree. For PCI this is a device tree node and it should be
> > > > > > the same here. So I am not saying that the devices on the bus need
> > > > > > to
> > > > > > be in the device tree (that can be optional, but may be useful in
> > > > > > some
> > > > > > situations where it is status and known).
> > > > >
> > > > > Sure, the PCI host bridges are not discoverable, have a bunch of
> > > > > resources, and do need to be in DT. The do
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Mon, Jan 16, 2023 at 01:23:53PM +, Abdellatif El Khlifi wrote:
> On Fri, Jan 13, 2023 at 11:00:28AM -0700, Simon Glass wrote:
> > Hi Abdellatif,
> >
> > On Fri, 13 Jan 2023 at 03:44, Abdellatif El Khlifi
> > wrote:
> > >
> > > On Thu, Jan 12, 2023 at 04:43:32PM -0700, Simon Glass wrote:
> > > > Hi Rob,
> > > >
> > > > On Wed, 11 Jan 2023 at 19:10, Rob Herring wrote:
> > > > >
> > > > > On Mon, Dec 19, 2022 at 1:21 PM Simon Glass wrote:
> > > > > >
> > > > > > Hi Abdellatif,
> > > > > >
> > > > > > On Mon, 19 Dec 2022 at 04:12, Abdellatif El Khlifi
> > > > > > wrote:
> > > > > > >
> > > > > > > On Mon, Dec 05, 2022 at 09:49:30AM -0600, Rob Herring wrote:
> > > > > > > > On Sun, Dec 4, 2022 at 1:22 PM Simon Glass
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > Hi Rob,
> > > > > > > > >
> > > > > > > > > On Tue, 29 Nov 2022 at 05:22, Rob Herring
> > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > On Fri, Nov 25, 2022 at 3:18 PM Simon Glass
> > > > > > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > > Hi Abdellatif,
> > > > > > > > > > >
> > > > > > > > > > > On Thu, 24 Nov 2022 at 06:21, Abdellatif El Khlifi
> > > > > > > > > > > wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > On Tue, Nov 22, 2022 at 07:09:16PM -0700, Simon Glass
> > > > > > > > > > > > wrote:
> > > > > > > > > > > > > should be called 'priov' and should beHi Abdellatif,
> > > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > [..]
> > > > > > > > > > >
> > > > > > > > > > > > > > +/**
> > > > > > > > > > > > > > + * ffa_device_get - create, bind and probe the
> > > > > > > > > > > > > > arm_ffa device
> > > > > > > > > > > > > > + * @pdev: the address of a device pointer (to be
> > > > > > > > > > > > > > filled when the arm_ffa bus device is created
> > > > > > > > > > > > > > + * successfully)
> > > > > > > > > > > > > > + *
> > > > > > > > > > > > > > + * This function makes sure the arm_ffa device is
> > > > > > > > > > > > > > + * created, bound to this driver, probed and ready
> > > > > > > > > > > > > > to use.
> > > > > > > > > > > > > > + * Arm FF-A transport is implemented through a
> > > > > > > > > > > > > > single U-Boot
> > > > > > > > > > > > > > + * device managing the FF-A bus (arm_ffa).
> > > > > > > > > > > > > > + *
> > > > > > > > > > > > > > + * Return:
> > > > > > > > > > > > > > + *
> > > > > > > > > > > > > > + * 0 on success. Otherwise, failure
> > > > > > > > > > > > > > + */
> > > > > > > > > > > > > > +int ffa_device_get(struct udevice **pdev)
> > > > > > > > > > > > > > +{
> > > > > > > > > > > > > > + int ret;
> > > > > > > > > > > > > > + struct udevice *dev = NULL;
> > > > > > > > > > > > > > +
> > > > > > > > > > > > > > + ret = device_bind(dm_root(),
> > > > > > > > > > > > > > DM_DRIVER_GET(arm_ffa), FFA_DRV_NAME, NULL,
> > > > > > > > > > > > > > ofnode_null(),
> > > > > > > > > > > > > > + &dev);
> > > > > > > > > > > > >
> > > > > > > > > > > > > Please add a DT binding. Even if only temporary, we
> > > > > > > > > > > > > need something for this.
> > > > > > > > > > > >
> > > > > > > > > > > > Thanks for the feedback. I'm happy to address all the
> > > > > > > > > > > > comments.
> > > > > > > > > > > >
> > > > > > > > > > > > Regarding DT binding and FF-A discovery. We agreed with
> > > > > > > > > > > > Linaro and Rob Herring
> > > > > > > > > > > > about the following:
> > > > > > > > > > > >
> > > > > > > > > > > > - DT is only for what we failed to make discoverable.
> > > > > > > > > > > > For hardware, we're stuck
> > > > > > > > > > > > with it. We shouldn't repeat that for software
> > > > > > > > > > > > interfaces. This approach is
> > > > > > > > > > > > already applied in the FF-A kernel driver which comes
> > > > > > > > > > > > with no DT support and
> > > > > > > > > > > > discovers the bus with bus_register() API [1].
> > > > > > > > > > >
> > > > > > > > > > > This may be the UEFI view, but it is not how U-Boot
> > > > > > > > > > > works. This is not something we are 'stuck' with. It is
> > > > > > > > > > > how we define what is present on a device. This is how
> > > > > > > > > > > the PCI bus works in U-Boot. It is best practice in
> > > > > > > > > > > U-Boot to use the device tree to make this things visible
> > > > > > > > > > > and configurable. Unlike with Linux there is no other way
> > > > > > > > > > > to provide configuration needed by these devices.
> > > > > > > > > >
> > > > > > > > > > Where do you get UEFI out of this?
> > > > > > > > >
> > > > > > > > > I assume it was UEFI as there was no discussion about this in
> > > > > > > > > U-Boot.
> > > > > > > > > Which firmware project was consulted about this?
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > It is the discoverability of hardware that is fixed (and we
> > > > > > > > > > are stuck
> > > > > > > > > > with). We can't change hardware. The disoverabilit
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Fri, Jan 13, 2023 at 11:00:28AM -0700, Simon Glass wrote:
> Hi Abdellatif,
>
> On Fri, 13 Jan 2023 at 03:44, Abdellatif El Khlifi
> wrote:
> >
> > On Thu, Jan 12, 2023 at 04:43:32PM -0700, Simon Glass wrote:
> > > Hi Rob,
> > >
> > > On Wed, 11 Jan 2023 at 19:10, Rob Herring wrote:
> > > >
> > > > On Mon, Dec 19, 2022 at 1:21 PM Simon Glass wrote:
> > > > >
> > > > > Hi Abdellatif,
> > > > >
> > > > > On Mon, 19 Dec 2022 at 04:12, Abdellatif El Khlifi
> > > > > wrote:
> > > > > >
> > > > > > On Mon, Dec 05, 2022 at 09:49:30AM -0600, Rob Herring wrote:
> > > > > > > On Sun, Dec 4, 2022 at 1:22 PM Simon Glass
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > Hi Rob,
> > > > > > > >
> > > > > > > > On Tue, 29 Nov 2022 at 05:22, Rob Herring
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > On Fri, Nov 25, 2022 at 3:18 PM Simon Glass
> > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > Hi Abdellatif,
> > > > > > > > > >
> > > > > > > > > > On Thu, 24 Nov 2022 at 06:21, Abdellatif El Khlifi
> > > > > > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > > On Tue, Nov 22, 2022 at 07:09:16PM -0700, Simon Glass
> > > > > > > > > > > wrote:
> > > > > > > > > > > > should be called 'priov' and should beHi Abdellatif,
> > > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > [..]
> > > > > > > > > >
> > > > > > > > > > > > > +/**
> > > > > > > > > > > > > + * ffa_device_get - create, bind and probe the
> > > > > > > > > > > > > arm_ffa device
> > > > > > > > > > > > > + * @pdev: the address of a device pointer (to be
> > > > > > > > > > > > > filled when the arm_ffa bus device is created
> > > > > > > > > > > > > + * successfully)
> > > > > > > > > > > > > + *
> > > > > > > > > > > > > + * This function makes sure the arm_ffa device is
> > > > > > > > > > > > > + * created, bound to this driver, probed and ready
> > > > > > > > > > > > > to use.
> > > > > > > > > > > > > + * Arm FF-A transport is implemented through a
> > > > > > > > > > > > > single U-Boot
> > > > > > > > > > > > > + * device managing the FF-A bus (arm_ffa).
> > > > > > > > > > > > > + *
> > > > > > > > > > > > > + * Return:
> > > > > > > > > > > > > + *
> > > > > > > > > > > > > + * 0 on success. Otherwise, failure
> > > > > > > > > > > > > + */
> > > > > > > > > > > > > +int ffa_device_get(struct udevice **pdev)
> > > > > > > > > > > > > +{
> > > > > > > > > > > > > + int ret;
> > > > > > > > > > > > > + struct udevice *dev = NULL;
> > > > > > > > > > > > > +
> > > > > > > > > > > > > + ret = device_bind(dm_root(),
> > > > > > > > > > > > > DM_DRIVER_GET(arm_ffa), FFA_DRV_NAME, NULL,
> > > > > > > > > > > > > ofnode_null(),
> > > > > > > > > > > > > + &dev);
> > > > > > > > > > > >
> > > > > > > > > > > > Please add a DT binding. Even if only temporary, we
> > > > > > > > > > > > need something for this.
> > > > > > > > > > >
> > > > > > > > > > > Thanks for the feedback. I'm happy to address all the
> > > > > > > > > > > comments.
> > > > > > > > > > >
> > > > > > > > > > > Regarding DT binding and FF-A discovery. We agreed with
> > > > > > > > > > > Linaro and Rob Herring
> > > > > > > > > > > about the following:
> > > > > > > > > > >
> > > > > > > > > > > - DT is only for what we failed to make discoverable. For
> > > > > > > > > > > hardware, we're stuck
> > > > > > > > > > > with it. We shouldn't repeat that for software
> > > > > > > > > > > interfaces. This approach is
> > > > > > > > > > > already applied in the FF-A kernel driver which comes
> > > > > > > > > > > with no DT support and
> > > > > > > > > > > discovers the bus with bus_register() API [1].
> > > > > > > > > >
> > > > > > > > > > This may be the UEFI view, but it is not how U-Boot works.
> > > > > > > > > > This is not something we are 'stuck' with. It is how we
> > > > > > > > > > define what is present on a device. This is how the PCI bus
> > > > > > > > > > works in U-Boot. It is best practice in U-Boot to use the
> > > > > > > > > > device tree to make this things visible and configurable.
> > > > > > > > > > Unlike with Linux there is no other way to provide
> > > > > > > > > > configuration needed by these devices.
> > > > > > > > >
> > > > > > > > > Where do you get UEFI out of this?
> > > > > > > >
> > > > > > > > I assume it was UEFI as there was no discussion about this in
> > > > > > > > U-Boot.
> > > > > > > > Which firmware project was consulted about this?
> > > > > > > >
> > > > > > > > >
> > > > > > > > > It is the discoverability of hardware that is fixed (and we
> > > > > > > > > are stuck
> > > > > > > > > with). We can't change hardware. The disoverability may be PCI
> > > > > > > > > VID/PID, USB device descriptors, or nothing. We only use DT
> > > > > > > > > when those
> > > > > > > > > are not sufficient. For a software interface, there is no
> > > > > > > > > reason to
> > > > > > > > > make them non-discoverable as the inte
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
Hi Abdellatif,
On Fri, 13 Jan 2023 at 03:44, Abdellatif El Khlifi
wrote:
>
> On Thu, Jan 12, 2023 at 04:43:32PM -0700, Simon Glass wrote:
> > Hi Rob,
> >
> > On Wed, 11 Jan 2023 at 19:10, Rob Herring wrote:
> > >
> > > On Mon, Dec 19, 2022 at 1:21 PM Simon Glass wrote:
> > > >
> > > > Hi Abdellatif,
> > > >
> > > > On Mon, 19 Dec 2022 at 04:12, Abdellatif El Khlifi
> > > > wrote:
> > > > >
> > > > > On Mon, Dec 05, 2022 at 09:49:30AM -0600, Rob Herring wrote:
> > > > > > On Sun, Dec 4, 2022 at 1:22 PM Simon Glass
> > > > > > wrote:
> > > > > > >
> > > > > > > Hi Rob,
> > > > > > >
> > > > > > > On Tue, 29 Nov 2022 at 05:22, Rob Herring wrote:
> > > > > > > >
> > > > > > > > On Fri, Nov 25, 2022 at 3:18 PM Simon Glass
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > Hi Abdellatif,
> > > > > > > > >
> > > > > > > > > On Thu, 24 Nov 2022 at 06:21, Abdellatif El Khlifi
> > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > On Tue, Nov 22, 2022 at 07:09:16PM -0700, Simon Glass wrote:
> > > > > > > > > > > should be called 'priov' and should beHi Abdellatif,
> > > > > > > > > > >
> > > > > > > > >
> > > > > > > > > [..]
> > > > > > > > >
> > > > > > > > > > > > +/**
> > > > > > > > > > > > + * ffa_device_get - create, bind and probe the arm_ffa
> > > > > > > > > > > > device
> > > > > > > > > > > > + * @pdev: the address of a device pointer (to be
> > > > > > > > > > > > filled when the arm_ffa bus device is created
> > > > > > > > > > > > + * successfully)
> > > > > > > > > > > > + *
> > > > > > > > > > > > + * This function makes sure the arm_ffa device is
> > > > > > > > > > > > + * created, bound to this driver, probed and ready to
> > > > > > > > > > > > use.
> > > > > > > > > > > > + * Arm FF-A transport is implemented through a single
> > > > > > > > > > > > U-Boot
> > > > > > > > > > > > + * device managing the FF-A bus (arm_ffa).
> > > > > > > > > > > > + *
> > > > > > > > > > > > + * Return:
> > > > > > > > > > > > + *
> > > > > > > > > > > > + * 0 on success. Otherwise, failure
> > > > > > > > > > > > + */
> > > > > > > > > > > > +int ffa_device_get(struct udevice **pdev)
> > > > > > > > > > > > +{
> > > > > > > > > > > > + int ret;
> > > > > > > > > > > > + struct udevice *dev = NULL;
> > > > > > > > > > > > +
> > > > > > > > > > > > + ret = device_bind(dm_root(),
> > > > > > > > > > > > DM_DRIVER_GET(arm_ffa), FFA_DRV_NAME, NULL,
> > > > > > > > > > > > ofnode_null(),
> > > > > > > > > > > > + &dev);
> > > > > > > > > > >
> > > > > > > > > > > Please add a DT binding. Even if only temporary, we need
> > > > > > > > > > > something for this.
> > > > > > > > > >
> > > > > > > > > > Thanks for the feedback. I'm happy to address all the
> > > > > > > > > > comments.
> > > > > > > > > >
> > > > > > > > > > Regarding DT binding and FF-A discovery. We agreed with
> > > > > > > > > > Linaro and Rob Herring
> > > > > > > > > > about the following:
> > > > > > > > > >
> > > > > > > > > > - DT is only for what we failed to make discoverable. For
> > > > > > > > > > hardware, we're stuck
> > > > > > > > > > with it. We shouldn't repeat that for software
> > > > > > > > > > interfaces. This approach is
> > > > > > > > > > already applied in the FF-A kernel driver which comes
> > > > > > > > > > with no DT support and
> > > > > > > > > > discovers the bus with bus_register() API [1].
> > > > > > > > >
> > > > > > > > > This may be the UEFI view, but it is not how U-Boot works.
> > > > > > > > > This is not something we are 'stuck' with. It is how we
> > > > > > > > > define what is present on a device. This is how the PCI bus
> > > > > > > > > works in U-Boot. It is best practice in U-Boot to use the
> > > > > > > > > device tree to make this things visible and configurable.
> > > > > > > > > Unlike with Linux there is no other way to provide
> > > > > > > > > configuration needed by these devices.
> > > > > > > >
> > > > > > > > Where do you get UEFI out of this?
> > > > > > >
> > > > > > > I assume it was UEFI as there was no discussion about this in
> > > > > > > U-Boot.
> > > > > > > Which firmware project was consulted about this?
> > > > > > >
> > > > > > > >
> > > > > > > > It is the discoverability of hardware that is fixed (and we are
> > > > > > > > stuck
> > > > > > > > with). We can't change hardware. The disoverability may be PCI
> > > > > > > > VID/PID, USB device descriptors, or nothing. We only use DT
> > > > > > > > when those
> > > > > > > > are not sufficient. For a software interface, there is no
> > > > > > > > reason to
> > > > > > > > make them non-discoverable as the interface can be fixed (at
> > > > > > > > least for
> > > > > > > > new things like FF-A).
> > > > > > >
> > > > > > > Here I am talking about the controller itself, the top-level node
> > > > > > > in
> > > > > > > the device tree. For PCI this is a device tree node and it should
> > > > > > > be
> > > > > > > the
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Thu, Jan 12, 2023 at 04:43:32PM -0700, Simon Glass wrote:
> Hi Rob,
>
> On Wed, 11 Jan 2023 at 19:10, Rob Herring wrote:
> >
> > On Mon, Dec 19, 2022 at 1:21 PM Simon Glass wrote:
> > >
> > > Hi Abdellatif,
> > >
> > > On Mon, 19 Dec 2022 at 04:12, Abdellatif El Khlifi
> > > wrote:
> > > >
> > > > On Mon, Dec 05, 2022 at 09:49:30AM -0600, Rob Herring wrote:
> > > > > On Sun, Dec 4, 2022 at 1:22 PM Simon Glass wrote:
> > > > > >
> > > > > > Hi Rob,
> > > > > >
> > > > > > On Tue, 29 Nov 2022 at 05:22, Rob Herring wrote:
> > > > > > >
> > > > > > > On Fri, Nov 25, 2022 at 3:18 PM Simon Glass
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > Hi Abdellatif,
> > > > > > > >
> > > > > > > > On Thu, 24 Nov 2022 at 06:21, Abdellatif El Khlifi
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > On Tue, Nov 22, 2022 at 07:09:16PM -0700, Simon Glass wrote:
> > > > > > > > > > should be called 'priov' and should beHi Abdellatif,
> > > > > > > > > >
> > > > > > > >
> > > > > > > > [..]
> > > > > > > >
> > > > > > > > > > > +/**
> > > > > > > > > > > + * ffa_device_get - create, bind and probe the arm_ffa
> > > > > > > > > > > device
> > > > > > > > > > > + * @pdev: the address of a device pointer (to be filled
> > > > > > > > > > > when the arm_ffa bus device is created
> > > > > > > > > > > + * successfully)
> > > > > > > > > > > + *
> > > > > > > > > > > + * This function makes sure the arm_ffa device is
> > > > > > > > > > > + * created, bound to this driver, probed and ready to
> > > > > > > > > > > use.
> > > > > > > > > > > + * Arm FF-A transport is implemented through a single
> > > > > > > > > > > U-Boot
> > > > > > > > > > > + * device managing the FF-A bus (arm_ffa).
> > > > > > > > > > > + *
> > > > > > > > > > > + * Return:
> > > > > > > > > > > + *
> > > > > > > > > > > + * 0 on success. Otherwise, failure
> > > > > > > > > > > + */
> > > > > > > > > > > +int ffa_device_get(struct udevice **pdev)
> > > > > > > > > > > +{
> > > > > > > > > > > + int ret;
> > > > > > > > > > > + struct udevice *dev = NULL;
> > > > > > > > > > > +
> > > > > > > > > > > + ret = device_bind(dm_root(),
> > > > > > > > > > > DM_DRIVER_GET(arm_ffa), FFA_DRV_NAME, NULL, ofnode_null(),
> > > > > > > > > > > + &dev);
> > > > > > > > > >
> > > > > > > > > > Please add a DT binding. Even if only temporary, we need
> > > > > > > > > > something for this.
> > > > > > > > >
> > > > > > > > > Thanks for the feedback. I'm happy to address all the
> > > > > > > > > comments.
> > > > > > > > >
> > > > > > > > > Regarding DT binding and FF-A discovery. We agreed with
> > > > > > > > > Linaro and Rob Herring
> > > > > > > > > about the following:
> > > > > > > > >
> > > > > > > > > - DT is only for what we failed to make discoverable. For
> > > > > > > > > hardware, we're stuck
> > > > > > > > > with it. We shouldn't repeat that for software interfaces.
> > > > > > > > > This approach is
> > > > > > > > > already applied in the FF-A kernel driver which comes with
> > > > > > > > > no DT support and
> > > > > > > > > discovers the bus with bus_register() API [1].
> > > > > > > >
> > > > > > > > This may be the UEFI view, but it is not how U-Boot works. This
> > > > > > > > is not something we are 'stuck' with. It is how we define what
> > > > > > > > is present on a device. This is how the PCI bus works in
> > > > > > > > U-Boot. It is best practice in U-Boot to use the device tree to
> > > > > > > > make this things visible and configurable. Unlike with Linux
> > > > > > > > there is no other way to provide configuration needed by these
> > > > > > > > devices.
> > > > > > >
> > > > > > > Where do you get UEFI out of this?
> > > > > >
> > > > > > I assume it was UEFI as there was no discussion about this in
> > > > > > U-Boot.
> > > > > > Which firmware project was consulted about this?
> > > > > >
> > > > > > >
> > > > > > > It is the discoverability of hardware that is fixed (and we are
> > > > > > > stuck
> > > > > > > with). We can't change hardware. The disoverability may be PCI
> > > > > > > VID/PID, USB device descriptors, or nothing. We only use DT when
> > > > > > > those
> > > > > > > are not sufficient. For a software interface, there is no reason
> > > > > > > to
> > > > > > > make them non-discoverable as the interface can be fixed (at
> > > > > > > least for
> > > > > > > new things like FF-A).
> > > > > >
> > > > > > Here I am talking about the controller itself, the top-level node in
> > > > > > the device tree. For PCI this is a device tree node and it should be
> > > > > > the same here. So I am not saying that the devices on the bus need
> > > > > > to
> > > > > > be in the device tree (that can be optional, but may be useful in
> > > > > > some
> > > > > > situations where it is status and known).
> > > > >
> > > > > Sure, the PCI host bridges are not discoverable, have a bunch of
> > > > > resources, and do need to be in DT
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
Hi Rob,
On Wed, 11 Jan 2023 at 19:10, Rob Herring wrote:
>
> On Mon, Dec 19, 2022 at 1:21 PM Simon Glass wrote:
> >
> > Hi Abdellatif,
> >
> > On Mon, 19 Dec 2022 at 04:12, Abdellatif El Khlifi
> > wrote:
> > >
> > > On Mon, Dec 05, 2022 at 09:49:30AM -0600, Rob Herring wrote:
> > > > On Sun, Dec 4, 2022 at 1:22 PM Simon Glass wrote:
> > > > >
> > > > > Hi Rob,
> > > > >
> > > > > On Tue, 29 Nov 2022 at 05:22, Rob Herring wrote:
> > > > > >
> > > > > > On Fri, Nov 25, 2022 at 3:18 PM Simon Glass
> > > > > > wrote:
> > > > > > >
> > > > > > > Hi Abdellatif,
> > > > > > >
> > > > > > > On Thu, 24 Nov 2022 at 06:21, Abdellatif El Khlifi
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > On Tue, Nov 22, 2022 at 07:09:16PM -0700, Simon Glass wrote:
> > > > > > > > > should be called 'priov' and should beHi Abdellatif,
> > > > > > > > >
> > > > > > >
> > > > > > > [..]
> > > > > > >
> > > > > > > > > > +/**
> > > > > > > > > > + * ffa_device_get - create, bind and probe the arm_ffa
> > > > > > > > > > device
> > > > > > > > > > + * @pdev: the address of a device pointer (to be filled
> > > > > > > > > > when the arm_ffa bus device is created
> > > > > > > > > > + * successfully)
> > > > > > > > > > + *
> > > > > > > > > > + * This function makes sure the arm_ffa device is
> > > > > > > > > > + * created, bound to this driver, probed and ready to use.
> > > > > > > > > > + * Arm FF-A transport is implemented through a single
> > > > > > > > > > U-Boot
> > > > > > > > > > + * device managing the FF-A bus (arm_ffa).
> > > > > > > > > > + *
> > > > > > > > > > + * Return:
> > > > > > > > > > + *
> > > > > > > > > > + * 0 on success. Otherwise, failure
> > > > > > > > > > + */
> > > > > > > > > > +int ffa_device_get(struct udevice **pdev)
> > > > > > > > > > +{
> > > > > > > > > > + int ret;
> > > > > > > > > > + struct udevice *dev = NULL;
> > > > > > > > > > +
> > > > > > > > > > + ret = device_bind(dm_root(),
> > > > > > > > > > DM_DRIVER_GET(arm_ffa), FFA_DRV_NAME, NULL, ofnode_null(),
> > > > > > > > > > + &dev);
> > > > > > > > >
> > > > > > > > > Please add a DT binding. Even if only temporary, we need
> > > > > > > > > something for this.
> > > > > > > >
> > > > > > > > Thanks for the feedback. I'm happy to address all the comments.
> > > > > > > >
> > > > > > > > Regarding DT binding and FF-A discovery. We agreed with Linaro
> > > > > > > > and Rob Herring
> > > > > > > > about the following:
> > > > > > > >
> > > > > > > > - DT is only for what we failed to make discoverable. For
> > > > > > > > hardware, we're stuck
> > > > > > > > with it. We shouldn't repeat that for software interfaces.
> > > > > > > > This approach is
> > > > > > > > already applied in the FF-A kernel driver which comes with no
> > > > > > > > DT support and
> > > > > > > > discovers the bus with bus_register() API [1].
> > > > > > >
> > > > > > > This may be the UEFI view, but it is not how U-Boot works. This
> > > > > > > is not something we are 'stuck' with. It is how we define what is
> > > > > > > present on a device. This is how the PCI bus works in U-Boot. It
> > > > > > > is best practice in U-Boot to use the device tree to make this
> > > > > > > things visible and configurable. Unlike with Linux there is no
> > > > > > > other way to provide configuration needed by these devices.
> > > > > >
> > > > > > Where do you get UEFI out of this?
> > > > >
> > > > > I assume it was UEFI as there was no discussion about this in U-Boot.
> > > > > Which firmware project was consulted about this?
> > > > >
> > > > > >
> > > > > > It is the discoverability of hardware that is fixed (and we are
> > > > > > stuck
> > > > > > with). We can't change hardware. The disoverability may be PCI
> > > > > > VID/PID, USB device descriptors, or nothing. We only use DT when
> > > > > > those
> > > > > > are not sufficient. For a software interface, there is no reason to
> > > > > > make them non-discoverable as the interface can be fixed (at least
> > > > > > for
> > > > > > new things like FF-A).
> > > > >
> > > > > Here I am talking about the controller itself, the top-level node in
> > > > > the device tree. For PCI this is a device tree node and it should be
> > > > > the same here. So I am not saying that the devices on the bus need to
> > > > > be in the device tree (that can be optional, but may be useful in some
> > > > > situations where it is status and known).
> > > >
> > > > Sure, the PCI host bridges are not discoverable, have a bunch of
> > > > resources, and do need to be in DT. The downstream devices only do if
> > > > they have extra resources such as when a device is soldered down on a
> > > > board rather than a standard slot.
> > > >
> > > > > We need something like:
> > > > >
> > > > > ff-a {
> > > > > compatible = "something";
> > > > > };
> > > > >
> > > > > I don't know what mechanism is actually used to communicate with it,
> > > > > but that
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Mon, Dec 19, 2022 at 1:21 PM Simon Glass wrote:
>
> Hi Abdellatif,
>
> On Mon, 19 Dec 2022 at 04:12, Abdellatif El Khlifi
> wrote:
> >
> > On Mon, Dec 05, 2022 at 09:49:30AM -0600, Rob Herring wrote:
> > > On Sun, Dec 4, 2022 at 1:22 PM Simon Glass wrote:
> > > >
> > > > Hi Rob,
> > > >
> > > > On Tue, 29 Nov 2022 at 05:22, Rob Herring wrote:
> > > > >
> > > > > On Fri, Nov 25, 2022 at 3:18 PM Simon Glass wrote:
> > > > > >
> > > > > > Hi Abdellatif,
> > > > > >
> > > > > > On Thu, 24 Nov 2022 at 06:21, Abdellatif El Khlifi
> > > > > > wrote:
> > > > > > >
> > > > > > > On Tue, Nov 22, 2022 at 07:09:16PM -0700, Simon Glass wrote:
> > > > > > > > should be called 'priov' and should beHi Abdellatif,
> > > > > > > >
> > > > > >
> > > > > > [..]
> > > > > >
> > > > > > > > > +/**
> > > > > > > > > + * ffa_device_get - create, bind and probe the arm_ffa device
> > > > > > > > > + * @pdev: the address of a device pointer (to be filled when
> > > > > > > > > the arm_ffa bus device is created
> > > > > > > > > + * successfully)
> > > > > > > > > + *
> > > > > > > > > + * This function makes sure the arm_ffa device is
> > > > > > > > > + * created, bound to this driver, probed and ready to use.
> > > > > > > > > + * Arm FF-A transport is implemented through a single U-Boot
> > > > > > > > > + * device managing the FF-A bus (arm_ffa).
> > > > > > > > > + *
> > > > > > > > > + * Return:
> > > > > > > > > + *
> > > > > > > > > + * 0 on success. Otherwise, failure
> > > > > > > > > + */
> > > > > > > > > +int ffa_device_get(struct udevice **pdev)
> > > > > > > > > +{
> > > > > > > > > + int ret;
> > > > > > > > > + struct udevice *dev = NULL;
> > > > > > > > > +
> > > > > > > > > + ret = device_bind(dm_root(), DM_DRIVER_GET(arm_ffa),
> > > > > > > > > FFA_DRV_NAME, NULL, ofnode_null(),
> > > > > > > > > + &dev);
> > > > > > > >
> > > > > > > > Please add a DT binding. Even if only temporary, we need
> > > > > > > > something for this.
> > > > > > >
> > > > > > > Thanks for the feedback. I'm happy to address all the comments.
> > > > > > >
> > > > > > > Regarding DT binding and FF-A discovery. We agreed with Linaro
> > > > > > > and Rob Herring
> > > > > > > about the following:
> > > > > > >
> > > > > > > - DT is only for what we failed to make discoverable. For
> > > > > > > hardware, we're stuck
> > > > > > > with it. We shouldn't repeat that for software interfaces. This
> > > > > > > approach is
> > > > > > > already applied in the FF-A kernel driver which comes with no
> > > > > > > DT support and
> > > > > > > discovers the bus with bus_register() API [1].
> > > > > >
> > > > > > This may be the UEFI view, but it is not how U-Boot works. This is
> > > > > > not something we are 'stuck' with. It is how we define what is
> > > > > > present on a device. This is how the PCI bus works in U-Boot. It is
> > > > > > best practice in U-Boot to use the device tree to make this things
> > > > > > visible and configurable. Unlike with Linux there is no other way
> > > > > > to provide configuration needed by these devices.
> > > > >
> > > > > Where do you get UEFI out of this?
> > > >
> > > > I assume it was UEFI as there was no discussion about this in U-Boot.
> > > > Which firmware project was consulted about this?
> > > >
> > > > >
> > > > > It is the discoverability of hardware that is fixed (and we are stuck
> > > > > with). We can't change hardware. The disoverability may be PCI
> > > > > VID/PID, USB device descriptors, or nothing. We only use DT when those
> > > > > are not sufficient. For a software interface, there is no reason to
> > > > > make them non-discoverable as the interface can be fixed (at least for
> > > > > new things like FF-A).
> > > >
> > > > Here I am talking about the controller itself, the top-level node in
> > > > the device tree. For PCI this is a device tree node and it should be
> > > > the same here. So I am not saying that the devices on the bus need to
> > > > be in the device tree (that can be optional, but may be useful in some
> > > > situations where it is status and known).
> > >
> > > Sure, the PCI host bridges are not discoverable, have a bunch of
> > > resources, and do need to be in DT. The downstream devices only do if
> > > they have extra resources such as when a device is soldered down on a
> > > board rather than a standard slot.
> > >
> > > > We need something like:
> > > >
> > > > ff-a {
> > > > compatible = "something";
> > > > };
> > > >
> > > > I don't know what mechanism is actually used to communicate with it,
> > > > but that will be enough to get the top-level driver started.
> > >
> > > There's discovery of FF-A itself and then discovery of FF-A features
> > > (e.g. partitions). Both of those are discoverable without DT. The
> > > first is done by checking the SMCCC version, then checking for FF-A
> > > presence and features. Putting this into DT is redundant. Worse, what
> > > if they
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
Hi Abdellatif,
On Mon, 19 Dec 2022 at 04:12, Abdellatif El Khlifi
wrote:
>
> On Mon, Dec 05, 2022 at 09:49:30AM -0600, Rob Herring wrote:
> > On Sun, Dec 4, 2022 at 1:22 PM Simon Glass wrote:
> > >
> > > Hi Rob,
> > >
> > > On Tue, 29 Nov 2022 at 05:22, Rob Herring wrote:
> > > >
> > > > On Fri, Nov 25, 2022 at 3:18 PM Simon Glass wrote:
> > > > >
> > > > > Hi Abdellatif,
> > > > >
> > > > > On Thu, 24 Nov 2022 at 06:21, Abdellatif El Khlifi
> > > > > wrote:
> > > > > >
> > > > > > On Tue, Nov 22, 2022 at 07:09:16PM -0700, Simon Glass wrote:
> > > > > > > should be called 'priov' and should beHi Abdellatif,
> > > > > > >
> > > > >
> > > > > [..]
> > > > >
> > > > > > > > +/**
> > > > > > > > + * ffa_device_get - create, bind and probe the arm_ffa device
> > > > > > > > + * @pdev: the address of a device pointer (to be filled when
> > > > > > > > the arm_ffa bus device is created
> > > > > > > > + * successfully)
> > > > > > > > + *
> > > > > > > > + * This function makes sure the arm_ffa device is
> > > > > > > > + * created, bound to this driver, probed and ready to use.
> > > > > > > > + * Arm FF-A transport is implemented through a single U-Boot
> > > > > > > > + * device managing the FF-A bus (arm_ffa).
> > > > > > > > + *
> > > > > > > > + * Return:
> > > > > > > > + *
> > > > > > > > + * 0 on success. Otherwise, failure
> > > > > > > > + */
> > > > > > > > +int ffa_device_get(struct udevice **pdev)
> > > > > > > > +{
> > > > > > > > + int ret;
> > > > > > > > + struct udevice *dev = NULL;
> > > > > > > > +
> > > > > > > > + ret = device_bind(dm_root(), DM_DRIVER_GET(arm_ffa),
> > > > > > > > FFA_DRV_NAME, NULL, ofnode_null(),
> > > > > > > > + &dev);
> > > > > > >
> > > > > > > Please add a DT binding. Even if only temporary, we need
> > > > > > > something for this.
> > > > > >
> > > > > > Thanks for the feedback. I'm happy to address all the comments.
> > > > > >
> > > > > > Regarding DT binding and FF-A discovery. We agreed with Linaro and
> > > > > > Rob Herring
> > > > > > about the following:
> > > > > >
> > > > > > - DT is only for what we failed to make discoverable. For hardware,
> > > > > > we're stuck
> > > > > > with it. We shouldn't repeat that for software interfaces. This
> > > > > > approach is
> > > > > > already applied in the FF-A kernel driver which comes with no DT
> > > > > > support and
> > > > > > discovers the bus with bus_register() API [1].
> > > > >
> > > > > This may be the UEFI view, but it is not how U-Boot works. This is
> > > > > not something we are 'stuck' with. It is how we define what is
> > > > > present on a device. This is how the PCI bus works in U-Boot. It is
> > > > > best practice in U-Boot to use the device tree to make this things
> > > > > visible and configurable. Unlike with Linux there is no other way to
> > > > > provide configuration needed by these devices.
> > > >
> > > > Where do you get UEFI out of this?
> > >
> > > I assume it was UEFI as there was no discussion about this in U-Boot.
> > > Which firmware project was consulted about this?
> > >
> > > >
> > > > It is the discoverability of hardware that is fixed (and we are stuck
> > > > with). We can't change hardware. The disoverability may be PCI
> > > > VID/PID, USB device descriptors, or nothing. We only use DT when those
> > > > are not sufficient. For a software interface, there is no reason to
> > > > make them non-discoverable as the interface can be fixed (at least for
> > > > new things like FF-A).
> > >
> > > Here I am talking about the controller itself, the top-level node in
> > > the device tree. For PCI this is a device tree node and it should be
> > > the same here. So I am not saying that the devices on the bus need to
> > > be in the device tree (that can be optional, but may be useful in some
> > > situations where it is status and known).
> >
> > Sure, the PCI host bridges are not discoverable, have a bunch of
> > resources, and do need to be in DT. The downstream devices only do if
> > they have extra resources such as when a device is soldered down on a
> > board rather than a standard slot.
> >
> > > We need something like:
> > >
> > > ff-a {
> > > compatible = "something";
> > > };
> > >
> > > I don't know what mechanism is actually used to communicate with it,
> > > but that will be enough to get the top-level driver started.
> >
> > There's discovery of FF-A itself and then discovery of FF-A features
> > (e.g. partitions). Both of those are discoverable without DT. The
> > first is done by checking the SMCCC version, then checking for FF-A
> > presence and features. Putting this into DT is redundant. Worse, what
> > if they disagree?
>
> Hi Simon,
>
> Do you agree with Rob, Ilias and myself that it makes more sense
> FF-A bus is discovered without a DT node and following the same approach as
> Linux ? (FF-A bus doesn't have a HW controller and is a purely SW bus,
> no configuration/description
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Mon, Dec 05, 2022 at 09:49:30AM -0600, Rob Herring wrote:
> On Sun, Dec 4, 2022 at 1:22 PM Simon Glass wrote:
> >
> > Hi Rob,
> >
> > On Tue, 29 Nov 2022 at 05:22, Rob Herring wrote:
> > >
> > > On Fri, Nov 25, 2022 at 3:18 PM Simon Glass wrote:
> > > >
> > > > Hi Abdellatif,
> > > >
> > > > On Thu, 24 Nov 2022 at 06:21, Abdellatif El Khlifi
> > > > wrote:
> > > > >
> > > > > On Tue, Nov 22, 2022 at 07:09:16PM -0700, Simon Glass wrote:
> > > > > > should be called 'priov' and should beHi Abdellatif,
> > > > > >
> > > >
> > > > [..]
> > > >
> > > > > > > +/**
> > > > > > > + * ffa_device_get - create, bind and probe the arm_ffa device
> > > > > > > + * @pdev: the address of a device pointer (to be filled when the
> > > > > > > arm_ffa bus device is created
> > > > > > > + * successfully)
> > > > > > > + *
> > > > > > > + * This function makes sure the arm_ffa device is
> > > > > > > + * created, bound to this driver, probed and ready to use.
> > > > > > > + * Arm FF-A transport is implemented through a single U-Boot
> > > > > > > + * device managing the FF-A bus (arm_ffa).
> > > > > > > + *
> > > > > > > + * Return:
> > > > > > > + *
> > > > > > > + * 0 on success. Otherwise, failure
> > > > > > > + */
> > > > > > > +int ffa_device_get(struct udevice **pdev)
> > > > > > > +{
> > > > > > > + int ret;
> > > > > > > + struct udevice *dev = NULL;
> > > > > > > +
> > > > > > > + ret = device_bind(dm_root(), DM_DRIVER_GET(arm_ffa),
> > > > > > > FFA_DRV_NAME, NULL, ofnode_null(),
> > > > > > > + &dev);
> > > > > >
> > > > > > Please add a DT binding. Even if only temporary, we need something
> > > > > > for this.
> > > > >
> > > > > Thanks for the feedback. I'm happy to address all the comments.
> > > > >
> > > > > Regarding DT binding and FF-A discovery. We agreed with Linaro and
> > > > > Rob Herring
> > > > > about the following:
> > > > >
> > > > > - DT is only for what we failed to make discoverable. For hardware,
> > > > > we're stuck
> > > > > with it. We shouldn't repeat that for software interfaces. This
> > > > > approach is
> > > > > already applied in the FF-A kernel driver which comes with no DT
> > > > > support and
> > > > > discovers the bus with bus_register() API [1].
> > > >
> > > > This may be the UEFI view, but it is not how U-Boot works. This is not
> > > > something we are 'stuck' with. It is how we define what is present on a
> > > > device. This is how the PCI bus works in U-Boot. It is best practice in
> > > > U-Boot to use the device tree to make this things visible and
> > > > configurable. Unlike with Linux there is no other way to provide
> > > > configuration needed by these devices.
> > >
> > > Where do you get UEFI out of this?
> >
> > I assume it was UEFI as there was no discussion about this in U-Boot.
> > Which firmware project was consulted about this?
> >
> > >
> > > It is the discoverability of hardware that is fixed (and we are stuck
> > > with). We can't change hardware. The disoverability may be PCI
> > > VID/PID, USB device descriptors, or nothing. We only use DT when those
> > > are not sufficient. For a software interface, there is no reason to
> > > make them non-discoverable as the interface can be fixed (at least for
> > > new things like FF-A).
> >
> > Here I am talking about the controller itself, the top-level node in
> > the device tree. For PCI this is a device tree node and it should be
> > the same here. So I am not saying that the devices on the bus need to
> > be in the device tree (that can be optional, but may be useful in some
> > situations where it is status and known).
>
> Sure, the PCI host bridges are not discoverable, have a bunch of
> resources, and do need to be in DT. The downstream devices only do if
> they have extra resources such as when a device is soldered down on a
> board rather than a standard slot.
>
> > We need something like:
> >
> > ff-a {
> > compatible = "something";
> > };
> >
> > I don't know what mechanism is actually used to communicate with it,
> > but that will be enough to get the top-level driver started.
>
> There's discovery of FF-A itself and then discovery of FF-A features
> (e.g. partitions). Both of those are discoverable without DT. The
> first is done by checking the SMCCC version, then checking for FF-A
> presence and features. Putting this into DT is redundant. Worse, what
> if they disagree?
Hi Simon,
Do you agree with Rob, Ilias and myself that it makes more sense
FF-A bus is discovered without a DT node and following the same approach as
Linux ? (FF-A bus doesn't have a HW controller and is a purely SW bus,
no configuration/description needed at DT level).
Your suggestions are always welcome.
cheers
>
> > If Linux does not want to use the node, that it another thing, but I
> > respectfully request that U-Boot's needs be considered more carefully.
>
> It's not really a big deal for just one compatible. It's the next 10
> f
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Sun, Dec 4, 2022 at 1:22 PM Simon Glass wrote:
>
> Hi Rob,
>
> On Tue, 29 Nov 2022 at 05:22, Rob Herring wrote:
> >
> > On Fri, Nov 25, 2022 at 3:18 PM Simon Glass wrote:
> > >
> > > Hi Abdellatif,
> > >
> > > On Thu, 24 Nov 2022 at 06:21, Abdellatif El Khlifi
> > > wrote:
> > > >
> > > > On Tue, Nov 22, 2022 at 07:09:16PM -0700, Simon Glass wrote:
> > > > > should be called 'priov' and should beHi Abdellatif,
> > > > >
> > >
> > > [..]
> > >
> > > > > > +/**
> > > > > > + * ffa_device_get - create, bind and probe the arm_ffa device
> > > > > > + * @pdev: the address of a device pointer (to be filled when the
> > > > > > arm_ffa bus device is created
> > > > > > + * successfully)
> > > > > > + *
> > > > > > + * This function makes sure the arm_ffa device is
> > > > > > + * created, bound to this driver, probed and ready to use.
> > > > > > + * Arm FF-A transport is implemented through a single U-Boot
> > > > > > + * device managing the FF-A bus (arm_ffa).
> > > > > > + *
> > > > > > + * Return:
> > > > > > + *
> > > > > > + * 0 on success. Otherwise, failure
> > > > > > + */
> > > > > > +int ffa_device_get(struct udevice **pdev)
> > > > > > +{
> > > > > > + int ret;
> > > > > > + struct udevice *dev = NULL;
> > > > > > +
> > > > > > + ret = device_bind(dm_root(), DM_DRIVER_GET(arm_ffa),
> > > > > > FFA_DRV_NAME, NULL, ofnode_null(),
> > > > > > + &dev);
> > > > >
> > > > > Please add a DT binding. Even if only temporary, we need something
> > > > > for this.
> > > >
> > > > Thanks for the feedback. I'm happy to address all the comments.
> > > >
> > > > Regarding DT binding and FF-A discovery. We agreed with Linaro and Rob
> > > > Herring
> > > > about the following:
> > > >
> > > > - DT is only for what we failed to make discoverable. For hardware,
> > > > we're stuck
> > > > with it. We shouldn't repeat that for software interfaces. This
> > > > approach is
> > > > already applied in the FF-A kernel driver which comes with no DT
> > > > support and
> > > > discovers the bus with bus_register() API [1].
> > >
> > > This may be the UEFI view, but it is not how U-Boot works. This is not
> > > something we are 'stuck' with. It is how we define what is present on a
> > > device. This is how the PCI bus works in U-Boot. It is best practice in
> > > U-Boot to use the device tree to make this things visible and
> > > configurable. Unlike with Linux there is no other way to provide
> > > configuration needed by these devices.
> >
> > Where do you get UEFI out of this?
>
> I assume it was UEFI as there was no discussion about this in U-Boot.
> Which firmware project was consulted about this?
>
> >
> > It is the discoverability of hardware that is fixed (and we are stuck
> > with). We can't change hardware. The disoverability may be PCI
> > VID/PID, USB device descriptors, or nothing. We only use DT when those
> > are not sufficient. For a software interface, there is no reason to
> > make them non-discoverable as the interface can be fixed (at least for
> > new things like FF-A).
>
> Here I am talking about the controller itself, the top-level node in
> the device tree. For PCI this is a device tree node and it should be
> the same here. So I am not saying that the devices on the bus need to
> be in the device tree (that can be optional, but may be useful in some
> situations where it is status and known).
Sure, the PCI host bridges are not discoverable, have a bunch of
resources, and do need to be in DT. The downstream devices only do if
they have extra resources such as when a device is soldered down on a
board rather than a standard slot.
> We need something like:
>
> ff-a {
> compatible = "something";
> };
>
> I don't know what mechanism is actually used to communicate with it,
> but that will be enough to get the top-level driver started.
There's discovery of FF-A itself and then discovery of FF-A features
(e.g. partitions). Both of those are discoverable without DT. The
first is done by checking the SMCCC version, then checking for FF-A
presence and features. Putting this into DT is redundant. Worse, what
if they disagree?
> If Linux does not want to use the node, that it another thing, but I
> respectfully request that U-Boot's needs be considered more carefully.
It's not really a big deal for just one compatible. It's the next 10
firmware things Arm comes up with. Or the let's add one property at a
time binding (mis)design that happens once we have a binding.
> I'd also like to see more willingness to accommodate open-source
> software in these designs.
I'm not sure what you are asking for here. Are you talking about FF-A
and Arm firmware interfaces itself, the DT binding for it, or
something else? I'd agree on the first part. I only saw this when the
binding landed on my plate. For bindings themselves, the firehose is
there. I can't pick out what you or others care and don't care about.
I try to steer common things to the
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
Hi Rob,
On Tue, 29 Nov 2022 at 05:22, Rob Herring wrote:
>
> On Fri, Nov 25, 2022 at 3:18 PM Simon Glass wrote:
> >
> > Hi Abdellatif,
> >
> > On Thu, 24 Nov 2022 at 06:21, Abdellatif El Khlifi
> > wrote:
> > >
> > > On Tue, Nov 22, 2022 at 07:09:16PM -0700, Simon Glass wrote:
> > > > should be called 'priov' and should beHi Abdellatif,
> > > >
> >
> > [..]
> >
> > > > > +/**
> > > > > + * ffa_device_get - create, bind and probe the arm_ffa device
> > > > > + * @pdev: the address of a device pointer (to be filled when the
> > > > > arm_ffa bus device is created
> > > > > + * successfully)
> > > > > + *
> > > > > + * This function makes sure the arm_ffa device is
> > > > > + * created, bound to this driver, probed and ready to use.
> > > > > + * Arm FF-A transport is implemented through a single U-Boot
> > > > > + * device managing the FF-A bus (arm_ffa).
> > > > > + *
> > > > > + * Return:
> > > > > + *
> > > > > + * 0 on success. Otherwise, failure
> > > > > + */
> > > > > +int ffa_device_get(struct udevice **pdev)
> > > > > +{
> > > > > + int ret;
> > > > > + struct udevice *dev = NULL;
> > > > > +
> > > > > + ret = device_bind(dm_root(), DM_DRIVER_GET(arm_ffa),
> > > > > FFA_DRV_NAME, NULL, ofnode_null(),
> > > > > + &dev);
> > > >
> > > > Please add a DT binding. Even if only temporary, we need something for
> > > > this.
> > >
> > > Thanks for the feedback. I'm happy to address all the comments.
> > >
> > > Regarding DT binding and FF-A discovery. We agreed with Linaro and Rob
> > > Herring
> > > about the following:
> > >
> > > - DT is only for what we failed to make discoverable. For hardware, we're
> > > stuck
> > > with it. We shouldn't repeat that for software interfaces. This
> > > approach is
> > > already applied in the FF-A kernel driver which comes with no DT
> > > support and
> > > discovers the bus with bus_register() API [1].
> >
> > This may be the UEFI view, but it is not how U-Boot works. This is not
> > something we are 'stuck' with. It is how we define what is present on a
> > device. This is how the PCI bus works in U-Boot. It is best practice in
> > U-Boot to use the device tree to make this things visible and configurable.
> > Unlike with Linux there is no other way to provide configuration needed by
> > these devices.
>
> Where do you get UEFI out of this?
I assume it was UEFI as there was no discussion about this in U-Boot.
Which firmware project was consulted about this?
>
> It is the discoverability of hardware that is fixed (and we are stuck
> with). We can't change hardware. The disoverability may be PCI
> VID/PID, USB device descriptors, or nothing. We only use DT when those
> are not sufficient. For a software interface, there is no reason to
> make them non-discoverable as the interface can be fixed (at least for
> new things like FF-A).
Here I am talking about the controller itself, the top-level node in
the device tree. For PCI this is a device tree node and it should be
the same here. So I am not saying that the devices on the bus need to
be in the device tree (that can be optional, but may be useful in some
situations where it is status and known). We need something like:
ff-a {
compatible = "something";
};
I don't know what mechanism is actually used to communicate with it,
but that will be enough to get the top-level driver started.
If Linux does not want to use the node, that it another thing, but I
respectfully request that U-Boot's needs be considered more carefully.
I'd also like to see more willingness to accommodate open-source
software in these designs.
Regards,
Simon
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
Hi all
On Mon, 28 Nov 2022 at 18:22, Rob Herring wrote:
>
> On Fri, Nov 25, 2022 at 3:18 PM Simon Glass wrote:
> >
> > Hi Abdellatif,
> >
> > On Thu, 24 Nov 2022 at 06:21, Abdellatif El Khlifi
> > wrote:
> > >
> > > On Tue, Nov 22, 2022 at 07:09:16PM -0700, Simon Glass wrote:
> > > > should be called 'priov' and should beHi Abdellatif,
> > > >
> >
> > [..]
> >
> > > > > +/**
> > > > > + * ffa_device_get - create, bind and probe the arm_ffa device
> > > > > + * @pdev: the address of a device pointer (to be filled when the
> > > > > arm_ffa bus device is created
> > > > > + * successfully)
> > > > > + *
> > > > > + * This function makes sure the arm_ffa device is
> > > > > + * created, bound to this driver, probed and ready to use.
> > > > > + * Arm FF-A transport is implemented through a single U-Boot
> > > > > + * device managing the FF-A bus (arm_ffa).
> > > > > + *
> > > > > + * Return:
> > > > > + *
> > > > > + * 0 on success. Otherwise, failure
> > > > > + */
> > > > > +int ffa_device_get(struct udevice **pdev)
> > > > > +{
> > > > > + int ret;
> > > > > + struct udevice *dev = NULL;
> > > > > +
> > > > > + ret = device_bind(dm_root(), DM_DRIVER_GET(arm_ffa),
> > > > > FFA_DRV_NAME, NULL, ofnode_null(),
> > > > > + &dev);
> > > >
> > > > Please add a DT binding. Even if only temporary, we need something for
> > > > this.
> > >
> > > Thanks for the feedback. I'm happy to address all the comments.
> > >
> > > Regarding DT binding and FF-A discovery. We agreed with Linaro and Rob
> > > Herring
> > > about the following:
> > >
> > > - DT is only for what we failed to make discoverable. For hardware, we're
> > > stuck
> > > with it. We shouldn't repeat that for software interfaces. This
> > > approach is
> > > already applied in the FF-A kernel driver which comes with no DT
> > > support and
> > > discovers the bus with bus_register() API [1].
> >
> > This may be the UEFI view, but it is not how U-Boot works. This is not
> > something we are 'stuck' with. It is how we define what is present on a
> > device. This is how the PCI bus works in U-Boot. It is best practice in
> > U-Boot to use the device tree to make this things visible and configurable.
> > Unlike with Linux there is no other way to provide configuration needed by
> > these devices.
>
> Where do you get UEFI out of this?
>
> It is the discoverability of hardware that is fixed (and we are stuck
> with). We can't change hardware. The disoverability may be PCI
> VID/PID, USB device descriptors, or nothing. We only use DT when those
> are not sufficient. For a software interface, there is no reason to
> make them non-discoverable as the interface can be fixed (at least for
> new things like FF-A).
I'll agree with Rob here. In fact the first version of the patchset
*did* have this as a DT node. We explicitly asked Abdellatif to
change this, so u-boot and the linux kernel can have an identical
approach in discovering FF-A
Regards
/Ilias
>
> Rob
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Fri, Nov 25, 2022 at 3:18 PM Simon Glass wrote:
>
> Hi Abdellatif,
>
> On Thu, 24 Nov 2022 at 06:21, Abdellatif El Khlifi
> wrote:
> >
> > On Tue, Nov 22, 2022 at 07:09:16PM -0700, Simon Glass wrote:
> > > should be called 'priov' and should beHi Abdellatif,
> > >
>
> [..]
>
> > > > +/**
> > > > + * ffa_device_get - create, bind and probe the arm_ffa device
> > > > + * @pdev: the address of a device pointer (to be filled when the
> > > > arm_ffa bus device is created
> > > > + * successfully)
> > > > + *
> > > > + * This function makes sure the arm_ffa device is
> > > > + * created, bound to this driver, probed and ready to use.
> > > > + * Arm FF-A transport is implemented through a single U-Boot
> > > > + * device managing the FF-A bus (arm_ffa).
> > > > + *
> > > > + * Return:
> > > > + *
> > > > + * 0 on success. Otherwise, failure
> > > > + */
> > > > +int ffa_device_get(struct udevice **pdev)
> > > > +{
> > > > + int ret;
> > > > + struct udevice *dev = NULL;
> > > > +
> > > > + ret = device_bind(dm_root(), DM_DRIVER_GET(arm_ffa),
> > > > FFA_DRV_NAME, NULL, ofnode_null(),
> > > > + &dev);
> > >
> > > Please add a DT binding. Even if only temporary, we need something for
> > > this.
> >
> > Thanks for the feedback. I'm happy to address all the comments.
> >
> > Regarding DT binding and FF-A discovery. We agreed with Linaro and Rob
> > Herring
> > about the following:
> >
> > - DT is only for what we failed to make discoverable. For hardware, we're
> > stuck
> > with it. We shouldn't repeat that for software interfaces. This approach
> > is
> > already applied in the FF-A kernel driver which comes with no DT support
> > and
> > discovers the bus with bus_register() API [1].
>
> This may be the UEFI view, but it is not how U-Boot works. This is not
> something we are 'stuck' with. It is how we define what is present on a
> device. This is how the PCI bus works in U-Boot. It is best practice in
> U-Boot to use the device tree to make this things visible and configurable.
> Unlike with Linux there is no other way to provide configuration needed by
> these devices.
Where do you get UEFI out of this?
It is the discoverability of hardware that is fixed (and we are stuck
with). We can't change hardware. The disoverability may be PCI
VID/PID, USB device descriptors, or nothing. We only use DT when those
are not sufficient. For a software interface, there is no reason to
make them non-discoverable as the interface can be fixed (at least for
new things like FF-A).
Rob
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
Hi Abdellatif,
On Thu, 24 Nov 2022 at 06:21, Abdellatif El Khlifi <
abdellatif.elkhl...@arm.com> wrote:
>
> On Tue, Nov 22, 2022 at 07:09:16PM -0700, Simon Glass wrote:
> > should be called 'priov' and should beHi Abdellatif,
> >
[..]
> > > +/**
> > > + * ffa_device_get - create, bind and probe the arm_ffa device
> > > + * @pdev: the address of a device pointer (to be filled when the
arm_ffa bus device is created
> > > + * successfully)
> > > + *
> > > + * This function makes sure the arm_ffa device is
> > > + * created, bound to this driver, probed and ready to use.
> > > + * Arm FF-A transport is implemented through a single U-Boot
> > > + * device managing the FF-A bus (arm_ffa).
> > > + *
> > > + * Return:
> > > + *
> > > + * 0 on success. Otherwise, failure
> > > + */
> > > +int ffa_device_get(struct udevice **pdev)
> > > +{
> > > + int ret;
> > > + struct udevice *dev = NULL;
> > > +
> > > + ret = device_bind(dm_root(), DM_DRIVER_GET(arm_ffa),
FFA_DRV_NAME, NULL, ofnode_null(),
> > > + &dev);
> >
> > Please add a DT binding. Even if only temporary, we need something for
this.
>
> Thanks for the feedback. I'm happy to address all the comments.
>
> Regarding DT binding and FF-A discovery. We agreed with Linaro and Rob
Herring
> about the following:
>
> - DT is only for what we failed to make discoverable. For hardware, we're
stuck
> with it. We shouldn't repeat that for software interfaces. This
approach is
> already applied in the FF-A kernel driver which comes with no DT
support and
> discovers the bus with bus_register() API [1].
This may be the UEFI view, but it is not how U-Boot works. This is not
something we are 'stuck' with. It is how we define what is present on a
device. This is how the PCI bus works in U-Boot. It is best practice in
U-Boot to use the device tree to make this things visible and configurable.
Unlike with Linux there is no other way to provide configuration needed by
these devices.
>
> - FF-A bus in U-Boot is used on demand by clients. We don't want to set
it up
> if no client is using it. For example, the EFI MM client tries to
discover the
> FF-A bus, if it succeeds it uses it. Otherwise, it uses OP-TEE protocol
[2].
> EFI MM client tries to discover the FF-A bus by calling
ffa_bus_discover()
> which is a discovery API provided by the FF-A core driver.
You are talking about probing, I think. Lazy init is build into U-Boot and
works fine. We don't need to invent another way to do it. Probe the device
if it is needed, but bind it always.
>
> Here is an overview about what ffa_bus_discover() does :
>
> client -> ffa_bus_discover() -> ffa_device_get() -> { device_bind() ,
device_probe() }
>
> device_probe() -> ffa_probe() -> { at this stage the FF-A driver tries to
discover the bus by executing FF-A discovery ABIs }
Yes it needs a bit of clean-up, to be honest. You need to add a device
pointer to the calls as well.
>
> Let's see what Ilias and Rob think about FF-A discovery and DT support.
>
> Cheers
> Abdellatif
>
> [1]:
https://elixir.bootlin.com/linux/v6.1-rc4/source/drivers/firmware/arm_ffa/bus.c#L213
> [2]:
https://lore.kernel.org/all/20221122131751.22747-10-abdellatif.elkhl...@arm.com/
>
Regards,
SImon
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
On Tue, Nov 22, 2022 at 07:09:16PM -0700, Simon Glass wrote:
> should be called 'priov' and should beHi Abdellatif,
>
> On Tue, 22 Nov 2022 at 06:18, Abdellatif El Khlifi
> wrote:
> >
> > Add the core driver implementing Arm Firmware Framework for Armv8-A v1.0
> >
> > The Firmware Framework for Arm A-profile processors (FF-A v1.0) [1]
> > describes interfaces (ABIs) that standardize communication
> > between the Secure World and Normal World leveraging TrustZone
> > technology.
> >
> > This driver uses 64-bit registers as per SMCCCv1.2 spec and comes
> > on top of the SMCCC layer. The driver provides the FF-A ABIs needed for
> > querying the FF-A framework from the secure world.
> >
> > The driver uses SMC32 calling convention which means using the first
> > 32-bit data of the Xn registers.
> >
> > All supported ABIs come with their 32-bit version except FFA_RXTX_MAP
> > which has 64-bit version supported.
> >
> > Both 32-bit and 64-bit direct messaging are supported which allows both
> > 32-bit and 64-bit clients to use the FF-A bus.
> >
> > In U-Boot FF-A design, FF-A is considered as a discoverable bus.
> > The Secure World is considered as one entity to communicate with
> > using the FF-A bus. FF-A communication is handled by one device and
> > one instance (the bus). This FF-A driver takes care of all the
> > interactions between Normal world and Secure World.
> >
> > The driver exports its operations to be used by upper layers.
> >
> > Exported operations:
> >
> > - partition_info_get
> > - sync_send_receive
> > - rxtx_unmap
> >
> > For more details please refer to the driver documentation [2].
> >
> > [1]: https://developer.arm.com/documentation/den0077/latest/
> > [2]: doc/arch/arm64.ffa.rst
> >
> > Signed-off-by: Abdellatif El Khlifi
> > Cc: Tom Rini
> > Cc: Simon Glass
> > Cc: Ilias Apalodimas
> > Cc: Jens Wiklander
> >
> > ---
> >
> > Changelog:
> > ===
> >
> > v8:
> >
> > * make ffa_get_partitions_info() second argument to be an SP count in both
> > modes
> > * update ffa_bus_prvdata_get() to return a pointer rather than a pointer
> > address
> > * remove packing from ffa_partition_info and ffa_send_direct_data structures
> > * pass the FF-A bus device to the bus operations
> >
> > v7:
> >
> > * add support for 32-bit direct messaging
> > * rename be_uuid_str_to_le_bin() to uuid_str_to_le_bin()
> > * improve the declaration of error handling mapping
> > * stating in doc/arch/arm64.ffa.rst that EFI runtime is not supported
> >
> > v6:
> >
> > * drop use of EFI runtime support (We decided with Linaro to add this later)
> > * drop discovery from initcalls (discovery will be on demand by FF-A users)
> > * set the alignment of the RX/TX buffers to the larger translation granule
> > size
> > * move FF-A RX/TX buffers unmapping at ExitBootServices() to a separate
> > commit
> > * update the documentation and move it to doc/arch/arm64.ffa.rst
> >
> > v4:
> >
> > * add doc/README.ffa.drv
> > * moving the FF-A driver work to drivers/firmware/arm-ffa
> > * use less #ifdefs in lib/efi_loader/efi_boottime.c and replace
> > #if defined by #if CONFIG_IS_ENABLED
> > * improving error handling by mapping the FF-A errors to standard errors
> > and logs
> > * replacing panics with an error log and returning an error code
> > * improving features discovery in FFA_FEATURES by introducing
> > rxtx_min_pages private data field
> > * add ffa_remove and ffa_unbind functions
> > * improve how the driver behaves when bus discovery is done more than
> > once
> >
> > v3:
> >
> > * align the interfaces of the U-Boot FF-A driver with those in the linux
> > FF-A driver
> > * remove the FF-A helper layer
> > * make the U-Boot FF-A driver independent from EFI
> > * provide an optional config that enables copying the driver data to EFI
> > runtime section at ExitBootServices service
> > * use 64-bit version of FFA_RXTX_MAP, FFA_MSG_SEND_DIRECT_{REQ, RESP}
> >
> > v2:
> >
> > * make FF-A bus discoverable using device_{bind, probe} APIs
> > * remove device tree support
> >
> > v1:
> >
> > * introduce FF-A bus driver with device tree support
> >
> > MAINTAINERS |7 +
> > doc/arch/arm64.ffa.rst| 218
> > doc/arch/index.rst|1 +
> > drivers/Kconfig |2 +
> > drivers/Makefile |1 +
> > drivers/firmware/arm-ffa/Kconfig | 30 +
> > drivers/firmware/arm-ffa/Makefile |6 +
> > drivers/firmware/arm-ffa/arm-ffa-uclass.c | 16 +
> > drivers/firmware/arm-ffa/arm_ffa_prv.h| 200
> > drivers/firmware/arm-ffa/core.c | 1315 +
> > include/arm_ffa.h | 97 ++
> > include/dm/uclass-id.h|4 +
> > 12 files changed, 1897 insertions(+)
> > create mode 100644 doc/arch/arm64.ffa.rst
> > create mode 100644 drivers/firmware/arm-ffa/Kconfig
> > create mode 1006
Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
should be called 'priov' and should beHi Abdellatif,
On Tue, 22 Nov 2022 at 06:18, Abdellatif El Khlifi
wrote:
>
> Add the core driver implementing Arm Firmware Framework for Armv8-A v1.0
>
> The Firmware Framework for Arm A-profile processors (FF-A v1.0) [1]
> describes interfaces (ABIs) that standardize communication
> between the Secure World and Normal World leveraging TrustZone
> technology.
>
> This driver uses 64-bit registers as per SMCCCv1.2 spec and comes
> on top of the SMCCC layer. The driver provides the FF-A ABIs needed for
> querying the FF-A framework from the secure world.
>
> The driver uses SMC32 calling convention which means using the first
> 32-bit data of the Xn registers.
>
> All supported ABIs come with their 32-bit version except FFA_RXTX_MAP
> which has 64-bit version supported.
>
> Both 32-bit and 64-bit direct messaging are supported which allows both
> 32-bit and 64-bit clients to use the FF-A bus.
>
> In U-Boot FF-A design, FF-A is considered as a discoverable bus.
> The Secure World is considered as one entity to communicate with
> using the FF-A bus. FF-A communication is handled by one device and
> one instance (the bus). This FF-A driver takes care of all the
> interactions between Normal world and Secure World.
>
> The driver exports its operations to be used by upper layers.
>
> Exported operations:
>
> - partition_info_get
> - sync_send_receive
> - rxtx_unmap
>
> For more details please refer to the driver documentation [2].
>
> [1]: https://developer.arm.com/documentation/den0077/latest/
> [2]: doc/arch/arm64.ffa.rst
>
> Signed-off-by: Abdellatif El Khlifi
> Cc: Tom Rini
> Cc: Simon Glass
> Cc: Ilias Apalodimas
> Cc: Jens Wiklander
>
> ---
>
> Changelog:
> ===
>
> v8:
>
> * make ffa_get_partitions_info() second argument to be an SP count in both
> modes
> * update ffa_bus_prvdata_get() to return a pointer rather than a pointer
> address
> * remove packing from ffa_partition_info and ffa_send_direct_data structures
> * pass the FF-A bus device to the bus operations
>
> v7:
>
> * add support for 32-bit direct messaging
> * rename be_uuid_str_to_le_bin() to uuid_str_to_le_bin()
> * improve the declaration of error handling mapping
> * stating in doc/arch/arm64.ffa.rst that EFI runtime is not supported
>
> v6:
>
> * drop use of EFI runtime support (We decided with Linaro to add this later)
> * drop discovery from initcalls (discovery will be on demand by FF-A users)
> * set the alignment of the RX/TX buffers to the larger translation granule
> size
> * move FF-A RX/TX buffers unmapping at ExitBootServices() to a separate commit
> * update the documentation and move it to doc/arch/arm64.ffa.rst
>
> v4:
>
> * add doc/README.ffa.drv
> * moving the FF-A driver work to drivers/firmware/arm-ffa
> * use less #ifdefs in lib/efi_loader/efi_boottime.c and replace
> #if defined by #if CONFIG_IS_ENABLED
> * improving error handling by mapping the FF-A errors to standard errors
> and logs
> * replacing panics with an error log and returning an error code
> * improving features discovery in FFA_FEATURES by introducing
> rxtx_min_pages private data field
> * add ffa_remove and ffa_unbind functions
> * improve how the driver behaves when bus discovery is done more than
> once
>
> v3:
>
> * align the interfaces of the U-Boot FF-A driver with those in the linux
> FF-A driver
> * remove the FF-A helper layer
> * make the U-Boot FF-A driver independent from EFI
> * provide an optional config that enables copying the driver data to EFI
> runtime section at ExitBootServices service
> * use 64-bit version of FFA_RXTX_MAP, FFA_MSG_SEND_DIRECT_{REQ, RESP}
>
> v2:
>
> * make FF-A bus discoverable using device_{bind, probe} APIs
> * remove device tree support
>
> v1:
>
> * introduce FF-A bus driver with device tree support
>
> MAINTAINERS |7 +
> doc/arch/arm64.ffa.rst| 218
> doc/arch/index.rst|1 +
> drivers/Kconfig |2 +
> drivers/Makefile |1 +
> drivers/firmware/arm-ffa/Kconfig | 30 +
> drivers/firmware/arm-ffa/Makefile |6 +
> drivers/firmware/arm-ffa/arm-ffa-uclass.c | 16 +
> drivers/firmware/arm-ffa/arm_ffa_prv.h| 200
> drivers/firmware/arm-ffa/core.c | 1315 +
> include/arm_ffa.h | 97 ++
> include/dm/uclass-id.h|4 +
> 12 files changed, 1897 insertions(+)
> create mode 100644 doc/arch/arm64.ffa.rst
> create mode 100644 drivers/firmware/arm-ffa/Kconfig
> create mode 100644 drivers/firmware/arm-ffa/Makefile
> create mode 100644 drivers/firmware/arm-ffa/arm-ffa-uclass.c
> create mode 100644 drivers/firmware/arm-ffa/arm_ffa_prv.h
> create mode 100644 drivers/firmware/arm-ffa/core.c
> create mode 100644 include/arm_ffa.h
This looks mostly OK to me. I have a few comments below.
[..]
> diff
[PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver
Add the core driver implementing Arm Firmware Framework for Armv8-A v1.0
The Firmware Framework for Arm A-profile processors (FF-A v1.0) [1]
describes interfaces (ABIs) that standardize communication
between the Secure World and Normal World leveraging TrustZone
technology.
This driver uses 64-bit registers as per SMCCCv1.2 spec and comes
on top of the SMCCC layer. The driver provides the FF-A ABIs needed for
querying the FF-A framework from the secure world.
The driver uses SMC32 calling convention which means using the first
32-bit data of the Xn registers.
All supported ABIs come with their 32-bit version except FFA_RXTX_MAP
which has 64-bit version supported.
Both 32-bit and 64-bit direct messaging are supported which allows both
32-bit and 64-bit clients to use the FF-A bus.
In U-Boot FF-A design, FF-A is considered as a discoverable bus.
The Secure World is considered as one entity to communicate with
using the FF-A bus. FF-A communication is handled by one device and
one instance (the bus). This FF-A driver takes care of all the
interactions between Normal world and Secure World.
The driver exports its operations to be used by upper layers.
Exported operations:
- partition_info_get
- sync_send_receive
- rxtx_unmap
For more details please refer to the driver documentation [2].
[1]: https://developer.arm.com/documentation/den0077/latest/
[2]: doc/arch/arm64.ffa.rst
Signed-off-by: Abdellatif El Khlifi
Cc: Tom Rini
Cc: Simon Glass
Cc: Ilias Apalodimas
Cc: Jens Wiklander
---
Changelog:
===
v8:
* make ffa_get_partitions_info() second argument to be an SP count in both
modes
* update ffa_bus_prvdata_get() to return a pointer rather than a pointer
address
* remove packing from ffa_partition_info and ffa_send_direct_data structures
* pass the FF-A bus device to the bus operations
v7:
* add support for 32-bit direct messaging
* rename be_uuid_str_to_le_bin() to uuid_str_to_le_bin()
* improve the declaration of error handling mapping
* stating in doc/arch/arm64.ffa.rst that EFI runtime is not supported
v6:
* drop use of EFI runtime support (We decided with Linaro to add this later)
* drop discovery from initcalls (discovery will be on demand by FF-A users)
* set the alignment of the RX/TX buffers to the larger translation granule size
* move FF-A RX/TX buffers unmapping at ExitBootServices() to a separate commit
* update the documentation and move it to doc/arch/arm64.ffa.rst
v4:
* add doc/README.ffa.drv
* moving the FF-A driver work to drivers/firmware/arm-ffa
* use less #ifdefs in lib/efi_loader/efi_boottime.c and replace
#if defined by #if CONFIG_IS_ENABLED
* improving error handling by mapping the FF-A errors to standard errors
and logs
* replacing panics with an error log and returning an error code
* improving features discovery in FFA_FEATURES by introducing
rxtx_min_pages private data field
* add ffa_remove and ffa_unbind functions
* improve how the driver behaves when bus discovery is done more than
once
v3:
* align the interfaces of the U-Boot FF-A driver with those in the linux
FF-A driver
* remove the FF-A helper layer
* make the U-Boot FF-A driver independent from EFI
* provide an optional config that enables copying the driver data to EFI
runtime section at ExitBootServices service
* use 64-bit version of FFA_RXTX_MAP, FFA_MSG_SEND_DIRECT_{REQ, RESP}
v2:
* make FF-A bus discoverable using device_{bind, probe} APIs
* remove device tree support
v1:
* introduce FF-A bus driver with device tree support
MAINTAINERS |7 +
doc/arch/arm64.ffa.rst| 218
doc/arch/index.rst|1 +
drivers/Kconfig |2 +
drivers/Makefile |1 +
drivers/firmware/arm-ffa/Kconfig | 30 +
drivers/firmware/arm-ffa/Makefile |6 +
drivers/firmware/arm-ffa/arm-ffa-uclass.c | 16 +
drivers/firmware/arm-ffa/arm_ffa_prv.h| 200
drivers/firmware/arm-ffa/core.c | 1315 +
include/arm_ffa.h | 97 ++
include/dm/uclass-id.h|4 +
12 files changed, 1897 insertions(+)
create mode 100644 doc/arch/arm64.ffa.rst
create mode 100644 drivers/firmware/arm-ffa/Kconfig
create mode 100644 drivers/firmware/arm-ffa/Makefile
create mode 100644 drivers/firmware/arm-ffa/arm-ffa-uclass.c
create mode 100644 drivers/firmware/arm-ffa/arm_ffa_prv.h
create mode 100644 drivers/firmware/arm-ffa/core.c
create mode 100644 include/arm_ffa.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 97b2f69f65..dcd32cf83a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -263,6 +263,13 @@ F: drivers/net/cortina_ni.h
F: drivers/net/phy/ca_phy.c
F: configs/cortina_presidio-asic-pnand_defconfig
+ARM FF-A
+M: Abdellatif El Khlifi
+S: Maintained
+F: doc/arch/arm64.ffa.rst
+F: drivers/firmware/arm-ffa/
+F: include/arm_ffa.h
+
ARM FREESCALE
