[PATCH 1/2] dt-bindings: arm,scmi: Do not use clocks for SCMI performance domains

[PATCH 1/2] dt-bindings: arm,scmi: Do not use clocks for SCMI performance domains

[Sudeep Holla]

Commit dd461cd9183f ("opp: Allow dev_pm_opp_get_opp_table() to return
-EPROBE_DEFER") handles -EPROBE_DEFER for the clock/interconnects within
_allocate_opp_table() which is called from dev_pm_opp_add and it
now propagates the error back to the caller.

SCMI performance domain re-used clock bindings to keep it simple. However
with the above mentioned change, if clock property is present in a device
node, opps can't be added until clk_get succeeds. So in order to fix the
issue, we can register dummy clocks which is completely ugly.

Since there are no upstream users for the SCMI performance domain clock
bindings, let us introduce separate performance domain bindings for the
same.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 .../devicetree/bindings/arm/arm,scmi.txt      | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

Hi Rob/Viresh,

This is actually a fix for the regression I reported here[1].
I am not adding fixes tag as I am targeting in the same release and
also because it is not directly related.

Regards,
Sudeep

[1] https://lore.kernel.org/r/20201015180555.gacdzkofpibkdn2e@bogus

P.S.:/me records that this binding needs to be moved to yaml in v5.11

diff --git a/Documentation/devicetree/bindings/arm/arm,scmi.txt b/Documentation/devicetree/bindings/arm/arm,scmi.txt
index 55deb68230eb..0a6c1b495403 100644
--- a/Documentation/devicetree/bindings/arm/arm,scmi.txt
+++ b/Documentation/devicetree/bindings/arm/arm,scmi.txt
@@ -44,7 +44,7 @@ as described in the following sections. If the platform supports dedicated
 mboxes, mbox-names and shmem shall be present in the sub-node corresponding
 to that protocol.

-Clock/Performance bindings for the clocks/OPPs based on SCMI Message Protocol
+Clock bindings for the clocks based on SCMI Message Protocol
 ------------------------------------------------------------

 This binding uses the common clock binding[1].
@@ -52,6 +52,19 @@ This binding uses the common clock binding[1].
 Required properties:
 - #clock-cells : Should be 1. Contains the Clock ID value used by SCMI commands.

+Performance bindings for the OPPs based on SCMI Message Protocol
+------------------------------------------------------------
+
+Required properties:
+- #perf-domain-cells: Should be 1. Contains the performance domain ID value
+		      used by SCMI commands.
+
+* Property arm,scmi-perf-domain
+
+Devices supporting SCMI performance domain must set their "arm,scmi-perf-domain"
+property with phandle to a SCMI performance domain controller followed by the
+performance domain.
+
 Power domain bindings for the power domains based on SCMI Message Protocol
 ------------------------------------------------------------

@@ -152,7 +165,7 @@ firmware {

 		scmi_dvfs: protocol@13 {
 			reg = <0x13>;
-			#clock-cells = <1>;
+			#perf-domain-cells = <1>;
 		};

 		scmi_clk: protocol@14 {
@@ -175,7 +188,7 @@ firmware {
 cpu@0 {
 	...
 	reg = <0 0>;
-	clocks = <&scmi_dvfs 0>;
+	arm,scmi-perf-domain = <&scmi_dvfs 0>;
 };

 hdlcd@7ff60000 {
--
2.17.1

[PATCH 2/2] firmware: arm_scmi: Move away from clock devicetree bindings

[Sudeep Holla]

Commit dd461cd9183f ("opp: Allow dev_pm_opp_get_opp_table() to return
-EPROBE_DEFER") handles -EPROBE_DEFER for the clock/interconnects within
_allocate_opp_table() which is called from dev_pm_opp_add and it
now propagates the error back to the caller.

This breaks SCMI performance domains as we will never succeed to add any
OPPs. A quick fix would be to register dummy clocks which is completely
ugly and bigger fix which may break with some other change in future.

It is better to add separate binding for the same and use it. A separate
SCMI performance domain binding is introduced and let us use it here.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/arm_scmi/perf.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c
index 3e1e87012c95..e2a47b3eead1 100644
--- a/drivers/firmware/arm_scmi/perf.c
+++ b/drivers/firmware/arm_scmi/perf.c
@@ -629,13 +629,13 @@ static void scmi_perf_domain_init_fc(const struct scmi_handle *handle,
 /* Device specific ops */
 static int scmi_dev_domain_id(struct device *dev)
 {
-	struct of_phandle_args clkspec;
+	struct of_phandle_args spec;

-	if (of_parse_phandle_with_args(dev->of_node, "clocks", "#clock-cells",
-				       0, &clkspec))
+	if (of_parse_phandle_with_args(dev->of_node, "arm,scmi-perf-domain",
+				       "#perf-domain-cells", 0, &spec))
 		return -EINVAL;

-	return clkspec.args[0];
+	return spec.args[0];
 }

 static int scmi_dvfs_device_opps_add(const struct scmi_handle *handle,
--
2.17.1

Re: [PATCH 1/2] dt-bindings: arm,scmi: Do not use clocks for SCMI performance domains

[Rob Herring]

On Tue, Oct 20, 2020 at 3:37 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> Commit dd461cd9183f ("opp: Allow dev_pm_opp_get_opp_table() to return
> -EPROBE_DEFER") handles -EPROBE_DEFER for the clock/interconnects within
> _allocate_opp_table() which is called from dev_pm_opp_add and it
> now propagates the error back to the caller.
>
> SCMI performance domain re-used clock bindings to keep it simple. However
> with the above mentioned change, if clock property is present in a device
> node, opps can't be added until clk_get succeeds. So in order to fix the
> issue, we can register dummy clocks which is completely ugly.
>
> Since there are no upstream users for the SCMI performance domain clock
> bindings, let us introduce separate performance domain bindings for the
> same.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  .../devicetree/bindings/arm/arm,scmi.txt      | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
>
> Hi Rob/Viresh,
>
> This is actually a fix for the regression I reported here[1].
> I am not adding fixes tag as I am targeting in the same release and
> also because it is not directly related.
>
> Regards,
> Sudeep
>
> [1] https://lore.kernel.org/r/20201015180555.gacdzkofpibkdn2e@bogus
>
> P.S.:/me records that this binding needs to be moved to yaml in v5.11
>
> diff --git a/Documentation/devicetree/bindings/arm/arm,scmi.txt b/Documentation/devicetree/bindings/arm/arm,scmi.txt
> index 55deb68230eb..0a6c1b495403 100644
> --- a/Documentation/devicetree/bindings/arm/arm,scmi.txt
> +++ b/Documentation/devicetree/bindings/arm/arm,scmi.txt
> @@ -44,7 +44,7 @@ as described in the following sections. If the platform supports dedicated
>  mboxes, mbox-names and shmem shall be present in the sub-node corresponding
>  to that protocol.
>
> -Clock/Performance bindings for the clocks/OPPs based on SCMI Message Protocol
> +Clock bindings for the clocks based on SCMI Message Protocol
>  ------------------------------------------------------------
>
>  This binding uses the common clock binding[1].
> @@ -52,6 +52,19 @@ This binding uses the common clock binding[1].
>  Required properties:
>  - #clock-cells : Should be 1. Contains the Clock ID value used by SCMI commands.
>
> +Performance bindings for the OPPs based on SCMI Message Protocol
> +------------------------------------------------------------
> +
> +Required properties:
> +- #perf-domain-cells: Should be 1. Contains the performance domain ID value
> +                     used by SCMI commands.

When is this not 1 (IOW, you only need this if variable)? How would it
be used outside SCMI (given it has a generic name)?

> +
> +* Property arm,scmi-perf-domain

Yet this doesn't have a generic name. You mentioned on IRC this is
aligned with QCom, but why can't QCom use the same property here?

Really though, why can't you give SCMI a CPUs MPIDR and get its domain?

Rob

Re: [PATCH 1/2] dt-bindings: arm,scmi: Do not use clocks for SCMI performance domains

[Sudeep Holla]

On Wed, Oct 21, 2020 at 11:20:27AM -0500, Rob Herring wrote:
> On Tue, Oct 20, 2020 at 3:37 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> > Commit dd461cd9183f ("opp: Allow dev_pm_opp_get_opp_table() to return
> > -EPROBE_DEFER") handles -EPROBE_DEFER for the clock/interconnects within
> > _allocate_opp_table() which is called from dev_pm_opp_add and it
> > now propagates the error back to the caller.
> >
> > SCMI performance domain re-used clock bindings to keep it simple. However
> > with the above mentioned change, if clock property is present in a device
> > node, opps can't be added until clk_get succeeds. So in order to fix the
> > issue, we can register dummy clocks which is completely ugly.
> >
> > Since there are no upstream users for the SCMI performance domain clock
> > bindings, let us introduce separate performance domain bindings for the
> > same.
> >
> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > ---
> >  .../devicetree/bindings/arm/arm,scmi.txt      | 19 ++++++++++++++++---
> >  1 file changed, 16 insertions(+), 3 deletions(-)
> >
> > Hi Rob/Viresh,
> >
> > This is actually a fix for the regression I reported here[1].
> > I am not adding fixes tag as I am targeting in the same release and
> > also because it is not directly related.
> >
> > Regards,
> > Sudeep
> >
> > [1] https://lore.kernel.org/r/20201015180555.gacdzkofpibkdn2e@bogus
> >
> > P.S.:/me records that this binding needs to be moved to yaml in v5.11
> >
> > diff --git a/Documentation/devicetree/bindings/arm/arm,scmi.txt b/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > index 55deb68230eb..0a6c1b495403 100644
> > --- a/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > +++ b/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > @@ -44,7 +44,7 @@ as described in the following sections. If the platform supports dedicated
> >  mboxes, mbox-names and shmem shall be present in the sub-node corresponding
> >  to that protocol.
> >
> > -Clock/Performance bindings for the clocks/OPPs based on SCMI Message Protocol
> > +Clock bindings for the clocks based on SCMI Message Protocol
> >  ------------------------------------------------------------
> >
> >  This binding uses the common clock binding[1].
> > @@ -52,6 +52,19 @@ This binding uses the common clock binding[1].
> >  Required properties:
> >  - #clock-cells : Should be 1. Contains the Clock ID value used by SCMI commands.
> >
> > +Performance bindings for the OPPs based on SCMI Message Protocol
> > +------------------------------------------------------------
> > +
> > +Required properties:
> > +- #perf-domain-cells: Should be 1. Contains the performance domain ID value
> > +                     used by SCMI commands.
>
> When is this not 1 (IOW, you only need this if variable)? How would it
> be used outside SCMI (given it has a generic name)?
>

Ah, I thought we need this if phandle is followed by 1 or more arguments.
If it is not compulsory I can drop this or make it scmi specific if we
need it.

> > +
> > +* Property arm,scmi-perf-domain
>
> Yet this doesn't have a generic name. You mentioned on IRC this is
> aligned with QCom, but why can't QCom use the same property here?
>

This is SCMI firmware driven while they have hardware driven perf/freq
domains. So different drivers, need to distinguish between the two.

> Really though, why can't you give SCMI a CPUs MPIDR and get its domain?

Not a bad idea, will check if we can add this to the future specification.
Anyways we still need something with existing version of the spec.

--
Regards,
Sudeep

Re: [PATCH 1/2] dt-bindings: arm,scmi: Do not use clocks for SCMI performance domains

[Sudeep Holla]

On Wed, Oct 21, 2020 at 11:20:27AM -0500, Rob Herring wrote:
> On Tue, Oct 20, 2020 at 3:37 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
> >

[...]

>
> When is this not 1 (IOW, you only need this if variable)? How would it
> be used outside SCMI (given it has a generic name)?
>
> > +
> > +* Property arm,scmi-perf-domain
>
[...]

> Really though, why can't you give SCMI a CPUs MPIDR and get its domain?
>

Now I remembered why we can't use MPIDR. The spec talks about perf domains
for devices in generic. CPU is just a special device. We will still need
a mechanism to get device performance domain. So MPIDR idea was dropped to
keep it uniform across all the devices.

--
Regards,
Sudeep

Re: [PATCH 1/2] dt-bindings: arm,scmi: Do not use clocks for SCMI performance domains

[Rob Herring]

On Wed, Oct 21, 2020 at 11:30 AM Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> On Wed, Oct 21, 2020 at 11:20:27AM -0500, Rob Herring wrote:
> > On Tue, Oct 20, 2020 at 3:37 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
> > >
> > > Commit dd461cd9183f ("opp: Allow dev_pm_opp_get_opp_table() to return
> > > -EPROBE_DEFER") handles -EPROBE_DEFER for the clock/interconnects within
> > > _allocate_opp_table() which is called from dev_pm_opp_add and it
> > > now propagates the error back to the caller.
> > >
> > > SCMI performance domain re-used clock bindings to keep it simple. However
> > > with the above mentioned change, if clock property is present in a device
> > > node, opps can't be added until clk_get succeeds. So in order to fix the
> > > issue, we can register dummy clocks which is completely ugly.
> > >
> > > Since there are no upstream users for the SCMI performance domain clock
> > > bindings, let us introduce separate performance domain bindings for the
> > > same.
> > >
> > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > > ---
> > >  .../devicetree/bindings/arm/arm,scmi.txt      | 19 ++++++++++++++++---
> > >  1 file changed, 16 insertions(+), 3 deletions(-)
> > >
> > > Hi Rob/Viresh,
> > >
> > > This is actually a fix for the regression I reported here[1].
> > > I am not adding fixes tag as I am targeting in the same release and
> > > also because it is not directly related.
> > >
> > > Regards,
> > > Sudeep
> > >
> > > [1] https://lore.kernel.org/r/20201015180555.gacdzkofpibkdn2e@bogus
> > >
> > > P.S.:/me records that this binding needs to be moved to yaml in v5.11
> > >
> > > diff --git a/Documentation/devicetree/bindings/arm/arm,scmi.txt b/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > > index 55deb68230eb..0a6c1b495403 100644
> > > --- a/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > > +++ b/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > > @@ -44,7 +44,7 @@ as described in the following sections. If the platform supports dedicated
> > >  mboxes, mbox-names and shmem shall be present in the sub-node corresponding
> > >  to that protocol.
> > >
> > > -Clock/Performance bindings for the clocks/OPPs based on SCMI Message Protocol
> > > +Clock bindings for the clocks based on SCMI Message Protocol
> > >  ------------------------------------------------------------
> > >
> > >  This binding uses the common clock binding[1].
> > > @@ -52,6 +52,19 @@ This binding uses the common clock binding[1].
> > >  Required properties:
> > >  - #clock-cells : Should be 1. Contains the Clock ID value used by SCMI commands.
> > >
> > > +Performance bindings for the OPPs based on SCMI Message Protocol
> > > +------------------------------------------------------------
> > > +
> > > +Required properties:
> > > +- #perf-domain-cells: Should be 1. Contains the performance domain ID value
> > > +                     used by SCMI commands.
> >
> > When is this not 1 (IOW, you only need this if variable)? How would it
> > be used outside SCMI (given it has a generic name)?
> >
>
> Ah, I thought we need this if phandle is followed by 1 or more arguments.
> If it is not compulsory I can drop this or make it scmi specific if we
> need it.

No, your options are fixed or variable number of cells. If this is
generic, then maybe it needs to be variable. If it's SCMI specific
then it can likely be fixed unless you can think of other information
you may need in the cells.

> > > +
> > > +* Property arm,scmi-perf-domain
> >
> > Yet this doesn't have a generic name. You mentioned on IRC this is
> > aligned with QCom, but why can't QCom use the same property here?
> >
>
> This is SCMI firmware driven while they have hardware driven perf/freq
> domains. So different drivers, need to distinguish between the two.

So what if they are different drivers. That's *always* the case. The
clock provider(s) for 'clocks' is different for every SoC? I doesn't
matter who is the provider, it's the same information being described.

Rob

Re: [PATCH 1/2] dt-bindings: arm,scmi: Do not use clocks for SCMI performance domains

[Rob Herring]

On Wed, Oct 21, 2020 at 1:19 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> On Wed, Oct 21, 2020 at 11:20:27AM -0500, Rob Herring wrote:
> > On Tue, Oct 20, 2020 at 3:37 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
> > >
>
> [...]
>
> >
> > When is this not 1 (IOW, you only need this if variable)? How would it
> > be used outside SCMI (given it has a generic name)?
> >
> > > +
> > > +* Property arm,scmi-perf-domain
> >
> [...]
>
> > Really though, why can't you give SCMI a CPUs MPIDR and get its domain?
> >
>
> Now I remembered why we can't use MPIDR. The spec talks about perf domains
> for devices in generic. CPU is just a special device. We will still need
> a mechanism to get device performance domain. So MPIDR idea was dropped to
> keep it uniform across all the devices.

What implications to the binding are there for non-CPU devices? Do
they need more cells? How does this integrate our plethora of other PM
related bindings?

So somewhere in the firmware we're defining device X is domain 0,
device Y is domain 1, etc. Then we do this again in DT. Seems fragile
to define this information twice. I guess that's true for any number
space SCMI defines.

Rob

Re: [PATCH 1/2] dt-bindings: arm,scmi: Do not use clocks for SCMI performance domains

[Sudeep Holla]

On Fri, Oct 23, 2020 at 08:21:21AM -0500, Rob Herring wrote:
> On Wed, Oct 21, 2020 at 11:30 AM Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> > On Wed, Oct 21, 2020 at 11:20:27AM -0500, Rob Herring wrote:
> > > On Tue, Oct 20, 2020 at 3:37 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
> > > >
> > > > Commit dd461cd9183f ("opp: Allow dev_pm_opp_get_opp_table() to return
> > > > -EPROBE_DEFER") handles -EPROBE_DEFER for the clock/interconnects within
> > > > _allocate_opp_table() which is called from dev_pm_opp_add and it
> > > > now propagates the error back to the caller.
> > > >
> > > > SCMI performance domain re-used clock bindings to keep it simple. However
> > > > with the above mentioned change, if clock property is present in a device
> > > > node, opps can't be added until clk_get succeeds. So in order to fix the
> > > > issue, we can register dummy clocks which is completely ugly.
> > > >
> > > > Since there are no upstream users for the SCMI performance domain clock
> > > > bindings, let us introduce separate performance domain bindings for the
> > > > same.
> > > >
> > > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > > > ---
> > > >  .../devicetree/bindings/arm/arm,scmi.txt      | 19 ++++++++++++++++---
> > > >  1 file changed, 16 insertions(+), 3 deletions(-)
> > > >
> > > > Hi Rob/Viresh,
> > > >
> > > > This is actually a fix for the regression I reported here[1].
> > > > I am not adding fixes tag as I am targeting in the same release and
> > > > also because it is not directly related.
> > > >
> > > > Regards,
> > > > Sudeep
> > > >
> > > > [1] https://lore.kernel.org/r/20201015180555.gacdzkofpibkdn2e@bogus
> > > >
> > > > P.S.:/me records that this binding needs to be moved to yaml in v5.11
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/arm/arm,scmi.txt b/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > > > index 55deb68230eb..0a6c1b495403 100644
> > > > --- a/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > > > +++ b/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > > > @@ -44,7 +44,7 @@ as described in the following sections. If the platform supports dedicated
> > > >  mboxes, mbox-names and shmem shall be present in the sub-node corresponding
> > > >  to that protocol.
> > > >
> > > > -Clock/Performance bindings for the clocks/OPPs based on SCMI Message Protocol
> > > > +Clock bindings for the clocks based on SCMI Message Protocol
> > > >  ------------------------------------------------------------
> > > >
> > > >  This binding uses the common clock binding[1].
> > > > @@ -52,6 +52,19 @@ This binding uses the common clock binding[1].
> > > >  Required properties:
> > > >  - #clock-cells : Should be 1. Contains the Clock ID value used by SCMI commands.
> > > >
> > > > +Performance bindings for the OPPs based on SCMI Message Protocol
> > > > +------------------------------------------------------------
> > > > +
> > > > +Required properties:
> > > > +- #perf-domain-cells: Should be 1. Contains the performance domain ID value
> > > > +                     used by SCMI commands.
> > >
> > > When is this not 1 (IOW, you only need this if variable)? How would it
> > > be used outside SCMI (given it has a generic name)?
> > >
> >
> > Ah, I thought we need this if phandle is followed by 1 or more arguments.
> > If it is not compulsory I can drop this or make it scmi specific if we
> > need it.
>
> No, your options are fixed or variable number of cells. If this is
> generic, then maybe it needs to be variable. If it's SCMI specific
> then it can likely be fixed unless you can think of other information
> you may need in the cells.
>

Understood.

> > > > +
> > > > +* Property arm,scmi-perf-domain
> > >
> > > Yet this doesn't have a generic name. You mentioned on IRC this is
> > > aligned with QCom, but why can't QCom use the same property here?
> > >
> >
> > This is SCMI firmware driven while they have hardware driven perf/freq
> > domains. So different drivers, need to distinguish between the two.
>
> So what if they are different drivers. That's *always* the case. The
> clock provider(s) for 'clocks' is different for every SoC? I doesn't
> matter who is the provider, it's the same information being described.
>

Fair enough. I was basing my argument on the fact that Qcom has users for
those bindings and I see limited scope for consolidation as that binding
has more information about the cpufreq-hw hardware block.

--
Regards,
Sudeep

Re: [PATCH 1/2] dt-bindings: arm,scmi: Do not use clocks for SCMI performance domains

[Sudeep Holla]

On Fri, Oct 23, 2020 at 08:34:05AM -0500, Rob Herring wrote:
> On Wed, Oct 21, 2020 at 1:19 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> > On Wed, Oct 21, 2020 at 11:20:27AM -0500, Rob Herring wrote:
> > > On Tue, Oct 20, 2020 at 3:37 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
> > > >
> >
> > [...]
> >
> > >
> > > When is this not 1 (IOW, you only need this if variable)? How would it
> > > be used outside SCMI (given it has a generic name)?
> > >
> > > > +
> > > > +* Property arm,scmi-perf-domain
> > >
> > [...]
> >
> > > Really though, why can't you give SCMI a CPUs MPIDR and get its domain?
> > >
> >
> > Now I remembered why we can't use MPIDR. The spec talks about perf domains
> > for devices in generic. CPU is just a special device. We will still need
> > a mechanism to get device performance domain. So MPIDR idea was dropped to
> > keep it uniform across all the devices.
>
> What implications to the binding are there for non-CPU devices? Do
> they need more cells? How does this integrate our plethora of other PM
> related bindings?
>

Ideally it is just a device perf domain ID. SCMI f/w will just assign
perf domain IDs for both CPUs and other devices like GPUs sequentially
without any distinction.

However, I can't speak about other aspects of PM especially on wild
variety of platforms we have on Arm.

Today even with SCMI each device/cpu needs to track clock or performance,
reset, power, voltage, ...etc domains and their IDs needs to be passed
via DT.

We are thinking of making all these device ID centric in future. It means
if the device tree had scmi device ID for each of them, we must be able to
perform any power management or configuration management on that device.
SCMI f/w must then abstract everything at device level. Just a thought
as of now and it aligns with some of the ACPI concepts.

> So somewhere in the firmware we're defining device X is domain 0,
> device Y is domain 1, etc. Then we do this again in DT. Seems fragile
> to define this information twice. I guess that's true for any number
> space SCMI defines.
>

Correct and agreed on your point. Any ideas to make this discoverable ?
Atleast with SCMI, we have been able to reduce the amount of information
just to that ID(though there are multiple ID space today for each aspects
of PM and config management). As I mentioned we would like to make it
device centric. Any thoughts on making IDs discoverable is appreciated.

We thought about names and other things during initial days of the
spec evolution, but we circled back to how does OS provide that info and
we go back to DT/ACPI which was not too bad at that time. We can see if
we can improve anything there.

--
Regards,
Sudeep

Re: [PATCH 1/2] dt-bindings: arm,scmi: Do not use clocks for SCMI performance domains

[Sudeep Holla]

On Fri, Oct 23, 2020 at 08:21:21AM -0500, Rob Herring wrote:
> On Wed, Oct 21, 2020 at 11:30 AM Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> > On Wed, Oct 21, 2020 at 11:20:27AM -0500, Rob Herring wrote:
> > > On Tue, Oct 20, 2020 at 3:37 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
> > > >
> > > > Commit dd461cd9183f ("opp: Allow dev_pm_opp_get_opp_table() to return
> > > > -EPROBE_DEFER") handles -EPROBE_DEFER for the clock/interconnects within
> > > > _allocate_opp_table() which is called from dev_pm_opp_add and it
> > > > now propagates the error back to the caller.
> > > >
> > > > SCMI performance domain re-used clock bindings to keep it simple. However
> > > > with the above mentioned change, if clock property is present in a device
> > > > node, opps can't be added until clk_get succeeds. So in order to fix the
> > > > issue, we can register dummy clocks which is completely ugly.
> > > >
> > > > Since there are no upstream users for the SCMI performance domain clock
> > > > bindings, let us introduce separate performance domain bindings for the
> > > > same.
> > > >
> > > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > > > ---
> > > >  .../devicetree/bindings/arm/arm,scmi.txt      | 19 ++++++++++++++++---
> > > >  1 file changed, 16 insertions(+), 3 deletions(-)
> > > >
> > > > Hi Rob/Viresh,
> > > >
> > > > This is actually a fix for the regression I reported here[1].
> > > > I am not adding fixes tag as I am targeting in the same release and
> > > > also because it is not directly related.
> > > >
> > > > Regards,
> > > > Sudeep
> > > >
> > > > [1] https://lore.kernel.org/r/20201015180555.gacdzkofpibkdn2e@bogus
> > > >
> > > > P.S.:/me records that this binding needs to be moved to yaml in v5.11
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/arm/arm,scmi.txt b/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > > > index 55deb68230eb..0a6c1b495403 100644
> > > > --- a/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > > > +++ b/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > > > @@ -44,7 +44,7 @@ as described in the following sections. If the platform supports dedicated
> > > >  mboxes, mbox-names and shmem shall be present in the sub-node corresponding
> > > >  to that protocol.
> > > >
> > > > -Clock/Performance bindings for the clocks/OPPs based on SCMI Message Protocol
> > > > +Clock bindings for the clocks based on SCMI Message Protocol
> > > >  ------------------------------------------------------------
> > > >
> > > >  This binding uses the common clock binding[1].
> > > > @@ -52,6 +52,19 @@ This binding uses the common clock binding[1].
> > > >  Required properties:
> > > >  - #clock-cells : Should be 1. Contains the Clock ID value used by SCMI commands.
> > > >
> > > > +Performance bindings for the OPPs based on SCMI Message Protocol
> > > > +------------------------------------------------------------
> > > > +
> > > > +Required properties:
> > > > +- #perf-domain-cells: Should be 1. Contains the performance domain ID value
> > > > +                     used by SCMI commands.
> > >
> > > When is this not 1 (IOW, you only need this if variable)? How would it
> > > be used outside SCMI (given it has a generic name)?
> > >
> >
> > Ah, I thought we need this if phandle is followed by 1 or more arguments.
> > If it is not compulsory I can drop this or make it scmi specific if we
> > need it.
> 
> No, your options are fixed or variable number of cells. If this is
> generic, then maybe it needs to be variable. If it's SCMI specific
> then it can likely be fixed unless you can think of other information
> you may need in the cells.
> 
> > > > +
> > > > +* Property arm,scmi-perf-domain
> > >
> > > Yet this doesn't have a generic name. You mentioned on IRC this is
> > > aligned with QCom, but why can't QCom use the same property here?
> > >
> >
> > This is SCMI firmware driven while they have hardware driven perf/freq
> > domains. So different drivers, need to distinguish between the two.
> 
> So what if they are different drivers. That's *always* the case. The
> clock provider(s) for 'clocks' is different for every SoC? I doesn't
> matter who is the provider, it's the same information being described.
> 


More agreed, another one fresh on the list today[1]

-- 
Regards,
Sudeep

[1] https://lore.kernel.org/lkml/1603441493-18554-3-git-send-email-hector.yuan@mediatek.com