lkml.org 
[lkml]   [2010]   [Jul]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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@ericsson.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


\
 
 \ /
  Last update: 2010-07-05 09:21    [W:0.096 / U:0.332 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site