Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sun, 14 May 2023 19:38:28 -0700
From:   Bjorn Andersson <andersson@kernel.org>
To:     Konrad Dybcio <konrad.dybcio@linaro.org>
Cc:     Bjorn Andersson <quic_bjorande@quicinc.com>,
        Andy Gross <agross@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley <conor+dt@kernel.org>,
        linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] arm64: dts: qcom: sc8280xp: Flush RSC sleep & wake votes
Message-ID: <20230515023828.jqrrqkit5ygovimp@ripper>
References: <20230512150425.3171122-1-quic_bjorande@quicinc.com>
 <f6ecd66b-e207-0ed9-0ff3-1febfdf5bce9@linaro.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <f6ecd66b-e207-0ed9-0ff3-1febfdf5bce9@linaro.org>
Precedence: bulk

On Sat, May 13, 2023 at 11:09:07AM +0200, Konrad Dybcio wrote:
> 
> 
> On 12.05.2023 17:04, Bjorn Andersson wrote:
> > The rpmh driver will cache sleep and wake votes until the cluster
> > power-domain is about to enter idle, to avoid unnecessary writes. So
> > associate the apps_rsc with the cluster pd, so that it can be notified
> > about this event.
> > 
> > Without this, only AMC votes are being commited.
> Ouch.
> 
> Should we make this required: in bindings and add it to all
> platforms?
> 

I though this was an optimization and in the absence of this callback
the driver would just write out wake and sleep sets as well. But per the
current implementation (and perhaps some underlying cause?) it is indeed
required, if you care about power consumption.

> > 
> > Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> > ---
> Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> Fixes: 152d1faf1e2f ("arm64: dts: qcom: add SC8280XP platform")
> 

The Fixes sounds reasonable.

Thanks,
Bjorn

> Konrad
> >  arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> > index 8fa9fbfe5d00..5c68f2182c2f 100644
> > --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> > @@ -3982,6 +3982,7 @@ apps_rsc: rsc@18200000 {
> >  			qcom,tcs-config = <ACTIVE_TCS  2>, <SLEEP_TCS   3>,
> >  					  <WAKE_TCS    3>, <CONTROL_TCS 1>;
> >  			label = "apps_rsc";
> > +			power-domains = <&CLUSTER_PD>;
> >  
> >  			apps_bcm_voter: bcm-voter {
> >  				compatible = "qcom,bcm-voter";