Hi Guenter,

On Sun, 4 Jul 2010 21:10:18 -0700, Guenter Roeck wrote:
> Signed-off-by: Guenter Roeck <[email protected]>
> ---
>  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-i2c" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to