linux-kernel - Re: [PATCH 2/9] hwmon: dts: Doc: Add DTS doc to explain how to use PWM FAN as a cooling device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5492E46C.3080100@roeck-us.net>
Date:	Thu, 18 Dec 2014 06:27:56 -0800
From:	Guenter Roeck <linux@...ck-us.net>
To:	Lukasz Majewski <l.majewski@...sung.com>,
	Eduardo Valentin <edubezval@...il.com>,
	Kamil Debski <k.debski@...sung.com>,
	Jean Delvare <jdelvare@...e.de>
CC:	lm-sensors@...sensors.org,
	Linux PM list <linux-pm@...r.kernel.org>,
	"linux-samsung-soc@...r.kernel.org" 
	<linux-samsung-soc@...r.kernel.org>, devicetree@...r.kernel.org,
	Lukasz Majewski <l.majewski@...ess.pl>,
	Kukjin Kim <kgene.kim@...sung.com>,
	linux-kernel@...r.kernel.org,
	Sylwester Nawrocki <s.nawrocki@...sung.com>
Subject: 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.majewski@...sung.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 10000 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-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/