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@xxxxxxx> 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@xxxxxxx> 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@xxxxxxx>
> > > ---
> > >  .../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