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@xxxxxxxxx> --- 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@xxxxxxxxx> + +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@xxxxxxxxx> + * + * 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@xxxxxxxxx>"); +MODULE_DESCRIPTION("Texas Instruments TMP108 temperature sensor driver"); +MODULE_LICENSE("GPL");
-- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html