* Re: [PATCH v3 7/7] arm64: dts: qcom: Add mx power domain as thermal warming device.
[not found] ` <1571254641-13626-8-git-send-email-thara.gopinath@linaro.org>
@ 2019-10-29 1:31 ` Rob Herring
2019-10-30 14:23 ` Thara Gopinath
0 siblings, 1 reply; 10+ messages in thread
From: Rob Herring @ 2019-10-29 1:31 UTC (permalink / raw)
To: Thara Gopinath
Cc: edubezval, rui.zhang, ulf.hansson, daniel.lezcano,
bjorn.andersson, agross, amit.kucheria, mark.rutland, rjw,
linux-pm, devicetree, linux-arm-msm, linux-kernel
On Wed, Oct 16, 2019 at 03:37:21PM -0400, Thara Gopinath wrote:
> RPMh hosts mx power domain that can be used to warm up the SoC.
> Add sub-node to rpmhpd node for mx to be recognized
> as thermal warming device on sdm845.
>
> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
> ---
> arch/arm64/boot/dts/qcom/sdm845.dtsi | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index 0222f48..0671c8a 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -3788,6 +3788,11 @@
> opp-level = <RPMH_REGULATOR_LEVEL_TURBO_L1>;
> };
> };
> +
> + mx_cdev: mx {
> + #cooling-cells = <2>;
> + .name = "mx";
Copy this from C code?
> + };
> };
>
> rsc_hlos: interconnect {
> --
> 2.1.4
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 6/7] dt-bindings: soc: qcom: Extend RPMh power controller binding to describe thermal warming device
[not found] ` <5DA89267.30806@linaro.org>
@ 2019-10-29 1:36 ` Rob Herring
2019-10-29 10:06 ` Ulf Hansson
0 siblings, 1 reply; 10+ messages in thread
From: Rob Herring @ 2019-10-29 1:36 UTC (permalink / raw)
To: Thara Gopinath
Cc: Ulf Hansson, Eduardo Valentin, Zhang Rui, Daniel Lezcano,
Bjorn Andersson, Andy Gross, amit.kucheria, Mark Rutland,
Rafael J. Wysocki, Linux PM, DTML, linux-arm-msm,
Linux Kernel Mailing List
On Thu, Oct 17, 2019 at 12:10:15PM -0400, Thara Gopinath wrote:
> On 10/17/2019 11:43 AM, Ulf Hansson wrote:
> > On Thu, 17 Oct 2019 at 17:28, Thara Gopinath <thara.gopinath@linaro.org> wrote:
> >>
> >> Hello Ulf,
> >> Thanks for the review!
> >>
> >> On 10/17/2019 05:04 AM, Ulf Hansson wrote:
> >>> On Wed, 16 Oct 2019 at 21:37, Thara Gopinath <thara.gopinath@linaro.org> wrote:
> >>>>
> >>>> RPMh power controller hosts mx domain that can be used as thermal
> >>>> warming device. Add a sub-node to specify this.
> >>>>
> >>>> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
> >>>> ---
> >>>> Documentation/devicetree/bindings/power/qcom,rpmpd.txt | 10 ++++++++++
> >>>> 1 file changed, 10 insertions(+)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/power/qcom,rpmpd.txt b/Documentation/devicetree/bindings/power/qcom,rpmpd.txt
> >>>> index eb35b22..fff695d 100644
> >>>> --- a/Documentation/devicetree/bindings/power/qcom,rpmpd.txt
> >>>> +++ b/Documentation/devicetree/bindings/power/qcom,rpmpd.txt
> >>>> @@ -18,6 +18,16 @@ Required Properties:
> >>>> Refer to <dt-bindings/power/qcom-rpmpd.h> for the level values for
> >>>> various OPPs for different platforms as well as Power domain indexes
> >>>>
> >>>> += SUBNODES
> >>>> +RPMh alsp hosts power domains that can behave as thermal warming device.
> >>>> +These are expressed as subnodes of the RPMh. The name of the node is used
> >>>> +to identify the power domain and must therefor be "mx".
> >>>> +
> >>>> +- #cooling-cells:
> >>>> + Usage: optional
> >>>> + Value type: <u32>
> >>>> + Definition: must be 2
> >>>> +
> >>>
> >>> Just wanted to express a minor thought about this. In general we use
> >>> subnodes of PM domain providers to represent the topology of PM
> >>> domains (subdomains), this is something different, which I guess is
> >>> fine.
> >>>
> >>> I assume the #cooling-cells is here tells us this is not a PM domain
> >>> provider, but a "cooling device provider"?
> >> Yep.
> >>>
> >>> Also, I wonder if it would be fine to specify "power-domains" here,
> >>> rather than using "name" as I think that is kind of awkward!?
> >> Do you mean "power-domain-names" ? I am using this to match against the
> >> genpd names defined in the provider driver.
> >
> > No. If you are using "power-domains" it means that you allow to
> > describe the specifier for the provider.
> Yep. But won't this look funny in DT ? The provider node will have a sub
> node with a power domain referencing to itself Like below: Is this ok ?
>
> rpmhpd: power-controller {
> compatible = "qcom,sdm845-rpmhpd";
> #power-domain-cells = <1>;
>
> ...
> ...
> mx_cdev: mx {
> #cooling-cells = <2>;
> power-domains = <&rpmhpd SDM845_MX>;
> };
>
The whole concept here seems all wrong to me. Isn't it what's in the
power domain that's the cooling device. A CPU power domain is not a
cooling device, the CPU is. Or we wouldn't make a clock a cooling
device, but what the clock drives.
Rob
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 6/7] dt-bindings: soc: qcom: Extend RPMh power controller binding to describe thermal warming device
2019-10-29 1:36 ` [PATCH v3 6/7] dt-bindings: soc: qcom: Extend RPMh power controller binding to describe " Rob Herring
@ 2019-10-29 10:06 ` Ulf Hansson
2019-10-29 20:16 ` Rob Herring
0 siblings, 1 reply; 10+ messages in thread
From: Ulf Hansson @ 2019-10-29 10:06 UTC (permalink / raw)
To: Rob Herring
Cc: Thara Gopinath, Eduardo Valentin, Zhang Rui, Daniel Lezcano,
Bjorn Andersson, Andy Gross, amit.kucheria, Mark Rutland,
Rafael J. Wysocki, Linux PM, DTML, linux-arm-msm,
Linux Kernel Mailing List
On Tue, 29 Oct 2019 at 02:36, Rob Herring <robh@kernel.org> wrote:
>
> On Thu, Oct 17, 2019 at 12:10:15PM -0400, Thara Gopinath wrote:
> > On 10/17/2019 11:43 AM, Ulf Hansson wrote:
> > > On Thu, 17 Oct 2019 at 17:28, Thara Gopinath <thara.gopinath@linaro.org> wrote:
> > >>
> > >> Hello Ulf,
> > >> Thanks for the review!
> > >>
> > >> On 10/17/2019 05:04 AM, Ulf Hansson wrote:
> > >>> On Wed, 16 Oct 2019 at 21:37, Thara Gopinath <thara.gopinath@linaro.org> wrote:
> > >>>>
> > >>>> RPMh power controller hosts mx domain that can be used as thermal
> > >>>> warming device. Add a sub-node to specify this.
> > >>>>
> > >>>> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
> > >>>> ---
> > >>>> Documentation/devicetree/bindings/power/qcom,rpmpd.txt | 10 ++++++++++
> > >>>> 1 file changed, 10 insertions(+)
> > >>>>
> > >>>> diff --git a/Documentation/devicetree/bindings/power/qcom,rpmpd.txt b/Documentation/devicetree/bindings/power/qcom,rpmpd.txt
> > >>>> index eb35b22..fff695d 100644
> > >>>> --- a/Documentation/devicetree/bindings/power/qcom,rpmpd.txt
> > >>>> +++ b/Documentation/devicetree/bindings/power/qcom,rpmpd.txt
> > >>>> @@ -18,6 +18,16 @@ Required Properties:
> > >>>> Refer to <dt-bindings/power/qcom-rpmpd.h> for the level values for
> > >>>> various OPPs for different platforms as well as Power domain indexes
> > >>>>
> > >>>> += SUBNODES
> > >>>> +RPMh alsp hosts power domains that can behave as thermal warming device.
> > >>>> +These are expressed as subnodes of the RPMh. The name of the node is used
> > >>>> +to identify the power domain and must therefor be "mx".
> > >>>> +
> > >>>> +- #cooling-cells:
> > >>>> + Usage: optional
> > >>>> + Value type: <u32>
> > >>>> + Definition: must be 2
> > >>>> +
> > >>>
> > >>> Just wanted to express a minor thought about this. In general we use
> > >>> subnodes of PM domain providers to represent the topology of PM
> > >>> domains (subdomains), this is something different, which I guess is
> > >>> fine.
> > >>>
> > >>> I assume the #cooling-cells is here tells us this is not a PM domain
> > >>> provider, but a "cooling device provider"?
> > >> Yep.
> > >>>
> > >>> Also, I wonder if it would be fine to specify "power-domains" here,
> > >>> rather than using "name" as I think that is kind of awkward!?
> > >> Do you mean "power-domain-names" ? I am using this to match against the
> > >> genpd names defined in the provider driver.
> > >
> > > No. If you are using "power-domains" it means that you allow to
> > > describe the specifier for the provider.
> > Yep. But won't this look funny in DT ? The provider node will have a sub
> > node with a power domain referencing to itself Like below: Is this ok ?
> >
> > rpmhpd: power-controller {
> > compatible = "qcom,sdm845-rpmhpd";
> > #power-domain-cells = <1>;
> >
> > ...
> > ...
> > mx_cdev: mx {
> > #cooling-cells = <2>;
> > power-domains = <&rpmhpd SDM845_MX>;
> > };
> >
>
> The whole concept here seems all wrong to me. Isn't it what's in the
> power domain that's the cooling device. A CPU power domain is not a
> cooling device, the CPU is. Or we wouldn't make a clock a cooling
> device, but what the clock drives.
Well, I don't think that's entirely correct description either.
As I see it, it's really the actual PM domain (that manages voltages
for a power island), that needs to stay in full power state and
increase its voltage level, as to warm up some of the silicon. It's
not a regular device, but more a characteristics of how the PM domain
can be used.
Kind regards
Uffe
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 6/7] dt-bindings: soc: qcom: Extend RPMh power controller binding to describe thermal warming device
2019-10-29 10:06 ` Ulf Hansson
@ 2019-10-29 20:16 ` Rob Herring
2019-10-30 9:27 ` Ulf Hansson
2019-10-30 14:27 ` Thara Gopinath
0 siblings, 2 replies; 10+ messages in thread
From: Rob Herring @ 2019-10-29 20:16 UTC (permalink / raw)
To: Ulf Hansson
Cc: Thara Gopinath, Eduardo Valentin, Zhang Rui, Daniel Lezcano,
Bjorn Andersson, Andy Gross, Amit Kucheria, Mark Rutland,
Rafael J. Wysocki, Linux PM, DTML, linux-arm-msm,
Linux Kernel Mailing List
On Tue, Oct 29, 2019 at 5:07 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Tue, 29 Oct 2019 at 02:36, Rob Herring <robh@kernel.org> wrote:
> >
> > On Thu, Oct 17, 2019 at 12:10:15PM -0400, Thara Gopinath wrote:
> > > On 10/17/2019 11:43 AM, Ulf Hansson wrote:
> > > > On Thu, 17 Oct 2019 at 17:28, Thara Gopinath <thara.gopinath@linaro.org> wrote:
> > > >>
> > > >> Hello Ulf,
> > > >> Thanks for the review!
> > > >>
> > > >> On 10/17/2019 05:04 AM, Ulf Hansson wrote:
> > > >>> On Wed, 16 Oct 2019 at 21:37, Thara Gopinath <thara.gopinath@linaro.org> wrote:
> > > >>>>
> > > >>>> RPMh power controller hosts mx domain that can be used as thermal
> > > >>>> warming device. Add a sub-node to specify this.
> > > >>>>
> > > >>>> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
> > > >>>> ---
> > > >>>> Documentation/devicetree/bindings/power/qcom,rpmpd.txt | 10 ++++++++++
> > > >>>> 1 file changed, 10 insertions(+)
> > > >>>>
> > > >>>> diff --git a/Documentation/devicetree/bindings/power/qcom,rpmpd.txt b/Documentation/devicetree/bindings/power/qcom,rpmpd.txt
> > > >>>> index eb35b22..fff695d 100644
> > > >>>> --- a/Documentation/devicetree/bindings/power/qcom,rpmpd.txt
> > > >>>> +++ b/Documentation/devicetree/bindings/power/qcom,rpmpd.txt
> > > >>>> @@ -18,6 +18,16 @@ Required Properties:
> > > >>>> Refer to <dt-bindings/power/qcom-rpmpd.h> for the level values for
> > > >>>> various OPPs for different platforms as well as Power domain indexes
> > > >>>>
> > > >>>> += SUBNODES
> > > >>>> +RPMh alsp hosts power domains that can behave as thermal warming device.
> > > >>>> +These are expressed as subnodes of the RPMh. The name of the node is used
> > > >>>> +to identify the power domain and must therefor be "mx".
> > > >>>> +
> > > >>>> +- #cooling-cells:
> > > >>>> + Usage: optional
> > > >>>> + Value type: <u32>
> > > >>>> + Definition: must be 2
> > > >>>> +
> > > >>>
> > > >>> Just wanted to express a minor thought about this. In general we use
> > > >>> subnodes of PM domain providers to represent the topology of PM
> > > >>> domains (subdomains), this is something different, which I guess is
> > > >>> fine.
> > > >>>
> > > >>> I assume the #cooling-cells is here tells us this is not a PM domain
> > > >>> provider, but a "cooling device provider"?
> > > >> Yep.
> > > >>>
> > > >>> Also, I wonder if it would be fine to specify "power-domains" here,
> > > >>> rather than using "name" as I think that is kind of awkward!?
> > > >> Do you mean "power-domain-names" ? I am using this to match against the
> > > >> genpd names defined in the provider driver.
> > > >
> > > > No. If you are using "power-domains" it means that you allow to
> > > > describe the specifier for the provider.
> > > Yep. But won't this look funny in DT ? The provider node will have a sub
> > > node with a power domain referencing to itself Like below: Is this ok ?
> > >
> > > rpmhpd: power-controller {
> > > compatible = "qcom,sdm845-rpmhpd";
> > > #power-domain-cells = <1>;
> > >
> > > ...
> > > ...
> > > mx_cdev: mx {
> > > #cooling-cells = <2>;
> > > power-domains = <&rpmhpd SDM845_MX>;
> > > };
> > >
> >
> > The whole concept here seems all wrong to me. Isn't it what's in the
> > power domain that's the cooling device. A CPU power domain is not a
> > cooling device, the CPU is. Or we wouldn't make a clock a cooling
> > device, but what the clock drives.
>
> Well, I don't think that's entirely correct description either.
>
> As I see it, it's really the actual PM domain (that manages voltages
> for a power island), that needs to stay in full power state and
> increase its voltage level, as to warm up some of the silicon. It's
> not a regular device, but more a characteristics of how the PM domain
> can be used.
First I've heard of Si needing warming...
I think I'd just expect the power domain provider to know which
domains to power on then.
Rob
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 6/7] dt-bindings: soc: qcom: Extend RPMh power controller binding to describe thermal warming device
2019-10-29 20:16 ` Rob Herring
@ 2019-10-30 9:27 ` Ulf Hansson
2019-10-30 14:27 ` Thara Gopinath
1 sibling, 0 replies; 10+ messages in thread
From: Ulf Hansson @ 2019-10-30 9:27 UTC (permalink / raw)
To: Rob Herring
Cc: Thara Gopinath, Eduardo Valentin, Zhang Rui, Daniel Lezcano,
Bjorn Andersson, Andy Gross, Amit Kucheria, Mark Rutland,
Rafael J. Wysocki, Linux PM, DTML, linux-arm-msm,
Linux Kernel Mailing List
On Tue, 29 Oct 2019 at 21:16, Rob Herring <robh@kernel.org> wrote:
>
> On Tue, Oct 29, 2019 at 5:07 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> >
> > On Tue, 29 Oct 2019 at 02:36, Rob Herring <robh@kernel.org> wrote:
> > >
> > > On Thu, Oct 17, 2019 at 12:10:15PM -0400, Thara Gopinath wrote:
> > > > On 10/17/2019 11:43 AM, Ulf Hansson wrote:
> > > > > On Thu, 17 Oct 2019 at 17:28, Thara Gopinath <thara.gopinath@linaro.org> wrote:
> > > > >>
> > > > >> Hello Ulf,
> > > > >> Thanks for the review!
> > > > >>
> > > > >> On 10/17/2019 05:04 AM, Ulf Hansson wrote:
> > > > >>> On Wed, 16 Oct 2019 at 21:37, Thara Gopinath <thara.gopinath@linaro.org> wrote:
> > > > >>>>
> > > > >>>> RPMh power controller hosts mx domain that can be used as thermal
> > > > >>>> warming device. Add a sub-node to specify this.
> > > > >>>>
> > > > >>>> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
> > > > >>>> ---
> > > > >>>> Documentation/devicetree/bindings/power/qcom,rpmpd.txt | 10 ++++++++++
> > > > >>>> 1 file changed, 10 insertions(+)
> > > > >>>>
> > > > >>>> diff --git a/Documentation/devicetree/bindings/power/qcom,rpmpd.txt b/Documentation/devicetree/bindings/power/qcom,rpmpd.txt
> > > > >>>> index eb35b22..fff695d 100644
> > > > >>>> --- a/Documentation/devicetree/bindings/power/qcom,rpmpd.txt
> > > > >>>> +++ b/Documentation/devicetree/bindings/power/qcom,rpmpd.txt
> > > > >>>> @@ -18,6 +18,16 @@ Required Properties:
> > > > >>>> Refer to <dt-bindings/power/qcom-rpmpd.h> for the level values for
> > > > >>>> various OPPs for different platforms as well as Power domain indexes
> > > > >>>>
> > > > >>>> += SUBNODES
> > > > >>>> +RPMh alsp hosts power domains that can behave as thermal warming device.
> > > > >>>> +These are expressed as subnodes of the RPMh. The name of the node is used
> > > > >>>> +to identify the power domain and must therefor be "mx".
> > > > >>>> +
> > > > >>>> +- #cooling-cells:
> > > > >>>> + Usage: optional
> > > > >>>> + Value type: <u32>
> > > > >>>> + Definition: must be 2
> > > > >>>> +
> > > > >>>
> > > > >>> Just wanted to express a minor thought about this. In general we use
> > > > >>> subnodes of PM domain providers to represent the topology of PM
> > > > >>> domains (subdomains), this is something different, which I guess is
> > > > >>> fine.
> > > > >>>
> > > > >>> I assume the #cooling-cells is here tells us this is not a PM domain
> > > > >>> provider, but a "cooling device provider"?
> > > > >> Yep.
> > > > >>>
> > > > >>> Also, I wonder if it would be fine to specify "power-domains" here,
> > > > >>> rather than using "name" as I think that is kind of awkward!?
> > > > >> Do you mean "power-domain-names" ? I am using this to match against the
> > > > >> genpd names defined in the provider driver.
> > > > >
> > > > > No. If you are using "power-domains" it means that you allow to
> > > > > describe the specifier for the provider.
> > > > Yep. But won't this look funny in DT ? The provider node will have a sub
> > > > node with a power domain referencing to itself Like below: Is this ok ?
> > > >
> > > > rpmhpd: power-controller {
> > > > compatible = "qcom,sdm845-rpmhpd";
> > > > #power-domain-cells = <1>;
> > > >
> > > > ...
> > > > ...
> > > > mx_cdev: mx {
> > > > #cooling-cells = <2>;
> > > > power-domains = <&rpmhpd SDM845_MX>;
> > > > };
> > > >
> > >
> > > The whole concept here seems all wrong to me. Isn't it what's in the
> > > power domain that's the cooling device. A CPU power domain is not a
> > > cooling device, the CPU is. Or we wouldn't make a clock a cooling
> > > device, but what the clock drives.
> >
> > Well, I don't think that's entirely correct description either.
> >
> > As I see it, it's really the actual PM domain (that manages voltages
> > for a power island), that needs to stay in full power state and
> > increase its voltage level, as to warm up some of the silicon. It's
> > not a regular device, but more a characteristics of how the PM domain
> > can be used.
>
> First I've heard of Si needing warming...
I guess people go to cooler places with their devices. :-)
>
> I think I'd just expect the power domain provider to know which
> domains to power on then.
Yeah, I agree. This seems reasonable.
Thanks!
Kind regards
Uffe
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 7/7] arm64: dts: qcom: Add mx power domain as thermal warming device.
2019-10-29 1:31 ` [PATCH v3 7/7] arm64: dts: qcom: Add mx power domain as thermal warming device Rob Herring
@ 2019-10-30 14:23 ` Thara Gopinath
0 siblings, 0 replies; 10+ messages in thread
From: Thara Gopinath @ 2019-10-30 14:23 UTC (permalink / raw)
To: Rob Herring
Cc: edubezval, rui.zhang, ulf.hansson, daniel.lezcano,
bjorn.andersson, agross, amit.kucheria, mark.rutland, rjw,
linux-pm, devicetree, linux-arm-msm, linux-kernel
On 10/28/2019 09:31 PM, Rob Herring wrote:
> On Wed, Oct 16, 2019 at 03:37:21PM -0400, Thara Gopinath wrote:
>> RPMh hosts mx power domain that can be used to warm up the SoC.
>> Add sub-node to rpmhpd node for mx to be recognized
>> as thermal warming device on sdm845.
>>
>> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
>> ---
>> arch/arm64/boot/dts/qcom/sdm845.dtsi | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
>> index 0222f48..0671c8a 100644
>> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
>> @@ -3788,6 +3788,11 @@
>> opp-level = <RPMH_REGULATOR_LEVEL_TURBO_L1>;
>> };
>> };
>> +
>> + mx_cdev: mx {
>> + #cooling-cells = <2>;
>> + .name = "mx";
>
> Copy this from C code?
Hi Rob,
What do you mean ?
>
>> + };
>> };
>>
>> rsc_hlos: interconnect {
>> --
>> 2.1.4
>>
--
Warm Regards
Thara
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 6/7] dt-bindings: soc: qcom: Extend RPMh power controller binding to describe thermal warming device
2019-10-29 20:16 ` Rob Herring
2019-10-30 9:27 ` Ulf Hansson
@ 2019-10-30 14:27 ` Thara Gopinath
1 sibling, 0 replies; 10+ messages in thread
From: Thara Gopinath @ 2019-10-30 14:27 UTC (permalink / raw)
To: Rob Herring, Ulf Hansson
Cc: Eduardo Valentin, Zhang Rui, Daniel Lezcano, Bjorn Andersson,
Andy Gross, Amit Kucheria, Mark Rutland, Rafael J. Wysocki,
Linux PM, DTML, linux-arm-msm, Linux Kernel Mailing List
Hi Rob,
Thanks for the review.
On 10/29/2019 04:16 PM, Rob Herring wrote:
> On Tue, Oct 29, 2019 at 5:07 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>
>> On Tue, 29 Oct 2019 at 02:36, Rob Herring <robh@kernel.org> wrote:
>>>
>>> On Thu, Oct 17, 2019 at 12:10:15PM -0400, Thara Gopinath wrote:
>>>> On 10/17/2019 11:43 AM, Ulf Hansson wrote:
>>>>> On Thu, 17 Oct 2019 at 17:28, Thara Gopinath <thara.gopinath@linaro.org> wrote:
>>>>>>
>>>>>> Hello Ulf,
>>>>>> Thanks for the review!
>>>>>>
>>>>>> On 10/17/2019 05:04 AM, Ulf Hansson wrote:
>>>>>>> On Wed, 16 Oct 2019 at 21:37, Thara Gopinath <thara.gopinath@linaro.org> wrote:
>>>>>>>>
>>>>>>>> RPMh power controller hosts mx domain that can be used as thermal
>>>>>>>> warming device. Add a sub-node to specify this.
>>>>>>>>
>>>>>>>> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
>>>>>>>> ---
>>>>>>>> Documentation/devicetree/bindings/power/qcom,rpmpd.txt | 10 ++++++++++
>>>>>>>> 1 file changed, 10 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/Documentation/devicetree/bindings/power/qcom,rpmpd.txt b/Documentation/devicetree/bindings/power/qcom,rpmpd.txt
>>>>>>>> index eb35b22..fff695d 100644
>>>>>>>> --- a/Documentation/devicetree/bindings/power/qcom,rpmpd.txt
>>>>>>>> +++ b/Documentation/devicetree/bindings/power/qcom,rpmpd.txt
>>>>>>>> @@ -18,6 +18,16 @@ Required Properties:
>>>>>>>> Refer to <dt-bindings/power/qcom-rpmpd.h> for the level values for
>>>>>>>> various OPPs for different platforms as well as Power domain indexes
>>>>>>>>
>>>>>>>> += SUBNODES
>>>>>>>> +RPMh alsp hosts power domains that can behave as thermal warming device.
>>>>>>>> +These are expressed as subnodes of the RPMh. The name of the node is used
>>>>>>>> +to identify the power domain and must therefor be "mx".
>>>>>>>> +
>>>>>>>> +- #cooling-cells:
>>>>>>>> + Usage: optional
>>>>>>>> + Value type: <u32>
>>>>>>>> + Definition: must be 2
>>>>>>>> +
>>>>>>>
>>>>>>> Just wanted to express a minor thought about this. In general we use
>>>>>>> subnodes of PM domain providers to represent the topology of PM
>>>>>>> domains (subdomains), this is something different, which I guess is
>>>>>>> fine.
>>>>>>>
>>>>>>> I assume the #cooling-cells is here tells us this is not a PM domain
>>>>>>> provider, but a "cooling device provider"?
>>>>>> Yep.
>>>>>>>
>>>>>>> Also, I wonder if it would be fine to specify "power-domains" here,
>>>>>>> rather than using "name" as I think that is kind of awkward!?
>>>>>> Do you mean "power-domain-names" ? I am using this to match against the
>>>>>> genpd names defined in the provider driver.
>>>>>
>>>>> No. If you are using "power-domains" it means that you allow to
>>>>> describe the specifier for the provider.
>>>> Yep. But won't this look funny in DT ? The provider node will have a sub
>>>> node with a power domain referencing to itself Like below: Is this ok ?
>>>>
>>>> rpmhpd: power-controller {
>>>> compatible = "qcom,sdm845-rpmhpd";
>>>> #power-domain-cells = <1>;
>>>>
>>>> ...
>>>> ...
>>>> mx_cdev: mx {
>>>> #cooling-cells = <2>;
>>>> power-domains = <&rpmhpd SDM845_MX>;
>>>> };
>>>>
>>>
>>> The whole concept here seems all wrong to me. Isn't it what's in the
>>> power domain that's the cooling device. A CPU power domain is not a
>>> cooling device, the CPU is. Or we wouldn't make a clock a cooling
>>> device, but what the clock drives.
>>
>> Well, I don't think that's entirely correct description either.
>>
>> As I see it, it's really the actual PM domain (that manages voltages
>> for a power island), that needs to stay in full power state and
>> increase its voltage level, as to warm up some of the silicon. It's
>> not a regular device, but more a characteristics of how the PM domain
>> can be used.
>
> First I've heard of Si needing warming...
Cold regions and non-closing of circuits is what I am told.
>
> I think I'd just expect the power domain provider to know which
> domains to power on then.
I will just retain #cooling-cells in the power domain provider and let
the driver identify the actual power domains.
>
> Rob
>
--
Warm Regards
Thara
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 3/7] thermal: core: Add late init hook to cooling device ops
[not found] ` <1571254641-13626-4-git-send-email-thara.gopinath@linaro.org>
@ 2019-12-03 17:04 ` Amit Kucheria
2019-12-03 17:09 ` Thara Gopinath
2019-12-03 17:11 ` Amit Kucheria
0 siblings, 2 replies; 10+ messages in thread
From: Amit Kucheria @ 2019-12-03 17:04 UTC (permalink / raw)
To: Thara Gopinath
Cc: Eduardo Valentin, Zhang Rui, Ulf Hansson, Daniel Lezcano,
Bjorn Andersson, Andy Gross, Mark Rutland, Rafael J. Wysocki,
Linux PM list,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-arm-msm, LKML
On Thu, Oct 17, 2019 at 1:07 AM Thara Gopinath
<thara.gopinath@linaro.org> wrote:
>
> Add a hook in thermal_cooling_device_ops to be called after
> the cooling device has been initialized and registered
> but before binding it to a thermal zone.
>
> In this patch series it is used to hook up a power domain
> to the device pointer of cooling device.
>
> It can be used for any other relevant late initializations
> of a cooling device as well.
Please describe WHY this hook is needed.
> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
> ---
> drivers/thermal/thermal_core.c | 13 +++++++++++++
> include/linux/thermal.h | 1 +
> 2 files changed, 14 insertions(+)
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 886e8fa..c2ecb73 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -994,6 +994,19 @@ __thermal_cooling_device_register(struct device_node *np,
> list_add(&cdev->node, &thermal_cdev_list);
> mutex_unlock(&thermal_list_lock);
>
> + /* Call into cdev late initialization if defined */
> + if (cdev->ops->late_init) {
> + result = cdev->ops->late_init(cdev);
> + if (result) {
> + ida_simple_remove(&thermal_cdev_ida, cdev->id);
> + put_device(&cdev->device);
> + mutex_lock(&thermal_list_lock);
> + list_del(&cdev->node);
> + mutex_unlock(&thermal_list_lock);
> + return ERR_PTR(result);
> + }
> + }
> +
> /* Update binding information for 'this' new cdev */
> bind_cdev(cdev);
>
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index e45659c..e94b3de 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -125,6 +125,7 @@ struct thermal_cooling_device_ops {
> struct thermal_zone_device *, unsigned long, u32 *);
> int (*power2state)(struct thermal_cooling_device *,
> struct thermal_zone_device *, u32, unsigned long *);
> + int (*late_init)(struct thermal_cooling_device *);
> };
>
> struct thermal_cooling_device {
> --
> 2.1.4
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 3/7] thermal: core: Add late init hook to cooling device ops
2019-12-03 17:04 ` [PATCH v3 3/7] thermal: core: Add late init hook to cooling device ops Amit Kucheria
@ 2019-12-03 17:09 ` Thara Gopinath
2019-12-03 17:11 ` Amit Kucheria
1 sibling, 0 replies; 10+ messages in thread
From: Thara Gopinath @ 2019-12-03 17:09 UTC (permalink / raw)
To: Amit Kucheria
Cc: Eduardo Valentin, Zhang Rui, Ulf Hansson, Daniel Lezcano,
Bjorn Andersson, Andy Gross, Mark Rutland, Rafael J. Wysocki,
Linux PM list,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-arm-msm, LKML
On 12/03/2019 12:04 PM, Amit Kucheria wrote:
> On Thu, Oct 17, 2019 at 1:07 AM Thara Gopinath
> <thara.gopinath@linaro.org> wrote:
>>
>> Add a hook in thermal_cooling_device_ops to be called after
>> the cooling device has been initialized and registered
>> but before binding it to a thermal zone.
>>
>> In this patch series it is used to hook up a power domain
>> to the device pointer of cooling device.
>>
>> It can be used for any other relevant late initializations
>> of a cooling device as well.
>
> Please describe WHY this hook is needed.
Hi Amit,
Thanks for the review. This patch is no longer relevant.
I had posted a v4 of this series based on other review comments[1]
1. https://lkml.org/lkml/2019/11/20/355
--
Warm Regards
Thara
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 3/7] thermal: core: Add late init hook to cooling device ops
2019-12-03 17:04 ` [PATCH v3 3/7] thermal: core: Add late init hook to cooling device ops Amit Kucheria
2019-12-03 17:09 ` Thara Gopinath
@ 2019-12-03 17:11 ` Amit Kucheria
1 sibling, 0 replies; 10+ messages in thread
From: Amit Kucheria @ 2019-12-03 17:11 UTC (permalink / raw)
To: Thara Gopinath
Cc: Eduardo Valentin, Zhang Rui, Ulf Hansson, Daniel Lezcano,
Bjorn Andersson, Andy Gross, Mark Rutland, Rafael J. Wysocki,
Linux PM list,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-arm-msm, LKML
On Tue, Dec 3, 2019 at 10:34 PM Amit Kucheria
<amit.kucheria@verdurent.com> wrote:
>
> On Thu, Oct 17, 2019 at 1:07 AM Thara Gopinath
> <thara.gopinath@linaro.org> wrote:
> >
> > Add a hook in thermal_cooling_device_ops to be called after
> > the cooling device has been initialized and registered
> > but before binding it to a thermal zone.
> >
> > In this patch series it is used to hook up a power domain
> > to the device pointer of cooling device.
> >
> > It can be used for any other relevant late initializations
> > of a cooling device as well.
>
> Please describe WHY this hook is needed.
Just noticed you dropped this for v4. Nevermind.
> > Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
> > ---
> > drivers/thermal/thermal_core.c | 13 +++++++++++++
> > include/linux/thermal.h | 1 +
> > 2 files changed, 14 insertions(+)
> >
> > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> > index 886e8fa..c2ecb73 100644
> > --- a/drivers/thermal/thermal_core.c
> > +++ b/drivers/thermal/thermal_core.c
> > @@ -994,6 +994,19 @@ __thermal_cooling_device_register(struct device_node *np,
> > list_add(&cdev->node, &thermal_cdev_list);
> > mutex_unlock(&thermal_list_lock);
> >
> > + /* Call into cdev late initialization if defined */
> > + if (cdev->ops->late_init) {
> > + result = cdev->ops->late_init(cdev);
> > + if (result) {
> > + ida_simple_remove(&thermal_cdev_ida, cdev->id);
> > + put_device(&cdev->device);
> > + mutex_lock(&thermal_list_lock);
> > + list_del(&cdev->node);
> > + mutex_unlock(&thermal_list_lock);
> > + return ERR_PTR(result);
> > + }
> > + }
> > +
> > /* Update binding information for 'this' new cdev */
> > bind_cdev(cdev);
> >
> > diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> > index e45659c..e94b3de 100644
> > --- a/include/linux/thermal.h
> > +++ b/include/linux/thermal.h
> > @@ -125,6 +125,7 @@ struct thermal_cooling_device_ops {
> > struct thermal_zone_device *, unsigned long, u32 *);
> > int (*power2state)(struct thermal_cooling_device *,
> > struct thermal_zone_device *, u32, unsigned long *);
> > + int (*late_init)(struct thermal_cooling_device *);
> > };
> >
> > struct thermal_cooling_device {
> > --
> > 2.1.4
> >
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-12-03 17:11 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1571254641-13626-1-git-send-email-thara.gopinath@linaro.org>
[not found] ` <1571254641-13626-8-git-send-email-thara.gopinath@linaro.org>
2019-10-29 1:31 ` [PATCH v3 7/7] arm64: dts: qcom: Add mx power domain as thermal warming device Rob Herring
2019-10-30 14:23 ` Thara Gopinath
[not found] ` <1571254641-13626-7-git-send-email-thara.gopinath@linaro.org>
[not found] ` <CAPDyKFqcKfmnNJ7j4Jb+JH739FBcHg5NBD6aR4H_N=zWGwm1ww@mail.gmail.com>
[not found] ` <5DA88892.5000408@linaro.org>
[not found] ` <CAPDyKFpYG7YADb6Xmm=8ug5=5X3d1y+JdkRvrnvtroeV3Yj62Q@mail.gmail.com>
[not found] ` <5DA89267.30806@linaro.org>
2019-10-29 1:36 ` [PATCH v3 6/7] dt-bindings: soc: qcom: Extend RPMh power controller binding to describe " Rob Herring
2019-10-29 10:06 ` Ulf Hansson
2019-10-29 20:16 ` Rob Herring
2019-10-30 9:27 ` Ulf Hansson
2019-10-30 14:27 ` Thara Gopinath
[not found] ` <1571254641-13626-4-git-send-email-thara.gopinath@linaro.org>
2019-12-03 17:04 ` [PATCH v3 3/7] thermal: core: Add late init hook to cooling device ops Amit Kucheria
2019-12-03 17:09 ` Thara Gopinath
2019-12-03 17:11 ` Amit Kucheria
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).