| Message ID | 1480223737-82080-2-git-send-email-john@jmuir.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
On 11/26/2016 09:15 PM, John Muir wrote: > Add support for the TI TMP108 temperature sensor with some device > configuration parameters. > > Signed-off-by: John Muir <john@jmuir.com> > --- > Documentation/devicetree/bindings/hwmon/tmp108.txt | 27 ++ > Documentation/hwmon/tmp108 | 38 ++ > drivers/hwmon/Kconfig | 11 + > drivers/hwmon/Makefile | 1 + > drivers/hwmon/tmp108.c | 429 +++++++++++++++++++++ > 5 files changed, 506 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hwmon/tmp108.txt > create mode 100644 Documentation/hwmon/tmp108 > create mode 100644 drivers/hwmon/tmp108.c > > diff --git a/Documentation/devicetree/bindings/hwmon/tmp108.txt b/Documentation/devicetree/bindings/hwmon/tmp108.txt This should be provided in a separate patch. > new file mode 100644 > index 0000000..210af63 > --- /dev/null > +++ b/Documentation/devicetree/bindings/hwmon/tmp108.txt > @@ -0,0 +1,27 @@ > +TMP108 temperature sensor > +------------------------- > + > +This device supports I2C only. > + > +Requires node properties: > +- compatible : "ti,tmp108" > +- reg : the I2C address of the device. This is 0x48, 0x49, 0x4a, or 0x4b. > + > +Optional node properties: > +- ti,thermostat-mode-comparitor : (boolean) select the comparitor mode for the s/comparitor/comparator/g > + thermostat rather than the default interrupt-mode. > +- ti,alert-active-high : (boolean) make the alert pin active-high instead of > + the default active-low. The driver doesn't support interrupts/alerts. Do those properties really add value ? > +- ti,conversion-rate-cHz : (integer, cHz) select the conversion frequency for > + continuous mode, in centi-Hz: 25, 100 (default), 400, or 1600. > +- ti,hysteresis : (integer, C) select the hysteresis value: 0, 1, 2, or 4 > + (celcius). > + We would nor mally make the conversion rate and hysteresis user configurable, with the update_interval and temp1_{min,max}_hyst attributes. Those are not really system configuration properties. > +Example: > + tmp108@48 { > + compatible = "ti,tmp108"; > + reg = <0x48>; > + ti,alert-active-high; > + ti,hysteresis = <2>; > + ti,conversion-rate-cHz = <25>; > + }; > diff --git a/Documentation/hwmon/tmp108 b/Documentation/hwmon/tmp108 > new file mode 100644 > index 0000000..ef2e9a3 > --- /dev/null > +++ b/Documentation/hwmon/tmp108 > @@ -0,0 +1,38 @@ > +Kernel driver tmp108 > +==================== > + > +Supported chips: > + * Texas Instruments TMP108 > + Prefix: 'tmp108' > + Addresses scanned: none > + Datasheet: http://www.ti.com/product/tmp108 > + > +Author: > + John Muir <john@jmuir.com> > + > +Description > +----------- > + > +The Texas Instruments TMP108 implements one temperature sensor. An alert pin > +can be set when temperatures exceed minimum or maximum values plus or minus a > +hysteresis value. > + > +The sensor is accurate to 0.75C over the range of -25 to +85 C, and to 1.0 > +degree from -40 to +125 C. Resolution of the sensor is 0.0625 degree. The > +operating temperature has a minimum of -55 C and a maximum of +150 C. > +Hysteresis values can be set to 0, 1, 2, or 4C. > + > +The TMP108 has a programmable update rate that can select between 8, 4, 1, and > +0.5 Hz. > + > +By default the TMP108 reads the temperature continuously. To conserve power, > +the TMP108 has a one-shot mode where the device is normally shut-down. When a > +one shot is requested the temperature is read, the result can be retrieved, > +and then the device is shut down automatically. (This driver only supports > +continuous mode.) > + > +The driver provides the common sysfs-interface for temperatures (see > +Documentation/hwmon/sysfs-interface under Temperatures). > + > +See Documentation/devicetree/bindings/hwmon/tmp108.txt for configuration > +properties. > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig > index 45cef3d..4c173de 100644 > --- a/drivers/hwmon/Kconfig > +++ b/drivers/hwmon/Kconfig > @@ -1591,6 +1591,17 @@ config SENSORS_TMP103 > This driver can also be built as a module. If so, the module > will be called tmp103. > > +config SENSORS_TMP108 > + tristate "Texas Instruments TMP108" > + depends on I2C > + select REGMAP_I2C > + help > + If you say yes here you get support for Texas Instruments TMP108 > + sensor chips. > + > + This driver can also be built as a module. If so, the module > + will be called tmp108. > + > config SENSORS_TMP401 > tristate "Texas Instruments TMP401 and compatibles" > depends on I2C > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile > index aecf4ba..51e5256 100644 > --- a/drivers/hwmon/Makefile > +++ b/drivers/hwmon/Makefile > @@ -152,6 +152,7 @@ obj-$(CONFIG_SENSORS_TC74) += tc74.o > obj-$(CONFIG_SENSORS_THMC50) += thmc50.o > obj-$(CONFIG_SENSORS_TMP102) += tmp102.o > obj-$(CONFIG_SENSORS_TMP103) += tmp103.o > +obj-$(CONFIG_SENSORS_TMP108) += tmp108.o > obj-$(CONFIG_SENSORS_TMP401) += tmp401.o > obj-$(CONFIG_SENSORS_TMP421) += tmp421.o > obj-$(CONFIG_SENSORS_TWL4030_MADC)+= twl4030-madc-hwmon.o > diff --git a/drivers/hwmon/tmp108.c b/drivers/hwmon/tmp108.c > new file mode 100644 > index 0000000..da64517 > --- /dev/null > +++ b/drivers/hwmon/tmp108.c > @@ -0,0 +1,429 @@ > +/* Texas Instruments TMP108 SMBus temperature sensor driver > + * > + * Copyright (C) 2016 John Muir <john@jmuir.com> > + * > + * 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/delay.h> > +#include <linux/module.h> > +#include <linux/init.h> > +#include <linux/slab.h> > +#include <linux/i2c.h> > +#include <linux/hwmon.h> > +#include <linux/hwmon-sysfs.h> > +#include <linux/err.h> > +#include <linux/mutex.h> > +#include <linux/device.h> > +#include <linux/jiffies.h> > +#include <linux/regmap.h> > +#include <linux/thermal.h> > +#include <linux/of.h> > + Alphabetic order, please. > +#define DRIVER_NAME "tmp108" > + > +#define TMP108_REG_TEMP 0x00 > +#define TMP108_REG_CONF 0x01 > +#define TMP108_REG_TLOW 0x02 > +#define TMP108_REG_THIGH 0x03 > + > +#define TMP108_TEMP_REG_COUNT 3 > + > +#define TMP108_TEMP_MIN_MC -50000 /* Minimum millicelcius. */ > +#define TMP108_TEMP_MAX_MC 127937 /* Maximum millicelcius. */ > + > +/* Configuration register bits. > + * Note: these bit definitions are byte swapped. > + */ > +#define TMP108_CONF_M0 0x0100 /* Sensor mode. */ > +#define TMP108_CONF_M1 0x0200 > +#define TMP108_CONF_TM 0x0400 /* Thermostat mode. */ > +#define TMP108_CONF_FL 0x0800 /* Watchdog flag - TLOW */ > +#define TMP108_CONF_FH 0x1000 /* Watchdog flag - THIGH */ > +#define TMP108_CONF_CR0 0x2000 /* Conversion rate. */ > +#define TMP108_CONF_CR1 0x4000 > +#define TMP108_CONF_ID 0x8000 > +#define TMP108_CONF_HYS0 0x0010 /* Hysteresis. */ > +#define TMP108_CONF_HYS1 0x0020 > +#define TMP108_CONF_POL 0x0080 /* Polarity of alert. */ > + > +/* Defaults set by the hardware upon reset. */ > +#define TMP108_CONF_DEFAULTS (TMP108_CONF_CR0 | TMP108_CONF_TM |\ > + TMP108_CONF_HYS0 | TMP108_CONF_M1) > +/* These bits are read-only. */ > +#define TMP108_CONF_READ_ONLY (TMP108_CONF_FL | TMP108_CONF_FH |\ > + TMP108_CONF_ID) > + > +#define TMP108_CONF_MODE_MASK (TMP108_CONF_M0|TMP108_CONF_M1) > +#define TMP108_MODE_SHUTDOWN 0x0000 > +#define TMP108_MODE_ONE_SHOT TMP108_CONF_M0 > +#define TMP108_MODE_CONTINUOUS TMP108_CONF_M1 /* Default */ > + > +#define TMP108_CONF_CONVRATE_MASK (TMP108_CONF_CR0|TMP108_CONF_CR1) > +#define TMP108_CONVRATE_0P25HZ 0x0000 > +#define TMP108_CONVRATE_1HZ TMP108_CONF_CR0 /* Default */ > +#define TMP108_CONVRATE_4HZ TMP108_CONF_CR1 > +#define TMP108_CONVRATE_16HZ (TMP108_CONF_CR0|TMP108_CONF_CR1) > + > +#define TMP108_CONF_HYSTERESIS_MASK (TMP108_CONF_HYS0|TMP108_CONF_HYS1) > +#define TMP108_HYSTERESIS_0C 0x0000 > +#define TMP108_HYSTERESIS_1C TMP108_CONF_HYS0 /* Default */ > +#define TMP108_HYSTERESIS_2C TMP108_CONF_HYS1 > +#define TMP108_HYSTERESIS_4C (TMP108_CONF_HYS0|TMP108_CONF_HYS1) > + > +#define TMP108_CONVERSION_TIME_MS 30 /* in milli-seconds */ > + > +struct tmp108 { > + struct regmap *regmap; > + struct device *hwmon_dev; > + struct thermal_zone_device *tz; > + u16 config; > + unsigned long ready_time; > +}; > + > +static const u8 tmp108_temp_reg[TMP108_TEMP_REG_COUNT] = { > + TMP108_REG_TEMP, > + TMP108_REG_TLOW, > + TMP108_REG_THIGH, > +}; > + > +/* convert 12-bit TMP108 register value to milliCelsius */ > +static inline int tmp108_temp_reg_to_mC(s16 val) > +{ > + return (val & ~0x01) * 1000 / 256; > +} > + > +/* convert milliCelsius to left adjusted 12-bit TMP108 register value */ > +static inline u16 tmp108_mC_to_temp_reg(int val) > +{ > + return (val * 256) / 1000; > +} > + > +static int tmp108_read_reg_temp(struct device *dev, int reg, int *temp) > +{ > + struct tmp108 *tmp108 = dev_get_drvdata(dev); > + unsigned int regval; > + int err; > + > + switch (reg) { > + case TMP108_REG_TEMP: > + /* Is it too early to return a conversion ? */ > + if (time_before(jiffies, tmp108->ready_time)) { > + dev_dbg(dev, "%s: Conversion not ready yet..\n", > + __func__); > + return -EAGAIN; > + } > + break; > + case TMP108_REG_TLOW: > + case TMP108_REG_THIGH: > + break; > + default: > + return -EOPNOTSUPP; > + } > + > + err = regmap_read(tmp108->regmap, reg, ®val); > + if (err < 0) > + return err; > + *temp = tmp108_temp_reg_to_mC(regval); > + > + return 0; > +} > + > +static ssize_t tmp108_show_temp(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + struct sensor_device_attribute *sda = to_sensor_dev_attr(attr); > + int temp; > + int err; > + > + if (sda->index >= ARRAY_SIZE(tmp108_temp_reg)) > + return -EINVAL; > + Unnecessary. That would be a programming error, which we don't check in the kernel. You could also provide the register directly in sda->index. > + err = tmp108_read_reg_temp(dev, tmp108_temp_reg[sda->index], &temp); > + if (err) > + return err; > + > + return snprintf(buf, PAGE_SIZE, "%d\n", temp); > +} > + > +static ssize_t tmp108_set_temp(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct sensor_device_attribute *sda = to_sensor_dev_attr(attr); > + struct tmp108 *tmp108 = dev_get_drvdata(dev); > + long temp; > + int err; > + > + if (sda->index >= ARRAY_SIZE(tmp108_temp_reg)) > + return -EINVAL; > + > + if (kstrtol(buf, 10, &temp) < 0) > + return -EINVAL; > + > + temp = clamp_val(temp, TMP108_TEMP_MIN_MC, TMP108_TEMP_MAX_MC); > + err = regmap_write(tmp108->regmap, tmp108_temp_reg[sda->index], > + tmp108_mC_to_temp_reg(temp)); > + if (err) > + return err; > + return count; > +} > + > +static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, tmp108_show_temp, NULL, 0); > +static SENSOR_DEVICE_ATTR(temp1_min, S_IWUSR | S_IRUGO, tmp108_show_temp, > + tmp108_set_temp, 1); > +static SENSOR_DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO, tmp108_show_temp, > + tmp108_set_temp, 2); > + You could also provide temp1_{min,max}_alarm. > +static struct attribute *tmp108_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, > + NULL > +}; > +ATTRIBUTE_GROUPS(tmp108); > + > +static int tmp108_get_temp(void *dev, int *temp) > +{ > + return tmp108_read_reg_temp(dev, TMP108_REG_TEMP, temp); > +} > + > +static const struct thermal_zone_of_device_ops tmp108_of_thermal_ops = { > + .get_temp = tmp108_get_temp, > +}; > + > +static void tmp108_update_ready_time(struct tmp108 *tmp108) > +{ > + tmp108->ready_time = jiffies; > + if ((tmp108->config & TMP108_CONF_MODE_MASK) > + == TMP108_MODE_CONTINUOUS) { Don't you want a "!" here ? Presumably the delay is only needed if the original configuration was not for continuous mode. > + tmp108->ready_time += > + msecs_to_jiffies(TMP108_CONVERSION_TIME_MS); > + } > +} > + > +static void tmp108_restore_config(void *data) > +{ > + struct tmp108 *tmp108 = data; > + > + regmap_write(tmp108->regmap, TMP108_REG_CONF, tmp108->config); > + tmp108_update_ready_time(tmp108); This is called when the driver is unloaded, meaning the call to tmp108_update_ready_time() does not really add any value. > +} > + > +static bool tmp108_is_writeable_reg(struct device *dev, unsigned int reg) > +{ > + return reg != TMP108_REG_TEMP; > +} > + > +static bool tmp108_is_volatile_reg(struct device *dev, unsigned int reg) > +{ > + return reg == TMP108_REG_TEMP; > +} > + > +static const struct regmap_config tmp108_regmap_config = { > + .reg_bits = 8, > + .val_bits = 16, > + .max_register = TMP108_REG_THIGH, > + .writeable_reg = tmp108_is_writeable_reg, > + .volatile_reg = tmp108_is_volatile_reg, > + .val_format_endian = REGMAP_ENDIAN_BIG, > + .cache_type = REGCACHE_RBTREE, > + .use_single_rw = true, > +}; > + > +static int tmp108_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + struct device *dev = &client->dev; > + struct device *hwmon_dev; > + struct tmp108 *tmp108; > + unsigned int regval; > + int err; > + u16 config; > + u32 convrate = 100; > + u32 hysteresis = 1; > + > + if (!i2c_check_functionality(client->adapter, > + I2C_FUNC_SMBUS_WORD_DATA)) { > + dev_err(dev, > + "adapter doesn't support SMBus word transactions\n"); > + return -ENODEV; > + } > + > + tmp108 = devm_kzalloc(dev, sizeof(*tmp108), GFP_KERNEL); > + if (!tmp108) > + return -ENOMEM; > + > + i2c_set_clientdata(client, tmp108); > + > + tmp108->regmap = devm_regmap_init_i2c(client, &tmp108_regmap_config); > + if (IS_ERR(tmp108->regmap)) > + return PTR_ERR(tmp108->regmap); > + > + err = regmap_read(tmp108->regmap, TMP108_REG_CONF, ®val); > + if (err < 0) { > + dev_err(dev, "error reading config register\n"); > + return err; > + } > + tmp108->config = regval; > + config = regval; > + > + /* At this time, only continuous mode is supported. */ > + config &= ~TMP108_CONF_MODE_MASK; > + config |= TMP108_MODE_CONTINUOUS; > + > + if (device_property_read_bool(dev, "ti,thermostat-mode-comparitor")) > + config &= ~TMP108_CONF_TM; > + else > + config |= TMP108_CONF_TM; > + > + if (device_property_read_bool(dev, "ti,alert-active-high")) > + config |= TMP108_CONF_POL; > + else > + config &= ~TMP108_CONF_POL; > + > + if (device_property_read_u32(dev, "ti,conversion-rate-cHz", &convrate) > + >= 0) { > + config &= ~TMP108_CONF_CONVRATE_MASK; > + switch (convrate) { > + case 25: > + config |= TMP108_CONVRATE_0P25HZ; > + break; > + case 100: > + config |= TMP108_CONVRATE_1HZ; > + break; > + case 400: > + config |= TMP108_CONVRATE_4HZ; > + break; > + case 1600: > + config |= TMP108_CONVRATE_16HZ; > + break; > + default: > + dev_err(dev, "conversion rate %u invalid: defaulting to 1Hz.\n", > + convrate); > + convrate = 100; > + config |= TMP108_CONVRATE_1HZ; > + break; > + } > + } > + > + if (device_property_read_u32(dev, "ti,hysteresis", &hysteresis) >= 0) { > + config &= ~TMP108_CONF_HYSTERESIS_MASK; > + switch (hysteresis) { > + case 0: > + config |= TMP108_HYSTERESIS_0C; > + break; > + case 1: > + config |= TMP108_HYSTERESIS_1C; > + break; > + case 2: > + config |= TMP108_HYSTERESIS_2C; > + break; > + case 4: > + config |= TMP108_HYSTERESIS_4C; > + break; > + default: > + dev_err(dev, "hysteresis value %u invalid: defaulting to 1C.\n", > + hysteresis); > + hysteresis = 1; > + config |= TMP108_HYSTERESIS_1C; > + break; > + } > + } > + > + err = regmap_write(tmp108->regmap, TMP108_REG_CONF, config); > + if (err < 0) { > + dev_err(dev, "error writing config register\n"); > + return err; > + } > + > + tmp108->config = config; > + tmp108_update_ready_time(tmp108); > + > + err = devm_add_action_or_reset(dev, tmp108_restore_config, tmp108); > + if (err) > + return err; > + > + hwmon_dev = hwmon_device_register_with_groups(dev, client->name, > + tmp108, tmp108_groups); Please consider using the devm_ function here. > + if (IS_ERR(hwmon_dev)) { > + dev_dbg(dev, "unable to register hwmon device\n"); > + return PTR_ERR(hwmon_dev); > + } > + > + tmp108->hwmon_dev = hwmon_dev; > + tmp108->tz = thermal_zone_of_sensor_register(hwmon_dev, 0, hwmon_dev, > + &tmp108_of_thermal_ops); > + if (IS_ERR(tmp108->tz)) > + return PTR_ERR(tmp108->tz); hwmon device not unregistered here. That would be fixed by using the devm function above. > + > + dev_info(dev, "%s, alert: active %s, hyst: %uC, conv: %ucHz\n", > + (tmp108->config & TMP108_CONF_TM) != 0 ? > + "interrupt" : "comparator", > + (tmp108->config & TMP108_CONF_POL) != 0 ? "high" : "low", > + hysteresis, convrate); > + return 0; > +} > + > +static int tmp108_remove(struct i2c_client *client) > +{ > + struct tmp108 *tmp108 = i2c_get_clientdata(client); > + > + thermal_zone_of_sensor_unregister(tmp108->hwmon_dev, tmp108->tz); > + hwmon_device_unregister(tmp108->hwmon_dev); > + > + return 0; > +} > + > +#ifdef CONFIG_PM_SLEEP > +static int tmp108_suspend(struct device *dev) I prefer __maybe_unused without #ifdef to ensure that the functions can be compiled even if unused. > +{ > + struct tmp108 *tmp108 = dev_get_drvdata(dev); > + > + return regmap_update_bits(tmp108->regmap, TMP108_REG_CONF, > + TMP108_CONF_MODE_MASK, TMP108_MODE_SHUTDOWN); > +} > + > +static int tmp108_resume(struct device *dev) > +{ > + struct tmp108 *tmp108 = dev_get_drvdata(dev); > + int err; > + > + err = regmap_write(tmp108->regmap, TMP108_REG_CONF, tmp108->config); > + > + tmp108_update_ready_time(tmp108); > + > + return err; > +} > +#endif /* CONFIG_PM */ > + > +static SIMPLE_DEV_PM_OPS(tmp108_dev_pm_ops, tmp108_suspend, tmp108_resume); > + > +static const struct i2c_device_id tmp108_id[] = { > + { "tmp108", 0 }, > + { } > +}; > +MODULE_DEVICE_TABLE(i2c, tmp108_id); > + > +static struct i2c_driver tmp108_driver = { > + .driver.name = DRIVER_NAME, > + .driver.pm = &tmp108_dev_pm_ops, > + .probe = tmp108_probe, > + .remove = tmp108_remove, > + .id_table = tmp108_id, > +}; > + > +module_i2c_driver(tmp108_driver); > + > +MODULE_AUTHOR("John Muir <john@jmuir.com>"); > +MODULE_DESCRIPTION("Texas Instruments TMP108 temperature sensor driver"); > +MODULE_LICENSE("GPL"); > -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> On Nov 27, 2016, at 4:19 AM, Guenter Roeck <linux@roeck-us.net> wrote: > > On 11/26/2016 09:15 PM, John Muir wrote: >> Add support for the TI TMP108 temperature sensor with some device >> configuration parameters. >> +- ti,alert-active-high : (boolean) make the alert pin active-high instead of >> + the default active-low. > > The driver doesn't support interrupts/alerts. Do those properties really add value ? Getting ahead of myself. I will create a patch for this in the future. >> +static void tmp108_update_ready_time(struct tmp108 *tmp108) >> +{ >> + tmp108->ready_time = jiffies; >> + if ((tmp108->config & TMP108_CONF_MODE_MASK) >> + == TMP108_MODE_CONTINUOUS) { > > Don't you want a "!" here ? Presumably the delay is only needed > if the original configuration was not for continuous mode. > >> + tmp108->ready_time += >> + msecs_to_jiffies(TMP108_CONVERSION_TIME_MS); >> + } >> +} The delay is required for both really. When the device is set into continuous mode it starts converting and the first temperature is ready (just under) 30ms later. For the (unsupported at this time) one-shot mode, there would likewise be a delay, but I was envisioning having the requesting task sleep while waiting for the data to be ready, rather than get an -EAGAIN until the data is ready. >> + hwmon_dev = hwmon_device_register_with_groups(dev, client->name, >> + tmp108, tmp108_groups); > > Please consider using the devm_ function here. > >> + if (IS_ERR(hwmon_dev)) { >> + dev_dbg(dev, "unable to register hwmon device\n"); >> + return PTR_ERR(hwmon_dev); >> + } >> + >> + tmp108->hwmon_dev = hwmon_dev; >> + tmp108->tz = thermal_zone_of_sensor_register(hwmon_dev, 0, hwmon_dev, >> + &tmp108_of_thermal_ops); >> + if (IS_ERR(tmp108->tz)) >> + return PTR_ERR(tmp108->tz); > > hwmon device not unregistered here. That would be fixed by using the devm > function above. > For the above two registrations and .remove function, I was worried that there would be an order problem between the i2c->remove and the device-managed cleanup. I’ll get deeper into that code to determine if that is a problem. For your other comments, I will make the necessary changes. Thanks! John. -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Nov 28, 2016 at 11:40:42AM -0800, John Muir wrote: > > > On Nov 27, 2016, at 4:19 AM, Guenter Roeck <linux@roeck-us.net> wrote: > > > > On 11/26/2016 09:15 PM, John Muir wrote: > >> Add support for the TI TMP108 temperature sensor with some device > >> configuration parameters. > >> +- ti,alert-active-high : (boolean) make the alert pin active-high instead of > >> + the default active-low. > > > > The driver doesn't support interrupts/alerts. Do those properties really add value ? > Getting ahead of myself. I will create a patch for this in the future. > > >> +static void tmp108_update_ready_time(struct tmp108 *tmp108) > >> +{ > >> + tmp108->ready_time = jiffies; > >> + if ((tmp108->config & TMP108_CONF_MODE_MASK) > >> + == TMP108_MODE_CONTINUOUS) { > > > > Don't you want a "!" here ? Presumably the delay is only needed > > if the original configuration was not for continuous mode. > > > >> + tmp108->ready_time += > >> + msecs_to_jiffies(TMP108_CONVERSION_TIME_MS); > >> + } > >> +} > The delay is required for both really. When the device is set into continuous mode it starts converting and the first temperature is ready (just under) 30ms later. > The datasheet states, though, that the chip will start converting as soon as the device powers up. The kernel would have to start really fast after that to have to wait another 30 ms. The current code ends up waiting if it doesn't have to (because it waits if the chip was originally configured for continuous mode), and not waiting if it has to (because the chip was not configured for continuous mode), which doesn't seem to be such a good idea. > For the (unsupported at this time) one-shot mode, there would likewise be a delay, but I was envisioning having the requesting task sleep while waiting for the data to be ready, rather than get an -EAGAIN until the data is ready. > We had that discussion before; the thermal subsystem doesn't like to be kept waiting. You would end up adding a lot of complexity for very little gain. It might make more sense to consider implementing runtime idle support instead of one-shot mode if power consumption is a concern. > >> + hwmon_dev = hwmon_device_register_with_groups(dev, client->name, > >> + tmp108, tmp108_groups); > > > > Please consider using the devm_ function here. > > > >> + if (IS_ERR(hwmon_dev)) { > >> + dev_dbg(dev, "unable to register hwmon device\n"); > >> + return PTR_ERR(hwmon_dev); > >> + } > >> + > >> + tmp108->hwmon_dev = hwmon_dev; > >> + tmp108->tz = thermal_zone_of_sensor_register(hwmon_dev, 0, hwmon_dev, > >> + &tmp108_of_thermal_ops); > >> + if (IS_ERR(tmp108->tz)) > >> + return PTR_ERR(tmp108->tz); > > > > hwmon device not unregistered here. That would be fixed by using the devm > > function above. > > > For the above two registrations and .remove function, I was worried that there would be an order problem between the i2c->remove and the device-managed cleanup. I’ll get deeper into that code to determine if that is a problem. > The thermal sensor registration will be removed first (in the remove function). devm_ functions are all unregistered / removed after the remove function was called (in the order of the devm_ call). Otherwise you would have trouble with devm_kzalloc() as well. Note that my followup-patch had a problem with the minimum hysteresis temperature; it needs to be higher than the lower limit, not lower. Thanks, Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> On Nov 28, 2016, at 11:58 AM, Guenter Roeck <linux@roeck-us.net> wrote: > > On Mon, Nov 28, 2016 at 11:40:42AM -0800, John Muir wrote: >>>> +static void tmp108_update_ready_time(struct tmp108 *tmp108) >>>> +{ >>>> + tmp108->ready_time = jiffies; >>>> + if ((tmp108->config & TMP108_CONF_MODE_MASK) >>>> + == TMP108_MODE_CONTINUOUS) { >>> >>> Don't you want a "!" here ? Presumably the delay is only needed >>> if the original configuration was not for continuous mode. >>> >>>> + tmp108->ready_time += >>>> + msecs_to_jiffies(TMP108_CONVERSION_TIME_MS); >>>> + } >>>> +} >> The delay is required for both really. When the device is set into continuous mode it starts converting and the first temperature is ready (just under) 30ms later. >> > > The datasheet states, though, that the chip will start converting as soon > as the device powers up. The kernel would have to start really fast after that > to have to wait another 30 ms. > > The current code ends up waiting if it doesn't have to (because it waits > if the chip was originally configured for continuous mode), and not waiting > if it has to (because the chip was not configured for continuous mode), > which doesn't seem to be such a good idea. You are referring to the statement under “Conversion Rate” in the data sheet? "After power-up or a general-call reset, the TMP108 immediately starts a conversion, as shown in Figure 9. The first result is available after 27 ms (typical).” The datasheet also states that in ’shutdown’ mode, the device is only powering the serial interface (see "Mode Bits”). So, I assumed that the conversions weren’t happening during in that state, and that there would be the delay after moving from the ‘shutdown' state (as is described by the one-shot mode as well where the device state returns to ’shutdown’ when the conversion has taken place). My intention was that the delay is enforced after PM resume, and for convenience I re-used the same function during probe to initialize the ready_time. I was assuming that the device could be in the shutdown state - for example if the module had previously been unloaded. I agree that this is not required at system startup time. Perhaps I should check to see if the previous configuration had the device in the ‘shutdown’ state, and in that instance apply the delay? In my opinion it doesn’t really matter either way. FYI, this same logic exists in the TMP102 device driver. The TMP102 device is very similar. > >> For the (unsupported at this time) one-shot mode, there would likewise be a delay, but I was envisioning having the requesting task sleep while waiting for the data to be ready, rather than get an -EAGAIN until the data is ready. >> > > We had that discussion before; the thermal subsystem doesn't like to be kept > waiting. You would end up adding a lot of complexity for very little gain. > It might make more sense to consider implementing runtime idle support > instead of one-shot mode if power consumption is a concern. OK thanks. I’ll leave this as unsupported for now. > >>>> + hwmon_dev = hwmon_device_register_with_groups(dev, client->name, >>>> + tmp108, tmp108_groups); >>> >>> Please consider using the devm_ function here. >>> >>>> + if (IS_ERR(hwmon_dev)) { >>>> + dev_dbg(dev, "unable to register hwmon device\n"); >>>> + return PTR_ERR(hwmon_dev); >>>> + } >>>> + >>>> + tmp108->hwmon_dev = hwmon_dev; >>>> + tmp108->tz = thermal_zone_of_sensor_register(hwmon_dev, 0, hwmon_dev, >>>> + &tmp108_of_thermal_ops); >>>> + if (IS_ERR(tmp108->tz)) >>>> + return PTR_ERR(tmp108->tz); >>> >>> hwmon device not unregistered here. That would be fixed by using the devm >>> function above. >>> >> For the above two registrations and .remove function, I was worried that there would be an order problem between the i2c->remove and the device-managed cleanup. I’ll get deeper into that code to determine if that is a problem. >> > > The thermal sensor registration will be removed first (in the remove function). > devm_ functions are all unregistered / removed after the remove function was > called (in the order of the devm_ call). Otherwise you would have trouble > with devm_kzalloc() as well. Great. > Note that my followup-patch had a problem with the minimum hysteresis > temperature; it needs to be higher than the lower limit, not lower. I’ll keep that in mind. Cheers, John. -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Nov 28, 2016 at 01:25:37PM -0800, John Muir wrote: > > > On Nov 28, 2016, at 11:58 AM, Guenter Roeck <linux@roeck-us.net> wrote: > > > > On Mon, Nov 28, 2016 at 11:40:42AM -0800, John Muir wrote: > >>>> +static void tmp108_update_ready_time(struct tmp108 *tmp108) > >>>> +{ > >>>> + tmp108->ready_time = jiffies; > >>>> + if ((tmp108->config & TMP108_CONF_MODE_MASK) > >>>> + == TMP108_MODE_CONTINUOUS) { > >>> > >>> Don't you want a "!" here ? Presumably the delay is only needed > >>> if the original configuration was not for continuous mode. > >>> > >>>> + tmp108->ready_time += > >>>> + msecs_to_jiffies(TMP108_CONVERSION_TIME_MS); > >>>> + } > >>>> +} > >> The delay is required for both really. When the device is set into continuous mode it starts converting and the first temperature is ready (just under) 30ms later. > >> > > > > The datasheet states, though, that the chip will start converting as soon > > as the device powers up. The kernel would have to start really fast after that > > to have to wait another 30 ms. > > > > The current code ends up waiting if it doesn't have to (because it waits > > if the chip was originally configured for continuous mode), and not waiting > > if it has to (because the chip was not configured for continuous mode), > > which doesn't seem to be such a good idea. > > You are referring to the statement under “Conversion Rate” in the data sheet? > > "After power-up or a general-call reset, the TMP108 immediately starts a conversion, as shown in Figure 9. The first result is available after 27 ms (typical).” > > The datasheet also states that in ’shutdown’ mode, the device is only powering the serial interface (see "Mode Bits”). So, I assumed that the conversions weren’t happening during in that state, and that there would be the delay after moving from the ‘shutdown' state (as is described by the one-shot mode as well where the device state returns to ’shutdown’ when the conversion has taken place). The device is, after power-on, not in shutdown mode. > > My intention was that the delay is enforced after PM resume, and for convenience I re-used the same function during probe to initialize the ready_time. I was assuming that the device could be in the shutdown state - for example if the module had previously been unloaded. I agree that this is not required at system startup time. Perhaps I should check to see if the previous configuration had the device in the ‘shutdown’ state, and in that instance apply the delay? In my opinion it doesn’t really matter either way. > > FYI, this same logic exists in the TMP102 device driver. The TMP102 device is very similar. > The tmp102 driver adds the timeout if the device was in shutdown mode (SD=1). if (tmp102->config_orig & TMP102_CONF_SD) { ... tmp102->ready_time += msecs_to_jiffies(CONVERSION_TIME_MS); } Your code adds the timeout if the device was in continuous operation mode (M1=1). if ((tmp108->config & TMP108_CONF_MODE_MASK) == TMP108_MODE_CONTINUOUS) { tmp108->ready_time += msecs_to_jiffies(TMP108_CONVERSION_TIME_MS); } Unless I am missing something, that is exactly the opposite. Side note: Per datasheet, M0 is irrelevant if M1=1. The above check does not match M1=1, M0=1, but that condition would still reflect continuous mode. Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Nov 28, 2016, at 2:19 PM, Guenter Roeck <linux@roeck-us.net> wrote: > > The tmp102 driver adds the timeout if the device was in shutdown mode (SD=1). > > if (tmp102->config_orig & TMP102_CONF_SD) { > ... > tmp102->ready_time += msecs_to_jiffies(CONVERSION_TIME_MS); > } > > Your code adds the timeout if the device was in continuous operation mode (M1=1). > > if ((tmp108->config & TMP108_CONF_MODE_MASK) > == TMP108_MODE_CONTINUOUS) { > tmp108->ready_time += > msecs_to_jiffies(TMP108_CONVERSION_TIME_MS); > } > > Unless I am missing something, that is exactly the opposite. Note that the TMP102 code is looking at ‘config_orig’ which was the initial device state, whereas in my proposed driver the code looks at the current configuration. The update_ready_time function was only intended to be called AFTER the device has moved to a new mode. For the probe case I simply ignored the fact that it was already in continuous mode at startup and lazily re-used the update_ready_time function causing the code to run through a few extra instructions. I was attempting to re-use the logic in multiple cases. I’ll update the code in my next patch series and you can re-review. > Side note: Per datasheet, M0 is irrelevant if M1=1. The above check does not > match M1=1, M0=1, but that condition would still reflect continuous mode. > OK. Noted. Thanks, John. -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Nov 28, 2016 at 03:10:38PM -0800, John Muir wrote: > On Nov 28, 2016, at 2:19 PM, Guenter Roeck <linux@roeck-us.net> wrote: > > > > The tmp102 driver adds the timeout if the device was in shutdown mode (SD=1). > > > > if (tmp102->config_orig & TMP102_CONF_SD) { > > ... > > tmp102->ready_time += msecs_to_jiffies(CONVERSION_TIME_MS); > > } > > > > Your code adds the timeout if the device was in continuous operation mode (M1=1). > > > > if ((tmp108->config & TMP108_CONF_MODE_MASK) > > == TMP108_MODE_CONTINUOUS) { > > tmp108->ready_time += > > msecs_to_jiffies(TMP108_CONVERSION_TIME_MS); > > } > > > > Unless I am missing something, that is exactly the opposite. > > Note that the TMP102 code is looking at ‘config_orig’ which was the initial device state, whereas in my proposed driver the code looks at the current configuration. > Ah, yes. Your code also uses tmp108->config in tmp108_restore_config(), which I guess confused me. Not sure I understand why you would want to add the wait time even if M1 was set before (and the tmp102 driver doesn't do that). Really, if that is what you want, it would be much simpler if you just unconditionally set ready_time to 30 ms after the current time after setting the M1 bit, no matter if it was set before or not. I personally don't think that would be such a good idea, but it would at least be less obfuscating than the current code. Also, I would suggest to either drop tmp108_restore_config(), or to have it restore the real original state. Thanks, Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/hwmon/tmp108.txt b/Documentation/devicetree/bindings/hwmon/tmp108.txt new file mode 100644 index 0000000..210af63 --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/tmp108.txt @@ -0,0 +1,27 @@ +TMP108 temperature sensor +------------------------- + +This device supports I2C only. + +Requires node properties: +- compatible : "ti,tmp108" +- reg : the I2C address of the device. This is 0x48, 0x49, 0x4a, or 0x4b. + +Optional node properties: +- ti,thermostat-mode-comparitor : (boolean) select the comparitor mode for the + thermostat rather than the default interrupt-mode. +- ti,alert-active-high : (boolean) make the alert pin active-high instead of + the default active-low. +- ti,conversion-rate-cHz : (integer, cHz) select the conversion frequency for + continuous mode, in centi-Hz: 25, 100 (default), 400, or 1600. +- ti,hysteresis : (integer, C) select the hysteresis value: 0, 1, 2, or 4 + (celcius). + +Example: + tmp108@48 { + compatible = "ti,tmp108"; + reg = <0x48>; + ti,alert-active-high; + ti,hysteresis = <2>; + ti,conversion-rate-cHz = <25>; + }; diff --git a/Documentation/hwmon/tmp108 b/Documentation/hwmon/tmp108 new file mode 100644 index 0000000..ef2e9a3 --- /dev/null +++ b/Documentation/hwmon/tmp108 @@ -0,0 +1,38 @@ +Kernel driver tmp108 +==================== + +Supported chips: + * Texas Instruments TMP108 + Prefix: 'tmp108' + Addresses scanned: none + Datasheet: http://www.ti.com/product/tmp108 + +Author: + John Muir <john@jmuir.com> + +Description +----------- + +The Texas Instruments TMP108 implements one temperature sensor. An alert pin +can be set when temperatures exceed minimum or maximum values plus or minus a +hysteresis value. + +The sensor is accurate to 0.75C over the range of -25 to +85 C, and to 1.0 +degree from -40 to +125 C. Resolution of the sensor is 0.0625 degree. The +operating temperature has a minimum of -55 C and a maximum of +150 C. +Hysteresis values can be set to 0, 1, 2, or 4C. + +The TMP108 has a programmable update rate that can select between 8, 4, 1, and +0.5 Hz. + +By default the TMP108 reads the temperature continuously. To conserve power, +the TMP108 has a one-shot mode where the device is normally shut-down. When a +one shot is requested the temperature is read, the result can be retrieved, +and then the device is shut down automatically. (This driver only supports +continuous mode.) + +The driver provides the common sysfs-interface for temperatures (see +Documentation/hwmon/sysfs-interface under Temperatures). + +See Documentation/devicetree/bindings/hwmon/tmp108.txt for configuration +properties. diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index 45cef3d..4c173de 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -1591,6 +1591,17 @@ config SENSORS_TMP103 This driver can also be built as a module. If so, the module will be called tmp103. +config SENSORS_TMP108 + tristate "Texas Instruments TMP108" + depends on I2C + select REGMAP_I2C + help + If you say yes here you get support for Texas Instruments TMP108 + sensor chips. + + This driver can also be built as a module. If so, the module + will be called tmp108. + config SENSORS_TMP401 tristate "Texas Instruments TMP401 and compatibles" depends on I2C diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index aecf4ba..51e5256 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -152,6 +152,7 @@ obj-$(CONFIG_SENSORS_TC74) += tc74.o obj-$(CONFIG_SENSORS_THMC50) += thmc50.o obj-$(CONFIG_SENSORS_TMP102) += tmp102.o obj-$(CONFIG_SENSORS_TMP103) += tmp103.o +obj-$(CONFIG_SENSORS_TMP108) += tmp108.o obj-$(CONFIG_SENSORS_TMP401) += tmp401.o obj-$(CONFIG_SENSORS_TMP421) += tmp421.o obj-$(CONFIG_SENSORS_TWL4030_MADC)+= twl4030-madc-hwmon.o diff --git a/drivers/hwmon/tmp108.c b/drivers/hwmon/tmp108.c new file mode 100644 index 0000000..da64517 --- /dev/null +++ b/drivers/hwmon/tmp108.c @@ -0,0 +1,429 @@ +/* Texas Instruments TMP108 SMBus temperature sensor driver + * + * Copyright (C) 2016 John Muir <john@jmuir.com> + * + * 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/delay.h> +#include <linux/module.h> +#include <linux/init.h> +#include <linux/slab.h> +#include <linux/i2c.h> +#include <linux/hwmon.h> +#include <linux/hwmon-sysfs.h> +#include <linux/err.h> +#include <linux/mutex.h> +#include <linux/device.h> +#include <linux/jiffies.h> +#include <linux/regmap.h> +#include <linux/thermal.h> +#include <linux/of.h> + +#define DRIVER_NAME "tmp108" + +#define TMP108_REG_TEMP 0x00 +#define TMP108_REG_CONF 0x01 +#define TMP108_REG_TLOW 0x02 +#define TMP108_REG_THIGH 0x03 + +#define TMP108_TEMP_REG_COUNT 3 + +#define TMP108_TEMP_MIN_MC -50000 /* Minimum millicelcius. */ +#define TMP108_TEMP_MAX_MC 127937 /* Maximum millicelcius. */ + +/* Configuration register bits. + * Note: these bit definitions are byte swapped. + */ +#define TMP108_CONF_M0 0x0100 /* Sensor mode. */ +#define TMP108_CONF_M1 0x0200 +#define TMP108_CONF_TM 0x0400 /* Thermostat mode. */ +#define TMP108_CONF_FL 0x0800 /* Watchdog flag - TLOW */ +#define TMP108_CONF_FH 0x1000 /* Watchdog flag - THIGH */ +#define TMP108_CONF_CR0 0x2000 /* Conversion rate. */ +#define TMP108_CONF_CR1 0x4000 +#define TMP108_CONF_ID 0x8000 +#define TMP108_CONF_HYS0 0x0010 /* Hysteresis. */ +#define TMP108_CONF_HYS1 0x0020 +#define TMP108_CONF_POL 0x0080 /* Polarity of alert. */ + +/* Defaults set by the hardware upon reset. */ +#define TMP108_CONF_DEFAULTS (TMP108_CONF_CR0 | TMP108_CONF_TM |\ + TMP108_CONF_HYS0 | TMP108_CONF_M1) +/* These bits are read-only. */ +#define TMP108_CONF_READ_ONLY (TMP108_CONF_FL | TMP108_CONF_FH |\ + TMP108_CONF_ID) + +#define TMP108_CONF_MODE_MASK (TMP108_CONF_M0|TMP108_CONF_M1) +#define TMP108_MODE_SHUTDOWN 0x0000 +#define TMP108_MODE_ONE_SHOT TMP108_CONF_M0 +#define TMP108_MODE_CONTINUOUS TMP108_CONF_M1 /* Default */ + +#define TMP108_CONF_CONVRATE_MASK (TMP108_CONF_CR0|TMP108_CONF_CR1) +#define TMP108_CONVRATE_0P25HZ 0x0000 +#define TMP108_CONVRATE_1HZ TMP108_CONF_CR0 /* Default */ +#define TMP108_CONVRATE_4HZ TMP108_CONF_CR1 +#define TMP108_CONVRATE_16HZ (TMP108_CONF_CR0|TMP108_CONF_CR1) + +#define TMP108_CONF_HYSTERESIS_MASK (TMP108_CONF_HYS0|TMP108_CONF_HYS1) +#define TMP108_HYSTERESIS_0C 0x0000 +#define TMP108_HYSTERESIS_1C TMP108_CONF_HYS0 /* Default */ +#define TMP108_HYSTERESIS_2C TMP108_CONF_HYS1 +#define TMP108_HYSTERESIS_4C (TMP108_CONF_HYS0|TMP108_CONF_HYS1) + +#define TMP108_CONVERSION_TIME_MS 30 /* in milli-seconds */ + +struct tmp108 { + struct regmap *regmap; + struct device *hwmon_dev; + struct thermal_zone_device *tz; + u16 config; + unsigned long ready_time; +}; + +static const u8 tmp108_temp_reg[TMP108_TEMP_REG_COUNT] = { + TMP108_REG_TEMP, + TMP108_REG_TLOW, + TMP108_REG_THIGH, +}; + +/* convert 12-bit TMP108 register value to milliCelsius */ +static inline int tmp108_temp_reg_to_mC(s16 val) +{ + return (val & ~0x01) * 1000 / 256; +} + +/* convert milliCelsius to left adjusted 12-bit TMP108 register value */ +static inline u16 tmp108_mC_to_temp_reg(int val) +{ + return (val * 256) / 1000; +} + +static int tmp108_read_reg_temp(struct device *dev, int reg, int *temp) +{ + struct tmp108 *tmp108 = dev_get_drvdata(dev); + unsigned int regval; + int err; + + switch (reg) { + case TMP108_REG_TEMP: + /* Is it too early to return a conversion ? */ + if (time_before(jiffies, tmp108->ready_time)) { + dev_dbg(dev, "%s: Conversion not ready yet..\n", + __func__); + return -EAGAIN; + } + break; + case TMP108_REG_TLOW: + case TMP108_REG_THIGH: + break; + default: + return -EOPNOTSUPP; + } + + err = regmap_read(tmp108->regmap, reg, ®val); + if (err < 0) + return err; + *temp = tmp108_temp_reg_to_mC(regval); + + return 0; +} + +static ssize_t tmp108_show_temp(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + struct sensor_device_attribute *sda = to_sensor_dev_attr(attr); + int temp; + int err; + + if (sda->index >= ARRAY_SIZE(tmp108_temp_reg)) + return -EINVAL; + + err = tmp108_read_reg_temp(dev, tmp108_temp_reg[sda->index], &temp); + if (err) + return err; + + return snprintf(buf, PAGE_SIZE, "%d\n", temp); +} + +static ssize_t tmp108_set_temp(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + struct sensor_device_attribute *sda = to_sensor_dev_attr(attr); + struct tmp108 *tmp108 = dev_get_drvdata(dev); + long temp; + int err; + + if (sda->index >= ARRAY_SIZE(tmp108_temp_reg)) + return -EINVAL; + + if (kstrtol(buf, 10, &temp) < 0) + return -EINVAL; + + temp = clamp_val(temp, TMP108_TEMP_MIN_MC, TMP108_TEMP_MAX_MC); + err = regmap_write(tmp108->regmap, tmp108_temp_reg[sda->index], + tmp108_mC_to_temp_reg(temp)); + if (err) + return err; + return count; +} + +static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, tmp108_show_temp, NULL, 0); +static SENSOR_DEVICE_ATTR(temp1_min, S_IWUSR | S_IRUGO, tmp108_show_temp, + tmp108_set_temp, 1); +static SENSOR_DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO, tmp108_show_temp, + tmp108_set_temp, 2); + +static struct attribute *tmp108_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, + NULL +}; +ATTRIBUTE_GROUPS(tmp108); + +static int tmp108_get_temp(void *dev, int *temp) +{ + return tmp108_read_reg_temp(dev, TMP108_REG_TEMP, temp); +} + +static const struct thermal_zone_of_device_ops tmp108_of_thermal_ops = { + .get_temp = tmp108_get_temp, +}; + +static void tmp108_update_ready_time(struct tmp108 *tmp108) +{ + tmp108->ready_time = jiffies; + if ((tmp108->config & TMP108_CONF_MODE_MASK) + == TMP108_MODE_CONTINUOUS) { + tmp108->ready_time += + msecs_to_jiffies(TMP108_CONVERSION_TIME_MS); + } +} + +static void tmp108_restore_config(void *data) +{ + struct tmp108 *tmp108 = data; + + regmap_write(tmp108->regmap, TMP108_REG_CONF, tmp108->config); + tmp108_update_ready_time(tmp108); +} + +static bool tmp108_is_writeable_reg(struct device *dev, unsigned int reg) +{ + return reg != TMP108_REG_TEMP; +} + +static bool tmp108_is_volatile_reg(struct device *dev, unsigned int reg) +{ + return reg == TMP108_REG_TEMP; +} + +static const struct regmap_config tmp108_regmap_config = { + .reg_bits = 8, + .val_bits = 16, + .max_register = TMP108_REG_THIGH, + .writeable_reg = tmp108_is_writeable_reg, + .volatile_reg = tmp108_is_volatile_reg, + .val_format_endian = REGMAP_ENDIAN_BIG, + .cache_type = REGCACHE_RBTREE, + .use_single_rw = true, +}; + +static int tmp108_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + struct device *dev = &client->dev; + struct device *hwmon_dev; + struct tmp108 *tmp108; + unsigned int regval; + int err; + u16 config; + u32 convrate = 100; + u32 hysteresis = 1; + + if (!i2c_check_functionality(client->adapter, + I2C_FUNC_SMBUS_WORD_DATA)) { + dev_err(dev, + "adapter doesn't support SMBus word transactions\n"); + return -ENODEV; + } + + tmp108 = devm_kzalloc(dev, sizeof(*tmp108), GFP_KERNEL); + if (!tmp108) + return -ENOMEM; + + i2c_set_clientdata(client, tmp108); + + tmp108->regmap = devm_regmap_init_i2c(client, &tmp108_regmap_config); + if (IS_ERR(tmp108->regmap)) + return PTR_ERR(tmp108->regmap); + + err = regmap_read(tmp108->regmap, TMP108_REG_CONF, ®val); + if (err < 0) { + dev_err(dev, "error reading config register\n"); + return err; + } + tmp108->config = regval; + config = regval; + + /* At this time, only continuous mode is supported. */ + config &= ~TMP108_CONF_MODE_MASK; + config |= TMP108_MODE_CONTINUOUS; + + if (device_property_read_bool(dev, "ti,thermostat-mode-comparitor")) + config &= ~TMP108_CONF_TM; + else + config |= TMP108_CONF_TM; + + if (device_property_read_bool(dev, "ti,alert-active-high")) + config |= TMP108_CONF_POL; + else + config &= ~TMP108_CONF_POL; + + if (device_property_read_u32(dev, "ti,conversion-rate-cHz", &convrate) + >= 0) { + config &= ~TMP108_CONF_CONVRATE_MASK; + switch (convrate) { + case 25: + config |= TMP108_CONVRATE_0P25HZ; + break; + case 100: + config |= TMP108_CONVRATE_1HZ; + break; + case 400: + config |= TMP108_CONVRATE_4HZ; + break; + case 1600: + config |= TMP108_CONVRATE_16HZ; + break; + default: + dev_err(dev, "conversion rate %u invalid: defaulting to 1Hz.\n", + convrate); + convrate = 100; + config |= TMP108_CONVRATE_1HZ; + break; + } + } + + if (device_property_read_u32(dev, "ti,hysteresis", &hysteresis) >= 0) { + config &= ~TMP108_CONF_HYSTERESIS_MASK; + switch (hysteresis) { + case 0: + config |= TMP108_HYSTERESIS_0C; + break; + case 1: + config |= TMP108_HYSTERESIS_1C; + break; + case 2: + config |= TMP108_HYSTERESIS_2C; + break; + case 4: + config |= TMP108_HYSTERESIS_4C; + break; + default: + dev_err(dev, "hysteresis value %u invalid: defaulting to 1C.\n", + hysteresis); + hysteresis = 1; + config |= TMP108_HYSTERESIS_1C; + break; + } + } + + err = regmap_write(tmp108->regmap, TMP108_REG_CONF, config); + if (err < 0) { + dev_err(dev, "error writing config register\n"); + return err; + } + + tmp108->config = config; + tmp108_update_ready_time(tmp108); + + err = devm_add_action_or_reset(dev, tmp108_restore_config, tmp108); + if (err) + return err; + + hwmon_dev = hwmon_device_register_with_groups(dev, client->name, + tmp108, tmp108_groups); + if (IS_ERR(hwmon_dev)) { + dev_dbg(dev, "unable to register hwmon device\n"); + return PTR_ERR(hwmon_dev); + } + + tmp108->hwmon_dev = hwmon_dev; + tmp108->tz = thermal_zone_of_sensor_register(hwmon_dev, 0, hwmon_dev, + &tmp108_of_thermal_ops); + if (IS_ERR(tmp108->tz)) + return PTR_ERR(tmp108->tz); + + dev_info(dev, "%s, alert: active %s, hyst: %uC, conv: %ucHz\n", + (tmp108->config & TMP108_CONF_TM) != 0 ? + "interrupt" : "comparator", + (tmp108->config & TMP108_CONF_POL) != 0 ? "high" : "low", + hysteresis, convrate); + return 0; +} + +static int tmp108_remove(struct i2c_client *client) +{ + struct tmp108 *tmp108 = i2c_get_clientdata(client); + + thermal_zone_of_sensor_unregister(tmp108->hwmon_dev, tmp108->tz); + hwmon_device_unregister(tmp108->hwmon_dev); + + return 0; +} + +#ifdef CONFIG_PM_SLEEP +static int tmp108_suspend(struct device *dev) +{ + struct tmp108 *tmp108 = dev_get_drvdata(dev); + + return regmap_update_bits(tmp108->regmap, TMP108_REG_CONF, + TMP108_CONF_MODE_MASK, TMP108_MODE_SHUTDOWN); +} + +static int tmp108_resume(struct device *dev) +{ + struct tmp108 *tmp108 = dev_get_drvdata(dev); + int err; + + err = regmap_write(tmp108->regmap, TMP108_REG_CONF, tmp108->config); + + tmp108_update_ready_time(tmp108); + + return err; +} +#endif /* CONFIG_PM */ + +static SIMPLE_DEV_PM_OPS(tmp108_dev_pm_ops, tmp108_suspend, tmp108_resume); + +static const struct i2c_device_id tmp108_id[] = { + { "tmp108", 0 }, + { } +}; +MODULE_DEVICE_TABLE(i2c, tmp108_id); + +static struct i2c_driver tmp108_driver = { + .driver.name = DRIVER_NAME, + .driver.pm = &tmp108_dev_pm_ops, + .probe = tmp108_probe, + .remove = tmp108_remove, + .id_table = tmp108_id, +}; + +module_i2c_driver(tmp108_driver); + +MODULE_AUTHOR("John Muir <john@jmuir.com>"); +MODULE_DESCRIPTION("Texas Instruments TMP108 temperature sensor driver"); +MODULE_LICENSE("GPL");
Add support for the TI TMP108 temperature sensor with some device configuration parameters. Signed-off-by: John Muir <john@jmuir.com> --- Documentation/devicetree/bindings/hwmon/tmp108.txt | 27 ++ Documentation/hwmon/tmp108 | 38 ++ drivers/hwmon/Kconfig | 11 + drivers/hwmon/Makefile | 1 + drivers/hwmon/tmp108.c | 429 +++++++++++++++++++++ 5 files changed, 506 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/tmp108.txt create mode 100644 Documentation/hwmon/tmp108 create mode 100644 drivers/hwmon/tmp108.c