[lm-sensors] [PATCH] hwmon: (pmbus) Fix low limit temperature alarms

[lm-sensors] [PATCH] hwmon: (pmbus) Fix low limit temperature alarms

[Guenter Roeck]

Temperature alarms are detected by checking the alarm bit and comparing
temperature limits against the current temperature. For low limits, this
comparison needs to be reversed (temp < limit instead of temp > limit).
This was not taken into account, resulting in wrong alarms if a temperature
fell below a low limit.

Fix by adding a low limit flag in the limit data structure. When creating the
sensor entry, the order of registers to compare is now reversed for low limits.

Signed-off-by: Guenter Roeck <guenter.roeck@ericsson.com>
---
Candidate for -stable.

 drivers/hwmon/pmbus/pmbus_core.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 1798f52..410aa12 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -978,6 +978,8 @@ static void pmbus_find_max_attr(struct i2c_client *client,
 struct pmbus_limit_attr {
 	u16 reg;		/* Limit register */
 	bool update;		/* True if register needs updates */
+	bool low;		/* True if low limit; for limits with compare
+				   functions only */
 	const char *attr;	/* Attribute name */
 	const char *alarm;	/* Alarm attribute name */
 	u32 sbit;		/* Alarm attribute status bit */
@@ -1029,7 +1031,8 @@ static bool pmbus_add_limit_attrs(struct i2c_client *client,
 				if (attr->compare) {
 					pmbus_add_boolean_cmp(data, name,
 						l->alarm, index,
-						cbase, cindex,
+						l->low ? cindex : cbase,
+						l->low ? cbase : cindex,
 						attr->sbase + page, l->sbit);
 				} else {
 					pmbus_add_boolean_reg(data, name,
@@ -1366,16 +1369,19 @@ static const struct pmbus_sensor_attr power_attributes[] = {
 static const struct pmbus_limit_attr temp_limit_attrs[] = {
 	{
 		.reg = PMBUS_UT_WARN_LIMIT,
+		.low = true,
 		.attr = "min",
 		.alarm = "min_alarm",
 		.sbit = PB_TEMP_UT_WARNING,
 	}, {
 		.reg = PMBUS_UT_FAULT_LIMIT,
+		.low = true,
 		.attr = "lcrit",
 		.alarm = "lcrit_alarm",
 		.sbit = PB_TEMP_UT_FAULT,
 	}, {
 		.reg = PMBUS_OT_WARN_LIMIT,
+		.low = true,
 		.attr = "max",
 		.alarm = "max_alarm",
 		.sbit = PB_TEMP_OT_WARNING,
@@ -1399,11 +1405,13 @@ static const struct pmbus_limit_attr temp_limit_attrs[] = {
 static const struct pmbus_limit_attr temp_limit_attrs23[] = {
 	{
 		.reg = PMBUS_UT_WARN_LIMIT,
+		.low = true,
 		.attr = "min",
 		.alarm = "min_alarm",
 		.sbit = PB_TEMP_UT_WARNING,
 	}, {
 		.reg = PMBUS_UT_FAULT_LIMIT,
+		.low = true,
 		.attr = "lcrit",
 		.alarm = "lcrit_alarm",
 		.sbit = PB_TEMP_UT_FAULT,
-- 
1.7.3.1


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (pmbus) Fix low limit temperature

[Jean Delvare]

Hi Guenter,

On Sat, 10 Sep 2011 13:27:54 -0700, Guenter Roeck wrote:
> Temperature alarms are detected by checking the alarm bit and comparing
> temperature limits against the current temperature. For low limits, this
> comparison needs to be reversed (temp < limit instead of temp > limit).
> This was not taken into account, resulting in wrong alarms if a temperature
> fell below a low limit.
> 
> Fix by adding a low limit flag in the limit data structure. When creating the
> sensor entry, the order of registers to compare is now reversed for low limits.
> 
> Signed-off-by: Guenter Roeck <guenter.roeck@ericsson.com>
> ---
> Candidate for -stable.
> 
>  drivers/hwmon/pmbus/pmbus_core.c |   10 +++++++++-
>  1 files changed, 9 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
> index 1798f52..410aa12 100644
> --- a/drivers/hwmon/pmbus/pmbus_core.c
> +++ b/drivers/hwmon/pmbus/pmbus_core.c
> @@ -978,6 +978,8 @@ static void pmbus_find_max_attr(struct i2c_client *client,
>  struct pmbus_limit_attr {
>  	u16 reg;		/* Limit register */
>  	bool update;		/* True if register needs updates */
> +	bool low;		/* True if low limit; for limits with compare
> +				   functions only */
>  	const char *attr;	/* Attribute name */
>  	const char *alarm;	/* Alarm attribute name */
>  	u32 sbit;		/* Alarm attribute status bit */
> @@ -1029,7 +1031,8 @@ static bool pmbus_add_limit_attrs(struct i2c_client *client,
>  				if (attr->compare) {
>  					pmbus_add_boolean_cmp(data, name,
>  						l->alarm, index,
> -						cbase, cindex,
> +						l->low ? cindex : cbase,
> +						l->low ? cbase : cindex,
>  						attr->sbase + page, l->sbit);
>  				} else {
>  					pmbus_add_boolean_reg(data, name,
> @@ -1366,16 +1369,19 @@ static const struct pmbus_sensor_attr power_attributes[] = {
>  static const struct pmbus_limit_attr temp_limit_attrs[] = {
>  	{
>  		.reg = PMBUS_UT_WARN_LIMIT,
> +		.low = true,
>  		.attr = "min",
>  		.alarm = "min_alarm",
>  		.sbit = PB_TEMP_UT_WARNING,
>  	}, {
>  		.reg = PMBUS_UT_FAULT_LIMIT,
> +		.low = true,
>  		.attr = "lcrit",
>  		.alarm = "lcrit_alarm",
>  		.sbit = PB_TEMP_UT_FAULT,
>  	}, {
>  		.reg = PMBUS_OT_WARN_LIMIT,
> +		.low = true,
>  		.attr = "max",
>  		.alarm = "max_alarm",
>  		.sbit = PB_TEMP_OT_WARNING,

This one doesn't look like a "low lmit", does it?

> @@ -1399,11 +1405,13 @@ static const struct pmbus_limit_attr temp_limit_attrs[] = {
>  static const struct pmbus_limit_attr temp_limit_attrs23[] = {
>  	{
>  		.reg = PMBUS_UT_WARN_LIMIT,
> +		.low = true,
>  		.attr = "min",
>  		.alarm = "min_alarm",
>  		.sbit = PB_TEMP_UT_WARNING,
>  	}, {
>  		.reg = PMBUS_UT_FAULT_LIMIT,
> +		.low = true,
>  		.attr = "lcrit",
>  		.alarm = "lcrit_alarm",
>  		.sbit = PB_TEMP_UT_FAULT,

Looks fine otherwise.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (pmbus) Fix low limit temperature

[Guenter Roeck]

Hi Jean,

On Sun, Sep 11, 2011 at 03:34:19AM -0400, Jean Delvare wrote:
> Hi Guenter,
> 
> On Sat, 10 Sep 2011 13:27:54 -0700, Guenter Roeck wrote:
> > Temperature alarms are detected by checking the alarm bit and comparing
> > temperature limits against the current temperature. For low limits, this
> > comparison needs to be reversed (temp < limit instead of temp > limit).
> > This was not taken into account, resulting in wrong alarms if a temperature
> > fell below a low limit.
> > 
> > Fix by adding a low limit flag in the limit data structure. When creating the
> > sensor entry, the order of registers to compare is now reversed for low limits.
> > 
> > Signed-off-by: Guenter Roeck <guenter.roeck@ericsson.com>
> > ---
> > Candidate for -stable.
> > 
> >  drivers/hwmon/pmbus/pmbus_core.c |   10 +++++++++-
> >  1 files changed, 9 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
> > index 1798f52..410aa12 100644
> > --- a/drivers/hwmon/pmbus/pmbus_core.c
> > +++ b/drivers/hwmon/pmbus/pmbus_core.c
> > @@ -978,6 +978,8 @@ static void pmbus_find_max_attr(struct i2c_client *client,
> >  struct pmbus_limit_attr {
> >  	u16 reg;		/* Limit register */
> >  	bool update;		/* True if register needs updates */
> > +	bool low;		/* True if low limit; for limits with compare
> > +				   functions only */
> >  	const char *attr;	/* Attribute name */
> >  	const char *alarm;	/* Alarm attribute name */
> >  	u32 sbit;		/* Alarm attribute status bit */
> > @@ -1029,7 +1031,8 @@ static bool pmbus_add_limit_attrs(struct i2c_client *client,
> >  				if (attr->compare) {
> >  					pmbus_add_boolean_cmp(data, name,
> >  						l->alarm, index,
> > -						cbase, cindex,
> > +						l->low ? cindex : cbase,
> > +						l->low ? cbase : cindex,
> >  						attr->sbase + page, l->sbit);
> >  				} else {
> >  					pmbus_add_boolean_reg(data, name,
> > @@ -1366,16 +1369,19 @@ static const struct pmbus_sensor_attr power_attributes[] = {
> >  static const struct pmbus_limit_attr temp_limit_attrs[] = {
> >  	{
> >  		.reg = PMBUS_UT_WARN_LIMIT,
> > +		.low = true,
> >  		.attr = "min",
> >  		.alarm = "min_alarm",
> >  		.sbit = PB_TEMP_UT_WARNING,
> >  	}, {
> >  		.reg = PMBUS_UT_FAULT_LIMIT,
> > +		.low = true,
> >  		.attr = "lcrit",
> >  		.alarm = "lcrit_alarm",
> >  		.sbit = PB_TEMP_UT_FAULT,
> >  	}, {
> >  		.reg = PMBUS_OT_WARN_LIMIT,
> > +		.low = true,
> >  		.attr = "max",
> >  		.alarm = "max_alarm",
> >  		.sbit = PB_TEMP_OT_WARNING,
> 
> This one doesn't look like a "low lmit", does it?
> 
Not really. I'll resend.

Thanks,
Guenter

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors