diff mbox series

[v2,08/11] dt-bindings: firmware: arm,scmi: Extend bindings for protocol@13

Message ID 20230713141738.23970-9-ulf.hansson@linaro.org
State Changes Requested, archived
Headers show
Series None | expand

Checks

Context Check Description
robh/checkpatch success
robh/patch-applied success
robh/dtbs-check warning build log
robh/dt-meta-schema success

Commit Message

Ulf Hansson July 13, 2023, 2:17 p.m. UTC
The protocol@13 node is describing the performance scaling option for the
ARM SCMI interface, as a clock provider. This is unnecessary limiting, as
performance scaling is in many cases not limited to switching a clock's
frequency.

Therefore, let's extend the binding so the interface can be modelled as a
generic performance domaintoo. The common way to describe this, is to use
the "power-domain" DT bindings, so let's use that.

Cc: Rob Herring <robh+dt@kernel.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v2:
	- Updated the DT binding to require "oneOf" #power-domain-cells or
	#clock-cells.

---
 .../devicetree/bindings/firmware/arm,scmi.yaml        | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

Comments

Sudeep Holla July 19, 2023, 3:17 p.m. UTC | #1
On Thu, Jul 13, 2023 at 04:17:35PM +0200, Ulf Hansson wrote:
> The protocol@13 node is describing the performance scaling option for the
> ARM SCMI interface, as a clock provider. This is unnecessary limiting, as
> performance scaling is in many cases not limited to switching a clock's
> frequency.
> 
> Therefore, let's extend the binding so the interface can be modelled as a
> generic performance domaintoo. The common way to describe this, is to use
> the "power-domain" DT bindings, so let's use that.
> 

One thing I forgot to ask earlier is how we can manage different domain IDs
for perf and power domains which is the case with current SCMI platforms as
the spec never mandated or can ever mandate the perf and power domains IDs
to match. They need not be same anyways.

--
Regards,
Sudeep
Ulf Hansson July 21, 2023, 11:42 a.m. UTC | #2
On Wed, 19 Jul 2023 at 17:17, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> On Thu, Jul 13, 2023 at 04:17:35PM +0200, Ulf Hansson wrote:
> > The protocol@13 node is describing the performance scaling option for the
> > ARM SCMI interface, as a clock provider. This is unnecessary limiting, as
> > performance scaling is in many cases not limited to switching a clock's
> > frequency.
> >
> > Therefore, let's extend the binding so the interface can be modelled as a
> > generic performance domaintoo. The common way to describe this, is to use
> > the "power-domain" DT bindings, so let's use that.
> >
>
> One thing I forgot to ask earlier is how we can manage different domain IDs
> for perf and power domains which is the case with current SCMI platforms as
> the spec never mandated or can ever mandate the perf and power domains IDs
> to match. They need not be same anyways.

Based upon what you describe above, I have modelled the perf-domain
and the power-domain as two separate power-domain providers.

A consumer device being hooked up to both domains, would specify the
domain IDs in the second power-domain-cell, along the lines of the
below. Then we would use power-domain-names to specify what each
power-domain represents.

power-domains = <&scmi_pd 2>, <&scmi_dvfs 4>;
power-domain-names = "power", "perf";

I hope this makes it clearer!?

Kind regards
Uffe
Sudeep Holla July 21, 2023, 11:55 a.m. UTC | #3
On Fri, Jul 21, 2023 at 01:42:43PM +0200, Ulf Hansson wrote:
> On Wed, 19 Jul 2023 at 17:17, Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> > On Thu, Jul 13, 2023 at 04:17:35PM +0200, Ulf Hansson wrote:
> > > The protocol@13 node is describing the performance scaling option for the
> > > ARM SCMI interface, as a clock provider. This is unnecessary limiting, as
> > > performance scaling is in many cases not limited to switching a clock's
> > > frequency.
> > >
> > > Therefore, let's extend the binding so the interface can be modelled as a
> > > generic performance domaintoo. The common way to describe this, is to use
> > > the "power-domain" DT bindings, so let's use that.
> > >
> >
> > One thing I forgot to ask earlier is how we can manage different domain IDs
> > for perf and power domains which is the case with current SCMI platforms as
> > the spec never mandated or can ever mandate the perf and power domains IDs
> > to match. They need not be same anyways.
> 
> Based upon what you describe above, I have modelled the perf-domain
> and the power-domain as two separate power-domain providers.
> 
> A consumer device being hooked up to both domains, would specify the
> domain IDs in the second power-domain-cell, along the lines of the
> below. Then we would use power-domain-names to specify what each
> power-domain represents.
> 
> power-domains = <&scmi_pd 2>, <&scmi_dvfs 4>;
> power-domain-names = "power", "perf";
>
> I hope this makes it clearer!?

Yes it make is clear definitely, but it does change the definition of the
generic binding of the "power-domains" property now. I am interesting in
the feedback from the binding maintainers with respect to that. Or is it
already present ? IIUC, the ones supported already are generally both
power and performance providers. May be it doesn't matter much, just
wanted to explicit ask and confirm those details.

--
Regards,
Sudeep
Rob Herring (Arm) July 21, 2023, 2:33 p.m. UTC | #4
On Fri, Jul 21, 2023 at 12:55:35PM +0100, Sudeep Holla wrote:
> On Fri, Jul 21, 2023 at 01:42:43PM +0200, Ulf Hansson wrote:
> > On Wed, 19 Jul 2023 at 17:17, Sudeep Holla <sudeep.holla@arm.com> wrote:
> > >
> > > On Thu, Jul 13, 2023 at 04:17:35PM +0200, Ulf Hansson wrote:
> > > > The protocol@13 node is describing the performance scaling option for the
> > > > ARM SCMI interface, as a clock provider. This is unnecessary limiting, as
> > > > performance scaling is in many cases not limited to switching a clock's
> > > > frequency.
> > > >
> > > > Therefore, let's extend the binding so the interface can be modelled as a
> > > > generic performance domaintoo. The common way to describe this, is to use
> > > > the "power-domain" DT bindings, so let's use that.
> > > >
> > >
> > > One thing I forgot to ask earlier is how we can manage different domain IDs
> > > for perf and power domains which is the case with current SCMI platforms as
> > > the spec never mandated or can ever mandate the perf and power domains IDs
> > > to match. They need not be same anyways.
> > 
> > Based upon what you describe above, I have modelled the perf-domain
> > and the power-domain as two separate power-domain providers.
> > 
> > A consumer device being hooked up to both domains, would specify the
> > domain IDs in the second power-domain-cell, along the lines of the
> > below. Then we would use power-domain-names to specify what each
> > power-domain represents.
> > 
> > power-domains = <&scmi_pd 2>, <&scmi_dvfs 4>;
> > power-domain-names = "power", "perf";
> >
> > I hope this makes it clearer!?
> 
> Yes it make is clear definitely, but it does change the definition of the
> generic binding of the "power-domains" property now. I am interesting in
> the feedback from the binding maintainers with respect to that. Or is it
> already present ? IIUC, the ones supported already are generally both
> power and performance providers. May be it doesn't matter much, just
> wanted to explicit ask and confirm those details.

I commented on v1.

Looks like abuse of "power-domains" to me, but nothing new really. 
Please define when to use a power domain vs. a perf domain and don't 
leave it up to the whims of the platform. Maybe perf domains was a 
mistake and they should be deprecated?

Rob
Sudeep Holla July 21, 2023, 6:38 p.m. UTC | #5
On Fri, Jul 21, 2023 at 08:33:04AM -0600, Rob Herring wrote:
> On Fri, Jul 21, 2023 at 12:55:35PM +0100, Sudeep Holla wrote:
> > On Fri, Jul 21, 2023 at 01:42:43PM +0200, Ulf Hansson wrote:
> > > On Wed, 19 Jul 2023 at 17:17, Sudeep Holla <sudeep.holla@arm.com> wrote:
> > > >
> > > > On Thu, Jul 13, 2023 at 04:17:35PM +0200, Ulf Hansson wrote:
> > > > > The protocol@13 node is describing the performance scaling option for the
> > > > > ARM SCMI interface, as a clock provider. This is unnecessary limiting, as
> > > > > performance scaling is in many cases not limited to switching a clock's
> > > > > frequency.
> > > > >
> > > > > Therefore, let's extend the binding so the interface can be modelled as a
> > > > > generic performance domaintoo. The common way to describe this, is to use
> > > > > the "power-domain" DT bindings, so let's use that.
> > > > >
> > > >
> > > > One thing I forgot to ask earlier is how we can manage different domain IDs
> > > > for perf and power domains which is the case with current SCMI platforms as
> > > > the spec never mandated or can ever mandate the perf and power domains IDs
> > > > to match. They need not be same anyways.
> > > 
> > > Based upon what you describe above, I have modelled the perf-domain
> > > and the power-domain as two separate power-domain providers.
> > > 
> > > A consumer device being hooked up to both domains, would specify the
> > > domain IDs in the second power-domain-cell, along the lines of the
> > > below. Then we would use power-domain-names to specify what each
> > > power-domain represents.
> > > 
> > > power-domains = <&scmi_pd 2>, <&scmi_dvfs 4>;
> > > power-domain-names = "power", "perf";
> > >
> > > I hope this makes it clearer!?
> > 
> > Yes it make is clear definitely, but it does change the definition of the
> > generic binding of the "power-domains" property now. I am interesting in
> > the feedback from the binding maintainers with respect to that. Or is it
> > already present ? IIUC, the ones supported already are generally both
> > power and performance providers. May be it doesn't matter much, just
> > wanted to explicit ask and confirm those details.
> 
> I commented on v1.
> 
> Looks like abuse of "power-domains" to me, but nothing new really. 
> Please define when to use a power domain vs. a perf domain and don't 
> leave it up to the whims of the platform. Maybe perf domains was a 
> mistake and they should be deprecated?
> 

Just a thought here, instead of deprecating it I was thinking if possible
to keep the power-domains and performance-domains separate and just extend
the genpd to handle the latter. There by we are not mixing up and creating
confusions that need more specific definitions in the binding(which is not
a big deal) but platforms getting it wrong inspite of that is a big problem.
Keep it separate makes it more aligned to the hardware and doesn't dilute
the definitions and probably avoids any possible mistakes due to that.

Sorry Ulf I am just not yet convinced to mix them up yet 😉 and wish you
don't convince me to. Let me know why the above suggestion won't work.
Ulf Hansson July 26, 2023, 11:12 a.m. UTC | #6
On Fri, 21 Jul 2023 at 20:38, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> On Fri, Jul 21, 2023 at 08:33:04AM -0600, Rob Herring wrote:
> > On Fri, Jul 21, 2023 at 12:55:35PM +0100, Sudeep Holla wrote:
> > > On Fri, Jul 21, 2023 at 01:42:43PM +0200, Ulf Hansson wrote:
> > > > On Wed, 19 Jul 2023 at 17:17, Sudeep Holla <sudeep.holla@arm.com> wrote:
> > > > >
> > > > > On Thu, Jul 13, 2023 at 04:17:35PM +0200, Ulf Hansson wrote:
> > > > > > The protocol@13 node is describing the performance scaling option for the
> > > > > > ARM SCMI interface, as a clock provider. This is unnecessary limiting, as
> > > > > > performance scaling is in many cases not limited to switching a clock's
> > > > > > frequency.
> > > > > >
> > > > > > Therefore, let's extend the binding so the interface can be modelled as a
> > > > > > generic performance domaintoo. The common way to describe this, is to use
> > > > > > the "power-domain" DT bindings, so let's use that.
> > > > > >
> > > > >
> > > > > One thing I forgot to ask earlier is how we can manage different domain IDs
> > > > > for perf and power domains which is the case with current SCMI platforms as
> > > > > the spec never mandated or can ever mandate the perf and power domains IDs
> > > > > to match. They need not be same anyways.
> > > >
> > > > Based upon what you describe above, I have modelled the perf-domain
> > > > and the power-domain as two separate power-domain providers.
> > > >
> > > > A consumer device being hooked up to both domains, would specify the
> > > > domain IDs in the second power-domain-cell, along the lines of the
> > > > below. Then we would use power-domain-names to specify what each
> > > > power-domain represents.
> > > >
> > > > power-domains = <&scmi_pd 2>, <&scmi_dvfs 4>;
> > > > power-domain-names = "power", "perf";
> > > >
> > > > I hope this makes it clearer!?
> > >
> > > Yes it make is clear definitely, but it does change the definition of the
> > > generic binding of the "power-domains" property now. I am interesting in
> > > the feedback from the binding maintainers with respect to that. Or is it
> > > already present ? IIUC, the ones supported already are generally both
> > > power and performance providers. May be it doesn't matter much, just
> > > wanted to explicit ask and confirm those details.
> >
> > I commented on v1.
> >
> > Looks like abuse of "power-domains" to me, but nothing new really.
> > Please define when to use a power domain vs. a perf domain and don't
> > leave it up to the whims of the platform. Maybe perf domains was a
> > mistake and they should be deprecated?
> >
>
> Just a thought here, instead of deprecating it I was thinking if possible
> to keep the power-domains and performance-domains separate and just extend
> the genpd to handle the latter. There by we are not mixing up and creating
> confusions that need more specific definitions in the binding(which is not
> a big deal) but platforms getting it wrong inspite of that is a big problem.
> Keep it separate makes it more aligned to the hardware and doesn't dilute
> the definitions and probably avoids any possible mistakes due to that.
>
> Sorry Ulf I am just not yet convinced to mix them up yet 😉 and wish you
> don't convince me to. Let me know why the above suggestion won't work.

The main point I think we need to consider too, is that on some
platforms, the power-domain and the performance-domain are managed
together by the FW. It is not really two separate things and hence it
would not quite be correct to describe it as two different types of
providers in DT.

If we should follow your suggestion above, to use the
performance-domain bindings, then I think we need an additional new
binding to cover the above mentioned case too. This would lead us into
having one binding for the power-domain, another for the
performance-domain and a third for the power+performance-domain.

In my opinion this sounds quite like a mess. I would rather keep using
the power-domain bindings for all these cases. Of course, it's a bit
of a stretch too, but I think it should be less confusing in the end,
assuming we extend/clarify the description of the power-domain
bindings, of course.

Did that convince you? :-)

Kind regards
Uffe
Ulf Hansson Aug. 21, 2023, 2:32 p.m. UTC | #7
On Wed, 26 Jul 2023 at 13:12, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Fri, 21 Jul 2023 at 20:38, Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> > On Fri, Jul 21, 2023 at 08:33:04AM -0600, Rob Herring wrote:
> > > On Fri, Jul 21, 2023 at 12:55:35PM +0100, Sudeep Holla wrote:
> > > > On Fri, Jul 21, 2023 at 01:42:43PM +0200, Ulf Hansson wrote:
> > > > > On Wed, 19 Jul 2023 at 17:17, Sudeep Holla <sudeep.holla@arm.com> wrote:
> > > > > >
> > > > > > On Thu, Jul 13, 2023 at 04:17:35PM +0200, Ulf Hansson wrote:
> > > > > > > The protocol@13 node is describing the performance scaling option for the
> > > > > > > ARM SCMI interface, as a clock provider. This is unnecessary limiting, as
> > > > > > > performance scaling is in many cases not limited to switching a clock's
> > > > > > > frequency.
> > > > > > >
> > > > > > > Therefore, let's extend the binding so the interface can be modelled as a
> > > > > > > generic performance domaintoo. The common way to describe this, is to use
> > > > > > > the "power-domain" DT bindings, so let's use that.
> > > > > > >
> > > > > >
> > > > > > One thing I forgot to ask earlier is how we can manage different domain IDs
> > > > > > for perf and power domains which is the case with current SCMI platforms as
> > > > > > the spec never mandated or can ever mandate the perf and power domains IDs
> > > > > > to match. They need not be same anyways.
> > > > >
> > > > > Based upon what you describe above, I have modelled the perf-domain
> > > > > and the power-domain as two separate power-domain providers.
> > > > >
> > > > > A consumer device being hooked up to both domains, would specify the
> > > > > domain IDs in the second power-domain-cell, along the lines of the
> > > > > below. Then we would use power-domain-names to specify what each
> > > > > power-domain represents.
> > > > >
> > > > > power-domains = <&scmi_pd 2>, <&scmi_dvfs 4>;
> > > > > power-domain-names = "power", "perf";
> > > > >
> > > > > I hope this makes it clearer!?
> > > >
> > > > Yes it make is clear definitely, but it does change the definition of the
> > > > generic binding of the "power-domains" property now. I am interesting in
> > > > the feedback from the binding maintainers with respect to that. Or is it
> > > > already present ? IIUC, the ones supported already are generally both
> > > > power and performance providers. May be it doesn't matter much, just
> > > > wanted to explicit ask and confirm those details.
> > >
> > > I commented on v1.
> > >
> > > Looks like abuse of "power-domains" to me, but nothing new really.
> > > Please define when to use a power domain vs. a perf domain and don't
> > > leave it up to the whims of the platform. Maybe perf domains was a
> > > mistake and they should be deprecated?
> > >
> >
> > Just a thought here, instead of deprecating it I was thinking if possible
> > to keep the power-domains and performance-domains separate and just extend
> > the genpd to handle the latter. There by we are not mixing up and creating
> > confusions that need more specific definitions in the binding(which is not
> > a big deal) but platforms getting it wrong inspite of that is a big problem.
> > Keep it separate makes it more aligned to the hardware and doesn't dilute
> > the definitions and probably avoids any possible mistakes due to that.
> >
> > Sorry Ulf I am just not yet convinced to mix them up yet 😉 and wish you
> > don't convince me to. Let me know why the above suggestion won't work.
>
> The main point I think we need to consider too, is that on some
> platforms, the power-domain and the performance-domain are managed
> together by the FW. It is not really two separate things and hence it
> would not quite be correct to describe it as two different types of
> providers in DT.
>
> If we should follow your suggestion above, to use the
> performance-domain bindings, then I think we need an additional new
> binding to cover the above mentioned case too. This would lead us into
> having one binding for the power-domain, another for the
> performance-domain and a third for the power+performance-domain.
>
> In my opinion this sounds quite like a mess. I would rather keep using
> the power-domain bindings for all these cases. Of course, it's a bit
> of a stretch too, but I think it should be less confusing in the end,
> assuming we extend/clarify the description of the power-domain
> bindings, of course.
>
> Did that convince you? :-)

Sudeep, Rob,

Can we try to conclude on the way forward?

Is it acceptable to keep using the power-domain bindings (with some
clarifications) for performance domains or should we start moving to
the performance-domain bindings?

If moving to the performance-domain binding, should we start migrating
existing users of the power-domain binding too - or what is your take
on this?

Kind regards
Uffe
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
index b138f3d23df8..563a87dfb31a 100644
--- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
+++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
@@ -149,8 +149,15 @@  properties:
       '#clock-cells':
         const: 1
 
-    required:
-      - '#clock-cells'
+      '#power-domain-cells':
+        const: 1
+
+    oneOf:
+      - required:
+          - '#clock-cells'
+
+      - required:
+          - '#power-domain-cells'
 
   protocol@14:
     $ref: '#/$defs/protocol-node'