On Thu, 17 Oct 2019 at 17:28, Thara Gopinath <thara.gopinath@xxxxxxxxxx> 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@xxxxxxxxxx> 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@xxxxxxxxxx> > >> --- > >> 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