Re: [PATCH 2/9] hwmon: dts: Doc: Add DTS doc to explain how to use PWM FAN as a cooling device
On Wed, 2015-01-14 at 14:56 +0100, Lukasz Majewski wrote: Hi Sjoerd, Could you review v2 of this patch series? http://www.spinics.net/lists/devicetree/msg63159.html I skimmed it yesterday, I'll try to find some time to send you my review comments later in the week/start of next. Do you happen to have a git branch with these patches in? That would make it convenient for me to give your patches a spin on my XU3 -- Sjoerd Simons sjoerd.sim...@collabora.co.uk Collabora Ltd. smime.p7s Description: S/MIME cryptographic signature
Re: [PATCH 2/9] hwmon: dts: Doc: Add DTS doc to explain how to use PWM FAN as a cooling device
Hi Sjoerd, Hey Lukasz, Blame the holiday season for my late reply ;) On Fri, 2014-12-19 at 17:13 +0100, Lukasz Majewski wrote: Hi Guenter, On Fri, Dec 19, 2014 at 04:32:24PM +0100, Lukasz Majewski wrote: Hi Sjoerd, Thanks for your feedback and sorry for a late reply. On Thu, 2014-12-18 at 11:13 +0100, Lukasz Majewski wrote: Several new properties to allow PWM fan working as a cooling device have been combined into this single commit. Signed-off-by: Lukasz Majewski l.majew...@samsung.com --- .../devicetree/bindings/hwmon/pwm-fan.txt | 28 ++ 1 file changed, 28 insertions(+) diff --git a/Documentation/devicetree/bindings/hwmon/pwm-fan.txt b/Documentation/devicetree/bindings/hwmon/pwm-fan.txt index 610757c..3877810 100644 --- a/Documentation/devicetree/bindings/hwmon/pwm-fan.txt +++ b/Documentation/devicetree/bindings/hwmon/pwm-fan.txt @@ -3,10 +3,38 @@ Bindings for a fan connected to the PWM lines Required properties: - compatible : pwm-fan - pwms : the PWM that is used to control the PWM fan +- cooling-pwm-values : PWM duty cycle values relative to + cooling-max-pwm-value correspondig to + proper cooling states +- default-pulse-width : Property specifying default pulse width for FAN + at system boot (zero to disable FAN on boot). + Allowed range is 0 to 255 The 0..255 range seems somewhat random. Would be nicer to either use the approach of pwm-backlight (iotw, have the range go from the first to the last entry of cooling-pwm-values) I'm OK to change the default-pulse-width to be similar to default-brightness-level (as it is in Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt) or simply have be the duty lenght in NS as entries instead of the current indirection. I'd prefer to keep the indirection - as it is utilized in the current pwm-fan.c driver. FWIW, devicetree information is supposed to be implementation independent. So this is a poor argument. Many other pwm drivers use the indirection - e.g. mentioned pwm-backlight. I don't specifically mind the indirection, i was just thinking out loud whether it added value (but if it's quite common, might indeed be good to keep the pattern). What i do dislike is the number of levels is being set to an arbitrary levels, as that will very rarely match the actual number of distinct pwm levels you One thing though, when following the pattern of the pwm-backlight driver; In pwm-backlight the highest index of brightness-levels is always 100% duty cycle.. On e.g. XU3 the vendor kernel never drives the fan at 100% duty (maximum of 91%). So it would be nice if the dt bindigns could model that e.g. by having: pwm-levels = 20; // 21 distinct pwm levels valid-pwm-level = 5 15 18; /* 5 15 and 18 are usable levels - pwm will default to highest level */ Could you review v2 of this patch series? http://www.spinics.net/lists/devicetree/msg63159.html Enabling pan to full RPM was the default behaviour in the current pwm-fan.c file. To be honest, there is no need to enable fan to full RPM speed in this board for following reasons: 1. In Odroid the FAN is optional (stacked on top of a heat sink) - very often it is just enough to only have the heat sink. 2. Odroid has thermal enabled by default and IMHO it would be more feasible to allow thermal to control fan from the very beginning. However, I can also understand if the policy for hwmon implies a rule to enable by default all fans to full RPM speed. Why and how does that all suggest that the current default behavior should be changed ? I wanted to avoid the unpleasant sound for full speed fan when thermal is not enabled by default. But as I said, I fully understand the policy and I would be happy to comply with it as thermal should reduce the fan speed anyway at boot time. Yeah, what happens on my XU3 is that u-boot sets the pwm to 100% duty and the thermal infrastructure turns it off as soon as it gets into control, which works quite nicely (and keeps my sanity as that fan at 100% is *loud*)... So if you want to avoid unpleasant sounds, just build with thermal :p -- Best regards, Lukasz Majewski Samsung RD Institute Poland (SRPOL) | Linux Platform Group -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/9] hwmon: dts: Doc: Add DTS doc to explain how to use PWM FAN as a cooling device
Hey Lukasz, Blame the holiday season for my late reply ;) On Fri, 2014-12-19 at 17:13 +0100, Lukasz Majewski wrote: Hi Guenter, On Fri, Dec 19, 2014 at 04:32:24PM +0100, Lukasz Majewski wrote: Hi Sjoerd, Thanks for your feedback and sorry for a late reply. On Thu, 2014-12-18 at 11:13 +0100, Lukasz Majewski wrote: Several new properties to allow PWM fan working as a cooling device have been combined into this single commit. Signed-off-by: Lukasz Majewski l.majew...@samsung.com --- .../devicetree/bindings/hwmon/pwm-fan.txt | 28 ++ 1 file changed, 28 insertions(+) diff --git a/Documentation/devicetree/bindings/hwmon/pwm-fan.txt b/Documentation/devicetree/bindings/hwmon/pwm-fan.txt index 610757c..3877810 100644 --- a/Documentation/devicetree/bindings/hwmon/pwm-fan.txt +++ b/Documentation/devicetree/bindings/hwmon/pwm-fan.txt @@ -3,10 +3,38 @@ Bindings for a fan connected to the PWM lines Required properties: - compatible : pwm-fan - pwms : the PWM that is used to control the PWM fan +- cooling-pwm-values : PWM duty cycle values relative to + cooling-max-pwm-value correspondig to + proper cooling states +- default-pulse-width : Property specifying default pulse width for FAN + at system boot (zero to disable FAN on boot). + Allowed range is 0 to 255 The 0..255 range seems somewhat random. Would be nicer to either use the approach of pwm-backlight (iotw, have the range go from the first to the last entry of cooling-pwm-values) I'm OK to change the default-pulse-width to be similar to default-brightness-level (as it is in Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt) or simply have be the duty lenght in NS as entries instead of the current indirection. I'd prefer to keep the indirection - as it is utilized in the current pwm-fan.c driver. FWIW, devicetree information is supposed to be implementation independent. So this is a poor argument. Many other pwm drivers use the indirection - e.g. mentioned pwm-backlight. I don't specifically mind the indirection, i was just thinking out loud whether it added value (but if it's quite common, might indeed be good to keep the pattern). What i do dislike is the number of levels is being set to an arbitrary levels, as that will very rarely match the actual number of distinct pwm levels you One thing though, when following the pattern of the pwm-backlight driver; In pwm-backlight the highest index of brightness-levels is always 100% duty cycle.. On e.g. XU3 the vendor kernel never drives the fan at 100% duty (maximum of 91%). So it would be nice if the dt bindigns could model that e.g. by having: pwm-levels = 20; // 21 distinct pwm levels valid-pwm-level = 5 15 18; /* 5 15 and 18 are usable levels - pwm will default to highest level */ Enabling pan to full RPM was the default behaviour in the current pwm-fan.c file. To be honest, there is no need to enable fan to full RPM speed in this board for following reasons: 1. In Odroid the FAN is optional (stacked on top of a heat sink) - very often it is just enough to only have the heat sink. 2. Odroid has thermal enabled by default and IMHO it would be more feasible to allow thermal to control fan from the very beginning. However, I can also understand if the policy for hwmon implies a rule to enable by default all fans to full RPM speed. Why and how does that all suggest that the current default behavior should be changed ? I wanted to avoid the unpleasant sound for full speed fan when thermal is not enabled by default. But as I said, I fully understand the policy and I would be happy to comply with it as thermal should reduce the fan speed anyway at boot time. Yeah, what happens on my XU3 is that u-boot sets the pwm to 100% duty and the thermal infrastructure turns it off as soon as it gets into control, which works quite nicely (and keeps my sanity as that fan at 100% is *loud*)... So if you want to avoid unpleasant sounds, just build with thermal :p -- Sjoerd Simons sjoerd.sim...@collabora.co.uk Collabora Ltd. smime.p7s Description: S/MIME cryptographic signature
Re: [PATCH 2/9] hwmon: dts: Doc: Add DTS doc to explain how to use PWM FAN as a cooling device
Hi Sjoerd,
Thanks for your feedback and sorry for a late reply.
On Thu, 2014-12-18 at 11:13 +0100, Lukasz Majewski wrote:
Several new properties to allow PWM fan working as a cooling device
have been combined into this single commit.
Signed-off-by: Lukasz Majewski l.majew...@samsung.com
---
.../devicetree/bindings/hwmon/pwm-fan.txt | 28
++ 1 file changed, 28 insertions(+)
diff --git a/Documentation/devicetree/bindings/hwmon/pwm-fan.txt
b/Documentation/devicetree/bindings/hwmon/pwm-fan.txt index
610757c..3877810 100644 ---
a/Documentation/devicetree/bindings/hwmon/pwm-fan.txt +++
b/Documentation/devicetree/bindings/hwmon/pwm-fan.txt @@ -3,10
+3,38 @@ Bindings for a fan connected to the PWM lines Required
properties:
- compatible : pwm-fan
- pwms : the PWM that is used to control the PWM fan
+- cooling-pwm-values : PWM duty cycle values relative to
+ cooling-max-pwm-value correspondig to
+ proper cooling states
+- default-pulse-width : Property specifying default pulse
width for FAN
+ at system boot (zero to disable FAN on
boot).
+ Allowed range is 0 to 255
The 0..255 range seems somewhat random. Would be nicer to either use
the approach of pwm-backlight (iotw, have the range go from the first
to the last entry of cooling-pwm-values)
I'm OK to change the default-pulse-width to be similar to
default-brightness-level (as it is in
Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt)
or simply have be the duty
lenght in NS as entries instead of the current indirection.
I'd prefer to keep the indirection - as it is utilized in the current
pwm-fan.c driver.
I assumed your cooling-pwm-values are a [0..255] range as well instead
of nanoseconds (would be good to make that more clear)?
Your assumption is correct. cooling-pwm-values are indeed in the
[0..255] range. I will add this information in v2.
Also having more consistent names would be nice.. To take
pwm-backlight as inspiration, cooling-levels and
default-cooling-level would make it more clear the second property
picks a default setting from the first one.
I agree.
One thing i do wonder, is having an explicit default setting useful?
Should it not default to maximum cooling unless otherwise configured
by either the thermal framework or sysfs ?
Enabling pan to full RPM was the default behaviour in the current
pwm-fan.c file.
To be honest, there is no need to enable fan to full RPM speed in this
board for following reasons:
1. In Odroid the FAN is optional (stacked on top of a heat sink) - very
often it is just enough to only have the heat sink.
2. Odroid has thermal enabled by default and IMHO it would be more
feasible to allow thermal to control fan from the very beginning.
However, I can also understand if the policy for hwmon implies a rule
to enable by default all fans to full RPM speed.
+Thorough description of the following bindings:
+ cooling-min-state = 0;
+ cooling-max-state = 3;
+ #cooling-cells = 2;
+ thermal-zone {
+ cpu_thermal: cpu-thermal {
+ cooling-maps {
+ map0 {
+trip = cpu_alert1;
+cooling-device = fan0 0 1;
+ };
+ };
+ };
+
+for PWM FAN used as cooling device can be found at:
+./Documentation/devicetree/bindings/thermal/thermal.txt
Example:
pwm-fan {
compatible = pwm-fan;
status = okay;
pwms = pwm 0 1 0;
+ cooling-min-state = 0;
+ cooling-max-state = 3;
+ #cooling-cells = 2;
+ cooling-pwm-values = 0 102 170 255;
+ default-pulse-width = 0;
};
--
Best regards,
Lukasz Majewski
Samsung RD Institute Poland (SRPOL) | Linux Platform Group
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/9] hwmon: dts: Doc: Add DTS doc to explain how to use PWM FAN as a cooling device
On Fri, Dec 19, 2014 at 04:32:24PM +0100, Lukasz Majewski wrote: Hi Sjoerd, Thanks for your feedback and sorry for a late reply. On Thu, 2014-12-18 at 11:13 +0100, Lukasz Majewski wrote: Several new properties to allow PWM fan working as a cooling device have been combined into this single commit. Signed-off-by: Lukasz Majewski l.majew...@samsung.com --- .../devicetree/bindings/hwmon/pwm-fan.txt | 28 ++ 1 file changed, 28 insertions(+) diff --git a/Documentation/devicetree/bindings/hwmon/pwm-fan.txt b/Documentation/devicetree/bindings/hwmon/pwm-fan.txt index 610757c..3877810 100644 --- a/Documentation/devicetree/bindings/hwmon/pwm-fan.txt +++ b/Documentation/devicetree/bindings/hwmon/pwm-fan.txt @@ -3,10 +3,38 @@ Bindings for a fan connected to the PWM lines Required properties: - compatible : pwm-fan - pwms : the PWM that is used to control the PWM fan +- cooling-pwm-values : PWM duty cycle values relative to + cooling-max-pwm-value correspondig to + proper cooling states +- default-pulse-width : Property specifying default pulse width for FAN + at system boot (zero to disable FAN on boot). + Allowed range is 0 to 255 The 0..255 range seems somewhat random. Would be nicer to either use the approach of pwm-backlight (iotw, have the range go from the first to the last entry of cooling-pwm-values) I'm OK to change the default-pulse-width to be similar to default-brightness-level (as it is in Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt) or simply have be the duty lenght in NS as entries instead of the current indirection. I'd prefer to keep the indirection - as it is utilized in the current pwm-fan.c driver. FWIW, devicetree information is supposed to be implementation independent. So this is a poor argument. Enabling pan to full RPM was the default behaviour in the current pwm-fan.c file. To be honest, there is no need to enable fan to full RPM speed in this board for following reasons: 1. In Odroid the FAN is optional (stacked on top of a heat sink) - very often it is just enough to only have the heat sink. 2. Odroid has thermal enabled by default and IMHO it would be more feasible to allow thermal to control fan from the very beginning. However, I can also understand if the policy for hwmon implies a rule to enable by default all fans to full RPM speed. Why and how does that all suggest that the current default behavior should be changed ? Guenter -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/9] hwmon: dts: Doc: Add DTS doc to explain how to use PWM FAN as a cooling device
Hi Guenter, On Fri, Dec 19, 2014 at 04:32:24PM +0100, Lukasz Majewski wrote: Hi Sjoerd, Thanks for your feedback and sorry for a late reply. On Thu, 2014-12-18 at 11:13 +0100, Lukasz Majewski wrote: Several new properties to allow PWM fan working as a cooling device have been combined into this single commit. Signed-off-by: Lukasz Majewski l.majew...@samsung.com --- .../devicetree/bindings/hwmon/pwm-fan.txt | 28 ++ 1 file changed, 28 insertions(+) diff --git a/Documentation/devicetree/bindings/hwmon/pwm-fan.txt b/Documentation/devicetree/bindings/hwmon/pwm-fan.txt index 610757c..3877810 100644 --- a/Documentation/devicetree/bindings/hwmon/pwm-fan.txt +++ b/Documentation/devicetree/bindings/hwmon/pwm-fan.txt @@ -3,10 +3,38 @@ Bindings for a fan connected to the PWM lines Required properties: - compatible : pwm-fan - pwms : the PWM that is used to control the PWM fan +- cooling-pwm-values : PWM duty cycle values relative to + cooling-max-pwm-value correspondig to + proper cooling states +- default-pulse-width : Property specifying default pulse width for FAN + at system boot (zero to disable FAN on boot). + Allowed range is 0 to 255 The 0..255 range seems somewhat random. Would be nicer to either use the approach of pwm-backlight (iotw, have the range go from the first to the last entry of cooling-pwm-values) I'm OK to change the default-pulse-width to be similar to default-brightness-level (as it is in Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt) or simply have be the duty lenght in NS as entries instead of the current indirection. I'd prefer to keep the indirection - as it is utilized in the current pwm-fan.c driver. FWIW, devicetree information is supposed to be implementation independent. So this is a poor argument. Many other pwm drivers use the indirection - e.g. mentioned pwm-backlight. Enabling pan to full RPM was the default behaviour in the current pwm-fan.c file. To be honest, there is no need to enable fan to full RPM speed in this board for following reasons: 1. In Odroid the FAN is optional (stacked on top of a heat sink) - very often it is just enough to only have the heat sink. 2. Odroid has thermal enabled by default and IMHO it would be more feasible to allow thermal to control fan from the very beginning. However, I can also understand if the policy for hwmon implies a rule to enable by default all fans to full RPM speed. Why and how does that all suggest that the current default behavior should be changed ? I wanted to avoid the unpleasant sound for full speed fan when thermal is not enabled by default. But as I said, I fully understand the policy and I would be happy to comply with it as thermal should reduce the fan speed anyway at boot time. Guenter -- Best regards, Lukasz Majewski Samsung RD Institute Poland (SRPOL) | Linux Platform Group -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/9] hwmon: dts: Doc: Add DTS doc to explain how to use PWM FAN as a cooling device
Hi Guenter,
On 12/18/2014 02:13 AM, Lukasz Majewski wrote:
Several new properties to allow PWM fan working as a cooling device
have been combined into this single commit.
Signed-off-by: Lukasz Majewski l.majew...@samsung.com
---
.../devicetree/bindings/hwmon/pwm-fan.txt | 28
++ 1 file changed, 28 insertions(+)
diff --git a/Documentation/devicetree/bindings/hwmon/pwm-fan.txt
b/Documentation/devicetree/bindings/hwmon/pwm-fan.txt index
610757c..3877810 100644 ---
a/Documentation/devicetree/bindings/hwmon/pwm-fan.txt +++
b/Documentation/devicetree/bindings/hwmon/pwm-fan.txt @@ -3,10
+3,38 @@ Bindings for a fan connected to the PWM lines Required
properties:
- compatible : pwm-fan
- pwms: the PWM that is used to control the PWM
fan +- cooling-pwm-values : PWM duty cycle values relative to
+ cooling-max-pwm-value correspondig to
+ proper cooling states
I don't understand what you mean with relative to. Please elaborate.
Do you mean associated with ?
I meant that cooling-pwm-values is no greater than
cooling-max-pwm-value (which was present in some earlier version of
this patch and had value of 255).
This description is wrong and will be reworded.
Where is cooling-max-pwm-value defined,
It was present in some early version of this patch.
and how is this all expected
to relate to the maximum duty cycle value provided by the pwm device ?
+- default-pulse-width : Property specifying default pulse
width for FAN
+ at system boot (zero to disable FAN on
boot).
+ Allowed range is 0 to 255
You'll need dt maintainer approval for the new properties.
I'm wondering how this patch series will get merged. It touches three
different subsystems (thermal, hwmon and device tree for Exynos).
For me it would be best to use thermal tree (hence Odroid U3 fan is
supposed to work as a cooling device) with ACKs from other subsystems
maintainers.
One thing I wonder about though is why you use default-pulse-width
and not default-pwm. Seems to be arbitrary. I don't see
pulse-width used anywhere in the upstream kernel.
Believe or not I've also considered the default-pwm name.
I am somewhat concerned that you define the new properties as
mandatory. That means existing configurations will fail, which does
not seem to be a good idea. It would be more appropriate to not
configure the thermal device if the new properties are not provided.
Very good point. I will do that for v2.
The newly introduced semantics also conflicts with the current
semantics, which sets the initial duty cycle initially to the maximum
allowed duty cycle as reported by the pwm device itself.
Ok. I will stick to above policy and then the default-pulse-width
property can be removed.
Guenter
+
+Thorough description of the following bindings:
+ cooling-min-state = 0;
+ cooling-max-state = 3;
+ #cooling-cells = 2;
+ thermal-zone {
+ cpu_thermal: cpu-thermal {
+ cooling-maps {
+ map0 {
+trip = cpu_alert1;
+cooling-device = fan0 0 1;
+ };
+ };
+ };
+
+for PWM FAN used as cooling device can be found at:
+./Documentation/devicetree/bindings/thermal/thermal.txt
Example:
pwm-fan {
compatible = pwm-fan;
status = okay;
pwms = pwm 0 1 0;
+ cooling-min-state = 0;
+ cooling-max-state = 3;
+ #cooling-cells = 2;
+ cooling-pwm-values = 0 102 170 255;
+ default-pulse-width = 0;
};
--
Best regards,
Lukasz Majewski
Samsung RD Institute Poland (SRPOL) | Linux Platform Group
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/9] hwmon: dts: Doc: Add DTS doc to explain how to use PWM FAN as a cooling device
Several new properties to allow PWM fan working as a cooling device have been
combined into this single commit.
Signed-off-by: Lukasz Majewski l.majew...@samsung.com
---
.../devicetree/bindings/hwmon/pwm-fan.txt | 28 ++
1 file changed, 28 insertions(+)
diff --git a/Documentation/devicetree/bindings/hwmon/pwm-fan.txt
b/Documentation/devicetree/bindings/hwmon/pwm-fan.txt
index 610757c..3877810 100644
--- a/Documentation/devicetree/bindings/hwmon/pwm-fan.txt
+++ b/Documentation/devicetree/bindings/hwmon/pwm-fan.txt
@@ -3,10 +3,38 @@ Bindings for a fan connected to the PWM lines
Required properties:
- compatible : pwm-fan
- pwms : the PWM that is used to control the PWM fan
+- cooling-pwm-values : PWM duty cycle values relative to
+ cooling-max-pwm-value correspondig to
+ proper cooling states
+- default-pulse-width : Property specifying default pulse width for FAN
+ at system boot (zero to disable FAN on boot).
+ Allowed range is 0 to 255
+
+Thorough description of the following bindings:
+ cooling-min-state = 0;
+ cooling-max-state = 3;
+ #cooling-cells = 2;
+ thermal-zone {
+ cpu_thermal: cpu-thermal {
+ cooling-maps {
+ map0 {
+trip = cpu_alert1;
+cooling-device = fan0 0 1;
+ };
+ };
+ };
+
+for PWM FAN used as cooling device can be found at:
+./Documentation/devicetree/bindings/thermal/thermal.txt
Example:
pwm-fan {
compatible = pwm-fan;
status = okay;
pwms = pwm 0 1 0;
+ cooling-min-state = 0;
+ cooling-max-state = 3;
+ #cooling-cells = 2;
+ cooling-pwm-values = 0 102 170 255;
+ default-pulse-width = 0;
};
--
2.0.0.rc2
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/9] hwmon: dts: Doc: Add DTS doc to explain how to use PWM FAN as a cooling device
On Thu, 2014-12-18 at 11:13 +0100, Lukasz Majewski wrote:
Several new properties to allow PWM fan working as a cooling device have been
combined into this single commit.
Signed-off-by: Lukasz Majewski l.majew...@samsung.com
---
.../devicetree/bindings/hwmon/pwm-fan.txt | 28
++
1 file changed, 28 insertions(+)
diff --git a/Documentation/devicetree/bindings/hwmon/pwm-fan.txt
b/Documentation/devicetree/bindings/hwmon/pwm-fan.txt
index 610757c..3877810 100644
--- a/Documentation/devicetree/bindings/hwmon/pwm-fan.txt
+++ b/Documentation/devicetree/bindings/hwmon/pwm-fan.txt
@@ -3,10 +3,38 @@ Bindings for a fan connected to the PWM lines
Required properties:
- compatible : pwm-fan
- pwms : the PWM that is used to control the PWM fan
+- cooling-pwm-values : PWM duty cycle values relative to
+ cooling-max-pwm-value correspondig to
+ proper cooling states
+- default-pulse-width : Property specifying default pulse width for FAN
+ at system boot (zero to disable FAN on boot).
+ Allowed range is 0 to 255
The 0..255 range seems somewhat random. Would be nicer to either use the
approach of pwm-backlight (iotw, have the range go from the first to the
last entry of cooling-pwm-values) or simply have be the duty lenght in
NS as entries instead of the current indirection.
I assumed your cooling-pwm-values are a [0..255] range as well instead
of nanoseconds (would be good to make that more clear)?
Also having more consistent names would be nice.. To take pwm-backlight
as inspiration, cooling-levels and default-cooling-level would make it
more clear the second property picks a default setting from the first
one.
One thing i do wonder, is having an explicit default setting useful?
Should it not default to maximum cooling unless otherwise configured by
either the thermal framework or sysfs ?
+Thorough description of the following bindings:
+ cooling-min-state = 0;
+ cooling-max-state = 3;
+ #cooling-cells = 2;
+ thermal-zone {
+ cpu_thermal: cpu-thermal {
+ cooling-maps {
+ map0 {
+ trip = cpu_alert1;
+ cooling-device = fan0 0 1;
+ };
+ };
+ };
+
+for PWM FAN used as cooling device can be found at:
+./Documentation/devicetree/bindings/thermal/thermal.txt
Example:
pwm-fan {
compatible = pwm-fan;
status = okay;
pwms = pwm 0 1 0;
+ cooling-min-state = 0;
+ cooling-max-state = 3;
+ #cooling-cells = 2;
+ cooling-pwm-values = 0 102 170 255;
+ default-pulse-width = 0;
};
smime.p7s
Description: S/MIME cryptographic signature
Re: [PATCH 2/9] hwmon: dts: Doc: Add DTS doc to explain how to use PWM FAN as a cooling device
On 12/18/2014 02:13 AM, Lukasz Majewski wrote:
Several new properties to allow PWM fan working as a cooling device have been
combined into this single commit.
Signed-off-by: Lukasz Majewski l.majew...@samsung.com
---
.../devicetree/bindings/hwmon/pwm-fan.txt | 28 ++
1 file changed, 28 insertions(+)
diff --git a/Documentation/devicetree/bindings/hwmon/pwm-fan.txt
b/Documentation/devicetree/bindings/hwmon/pwm-fan.txt
index 610757c..3877810 100644
--- a/Documentation/devicetree/bindings/hwmon/pwm-fan.txt
+++ b/Documentation/devicetree/bindings/hwmon/pwm-fan.txt
@@ -3,10 +3,38 @@ Bindings for a fan connected to the PWM lines
Required properties:
- compatible : pwm-fan
- pwms: the PWM that is used to control the PWM fan
+- cooling-pwm-values : PWM duty cycle values relative to
+ cooling-max-pwm-value correspondig to
+ proper cooling states
I don't understand what you mean with relative to. Please elaborate.
Do you mean associated with ?
Where is cooling-max-pwm-value defined, and how is this all expected
to relate to the maximum duty cycle value provided by the pwm device ?
+- default-pulse-width : Property specifying default pulse width for FAN
+ at system boot (zero to disable FAN on boot).
+ Allowed range is 0 to 255
You'll need dt maintainer approval for the new properties.
One thing I wonder about though is why you use default-pulse-width
and not default-pwm. Seems to be arbitrary. I don't see pulse-width
used anywhere in the upstream kernel.
I am somewhat concerned that you define the new properties as mandatory.
That means existing configurations will fail, which does not seem to be
a good idea. It would be more appropriate to not configure the thermal device
if the new properties are not provided.
The newly introduced semantics also conflicts with the current semantics,
which sets the initial duty cycle initially to the maximum allowed duty cycle
as reported by the pwm device itself.
Guenter
+
+Thorough description of the following bindings:
+ cooling-min-state = 0;
+ cooling-max-state = 3;
+ #cooling-cells = 2;
+ thermal-zone {
+ cpu_thermal: cpu-thermal {
+ cooling-maps {
+ map0 {
+trip = cpu_alert1;
+cooling-device = fan0 0 1;
+ };
+ };
+ };
+
+for PWM FAN used as cooling device can be found at:
+./Documentation/devicetree/bindings/thermal/thermal.txt
Example:
pwm-fan {
compatible = pwm-fan;
status = okay;
pwms = pwm 0 1 0;
+ cooling-min-state = 0;
+ cooling-max-state = 3;
+ #cooling-cells = 2;
+ cooling-pwm-values = 0 102 170 255;
+ default-pulse-width = 0;
};
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
