linux-kernel - Re: [PATCH/RFC v2 4/4] hwmon: sysfs API updates

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: <20100705091857.77ce0077@hyperion.delvare>
Date:	Mon, 5 Jul 2010 09:18:57 +0200
From:	Jean Delvare <khali@...ux-fr.org>
To:	Guenter Roeck <guenter.roeck@...csson.com>
Cc:	Jonathan Cameron <kernel@...23.retrosnub.co.uk>,
	Randy Dunlap <rdunlap@...otime.net>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"Ira W. Snyder" <iws@...o.caltech.edu>,
	"Darrick J. Wong" <djwong@...ibm.com>,
	"Ben Dooks (embedded platforms)" <ben-linux@...ff.org>,
	Hans de Goede <hdegoede@...hat.com>,
	Mark Brown <broonie@...nsource.wolfsonmicro.com>,
	Samuel Ortiz <sameo@...ux.intel.com>,
	Crane Cai <crane.cai@....com>,
	Linus Walleij <linus.walleij@...ricsson.com>,
	Ralf Baechle <ralf@...ux-mips.org>, lm-sensors@...sensors.org,
	linux-kernel@...r.kernel.org, linux-i2c@...r.kernel.org,
	Guenter Roeck <guenter.roeck@...csson.com>
Subject: Re: [PATCH/RFC v2 4/4] hwmon: sysfs API updates

Hi Guenter,

On Sun, 4 Jul 2010 21:10:18 -0700, Guenter Roeck wrote:
> Signed-off-by: Guenter Roeck <guenter.roeck@...csson.com>
> ---
>  Documentation/hwmon/sysfs-interface |   37 +++++++++++++++++++++++++++++-----
>  1 files changed, 31 insertions(+), 6 deletions(-)

As usual, I don't have the time to review the code, but I'd like to at
least comment on the sysfs interface changes:

> 
> diff --git a/Documentation/hwmon/sysfs-interface b/Documentation/hwmon/sysfs-interface
> index d4e2917..2dcec0f 100644
> --- a/Documentation/hwmon/sysfs-interface
> +++ b/Documentation/hwmon/sysfs-interface
> @@ -421,11 +421,12 @@ power[1-*]_accuracy		Accuracy of the power meter.
>  				Unit: Percent
>  				RO
>  
> -power[1-*]_alarm		1 if the system is drawing more power than the
> -				cap allows; 0 otherwise.  A poll notification is
> -				sent to this file when the power use exceeds the
> -				cap.  This file only appears if the cap is known
> -				to be enforced by hardware.
> +power[1-*]_alarm		1 if the system is drawing more power than cap
> +				or max allows; 0 otherwise.  A poll notification
> +				is sent to this file when the power use exceeds
> +				the cap or max limit. If only cap is supported,
> +				this file only appears if the cap is known to be
> +				enforced by hardware.
>  				RO
>  
>  power[1-*]_cap			If power use rises above this limit, the
> @@ -450,6 +451,18 @@ power[1-*]_cap_min		Minimum cap that can be set.
>  				Unit: microWatt
>  				RO
>  
> +power[1-*]_max			Maximum power.
> +				Unit: microWatt
> +				RW
> +
> +power[1-*]_crit			Critical maximum power.
> +				If power rises to or above this limit, the
> +				system will take drastic action to reduce power
> +				consumption, such as a system shutdown. At the
> +				very least, a power fault will be generated.
> +				Unit: microWatt
> +				RO

Why RO and not RW as every other limit file?

> +
>  **********
>  * Energy *
>  **********
> @@ -471,8 +484,14 @@ limit-related alarms, not both. The driver should just reflect the hardware
>  implementation.
>  
>  in[0-*]_alarm
> +in[0-*]_crit_alarm
> +curr[1-*]_alarm
> +curr[1-*]_crit_alarm
> +power[1-*]_alarm
> +power[1-*]_crit_alarm
>  fan[1-*]_alarm
>  temp[1-*]_alarm
> +temp[1-*]_crit_alarm
>  		Channel alarm
>  		0: no alarm
>  		1: alarm

The limit-specific alarms (*_crit_alarm) go in the second section,
below. And as a matter of fact, you've already added some of them
there...

> @@ -482,10 +501,17 @@ OR
>  
>  in[0-*]_min_alarm
>  in[0-*]_max_alarm
> +in[0-*]_lcrit_alarm
> +in[0-*]_crit_alarm
> +curr[1-*]_lcrit_alarm
> +curr[1-*]_crit_alarm

No _min and _max alarm for curr?

> +power[1-*]_min_alarm
> +power[1-*]_max_alarm
>  fan[1-*]_min_alarm
>  fan[1-*]_max_alarm
>  temp[1-*]_min_alarm
>  temp[1-*]_max_alarm
> +temp[1-*]_lcrit_alarm
>  temp[1-*]_crit_alarm
>  		Limit alarm
>  		0: no alarm
> @@ -497,7 +523,6 @@ to notify open diodes, unconnected fans etc. where the hardware
>  supports it. When this boolean has value 1, the measurement for that
>  channel should not be trusted.
>  
> -in[0-*]_fault

I've removed it already in a separate patch, so your patch won't apply
if you try to remove it again.

>  fan[1-*]_fault
>  temp[1-*]_fault
>  		Input fault condition

In general, I'm happy with the proposed changes.

-- 
Jean Delvare
--
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/