linux-kernel - Re: [PATCH] arm64: dts: qcom: sc8280xp: Vote for CX in USB controllers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20230117161055.GF2350793@hu-bjorande-lv.qualcomm.com>
Date:   Tue, 17 Jan 2023 08:10:55 -0800
From:   Bjorn Andersson <quic_bjorande@...cinc.com>
To:     Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
CC:     Konrad Dybcio <konrad.dybcio@...aro.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        <linux-arm-msm@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>,
        Johan Hovold <johan+linaro@...nel.org>
Subject: Re: [PATCH] arm64: dts: qcom: sc8280xp: Vote for CX in USB
 controllers

On Thu, Jan 12, 2023 at 07:00:29PM +0200, Dmitry Baryshkov wrote:
> 12 января 2023 г. 16:21:14 GMT+02:00, Konrad Dybcio <konrad.dybcio@...aro.org> пишет:
> >
> >
> >On 12.01.2023 14:51, Bjorn Andersson wrote:
> >> Running GCC_USB30_*_MASTER_CLK at 200MHz requires CX at nominal level,
> >> not doing so results in occasional lockups. This was previously hidden
> >> by the fact that the display stack incorrectly voted for CX (instead of
> >> MMCX).
> >> 
> >> Fixes: 152d1faf1e2f ("arm64: dts: qcom: add SC8280XP platform")
> >> Signed-off-by: Bjorn Andersson <quic_bjorande@...cinc.com>
> >> ---https://git.codelinaro.org/clo/la/kernel/msm-5.4/-/blob/LV.AU.1.2.3.r1-03600-gen3meta.0/drivers/clk/qcom/gcc-direwolf.c#L2703-2725
> >
> >Maybe in the future there could be some power savings for lower freqs..
> 
> I had the same question. If the vote is not static, but depends on the
> freq, shouldn't this be to implemented as an opp + table?
> 

The upstream Linux driver does not dynamically adjust the rate of the
core clock, so whenever the device isn't suspended it will tick at
200MHz and require nominal voltage.

The downstream driver does adjust the core clock rate based on the link,
and can thereby adjust the voltage level as well. So once this is
supported upstream, replacing this with an opp-table would be
appropriate.

Regards,
Bjorn

> 
> >
> >Reviewed-by: Konrad Dybcio <konrad.dybcio@...aro.org>
> >
> >Konrad
> >>  arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 2 ++
> >>  1 file changed, 2 insertions(+)
> >> 
> >> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> >> index 2ed17baf50d3..4f4353f84cba 100644
> >> --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> >> +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> >> @@ -2265,6 +2265,7 @@ usb_0: usb@...8800 {
> >>  					  "ss_phy_irq";
> >>  
> >>  			power-domains = <&gcc USB30_PRIM_GDSC>;
> >> +			required-opps = <&rpmhpd_opp_nom>;
> >>  
> >>  			resets = <&gcc GCC_USB30_PRIM_BCR>;
> >>  
> >> @@ -2319,6 +2320,7 @@ usb_1: usb@...8800 {
> >>  					  "ss_phy_irq";
> >>  
> >>  			power-domains = <&gcc USB30_SEC_GDSC>;
> >> +			required-opps = <&rpmhpd_opp_nom>;
> >>  
> >>  			resets = <&gcc GCC_USB30_SEC_BCR>;
> >>  
>