Hi Jean,
On Wed, Oct 06, 2010 at 12:19:31PM -0400, Jean Delvare wrote:
> Hi Guenter,
>
> Sorry for being a little slow, but...
>
> On Wed, 15 Sep 2010 18:52:30 -0700, Guenter Roeck wrote:
> > Signed-off-by: Guenter Roeck <guenter.roeck@xxxxxxxxxxxx>
> > ---
> > Documentation/hwmon/lm90 | 17 +++
> > drivers/hwmon/Kconfig | 4 +-
> > drivers/hwmon/lm90.c | 250 ++++++++++++++++++++++++++++++++++++++++++----
> > 3 files changed, 247 insertions(+), 24 deletions(-)
> >
> > (...)
> > @@ -154,6 +160,8 @@ enum chips { lm90, adm1032, lm99, lm86, max6657, max6659, adt7461, max6680,
> > #define LM90_HAVE_LOCAL_EXT (1 << 2) /* extended local temperature */
> > #define LM90_HAVE_REM_LIMIT_EXT (1 << 3) /* extended remote limit */
> > #define LM90_HAVE_EMERGENCY (1 << 4) /* 3rd upper (emergency) limit */
> > +#define LM90_HAVE_EMERGENCY_ALARM (1 << 5)/* emergency alarm */
> > +#define LM90_HAVE_TEMP3 (1 << 6) /* 3rd temperature sensor */
> >
> > /*
> > * Functions declaration
> > (...)
> > @@ -1135,7 +1321,27 @@ static struct lm90_data *lm90_update_device(struct device *dev)
> > lm90_read_reg(client, MAX6659_REG_R_REMOTE_EMERG,
> > &data->temp8[5]);
> > }
> > - lm90_read_reg(client, LM90_REG_R_STATUS, &data->alarms);
> > + lm90_read_reg(client, LM90_REG_R_STATUS, &alarms);
> > + data->alarms = alarms; /* save as 16 bit value */
> > +
> > + if (data->kind == max6696) {
>
> Wouldn't this be better written:
>
> if (data->flags & LM90_HAVE_TEMP3) {
>
> > + lm90_select_remote_channel(client, data, 1);
> > + lm90_read_reg(client, LM90_REG_R_REMOTE_CRIT,
> > + &data->temp8[6]);
> > + lm90_read_reg(client, MAX6659_REG_R_REMOTE_EMERG,
> > + &data->temp8[7]);
> > + lm90_read16(client, LM90_REG_R_REMOTE_TEMPH,
> > + LM90_REG_R_REMOTE_TEMPL, &data->temp11[5]);
> > + if (!lm90_read_reg(client, LM90_REG_R_REMOTE_LOWH, &h))
> > + data->temp11[6] = h << 8;
> > + if (!lm90_read_reg(client, LM90_REG_R_REMOTE_HIGHH, &h))
> > + data->temp11[7] = h << 8;
> > + lm90_select_remote_channel(client, data, 0);
> > +
>
> and:
>
> if (data->flags & (LM90_HAVE_TEMP3 | LM90_HAVE_EMERGENCY_ALARM))
>
Possibly. I didn't do it on purpose, though, since it is not clear yet if the registers
used to access those sensors on future chips will be the same or completely different.
I figured it is better to play safe and implement the actual register accesses
by checking ->kind instead of using the capability flags.
Later on it might end up as
if (data->flags & LM90_HAVE_TEMP3) {
switch (data->kind) {
case max6696:
...
break;
case newchip:
...
break;
}
}
Guenter
_______________________________________________
lm-sensors mailing list
lm-sensors@xxxxxxxxxxxxxx
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
