Re: [PATCH 09/16] dt-bindings: firmware: arm,scmi: Extend bindings for protocol@13

[Sudeep Holla]

On Thu, Jun 15, 2023 at 11:39:06AM +0200, Ulf Hansson wrote:
> On Thu, 15 Jun 2023 at 10:44, Sudeep Holla <sudeep.holla@xxxxxxx> wrote:
> >
> > On Wed, Jun 07, 2023 at 02:46:21PM +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 domain" too. The common way to describe this, is to
> > > use the "power-domain" bindings, so let's use that.
> > >
> > > Cc: Rob Herring <robh+dt@xxxxxxxxxx>
> > > Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@xxxxxxxxxx>
> > > Cc: Conor Dooley <conor+dt@xxxxxxxxxx>
> > > Cc: devicetree@xxxxxxxxxxxxxxx
> > > Signed-off-by: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
> > > ---
> > >  Documentation/devicetree/bindings/firmware/arm,scmi.yaml | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > index 5824c43e9893..cff9d1e4cea1 100644
> > > --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > @@ -145,8 +145,8 @@ properties:
> > >        '#clock-cells':
> > >          const: 1
> > >
> > > -    required:
> > > -      - '#clock-cells'
> >
> > I am yet to look at the patches, just looked at this binding changes for now.
> >
> > Won't this break compatibility with existing DTBs. IMO, this is strict
> > no no, you can't drop #clock-cells. I wanted to add performance-domains
> > here as alternative but decided to not as I knew you were working on this.
> 
> Thanks for reviewing!
> 
> The point with the suggested change was to allow any kind of
> combination of using #clock-cells and/or #power-domain-cells. Honestly
> I didn't really know how to best express that in the binding, but
> maybe someone can help me out here?
>

Even I don't know exact details, but there are rules like if this
property is present, some other property can't be there or something
on the similar lines. I have vague idea/recollection from my previous
experiments which probably was not needed then and hence I can't just
point to any examples unless I go and search myself.

> I think enforcing #clock-cells to be used is unnecessary. Making it
> optional should not break existing DTBs, right?

Correct. That is what I meant, it is either #clock-cells or
#power-domain-cells

>
> Moreover, currently it seems to be only Juno that uses "protocol@13"
> and the "#clock-cells" (at least by looking at the DTSes in-kernel).

Yes only one that has upstream DTS changes, but for sure it is used on
couple of other platforms. So for we are still far away from deprecate it
but we can eventually once users of it are ready to use new binding.

> So, I wonder if it's really such a big deal to update the DT bindings
> for "protocol@13" at this point, but I may not have the complete
> picture.
>

Yes it does break compatibility. Yes I know Juno is not a production
platform, but associating DT with kernel change makes is hard to switch
to older stable kernel versions without DT change which I really hate as
I will be wondering which SCMI perf is not working with stable kernel few
months down the line. So yes, we are not dropping the support for old
bindings even if it just for Juno(though I am sure it is not the only one).
I have spent time on such silly things when we were in the process of
pushing these bindings initially upstream. I prefer not to repeat that.

--
Regards,
Sudeep