Message-ID: <CAL_JsqJOqKeDRuASWxCT=EA5LJbONpCX=Re8=XxKUbPToWy2Dg@mail.gmail.com>
Date: Fri, 28 Feb 2025 07:34:09 -0600
From: Rob Herring <robh@...nel.org>
To: Peng Fan <peng.fan@....nxp.com>, saravanak@...gle.com
Cc: sudeep.holla@....com, cristian.marussi@....com, krzk+dt@...nel.org, 
	conor+dt@...nel.org, arm-scmi@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Peng Fan <peng.fan@....com>
Subject: Re: [RFC] dt-bindings: firmware: scmi: Introduce compatible string

On Wed, Feb 26, 2025 at 8:02 PM Peng Fan <peng.fan@....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@....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@....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

Powered by blists - more mailing lists