| Message ID | 20221230160103.250996-1-krzysztof.kozlowski@linaro.org (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
| Series | [1/7] arm64: dts: qcom: sc8280xp: remove GCC from CX power domain | expand |
On 30/12/2022 17:00, Krzysztof Kozlowski wrote: > Bindings do not allow power-domain property in GCC clock controller and > documentation does not indicate that GCC is part of VDD_CX. > > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > --- > > Maybe the bindings should be fixed? Maybe this was added as workaround? > Anyway looking at documentation I do not see such relation, except > downstream vdd_cx-supply (which is the same as in other SoCs and we do > not represent it in upstream). > --- This one patch is a duplicate - I sent it already as RFT. Sorry for the noise. Best regards, Krzysztof
On 30.12.2022 17:00, Krzysztof Kozlowski wrote: > Bindings do not allow power-domain property in GCC clock controller and > documentation does not indicate that GCC is part of VDD_CX. > > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > --- > > Maybe the bindings should be fixed? Maybe this was added as workaround? > Anyway looking at documentation I do not see such relation, except > downstream vdd_cx-supply (which is the same as in other SoCs and we do > not represent it in upstream). Some clocks scale with _CX, which is annotated on downstream with vdd-levels. We take care of that by using opp tables in consumer drivers. Usually if power-domains is added to a clock controller, it means that at least one of the clocks needs the power domain to be on (which.. should be true for CX if the ARM part runs anyway, no?), as for example VDD_MX/VDD_GFX may not be on at boot and trying to enable such clocks would result in a big kaboom.. TL;DR: if nothing exploded, it's fine to remove it Konrad > --- > arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > index 1d1420c8720c..d14663c9f34c 100644 > --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > @@ -799,7 +799,6 @@ gcc: clock-controller@100000 { > <&pcie4_phy>, > <0>, > <0>; > - power-domains = <&rpmhpd SC8280XP_CX>; > }; > > ipcc: mailbox@408000 {
On Fri, Dec 30, 2022 at 4:04 PM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > Bindings do not allow power-domain property in GCC clock controller and > documentation does not indicate that GCC is part of VDD_CX. > > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > --- > > Maybe the bindings should be fixed? Maybe this was added as workaround? > Anyway looking at documentation I do not see such relation, except > downstream vdd_cx-supply (which is the same as in other SoCs and we do > not represent it in upstream). > --- > arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > index 1d1420c8720c..d14663c9f34c 100644 > --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > @@ -799,7 +799,6 @@ gcc: clock-controller@100000 { > <&pcie4_phy>, > <0>, > <0>; > - power-domains = <&rpmhpd SC8280XP_CX>; > }; > > ipcc: mailbox@408000 { > -- > 2.34.1 > You'd be better off adding the documentation. The CX rail is required to power the clock controller. If you are pulling this out and finding nothing breaks, its probably because the bootloader left it on. Per my understanding of what dts is though, we ought to be representing the hardware dependency. gcc is a root clock for just about every peripheral so, you can be sure it is on when you boot. Seems to me as if the right thing to do is to retain the dts and update the documentation. --- bod
On 31/12/2022 00:41, Konrad Dybcio wrote: > > > On 30.12.2022 17:00, Krzysztof Kozlowski wrote: >> Bindings do not allow power-domain property in GCC clock controller and >> documentation does not indicate that GCC is part of VDD_CX. >> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >> >> --- >> >> Maybe the bindings should be fixed? Maybe this was added as workaround? >> Anyway looking at documentation I do not see such relation, except >> downstream vdd_cx-supply (which is the same as in other SoCs and we do >> not represent it in upstream). > Some clocks scale with _CX, which is annotated on downstream with vdd-levels. > We take care of that by using opp tables in consumer drivers. Usually if > power-domains is added to a clock controller, it means that at least one of > the clocks needs the power domain to be on (which.. should be true for CX > if the ARM part runs anyway, no?), as for example VDD_MX/VDD_GFX may not be > on at boot and trying to enable such clocks would result in a big kaboom.. > > TL;DR: if nothing exploded, it's fine to remove it According to Bjorn, we should keep the domain. Best regards, Krzysztof
diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi index 1d1420c8720c..d14663c9f34c 100644 --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi @@ -799,7 +799,6 @@ gcc: clock-controller@100000 { <&pcie4_phy>, <0>, <0>; - power-domains = <&rpmhpd SC8280XP_CX>; }; ipcc: mailbox@408000 {
Bindings do not allow power-domain property in GCC clock controller and documentation does not indicate that GCC is part of VDD_CX. Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> --- Maybe the bindings should be fixed? Maybe this was added as workaround? Anyway looking at documentation I do not see such relation, except downstream vdd_cx-supply (which is the same as in other SoCs and we do not represent it in upstream). --- arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 1 - 1 file changed, 1 deletion(-)