Re: Re: [PATCH 1/4] hwmon: Add driver for wsen tids-2521020222501
From: Thomas Marangoni
Date: Wed Nov 19 2025 - 05:40:15 EST
On 11/17/25 17:24, Guenter Roeck wrote:
On 11/17/25 04:38, Thomas Marangoni wrote:Yes, you are right its just the order code. I've removed it from the next version
This commit adds support for the wsen tids-2521020222501. It is a
What is the relevance of "-2521020222501" ? As far as I can see it is just
the order code, and the sensor is just "WSEN-TIDS", as suggested by the
documentation. I would sugest to drop the number unless it has some actual
relevance - and, if it does, provide a rationale for it that is better than
"this is the oder code". The order code can change anytime, after all.
of the patch.
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_hyst
- hwmon_temp_max_hyst
- hwmon_chip_update_interval
Additional notes:
- The update interval only supports four fixed values.
- The alarm is reset on reading.
---
If available, please send me a register dump so I can implement unit test code.
I don't know exactly what you mean with a register dump, I made one with
i2cdump. If that's not enough can you refer me to a documentation or give me
a command I can run?
This is the i2cdump:
0 1 2 3 4 5 6 7 8 9 a b c d e f
00: 00 a0 00 00 4c 00 80 0c 17 e0 00 00 00 00 04 00
drivers/hwmon/Kconfig | 10 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/tids.c | 560 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 571 insertions(+)
create mode 100644 drivers/hwmon/tids.c
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 8c852bcac26f..5e578241001f 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -2647,6 +2647,16 @@ config SENSORS_WM8350
This driver can also be built as a module. If so, the module
will be called wm8350-hwmon.
+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_ULTRA45
tristate "Sun Ultra45 PIC16F747"
depends on SPARC64
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index a8de5bc69f2a..def052b2bdfa 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -244,6 +244,7 @@ obj-$(CONFIG_SENSORS_W83L785TS) += w83l785ts.o
obj-$(CONFIG_SENSORS_W83L786NG) += w83l786ng.o
obj-$(CONFIG_SENSORS_WM831X) += wm831x-hwmon.o
obj-$(CONFIG_SENSORS_WM8350) += wm8350-hwmon.o
+obj-$(CONFIG_SENSORS_TIDS) += tids.o
obj-$(CONFIG_SENSORS_XGENE) += xgene-hwmon.o
obj-$(CONFIG_SENSORS_OCC) += occ/
diff --git a/drivers/hwmon/tids.c b/drivers/hwmon/tids.c
new file mode 100644
index 000000000000..0176a5e8b574
--- /dev/null
+++ b/drivers/hwmon/tids.c
@@ -0,0 +1,560 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/*
+ * Copyright (c) BECOM Electronics GmbH
+ *
+ * wsen_tids_2521020222501.c - Linux hwmon driver for WSEN-TIDS
+ * 2521020222501 Temperature sensor
+ *
+ * Author: Thomas Marangoni <thomas.marangoni@xxxxxxxxxxxxxxx>
+ */
+
+#include <linux/bits.h>
+#include <linux/delay.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
Unnecessary include.
+#include <linux/i2c.h>Not used.
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+/*
+ * TIDS registers
+ */
+#define TIDS_CMD_DEVICE_ID 0x01
+#define TIDS_CMD_T_H_LIMIT 0x02
+#define TIDS_CMD_T_L_LIMIT 0x03
+#define TIDS_CMD_CTRL 0x04
+#define TIDS_CMD_STATUS 0x05
+#define TIDS_CMD_DATA_T_L 0x06
+#define TIDS_CMD_DATA_T_H 0x07
+#define TIDS_CMD_SOFT_REST 0x0C
+
+/*
+ * TIDS device IDs
+ */
+#define TIDS_ID_2521020222501 0xa0
+
+enum tids_update_interval {
+ tids_update_interval_40ms = 0,
+ tids_update_interval_20ms = 1,
+ tids_update_interval_10ms = 2,
+ tids_update_interval_5ms = 3,
+};
+
+/* TIDS control register */
+static const struct reg_field tids_reg_field_ctrl_one_shot =
+ REG_FIELD(TIDS_CMD_CTRL, 0, 0);
+static const struct reg_field tids_reg_field_ctrl_freerun =
+ REG_FIELD(TIDS_CMD_CTRL, 2, 2);
+static const struct reg_field tids_reg_field_ctrl_if_add_inc =
+ REG_FIELD(TIDS_CMD_CTRL, 3, 3);
+static const struct reg_field tids_reg_field_ctrl_avg =
+ REG_FIELD(TIDS_CMD_CTRL, 4, 5);
+static const struct reg_field tids_reg_field_ctrl_bdu =
+ REG_FIELD(TIDS_CMD_CTRL, 6, 6);
+/* TIDS status register */
+static const struct reg_field tids_reg_field_status_busy =
+ REG_FIELD(TIDS_CMD_STATUS, 0, 0);
+static const struct reg_field tids_reg_field_status_over_thl =
+ REG_FIELD(TIDS_CMD_STATUS, 1, 1);
+static const struct reg_field tids_reg_field_status_under_tll =
+ REG_FIELD(TIDS_CMD_STATUS, 2, 2);
+/* TIDS reset register */
+static const struct reg_field tids_reg_field_soft_reset =
+ REG_FIELD(TIDS_CMD_SOFT_REST, 1, 1);
+
I seem to be missing something. Why would it make sense to allocate
all those regmap fields and not just use regmap_update_bits() ?
I don't see any benefit in that complexity, especially for single-bit
"fields".
I was thinking it is a bit cleaner in code-style when using fields. In v2,
regfields have been removed and replaced with regmap functions.
+struct tids_data {
+ struct i2c_client *client;
+
+ struct regmap *regmap;
+
+ /* regmap field for ctrl register */
+ struct regmap_field *reg_ctrl_one_shot;
+ struct regmap_field *reg_ctrl_freerun;
+ struct regmap_field *reg_ctrl_if_add_inc;
+ struct regmap_field *reg_ctrl_avg;
+ struct regmap_field *reg_ctrl_bdu;
+
+ /* regmap field for status register */
+ struct regmap_field *reg_status_busy;
+ struct regmap_field *reg_status_over_thl;
+ struct regmap_field *reg_status_under_tll;
+
+ /* regmap field for soft reset register*/
+ struct regmap_field *reg_soft_reset;
+
+ int irq;
+ int temperature;
+};
+
+static ssize_t tids_interval_read(struct device *dev, long *val)
+{
+ int ret = 0;
+ unsigned int avg_value = 0;
Unnecessary initializations.
+ struct tids_data *data = dev_get_drvdata(dev);
+
+ ret = regmap_field_read(data->reg_ctrl_avg, &avg_value);
+ if (ret < 0)
+ return ret;
+
+ switch (avg_value) {
+ case tids_update_interval_40ms:
+ *val = 40;
+ break;
+ case tids_update_interval_20ms:
+ *val = 20;
+ break;
+ case tids_update_interval_10ms:
+ *val = 10;
+ break;
+ case tids_update_interval_5ms:
+ *val = 5;
+ break;
+ default:
+ return -EINVAL;
Reading a value from a chip can not return -EINVAL.
EINVAL is "Invalid argument", or bad user input, not bad data from a chip.
On top of that, the regmap field is defined as two bits. The value returned
can not be out of range. A simple array read would do the trick.
static u8 update_intervals[] = { 40, 20, 10, 5 };
return update_intervals[avg_value];
+ }
+
+ return 0;
+}
+
+static ssize_t tids_interval_write(struct device *dev, long val)
+{
+ unsigned int avg_value = 0;
+ struct tids_data *data = dev_get_drvdata(dev);
Again, please avoid unnecessary variable initializations. I am not going to
mention this again; please fix everywhere.
Also, please use "reverse christmas tree" (longest declaration first)
for variable declarations.
+
+ switch (val) {
+ case 40:
+ avg_value = tids_update_interval_40ms;
+ break;
+ case 20:
+ avg_value = tids_update_interval_20ms;
+ break;
+ case 10:
+ avg_value = tids_update_interval_10ms;
+ break;
+ case 5:
+ avg_value = tids_update_interval_5ms;
+ break;
+ default:
+ return -EINVAL;
+ }
+
This code should, similar to other drivers, approximate the provided value.
instead of letting the user figure out valid values. I would suggest to use
find_closest() or find_closest_descending() to determine valid values.
+ return regmap_field_write(data->reg_ctrl_avg, avg_value);
+}
+
+static int tids_temperature1_read(struct device *dev, long *val)
+{
+ int ret;
+ u8 buf[2] = { 0 };
+ struct tids_data *data = dev_get_drvdata(dev);
+
+ ret = regmap_bulk_read(data->regmap, TIDS_CMD_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)
+{
+ int ret = 0;
+ unsigned int reg_data = 0;
+ struct tids_data *data = dev_get_drvdata(dev);
+
+ if (attr == hwmon_temp_min_alarm)
+ ret = regmap_field_read(data->reg_status_under_tll, ®_data);
+ else if (attr == hwmon_temp_max_alarm)
+ ret = regmap_field_read(data->reg_status_over_thl, ®_data);
+ else
+ return -EOPNOTSUPP;
+
+ if (ret < 0)
+ return ret;
+
+ *val = reg_data;
+
+ return 0;
+}
+
+static int tids_temperature_hyst_read(struct device *dev, u32 attr, long *val)
+{
+ int ret;
+ struct tids_data *data = dev_get_drvdata(dev);
+ unsigned int reg_data = 0;
+
+ if (attr == hwmon_temp_min_hyst)
+ ret = regmap_read(data->regmap, TIDS_CMD_T_L_LIMIT, ®_data);
+ else if (attr == hwmon_temp_max_hyst)
+ ret = regmap_read(data->regmap, TIDS_CMD_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_hyst_write(struct device *dev, u32 attr,
+ long val)
+{
+ u8 reg_data = 0;
+ struct tids_data *data = dev_get_drvdata(dev);
+
+ // temperature in °mC
+ if (val < -39680 || val > 122880)
+ return -EINVAL;
Please use clamp_val().
+
+ // temperature to register conversion in °mC
You are using c++ comment style for single-line comments and C comment style for
multi-line comments. I am not a friend of C++-style comments, but I accept it.
However, I do ask for consistency. Either/or, please, but not both.
+ reg_data = (u8)((val / 640) + 63);
Candidate for DIV_ROUND_CLOSEST() ?
+
+ if (attr == hwmon_temp_min_hyst)
+ return regmap_write(data->regmap, TIDS_CMD_T_L_LIMIT, reg_data);
+ else if (attr == hwmon_temp_max_hyst)
+ return regmap_write(data->regmap, TIDS_CMD_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_hyst:
+ case hwmon_temp_max_hyst:
+ 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_hyst:
+ case hwmon_temp_max_hyst:
+ return tids_temperature_hyst_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_hyst:
+ case hwmon_temp_max_hyst:
+ return tids_temperature_hyst_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_HYST |
+ HWMON_T_MAX_HYST),
Why do you use "hyst" for limit attributes ? A hysteresis without limit does
not make sense. The datasheet says that those are limits (thresholds),
not hysteresis values. The datasheet doesn't even mention the term
"hysteresis".
The documentation of the subsystem wasn't completely clear to me about which
one to pick. I changed it in v2 of this patch.
+ 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 >= 0x02 && reg <= 0x04)
+ return true;
+
+ if (reg == 0x0c)
+ return true;
+
Registers are defined. Please use the definitions here and in the functions below.
+ return false;
+}
+
+static bool tids_regmap_readable_reg(struct device *dev, unsigned int reg)
+{
+ if (reg >= 0x01 && reg <= 0x07)
+ return true;
+
+ if (reg == 0x0c)
+ return true;
+
+ return false;
+}
+
+static bool tids_regmap_volatile_reg(struct device *dev, unsigned int reg)
+{
+ if (reg >= 0x05 && reg <= 0x07)
+ return true;
+
+ if (reg == 0x0c)
+ return true;
+
+ return false;
+}
+
+static const struct regmap_config regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = TIDS_CMD_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_probe_regmap(struct tids_data *data)
+{
+ struct device *dev = &data->client->dev;
+
+ /* Init regmap */
+ data->regmap = devm_regmap_init_i2c(data->client, ®map_config);
+ if (IS_ERR(data->regmap))
+ return dev_err_probe(dev, PTR_ERR(data->regmap),
+ "regmap initialization failed\n");
+
+ /* Allocate regmap_field for ctrl register */
+ data->reg_ctrl_one_shot = devm_regmap_field_alloc(
+ dev, data->regmap, tids_reg_field_ctrl_one_shot);
+ if (IS_ERR(data->reg_ctrl_one_shot))
+ return dev_err_probe(
+ dev, PTR_ERR(data->reg_ctrl_one_shot),
+ "regmap_field reg_ctrl_one_shot initialization failed\n");
+
+ data->reg_ctrl_freerun = devm_regmap_field_alloc(
+ dev, data->regmap, tids_reg_field_ctrl_freerun);
+ if (IS_ERR(data->reg_ctrl_freerun))
+ return dev_err_probe(
+ dev, PTR_ERR(data->reg_ctrl_freerun),
+ "regmap_field reg_ctrl_freerun initialization failed\n");
+
+ data->reg_ctrl_if_add_inc = devm_regmap_field_alloc(
+ dev, data->regmap, tids_reg_field_ctrl_if_add_inc);
+ if (IS_ERR(data->reg_ctrl_if_add_inc))
+ return dev_err_probe(
+ dev, PTR_ERR(data->reg_ctrl_if_add_inc),
+ "regmap_field reg_ctrl_if_add_inc initialization failed\n");
+
+ data->reg_ctrl_avg = devm_regmap_field_alloc(dev, data->regmap,
+ tids_reg_field_ctrl_avg);
+ if (IS_ERR(data->reg_ctrl_avg))
+ return dev_err_probe(
+ dev, PTR_ERR(data->reg_ctrl_avg),
+ "regmap_field reg_ctrl_avg initialization failed\n");
+
+ data->reg_ctrl_bdu = devm_regmap_field_alloc(dev, data->regmap,
+ tids_reg_field_ctrl_bdu);
+ if (IS_ERR(data->reg_ctrl_bdu))
+ return dev_err_probe(
+ dev, PTR_ERR(data->reg_ctrl_bdu),
+ "regmap_field reg_ctrl_bdu initialization failed\n");
+
+ /* Allocate regmap_field for status register */
+ data->reg_status_busy = devm_regmap_field_alloc(
+ dev, data->regmap, tids_reg_field_status_busy);
+ if (IS_ERR(data->reg_status_busy))
+ return dev_err_probe(
+ dev, PTR_ERR(data->reg_status_busy),
+ "regmap_field reg_status_busy initialization failed\n");
+
+ data->reg_status_over_thl = devm_regmap_field_alloc(
+ dev, data->regmap, tids_reg_field_status_over_thl);
+ if (IS_ERR(data->reg_status_over_thl))
+ return dev_err_probe(
+ dev, PTR_ERR(data->reg_status_over_thl),
+ "regmap_field reg_status_over_thl initialization failed\n");
+
+ data->reg_status_under_tll = devm_regmap_field_alloc(
+ dev, data->regmap, tids_reg_field_status_under_tll);
+ if (IS_ERR(data->reg_status_under_tll))
+ return dev_err_probe(
+ dev, PTR_ERR(data->reg_status_under_tll),
+ "regmap_field reg_status_under_tll initialization failed\n");
+
+ /* Allocate regmap_field for software_reset */
+ data->reg_soft_reset = devm_regmap_field_alloc(
+ dev, data->regmap, tids_reg_field_soft_reset);
+ if (IS_ERR(data->reg_soft_reset))
+ return dev_err_probe(
+ dev, PTR_ERR(data->reg_soft_reset),
+ "regmap_field reg_soft_reset initialization failed\n");
Following up on the above, that is a lot of code just to avoid using regmap_update_bits().
Again, what am I missing ?
+
+ 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 = 0;
+
+ data = devm_kzalloc(device, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ data->client = client;
+
+ ret = tids_probe_regmap(data);
+ if (ret != 0)
+ return ret;
+
+ /* Read device id, to check if i2c is working */
+ ret = regmap_read(data->regmap, TIDS_CMD_DEVICE_ID, &value);
+ if (ret < 0)
+ return ret;
Why read this but not check it ?
+
+ /* Triggering soft reset */
+ ret = regmap_field_write(data->reg_soft_reset, 1);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_field_write(data->reg_soft_reset, 0);
+ if (ret < 0)
+ return ret;
+
+ /* Allowing bulk read */
+ ret = regmap_field_write(data->reg_ctrl_if_add_inc, 1);
+ if (ret < 0)
+ return ret;
+
+ /* Set meassurement interval */
+ ret = regmap_field_write(data->reg_ctrl_avg, tids_update_interval_40ms);
+ if (ret < 0)
+ return ret;
+
+ /* Set device to free run mode */
+ ret = regmap_field_write(data->reg_ctrl_freerun, 1);
+ if (ret < 0)
+ return ret;
+
+ /* Don't update temperature register until high and low value are read */
+ ret = regmap_field_write(data->reg_ctrl_bdu, 1);
+ if (ret < 0)
+ return ret;
+
Please move this code into a separate _init function. Also, is it really necessary
to write all those control register values bit by bit ? If so, that should be explained
since it adds a lot of non-obvious complexity to the code.
+ hwmon_dev = devm_hwmon_device_register_with_info(
+ device, device->driver->name, data, &tids_chip_info, NULL);
Just use "tids" instead of "device->driver->name".
+
+ return PTR_ERR_OR_ZERO(hwmon_dev);
+}
+
+static int tids_suspend(struct device *dev)
+{
+ struct tids_data *data = dev_get_drvdata(dev);
+
+ return regmap_field_write(data->reg_ctrl_freerun, 0);
+}
+
+static int tids_resume(struct device *dev)
+{
+ struct tids_data *data = dev_get_drvdata(dev);
+
+ return regmap_field_write(data->reg_ctrl_freerun, 1);
+}
+
+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-2521020222501" },
+ {}
+};
+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@xxxxxxxxxxxxxxx>");
+MODULE_DESCRIPTION("WSEN TIDS temperature sensor driver");
+MODULE_LICENSE("GPL");
I've gone through all the other notes and included them in the next version of the
patch. I will send the next patch series within the next few hours.