Message-ID: <CAPDyKFoWcyznGM7EYkuhSAB+6nNBvsPt77o5B3hyL06pwiFYMA@mail.gmail.com>
Date:   Fri, 16 Jun 2023 13:48:27 +0200
From:   Ulf Hansson <ulf.hansson@...aro.org>
To:     Sudeep Holla <sudeep.holla@....com>
Cc:     Cristian Marussi <cristian.marussi@....com>,
        Viresh Kumar <vireshk@...nel.org>, Nishanth Menon <nm@...com>,
        Stephen Boyd <sboyd@...nel.org>,
        Nikunj Kela <nkela@...cinc.com>,
        Prasad Sodagudi <psodagud@...cinc.com>,
        Alexandre Torgue <alexandre.torgue@...s.st.com>,
        linux-pm@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org, Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>, devicetree@...r.kernel.org
Subject: Re: [PATCH 09/16] dt-bindings: firmware: arm,scmi: Extend bindings
 for protocol@13

On Thu, 15 Jun 2023 at 15:30, Sudeep Holla <sudeep.holla@....com> wrote:
>
> 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@....com> 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@...nel.org>
> > > > Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>
> > > > Cc: Conor Dooley <conor+dt@...nel.org>
> > > > Cc: devicetree@...r.kernel.org
> > > > Signed-off-by: Ulf Hansson <ulf.hansson@...aro.org>
> > > > ---
> > > >  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 will figure it out, np!

>
> > 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

Should we allow both? Or maybe that is just confusing?

In either case, I am converting the scmi cpufreq driver to cope with
using #power-domain-cells too, as that is useful regardless I think.
However, that's a separate series on top of $subject series.

>
> >
> > 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.

Okay, let's discuss when to deprecate it, but let's do that later on.

>
> > 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.

Okay, thanks for sharing the information. Let's simply follow the
regular path of how we deal with deprecating DT bindings then.

Kind regards
Uffe

Powered by blists - more mailing lists