[PATCH 0/2] hwmon: Add support for TI TPS389004/TPS389006/TPS389008

[PATCH 0/2] hwmon: Add support for TI TPS389004/TPS389006/TPS389008

[Luna Hart via B4 Relay]

This series adds a binding and hwmon driver for the TI TPS38900x family
of I2C voltage supervisor/monitors. The driver implements voltage
monitoring functionality from the ADCs of the monitor chip. It supports
basic configuration of voltage channels, specifically the ability to set
the high-range (4x) voltage scale.

Tested on TI J784S4 on custom hardware with a TPS389006. Kernel 6.18 was
used with these patches during testing.

This is built atop previous unmerged work from Flaviu Nistor, but
features a large rewrite of the driver to utilize regmaps.

Signed-off-by: Luna Hart <lhart.opensource@witekio.com>
---
Luna Hart (2):
      dt-bindings: hwmon: Add TI TPS389004/TPS389006/TPS389008
      hwmon: (tps38900x) Add driver for TI TPS389004/TPS389006/TPS389008

 .../devicetree/bindings/hwmon/ti,tps38900x.yaml    | 137 ++++++++++
 Documentation/hwmon/index.rst                      |   1 +
 Documentation/hwmon/tps38900x.rst                  |  55 ++++
 MAINTAINERS                                        |   8 +
 drivers/hwmon/Kconfig                              |  11 +
 drivers/hwmon/Makefile                             |   1 +
 drivers/hwmon/tps38900x.c                          | 281 +++++++++++++++++++++
 7 files changed, 494 insertions(+)
---
base-commit: 4781ca52761e666cf18b591e6bb0478396c90320
change-id: 20260929-lhart-dev-tps38900x-f932bcdb4057

Best regards,
-- 
Luna Hart <lhart.opensource@witekio.com>

[PATCH 1/2] dt-bindings: hwmon: Add TI TPS389004/TPS389006/TPS389008

[Luna Hart via B4 Relay]

From: Luna Hart <lhart.opensource@witekio.com>

Add a binding for the TI TPS389004, TPS389006, and TPS389008 I2C
voltage supervisors and monitors, which provide 4, 6, and 8 voltage
inputs respectively. Each input may be described by an optional child
node, containing additional properties including the label and ADC
scaling factor.

Based on an earlier binding by Flaviu Nistor.

Signed-off-by: Luna Hart <lhart.opensource@witekio.com>
---
 .../devicetree/bindings/hwmon/ti,tps38900x.yaml    | 137 +++++++++++++++++++++
 1 file changed, 137 insertions(+)

diff --git a/Documentation/devicetree/bindings/hwmon/ti,tps38900x.yaml b/Documentation/devicetree/bindings/hwmon/ti,tps38900x.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..cdd74ddd03e748f271628f1cd2f2551b9c983785
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/ti,tps38900x.yaml
@@ -0,0 +1,137 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hwmon/ti,tps38900x.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TPS38900x voltage monitor
+
+maintainers:
+  - Luna Hart <lhart.opensource@witekio.com>
+
+description: |
+  Voltage monitor from TI (TPS389008, TPS389006, TPS389004) on I2C bus.
+
+  Datasheets:
+    https://www.ti.com/lit/ds/symlink/tps389006.pdf
+    https://www.ti.com/lit/ds/symlink/tps389006-q1.pdf
+
+properties:
+  compatible:
+    enum:
+      - ti,tps389004
+      - ti,tps389006
+      - ti,tps389008
+
+  reg:
+    maxItems: 1
+
+  "#address-cells":
+    description: Required only if a child node is present.
+    const: 1
+
+  "#size-cells":
+    description: Required only if a child node is present.
+    const: 0
+
+patternProperties:
+  "^vmon@[0-7]$":
+    description: |
+      The node may have optional child nodes for each voltage channel.
+      There are either 4, 6, or 8 available depending on the specific part
+      being used.
+      If no child nodes are present, all channels will be exposed for
+      monitoring. Otherwise, only the provided nodes will be created.
+    type: object
+    additionalProperties: false
+    properties:
+      reg:
+        description: |
+          Must be 0 to 7, corresponding to respective
+          ports of the TPS38900x.
+        enum: [ 0, 1, 2, 3, 4, 5, 6, 7 ]
+
+      label:
+        description: Name of the input source.
+
+      ti,4x-scaling:
+        description: |
+          Enables the high range ADC voltage monitoring range. Default ADC
+          settings will read from 0.2V to 1.475V using 5mV steps. In the 4x
+          mode, the valid range is 0.8V to 5.9V using 20mV steps.
+        type: boolean
+
+    required:
+      - reg
+
+required:
+  - compatible
+  - reg
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          const: ti,tps389004
+    then:
+      patternProperties:
+        "^vmon@[4-7]$": false
+
+  - if:
+      properties:
+        compatible:
+          const: ti,tps389006
+    then:
+      patternProperties:
+        "^vmon@[67]$": false
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        voltage-monitor@30 {
+            compatible = "ti,tps389008";
+            reg = <0x30>;
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            vmon@0 {
+                reg = <0x0>;
+                label = "input0";
+                ti,4x-scaling;
+            };
+            vmon@1 {
+                reg = <0x1>;
+                label = "input1";
+                ti,4x-scaling;
+            };
+            vmon@2 {
+                reg = <0x2>;
+                label = "input2";
+            };
+            vmon@3 {
+                reg = <0x3>;
+                label = "input3";
+            };
+            vmon@4 {
+                reg = <0x4>;
+                label = "input4";
+            };
+            vmon@5 {
+                reg = <0x5>;
+                label = "input5";
+            };
+            vmon@6 {
+                reg = <0x6>;
+                label = "input6";
+            };
+            vmon@7 {
+                reg = <0x7>;
+                label = "input7";
+            };
+        };
+    };

-- 
2.47.3

[PATCH 2/2] hwmon: (tps38900x) Add driver for TI TPS389004/TPS389006/TPS389008

[Luna Hart via B4 Relay]

From: Luna Hart <lhart.opensource@witekio.com>

Add support for the TI TPS389004/TPS389006/TPS389008 I2C voltage
supervisors and monitors. This initial implementation supports enabling
ADC monitoring from the chip and reading back the measured voltage in
millivolts. All functionality is provided over sysfs. Device tree
configurations may be used to further configure each voltage channel.

This work is based on an initial implementation by Flaviu Nistor.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Luna Hart <lhart.opensource@witekio.com>
---
 Documentation/hwmon/index.rst     |   1 +
 Documentation/hwmon/tps38900x.rst |  55 ++++++++
 MAINTAINERS                       |   8 ++
 drivers/hwmon/Kconfig             |  11 ++
 drivers/hwmon/Makefile            |   1 +
 drivers/hwmon/tps38900x.c         | 281 ++++++++++++++++++++++++++++++++++++++
 6 files changed, 357 insertions(+)

diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
index 199ca79621e94a3f2e1aed3e2a49a1b43c9f41d6..f7c5747a813efab4d7bc55745eb595140362648d 100644
--- a/Documentation/hwmon/index.rst
+++ b/Documentation/hwmon/index.rst
@@ -279,6 +279,7 @@ Hardware Monitoring Kernel Drivers
    tmp513
    tps23861
    tps25990
+   tps38900x
    tps40422
    tps53679
    tps546d24
diff --git a/Documentation/hwmon/tps38900x.rst b/Documentation/hwmon/tps38900x.rst
new file mode 100644
index 0000000000000000000000000000000000000000..e232a179e0062b49dd2a78cd38790a6b5dae0774
--- /dev/null
+++ b/Documentation/hwmon/tps38900x.rst
@@ -0,0 +1,55 @@
+.. SPDX-License-Identifier: GPL-2.0-or-later
+
+Kernel Driver tps38900x
+=======================
+
+Supported chips:
+
+  * Texas Instruments TPS389008, TPS389006, TPS389004
+
+    Prefix: 'tps38900x'
+
+    Addresses scanned: -
+
+    Datasheet: https://www.ti.com/lit/ds/symlink/tps389006.pdf
+
+Author:
+
+  - Luna Hart <lhart.opensource@witekio.com>
+
+Description
+-----------
+
+This driver implements support for the TI TPS38900x family of voltage monitor.
+Supported chips include the TPS389004, TPS389006, and TPS389008.
+
+The driver currently supports reading out current voltage levels from the ADCs.
+Features like interrupts and sequencing readings are not currently supported.
+
+All ADC measurements are 8 bit, and there are 2 supported measurement scales.
+
+  - 1x scaling: 200 to 1475 mV (5mV steps) (default)
+  - 4x scaling: 800 to 5900 mV (20mV steps)
+
+Voltage values are read from sysfs. By default, all ADCs are exposed. However,
+when channels are defined via child nodes in the device tree: only the channels
+that explicitly have a device tree node will be available.
+
+Known Issues
+------------
+
+Interrupts and sequenced readings functions of the tps38900x family are not
+implemented.
+
+sysfs-Interface
+---------------
+
+The complete list of sysfs attributes provided for each channel:
+
+=============================== ======= ========================================
+Name                            Perm    Description
+=============================== ======= ========================================
+in[01234567]_input              RO      Voltage channel input
+in[01234567]_label              RO      Voltage channel label
+in[01234567]_enable             RW      Voltage channel enable controls
+=============================== ======= ========================================
diff --git a/MAINTAINERS b/MAINTAINERS
index 5ad12c57ca5f546079f1dd5d139f27daa11e99c7..b08c52e929e5d15e182f83f2f7bdfef70002f7a3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -27082,6 +27082,14 @@ F:	Documentation/devicetree/bindings/hwmon/ti,tps23861.yaml
 F:	Documentation/hwmon/tps23861.rst
 F:	drivers/hwmon/tps23861.c
 
+TEXAS INSTRUMENTS TPS38900X VOLTAGE MONITOR DRIVER
+M:	Luna Hart <lhart.opensource@witekio.com>
+L:	linux-hwmon@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/hwmon/ti,tps38900x.yaml
+F:	Documentation/hwmon/tps38900x.rst
+F:	drivers/hwmon/tps38900x.c
+
 TEXAS INSTRUMENTS TPS6131X FLASH LED DRIVER
 M:	Matthias Fend <matthias.fend@emfend.at>
 L:	linux-leds@vger.kernel.org
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 18c682e1a4756a35e1433493271ba65b65f7fbe8..91f0c9fa7f436c73bfbe9a07a154cceb8f4af50f 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1503,6 +1503,17 @@ config SENSORS_TPS23861
 	  This driver can also be built as a module. If so, the module
 	  will be called tps23861.
 
+config SENSORS_TPS38900X
+	tristate "TI TPS38900x VMON Driver"
+	depends on I2C
+	select REGMAP_I2C
+	help
+	  If you say yes here you get support for Texas Instruments
+	  TPS389004/TPS389006/TPS389008 voltage monitors.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called tps38900x.
+
 config SENSORS_MENF21BMC_HWMON
 	tristate "MEN 14F021P00 BMC Hardware Monitoring"
 	depends on MFD_MENF21BMC
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index d4c345ed7261f9fc459c170af899ef40197839e3..e43804a53415111f15dba875c909dd9ac849aa33 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -181,6 +181,7 @@ obj-$(CONFIG_SENSORS_MCP3021)	+= mcp3021.o
 obj-$(CONFIG_SENSORS_MCP9982)	+= mcp9982.o
 obj-$(CONFIG_SENSORS_TC654)	+= tc654.o
 obj-$(CONFIG_SENSORS_TPS23861)	+= tps23861.o
+obj-$(CONFIG_SENSORS_TPS38900X)	+= tps38900x.o
 obj-$(CONFIG_SENSORS_MLXREG_FAN) += mlxreg-fan.o
 obj-$(CONFIG_SENSORS_MENF21BMC_HWMON) += menf21bmc_hwmon.o
 obj-$(CONFIG_SENSORS_MINISFORUM_UM780XTX) += minisforum-um780xtx.o
diff --git a/drivers/hwmon/tps38900x.c b/drivers/hwmon/tps38900x.c
new file mode 100644
index 0000000000000000000000000000000000000000..659d61b1cb55c79d42d4ba6dc69d19334f3292ca
--- /dev/null
+++ b/drivers/hwmon/tps38900x.c
@@ -0,0 +1,281 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Driver for Texas Instruments TPS389004/TPS389006/TPS389008 i2c voltage
+ * supervisor and monitor.
+ *
+ * Author: Luna Hart <lhart.opensource@witekio.com>
+ *
+ * Datasheet: https://www.ti.com/lit/ds/symlink/tps389006.pdf
+ * Datasheet: https://www.ti.com/lit/ds/symlink/tps389006-q1.pdf
+ */
+
+#include <linux/bits.h>
+#include <linux/hwmon.h>
+#include <linux/i2c.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+
+#define TPS38900X_MAX_CHANNELS 8
+#define TPS38900X_BANK_LEN 0x100
+
+// Registers, stored over 2 banks of 256 bytes.
+#define TPS38900X_VRANGE_MULT 0x1f
+#define TPS38900X_MON_LVL(ch) (0x40 + (ch))
+#define TPS38900X_BANK_SEL 0xf0
+#define TPS38900X_MON_CH_EN 0x11e
+
+#define TPS38900X_LSB_MV 5
+#define TPS38900X_OFFSET_MV 200
+
+struct tps38900x_data {
+	struct regmap *regmap;
+	const char *label[TPS38900X_MAX_CHANNELS];
+	unsigned int num_channels;
+	// bitmasks of channel states
+	u8 channel_present;
+	u8 channel_scale_4x;
+};
+
+static umode_t tps38900x_is_visible(const void *data,
+				    enum hwmon_sensor_types type, u32 attr,
+				    int channel)
+{
+	const struct tps38900x_data *priv = data;
+
+	if (!(priv->channel_present & BIT(channel)))
+		return 0;
+
+	switch (attr) {
+	case hwmon_in_input:
+		return 0444;
+	case hwmon_in_enable:
+		return 0644;
+	case hwmon_in_label:
+		return priv->label[channel] ? 0444 : 0;
+	}
+
+	return 0;
+}
+
+static int tps38900x_read(struct device *dev, enum hwmon_sensor_types type,
+			  u32 attr, int channel, long *val)
+{
+	struct tps38900x_data *priv = dev_get_drvdata(dev);
+	unsigned int regval;
+	int ret;
+	int val_scale;
+
+	switch (attr) {
+	case hwmon_in_input:
+		ret = regmap_read(priv->regmap, TPS38900X_MON_CH_EN, &regval);
+		if (ret)
+			return ret;
+
+		if (!(regval & BIT(channel)))
+			return -ENODATA;
+
+		ret = regmap_read(priv->regmap, TPS38900X_MON_LVL(channel),
+				  &regval);
+		if (ret)
+			return ret;
+
+		val_scale = priv->channel_scale_4x & BIT(channel) ? 4 : 1;
+		*val = (regval * TPS38900X_LSB_MV + TPS38900X_OFFSET_MV) *
+		       val_scale;
+		return 0;
+	case hwmon_in_enable:
+		ret = regmap_read(priv->regmap, TPS38900X_MON_CH_EN, &regval);
+		if (ret)
+			return ret;
+
+		*val = !!(regval & BIT(channel));
+		return 0;
+	}
+
+	return -EOPNOTSUPP;
+}
+
+static int tps38900x_write(struct device *dev, enum hwmon_sensor_types type,
+			   u32 attr, int channel, long enable)
+{
+	struct tps38900x_data *priv = dev_get_drvdata(dev);
+
+	switch (attr) {
+	case hwmon_in_enable:
+		if (enable != 0 && enable != 1)
+			return -EINVAL;
+
+		return regmap_update_bits(priv->regmap, TPS38900X_MON_CH_EN,
+					  BIT(channel),
+					  enable ? BIT(channel) : 0);
+	}
+
+	return -EOPNOTSUPP;
+}
+
+static int tps38900x_read_string(struct device *dev,
+				 enum hwmon_sensor_types type, u32 attr,
+				 int channel, const char **str)
+{
+	struct tps38900x_data *priv = dev_get_drvdata(dev);
+
+	switch (attr) {
+	case hwmon_in_label:
+		*str = priv->label[channel];
+		return 0;
+	}
+
+	return -EOPNOTSUPP;
+}
+
+static const struct hwmon_ops tps38900x_hwmon_ops = {
+	.is_visible = tps38900x_is_visible,
+	.read = tps38900x_read,
+	.write = tps38900x_write,
+	.read_string = tps38900x_read_string,
+};
+
+#define TPS38900X_IN_CONFIG (HWMON_I_INPUT | HWMON_I_ENABLE | HWMON_I_LABEL)
+
+static const struct hwmon_channel_info *const tps38900x_info[] = {
+	HWMON_CHANNEL_INFO(in, TPS38900X_IN_CONFIG, TPS38900X_IN_CONFIG,
+			   TPS38900X_IN_CONFIG, TPS38900X_IN_CONFIG,
+			   TPS38900X_IN_CONFIG, TPS38900X_IN_CONFIG,
+			   TPS38900X_IN_CONFIG, TPS38900X_IN_CONFIG),
+	NULL
+};
+
+static const struct hwmon_chip_info tps38900x_chip_info = {
+	.ops = &tps38900x_hwmon_ops,
+	.info = tps38900x_info,
+};
+
+static const struct regmap_range_cfg tps38900x_bank_range = {
+	.name = "tps38900x-mem-bank",
+	.range_min = 0x00,
+	.range_max = (2 * TPS38900X_BANK_LEN) - 1,
+	.selector_reg = TPS38900X_BANK_SEL,
+	.selector_mask = BIT(0),
+	.window_start = 0,
+	.window_len = TPS38900X_BANK_LEN,
+};
+
+static bool tps38900x_volatile_reg(struct device *dev, unsigned int reg)
+{
+	// ADC results region is volatile
+	if (reg >= TPS38900X_MON_LVL(0) &&
+	    reg < TPS38900X_MON_LVL(TPS38900X_MAX_CHANNELS))
+		return true;
+
+	return false;
+}
+
+static const struct regmap_config tps38900x_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = 2 * TPS38900X_BANK_LEN - 1,
+	.ranges = &tps38900x_bank_range,
+	.num_ranges = 1,
+	.volatile_reg = tps38900x_volatile_reg,
+	.cache_type = REGCACHE_MAPLE,
+};
+
+static int tps38900x_parse_dt_channels(struct device *dev,
+				       struct tps38900x_data *priv)
+{
+	u32 reg;
+	int ret;
+
+	device_for_each_child_node_scoped(dev, child) {
+		ret = fwnode_property_read_u32(child, "reg", &reg);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "missing reg property\n");
+
+		if (reg >= priv->num_channels)
+			return dev_err_probe(dev, -EINVAL,
+					     "reg out of range: %u (max %u)\n",
+					     reg, priv->num_channels);
+
+		priv->channel_present |= BIT(reg);
+		if (fwnode_property_read_bool(child, "ti,4x-scaling"))
+			priv->channel_scale_4x |= BIT(reg);
+
+		fwnode_property_read_string(child, "label", &priv->label[reg]);
+	}
+
+	// When no channels are explicitly described in the DT, expose all of them.
+	if (!priv->channel_present)
+		priv->channel_present = GENMASK(priv->num_channels - 1, 0);
+
+	return 0;
+}
+
+static int tps38900x_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct tps38900x_data *priv;
+	struct device *hwmon_dev;
+	int ret;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
+		return -ENODEV;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->num_channels = (uintptr_t)i2c_get_match_data(client);
+
+	priv->regmap = devm_regmap_init_i2c(client, &tps38900x_regmap_config);
+	if (IS_ERR(priv->regmap))
+		return dev_err_probe(dev, PTR_ERR(priv->regmap),
+				     "failed to allocate register map\n");
+
+	ret = tps38900x_parse_dt_channels(dev, priv);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(priv->regmap, TPS38900X_VRANGE_MULT,
+			   priv->channel_scale_4x);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "failed to write voltage range\n");
+
+	hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name, priv,
+							 &tps38900x_chip_info, NULL);
+
+	return PTR_ERR_OR_ZERO(hwmon_dev);
+}
+
+static const struct i2c_device_id tps38900x_id[] = {
+	{ .name = "tps389004", .driver_data = 4 },
+	{ .name = "tps389006", .driver_data = 6 },
+	{ .name = "tps389008", .driver_data = 8 },
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, tps38900x_id);
+
+static const struct of_device_id tps38900x_of_match[] = {
+	{ .compatible = "ti,tps389004", .data = (void *)4 },
+	{ .compatible = "ti,tps389006", .data = (void *)6 },
+	{ .compatible = "ti,tps389008", .data = (void *)8 },
+	{}
+};
+MODULE_DEVICE_TABLE(of, tps38900x_of_match);
+
+static struct i2c_driver tps38900x_driver = {
+	.driver = {
+		.name = "tps38900x",
+		.of_match_table = tps38900x_of_match,
+	},
+	.probe = tps38900x_probe,
+	.id_table = tps38900x_id,
+};
+module_i2c_driver(tps38900x_driver);
+
+MODULE_AUTHOR("Luna Hart <lhart.opensource@witekio.com>");
+MODULE_DESCRIPTION("TI TPS389004/TPS389006/TPS389008 voltage monitor driver");
+MODULE_LICENSE("GPL");

-- 
2.47.3

Re: [PATCH 2/2] hwmon: (tps38900x) Add driver for TI TPS389004/TPS389006/TPS389008

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] hwmon: undefined behavior in GENMASK() due to unchecked i2c_get_match_data()
--

commit 5632ba367123600a6ee38c612901f594c35ee0df
Author: Luna Hart <lhart.opensource@witekio.com>

hwmon: (tps38900x) Add driver for TI TPS389004/TPS389006/TPS389008

This commit adds support for the TI TPS389004/TPS389006/TPS389008 I2C
voltage supervisors and monitors. It allows enabling ADC monitoring and
reading back the measured voltage in millivolts via sysfs.

> diff --git a/drivers/hwmon/tps38900x.c b/drivers/hwmon/tps38900x.c
> index 0000000000000..659d61b1cb55c 100644
> --- /dev/null
> +++ b/drivers/hwmon/tps38900x.c
[ ... ]
> +static int tps38900x_parse_dt_channels(struct device *dev,
> +				       struct tps38900x_data *priv)
> +{
[ ... ]
> +	// When no channels are explicitly described in the DT, expose all of them.
> +	if (!priv->channel_present)
> +		priv->channel_present = GENMASK(priv->num_channels - 1, 0);

[Severity: Medium]
Could this result in undefined behavior in tps38900x_parse_dt_channels()
in drivers/hwmon/tps38900x.c? If the driver is bound via sysfs
driver_override using an I2C device name not present in the driver's ID
table, num_channels would be 0. This evaluates to GENMASK(-1, 0), which
performs a bitwise shift by BITS_PER_LONG bits.

> +
> +	return 0;
> +}
> +
> +static int tps38900x_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	struct tps38900x_data *priv;
> +	struct device *hwmon_dev;
> +	int ret;
> +
> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
> +		return -ENODEV;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->num_channels = (uintptr_t)i2c_get_match_data(client);

[Severity: Medium]
Does this code in tps38900x_probe() in drivers/hwmon/tps38900x.c allow 0 to
be stored as the number of channels? If i2c_get_match_data() returns NULL
when a user binds the driver via sysfs driver_override, num_channels becomes
0 without being checked.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929-lhart-dev-tps38900x-v1-0-7045c5894152@witekio.com?part=2

Re: [PATCH 1/2] dt-bindings: hwmon: Add TI TPS389004/TPS389006/TPS389008

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dt-bindings: silent schema validation bypass due to invalid compatible match
--

commit 6f785a9cad4f5ba27f5d71565ba1ca2b776da113
Author: Luna Hart <lhart.opensource@witekio.com>

dt-bindings: hwmon: Add TI TPS389004/TPS389006/TPS389008

Add a binding for the TI TPS389004, TPS389006, and TPS389008 I2C voltage
supervisors and monitors, which provide 4, 6, and 8 voltage inputs
respectively. Each input may be described by an optional child node,
containing additional properties including the label and ADC scaling factor.

> diff --git a/Documentation/devicetree/bindings/hwmon/ti,tps38900x.yaml b/Documentation/devicetree/bindings/hwmon/ti,tps38900x.yaml
> new file mode 100644
> index 0000000000000..cdd74ddd03e74
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/ti,tps38900x.yaml
[ ... ]
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          const: ti,tps389004

[Severity: Medium]
Will this properly evaluate the compatible property?

In device tree bindings, dt-schema evaluates the compatible property as an
array of strings (for example, ["ti,tps389004"]). The const keyword strictly
checks for exact equivalence. Since a scalar string is not equivalent to an
array, this if condition will always evaluate to false.

Because of this, the hardware-specific constraints in the then block
restricting out-of-bounds voltage channels for the 4-channel variant are never
applied, allowing invalid configurations to silently pass dt_binding_check.

Should this use contains: const: ti,tps389004 instead?

> +    then:
> +      patternProperties:
> +        "^vmon@[4-7]$": false
> +
> +  - if:
> +      properties:
> +        compatible:
> +          const: ti,tps389006

[Severity: Medium]
Is this block also affected by the same array matching syntax issue?

Just like the 4-channel variant above, this condition will fail to match the
compatible array, bypassing the channel restrictions for the TPS389006.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929-lhart-dev-tps38900x-v1-0-7045c5894152@witekio.com?part=1

Re: [PATCH 1/2] dt-bindings: hwmon: Add TI TPS389004/TPS389006/TPS389008

[Luna HART]

On Tue, 29 Sep 2026 at 16:17, sashiko-bot@kernel.org wrote:
> > +allOf:
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          const: ti,tps389004
>
> [Severity: Medium]
> Will this properly evaluate the compatible property?
[...]
> Should this use contains: const: ti,tps389004 instead?

I believe this is a non issue. This pattern is also used in an existing schema file:
Documentation/devicetree/bindings/hwmon/microchip,emc1812.yaml

Additionally, I have tested on my own machine that the schema rejects
correctly. I replaced the compatible line on ti,tps38900x.yaml:97 with
"ti,tps389006". When running `make dt_binding_check` on this file, it
generates an error, as expected, about the channel 6 and 7 definitions. 

Documentation/devicetree/bindings/hwmon/ti,tps38900x.example.dtb: voltage-monitor@30 (ti,tps389006): False schema does not allow {'reg': [[6]], 'label': ['input6']}
	from schema $id: http://devicetree.org/schemas/hwmon/ti,tps38900x.yaml
Documentation/devicetree/bindings/hwmon/ti,tps38900x.example.dtb: voltage-monitor@30 (ti,tps389006): False schema does not allow {'reg': [[7]], 'label': ['input7']}
	from schema $id: http://devicetree.org/schemas/hwmon/ti,tps38900x.yaml

That said, this is my first attempt at up-streaming a patch, and I could be
missing something here.

Re: [PATCH 1/2] dt-bindings: hwmon: Add TI TPS389004/TPS389006/TPS389008

[Krzysztof Kozlowski]

On Tue, Sep 29, 2026 at 04:04:04PM -0700, Luna Hart wrote:
> Add a binding for the TI TPS389004, TPS389006, and TPS389008 I2C
> voltage supervisors and monitors, which provide 4, 6, and 8 voltage
> inputs respectively. Each input may be described by an optional child
> node, containing additional properties including the label and ADC
> scaling factor.
> 
> Based on an earlier binding by Flaviu Nistor.
> 
> Signed-off-by: Luna Hart <lhart.opensource@witekio.com>
> ---
>  .../devicetree/bindings/hwmon/ti,tps38900x.yaml    | 137 +++++++++++++++++++++

Preference is to name after compatible, so ti,tps389004

>  1 file changed, 137 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/ti,tps38900x.yaml b/Documentation/devicetree/bindings/hwmon/ti,tps38900x.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..cdd74ddd03e748f271628f1cd2f2551b9c983785
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/ti,tps38900x.yaml
> @@ -0,0 +1,137 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/ti,tps38900x.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TPS38900x voltage monitor
> +
> +maintainers:
> +  - Luna Hart <lhart.opensource@witekio.com>
> +
> +description: |
> +  Voltage monitor from TI (TPS389008, TPS389006, TPS389004) on I2C bus.
> +
> +  Datasheets:
> +    https://www.ti.com/lit/ds/symlink/tps389006.pdf
> +    https://www.ti.com/lit/ds/symlink/tps389006-q1.pdf
> +
> +properties:
> +  compatible:
> +    enum:
> +      - ti,tps389004
> +      - ti,tps389006
> +      - ti,tps389008
> +
> +  reg:
> +    maxItems: 1
> +
> +  "#address-cells":
> +    description: Required only if a child node is present.

Drop description, obvious. Schema and DT spec tells that.

> +    const: 1
> +
> +  "#size-cells":
> +    description: Required only if a child node is present.

Ditto

> +    const: 0
> +
> +patternProperties:
> +  "^vmon@[0-7]$":

How is this called for other devices? "channel"?

> +    description: |

Do not need '|' unless you need to preserve formatting.

> +      The node may have optional child nodes for each voltage channel.
> +      There are either 4, 6, or 8 available depending on the specific part
> +      being used.

Drop sentence, schema will tell that.

> +      If no child nodes are present, all channels will be exposed for
> +      monitoring. Otherwise, only the provided nodes will be created.

That's a bit odd behavior. Is it expected for hwmon?

> +    type: object
> +    additionalProperties: false

Blank line

> +    properties:
> +      reg:
> +        description: |

Do not need '|' unless you need to preserve formatting.

> +          Must be 0 to 7, corresponding to respective
> +          ports of the TPS38900x.
> +        enum: [ 0, 1, 2, 3, 4, 5, 6, 7 ]
> +
> +      label:
> +        description: Name of the input source.
> +
> +      ti,4x-scaling:
> +        description: |

Same, drop |

> +          Enables the high range ADC voltage monitoring range. Default ADC
> +          settings will read from 0.2V to 1.475V using 5mV steps. In the 4x
> +          mode, the valid range is 0.8V to 5.9V using 20mV steps.
> +        type: boolean
> +
> +    required:
> +      - reg
> +
> +required:
> +  - compatible
> +  - reg

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: hwmon: Add TI TPS389004/TPS389006/TPS389008

[Krzysztof Kozlowski]

On Wed, Sep 30, 2026 at 11:28:08PM +0000, Luna HART wrote:
> On Tue, 29 Sep 2026 at 16:17, sashiko-bot@kernel.org wrote:
> > > +allOf:
> > > +  - if:
> > > +      properties:
> > > +        compatible:
> > > +          const: ti,tps389004
> >
> > [Severity: Medium]
> > Will this properly evaluate the compatible property?
> [...]
> > Should this use contains: const: ti,tps389004 instead?
> 
> I believe this is a non issue. This pattern is also used in an existing schema file:

Correct.

Best regards,
Krzysztof

Re: [PATCH 2/2] hwmon: (tps38900x) Add driver for TI TPS389004/TPS389006/TPS389008

[Krzysztof Kozlowski]

On Tue, Sep 29, 2026 at 04:04:05PM -0700, Luna Hart wrote:
> +static const struct i2c_device_id tps38900x_id[] = {
> +	{ .name = "tps389004", .driver_data = 4 },
> +	{ .name = "tps389006", .driver_data = 6 },
> +	{ .name = "tps389008", .driver_data = 8 },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(i2c, tps38900x_id);
> +
> +static const struct of_device_id tps38900x_of_match[] = {
> +	{ .compatible = "ti,tps389004", .data = (void *)4 },
> +	{ .compatible = "ti,tps389006", .data = (void *)6 },
> +	{ .compatible = "ti,tps389008", .data = (void *)8 },

Does this mean programming model is exactly the same for all of them,
just additional channels?

If so, then these could be made compatible in the binding - use
fallback, to the ti,tps389004. I don't insist, though.

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: hwmon: Add TI TPS389004/TPS389006/TPS389008

[Luna Hart]

On 10/1/26 1:17 AM, Krzysztof Kozlowski wrote:
>> +    const: 0
>> +
>> +patternProperties:
>> +  "^vmon@[0-7]$":
> 
> How is this called for other devices? "channel"?

It looks like "channel" is most common (perhaps "input" would be valid
too, from the ti,ina3221).

The V2 patch set changes this to "channel".


>> +      If no child nodes are present, all channels will be exposed for>> +      monitoring. Otherwise, only the provided nodes will be created.
> 
> That's a bit odd behavior. Is it expected for hwmon?

It seems not. Changing this in V2 to enable all channels by default.
Disabling channels by using the status prop explicitly instead.

--

Thank you for the quick response and review of these changes.

All other comments will be addressed in the V2 patch set, which I will
share as soon as possible.


Regards,

Luna Hart

Re: [PATCH 2/2] hwmon: (tps38900x) Add driver for TI TPS389004/TPS389006/TPS389008

[Luna Hart]

On 10/1/26 1:19 AM, Krzysztof Kozlowski wrote:
> On Tue, Sep 29, 2026 at 04:04:05PM -0700, Luna Hart wrote:
>> +static const struct i2c_device_id tps38900x_id[] = {
>> +	{ .name = "tps389004", .driver_data = 4 },
>> +	{ .name = "tps389006", .driver_data = 6 },
>> +	{ .name = "tps389008", .driver_data = 8 },
>> +	{}
>> +};
>> +MODULE_DEVICE_TABLE(i2c, tps38900x_id);
>> +
>> +static const struct of_device_id tps38900x_of_match[] = {
>> +	{ .compatible = "ti,tps389004", .data = (void *)4 },
>> +	{ .compatible = "ti,tps389006", .data = (void *)6 },
>> +	{ .compatible = "ti,tps389008", .data = (void *)8 },
> 
> Does this mean programming model is exactly the same for all of them,
> just additional channels?
> 
> If so, then these could be made compatible in the binding - use
> fallback, to the ti,tps389004. I don't insist, though.
> 
> Best regards,
> Krzysztof
> 

This is correct. The programming is the same across these parts,
they are differentiated just by number of channels. The tps389006
datasheet also applies to the tps389004.

That said, while double checking this, I could not find any
substantial reference for the tps389004. I could not find a
datasheet or any listing of parts with that name. The closest
I found was a mention about a nameless 4 channel version of this
part in some developer documentation. Expectation of a tps389004
came from the previous driver.

As such, I have dropped the tps389004. Leaving only the *006 and
*008, which share a datasheet.

For the fallback: I'm unsure if it would be helpful in practice here.
What does a fallback look like for the bindings? Could you point me
at an example?

Regards,
Luna Hart