* [PATCH] arm64: dts: qcom: sc8280xp: Vote for CX in USB controllers
@ 2023-01-12 13:51 Bjorn Andersson
2023-01-12 14:21 ` Konrad Dybcio
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Bjorn Andersson @ 2023-01-12 13:51 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio
Cc: Krzysztof Kozlowski, linux-arm-msm, devicetree, linux-kernel,
Johan Hovold
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@quicinc.com>
---
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@a6f8800 {
"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@a8f8800 {
"ss_phy_irq";
power-domains = <&gcc USB30_SEC_GDSC>;
+ required-opps = <&rpmhpd_opp_nom>;
resets = <&gcc GCC_USB30_SEC_BCR>;
--
2.37.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] arm64: dts: qcom: sc8280xp: Vote for CX in USB controllers
2023-01-12 13:51 [PATCH] arm64: dts: qcom: sc8280xp: Vote for CX in USB controllers Bjorn Andersson
@ 2023-01-12 14:21 ` Konrad Dybcio
2023-01-12 17:00 ` Dmitry Baryshkov
2023-01-12 15:49 ` Johan Hovold
2023-01-12 16:12 ` Bjorn Andersson
2 siblings, 1 reply; 6+ messages in thread
From: Konrad Dybcio @ 2023-01-12 14:21 UTC (permalink / raw)
To: Bjorn Andersson, Bjorn Andersson
Cc: Krzysztof Kozlowski, linux-arm-msm, devicetree, linux-kernel,
Johan Hovold
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@quicinc.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..
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.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@a6f8800 {
> "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@a8f8800 {
> "ss_phy_irq";
>
> power-domains = <&gcc USB30_SEC_GDSC>;
> + required-opps = <&rpmhpd_opp_nom>;
>
> resets = <&gcc GCC_USB30_SEC_BCR>;
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] arm64: dts: qcom: sc8280xp: Vote for CX in USB controllers
2023-01-12 14:21 ` Konrad Dybcio
@ 2023-01-12 17:00 ` Dmitry Baryshkov
2023-01-17 16:10 ` Bjorn Andersson
0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Baryshkov @ 2023-01-12 17:00 UTC (permalink / raw)
To: Konrad Dybcio, Bjorn Andersson, Bjorn Andersson
Cc: Krzysztof Kozlowski, linux-arm-msm, devicetree, linux-kernel,
Johan Hovold
12 января 2023 г. 16:21:14 GMT+02:00, Konrad Dybcio <konrad.dybcio@linaro.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@quicinc.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?
>
>Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.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@a6f8800 {
>> "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@a8f8800 {
>> "ss_phy_irq";
>>
>> power-domains = <&gcc USB30_SEC_GDSC>;
>> + required-opps = <&rpmhpd_opp_nom>;
>>
>> resets = <&gcc GCC_USB30_SEC_BCR>;
>>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] arm64: dts: qcom: sc8280xp: Vote for CX in USB controllers
2023-01-12 17:00 ` Dmitry Baryshkov
@ 2023-01-17 16:10 ` Bjorn Andersson
0 siblings, 0 replies; 6+ messages in thread
From: Bjorn Andersson @ 2023-01-17 16:10 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Konrad Dybcio, Bjorn Andersson, Krzysztof Kozlowski,
linux-arm-msm, devicetree, linux-kernel, Johan Hovold
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@linaro.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@quicinc.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@linaro.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@a6f8800 {
> >> "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@a8f8800 {
> >> "ss_phy_irq";
> >>
> >> power-domains = <&gcc USB30_SEC_GDSC>;
> >> + required-opps = <&rpmhpd_opp_nom>;
> >>
> >> resets = <&gcc GCC_USB30_SEC_BCR>;
> >>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] arm64: dts: qcom: sc8280xp: Vote for CX in USB controllers
2023-01-12 13:51 [PATCH] arm64: dts: qcom: sc8280xp: Vote for CX in USB controllers Bjorn Andersson
2023-01-12 14:21 ` Konrad Dybcio
@ 2023-01-12 15:49 ` Johan Hovold
2023-01-12 16:12 ` Bjorn Andersson
2 siblings, 0 replies; 6+ messages in thread
From: Johan Hovold @ 2023-01-12 15:49 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Bjorn Andersson, Konrad Dybcio, Krzysztof Kozlowski,
linux-arm-msm, devicetree, linux-kernel, Johan Hovold
On Thu, Jan 12, 2023 at 05:51:17AM -0800, 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@quicinc.com>
> ---
> 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@a6f8800 {
> "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@a8f8800 {
> "ss_phy_irq";
>
> power-domains = <&gcc USB30_SEC_GDSC>;
> + required-opps = <&rpmhpd_opp_nom>;
>
> resets = <&gcc GCC_USB30_SEC_BCR>;
Looks good. Perhaps you can send a binding patch adding 'required-oops'
as well.
Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
Johan
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] arm64: dts: qcom: sc8280xp: Vote for CX in USB controllers
2023-01-12 13:51 [PATCH] arm64: dts: qcom: sc8280xp: Vote for CX in USB controllers Bjorn Andersson
2023-01-12 14:21 ` Konrad Dybcio
2023-01-12 15:49 ` Johan Hovold
@ 2023-01-12 16:12 ` Bjorn Andersson
2 siblings, 0 replies; 6+ messages in thread
From: Bjorn Andersson @ 2023-01-12 16:12 UTC (permalink / raw)
To: konrad.dybcio, quic_bjorande
Cc: linux-kernel, devicetree, krzysztof.kozlowski+dt, johan+linaro,
linux-arm-msm
On Thu, 12 Jan 2023 05:51:17 -0800, 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).
>
>
Applied, thanks!
[1/1] arm64: dts: qcom: sc8280xp: Vote for CX in USB controllers
commit: fe07640280cd29ac2997a617a1fb5487feef9387
Best regards,
--
Bjorn Andersson <andersson@kernel.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-01-17 16:11 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-12 13:51 [PATCH] arm64: dts: qcom: sc8280xp: Vote for CX in USB controllers Bjorn Andersson
2023-01-12 14:21 ` Konrad Dybcio
2023-01-12 17:00 ` Dmitry Baryshkov
2023-01-17 16:10 ` Bjorn Andersson
2023-01-12 15:49 ` Johan Hovold
2023-01-12 16:12 ` Bjorn Andersson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox