Add support for the wsen tids. It is a low cost
and small-form-factor i2c temperature sensor.
It supports the following features:
- Continuous temperature reading in four intervals: 5 ms, 10 ms,
20 ms and 40 ms.
- Low temperature alarm
- High temperature alarm
The driver supports following hwmon features:
- hwmon_temp_input
- hwmon_temp_min_alarm
- hwmon_temp_max_alarm
- hwmon_temp_min
- hwmon_temp_max
- hwmon_chip_update_interval
Additional notes:
- The update interval only supports four fixed values.
- The alarm is reset on reading.
Signed-off-by: Thomas Marangoni <Thomas.Marangoni@becom-group.com>
---
drivers/hwmon/Kconfig | 10 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/tids.c | 447 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 458 insertions(+)
create mode 100644 drivers/hwmon/tids.c
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 157678b821fc..2737350bb661 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -2368,6 +2368,16 @@ config SENSORS_THMC50
This driver can also be built as a module. If so, the module
will be called thmc50.
+config SENSORS_TIDS
+ tristate "TIDS"
+ depends on I2C
+ help
+ If you say yes here you get support for the temperature
+ sensor WSEN TIDS from Würth Elektronik.
+
+ This driver can also be built as a module. If so, the module
+ will be called tids.
+
config SENSORS_TMP102
tristate "Texas Instruments TMP102"
depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index eade8e3b1bde..4eb77be3df67 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -227,6 +227,7 @@ obj-$(CONFIG_SENSORS_SY7636A) += sy7636a-hwmon.o
obj-$(CONFIG_SENSORS_AMC6821) += amc6821.o
obj-$(CONFIG_SENSORS_TC74) += tc74.o
obj-$(CONFIG_SENSORS_THMC50) += thmc50.o
+obj-$(CONFIG_SENSORS_TIDS) += tids.o
obj-$(CONFIG_SENSORS_TMP102) += tmp102.o
obj-$(CONFIG_SENSORS_TMP103) += tmp103.o
obj-$(CONFIG_SENSORS_TMP108) += tmp108.o
diff --git a/drivers/hwmon/tids.c b/drivers/hwmon/tids.c
new file mode 100644
index 000000000000..62e778202a5f
--- /dev/null
+++ b/drivers/hwmon/tids.c
@@ -0,0 +1,447 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/*
+ * Copyright (c) BECOM Electronics GmbH
+ *
+ * wsen_tids.c - Linux hwmon driver for WSEN-TIDS Temperature sensor
+ *
+ * Author: Thomas Marangoni <thomas.marangoni@becom-group.com>
+ */
+
+#include <linux/util_macros.h>
+#include <linux/regmap.h>
+#include <linux/minmax.h>
+#include <linux/hwmon.h>
+#include <linux/bits.h>
+#include <linux/math.h>
+#include <linux/i2c.h>
+
+/*
+ * TIDS registers
+ */
+#define TIDS_REG_DEVICE_ID 0x01
+#define TIDS_REG_T_H_LIMIT 0x02
+#define TIDS_REG_T_L_LIMIT 0x03
+#define TIDS_REG_CTRL 0x04
+#define TIDS_REG_STATUS 0x05
+#define TIDS_REG_DATA_T_L 0x06
+#define TIDS_REG_DATA_T_H 0x07
+#define TIDS_REG_SOFT_REST 0x0C
+
+#define TIDS_CTRL_ONE_SHOT_MASK BIT(0)
+#define TIDS_CTRL_FREERUN_MASK BIT(2)
+#define TIDS_CTRL_IF_ADD_INC_MASK BIT(3)
+#define TIDS_CTRL_AVG_MASK GENMASK(5, 4)
+#define TIDS_CTRL_AVG_SHIFT 4
+#define TIDS_CTRL_BDU_MASK BIT(6)
+
+#define TIDS_STATUS_BUSY_MASK BIT(0)
+#define TIDS_STATUS_OVER_THL_MASK BIT(1)
+#define TIDS_STATUS_UNDER_TLL_MASK BIT(2)
+
+#define TIDS_SOFT_REST_MASK BIT(1)
+
+/*
+ * TIDS device IDs
+ */
+#define TIDS_ID 0xa0
+
+struct tids_data {
+ struct i2c_client *client;
+
+ struct regmap *regmap;
+
+ int irq;
+ int temperature;
+};
+
+static u8 update_intervals[] = { 40, 20, 10, 5 };
+
+static ssize_t tids_interval_read(struct device *dev, long *val)
+{
+ struct tids_data *data = dev_get_drvdata(dev);
+ unsigned int avg_value = 0;
+ int ret;
+
+ ret = regmap_read(data->regmap, TIDS_REG_CTRL, &avg_value);
+ if (ret < 0)
+ return ret;
+
+ avg_value = (avg_value & TIDS_CTRL_AVG_MASK) >> TIDS_CTRL_AVG_SHIFT;
+
+ *val = update_intervals[avg_value];
+
+ return 0;
+}
+
+static ssize_t tids_interval_write(struct device *dev, long val)
+{
+ struct tids_data *data = dev_get_drvdata(dev);
+ unsigned int avg_value;
+
+ avg_value = find_closest_descending(val, update_intervals,
+ ARRAY_SIZE(update_intervals));
+
+ return regmap_write_bits(data->regmap, TIDS_REG_CTRL,
+ TIDS_CTRL_AVG_MASK,
+ avg_value << TIDS_CTRL_AVG_SHIFT);
+}
+
+static int tids_temperature1_read(struct device *dev, long *val)
+{
+ struct tids_data *data = dev_get_drvdata(dev);
+ u8 buf[2] = { 0 };
+ int ret;
+
+ ret = regmap_bulk_read(data->regmap, TIDS_REG_DATA_T_L, buf, 2);
+ if (ret < 0)
+ return ret;
+
+ /* temperature in °mC */
+ *val = (((s16)(buf[1] << 8) | buf[0])) * 10;
+
+ return 0;
+}
+
+static ssize_t tids_temperature_alarm_read(struct device *dev, u32 attr,
+ long *val)
+{
+ struct tids_data *data = dev_get_drvdata(dev);
+ int ret;
+
+ if (attr == hwmon_temp_min_alarm)
+ ret = regmap_test_bits(data->regmap, TIDS_REG_STATUS,
+ TIDS_STATUS_UNDER_TLL_MASK);
+ else if (attr == hwmon_temp_max_alarm)
+ ret = regmap_test_bits(data->regmap, TIDS_REG_STATUS,
+ TIDS_STATUS_OVER_THL_MASK);
+ else
+ return -EOPNOTSUPP;
+
+ if (ret < 0)
+ return ret;
+
+ *val = ret;
+
+ return 0;
+}
+
+static int tids_temperature_minmax_read(struct device *dev, u32 attr, long *val)
+{
+ struct tids_data *data = dev_get_drvdata(dev);
+ unsigned int reg_data = 0;
+ int ret;
+
+ if (attr == hwmon_temp_min)
+ ret = regmap_read(data->regmap, TIDS_REG_T_L_LIMIT, ®_data);
+ else if (attr == hwmon_temp_max)
+ ret = regmap_read(data->regmap, TIDS_REG_T_H_LIMIT, ®_data);
+ else
+ return -EOPNOTSUPP;
+
+ if (ret < 0)
+ return ret;
+
+ /* temperature from register conversion in °mC */
+ *val = (((u8)reg_data - 63) * 640);
+
+ return 0;
+}
+
+static ssize_t tids_temperature_minmax_write(struct device *dev, u32 attr,
+ long val)
+{
+ struct tids_data *data = dev_get_drvdata(dev);
+ u8 reg_data;
+
+ /* temperature in °mC */
+ val = clamp_val(val, -39680, 122880);
+ /* temperature to register conversion in °mC */
+ reg_data = (u8)(DIV_ROUND_CLOSEST(val, 640) + 63);
+
+ if (attr == hwmon_temp_min)
+ return regmap_write(data->regmap, TIDS_REG_T_L_LIMIT, reg_data);
+ else if (attr == hwmon_temp_max)
+ return regmap_write(data->regmap, TIDS_REG_T_H_LIMIT, reg_data);
+ else
+ return -EOPNOTSUPP;
+}
+
+static umode_t tids_hwmon_visible(const void *data,
+ enum hwmon_sensor_types type, u32 attr,
+ int channel)
+{
+ umode_t mode = 0;
+
+ switch (type) {
+ case hwmon_temp:
+ switch (attr) {
+ case hwmon_temp_input:
+ case hwmon_temp_min_alarm:
+ case hwmon_temp_max_alarm:
+ mode = 0444;
+ break;
+ case hwmon_temp_min:
+ case hwmon_temp_max:
+ mode = 0644;
+ break;
+ default:
+ break;
+ }
+ break;
+ case hwmon_chip:
+ switch (attr) {
+ case hwmon_chip_update_interval:
+ mode = 0644;
+ break;
+ default:
+ break;
+ }
+ break;
+ default:
+ break;
+ }
+
+ return mode;
+}
+
+static int tids_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
+ u32 attr, int channel, long *val)
+{
+ switch (type) {
+ case hwmon_temp:
+ switch (attr) {
+ case hwmon_temp_input:
+ return tids_temperature1_read(dev, val);
+ case hwmon_temp_min_alarm:
+ case hwmon_temp_max_alarm:
+ return tids_temperature_alarm_read(dev, attr, val);
+ case hwmon_temp_min:
+ case hwmon_temp_max:
+ return tids_temperature_minmax_read(dev, attr, val);
+ default:
+ return -EOPNOTSUPP;
+ }
+ case hwmon_chip:
+ switch (attr) {
+ case hwmon_chip_update_interval:
+ return tids_interval_read(dev, val);
+ default:
+ return -EOPNOTSUPP;
+ }
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static int tids_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
+ u32 attr, int channel, long val)
+{
+ switch (type) {
+ case hwmon_temp:
+ switch (attr) {
+ case hwmon_temp_min:
+ case hwmon_temp_max:
+ return tids_temperature_minmax_write(dev, attr, val);
+ default:
+ return -EOPNOTSUPP;
+ }
+ case hwmon_chip:
+ switch (attr) {
+ case hwmon_chip_update_interval:
+ return tids_interval_write(dev, val);
+ default:
+ return -EOPNOTSUPP;
+ }
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static const struct hwmon_channel_info *const tids_info[] = {
+ HWMON_CHANNEL_INFO(chip, HWMON_C_UPDATE_INTERVAL),
+ HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT | HWMON_T_MIN_ALARM |
+ HWMON_T_MAX_ALARM | HWMON_T_MIN | HWMON_T_MAX),
+ NULL
+};
+
+static const struct hwmon_ops tids_ops = {
+ .is_visible = tids_hwmon_visible,
+ .read = tids_hwmon_read,
+ .write = tids_hwmon_write,
+};
+
+static const struct hwmon_chip_info tids_chip_info = {
+ .ops = &tids_ops,
+ .info = tids_info,
+};
+
+static bool tids_regmap_writeable_reg(struct device *dev, unsigned int reg)
+{
+ if (reg >= TIDS_REG_T_H_LIMIT && reg <= TIDS_REG_CTRL)
+ return true;
+
+ if (reg == TIDS_REG_SOFT_REST)
+ return true;
+
+ return false;
+}
+
+static bool tids_regmap_readable_reg(struct device *dev, unsigned int reg)
+{
+ if (reg >= TIDS_REG_DEVICE_ID && reg <= TIDS_REG_DATA_T_H)
+ return true;
+
+ if (reg == TIDS_REG_SOFT_REST)
+ return true;
+
+ return false;
+}
+
+static bool tids_regmap_volatile_reg(struct device *dev, unsigned int reg)
+{
+ if (reg >= TIDS_REG_STATUS && reg <= TIDS_REG_DATA_T_H)
+ return true;
+
+ if (reg == TIDS_REG_SOFT_REST)
+ return true;
+
+ return false;
+}
+
+static const struct regmap_config regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = TIDS_REG_SOFT_REST,
+ .writeable_reg = tids_regmap_writeable_reg,
+ .readable_reg = tids_regmap_readable_reg,
+ .volatile_reg = tids_regmap_volatile_reg,
+ .use_single_read = 0,
+};
+
+static int tids_init(struct tids_data *data)
+{
+ int ret;
+
+ /* Triggering soft reset */
+ ret = regmap_write_bits(data->regmap, TIDS_REG_SOFT_REST,
+ TIDS_SOFT_REST_MASK, TIDS_SOFT_REST_MASK);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_clear_bits(data->regmap, TIDS_REG_SOFT_REST,
+ TIDS_SOFT_REST_MASK);
+ if (ret < 0)
+ return ret;
+
+ /* Allowing bulk read */
+ ret = regmap_write_bits(data->regmap, TIDS_REG_CTRL,
+ TIDS_CTRL_IF_ADD_INC_MASK,
+ TIDS_CTRL_IF_ADD_INC_MASK);
+ if (ret < 0)
+ return ret;
+
+ /* Set meassurement interval */
+ ret = regmap_clear_bits(data->regmap, TIDS_REG_CTRL,
+ TIDS_CTRL_AVG_MASK);
+ if (ret < 0)
+ return ret;
+
+ /* Set device to free run mode */
+ ret = regmap_write_bits(data->regmap, TIDS_REG_CTRL,
+ TIDS_CTRL_FREERUN_MASK, TIDS_CTRL_FREERUN_MASK);
+ if (ret < 0)
+ return ret;
+
+ /* Don't update temperature register until high and low value are read */
+ ret = regmap_write_bits(data->regmap, TIDS_REG_CTRL, TIDS_CTRL_BDU_MASK,
+ TIDS_CTRL_BDU_MASK);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+static int tids_probe(struct i2c_client *client)
+{
+ struct device *device = &client->dev;
+ struct device *hwmon_dev;
+ struct tids_data *data;
+ unsigned int value;
+ int ret;
+
+ data = devm_kzalloc(device, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ data->client = client;
+
+ /* Init regmap */
+ data->regmap = devm_regmap_init_i2c(data->client, ®map_config);
+ if (IS_ERR(data->regmap))
+ return dev_err_probe(device, PTR_ERR(data->regmap),
+ "regmap initialization failed\n");
+
+ /* Read device id, to check if i2c is working */
+ ret = regmap_read(data->regmap, TIDS_REG_DEVICE_ID, &value);
+ if (ret < 0)
+ return ret;
+
+ if (value != TIDS_ID)
+ return -ENODEV;
+
+ tids_init(data);
+
+ hwmon_dev = devm_hwmon_device_register_with_info(device, "tids", data,
+ &tids_chip_info, NULL);
+
+ return PTR_ERR_OR_ZERO(hwmon_dev);
+}
+
+static int tids_suspend(struct device *dev)
+{
+ struct tids_data *data = dev_get_drvdata(dev);
+
+ return regmap_clear_bits(data->regmap, TIDS_REG_CTRL,
+ TIDS_CTRL_FREERUN_MASK);
+}
+
+static int tids_resume(struct device *dev)
+{
+ struct tids_data *data = dev_get_drvdata(dev);
+
+ return regmap_write_bits(data->regmap, TIDS_REG_CTRL,
+ TIDS_CTRL_FREERUN_MASK,
+ TIDS_CTRL_FREERUN_MASK);
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(tids_dev_pm_ops, tids_resume, tids_suspend);
+
+static const struct i2c_device_id tids_id[] = {
+ { "tids", 0 },
+ {},
+};
+MODULE_DEVICE_TABLE(i2c, tids_id);
+
+static const struct of_device_id tids_of_match[] = {
+ { .compatible = "wsen,tids" },
+ {}
+};
+MODULE_DEVICE_TABLE(of, tids_of_match);
+
+static struct i2c_driver tids_driver = {
+ .class = I2C_CLASS_HWMON,
+ .driver = {
+ .name = "tids",
+ .pm = pm_sleep_ptr(&tids_dev_pm_ops),
+ .of_match_table = tids_of_match,
+ },
+ .probe = tids_probe,
+ .id_table = tids_id,
+};
+
+module_i2c_driver(tids_driver);
+
+MODULE_AUTHOR("Thomas Marangoni <Thomas.Marangoni@becom-group.com>");
+MODULE_DESCRIPTION("WSEN TIDS temperature sensor driver");
+MODULE_LICENSE("GPL");
--
2.51.1
On 11/19/25 04:51, Thomas Marangoni wrote:
> Add support for the wsen tids. It is a low cost
> and small-form-factor i2c temperature sensor.
>
Additional feedback:
> +
> +static ssize_t tids_interval_write(struct device *dev, long val)
> +{
> + struct tids_data *data = dev_get_drvdata(dev);
> + unsigned int avg_value;
> +
> + avg_value = find_closest_descending(val, update_intervals,
> + ARRAY_SIZE(update_intervals));
> +
Turns out find_closest_descending() can not handle large negative values
(close to the limit) correctly. val needs to be clamped to a reasonable range
(say, [0, 100]) before passing it to find_closest_descending().
> + return regmap_write_bits(data->regmap, TIDS_REG_CTRL,
> + TIDS_CTRL_AVG_MASK,
> + avg_value << TIDS_CTRL_AVG_SHIFT);
> +}
> +
> +static int tids_temperature1_read(struct device *dev, long *val)
The "1" in the function name is really not needed here.
> +{
> + struct tids_data *data = dev_get_drvdata(dev);
> + u8 buf[2] = { 0 };
> + int ret;
> +
> + ret = regmap_bulk_read(data->regmap, TIDS_REG_DATA_T_L, buf, 2);
> + if (ret < 0)
> + return ret;
> +
> + /* temperature in °mC */
> + *val = (((s16)(buf[1] << 8) | buf[0])) * 10;
> +
> + return 0;
> +}
> +
> +static ssize_t tids_temperature_alarm_read(struct device *dev, u32 attr,
> + long *val)
> +{
> + struct tids_data *data = dev_get_drvdata(dev);
> + int ret;
> +
> + if (attr == hwmon_temp_min_alarm)
> + ret = regmap_test_bits(data->regmap, TIDS_REG_STATUS,
> + TIDS_STATUS_UNDER_TLL_MASK);
> + else if (attr == hwmon_temp_max_alarm)
> + ret = regmap_test_bits(data->regmap, TIDS_REG_STATUS,
> + TIDS_STATUS_OVER_THL_MASK);
> + else
> + return -EOPNOTSUPP;
> +
> + if (ret < 0)
> + return ret;
> +
> + *val = ret;
> +
> + return 0;
> +}
> +
> +static int tids_temperature_minmax_read(struct device *dev, u32 attr, long *val)
> +{
> + struct tids_data *data = dev_get_drvdata(dev);
> + unsigned int reg_data = 0;
> + int ret;
> +
> + if (attr == hwmon_temp_min)
> + ret = regmap_read(data->regmap, TIDS_REG_T_L_LIMIT, ®_data);
> + else if (attr == hwmon_temp_max)
> + ret = regmap_read(data->regmap, TIDS_REG_T_H_LIMIT, ®_data);
> + else
> + return -EOPNOTSUPP;
> +
> + if (ret < 0)
> + return ret;
> +
> + /* temperature from register conversion in °mC */
> + *val = (((u8)reg_data - 63) * 640);
> +
> + return 0;
> +}
> +
> +static ssize_t tids_temperature_minmax_write(struct device *dev, u32 attr,
> + long val)
> +{
> + struct tids_data *data = dev_get_drvdata(dev);
> + u8 reg_data;
> +
> + /* temperature in °mC */
> + val = clamp_val(val, -39680, 122880);
(0 - 63) * 640 = -40320
While this is a bit below the "official" limit, it is the default value in
the chip register. Writing a limit that is read from the chip should be supported,
so the range should be clamped to [-40320, 122880].
Thanks,
Guenter
Hi Thomas,
I'm a bit surprised why you have me on Cc. Since I'm here, let me give some inputs...
On Wed, 2025-11-19 at 13:51 +0100, Thomas Marangoni wrote:
> Add support for the wsen tids. It is a low cost
> and small-form-factor i2c temperature sensor.
>
> It supports the following features:
> - Continuous temperature reading in four intervals: 5 ms, 10 ms,
> 20 ms and 40 ms.
> - Low temperature alarm
> - High temperature alarm
>
> The driver supports following hwmon features:
> - hwmon_temp_input
> - hwmon_temp_min_alarm
> - hwmon_temp_max_alarm
> - hwmon_temp_min
> - hwmon_temp_max
> - hwmon_chip_update_interval
>
> Additional notes:
> - The update interval only supports four fixed values.
> - The alarm is reset on reading.
>
> Signed-off-by: Thomas Marangoni <Thomas.Marangoni@becom-group.com>
> ---
> drivers/hwmon/Kconfig | 10 +
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/tids.c | 447 +++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 458 insertions(+)
> create mode 100644 drivers/hwmon/tids.c
>
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 157678b821fc..2737350bb661 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -2368,6 +2368,16 @@ config SENSORS_THMC50
> This driver can also be built as a module. If so, the module
> will be called thmc50.
>
> +config SENSORS_TIDS
> + tristate "TIDS"
> + depends on I2C
> + help
> + If you say yes here you get support for the temperature
> + sensor WSEN TIDS from Würth Elektronik.
> +
> + This driver can also be built as a module. If so, the module
> + will be called tids.
> +
> config SENSORS_TMP102
> tristate "Texas Instruments TMP102"
> depends on I2C
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index eade8e3b1bde..4eb77be3df67 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -227,6 +227,7 @@ obj-$(CONFIG_SENSORS_SY7636A) += sy7636a-hwmon.o
> obj-$(CONFIG_SENSORS_AMC6821) += amc6821.o
> obj-$(CONFIG_SENSORS_TC74) += tc74.o
> obj-$(CONFIG_SENSORS_THMC50) += thmc50.o
> +obj-$(CONFIG_SENSORS_TIDS) += tids.o
> obj-$(CONFIG_SENSORS_TMP102) += tmp102.o
> obj-$(CONFIG_SENSORS_TMP103) += tmp103.o
> obj-$(CONFIG_SENSORS_TMP108) += tmp108.o
> diff --git a/drivers/hwmon/tids.c b/drivers/hwmon/tids.c
> new file mode 100644
> index 000000000000..62e778202a5f
> --- /dev/null
> +++ b/drivers/hwmon/tids.c
> @@ -0,0 +1,447 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +/*
> + * Copyright (c) BECOM Electronics GmbH
> + *
> + * wsen_tids.c - Linux hwmon driver for WSEN-TIDS Temperature sensor
> + *
> + * Author: Thomas Marangoni <thomas.marangoni@becom-group.com>
> + */
> +
> +#include <linux/util_macros.h>
> +#include <linux/regmap.h>
> +#include <linux/minmax.h>
> +#include <linux/hwmon.h>
> +#include <linux/bits.h>
> +#include <linux/math.h>
> +#include <linux/i2c.h>
> +
> +/*
> + * TIDS registers
> + */
> +#define TIDS_REG_DEVICE_ID 0x01
> +#define TIDS_REG_T_H_LIMIT 0x02
> +#define TIDS_REG_T_L_LIMIT 0x03
> +#define TIDS_REG_CTRL 0x04
> +#define TIDS_REG_STATUS 0x05
> +#define TIDS_REG_DATA_T_L 0x06
> +#define TIDS_REG_DATA_T_H 0x07
> +#define TIDS_REG_SOFT_REST 0x0C
> +
> +#define TIDS_CTRL_ONE_SHOT_MASK BIT(0)
> +#define TIDS_CTRL_FREERUN_MASK BIT(2)
> +#define TIDS_CTRL_IF_ADD_INC_MASK BIT(3)
> +#define TIDS_CTRL_AVG_MASK GENMASK(5, 4)
> +#define TIDS_CTRL_AVG_SHIFT 4
> +#define TIDS_CTRL_BDU_MASK BIT(6)
> +
> +#define TIDS_STATUS_BUSY_MASK BIT(0)
> +#define TIDS_STATUS_OVER_THL_MASK BIT(1)
> +#define TIDS_STATUS_UNDER_TLL_MASK BIT(2)
> +
> +#define TIDS_SOFT_REST_MASK BIT(1)
> +
> +/*
> + * TIDS device IDs
> + */
> +#define TIDS_ID 0xa0
> +
> +struct tids_data {
> + struct i2c_client *client;
> +
> + struct regmap *regmap;
> +
> + int irq;
> + int temperature;
> +};
> +
> +static u8 update_intervals[] = { 40, 20, 10, 5 };
static const?
> +
> +static ssize_t tids_interval_read(struct device *dev, long *val)
> +{
> + struct tids_data *data = dev_get_drvdata(dev);
> + unsigned int avg_value = 0;
> + int ret;
> +
> + ret = regmap_read(data->regmap, TIDS_REG_CTRL, &avg_value);
> + if (ret < 0)
> + return ret;
> +
> + avg_value = (avg_value & TIDS_CTRL_AVG_MASK) >> TIDS_CTRL_AVG_SHIFT;
> +
> + *val = update_intervals[avg_value];
> +
> + return 0;
> +}
> +
> +static ssize_t tids_interval_write(struct device *dev, long val)
> +{
> + struct tids_data *data = dev_get_drvdata(dev);
> + unsigned int avg_value;
> +
> + avg_value = find_closest_descending(val, update_intervals,
> + ARRAY_SIZE(update_intervals));
> +
> + return regmap_write_bits(data->regmap, TIDS_REG_CTRL,
> + TIDS_CTRL_AVG_MASK,
> + avg_value << TIDS_CTRL_AVG_SHIFT);
> +}
> +
> +static int tids_temperature1_read(struct device *dev, long *val)
> +{
> + struct tids_data *data = dev_get_drvdata(dev);
> + u8 buf[2] = { 0 };
Seems like __le16?
> + int ret;
> +
> + ret = regmap_bulk_read(data->regmap, TIDS_REG_DATA_T_L, buf, 2);
> + if (ret < 0)
> + return ret;
> +
> + /* temperature in °mC */
> + *val = (((s16)(buf[1] << 8) | buf[0])) * 10;
Then __le16_to_cpu()?
> +
> + return 0;
> +}
> +
> +static ssize_t tids_temperature_alarm_read(struct device *dev, u32 attr,
> + long *val)
> +{
> + struct tids_data *data = dev_get_drvdata(dev);
> + int ret;
> +
> + if (attr == hwmon_temp_min_alarm)
> + ret = regmap_test_bits(data->regmap, TIDS_REG_STATUS,
> + TIDS_STATUS_UNDER_TLL_MASK);
> + else if (attr == hwmon_temp_max_alarm)
> + ret = regmap_test_bits(data->regmap, TIDS_REG_STATUS,
> + TIDS_STATUS_OVER_THL_MASK);
Instead of passing attr and have this if() else why not passing the proper mask? Then
just regmap_read(regmag, reg, ...)?
> + else
> + return -EOPNOTSUPP;
> +
> + if (ret < 0)
> + return ret;
> +
> + *val = ret;
> +
> + return 0;
> +}
> +
> +static int tids_temperature_minmax_read(struct device *dev, u32 attr, long *val)
> +{
> + struct tids_data *data = dev_get_drvdata(dev);
> + unsigned int reg_data = 0;
> + int ret;
> +
> + if (attr == hwmon_temp_min)
> + ret = regmap_read(data->regmap, TIDS_REG_T_L_LIMIT, ®_data);
> + else if (attr == hwmon_temp_max)
> + ret = regmap_read(data->regmap, TIDS_REG_T_H_LIMIT, ®_data);
> + else
> + return -EOPNOTSUPP;
Same as above but with the proper register
> +
> + if (ret < 0)
> + return ret;
> +
> + /* temperature from register conversion in °mC */
> + *val = (((u8)reg_data - 63) * 640);
Why the cast?
> +
> + return 0;
> +}
> +
> +static ssize_t tids_temperature_minmax_write(struct device *dev, u32 attr,
> + long val)
> +{
> + struct tids_data *data = dev_get_drvdata(dev);
> + u8 reg_data;
> +
> + /* temperature in °mC */
> + val = clamp_val(val, -39680, 122880);
> + /* temperature to register conversion in °mC */
> + reg_data = (u8)(DIV_ROUND_CLOSEST(val, 640) + 63);
> +
> + if (attr == hwmon_temp_min)
> + return regmap_write(data->regmap, TIDS_REG_T_L_LIMIT, reg_data);
> + else if (attr == hwmon_temp_max)
> + return regmap_write(data->regmap, TIDS_REG_T_H_LIMIT, reg_data);
> + else
> + return -EOPNOTSUPP;
Redundant else if () and else
...
>
> +
> +static int tids_init(struct tids_data *data)
> +{
> + int ret;
> +
> + /* Triggering soft reset */
> + ret = regmap_write_bits(data->regmap, TIDS_REG_SOFT_REST,
> + TIDS_SOFT_REST_MASK, TIDS_SOFT_REST_MASK);
> + if (ret < 0)
> + return ret;
> +
No need for sleep some time? Typically that's defined on the datasheet.
> + ret = regmap_clear_bits(data->regmap, TIDS_REG_SOFT_REST,
> + TIDS_SOFT_REST_MASK);
> + if (ret < 0)
> + return ret;
> +
> + /* Allowing bulk read */
> + ret = regmap_write_bits(data->regmap, TIDS_REG_CTRL,
> + TIDS_CTRL_IF_ADD_INC_MASK,
> + TIDS_CTRL_IF_ADD_INC_MASK);
> + if (ret < 0)
> + return ret;
> +
> + /* Set meassurement interval */
> + ret = regmap_clear_bits(data->regmap, TIDS_REG_CTRL,
> + TIDS_CTRL_AVG_MASK);
> + if (ret < 0)
> + return ret;
> +
> + /* Set device to free run mode */
> + ret = regmap_write_bits(data->regmap, TIDS_REG_CTRL,
> + TIDS_CTRL_FREERUN_MASK, TIDS_CTRL_FREERUN_MASK);
> + if (ret < 0)
> + return ret;
> +
> + /* Don't update temperature register until high and low value are read */
> + ret = regmap_write_bits(data->regmap, TIDS_REG_CTRL, TIDS_CTRL_BDU_MASK,
> + TIDS_CTRL_BDU_MASK);
return regmap_write_bits();
> + if (ret < 0)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int tids_probe(struct i2c_client *client)
> +{
> + struct device *device = &client->dev;
> + struct device *hwmon_dev;
> + struct tids_data *data;
> + unsigned int value;
> + int ret;
> +
> + data = devm_kzalloc(device, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + data->client = client;
> +
> + /* Init regmap */
The comment does not add any added value.
> + data->regmap = devm_regmap_init_i2c(data->client, ®map_config);
> + if (IS_ERR(data->regmap))
> + return dev_err_probe(device, PTR_ERR(data->regmap),
> + "regmap initialization failed\n");
> +
> + /* Read device id, to check if i2c is working */
Same
> + ret = regmap_read(data->regmap, TIDS_REG_DEVICE_ID, &value);
> + if (ret < 0)
> + return ret;
> +
> + if (value != TIDS_ID)
> + return -ENODEV;
> +
> + tids_init(data);
Check for the return value.
> +
> + hwmon_dev = devm_hwmon_device_register_with_info(device, "tids", data,
> + &tids_chip_info, NULL);
> +
> + return PTR_ERR_OR_ZERO(hwmon_dev);
> +}
> +
> +static int tids_suspend(struct device *dev)
> +{
> + struct tids_data *data = dev_get_drvdata(dev);
> +
> + return regmap_clear_bits(data->regmap, TIDS_REG_CTRL,
> + TIDS_CTRL_FREERUN_MASK);
> +}
> +
> +static int tids_resume(struct device *dev)
> +{
> + struct tids_data *data = dev_get_drvdata(dev);
> +
> + return regmap_write_bits(data->regmap, TIDS_REG_CTRL,
> + TIDS_CTRL_FREERUN_MASK,
> + TIDS_CTRL_FREERUN_MASK);
> +}
> +
> +static DEFINE_SIMPLE_DEV_PM_OPS(tids_dev_pm_ops, tids_resume, tids_suspend);
> +
> +static const struct i2c_device_id tids_id[] = {
> + { "tids", 0 },
No need for 0
> + {},
The above is already a terminator so you can drop the comma
- Nuno Sá
On 11/19/25 07:05, Nuno Sá wrote:
> Hi Thomas,
>
> I'm a bit surprised why you have me on Cc. Since I'm here, let me give some inputs...
>
Excellent feedback though. Some more comments inline.
> On Wed, 2025-11-19 at 13:51 +0100, Thomas Marangoni wrote:
>> Add support for the wsen tids. It is a low cost
>> and small-form-factor i2c temperature sensor.
>>
>> It supports the following features:
>> - Continuous temperature reading in four intervals: 5 ms, 10 ms,
>> 20 ms and 40 ms.
>> - Low temperature alarm
>> - High temperature alarm
>>
>> The driver supports following hwmon features:
>> - hwmon_temp_input
>> - hwmon_temp_min_alarm
>> - hwmon_temp_max_alarm
>> - hwmon_temp_min
>> - hwmon_temp_max
>> - hwmon_chip_update_interval
>>
>> Additional notes:
>> - The update interval only supports four fixed values.
>> - The alarm is reset on reading.
>>
>> Signed-off-by: Thomas Marangoni <Thomas.Marangoni@becom-group.com>
>> ---
>> drivers/hwmon/Kconfig | 10 +
>> drivers/hwmon/Makefile | 1 +
>> drivers/hwmon/tids.c | 447 +++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 458 insertions(+)
>> create mode 100644 drivers/hwmon/tids.c
>>
>> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
>> index 157678b821fc..2737350bb661 100644
>> --- a/drivers/hwmon/Kconfig
>> +++ b/drivers/hwmon/Kconfig
>> @@ -2368,6 +2368,16 @@ config SENSORS_THMC50
>> This driver can also be built as a module. If so, the module
>> will be called thmc50.
>>
>> +config SENSORS_TIDS
>> + tristate "TIDS"
>> + depends on I2C
>> + help
>> + If you say yes here you get support for the temperature
>> + sensor WSEN TIDS from Würth Elektronik.
>> +
>> + This driver can also be built as a module. If so, the module
>> + will be called tids.
>> +
>> config SENSORS_TMP102
>> tristate "Texas Instruments TMP102"
>> depends on I2C
>> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
>> index eade8e3b1bde..4eb77be3df67 100644
>> --- a/drivers/hwmon/Makefile
>> +++ b/drivers/hwmon/Makefile
>> @@ -227,6 +227,7 @@ obj-$(CONFIG_SENSORS_SY7636A) += sy7636a-hwmon.o
>> obj-$(CONFIG_SENSORS_AMC6821) += amc6821.o
>> obj-$(CONFIG_SENSORS_TC74) += tc74.o
>> obj-$(CONFIG_SENSORS_THMC50) += thmc50.o
>> +obj-$(CONFIG_SENSORS_TIDS) += tids.o
>> obj-$(CONFIG_SENSORS_TMP102) += tmp102.o
>> obj-$(CONFIG_SENSORS_TMP103) += tmp103.o
>> obj-$(CONFIG_SENSORS_TMP108) += tmp108.o
>> diff --git a/drivers/hwmon/tids.c b/drivers/hwmon/tids.c
>> new file mode 100644
>> index 000000000000..62e778202a5f
>> --- /dev/null
>> +++ b/drivers/hwmon/tids.c
>> @@ -0,0 +1,447 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +
>> +/*
>> + * Copyright (c) BECOM Electronics GmbH
>> + *
>> + * wsen_tids.c - Linux hwmon driver for WSEN-TIDS Temperature sensor
>> + *
>> + * Author: Thomas Marangoni <thomas.marangoni@becom-group.com>
>> + */
>> +
>> +#include <linux/util_macros.h>
>> +#include <linux/regmap.h>
>> +#include <linux/minmax.h>
>> +#include <linux/hwmon.h>
>> +#include <linux/bits.h>
>> +#include <linux/math.h>
>> +#include <linux/i2c.h>
Alphabetic order please.
>> +
>> +/*
>> + * TIDS registers
>> + */
>> +#define TIDS_REG_DEVICE_ID 0x01
>> +#define TIDS_REG_T_H_LIMIT 0x02
>> +#define TIDS_REG_T_L_LIMIT 0x03
>> +#define TIDS_REG_CTRL 0x04
>> +#define TIDS_REG_STATUS 0x05
>> +#define TIDS_REG_DATA_T_L 0x06
>> +#define TIDS_REG_DATA_T_H 0x07
>> +#define TIDS_REG_SOFT_REST 0x0C
>> +
>> +#define TIDS_CTRL_ONE_SHOT_MASK BIT(0)
>> +#define TIDS_CTRL_FREERUN_MASK BIT(2)
>> +#define TIDS_CTRL_IF_ADD_INC_MASK BIT(3)
>> +#define TIDS_CTRL_AVG_MASK GENMASK(5, 4)
>> +#define TIDS_CTRL_AVG_SHIFT 4
>> +#define TIDS_CTRL_BDU_MASK BIT(6)
>> +
>> +#define TIDS_STATUS_BUSY_MASK BIT(0)
>> +#define TIDS_STATUS_OVER_THL_MASK BIT(1)
>> +#define TIDS_STATUS_UNDER_TLL_MASK BIT(2)
>> +
>> +#define TIDS_SOFT_REST_MASK BIT(1)
>> +
>> +/*
>> + * TIDS device IDs
>> + */
>> +#define TIDS_ID 0xa0
>> +
>> +struct tids_data {
>> + struct i2c_client *client;
>> +
Unused
>> + struct regmap *regmap;
>> +
>> + int irq;
Unused
>> + int temperature;
Unused
>> +};
>> +
>> +static u8 update_intervals[] = { 40, 20, 10, 5 };
>
> static const?
>
>> +
>> +static ssize_t tids_interval_read(struct device *dev, long *val)
>> +{
>> + struct tids_data *data = dev_get_drvdata(dev);
>> + unsigned int avg_value = 0;
Unnecessary initialization.
>> + int ret;
>> +
>> + ret = regmap_read(data->regmap, TIDS_REG_CTRL, &avg_value);
>> + if (ret < 0)
>> + return ret;
>> +
>> + avg_value = (avg_value & TIDS_CTRL_AVG_MASK) >> TIDS_CTRL_AVG_SHIFT;
>> +
>> + *val = update_intervals[avg_value];
>> +
>> + return 0;
>> +}
>> +
>> +static ssize_t tids_interval_write(struct device *dev, long val)
>> +{
>> + struct tids_data *data = dev_get_drvdata(dev);
>> + unsigned int avg_value;
>> +
>> + avg_value = find_closest_descending(val, update_intervals,
>> + ARRAY_SIZE(update_intervals));
>> +
>> + return regmap_write_bits(data->regmap, TIDS_REG_CTRL,
>> + TIDS_CTRL_AVG_MASK,
>> + avg_value << TIDS_CTRL_AVG_SHIFT);
>> +}
>> +
>> +static int tids_temperature1_read(struct device *dev, long *val)
>> +{
>> + struct tids_data *data = dev_get_drvdata(dev);
>> + u8 buf[2] = { 0 };
>
> Seems like __le16?
Also, the initialization is unnecessary.
>> + int ret;
>> +
>> + ret = regmap_bulk_read(data->regmap, TIDS_REG_DATA_T_L, buf, 2);
>> + if (ret < 0)
>> + return ret;
>> +
>> + /* temperature in °mC */
>> + *val = (((s16)(buf[1] << 8) | buf[0])) * 10;
>
> Then __le16_to_cpu()?
>
>> +
>> + return 0;
>> +}
>> +
>> +static ssize_t tids_temperature_alarm_read(struct device *dev, u32 attr,
>> + long *val)
>> +{
>> + struct tids_data *data = dev_get_drvdata(dev);
>> + int ret;
>> +
>> + if (attr == hwmon_temp_min_alarm)
>> + ret = regmap_test_bits(data->regmap, TIDS_REG_STATUS,
>> + TIDS_STATUS_UNDER_TLL_MASK);
>> + else if (attr == hwmon_temp_max_alarm)
>> + ret = regmap_test_bits(data->regmap, TIDS_REG_STATUS,
>> + TIDS_STATUS_OVER_THL_MASK);
>
> Instead of passing attr and have this if() else why not passing the proper mask? Then
> just regmap_read(regmag, reg, ...)?
>
>> + else
>> + return -EOPNOTSUPP;
>> +
>> + if (ret < 0)
>> + return ret;
>> +
>> + *val = ret;
>> +
>> + return 0;
>> +}
>> +
>> +static int tids_temperature_minmax_read(struct device *dev, u32 attr, long *val)
>> +{
>> + struct tids_data *data = dev_get_drvdata(dev);
>> + unsigned int reg_data = 0;
Unnecessary initialization.
>> + int ret;
>> +
>> + if (attr == hwmon_temp_min)
>> + ret = regmap_read(data->regmap, TIDS_REG_T_L_LIMIT, ®_data);
>> + else if (attr == hwmon_temp_max)
>> + ret = regmap_read(data->regmap, TIDS_REG_T_H_LIMIT, ®_data);
>> + else
>> + return -EOPNOTSUPP;
>
> Same as above but with the proper register
>
>> +
>> + if (ret < 0)
>> + return ret;
>> +
>> + /* temperature from register conversion in °mC */
>> + *val = (((u8)reg_data - 63) * 640);
>
> Why the cast?
>
It is actually worse than that: The compiler will take the expression
as unsigned and won't sign extend to negative values. So a register value of,
say, 62 will be reported as 255 degrees C. The cast needs to be to int for
this to work.
>> +
>> + return 0;
>> +}
>> +
>> +static ssize_t tids_temperature_minmax_write(struct device *dev, u32 attr,
>> + long val)
>> +{
>> + struct tids_data *data = dev_get_drvdata(dev);
>> + u8 reg_data;
>> +
>> + /* temperature in °mC */
>> + val = clamp_val(val, -39680, 122880);
>> + /* temperature to register conversion in °mC */
>> + reg_data = (u8)(DIV_ROUND_CLOSEST(val, 640) + 63);
>> +
>> + if (attr == hwmon_temp_min)
>> + return regmap_write(data->regmap, TIDS_REG_T_L_LIMIT, reg_data);
>> + else if (attr == hwmon_temp_max)
>> + return regmap_write(data->regmap, TIDS_REG_T_H_LIMIT, reg_data);
>> + else
>> + return -EOPNOTSUPP;
>
> Redundant else if () and else
>
Also, as elsewhere, better to pass the register as argument.
> ...
>
>>
>> +
>> +static int tids_init(struct tids_data *data)
>> +{
>> + int ret;
>> +
>> + /* Triggering soft reset */
>> + ret = regmap_write_bits(data->regmap, TIDS_REG_SOFT_REST,
>> + TIDS_SOFT_REST_MASK, TIDS_SOFT_REST_MASK);
>> + if (ret < 0)
>> + return ret;
>> +
>
> No need for sleep some time? Typically that's defined on the datasheet.
>
It doesn't say.
>> + ret = regmap_clear_bits(data->regmap, TIDS_REG_SOFT_REST,
>> + TIDS_SOFT_REST_MASK);
>> + if (ret < 0)
>> + return ret;
>> +
>> + /* Allowing bulk read */
>> + ret = regmap_write_bits(data->regmap, TIDS_REG_CTRL,
>> + TIDS_CTRL_IF_ADD_INC_MASK,
>> + TIDS_CTRL_IF_ADD_INC_MASK);
>> + if (ret < 0)
>> + return ret;
>> +
>> + /* Set meassurement interval */
measurement
>> + ret = regmap_clear_bits(data->regmap, TIDS_REG_CTRL,
>> + TIDS_CTRL_AVG_MASK);
>> + if (ret < 0)
>> + return ret;
>> +
>> + /* Set device to free run mode */
>> + ret = regmap_write_bits(data->regmap, TIDS_REG_CTRL,
>> + TIDS_CTRL_FREERUN_MASK, TIDS_CTRL_FREERUN_MASK);
>> + if (ret < 0)
>> + return ret;
>> +
>> + /* Don't update temperature register until high and low value are read */
>> + ret = regmap_write_bits(data->regmap, TIDS_REG_CTRL, TIDS_CTRL_BDU_MASK,
>> + TIDS_CTRL_BDU_MASK);
>
> return regmap_write_bits();
>
>> + if (ret < 0)
>> + return ret;
>> +
>> + return 0;
>> +}
>> +
>> +static int tids_probe(struct i2c_client *client)
>> +{
>> + struct device *device = &client->dev;
>> + struct device *hwmon_dev;
>> + struct tids_data *data;
>> + unsigned int value;
>> + int ret;
>> +
>> + data = devm_kzalloc(device, sizeof(*data), GFP_KERNEL);
>> + if (!data)
>> + return -ENOMEM;
>> +
>> + data->client = client;
>> +
>> + /* Init regmap */
>
> The comment does not add any added value.
>
>> + data->regmap = devm_regmap_init_i2c(data->client, ®map_config);
>> + if (IS_ERR(data->regmap))
>> + return dev_err_probe(device, PTR_ERR(data->regmap),
>> + "regmap initialization failed\n");
>> +
>> + /* Read device id, to check if i2c is working */
>
> Same
>
>> + ret = regmap_read(data->regmap, TIDS_REG_DEVICE_ID, &value);
>> + if (ret < 0)
>> + return ret;
>> +
>> + if (value != TIDS_ID)
>> + return -ENODEV;
>> +
>> + tids_init(data);
>
> Check for the return value.
>
>> +
>> + hwmon_dev = devm_hwmon_device_register_with_info(device, "tids", data,
>> + &tids_chip_info, NULL);
>> +
>> + return PTR_ERR_OR_ZERO(hwmon_dev);
>> +}
>> +
>> +static int tids_suspend(struct device *dev)
>> +{
>> + struct tids_data *data = dev_get_drvdata(dev);
>> +
>> + return regmap_clear_bits(data->regmap, TIDS_REG_CTRL,
>> + TIDS_CTRL_FREERUN_MASK);
>> +}
>> +
>> +static int tids_resume(struct device *dev)
>> +{
>> + struct tids_data *data = dev_get_drvdata(dev);
>> +
>> + return regmap_write_bits(data->regmap, TIDS_REG_CTRL,
>> + TIDS_CTRL_FREERUN_MASK,
>> + TIDS_CTRL_FREERUN_MASK);
Is this sufficient ? Does the chip possible require an init sequence
and updating the limit values ?
Check drivers/hwmon/jc42.c which saves and restores the configuration using
regmap API functions. Maybe something similar that would be useful here
as well.
>> +}
>> +
>> +static DEFINE_SIMPLE_DEV_PM_OPS(tids_dev_pm_ops, tids_resume, tids_suspend);
>> +
>> +static const struct i2c_device_id tids_id[] = {
>> + { "tids", 0 },
>
> No need for 0
>
>> + {},
>
> The above is already a terminator so you can drop the comma
>
> - Nuno Sá
>
© 2016 - 2025 Red Hat, Inc.