All of lore.kernel.org
 help / color / mirror / Atom feed
* [Query] thermal: Who is using "cooling-{min|max}-level}" properties ?
@ 2018-02-07  6:59 Viresh Kumar
  2018-02-07 10:04 ` Daniel Lezcano
  0 siblings, 1 reply; 13+ messages in thread
From: Viresh Kumar @ 2018-02-07  6:59 UTC (permalink / raw)
  To: Zhang Rui, Eduardo Valentin
  Cc: Vincent Guittot, linux-pm, robh+dt, devicetree, daniel.lezcano,
	Punit.Agrawal, ionela.voinescu

Looks like I am missing the obvious and sorry for the noise in advance.

But I couldn't find any code that parses the "cooling-{min|max}-level" DT
bindings in the kernel. Yeah, almost every ARM platform have these properties
set for their CPU nodes in DT, but I don't see how these are getting used.

I even tried to remove them for my hikey platform and everything worked as if
nothing has changed (which I was expecting anyway). The deal is that the
->get_max_state() callback of the cooling drivers are getting called to get the
range at runtime and none of them refers to these bindings.

Removing code is always fun and I would be happy to post a series to clean
things up if everyone agrees. Please let me know if my understanding is correct
and if it would be fine to remove these bindings completely.

Thanks.

-- 
viresh

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Query] thermal: Who is using "cooling-{min|max}-level}" properties ?
  2018-02-07  6:59 [Query] thermal: Who is using "cooling-{min|max}-level}" properties ? Viresh Kumar
@ 2018-02-07 10:04 ` Daniel Lezcano
       [not found]   ` <c7bd6ed5-b93e-3a07-b4a2-4c3b1eef5fac-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
  0 siblings, 1 reply; 13+ messages in thread
From: Daniel Lezcano @ 2018-02-07 10:04 UTC (permalink / raw)
  To: Viresh Kumar, Zhang Rui, Eduardo Valentin
  Cc: Vincent Guittot, linux-pm-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Punit.Agrawal-5wv7dgnIgG8,
	ionela.voinescu-5wv7dgnIgG8

On 07/02/2018 07:59, Viresh Kumar wrote:
> Looks like I am missing the obvious and sorry for the noise in advance.
> 
> But I couldn't find any code that parses the "cooling-{min|max}-level" DT
> bindings in the kernel. Yeah, almost every ARM platform have these properties
> set for their CPU nodes in DT, but I don't see how these are getting used.
> 
> I even tried to remove them for my hikey platform and everything worked as if
> nothing has changed (which I was expecting anyway). The deal is that the
> ->get_max_state() callback of the cooling drivers are getting called to get the
> range at runtime and none of them refers to these bindings.
> 
> Removing code is always fun and I would be happy to post a series to clean
> things up if everyone agrees. Please let me know if my understanding is correct
> and if it would be fine to remove these bindings completely.

Yeah, this is a bit fuzzy. There are still cooling-{min|max}-*state*
definitions in the DTs

Documentation/devicetree/bindings/hwmon/pwm-fan.txt:
cooling-min-state = <0>;
arch/arm/boot/dts/exynos4412-odroidu3.dts:
cooling-min-state = <0>;
arch/arm/boot/dts/exynos5410-odroidxu.dts:
cooling-min-state = <0>;
arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi:
cooling-min-state = <0>;
arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi:
cooling-min-state = <0>;


IIUC, the cooling-device in the mapping defines the min and max states
which are the values used in the thermal zone binding.

And the cooling-[min|max]-level are the hardware limits:

cooling-min-level <= cooling-device min|max state <= cooling-max-level

For example on the hikey6220:

The DT specifies for CPU0:

	operating-points-v2 = <&cpu_opp_table>;
	cooling-min-level = <4>;
	cooling-max-level = <0>;
	#cooling-cells = <2>; /* min followed by max */

and for the cooling device for map0

	cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;

So if my understanding is correct, the optional property
cooling-[min|max]-level is used to make sure the cooling device min
state and max state are in the hardware boundaries. And the thermal
framework misses to do the consistency check at init.

I'm not sure how useful are cooling-{min|max}-*state* bindings.

-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Query] thermal: Who is using "cooling-{min|max}-level}" properties ?
       [not found]   ` <c7bd6ed5-b93e-3a07-b4a2-4c3b1eef5fac-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
@ 2018-02-07 10:24     ` Viresh Kumar
  2018-02-07 10:45       ` Daniel Lezcano
  0 siblings, 1 reply; 13+ messages in thread
From: Viresh Kumar @ 2018-02-07 10:24 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Zhang Rui, Eduardo Valentin, Vincent Guittot,
	linux-pm-u79uwXL29TY76Z2rM5mHXA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Punit.Agrawal-5wv7dgnIgG8,
	ionela.voinescu-5wv7dgnIgG8

On 07-02-18, 11:04, Daniel Lezcano wrote:
> Yeah, this is a bit fuzzy. There are still cooling-{min|max}-*state*
> definitions in the DTs
> 
> Documentation/devicetree/bindings/hwmon/pwm-fan.txt:
> cooling-min-state = <0>;
> arch/arm/boot/dts/exynos4412-odroidu3.dts:
> cooling-min-state = <0>;
> arch/arm/boot/dts/exynos5410-odroidxu.dts:
> cooling-min-state = <0>;
> arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi:
> cooling-min-state = <0>;
> arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi:
> cooling-min-state = <0>;
> 
> 
> IIUC, the cooling-device in the mapping defines the min and max states
> which are the values used in the thermal zone binding.
> 
> And the cooling-[min|max]-level are the hardware limits:
> 
> cooling-min-level <= cooling-device min|max state <= cooling-max-level
> 
> For example on the hikey6220:
> 
> The DT specifies for CPU0:
> 
> 	operating-points-v2 = <&cpu_opp_table>;
> 	cooling-min-level = <4>;
> 	cooling-max-level = <0>;
> 	#cooling-cells = <2>; /* min followed by max */
> 
> and for the cooling device for map0
> 
> 	cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> 
> So if my understanding is correct, the optional property
> cooling-[min|max]-level is used to make sure the cooling device min
> state and max state are in the hardware boundaries. And the thermal
> framework misses to do the consistency check at init.
> 
> I'm not sure how useful are cooling-{min|max}-*state* bindings.

Okay, but we aren't validating it this way in code. Isn't it ?

-- 
viresh
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Query] thermal: Who is using "cooling-{min|max}-level}" properties ?
  2018-02-07 10:24     ` Viresh Kumar
@ 2018-02-07 10:45       ` Daniel Lezcano
  2018-02-09  6:42         ` Viresh Kumar
  0 siblings, 1 reply; 13+ messages in thread
From: Daniel Lezcano @ 2018-02-07 10:45 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Zhang Rui, Eduardo Valentin, Vincent Guittot, linux-pm, robh+dt,
	devicetree, Punit.Agrawal, ionela.voinescu

On 07/02/2018 11:24, Viresh Kumar wrote:
> On 07-02-18, 11:04, Daniel Lezcano wrote:
>> Yeah, this is a bit fuzzy. There are still cooling-{min|max}-*state*
>> definitions in the DTs
>>
>> Documentation/devicetree/bindings/hwmon/pwm-fan.txt:
>> cooling-min-state = <0>;
>> arch/arm/boot/dts/exynos4412-odroidu3.dts:
>> cooling-min-state = <0>;
>> arch/arm/boot/dts/exynos5410-odroidxu.dts:
>> cooling-min-state = <0>;
>> arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi:
>> cooling-min-state = <0>;
>> arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi:
>> cooling-min-state = <0>;
>>
>>
>> IIUC, the cooling-device in the mapping defines the min and max states
>> which are the values used in the thermal zone binding.
>>
>> And the cooling-[min|max]-level are the hardware limits:
>>
>> cooling-min-level <= cooling-device min|max state <= cooling-max-level
>>
>> For example on the hikey6220:
>>
>> The DT specifies for CPU0:
>>
>> 	operating-points-v2 = <&cpu_opp_table>;
>> 	cooling-min-level = <4>;
>> 	cooling-max-level = <0>;
>> 	#cooling-cells = <2>; /* min followed by max */
>>
>> and for the cooling device for map0
>>
>> 	cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
>>
>> So if my understanding is correct, the optional property
>> cooling-[min|max]-level is used to make sure the cooling device min
>> state and max state are in the hardware boundaries. And the thermal
>> framework misses to do the consistency check at init.
>>
>> I'm not sure how useful are cooling-{min|max}-*state* bindings.

I meant here cooling-{min|max}-*level*

> Okay, but we aren't validating it this way in code. Isn't it ?

Yes, that is my understanding. cooling-min-level and cooling-max-level
are not used in the thermal framework code today.

So if they are defined, we should check the cooling-device max and min
are in the boundaries (if they are different from no-limit).

That needs confirmation from the maintainers, I'm not 100% sure.


-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Query] thermal: Who is using "cooling-{min|max}-level}" properties ?
  2018-02-07 10:45       ` Daniel Lezcano
@ 2018-02-09  6:42         ` Viresh Kumar
  2018-02-09  9:15           ` Daniel Lezcano
  0 siblings, 1 reply; 13+ messages in thread
From: Viresh Kumar @ 2018-02-09  6:42 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Zhang Rui, Eduardo Valentin, Vincent Guittot, linux-pm, robh+dt,
	devicetree, Punit.Agrawal, ionela.voinescu

On 07-02-18, 11:45, Daniel Lezcano wrote:
> Yes, that is my understanding. cooling-min-level and cooling-max-level
> are not used in the thermal framework code today.

Right.

> So if they are defined, we should check the cooling-device max and min
> are in the boundaries (if they are different from no-limit).

Hmm, I am not sure. We do compare those values (from maps) with the
max reported by the cooling driver, so there is some boundary check
happening. And I don't think it is worth comparing the min/max values
from DT cooling device's nodes. Why not just leave those for the
driver to return (which is already happening btw).

I don't think it would be correct for the thermal core to go and look
at the min/max properties of the cooling device directly, as those are
more for the thermal driver's help. The thermal core should just call
the get_max_state() callback and that's it.

Anyway, we can take decision on the binding itself after some time but
I will send some patches to get rid of this property from CPU nodes
for now. It doesn't make sense to have it there (anyway it is
optional), as the cpu cooling devices are kind of virtual cooling
devices which rely on OPP or freq-table currently. Maybe I will begin
by just updating one platform and once that is merged, update
everything else as well.

-- 
viresh

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Query] thermal: Who is using "cooling-{min|max}-level}" properties ?
  2018-02-09  6:42         ` Viresh Kumar
@ 2018-02-09  9:15           ` Daniel Lezcano
       [not found]             ` <539699c7-9509-dea2-2b31-c5f6749c99c4-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
  0 siblings, 1 reply; 13+ messages in thread
From: Daniel Lezcano @ 2018-02-09  9:15 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Zhang Rui, Eduardo Valentin, Vincent Guittot, linux-pm, robh+dt,
	devicetree, Punit.Agrawal, ionela.voinescu

On 09/02/2018 07:42, Viresh Kumar wrote:
> On 07-02-18, 11:45, Daniel Lezcano wrote:
>> Yes, that is my understanding. cooling-min-level and cooling-max-level
>> are not used in the thermal framework code today.
> 
> Right.
> 
>> So if they are defined, we should check the cooling-device max and min
>> are in the boundaries (if they are different from no-limit).
> 
> Hmm, I am not sure. We do compare those values (from maps) with the
> max reported by the cooling driver, so there is some boundary check
> happening. And I don't think it is worth comparing the min/max values
> from DT cooling device's nodes. Why not just leave those for the
> driver to return (which is already happening btw).
> 
> I don't think it would be correct for the thermal core to go and look
> at the min/max properties of the cooling device directly, as those are
> more for the thermal driver's help. The thermal core should just call
> the get_max_state() callback and that's it.
> 
> Anyway, we can take decision on the binding itself after some time but
> I will send some patches to get rid of this property from CPU nodes
> for now. It doesn't make sense to have it there (anyway it is
> optional), as the cpu cooling devices are kind of virtual cooling
> devices which rely on OPP or freq-table currently. Maybe I will begin
> by just updating one platform and once that is merged, update
> everything else as well.

What about the "cooling-cells" ? Its usage is unclear in the code and
I'm not sure it is really needed.


-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Query] thermal: Who is using "cooling-{min|max}-level}" properties ?
       [not found]             ` <539699c7-9509-dea2-2b31-c5f6749c99c4-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
@ 2018-02-09  9:24               ` Viresh Kumar
  2018-02-09 12:14                 ` Daniel Lezcano
  0 siblings, 1 reply; 13+ messages in thread
From: Viresh Kumar @ 2018-02-09  9:24 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Zhang Rui, Eduardo Valentin, Vincent Guittot,
	linux-pm-u79uwXL29TY76Z2rM5mHXA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Punit.Agrawal-5wv7dgnIgG8,
	ionela.voinescu-5wv7dgnIgG8

On 09-02-18, 10:15, Daniel Lezcano wrote:
> On 09/02/2018 07:42, Viresh Kumar wrote:
> > On 07-02-18, 11:45, Daniel Lezcano wrote:
> >> Yes, that is my understanding. cooling-min-level and cooling-max-level
> >> are not used in the thermal framework code today.
> > 
> > Right.
> > 
> >> So if they are defined, we should check the cooling-device max and min
> >> are in the boundaries (if they are different from no-limit).
> > 
> > Hmm, I am not sure. We do compare those values (from maps) with the
> > max reported by the cooling driver, so there is some boundary check
> > happening. And I don't think it is worth comparing the min/max values
> > from DT cooling device's nodes. Why not just leave those for the
> > driver to return (which is already happening btw).
> > 
> > I don't think it would be correct for the thermal core to go and look
> > at the min/max properties of the cooling device directly, as those are
> > more for the thermal driver's help. The thermal core should just call
> > the get_max_state() callback and that's it.
> > 
> > Anyway, we can take decision on the binding itself after some time but
> > I will send some patches to get rid of this property from CPU nodes
> > for now. It doesn't make sense to have it there (anyway it is
> > optional), as the cpu cooling devices are kind of virtual cooling
> > devices which rely on OPP or freq-table currently. Maybe I will begin
> > by just updating one platform and once that is merged, update
> > everything else as well.

So I eventually updated everything as very small number of platforms
were actually using it.

https://lkml.kernel.org/r/cover.1518166039.git.viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org

> What about the "cooling-cells" ? Its usage is unclear in the code and
> I'm not sure it is really needed.

As I can see, it is actually used (sometimes a bit indirectly).

- This field is used to check if a device is a cooling device or not.
  For example, the cpu-cooling device wouldn't be registered by the
  cpufreq drivers unless this property is present in the CPU node. So
  this is actually used.

- This field (and its value) also comes into picture while parsing the
  "cooling-maps". The "cooling-device" property in the map needs to
  have "cooing-cells" + 1 fields. The first one is the cooling-device
  phandle, followed by min/max states.

-- 
viresh
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Query] thermal: Who is using "cooling-{min|max}-level}" properties ?
  2018-02-09  9:24               ` Viresh Kumar
@ 2018-02-09 12:14                 ` Daniel Lezcano
       [not found]                   ` <5722441d-c8be-4d29-fab7-28b4e5716961-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
  0 siblings, 1 reply; 13+ messages in thread
From: Daniel Lezcano @ 2018-02-09 12:14 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Zhang Rui, Eduardo Valentin, Vincent Guittot, linux-pm, robh+dt,
	devicetree, Punit.Agrawal, ionela.voinescu

On 09/02/2018 10:24, Viresh Kumar wrote:
> On 09-02-18, 10:15, Daniel Lezcano wrote:
>> On 09/02/2018 07:42, Viresh Kumar wrote:
>>> On 07-02-18, 11:45, Daniel Lezcano wrote:
>>>> Yes, that is my understanding. cooling-min-level and cooling-max-level
>>>> are not used in the thermal framework code today.
>>>
>>> Right.
>>>
>>>> So if they are defined, we should check the cooling-device max and min
>>>> are in the boundaries (if they are different from no-limit).
>>>
>>> Hmm, I am not sure. We do compare those values (from maps) with the
>>> max reported by the cooling driver, so there is some boundary check
>>> happening. And I don't think it is worth comparing the min/max values
>>> from DT cooling device's nodes. Why not just leave those for the
>>> driver to return (which is already happening btw).
>>>
>>> I don't think it would be correct for the thermal core to go and look
>>> at the min/max properties of the cooling device directly, as those are
>>> more for the thermal driver's help. The thermal core should just call
>>> the get_max_state() callback and that's it.
>>>
>>> Anyway, we can take decision on the binding itself after some time but
>>> I will send some patches to get rid of this property from CPU nodes
>>> for now. It doesn't make sense to have it there (anyway it is
>>> optional), as the cpu cooling devices are kind of virtual cooling
>>> devices which rely on OPP or freq-table currently. Maybe I will begin
>>> by just updating one platform and once that is merged, update
>>> everything else as well.
> 
> So I eventually updated everything as very small number of platforms
> were actually using it.
> 
> https://lkml.kernel.org/r/cover.1518166039.git.viresh.kumar@linaro.org

Yep, saw them.

>> What about the "cooling-cells" ? Its usage is unclear in the code and
>> I'm not sure it is really needed.
> 
> As I can see, it is actually used (sometimes a bit indirectly).

(yeah, that was the meaning of 'unclear')

> - This field is used to check if a device is a cooling device or not.
>   For example, the cpu-cooling device wouldn't be registered by the
>   cpufreq drivers unless this property is present in the CPU node. So
>   this is actually used.
>
> - This field (and its value) also comes into picture while parsing the
>   "cooling-maps". The "cooling-device" property in the map needs to
>   have "cooing-cells" + 1 fields. The first one is the cooling-device
>   phandle, followed by min/max states.

Right. The semantic is unclear. The expected cooling-cells value is
always 2 (the code ignore values greater than two and yell if it is less
than 2). And the cooling-device binding tells there are two states.

So the question is why do we need a cooling-cells as the information is
pointless here (always 2) ?

I see this field is artificially used to tell the cpufreq driver "please
register me as a cooling device". This is inconsistent from my pov.

Furthermore, the thermal-zone with the cooling device binds with the CPU
phandle, not the cooling-cells.

Putting apart the device binding changes discussion for the moment. Why
not register the cpufreq driver in all the cases and then drop
cooling-cells ? So when the opps are present, its registers in the
cpufreq->ready callback.





-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Query] thermal: Who is using "cooling-{min|max}-level}" properties ?
       [not found]                   ` <5722441d-c8be-4d29-fab7-28b4e5716961-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
@ 2018-02-12  6:10                     ` Viresh Kumar
  2018-02-12  8:39                       ` Daniel Lezcano
  0 siblings, 1 reply; 13+ messages in thread
From: Viresh Kumar @ 2018-02-12  6:10 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Zhang Rui, Eduardo Valentin, Vincent Guittot,
	linux-pm-u79uwXL29TY76Z2rM5mHXA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Punit.Agrawal-5wv7dgnIgG8,
	ionela.voinescu-5wv7dgnIgG8

On 09-02-18, 13:14, Daniel Lezcano wrote:
> Right. The semantic is unclear. The expected cooling-cells value is
> always 2 (the code ignore values greater than two and yell if it is less
> than 2). And the cooling-device binding tells there are two states.

They are fixed to 2 as we don't need to use it differently for now.
But that's more about how Linux is using this stuff. The binding still
needs to provide a way to have more cells than just 2.

> So the question is why do we need a cooling-cells as the information is
> pointless here (always 2) ?

To show that the device is a "cooling-device" and I see that
consistent with everything else in DT, for example: interrupt-cells,
gpio-cells, clock-cells, reset-cells, dma-cells. The "*-cells"
properties is used widely to tell what the device can behave as, i.e.
a cooling-device in our case.

Now we always use 2 parameters exactly is a different thing all
together :)

> I see this field is artificially used to tell the cpufreq driver "please
> register me as a cooling device". This is inconsistent from my pov.

But that's how its used for every other controller, what's different
here ?

> Furthermore, the thermal-zone with the cooling device binds with the CPU
> phandle, not the cooling-cells.

Yeah, because that's the device really.

> Putting apart the device binding changes discussion for the moment. Why
> not register the cpufreq driver in all the cases and then drop

s/cpufreq driver/cpufreq cooling driver/ ??

> cooling-cells ? So when the opps are present, its registers in the
> cpufreq->ready callback.

How will someone tell if they don't need the cooling infrastructure ?
Specially for multiplatform thing.

-- 
viresh
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Query] thermal: Who is using "cooling-{min|max}-level}" properties ?
  2018-02-12  6:10                     ` Viresh Kumar
@ 2018-02-12  8:39                       ` Daniel Lezcano
  2018-02-12  8:52                         ` Viresh Kumar
  0 siblings, 1 reply; 13+ messages in thread
From: Daniel Lezcano @ 2018-02-12  8:39 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Zhang Rui, Eduardo Valentin, Vincent Guittot, linux-pm, robh+dt,
	devicetree, Punit.Agrawal, ionela.voinescu

On 12/02/2018 07:10, Viresh Kumar wrote:
> On 09-02-18, 13:14, Daniel Lezcano wrote:
>> Right. The semantic is unclear. The expected cooling-cells value is
>> always 2 (the code ignore values greater than two and yell if it is less
>> than 2). And the cooling-device binding tells there are two states.
> 
> They are fixed to 2 as we don't need to use it differently for now.
> But that's more about how Linux is using this stuff. The binding still
> needs to provide a way to have more cells than just 2.
> 
>> So the question is why do we need a cooling-cells as the information is
>> pointless here (always 2) ?
> 
> To show that the device is a "cooling-device" and I see that
> consistent with everything else in DT, for example: interrupt-cells,
> gpio-cells, clock-cells, reset-cells, dma-cells. The "*-cells"
> properties is used widely to tell what the device can behave as, i.e.
> a cooling-device in our case.

Ah, yes. That's right.

Well I don't know. If that is the way the other drivers are doing, I
suppose we should keep it as it is.

> Now we always use 2 parameters exactly is a different thing all
> together :)
> 
>> I see this field is artificially used to tell the cpufreq driver "please
>> register me as a cooling device". This is inconsistent from my pov.
> 
> But that's how its used for every other controller, what's different
> here ?
> 
>> Furthermore, the thermal-zone with the cooling device binds with the CPU
>> phandle, not the cooling-cells.
> 
> Yeah, because that's the device really.
> 
>> Putting apart the device binding changes discussion for the moment. Why
>> not register the cpufreq driver in all the cases and then drop
> 
> s/cpufreq driver/cpufreq cooling driver/ ??

yes.

>> cooling-cells ? So when the opps are present, its registers in the
>> cpufreq->ready callback.
> 
> How will someone tell if they don't need the cooling infrastructure ?
> Specially for multiplatform thing.

They don't define a cooling device in the DT ?


-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Query] thermal: Who is using "cooling-{min|max}-level}" properties ?
  2018-02-12  8:39                       ` Daniel Lezcano
@ 2018-02-12  8:52                         ` Viresh Kumar
  2018-02-12  9:52                           ` Daniel Lezcano
  0 siblings, 1 reply; 13+ messages in thread
From: Viresh Kumar @ 2018-02-12  8:52 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Zhang Rui, Eduardo Valentin, Vincent Guittot, linux-pm, robh+dt,
	devicetree, Punit.Agrawal, ionela.voinescu

On 12-02-18, 09:39, Daniel Lezcano wrote:
> On 12/02/2018 07:10, Viresh Kumar wrote:
> > On 09-02-18, 13:14, Daniel Lezcano wrote:
> >> Putting apart the device binding changes discussion for the moment. Why
> >> not register the cpufreq driver in all the cases and then drop
> > 
> > s/cpufreq driver/cpufreq cooling driver/ ??
> 
> yes.
> 
> >> cooling-cells ? So when the opps are present, its registers in the
> >> cpufreq->ready callback.
> > 
> > How will someone tell if they don't need the cooling infrastructure ?
> > Specially for multiplatform thing.
> 
> They don't define a cooling device in the DT ?

Sorry but I am getting a bit confused on what you are suggesting.
Maybe you need to rephrase it once more for me, from the very
beginning :)

-- 
viresh

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Query] thermal: Who is using "cooling-{min|max}-level}" properties ?
  2018-02-12  8:52                         ` Viresh Kumar
@ 2018-02-12  9:52                           ` Daniel Lezcano
  2018-02-12 10:17                             ` Viresh Kumar
  0 siblings, 1 reply; 13+ messages in thread
From: Daniel Lezcano @ 2018-02-12  9:52 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Zhang Rui, Eduardo Valentin, Vincent Guittot,
	linux-pm-u79uwXL29TY76Z2rM5mHXA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Punit.Agrawal-5wv7dgnIgG8,
	ionela.voinescu-5wv7dgnIgG8

On 12/02/2018 09:52, Viresh Kumar wrote:
> On 12-02-18, 09:39, Daniel Lezcano wrote:
>> On 12/02/2018 07:10, Viresh Kumar wrote:
>>> On 09-02-18, 13:14, Daniel Lezcano wrote:
>>>> Putting apart the device binding changes discussion for the moment. Why
>>>> not register the cpufreq driver in all the cases and then drop
>>>
>>> s/cpufreq driver/cpufreq cooling driver/ ??
>>
>> yes.
>>
>>>> cooling-cells ? So when the opps are present, its registers in the
>>>> cpufreq->ready callback.
>>>
>>> How will someone tell if they don't need the cooling infrastructure ?
>>> Specially for multiplatform thing.
>>
>> They don't define a cooling device in the DT ?
> 
> Sorry but I am getting a bit confused on what you are suggesting.
> Maybe you need to rephrase it once more for me, from the very
> beginning :)

Not sure it is worth to go further in the discussion. I realize the
*-cells is widely used everywhere. I was wondering if cooling-cells
still have a meaning if we remove the min|max-levels as it is always
two. But so far it is used as a cooling property for the device in any
case and removing it *is* inconsistent with the rest of the code.

Just to clarify my comment above, I understood you were asking how to
not use the cpu as a cooling device. As the binding between the cooling
device and the thermal zone happens when the DT nodes are the same, when
registering the cpufreq driver as a cooling device and when the
cooling-device registers itself at the cooling-map parsing time, if we
remove the cooling-device in the cooling-map, the binding never happens
thus the cpu is never used as a cooling device.  However, these are all
required fields, so the only way to do that is to remove the entire
thermal zone node, not sure it is what we want :)


-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Query] thermal: Who is using "cooling-{min|max}-level}" properties ?
  2018-02-12  9:52                           ` Daniel Lezcano
@ 2018-02-12 10:17                             ` Viresh Kumar
  0 siblings, 0 replies; 13+ messages in thread
From: Viresh Kumar @ 2018-02-12 10:17 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Zhang Rui, Eduardo Valentin, Vincent Guittot, linux-pm, robh+dt,
	devicetree, Punit.Agrawal, ionela.voinescu

On 12-02-18, 10:52, Daniel Lezcano wrote:
> Just to clarify my comment above, I understood you were asking how to
> not use the cpu as a cooling device. As the binding between the cooling
> device and the thermal zone happens when the DT nodes are the same, when
> registering the cpufreq driver as a cooling device and when the
> cooling-device registers itself at the cooling-map parsing time, if we
> remove the cooling-device in the cooling-map, the binding never happens
> thus the cpu is never used as a cooling device.

Ahh, understood your comment now.

> However, these are all
> required fields, so the only way to do that is to remove the entire
> thermal zone node, not sure it is what we want :)

Yeah, so thing stay as is for now :)

-- 
viresh

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2018-02-12 10:17 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-07  6:59 [Query] thermal: Who is using "cooling-{min|max}-level}" properties ? Viresh Kumar
2018-02-07 10:04 ` Daniel Lezcano
     [not found]   ` <c7bd6ed5-b93e-3a07-b4a2-4c3b1eef5fac-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2018-02-07 10:24     ` Viresh Kumar
2018-02-07 10:45       ` Daniel Lezcano
2018-02-09  6:42         ` Viresh Kumar
2018-02-09  9:15           ` Daniel Lezcano
     [not found]             ` <539699c7-9509-dea2-2b31-c5f6749c99c4-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2018-02-09  9:24               ` Viresh Kumar
2018-02-09 12:14                 ` Daniel Lezcano
     [not found]                   ` <5722441d-c8be-4d29-fab7-28b4e5716961-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2018-02-12  6:10                     ` Viresh Kumar
2018-02-12  8:39                       ` Daniel Lezcano
2018-02-12  8:52                         ` Viresh Kumar
2018-02-12  9:52                           ` Daniel Lezcano
2018-02-12 10:17                             ` Viresh Kumar

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.