Hi, On 05/04/2011 05:42 PM, Jean Delvare wrote:
Hi Hans, On Thu, 21 Apr 2011 15:35:19 +0200, Hans de Goede wrote:The sch5627 needs to be explicitly told to start an adc conversion for Vbat, once in a while. Without this Vbat may read 0, and will never get updated. Signed-off-By: Hans de Goede<hdegoede@xxxxxxxxxx> --- drivers/hwmon/sch5627.c | 17 ++++++++++++++++- 1 files changed, 16 insertions(+), 1 deletions(-) diff --git a/drivers/hwmon/sch5627.c b/drivers/hwmon/sch5627.c index 09a47bf..e23ea34 100644 --- a/drivers/hwmon/sch5627.c +++ b/drivers/hwmon/sch5627.c @@ -94,6 +94,7 @@ static const char * const SCH5627_IN_LABELS[SCH5627_NO_IN] = { struct sch5627_data { unsigned short addr; struct device *hwmon_dev; + u8 control; u8 temp_max[SCH5627_NO_TEMPS]; u8 temp_crit[SCH5627_NO_TEMPS]; u16 fan_min[SCH5627_NO_FANS]; @@ -101,6 +102,7 @@ struct sch5627_data { struct mutex update_lock; char valid; /* !=0 if following fields are valid */ unsigned long last_updated; /* In jiffies */ + unsigned long last_battery; /* In jiffies */ u16 temp[SCH5627_NO_TEMPS]; u16 fan[SCH5627_NO_FANS]; u16 in[SCH5627_NO_IN]; @@ -327,6 +329,14 @@ static struct sch5627_data *sch5627_update_device(struct device *dev) } data->last_updated = jiffies; + } + + /* Trigger a Vbat voltage measurement every minute */I would even have gone for longer than this, at least 5 minutes if not even 1 hour. AFAIK monitoring drains the battery's power, and the battery isn't used during run-time so monitoring it in real-time is not critical.
Ok, will make it 5 minutes in the next revision.
+ if (time_after(jiffies, data->last_battery + 60 * HZ) || + !data->valid) { + sch5627_write_virtual_reg(data, SCH5627_REG_CTRL, + data->control | 0x10); + data->last_battery = jiffies; data->valid = 1; }This is confusing. What is data->valid supposed to represent now? I don't think you should mess up with data->valid here. You already enabled one sampling of Vbat during probe, so you should have set data->last_battery when you did, and only use data->last_battery after that. Furthermore, wouldn't it make more sense to do that _before_ reading the samples from the chip? With some luck it may let you get a fresh reading for Vbat, instead of possibly hours old one. I understand there is no guarantee, but I see no reason to not at least take our chance.
Ah, I should have read better before saying I had a question about this in my previous mail. I'll take over your suggestions and do a new version of this patch set tomorrow. Regards, Hans _______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
