On Tue, Jan 24, 2017 at 04:02:20PM +0100, Andrea Merello wrote: > This patch adds a HWMON driver for ST Microelectronics STTS751 > temperature sensors. > > It does support manual-triggered conversions as well as automatic > conversions. The latter is used when the "event" or "therm" function > is present (declaring the physical wire is attached in the DT). > > Thanks-to: LABBE Corentin [for suggestions] > Signed-off-by: Andrea Merello <andrea.merello@xxxxxxxxx> > Cc: LABBE Corentin <clabbe.montjoie@xxxxxxxxx> > Cc: Guenter Roeck <linux@xxxxxxxxxxxx> > Cc: Jean Delvare <jdelvare@xxxxxxxx> > --- No change log, meaning we have to review the entire driver again or look up older versions and comments ourselves. This will take a while. For now, just a few quick commnents. For the future, please consider providing a changelog. > drivers/hwmon/Kconfig | 10 + > drivers/hwmon/Makefile | 1 + > drivers/hwmon/stts751.c | 809 ++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 820 insertions(+) > create mode 100644 drivers/hwmon/stts751.c > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig > index d1f82f2..8fdd241 100644 > --- a/drivers/hwmon/Kconfig > +++ b/drivers/hwmon/Kconfig > @@ -1448,6 +1448,16 @@ config SENSORS_SCH5636 > This driver can also be built as a module. If so, the module > will be called sch5636. > > +config SENSORS_STTS751 > + tristate "ST Microelectronics STTS751" > + depends on I2C > + help > + If you say yes here you get support for STTS751 > + temperature sensor chips. > + > + This driver can also be built as a module. If so, the module > + will be called stts751. > + > config SENSORS_SMM665 > tristate "Summit Microelectronics SMM665" > depends on I2C > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile > index 7476b22..1114130 100644 > --- a/drivers/hwmon/Makefile > +++ b/drivers/hwmon/Makefile > @@ -147,6 +147,7 @@ obj-$(CONFIG_SENSORS_SMM665) += smm665.o > obj-$(CONFIG_SENSORS_SMSC47B397)+= smsc47b397.o > obj-$(CONFIG_SENSORS_SMSC47M1) += smsc47m1.o > obj-$(CONFIG_SENSORS_SMSC47M192)+= smsc47m192.o > +obj-$(CONFIG_SENSORS_STTS751) += stts751.o > obj-$(CONFIG_SENSORS_AMC6821) += amc6821.o > obj-$(CONFIG_SENSORS_TC74) += tc74.o > obj-$(CONFIG_SENSORS_THMC50) += thmc50.o > diff --git a/drivers/hwmon/stts751.c b/drivers/hwmon/stts751.c > new file mode 100644 > index 0000000..0d4716f > --- /dev/null > +++ b/drivers/hwmon/stts751.c > @@ -0,0 +1,809 @@ > +/* > + * STTS751 sensor driver > + * > + * Copyright (C) 2016-2017 Istituto Italiano di Tecnologia - RBCS - EDL > + * Robotics, Brain and Cognitive Sciences department > + * Electronic Design Laboratory > + * > + * Written by Andrea Merello <andrea.merello@xxxxxxxxx> > + * > + * Based on LM95241 driver and LM90 driver > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#include <linux/bitops.h> > +#include <linux/err.h> > +#include <linux/hwmon.h> > +#include <linux/hwmon-sysfs.h> > +#include <linux/i2c.h> > +#include <linux/init.h> > +#include <linux/interrupt.h> > +#include <linux/jiffies.h> > +#include <linux/module.h> > +#include <linux/mutex.h> > +#include <linux/property.h> > +#include <linux/slab.h> > +#include <linux/sysfs.h> > +#include <linux/util_macros.h> > + > +#define DEVNAME "stts751" > + > +static const unsigned short normal_i2c[] = { > + 0x48, 0x49, 0x38, 0x39, /* STTS751-0 */ > + 0x4A, 0x4B, 0x3A, 0x3B, /* STTS751-1 */ > + I2C_CLIENT_END }; > + > +#define STTS751_REG_TEMP_H 0x00 > +#define STTS751_REG_STATUS 0x01 > +#define STTS751_STATUS_TRIPT BIT(0) > +#define STTS751_STATUS_TRIPL BIT(5) > +#define STTS751_STATUS_TRIPH BIT(6) > +#define STTS751_REG_TEMP_L 0x02 > +#define STTS751_REG_CONF 0x03 > +#define STTS751_CONF_RES_MASK 0x0C > +#define STTS751_CONF_RES_SHIFT 2 > +#define STTS751_CONF_EVENT_DIS BIT(7) > +#define STTS751_CONF_STOP BIT(6) > +#define STTS751_REG_RATE 0x04 > +#define STTS751_REG_HLIM_H 0x05 > +#define STTS751_REG_HLIM_L 0x06 > +#define STTS751_REG_LLIM_H 0x07 > +#define STTS751_REG_LLIM_L 0x08 > +#define STTS751_REG_TLIM 0x20 > +#define STTS751_REG_HYST 0x21 > +#define STTS751_REG_SMBUS_TO 0x22 > + > +#define STTS751_REG_PROD_ID 0xFD > +#define STTS751_REG_MAN_ID 0xFE > +#define STTS751_REG_REV_ID 0xFF > + > +#define STTS751_0_PROD_ID 0x00 > +#define STTS751_1_PROD_ID 0x01 > +#define ST_MAN_ID 0x53 > + > +/* > + * Possible update intervals are (in mS): > + * 16000, 8000, 4000, 2000, 1000, 500, 250, 125, 62.5, 31.25 > + * However we are not going to complicate things too much and we stick to the > + * approx value in mS. > + */ > +static const int stts751_intervals[] = { > + 16000, 8000, 4000, 2000, 1000, 500, 250, 125, 63, 31 > +}; > + > +static const struct i2c_device_id stts751_id[] = { > + { "stts751", 0 }, > + { } > +}; > + > +struct stts751_priv { > + struct device *dev; > + struct i2c_client *client; > + struct mutex access_lock; > + u8 interval; > + int res; > + int event_max, event_min; > + int therm; > + int hyst; > + bool smbus_timeout; > + int temp; > + unsigned long last_update, last_alert_update; > + u8 config; > + bool min_alert, max_alert, therm_trip; > + bool data_valid, alert_valid; > +}; > + > +/* > + * These functions converts temperature from HW format to integer format and > + * vice-vers. They are (mostly) taken from lm90 driver. Unit is in mC. > + */ > +static int stts751_to_deg(s32 hw_val) > +{ > + return hw_val * 125 / 32; > +} > + > +static s32 stts751_to_hw(int val) > +{ > + s32 hw_val; > + > + if (val < 0) > + hw_val = (val - 62) / 125 * 32; > + else > + hw_val = (val + 62) / 125 * 32; > + How about "return DIV_ROUND_CLOSEST(val, 125) * 32;" ? > + return hw_val; > +} > + > +static int stts751_adjust_resolution(struct stts751_priv *priv) > +{ > + u8 res; > + > + switch (priv->interval) { > + case 9: > + /* 10 bits */ > + res = 0; > + break; > + case 8: > + /* 11 bits */ > + res = 1; > + break; > + default: > + /* 12 bits */ > + res = 3; > + break; > + } > + > + if (priv->res == res) > + return 0; > + > + priv->config &= ~STTS751_CONF_RES_MASK; > + priv->config |= res << STTS751_CONF_RES_SHIFT; > + > + return i2c_smbus_write_byte_data(priv->client, > + STTS751_REG_CONF, priv->config); > +} > + > +static int stts751_update_temp(struct stts751_priv *priv) > +{ > + s32 integer1, integer2, frac; > + int ret = 0; > + > + /* > + * There is a trick here, like in the lm90 driver. We have to read two > + * registers to get the sensor temperature, but we have to beware a > + * conversion could occur between the readings. We could use the > + * one-shot conversion register, but we don't want to do this (disables > + * hardware monitoring). So the solution used here is to read the high > + * byte once, then the low byte, then the high byte again. If the new > + * high byte matches the old one, then we have a valid reading. Else we > + * have to read the low byte again, and now we believe we have a correct > + * reading. > + */ > + integer1 = i2c_smbus_read_byte_data(priv->client, STTS751_REG_TEMP_H); > + if (integer1 < 0) { > + dev_dbg(&priv->client->dev, > + "I2C read failed (temp H). ret: %x\n", ret); > + ret = integer1; > + goto exit; > + } > + > + frac = i2c_smbus_read_byte_data(priv->client, STTS751_REG_TEMP_L); > + if (frac < 0) { > + dev_dbg(&priv->client->dev, > + "I2C read failed (temp L). ret: %x\n", ret); > + ret = frac; > + goto exit; > + } > + > + integer2 = i2c_smbus_read_byte_data(priv->client, STTS751_REG_TEMP_H); > + if (integer2 < 0) { > + dev_dbg(&priv->client->dev, > + "I2C 2nd read failed (temp H). ret: %x\n", ret); > + ret = integer2; > + goto exit; > + } > + > + if (integer1 != integer2) { > + frac = i2c_smbus_read_byte_data(priv->client, > + STTS751_REG_TEMP_L); > + if (frac < 0) { > + dev_dbg(&priv->client->dev, > + "I2C 2nd read failed (temp L). ret: %x\n", ret); > + ret = frac; > + goto exit; > + } > + } > + > +exit: > + if (ret) > + return ret; If the code can return immediately, please do so. > + > + priv->temp = stts751_to_deg((integer1 << 8) | frac); > + return ret; > +} > + > +static int stts751_set_temp_reg16(struct stts751_priv *priv, int temp, > + u8 hreg, u8 lreg) > +{ > + s32 hwval; > + int ret; > + > + hwval = stts751_to_hw(temp); > + > + mutex_lock(&priv->access_lock); > + ret = i2c_smbus_write_byte_data(priv->client, hreg, hwval >> 8); > + if (!ret) > + ret = i2c_smbus_write_byte_data(priv->client, lreg, > + hwval & 0xff); > + mutex_unlock(&priv->access_lock); > + > + return ret; > +} > + > +static int stts751_set_temp_reg8(struct stts751_priv *priv, int temp, u8 reg) > +{ > + s32 hwval; > + int ret; > + > + hwval = stts751_to_hw(temp); > + > + mutex_lock(&priv->access_lock); > + ret = i2c_smbus_write_byte_data(priv->client, reg, hwval >> 8); > + mutex_unlock(&priv->access_lock); The lock is in the wrong location. It needs to be in the calling code, since the calling code updates context variables. The same is true for stts751_set_temp_reg16(). > + > + return ret; > +} > + > +static int stts751_read_reg16(struct stts751_priv *priv, int *temp, > + u8 hreg, u8 lreg) > +{ > + int integer, frac; > + > + integer = i2c_smbus_read_byte_data(priv->client, hreg); > + if (integer < 0) > + return integer; > + > + frac = i2c_smbus_read_byte_data(priv->client, lreg); > + if (frac < 0) > + return frac; > + > + *temp = stts751_to_deg((integer << 8) | frac); > + > + return 0; > +} > + > +static int stts751_read_reg8(struct stts751_priv *priv, int *temp, u8 reg) > +{ > + int integer; > + > + integer = i2c_smbus_read_byte_data(priv->client, reg); > + if (integer < 0) > + return integer; > + > + *temp = stts751_to_deg(integer << 8); > + > + return 0; > +} > + > +/* > + * Update alert flags without waiting for cache to expire. We detects alerts > + * immediately for the sake of the alert handler; we still need to deal with > + * caching to workaround the fact that the status register gets cleared when Did we have this before ? The datasheet claims that the status is only cleared if the condition no longer exists. If that is incorrect, please explain here, or it will come up again. > + * reading it. > + */ > +static int stts751_update_alert(struct stts751_priv *priv) > +{ > + int ret; > + bool conv_done; > + int cache_time = > + DIV_ROUND_UP(stts751_intervals[priv->interval] * HZ, 1000); How about using msecs_to_jiffies() ? > + > + /* > + * Add another 10% because if we run faster than the HW conversion > + * rate we will end up in reporting incorrectly alarms. > + */ > + cache_time += cache_time / 10; > + > + ret = i2c_smbus_read_byte_data(priv->client, STTS751_REG_STATUS); > + if (ret < 0) > + return ret; > + > + dev_dbg(priv->dev, "status reg %x\n", ret); > + conv_done = ret & (STTS751_STATUS_TRIPH | STTS751_STATUS_TRIPL); > + /* > + * Reset the cache if the cache time expired, or if we are sure > + * we have valid data from a device conversion, or if we know > + * our cache has been never written. > + * > + * Note that when the cache has been never written the point is > + * to correctly initialize the timestamp, rather than clearing > + * the cache values. > + * > + * Note that updating the cache timestamp when we get an alarm flag > + * is required, otherwise we could incorrectly report alarms to be zero. > + */ > + if (time_after(jiffies, priv->last_alert_update + cache_time) || > + conv_done || !priv->alert_valid) { > + priv->max_alert = false; > + priv->min_alert = false; > + priv->alert_valid = true; > + priv->last_alert_update = jiffies; > + dev_dbg(priv->dev, "invalidating alert cache\n"); > + } > + > + priv->max_alert |= !!(ret & STTS751_STATUS_TRIPH); > + priv->min_alert |= !!(ret & STTS751_STATUS_TRIPL); > + priv->therm_trip = !!(ret & STTS751_STATUS_TRIPT); > + > + dev_dbg(priv->dev, "max_alert: %d, min_alert: %d, therm_trip: %d\n", > + priv->max_alert, priv->min_alert, priv->therm_trip); > + > + return 0; > +} > + > +static void stts751_alert(struct i2c_client *client, > + enum i2c_alert_protocol type, unsigned int data) > +{ > + int ret; > + struct stts751_priv *priv = i2c_get_clientdata(client); > + > + if (type != I2C_PROTOCOL_SMBUS_ALERT) > + return; > + > + dev_dbg(&client->dev, "alert!"); > + > + mutex_lock(&priv->access_lock); > + ret = stts751_update_alert(priv); > + if (ret < 0) { > + /* default to worst case */ > + priv->max_alert = true; > + priv->min_alert = true; > + > + dev_warn(&priv->client->dev, > + "Alert received, but can't communicate to the device. Something bad happening? Triggering all alarms!"); > + } > + > + if (priv->max_alert) { > + dev_notice(&client->dev, "got alert for HIGH temperature"); > + > + /* unblock alert poll */ > + sysfs_notify(&priv->dev->kobj, NULL, "temp1_event_max_alert"); > + kobject_uevent(&priv->dev->kobj, KOBJ_CHANGE); > + } > + > + if (priv->min_alert) { > + dev_notice(&client->dev, "got alert for LOW temperature"); > + > + /* unblock alert poll */ > + sysfs_notify(&priv->dev->kobj, NULL, "temp1_event_min_alert"); > + kobject_uevent(&priv->dev->kobj, KOBJ_CHANGE); This way you can get two uevents, which is really undesirable. Maybe move uevent creation into a separate if(). > + } > + mutex_unlock(&priv->access_lock); > +} > + > +static int stts751_update(struct stts751_priv *priv) > +{ > + int ret = 0; > + int cache_time = stts751_intervals[priv->interval] / 1000; jiffies and the parameter for time_after() are in HZ. Does this assume that HZ=1000 ? msecs_to_jiffies() might be more appropriate. > + > + mutex_lock(&priv->access_lock); > + if (time_after(jiffies, priv->last_update + cache_time) || > + !priv->data_valid) { > + priv->data_valid = true; Not really, only after stts751_update_temp() succeeded. > + priv->last_update = jiffies; Same here. > + > + ret = stts751_update_temp(priv); > + if (ret) > + goto exit; > + > + ret = stts751_update_alert(priv); if (!ret) ret = stts751_update_alert(priv); would avoid the goto. > + } > +exit: > + mutex_unlock(&priv->access_lock); > + > + return ret; > +} > + > +static ssize_t show_max_alarm(struct device *dev, struct device_attribute *attr, > + char *buf) > +{ > + int ret; > + struct stts751_priv *priv = dev_get_drvdata(dev); > + > + ret = stts751_update(priv); > + if (ret < 0) > + return ret; > + > + return snprintf(buf, PAGE_SIZE - 1, "%d\n", priv->max_alert); > +} > + > +static ssize_t show_min_alarm(struct device *dev, struct device_attribute *attr, > + char *buf) > +{ > + int ret; > + struct stts751_priv *priv = dev_get_drvdata(dev); > + > + ret = stts751_update(priv); > + if (ret < 0) > + return ret; > + > + return snprintf(buf, PAGE_SIZE - 1, "%d\n", priv->min_alert); > +} > + > +static ssize_t show_input(struct device *dev, struct device_attribute *attr, > + char *buf) > +{ > + int ret; > + struct stts751_priv *priv = dev_get_drvdata(dev); > + > + ret = stts751_update(priv); > + if (ret < 0) > + return ret; > + > + return snprintf(buf, PAGE_SIZE - 1, "%d\n", priv->temp); > +} > + > +static ssize_t show_therm(struct device *dev, struct device_attribute *attr, > + char *buf) > +{ > + struct stts751_priv *priv = dev_get_drvdata(dev); > + > + return snprintf(buf, PAGE_SIZE - 1, "%d\n", priv->therm); > +} > + > +static int do_set_hyst(struct stts751_priv *priv, int temp) > +{ > + /* HW works in range -64C to +127.937C */ > + temp = clamp_val(temp, -64000, priv->therm); > + priv->hyst = temp; > + dev_dbg(priv->dev, "setting hyst %d", temp); > + temp = priv->therm - temp; > + > + return stts751_set_temp_reg8(priv, temp, STTS751_REG_HYST); > +} > + > +static ssize_t set_therm(struct device *dev, struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + int ret; > + long temp; > + struct stts751_priv *priv = dev_get_drvdata(dev); > + > + if (kstrtol(buf, 10, &temp) < 0) > + return -EINVAL; > + /* HW works in range -64C to +127.937C */ > + temp = clamp_val(temp, -64000, 127937); > + ret = stts751_set_temp_reg8(priv, temp, STTS751_REG_TLIM); > + if (ret) > + return ret; > + > + dev_dbg(dev, "setting therm %ld", temp); > + priv->therm = temp; > + > + /* > + * hysteresis reg is relative to therm, so we need to update > + * it as well. > + */ > + ret = do_set_hyst(priv, priv->hyst); The basic expectation here is that the hysteresis value changes automatically. If the old hysteresis was 8 degrees below the limit, it should still be 8 degrees below the new limit. This code tries to restore the old (absolute) value, which hardly ever makes any sense. > + if (ret) > + return ret; > + > + return count; > +} > + > +static ssize_t show_hyst(struct device *dev, struct device_attribute *attr, > + char *buf) > +{ > + struct stts751_priv *priv = dev_get_drvdata(dev); > + > + return snprintf(buf, PAGE_SIZE - 1, "%d\n", priv->hyst); > +} > + > +static ssize_t set_hyst(struct device *dev, struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + int ret; > + long temp; > + > + struct stts751_priv *priv = dev_get_drvdata(dev); > + > + if (kstrtol(buf, 10, &temp) < 0) > + return -EINVAL; > + ret = do_set_hyst(priv, temp); > + if (ret) > + return ret; > + return count; > +} > + > +static ssize_t show_therm_trip(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + int ret; > + struct stts751_priv *priv = dev_get_drvdata(dev); > + > + ret = stts751_update(priv); > + if (ret < 0) > + return ret; > + > + return snprintf(buf, PAGE_SIZE - 1, "%d\n", priv->therm_trip); > +} > + > +static ssize_t show_max(struct device *dev, struct device_attribute *attr, > + char *buf) > +{ > + struct stts751_priv *priv = dev_get_drvdata(dev); > + > + return snprintf(buf, PAGE_SIZE - 1, "%d\n", priv->event_max); > +} > + > +static ssize_t set_max(struct device *dev, struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + int ret; > + long temp; > + struct stts751_priv *priv = dev_get_drvdata(dev); > + > + if (kstrtol(buf, 10, &temp) < 0) > + return -EINVAL; > + /* HW works in range -64C to +127.937C */ > + temp = clamp_val(temp, priv->event_min, 127937); > + ret = stts751_set_temp_reg16(priv, temp, > + STTS751_REG_HLIM_H, STTS751_REG_HLIM_L); > + if (ret) > + return ret; > + > + dev_dbg(dev, "setting event max %ld", temp); > + priv->event_max = temp; As mentioned above, the writes are all racy. Two writes in parallel may result in inconsistent register values vs. values written into event_max etc. > + return count; > +} > + > +static ssize_t show_min(struct device *dev, struct device_attribute *attr, > + char *buf) > +{ > + struct stts751_priv *priv = dev_get_drvdata(dev); > + > + return snprintf(buf, PAGE_SIZE - 1, "%d\n", priv->event_min); > +} > + > +static ssize_t set_min(struct device *dev, struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + int ret; > + long temp; > + struct stts751_priv *priv = dev_get_drvdata(dev); > + > + if (kstrtol(buf, 10, &temp) < 0) > + return -EINVAL; > + /* HW works in range -64C to +127.937C */ > + temp = clamp_val(temp, -64000, priv->event_max); > + ret = stts751_set_temp_reg16(priv, temp, > + STTS751_REG_LLIM_H, STTS751_REG_LLIM_L); > + if (ret) > + return ret; > + > + dev_dbg(dev, "setting event min %ld", temp); > + > + priv->event_min = temp; > + return count; > +} > + > +static ssize_t show_interval(struct device *dev, struct device_attribute *attr, > + char *buf) > +{ > + struct stts751_priv *priv = dev_get_drvdata(dev); > + > + return snprintf(buf, PAGE_SIZE - 1, "%d\n", > + stts751_intervals[priv->interval]); > +} > + > +static ssize_t set_interval(struct device *dev, struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + unsigned long val; > + int idx; > + int ret = 0; > + struct stts751_priv *priv = dev_get_drvdata(dev); > + > + if (kstrtoul(buf, 10, &val) < 0) > + return -EINVAL; > + > + idx = find_closest_descending(val, stts751_intervals, > + ARRAY_SIZE(stts751_intervals)); > + > + dev_dbg(dev, "setting interval. req:%lu, idx: %d, val: %d", val, idx, > + stts751_intervals[idx]); > + > + if (priv->interval == idx) > + return count; > + > + mutex_lock(&priv->access_lock); > + > + /* > + * In early development stages I've become suspicious about the chip > + * starting to misbehave if I ever set, even briefly, an invalid > + * configuration. While I'm not sure this is really needed, be > + * conservative and set rate/resolution in such an order that avoids > + * passing through an invalid configuration. > + */ > + > + /* speed up: lower the resolution, then modify convrate */ > + if (priv->interval < idx) { > + priv->interval = idx; > + ret = stts751_adjust_resolution(priv); > + if (ret) > + goto exit; > + } > + > + ret = i2c_smbus_write_byte_data(priv->client, STTS751_REG_RATE, idx); > + if (ret) > + goto exit; > + /* slow down: modify convrate, then raise resolution */ > + if (priv->interval != idx) { > + priv->interval = idx; > + ret = stts751_adjust_resolution(priv); > + if (ret) > + goto exit; > + } > +exit: > + mutex_unlock(&priv->access_lock); > + > + return count; > +} > + > +static int stts751_detect(struct i2c_client *new_client, > + struct i2c_board_info *info) > +{ > + struct i2c_adapter *adapter = new_client->adapter; > + const char *name; > + int mfg_id, prod_id, rev_id; > + > + if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) > + return -ENODEV; > + > + mfg_id = i2c_smbus_read_byte_data(new_client, ST_MAN_ID); > + if (mfg_id != ST_MAN_ID) > + return -ENODEV; > + > + prod_id = i2c_smbus_read_byte_data(new_client, STTS751_REG_PROD_ID); > + > + switch (prod_id) { > + case STTS751_0_PROD_ID: > + name = "STTS751-0"; > + break; > + case STTS751_1_PROD_ID: > + name = "STTS751-1"; > + break; > + default: > + return -ENODEV; > + } > + dev_dbg(&new_client->dev, "Chip %s detected!", name); I'll never understand what the "!" in such messages is for ;-). > + > + rev_id = i2c_smbus_read_byte_data(new_client, STTS751_REG_REV_ID); > + if (rev_id < 0) > + return -ENODEV; > + if (rev_id != 0x1) { > + dev_notice(&new_client->dev, > + "Chip revision 0x%x is untested\nPlease report whether it works to andrea.merello@xxxxxxxxx", > + rev_id); > + } > + > + strlcpy(info->type, stts751_id[0].name, I2C_NAME_SIZE); > + return 0; > +} > + > +static int stts751_read_chip_config(struct stts751_priv *priv) > +{ > + int ret; > + int tmp; > + > + ret = i2c_smbus_read_byte_data(priv->client, STTS751_REG_CONF); > + if (ret < 0) > + return ret; > + priv->config = ret; > + priv->res = (ret & STTS751_CONF_RES_MASK) >> STTS751_CONF_RES_SHIFT; > + > + ret = i2c_smbus_read_byte_data(priv->client, STTS751_REG_RATE); > + if (ret < 0) > + return ret; > + priv->interval = ret; > + > + ret = stts751_read_reg16(priv, &priv->event_max, > + STTS751_REG_HLIM_H, STTS751_REG_HLIM_L); > + if (ret) > + return ret; > + > + ret = stts751_read_reg16(priv, &priv->event_min, > + STTS751_REG_LLIM_H, STTS751_REG_LLIM_L); > + if (ret) > + return ret; > + > + ret = stts751_read_reg8(priv, &priv->therm, STTS751_REG_TLIM); > + if (ret) > + return ret; > + > + ret = stts751_read_reg8(priv, &tmp, STTS751_REG_HYST); > + if (ret) > + return ret; > + priv->hyst = priv->therm - tmp; > + > + return ret; > +} > + > +static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_input, NULL, 0); > +static SENSOR_DEVICE_ATTR(temp1_min, S_IWUSR | S_IRUGO, show_min, set_min, 0); > +static SENSOR_DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO, show_max, set_max, 0); > +static SENSOR_DEVICE_ATTR(temp1_min_alarm, S_IRUGO, show_min_alarm, NULL, 0); > +static SENSOR_DEVICE_ATTR(temp1_max_alarm, S_IRUGO, show_max_alarm, NULL, 0); > +static SENSOR_DEVICE_ATTR(temp1_crit, S_IWUSR | S_IRUGO, show_therm, > + set_therm, 0); > +static SENSOR_DEVICE_ATTR(temp1_crit_hyst, S_IWUSR | S_IRUGO, show_hyst, > + set_hyst, 0); > +static SENSOR_DEVICE_ATTR(temp1_crit_alarm, S_IRUGO, show_therm_trip, NULL, 0); > +static SENSOR_DEVICE_ATTR(update_interval, S_IWUSR | S_IRUGO, > + show_interval, set_interval, 0); > + > +static struct attribute *stts751_attrs[] = { > + &sensor_dev_attr_temp1_input.dev_attr.attr, > + &sensor_dev_attr_temp1_min.dev_attr.attr, > + &sensor_dev_attr_temp1_max.dev_attr.attr, > + &sensor_dev_attr_temp1_min_alarm.dev_attr.attr, > + &sensor_dev_attr_temp1_max_alarm.dev_attr.attr, > + &sensor_dev_attr_temp1_crit.dev_attr.attr, > + &sensor_dev_attr_temp1_crit_hyst.dev_attr.attr, > + &sensor_dev_attr_temp1_crit_alarm.dev_attr.attr, > + &sensor_dev_attr_update_interval.dev_attr.attr, > + NULL > +}; > +ATTRIBUTE_GROUPS(stts751); > + > +static int stts751_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + struct stts751_priv *priv; > + int ret; > + bool smbus_to; > + > + priv = devm_kzalloc(&client->dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + priv->client = client; > + i2c_set_clientdata(client, priv); > + mutex_init(&priv->access_lock); > + > + if (device_property_present(&client->dev, > + "smbus-timeout-disable")) { > + smbus_to = !device_property_read_bool(&client->dev, > + "smbus-timeout-disable"); > + > + ret = i2c_smbus_write_byte_data(priv->client, > + STTS751_REG_SMBUS_TO, > + smbus_to ? 0x80 : 0); > + if (ret) > + return ret; > + } > + > + ret = stts751_read_chip_config(priv); > + if (ret) > + return ret; > + > + priv->config &= ~(STTS751_CONF_STOP | STTS751_CONF_EVENT_DIS); > + ret = i2c_smbus_write_byte_data(priv->client, > + STTS751_REG_CONF, priv->config); > + if (ret) > + return ret; > + > + priv->dev = devm_hwmon_device_register_with_groups(&client->dev, > + client->name, priv, > + stts751_groups); > + return PTR_ERR_OR_ZERO(priv->dev); > +} > + > +MODULE_DEVICE_TABLE(i2c, stts751_id); > + > +static struct i2c_driver stts751_driver = { > + .class = I2C_CLASS_HWMON, > + .driver = { > + .name = DEVNAME, > + }, > + .probe = stts751_probe, > + .id_table = stts751_id, > + .detect = stts751_detect, > + .alert = stts751_alert, > + .address_list = normal_i2c, > +}; > + > +module_i2c_driver(stts751_driver); > + > +MODULE_AUTHOR("Andrea Merello <andrea.merello@xxxxxxxxx>"); > +MODULE_DESCRIPTION("STTS751 sensor driver"); > +MODULE_LICENSE("GPL"); > -- > 2.7.4 > -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html