mbox series

[v1,0/3] clk: qcom: handle power domains supplies for GDSC

Message ID 20201005225914.315852-1-dmitry.baryshkov@linaro.org
Headers show
Series clk: qcom: handle power domains supplies for GDSC | expand

Message

Dmitry Baryshkov Oct. 5, 2020, 10:59 p.m. UTC 
On SM8250 MDSS_GDSC (and the rest of display clock controller) is
supplied power by MMCX power domain. Handle this link in GDSC code by
binding the power domain in dts file.

This patchset depends on [1]

Changes since RFC:
 - Fix naming of gdsc_supply_on/gdsc_supply_off functions
 - Fix detaching of solo gdsc's power domain in error handling code
 - Drop the dts patch, as respective display nodes are still not
   submitted to the mailing list.

[1] https://lore.kernel.org/linux-arm-msm/20200927190653.13876-1-jonathan@marek.ca/

Comments

Stephen Boyd Oct. 14, 2020, 12:46 a.m. UTC | #1 
Quoting Dmitry Baryshkov (2020-10-05 15:59:12)
> SM8250 requires special power domain for accessing MMDS_GDSC registers.

Heh, not sure it's special.

> Add bindings for the MMCX power domain.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
>  .../bindings/clock/qcom,sdm845-dispcc.yaml    | 28 +++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/clock/qcom,sdm845-dispcc.yaml b/Documentation/devicetree/bindings/clock/qcom,sdm845-dispcc.yaml
> index 4a3be733d042..ff0db55470ac 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,sdm845-dispcc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,sdm845-dispcc.yaml
> @@ -97,5 +108,22 @@ examples:
>        #clock-cells = <1>;
>        #reset-cells = <1>;
>        #power-domain-cells = <1>;
> +      /* this is a part of sm8250 setup the power domain example */
> +      power-domains = <&rpmhpd SDM845_CX>;
> +      power-domain-names = "mmcx";
> +      required-opps = <&rpmhpd_opp_low_svs>;
> +    };
> +    rpmhpd: power-controller {

Do we need this node in the example? I think it isn't required but I
guess it's OK.

> +      compatible = "qcom,sdm845-rpmhpd";
> +      #power-domain-cells = <1>;
> +      operating-points-v2 = <&rpmhpd_opp_table>;
> +
> +      rpmhpd_opp_table: opp-table {
> +        compatible = "operating-points-v2";
> +
> +        rpmhpd_opp_low_svs: opp3 {
Stephen Boyd Oct. 14, 2020, 2:16 a.m. UTC | #2 
Quoting Dmitry Baryshkov (2020-10-05 15:59:14)
> diff --git a/drivers/clk/qcom/dispcc-sm8250.c b/drivers/clk/qcom/dispcc-sm8250.c
> index 07a98d3f882d..3941054a7b07 100644
> --- a/drivers/clk/qcom/dispcc-sm8250.c
> +++ b/drivers/clk/qcom/dispcc-sm8250.c
> @@ -963,6 +963,8 @@ static struct gdsc mdss_gdsc = {
>         },
>         .pwrsts = PWRSTS_OFF_ON,
>         .flags = HW_CTRL,
> +       .domain = "mmcx",
> +       .perf_idx = 0,

Does a perf_idx of 0 mean off? Or just "not off"?
Stephen Boyd Oct. 14, 2020, 2:17 a.m. UTC | #3 
Quoting Dmitry Baryshkov (2020-10-05 15:59:12)
> @@ -97,5 +108,22 @@ examples:
>        #clock-cells = <1>;
>        #reset-cells = <1>;
>        #power-domain-cells = <1>;
> +      /* this is a part of sm8250 setup the power domain example */

What does this comment mean?

> +      power-domains = <&rpmhpd SDM845_CX>;
> +      power-domain-names = "mmcx";
> +      required-opps = <&rpmhpd_opp_low_svs>;
> +    };
Dmitry Baryshkov Oct. 14, 2020, 9:36 a.m. UTC | #4 
On 14/10/2020 03:46, Stephen Boyd wrote:
> Quoting Dmitry Baryshkov (2020-10-05 15:59:12)
>> SM8250 requires special power domain for accessing MMDS_GDSC registers.
> 
> Heh, not sure it's special.
> 
>> Add bindings for the MMCX power domain.
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> Reviewed-by: Rob Herring <robh@kernel.org>
>> ---
>>   .../bindings/clock/qcom,sdm845-dispcc.yaml    | 28 +++++++++++++++++++
>>   1 file changed, 28 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/clock/qcom,sdm845-dispcc.yaml b/Documentation/devicetree/bindings/clock/qcom,sdm845-dispcc.yaml
>> index 4a3be733d042..ff0db55470ac 100644
>> --- a/Documentation/devicetree/bindings/clock/qcom,sdm845-dispcc.yaml
>> +++ b/Documentation/devicetree/bindings/clock/qcom,sdm845-dispcc.yaml
>> @@ -97,5 +108,22 @@ examples:
>>         #clock-cells = <1>;
>>         #reset-cells = <1>;
>>         #power-domain-cells = <1>;
>> +      /* this is a part of sm8250 setup the power domain example */
>> +      power-domains = <&rpmhpd SDM845_CX>;
>> +      power-domain-names = "mmcx";
>> +      required-opps = <&rpmhpd_opp_low_svs>;
>> +    };
>> +    rpmhpd: power-controller {
> 
> Do we need this node in the example? I think it isn't required but I
> guess it's OK.

It is to be able to resolve "power-domains" and "required-opps" 
properties values.

>> +      compatible = "qcom,sdm845-rpmhpd";
>> +      #power-domain-cells = <1>;
>> +      operating-points-v2 = <&rpmhpd_opp_table>;
>> +
>> +      rpmhpd_opp_table: opp-table {
>> +        compatible = "operating-points-v2";
>> +
>> +        rpmhpd_opp_low_svs: opp3 {
Dmitry Baryshkov Oct. 14, 2020, 9:46 a.m. UTC | #5 
On 14/10/2020 05:16, Stephen Boyd wrote:
> Quoting Dmitry Baryshkov (2020-10-05 15:59:14)
>> diff --git a/drivers/clk/qcom/dispcc-sm8250.c b/drivers/clk/qcom/dispcc-sm8250.c
>> index 07a98d3f882d..3941054a7b07 100644
>> --- a/drivers/clk/qcom/dispcc-sm8250.c
>> +++ b/drivers/clk/qcom/dispcc-sm8250.c
>> @@ -963,6 +963,8 @@ static struct gdsc mdss_gdsc = {
>>          },
>>          .pwrsts = PWRSTS_OFF_ON,
>>          .flags = HW_CTRL,
>> +       .domain = "mmcx",
>> +       .perf_idx = 0,
> 
> Does a perf_idx of 0 mean off? Or just "not off"?


It means 'use performance state with index 0 declared in the DTS'.