Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 25 Oct 2021 14:43:23 -0700
From:   Bjorn Andersson <bjorn.andersson@linaro.org>
To:     Stephen Boyd <swboyd@chromium.org>,
        Viresh Kumar <viresh.kumar@linaro.org>,
        Ulf Hansson <ulf.hansson@linaro.org>
Cc:     Sandeep Maheswaram <quic_c_sanm@quicinc.com>,
        Rob Herring <robh+dt@kernel.org>,
        Andy Gross <agross@kernel.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Felipe Balbi <balbi@kernel.org>,
        Doug Anderson <dianders@chromium.org>,
        Matthias Kaehlcke <mka@chromium.org>,
        devicetree@vger.kernel.org, linux-arm-msm@vger.kernel.org,
        linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
        quic_pkondeti@quicinc.com, quic_ppratap@quicinc.com
Subject: Re: [PATCH v2 1/3] dt-bindings: usb: qcom,dwc3: Add multi-pd
 bindings for dwc3 qcom
Message-ID: <YXck+xCJQBRGqTCw@ripper>
References: <1635152851-23660-1-git-send-email-quic_c_sanm@quicinc.com>
 <1635152851-23660-2-git-send-email-quic_c_sanm@quicinc.com>
 <YXcBK7zqny0s4gd4@ripper>
 <CAE-0n51k8TycXjEkH7rHYo0j7cYbKJOnOn1keVhx2yyTcBNnvg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAE-0n51k8TycXjEkH7rHYo0j7cYbKJOnOn1keVhx2yyTcBNnvg@mail.gmail.com>
Precedence: bulk

On Mon 25 Oct 13:17 PDT 2021, Stephen Boyd wrote:

> Quoting Bjorn Andersson (2021-10-25 12:10:35)
> > On Mon 25 Oct 02:07 PDT 2021, Sandeep Maheswaram wrote:
> >
> > > Add multi pd bindings to set performance state for cx domain
> > > to maintain minimum corner voltage for USB clocks.
> > >
> > > Signed-off-by: Sandeep Maheswaram <quic_c_sanm@quicinc.com>
> > > ---
> > > v2:
> > > Make cx domain mandatory.
> > >
> > >  Documentation/devicetree/bindings/usb/qcom,dwc3.yaml | 8 +++++++-
> > >  1 file changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
> > > index 2bdaba0..fd595a8 100644
> > > --- a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
> > > +++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
> > > @@ -42,7 +42,13 @@ properties:
> > >
> > >    power-domains:
> > >      description: specifies a phandle to PM domain provider node
> > > -    maxItems: 1
> > > +    minItems: 2
> > > +    items:
> > > +      - description: cx power domain
> > > +      - description: USB gdsc power domain
> > > +
> > > +  required-opps:
> > > +    description: specifies the performance state to power domain
> >
> > I'm still worried about the fact that we can't just rely on the USB GDSC
> > being a subdomin of CX in order to just "turn on" CX.
> >
> > Afaict accepting this path forward means that for any device that sits
> > in a GDSC power domain we will have to replicate this series for the
> > related driver.
> >
> 
> I suspect the problem is that it's not just "turn on" but wanting to
> turn it on and then set the performance state to some value based on the
> clk frequency.

I don't see an opp-table involved, just the required-opps for the
purpose of turning CX on a little bit more. Perhaps I'm missing
something here though.

> Maybe the simplest version of that could be supported
> somehow by having dev_pm_opp_set_rate() figure out that the 'level'
> applies to the parent power domain instead of the child one?

Having the performance_state request cascade up through the GDSC sounds
like a nice solution; I've not looked at the code to see if this is
feasible though.

> Or we may need to make another part of the OPP binding to indicate the
> relationship between the power domain and the OPP and the parent of
> the power domain.

I suspect this would be useful if a power-domain provider needs to
translate a performance_state into a different supply-performance_state.
Not sure if we have such case currently; these examples are all an
adjustable power-domain with "gating" subdomains.


PS. I think we have the same problem in the display subsystem, the
sub-blocks are powered by MDSS_GDSC, which is a subdomain of MMCX. We
trust the parent mdss node to keep the GDSC powered and specify MMCX as
the power-domain for the children, so that we can affect their levels by
respective opp-table.

Regards,
Bjorn