| Message ID | 20230408134820.76050-2-krzysztof.kozlowski@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/3] dt-bindings: clock: qcom,gcc-sm8250: add missing bi_tcxo_ao clock | expand |
On 8.04.2023 15:48, Krzysztof Kozlowski wrote: > The GCC clock controller needs CX power domain, at least according to > DTS: > > sc7180-trogdor-pompom-r3.dtb: clock-controller@100000: Unevaluated properties are not allowed ('power-domains' was unexpected) > > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > --- +CC Rajendra (author of 5d6fc6321db1 ("arm64: dts: qcom: sc7180: Add required-opps for USB")) Rajendra, shouldn't SC7180 GCC have PM ops to make sure a vote is only there when AP is active? Are all GDSCs powered by CX? If not, wouldn't this also need power-domain-names to facilitate e.g. potential MX-powered ones? Konrad > .../devicetree/bindings/clock/qcom,gcc-sc7180.yaml | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/Documentation/devicetree/bindings/clock/qcom,gcc-sc7180.yaml b/Documentation/devicetree/bindings/clock/qcom,gcc-sc7180.yaml > index 06dce0c6b7d0..8bf9b6f49550 100644 > --- a/Documentation/devicetree/bindings/clock/qcom,gcc-sc7180.yaml > +++ b/Documentation/devicetree/bindings/clock/qcom,gcc-sc7180.yaml > @@ -32,6 +32,10 @@ properties: > - const: bi_tcxo_ao > - const: sleep_clk > > + power-domains: > + items: > + - description: CX domain > + > required: > - compatible > - clocks > @@ -45,6 +49,8 @@ unevaluatedProperties: false > examples: > - | > #include <dt-bindings/clock/qcom,rpmh.h> > + #include <dt-bindings/power/qcom-rpmpd.h> > + > clock-controller@100000 { > compatible = "qcom,gcc-sc7180"; > reg = <0x00100000 0x1f0000>; > @@ -52,6 +58,7 @@ examples: > <&rpmhcc RPMH_CXO_CLK_A>, > <&sleep_clk>; > clock-names = "bi_tcxo", "bi_tcxo_ao", "sleep_clk"; > + power-domains = <&rpmhpd SC7180_CX>; > #clock-cells = <1>; > #reset-cells = <1>; > #power-domain-cells = <1>;
On 4/8/2023 7:33 PM, Konrad Dybcio wrote: > > > On 8.04.2023 15:48, Krzysztof Kozlowski wrote: >> The GCC clock controller needs CX power domain, at least according to >> DTS: >> >> sc7180-trogdor-pompom-r3.dtb: clock-controller@100000: Unevaluated properties are not allowed ('power-domains' was unexpected) >> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >> --- > +CC Rajendra (author of 5d6fc6321db1 ("arm64: dts: qcom: > sc7180: Add required-opps for USB")) > > Rajendra, shouldn't SC7180 GCC have PM ops to make sure a vote > is only there when AP is active? hmm, I am not quite sure why we would want the performance votes from peripherals dropped when CPUs go down in idle? > Are all GDSCs powered by CX? > If not, wouldn't this also need power-domain-names to > facilitate e.g. potential MX-powered ones? For sc7180 GCC, yes. > > Konrad >> .../devicetree/bindings/clock/qcom,gcc-sc7180.yaml | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/clock/qcom,gcc-sc7180.yaml b/Documentation/devicetree/bindings/clock/qcom,gcc-sc7180.yaml >> index 06dce0c6b7d0..8bf9b6f49550 100644 >> --- a/Documentation/devicetree/bindings/clock/qcom,gcc-sc7180.yaml >> +++ b/Documentation/devicetree/bindings/clock/qcom,gcc-sc7180.yaml >> @@ -32,6 +32,10 @@ properties: >> - const: bi_tcxo_ao >> - const: sleep_clk >> >> + power-domains: >> + items: >> + - description: CX domain >> + >> required: >> - compatible >> - clocks >> @@ -45,6 +49,8 @@ unevaluatedProperties: false >> examples: >> - | >> #include <dt-bindings/clock/qcom,rpmh.h> >> + #include <dt-bindings/power/qcom-rpmpd.h> >> + >> clock-controller@100000 { >> compatible = "qcom,gcc-sc7180"; >> reg = <0x00100000 0x1f0000>; >> @@ -52,6 +58,7 @@ examples: >> <&rpmhcc RPMH_CXO_CLK_A>, >> <&sleep_clk>; >> clock-names = "bi_tcxo", "bi_tcxo_ao", "sleep_clk"; >> + power-domains = <&rpmhpd SC7180_CX>; >> #clock-cells = <1>; >> #reset-cells = <1>; >> #power-domain-cells = <1>;
On 4/11/2023 4:57 PM, Konrad Dybcio wrote: > > > On 11.04.2023 06:56, Rajendra Nayak wrote: >> >> >> On 4/8/2023 7:33 PM, Konrad Dybcio wrote: >>> >>> >>> On 8.04.2023 15:48, Krzysztof Kozlowski wrote: >>>> The GCC clock controller needs CX power domain, at least according to >>>> DTS: >>>> >>>> sc7180-trogdor-pompom-r3.dtb: clock-controller@100000: Unevaluated properties are not allowed ('power-domains' was unexpected) >>>> >>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >>>> --- >>> +CC Rajendra (author of 5d6fc6321db1 ("arm64: dts: qcom: >>> sc7180: Add required-opps for USB")) >>> >>> Rajendra, shouldn't SC7180 GCC have PM ops to make sure a vote >>> is only there when AP is active? > So IIUC, CX is never supposed to be shut down? Atleast sc7180 and sc7280 do not support full CX shutdown (or power collapse as its called), it only transitions to a Retention state and even that in the system-wide suspend path only > > Konrad >> >> hmm, I am not quite sure why we would want the performance votes >> from peripherals dropped when CPUs go down in idle? >> >>> Are all GDSCs powered by CX? >>> If not, wouldn't this also need power-domain-names to >>> facilitate e.g. potential MX-powered ones? >> >> For sc7180 GCC, yes. >> >>> >>> Konrad >>>> .../devicetree/bindings/clock/qcom,gcc-sc7180.yaml | 7 +++++++ >>>> 1 file changed, 7 insertions(+) >>>> >>>> diff --git a/Documentation/devicetree/bindings/clock/qcom,gcc-sc7180.yaml b/Documentation/devicetree/bindings/clock/qcom,gcc-sc7180.yaml >>>> index 06dce0c6b7d0..8bf9b6f49550 100644 >>>> --- a/Documentation/devicetree/bindings/clock/qcom,gcc-sc7180.yaml >>>> +++ b/Documentation/devicetree/bindings/clock/qcom,gcc-sc7180.yaml >>>> @@ -32,6 +32,10 @@ properties: >>>> - const: bi_tcxo_ao >>>> - const: sleep_clk >>>> + power-domains: >>>> + items: >>>> + - description: CX domain >>>> + >>>> required: >>>> - compatible >>>> - clocks >>>> @@ -45,6 +49,8 @@ unevaluatedProperties: false >>>> examples: >>>> - | >>>> #include <dt-bindings/clock/qcom,rpmh.h> >>>> + #include <dt-bindings/power/qcom-rpmpd.h> >>>> + >>>> clock-controller@100000 { >>>> compatible = "qcom,gcc-sc7180"; >>>> reg = <0x00100000 0x1f0000>; >>>> @@ -52,6 +58,7 @@ examples: >>>> <&rpmhcc RPMH_CXO_CLK_A>, >>>> <&sleep_clk>; >>>> clock-names = "bi_tcxo", "bi_tcxo_ao", "sleep_clk"; >>>> + power-domains = <&rpmhpd SC7180_CX>; >>>> #clock-cells = <1>; >>>> #reset-cells = <1>; >>>> #power-domain-cells = <1>;
On 11.04.2023 15:31, Rajendra Nayak wrote: > > On 4/11/2023 4:57 PM, Konrad Dybcio wrote: >> >> >> On 11.04.2023 06:56, Rajendra Nayak wrote: >>> >>> >>> On 4/8/2023 7:33 PM, Konrad Dybcio wrote: >>>> >>>> >>>> On 8.04.2023 15:48, Krzysztof Kozlowski wrote: >>>>> The GCC clock controller needs CX power domain, at least according to >>>>> DTS: >>>>> >>>>> sc7180-trogdor-pompom-r3.dtb: clock-controller@100000: Unevaluated properties are not allowed ('power-domains' was unexpected) >>>>> >>>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >>>>> --- >>>> +CC Rajendra (author of 5d6fc6321db1 ("arm64: dts: qcom: >>>> sc7180: Add required-opps for USB")) >>>> >>>> Rajendra, shouldn't SC7180 GCC have PM ops to make sure a vote >>>> is only there when AP is active? >> So IIUC, CX is never supposed to be shut down? > > Atleast sc7180 and sc7280 do not support full CX shutdown (or power > collapse as its called), it only transitions to a Retention state > and even that in the system-wide suspend path only And won't outstanding votes on that resource prevent the system from entering a system-wide low power state? Konrad > >> >> Konrad >>> >>> hmm, I am not quite sure why we would want the performance votes >>> from peripherals dropped when CPUs go down in idle? >>> >>>> Are all GDSCs powered by CX? >>>> If not, wouldn't this also need power-domain-names to >>>> facilitate e.g. potential MX-powered ones? >>> >>> For sc7180 GCC, yes. >>> >>>> >>>> Konrad >>>>> .../devicetree/bindings/clock/qcom,gcc-sc7180.yaml | 7 +++++++ >>>>> 1 file changed, 7 insertions(+) >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/clock/qcom,gcc-sc7180.yaml b/Documentation/devicetree/bindings/clock/qcom,gcc-sc7180.yaml >>>>> index 06dce0c6b7d0..8bf9b6f49550 100644 >>>>> --- a/Documentation/devicetree/bindings/clock/qcom,gcc-sc7180.yaml >>>>> +++ b/Documentation/devicetree/bindings/clock/qcom,gcc-sc7180.yaml >>>>> @@ -32,6 +32,10 @@ properties: >>>>> - const: bi_tcxo_ao >>>>> - const: sleep_clk >>>>> + power-domains: >>>>> + items: >>>>> + - description: CX domain >>>>> + >>>>> required: >>>>> - compatible >>>>> - clocks >>>>> @@ -45,6 +49,8 @@ unevaluatedProperties: false >>>>> examples: >>>>> - | >>>>> #include <dt-bindings/clock/qcom,rpmh.h> >>>>> + #include <dt-bindings/power/qcom-rpmpd.h> >>>>> + >>>>> clock-controller@100000 { >>>>> compatible = "qcom,gcc-sc7180"; >>>>> reg = <0x00100000 0x1f0000>; >>>>> @@ -52,6 +58,7 @@ examples: >>>>> <&rpmhcc RPMH_CXO_CLK_A>, >>>>> <&sleep_clk>; >>>>> clock-names = "bi_tcxo", "bi_tcxo_ao", "sleep_clk"; >>>>> + power-domains = <&rpmhpd SC7180_CX>; >>>>> #clock-cells = <1>; >>>>> #reset-cells = <1>; >>>>> #power-domain-cells = <1>;
diff --git a/Documentation/devicetree/bindings/clock/qcom,gcc-sc7180.yaml b/Documentation/devicetree/bindings/clock/qcom,gcc-sc7180.yaml index 06dce0c6b7d0..8bf9b6f49550 100644 --- a/Documentation/devicetree/bindings/clock/qcom,gcc-sc7180.yaml +++ b/Documentation/devicetree/bindings/clock/qcom,gcc-sc7180.yaml @@ -32,6 +32,10 @@ properties: - const: bi_tcxo_ao - const: sleep_clk + power-domains: + items: + - description: CX domain + required: - compatible - clocks @@ -45,6 +49,8 @@ unevaluatedProperties: false examples: - | #include <dt-bindings/clock/qcom,rpmh.h> + #include <dt-bindings/power/qcom-rpmpd.h> + clock-controller@100000 { compatible = "qcom,gcc-sc7180"; reg = <0x00100000 0x1f0000>; @@ -52,6 +58,7 @@ examples: <&rpmhcc RPMH_CXO_CLK_A>, <&sleep_clk>; clock-names = "bi_tcxo", "bi_tcxo_ao", "sleep_clk"; + power-domains = <&rpmhpd SC7180_CX>; #clock-cells = <1>; #reset-cells = <1>; #power-domain-cells = <1>;
The GCC clock controller needs CX power domain, at least according to DTS: sc7180-trogdor-pompom-r3.dtb: clock-controller@100000: Unevaluated properties are not allowed ('power-domains' was unexpected) Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> --- .../devicetree/bindings/clock/qcom,gcc-sc7180.yaml | 7 +++++++ 1 file changed, 7 insertions(+)