diff mbox series

[v3,6/7] dt-bindings: soc: qcom: Extend RPMh power controller binding to describe thermal warming device

Message ID 1571254641-13626-7-git-send-email-thara.gopinath@linaro.org
State New
Headers show
Series Introduce Power domain based warming device driver | expand

Commit Message

Thara Gopinath Oct. 16, 2019, 7:37 p.m. UTC
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(+)

-- 
2.1.4

Comments

Ulf Hansson Oct. 17, 2019, 3:43 p.m. UTC | #1
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.

From Linux point of view, it means you can use dev_pm_domain_attach()
to hook up the corresponding device with the PM domain.

Using "power-domain-names" is just to allow to specify a name rather
than an index, which makes sense if there is more than one index.
Perhaps you can state that the "power-domain-names" should be there
anyway, to be a little bit future proof if ever multiple index
(multiple PM domains).

Kind regards
Uffe
Thara Gopinath Oct. 17, 2019, 4:10 p.m. UTC | #2
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>;
                                };
				
> 

> From Linux point of view, it means you can use dev_pm_domain_attach()

> to hook up the corresponding device with the PM domain.


Yes. Only the thermal framework does not populate cdev->dev->of_node.
But it should be a trivial thing to fix it. Also if I end up creating a
separate device, it should not matter.
> 

> Using "power-domain-names" is just to allow to specify a name rather

> than an index, which makes sense if there is more than one index.

> Perhaps you can state that the "power-domain-names" should be there

> anyway, to be a little bit future proof if ever multiple index

> (multiple PM domains).

> 

> Kind regards

> Uffe

> 



-- 
Warm Regards
Thara
Rob Herring (Arm) Oct. 29, 2019, 8:16 p.m. UTC | #3
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
diff mbox series

Patch

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
+
 Example: rpmh power domain controller and OPP table
 
 #include <dt-bindings/power/qcom-rpmhpd.h>