[RFC] dt-bindings: firmware: scmi: Introduce compatible string

From: Peng Fan <peng.fan@nxp.com>

Add compatible string for the protocols by adding new nodes
The current nodename pattern is "protocol@[0-9a-f]+$", the new node
name will be "scmi-[a-z\-]+$".
With compatible string and new nodename, cpufreq and devfreq could be
separated into two nodes. And fwdevlink could correctly link suppliers
and consumers.
With compatible string, and driver updated.
- Differnet vendor drivers with same SCMI protocol ID could be built in
  without concerning vendor A's driver got probed when using vendor B's
  SoC
- NXP scmi pinctrl and ARM scmi pinctrl could be both built in, without
  concerning arm scmi platform takes nxp scmi pinctrl node as supplier.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---

RFC:
 This may sounds like that adding compatible to resovle linux driver issue.
 Yes indeed. current scmi framework limitation makes it not work well with
 fwdevlink, wrong suppliers maybe linked to consumers.
 I have tried various's method to not introduce compatible, but rejected by
 fwdevlink maintainer or scmi maintainer
 There was a long discussion in [1][2][3].
 [1] https://lore.kernel.org/arm-scmi/20240729070325.2065286-1-peng.fan@oss.nxp.com/
 [2] https://lore.kernel.org/arm-scmi/20241225-scmi-fwdevlink-v1-0-e9a3a5341362@nxp.com/T/#mdd17c4b9b11af9fae0d5b6ec2e13756c2c6f977d
 [3] https://lore.kernel.org/arm-scmi/20250120-scmi-fwdevlink-v2-0-3af2fa37dbac@nxp.com/

 The binding changes are posted out to see whether DT maintainer's view on
 whether introduce compatible string is welcomed or not.
 I not include driver changes, because this is just to see whether people
 are happy with this or not.

Quote Sudeep's reply"
I am not blocking you. What I mentioned is I don't agree that DT can be used
to resolve this issue, but I don't have time or alternate solution ATM. So
if you propose DT based solution and the maintainers agree for the proposed
bindings I will take a look and help you to make that work. But I will raise
any objections I may have if the proposal has issues mainly around the
compatibility and ease of maintenance.
"

 So If compatible string is agreed, I could continue
 update driver part in next version or SCMI maintainer could help.

 .../bindings/firmware/arm,scmi.yaml           | 300 ++++++++++++++++--
 .../firmware/nxp,imx95-scmi-pinctrl.yaml      |   4 +
 .../bindings/firmware/nxp,imx95-scmi.yaml     |   6 +
 3 files changed, 275 insertions(+), 35 deletions(-)

On Wed, Feb 26, 2025 at 05:44:56PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> Add compatible string for the protocols by adding new nodes
> The current nodename pattern is "protocol@[0-9a-f]+$", the new node
> name will be "scmi-[a-z\-]+$".
> With compatible string and new nodename, cpufreq and devfreq could be
> separated into two nodes. And fwdevlink could correctly link suppliers
> and consumers.
> With compatible string, and driver updated.
> - Differnet vendor drivers with same SCMI protocol ID could be built in
>   without concerning vendor A's driver got probed when using vendor B's
>   SoC
> - NXP scmi pinctrl and ARM scmi pinctrl could be both built in, without
>   concerning arm scmi platform takes nxp scmi pinctrl node as supplier.

How are you going to handle DTs which aren't updated and still don't 
have compatible strings? Seems like that would be messy if not 
impossible.

> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> 
> RFC:
>  This may sounds like that adding compatible to resovle linux driver issue.
>  Yes indeed. current scmi framework limitation makes it not work well with
>  fwdevlink, wrong suppliers maybe linked to consumers.
>  I have tried various's method to not introduce compatible, but rejected by
>  fwdevlink maintainer or scmi maintainer
>  There was a long discussion in [1][2][3].
>  [1] https://lore.kernel.org/arm-scmi/20240729070325.2065286-1-peng.fan@oss.nxp.com/
>  [2] https://lore.kernel.org/arm-scmi/20241225-scmi-fwdevlink-v1-0-e9a3a5341362@nxp.com/T/#mdd17c4b9b11af9fae0d5b6ec2e13756c2c6f977d
>  [3] https://lore.kernel.org/arm-scmi/20250120-scmi-fwdevlink-v2-0-3af2fa37dbac@nxp.com/
> 
>  The binding changes are posted out to see whether DT maintainer's view on
>  whether introduce compatible string is welcomed or not.
>  I not include driver changes, because this is just to see whether people
>  are happy with this or not.
> 
> Quote Sudeep's reply"
> I am not blocking you. What I mentioned is I don't agree that DT can be used
> to resolve this issue, but I don't have time or alternate solution ATM. So
> if you propose DT based solution and the maintainers agree for the proposed
> bindings I will take a look and help you to make that work. But I will raise
> any objections I may have if the proposal has issues mainly around the
> compatibility and ease of maintenance.
> "

This all looks to me like SCMI has failed to provide common interfaces.

I'm indifferent. If everyone involved thinks adding compatibles will 
solve whatever the issues are, then it's going to be fine with me 
(other than the issue above). It doesn't seem like you have that, so I 
don't know that I'd keep going down this path.

Rob

On Wed, Feb 26, 2025 at 10:09:45AM -0600, Rob Herring wrote:
> On Wed, Feb 26, 2025 at 05:44:56PM +0800, Peng Fan (OSS) wrote:
> > Quote Sudeep's reply"
> > I am not blocking you. What I mentioned is I don't agree that DT can be used
> > to resolve this issue, but I don't have time or alternate solution ATM. So
> > if you propose DT based solution and the maintainers agree for the proposed
> > bindings I will take a look and help you to make that work. But I will raise
> > any objections I may have if the proposal has issues mainly around the
> > compatibility and ease of maintenance.
> > "
>
> This all looks to me like SCMI has failed to provide common interfaces.
>

We can look into this if having such common interface can solve this problem.

> I'm indifferent. If everyone involved thinks adding compatibles will
> solve whatever the issues are, then it's going to be fine with me
> (other than the issue above). It doesn't seem like you have that, so I
> don't know that I'd keep going down this path.

Sorry if I was ambiguous with my stance as quoted above. For me, 2 devices
pointing to the same node seems implementation issue rather than fixing/
working around by extending DT bindings like this $subject patch is
attempting.

If you disagree with that and think 2 devices in the kernel shouldn't
point to the same device tree node, then yes I see this is right approach
to take. ATM I don't know which is correct and what are other developer's
include DT maintainer opinion on this. I just didn't like the way Peng
was trying to solve it with some block/allow list which wouldn't have
fixed the issue or just created new ones.

--
Regards,
Sudeep

On Wed, Feb 26, 2025 at 10:09:45AM -0600, Rob Herring wrote:
>On Wed, Feb 26, 2025 at 05:44:56PM +0800, Peng Fan (OSS) wrote:
>> From: Peng Fan <peng.fan@nxp.com>
>> 
>> Add compatible string for the protocols by adding new nodes
>> The current nodename pattern is "protocol@[0-9a-f]+$", the new node
>> name will be "scmi-[a-z\-]+$".
>> With compatible string and new nodename, cpufreq and devfreq could be
>> separated into two nodes. And fwdevlink could correctly link suppliers
>> and consumers.
>> With compatible string, and driver updated.
>> - Differnet vendor drivers with same SCMI protocol ID could be built in
>>   without concerning vendor A's driver got probed when using vendor B's
>>   SoC
>> - NXP scmi pinctrl and ARM scmi pinctrl could be both built in, without
>>   concerning arm scmi platform takes nxp scmi pinctrl node as supplier.
>
>How are you going to handle DTs which aren't updated and still don't 
>have compatible strings? Seems like that would be messy if not 
>impossible.

The goal is to support 'reg' based protocol node and compatible based
protocol node both. I could not promise what the end would be, but things
will be tried to make clean.

>
>> 
>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>> ---
>> 
>> RFC:
>>  This may sounds like that adding compatible to resovle linux driver issue.
>>  Yes indeed. current scmi framework limitation makes it not work well with
>>  fwdevlink, wrong suppliers maybe linked to consumers.
>>  I have tried various's method to not introduce compatible, but rejected by
>>  fwdevlink maintainer or scmi maintainer
>>  There was a long discussion in [1][2][3].
>>  [1] https://lore.kernel.org/arm-scmi/20240729070325.2065286-1-peng.fan@oss.nxp.com/
>>  [2] https://lore.kernel.org/arm-scmi/20241225-scmi-fwdevlink-v1-0-e9a3a5341362@nxp.com/T/#mdd17c4b9b11af9fae0d5b6ec2e13756c2c6f977d
>>  [3] https://lore.kernel.org/arm-scmi/20250120-scmi-fwdevlink-v2-0-3af2fa37dbac@nxp.com/
>> 
>>  The binding changes are posted out to see whether DT maintainer's view on
>>  whether introduce compatible string is welcomed or not.
>>  I not include driver changes, because this is just to see whether people
>>  are happy with this or not.
>> 
>> Quote Sudeep's reply"
>> I am not blocking you. What I mentioned is I don't agree that DT can be used
>> to resolve this issue, but I don't have time or alternate solution ATM. So
>> if you propose DT based solution and the maintainers agree for the proposed
>> bindings I will take a look and help you to make that work. But I will raise
>> any objections I may have if the proposal has issues mainly around the
>> compatibility and ease of maintenance.
>> "
>
>This all looks to me like SCMI has failed to provide common interfaces.

What kind common interfaces from your view?

>
>I'm indifferent. If everyone involved thinks adding compatibles will 
>solve whatever the issues are, then it's going to be fine with me 
>(other than the issue above). It doesn't seem like you have that, so I 
>don't know that I'd keep going down this path.

There is no way to build correct supplier and consumer using fw_devlink with 
current scmi reg based protocol node.

To build correct fw_devlink supplier and consumer, need provide
more nodes, not one node for multiple devices.

As fw_devlink maintainer said in
https://lore.kernel.org/arm-scmi/CAGETcx8m48cy-EzP6_uoGN7KWsQw=CfZWQ-hNUzz_7LZ0voG8A@mail.gmail.com/:
I also pasted at end.

"
The problem isn't so much that fw_devlink doesn't want to support
multiple devices getting instantiated from one DT node. The problem is
that there's no way to know which of the multiple devices is the real
supplier just by looking at the information in devicetree/firmware
(the fw in fw_devlink). And keep in mind that one of the main
requirements of fw_devlink is to work before any driver is loaded and
not depend on drivers for correctness of the dependency information
because it needs to work on a fully modular kernel too. So, fw_devlink
just picks the first device that's instantiated from a DT node.

I really hate folks creating multiple devices from one DT node. One IP
block can support multiple things, there's no need to instantiate
multiple devices for it. The same driver could have just as easily
registered with multiple frameworks. So, ideally I'd want us to fix
this issue in the SCMI framework code. In the case where the same SCMI
node is creating two devices, can they both probe successfully? If
yes, why are we not using a child node or a separate node for this
second device? If it's always one or the other, why are we creating
two devices? Can you please point to specific upstream DT examples for
me to get a better handle on what's going on?

Btw, there is the deferred_probe_timeout command line option that can
be used so that fw_devlink stops enforcing dependencies where there
are no supplier drivers for a device after a timeout. It's not ideal,
but it's something to unblock you.

The best fw_devlink could do is just not enforce any dependencies if
there is more than one device instantiated for a given supplier DT
node.
"

Thanks,
Peng
>
>Rob

On Wed, Feb 26, 2025 at 05:19:53PM +0000, Sudeep Holla wrote:
>On Wed, Feb 26, 2025 at 10:09:45AM -0600, Rob Herring wrote:
>> On Wed, Feb 26, 2025 at 05:44:56PM +0800, Peng Fan (OSS) wrote:
>> > Quote Sudeep's reply"
>> > I am not blocking you. What I mentioned is I don't agree that DT can be used
>> > to resolve this issue, but I don't have time or alternate solution ATM. So
>> > if you propose DT based solution and the maintainers agree for the proposed
>> > bindings I will take a look and help you to make that work. But I will raise
>> > any objections I may have if the proposal has issues mainly around the
>> > compatibility and ease of maintenance.
>> > "
>>
>> This all looks to me like SCMI has failed to provide common interfaces.
>>
>
>We can look into this if having such common interface can solve this problem.
>
>> I'm indifferent. If everyone involved thinks adding compatibles will
>> solve whatever the issues are, then it's going to be fine with me
>> (other than the issue above). It doesn't seem like you have that, so I
>> don't know that I'd keep going down this path.
>
>Sorry if I was ambiguous with my stance as quoted above. For me, 2 devices
>pointing to the same node seems implementation issue rather than fixing/
>working around by extending DT bindings like this $subject patch is
>attempting.
>
>If you disagree with that and think 2 devices in the kernel shouldn't
>point to the same device tree node, then yes I see this is right approach
>to take. ATM I don't know which is correct and what are other developer's
>include DT maintainer opinion on this. I just didn't like the way Peng
>was trying to solve it with some block/allow list which wouldn't have
>fixed the issue or just created new ones.

With compatible string, no need block/allow list anymore I think.

But honestly I have not spend efforts on do driver changes to support
compatible string. If in the end we all agree on the proposal,
I could start on driver changes.

Thanks,
Peng

>
>--
>Regards,
>Sudeep

On Thu, Feb 27, 2025 at 11:15:51AM +0800, Peng Fan wrote:
> On Wed, Feb 26, 2025 at 05:19:53PM +0000, Sudeep Holla wrote:
> >On Wed, Feb 26, 2025 at 10:09:45AM -0600, Rob Herring wrote:
> >> On Wed, Feb 26, 2025 at 05:44:56PM +0800, Peng Fan (OSS) wrote:
> >> > Quote Sudeep's reply"
> >> > I am not blocking you. What I mentioned is I don't agree that DT can be used
> >> > to resolve this issue, but I don't have time or alternate solution ATM. So
> >> > if you propose DT based solution and the maintainers agree for the proposed
> >> > bindings I will take a look and help you to make that work. But I will raise
> >> > any objections I may have if the proposal has issues mainly around the
> >> > compatibility and ease of maintenance.
> >> > "
> >>
> >> This all looks to me like SCMI has failed to provide common interfaces.
> >>
> >
> >We can look into this if having such common interface can solve this problem.
> >
> >> I'm indifferent. If everyone involved thinks adding compatibles will
> >> solve whatever the issues are, then it's going to be fine with me
> >> (other than the issue above). It doesn't seem like you have that, so I
> >> don't know that I'd keep going down this path.
> >
> >Sorry if I was ambiguous with my stance as quoted above. For me, 2 devices
> >pointing to the same node seems implementation issue rather than fixing/
> >working around by extending DT bindings like this $subject patch is
> >attempting.
> >
> >If you disagree with that and think 2 devices in the kernel shouldn't
> >point to the same device tree node, then yes I see this is right approach
> >to take. ATM I don't know which is correct and what are other developer's
> >include DT maintainer opinion on this. I just didn't like the way Peng
> >was trying to solve it with some block/allow list which wouldn't have
> >fixed the issue or just created new ones.
> 
> With compatible string, no need block/allow list anymore I think.
> 

I completely understand that, I was referring to your earlier alternative
solution to this $subject approach. Sorry if that was evidently clear.

On Wed, Feb 26, 2025 at 05:44:56PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> Add compatible string for the protocols by adding new nodes
> The current nodename pattern is "protocol@[0-9a-f]+$", the new node

Hi Peng,

> name will be "scmi-[a-z\-]+$".
> With compatible string and new nodename, cpufreq and devfreq could be
> separated into two nodes. And fwdevlink could correctly link suppliers
> and consumers.

beside the backward compatibility issues that Rob mentioned, the thing
that worries me most is that, while the current bindings describe the
SCMI protocols because the protocols are WHAT the platform FW exposes
(and that is all that is needed by drivers to refer to a protocol and
its resources)...here you are getting rid of all of this, and moving to
describe basically the various devices that will use a protocol,
potentially the same protocol, just to have a distinct fw_node ...
(...I mean my understanding is that there wont be any protocol nodes
left when the scmi- variant are present and, once, somehow, we will
have transitioned into this...right ?)

I haven't really had the time to go through properly your proposed
solution to understand fully all its possible side-effects and how
many SCMI features could be destroyed by removing protocol nodes
descriptor as a whole...but...off the top of my head, as a quick
example, how you will define a per-protocol dedicated transport
channel in this new scenario ?
...because You wont have anymore a protocol descriptor where to fit
this AND you could have multiple DT nodes describing drivers that use
that SAME protocol, so using this new nodes to fit the same
transport-chan descriptors wont be possible either....

IOW, sincerely, I understand you want to resolve the problem with
fw_devlink (me too), but nuking down everything, while loosing, possibly,
a number of the existing functionalities of the SCMI stack just to make
it work with fw_devlink at all cost it does not seem to me an acceptable
trade-off...

...killing the whole existing protocols descriptors structure seems to
me a recipe for disaster, also because, it just goes against the very
essence of the objects that the FW exposes and the bindings can describe:
as an example, the SCMI platform server manage and exposes PERF_PROTOCOL
and its related DOMAINS (all fully discoverable without any bindings),
so, THAT is what is described in the bindings and referred by SCMI driver
users: SCMI FW does NOT handle/expose TWO distinct perf devices, like the
cpufreq/devfreq-device that you are trying to describe...

As Sudeep mentioned, IMHO this seems mostly an *unsolved* implementation
problem more than an actual issue with the bindings and how we describe
SCMI resources that we need to refer to..

> With compatible string, and driver updated.
> - Differnet vendor drivers with same SCMI protocol ID could be built in
>   without concerning vendor A's driver got probed when using vendor B's
>   SoC

as said, this is a corner case that is easily solvable with the current
layout (and I will post a patch soon-ish to addess this...)

> - NXP scmi pinctrl and ARM scmi pinctrl could be both built in, without
>   concerning arm scmi platform takes nxp scmi pinctrl node as supplier.
> 

..the only real issue is the general fw_devlink issue as in cpufreq vs
devfreq...

Thanks,
Cristian

On Wed, Feb 26, 2025 at 8:02 PM Peng Fan <peng.fan@oss.nxp.com> wrote:
>
> On Wed, Feb 26, 2025 at 10:09:45AM -0600, Rob Herring wrote:
> >On Wed, Feb 26, 2025 at 05:44:56PM +0800, Peng Fan (OSS) wrote:
> >> From: Peng Fan <peng.fan@nxp.com>
> >>
> >> Add compatible string for the protocols by adding new nodes
> >> The current nodename pattern is "protocol@[0-9a-f]+$", the new node
> >> name will be "scmi-[a-z\-]+$".
> >> With compatible string and new nodename, cpufreq and devfreq could be
> >> separated into two nodes. And fwdevlink could correctly link suppliers
> >> and consumers.
> >> With compatible string, and driver updated.
> >> - Differnet vendor drivers with same SCMI protocol ID could be built in
> >>   without concerning vendor A's driver got probed when using vendor B's
> >>   SoC
> >> - NXP scmi pinctrl and ARM scmi pinctrl could be both built in, without
> >>   concerning arm scmi platform takes nxp scmi pinctrl node as supplier.
> >
> >How are you going to handle DTs which aren't updated and still don't
> >have compatible strings? Seems like that would be messy if not
> >impossible.
>
> The goal is to support 'reg' based protocol node and compatible based
> protocol node both. I could not promise what the end would be, but things
> will be tried to make clean.
>
> >
> >>
> >> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> >> ---
> >>
> >> RFC:
> >>  This may sounds like that adding compatible to resovle linux driver issue.
> >>  Yes indeed. current scmi framework limitation makes it not work well with
> >>  fwdevlink, wrong suppliers maybe linked to consumers.
> >>  I have tried various's method to not introduce compatible, but rejected by
> >>  fwdevlink maintainer or scmi maintainer
> >>  There was a long discussion in [1][2][3].
> >>  [1] https://lore.kernel.org/arm-scmi/20240729070325.2065286-1-peng.fan@oss.nxp.com/
> >>  [2] https://lore.kernel.org/arm-scmi/20241225-scmi-fwdevlink-v1-0-e9a3a5341362@nxp.com/T/#mdd17c4b9b11af9fae0d5b6ec2e13756c2c6f977d
> >>  [3] https://lore.kernel.org/arm-scmi/20250120-scmi-fwdevlink-v2-0-3af2fa37dbac@nxp.com/
> >>
> >>  The binding changes are posted out to see whether DT maintainer's view on
> >>  whether introduce compatible string is welcomed or not.
> >>  I not include driver changes, because this is just to see whether people
> >>  are happy with this or not.
> >>
> >> Quote Sudeep's reply"
> >> I am not blocking you. What I mentioned is I don't agree that DT can be used
> >> to resolve this issue, but I don't have time or alternate solution ATM. So
> >> if you propose DT based solution and the maintainers agree for the proposed
> >> bindings I will take a look and help you to make that work. But I will raise
> >> any objections I may have if the proposal has issues mainly around the
> >> compatibility and ease of maintenance.
> >> "
> >
> >This all looks to me like SCMI has failed to provide common interfaces.
>
> What kind common interfaces from your view?

"NXP scmi pinctrl and ARM scmi pinctrl "

I take it that the ARM implementation is not sufficient, so NXP had to
create their own?


> >I'm indifferent. If everyone involved thinks adding compatibles will
> >solve whatever the issues are, then it's going to be fine with me
> >(other than the issue above). It doesn't seem like you have that, so I
> >don't know that I'd keep going down this path.
>
> There is no way to build correct supplier and consumer using fw_devlink with
> current scmi reg based protocol node.
>
> To build correct fw_devlink supplier and consumer, need provide
> more nodes, not one node for multiple devices.
>
> As fw_devlink maintainer said in
> https://lore.kernel.org/arm-scmi/CAGETcx8m48cy-EzP6_uoGN7KWsQw=CfZWQ-hNUzz_7LZ0voG8A@mail.gmail.com/:
> I also pasted at end.
>
> "
> The problem isn't so much that fw_devlink doesn't want to support
> multiple devices getting instantiated from one DT node. The problem is
> that there's no way to know which of the multiple devices is the real
> supplier just by looking at the information in devicetree/firmware
> (the fw in fw_devlink). And keep in mind that one of the main
> requirements of fw_devlink is to work before any driver is loaded and
> not depend on drivers for correctness of the dependency information
> because it needs to work on a fully modular kernel too. So, fw_devlink
> just picks the first device that's instantiated from a DT node.
>
> I really hate folks creating multiple devices from one DT node. One IP
> block can support multiple things, there's no need to instantiate
> multiple devices for it. The same driver could have just as easily
> registered with multiple frameworks. So, ideally I'd want us to fix
> this issue in the SCMI framework code. In the case where the same SCMI
> node is creating two devices, can they both probe successfully? If
> yes, why are we not using a child node or a separate node for this
> second device? If it's always one or the other, why are we creating
> two devices? Can you please point to specific upstream DT examples for
> me to get a better handle on what's going on?

I'm not clear on why you need 2 devices for 1 protocol.

This is a common thing where DT nodes and OS driver instances are not
1:1. We have to support that because h/w structure is not always going
to perfectly match what one OS's driver model prefers (which can
evolve over time). DT is not an abstraction layer for that. Creating
child nodes as suggested is NOT what I want to see.

There's 2 ways we can handle this:
- One driver registers with multiple frameworks
- The parent driver creates child devices. The child devices can
either reuse the parent DT node (i.e. set dev.of_node) or just get it
from the parent device if needed.

Unfortunately, I think we have to support both ways in general. I
don't know about SCMI specifically.

Perhaps fw_devlink needs a way for the parent driver to resolve the
dependencies. So fw_devlink finds the parent node as a dependency, and
then somehow calls its driver which can then check if the child driver
is ready. It would need to know the type of consumer though.

Rob

On Fri, Feb 28, 2025 at 07:34:09AM -0600, Rob Herring wrote:
>
> - The parent driver creates child devices. The child devices can
> either reuse the parent DT node (i.e. set dev.of_node) or just get it
> from the parent device if needed.
>

This is exactly what I was thinking to deal with the issue since this
discussion started. I will give this a go. I believe this must solve
the issue, but I didn't want to spit it out loud until I tried to hack
and check.

--
Regards,
Sudeep

On Fri, Feb 28, 2025 at 8:04 AM Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> On Fri, Feb 28, 2025 at 07:34:09AM -0600, Rob Herring wrote:
> >
> > - The parent driver creates child devices. The child devices can
> > either reuse the parent DT node (i.e. set dev.of_node) or just get it
> > from the parent device if needed.
> >
>
> This is exactly what I was thinking to deal with the issue since this
> discussion started. I will give this a go. I believe this must solve
> the issue, but I didn't want to spit it out loud until I tried to hack
> and check.

The issue with fw_devlink is that it only checks the dependency of the
parent which won't be enough. When the parent's probe creates the
child device, that doesn't mean the child has probed. The child driver
might not be loaded and/or probe is async. I don't think there's
anyway for the parent probe to wait for child drivers to be probed and
ready. I think there's similar issues with the DWC3 wrapper and core
driver split.

Rob

Hi Cristian,
On Thu, Feb 27, 2025 at 11:48:51AM +0000, Cristian Marussi wrote:
>On Wed, Feb 26, 2025 at 05:44:56PM +0800, Peng Fan (OSS) wrote:
>> From: Peng Fan <peng.fan@nxp.com>
>> 
>> Add compatible string for the protocols by adding new nodes
>> The current nodename pattern is "protocol@[0-9a-f]+$", the new node
>
>Hi Peng,
>
>> name will be "scmi-[a-z\-]+$".
>> With compatible string and new nodename, cpufreq and devfreq could be
>> separated into two nodes. And fwdevlink could correctly link suppliers
>> and consumers.
>
>beside the backward compatibility issues that Rob mentioned, the thing
>that worries me most is that, while the current bindings describe the
>SCMI protocols because the protocols are WHAT the platform FW exposes
>(and that is all that is needed by drivers to refer to a protocol and
>its resources)...here you are getting rid of all of this, and moving to
>describe basically the various devices that will use a protocol,
>potentially the same protocol, just to have a distinct fw_node ...
>(...I mean my understanding is that there wont be any protocol nodes
>left when the scmi- variant are present and, once, somehow, we will
>have transitioned into this...right ?)
>
>I haven't really had the time to go through properly your proposed
>solution to understand fully all its possible side-effects and how
>many SCMI features could be destroyed by removing protocol nodes
>descriptor as a whole...but...off the top of my head, as a quick
>example, how you will define a per-protocol dedicated transport
>channel in this new scenario ?

This is an issue for current RFC.

>...because You wont have anymore a protocol descriptor where to fit
>this AND you could have multiple DT nodes describing drivers that use
>that SAME protocol, so using this new nodes to fit the same
>transport-chan descriptors wont be possible either....

The other alternative would be

protocol@abc {
	//
	current property not change.

	// Add two new optional subnodes
	cpufreq {
		compatible = "arm,scmi-cpufreq";
	};
	devfreq {
		compatible = "arm,scmi-devfreq";
	};
};

protocol@abc {
	//
	current property not change.
	//add a new optional compatible
	compatible = "arm,scmi-pinctrl";
};

If compatible exists, the driver could use compatible to match.

>
>IOW, sincerely, I understand you want to resolve the problem with
>fw_devlink (me too), but nuking down everything, while loosing, possibly,
>a number of the existing functionalities of the SCMI stack just to make
>it work with fw_devlink at all cost it does not seem to me an acceptable
>trade-off...
>
>...killing the whole existing protocols descriptors structure seems to
>me a recipe for disaster, also because, it just goes against the very
>essence of the objects that the FW exposes and the bindings can describe:

no. I was trying to bring backwards compatible solution.

If compatible is there, use compatible. If no compatible, still use reg.

>as an example, the SCMI platform server manage and exposes PERF_PROTOCOL
>and its related DOMAINS (all fully discoverable without any bindings),
>so, THAT is what is described in the bindings and referred by SCMI driver
>users: SCMI FW does NOT handle/expose TWO distinct perf devices, like the
>cpufreq/devfreq-device that you are trying to describe...

I understand, fw only provides one interface, but linux use two distinct
drivers. But this depends on how to abstract the interface for OS usage.

The interface supports CPU freq scaling and peripherals freq scaling,
I think it should be ok to add one node for each, as below:
protocol@xx {
	cpufreq {}
	devfreq {}
}

Thanks,
Peng

>
>As Sudeep mentioned, IMHO this seems mostly an *unsolved* implementation
>problem more than an actual issue with the bindings and how we describe
>SCMI resources that we need to refer to..
>
>> With compatible string, and driver updated.
>> - Differnet vendor drivers with same SCMI protocol ID could be built in
>>   without concerning vendor A's driver got probed when using vendor B's
>>   SoC
>
>as said, this is a corner case that is easily solvable with the current
>layout (and I will post a patch soon-ish to addess this...)
>
>> - NXP scmi pinctrl and ARM scmi pinctrl could be both built in, without
>>   concerning arm scmi platform takes nxp scmi pinctrl node as supplier.
>> 
>
>..the only real issue is the general fw_devlink issue as in cpufreq vs
>devfreq...

Not sure iiodev or thermal has same issue or not.

>
>Thanks,
>Cristian

Hi Rob,
On Fri, Feb 28, 2025 at 08:17:03AM -0600, Rob Herring wrote:
>On Fri, Feb 28, 2025 at 8:04 AM Sudeep Holla <sudeep.holla@arm.com> wrote:
>>
>> On Fri, Feb 28, 2025 at 07:34:09AM -0600, Rob Herring wrote:
>> >
>> > - The parent driver creates child devices. The child devices can
>> > either reuse the parent DT node (i.e. set dev.of_node) or just get it
>> > from the parent device if needed.
>> >
>>
>> This is exactly what I was thinking to deal with the issue since this
>> discussion started. I will give this a go. I believe this must solve
>> the issue, but I didn't want to spit it out loud until I tried to hack
>> and check.
>

>The issue with fw_devlink is that it only checks the dependency of the
>parent which won't be enough. When the parent's probe creates the
>child device, that doesn't mean the child has probed. The child driver
>might not be loaded and/or probe is async. I don't think there's
>anyway for the parent probe to wait for child drivers to be probed and

Please forgive if my understanding is wrong.
Based on device tree, there is fwnode link created using fwnode_link_add,
then in device_add, the fw_devlink_link_device will do the device supplier
and consumer link. It is just device level link, not related to child's probe.

So if child device(work as supplier) is created only when a prarent's probe
done, the consumer of the child device will not have link ready and consumer
device's driver will not probe until the child device created and probe done.

Thanks,
Peng

>ready. I think there's similar issues with the DWC3 wrapper and core
>driver split.
>
>Rob