[PATCH v4 1/3] dt-bindings: hwmon: ti,tmp108: Add P3T1035,P3T2030

[PATCH v4 1/3] dt-bindings: hwmon: ti,tmp108: Add P3T1035,P3T2030

[Mayank Mahajan]

Document the NXP P3T1035 and P3T2030 compatibles in TMP108.

Signed-off-by: Mayank Mahajan <mayankmahajan.x@nxp.com>
---
V1 -> V2:
- No changes in v2.
V2 -> V3:
- Add P3T1035 fallback for P3T2030 as they are functionally identical.
- Add comment in the description explaining the use of P3T2030.
V3 -> V4:
- Remove comment in the description explaining the schema.

 .../devicetree/bindings/hwmon/ti,tmp108.yaml  | 21 ++++++++++++-------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/hwmon/ti,tmp108.yaml b/Documentation/devicetree/bindings/hwmon/ti,tmp108.yaml
index a6f9319e068d..9f6c9f6fa561 100644
--- a/Documentation/devicetree/bindings/hwmon/ti,tmp108.yaml
+++ b/Documentation/devicetree/bindings/hwmon/ti,tmp108.yaml
@@ -4,27 +4,32 @@
 $id: http://devicetree.org/schemas/hwmon/ti,tmp108.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#

-title: TMP108/P3T1085(NXP) temperature sensor
+title: TMP108/P3T1035/P3T1085/P3T2030 temperature sensor

 maintainers:
   - Krzysztof Kozlowski <krzk@kernel.org>

 description: |
-  The TMP108/P3T1085(NXP) is a digital-output temperature sensor with a
-  dynamically-programmable limit window, and under- and overtemperature
-  alert functions.
+  The TMP108 or NXP P3T Family (P3T1035, P3T1085 and P3T2030) is a digital-
+  output temperature sensor with a dynamically-programmable limit window,
+  and under- and over-temperature alert functions.

-  P3T1085(NXP) support I3C.
+  NXP P3T Family (P3T1035, P3T1085 and P3T2030) supports I3C.

   Datasheets:
     https://www.ti.com/product/TMP108
     https://www.nxp.com/docs/en/data-sheet/P3T1085UK.pdf
+    https://www.nxp.com/docs/en/data-sheet/P3T1035XUK_P3T2030XUK.pdf

 properties:
   compatible:
-    enum:
-      - nxp,p3t1085
-      - ti,tmp108
+    oneOf:
+      - items:
+          - const: nxp,p3t2030
+          - const: nxp,p3t1035
+      - const: nxp,p3t1035
+      - const: nxp,p3t1085
+      - const: ti,tmp108

   interrupts:
     items:

base-commit: 983d014aafb14ee5e4915465bf8948e8f3a723b5
--
2.34.1

[PATCH v4 2/3] hwmon: (tmp108) Add support for P3T1035 and P3T2030

[Mayank Mahajan]

Add support for the P3T1035 & P3T2030 temperature sensor. While mostly
compatible with the TMP108, P3T1035 uses an 8-bit configuration register
instead of the 16-bit layout used by TMP108. Updated driver to handle
this difference during configuration read/write.

Signed-off-by: Mayank Mahajan <mayankmahajan.x@nxp.com>
---
V1 -> V2:
- Disabled hysteresis in is_visible function for P3T1035.
- Added tables for conversion rate similar to the LM75 driver.
- Implemented different bus access depending on the chip being used.
   - Removed regmap for 8 bits; now we are using one regmap as before.
   - Added read and write functions for i2c and i3c for use with regmap.
   - Mapped the 8-bit configuration register to a 16 bit value for P3T1035.
V2 -> V3:
- Remove changes not relevant to adding a new device in the driver.
- Address warnings due to incorrect usage of casting operations.
- Remove the usage of P3T2030 as it's functionally identical to P3T1035.
V3 -> V4:
- Add GENMASK for getting mask for conversion rates.
- Add static arrays for containing sample times for different sensors.
- Remove redundant code such as checking for NULL pointer in probe.
- Improve readability by removing double negation.
- Remove type cast where not required; make reg_buf & val_buf local.

 drivers/hwmon/Kconfig  |   2 +-
 drivers/hwmon/tmp108.c | 203 +++++++++++++++++++++++++++++++++--------
 2 files changed, 164 insertions(+), 41 deletions(-)

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 157678b821fc..31969bddc812 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -2398,7 +2398,7 @@ config SENSORS_TMP108
 	select REGMAP_I3C if I3C
 	help
 	  If you say yes here you get support for Texas Instruments TMP108
-	  sensor chips and NXP P3T1085.
+	  sensor chips, NXP temperature sensors P3T1035, P3T1085 and P3T2030.

 	  This driver can also be built as a module. If so, the module
 	  will be called tmp108.
diff --git a/drivers/hwmon/tmp108.c b/drivers/hwmon/tmp108.c
index 60a237cbedbc..d308e2aed18a 100644
--- a/drivers/hwmon/tmp108.c
+++ b/drivers/hwmon/tmp108.c
@@ -17,9 +17,16 @@
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
+#include <linux/util_macros.h>

 #define	DRIVER_NAME "tmp108"

+enum tmp108_hw_id {
+	P3T1035_ID,		/* For sensors p3t1035 and p3t2030 */
+	P3T1085_ID,
+	TMP108_ID,
+};
+
 #define	TMP108_REG_TEMP		0x00
 #define	TMP108_REG_CONF		0x01
 #define	TMP108_REG_TLOW		0x02
@@ -61,6 +68,7 @@
 #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_CONVRATE_SHIFT		13

 #define TMP108_CONF_HYSTERESIS_MASK	(TMP108_CONF_HYS0|TMP108_CONF_HYS1)
 #define TMP108_HYSTERESIS_0C		0x0000
@@ -70,12 +78,23 @@

 #define TMP108_CONVERSION_TIME_MS	30	/* in milli-seconds */

+#define TMP108_CONF_CR0_POS		13
+#define TMP108_CONF_CR1_POS		14
+#define TMP108_CONF_CONVRATE_FLD	GENMASK(TMP108_CONF_CR1_POS, TMP108_CONF_CR0_POS)
+
 struct tmp108 {
-	struct regmap *regmap;
-	u16 orig_config;
-	unsigned long ready_time;
+	struct regmap		*regmap;
+	u16			orig_config;
+	unsigned long		ready_time;
+	enum tmp108_hw_id	hw_id;
+	bool			config_reg_16bits;
+	ushort			*sample_times;
+	size_t			n_sample_times;
 };

+ushort p3t1035_sample_times[] = {4000, 1000, 250, 125};
+ushort tmp108_sample_times[] = {4000, 1000, 250, 63};
+
 /* convert 12-bit TMP108 register value to milliCelsius */
 static inline int tmp108_temp_reg_to_mC(s16 val)
 {
@@ -101,21 +120,7 @@ static int tmp108_read(struct device *dev, enum hwmon_sensor_types type,
 					  &regval);
 			if (err < 0)
 				return err;
-			switch (regval & TMP108_CONF_CONVRATE_MASK) {
-			case TMP108_CONVRATE_0P25HZ:
-			default:
-				*temp = 4000;
-				break;
-			case TMP108_CONVRATE_1HZ:
-				*temp = 1000;
-				break;
-			case TMP108_CONVRATE_4HZ:
-				*temp = 250;
-				break;
-			case TMP108_CONVRATE_16HZ:
-				*temp = 63;
-				break;
-			}
+			*temp = tmp108->sample_times[FIELD_GET(TMP108_CONF_CONVRATE_FLD, regval)];
 			return 0;
 		}
 		return -EOPNOTSUPP;
@@ -192,22 +197,17 @@ static int tmp108_write(struct device *dev, enum hwmon_sensor_types type,
 {
 	struct tmp108 *tmp108 = dev_get_drvdata(dev);
 	u32 regval, mask;
+	u8 index;
 	int err;

 	if (type == hwmon_chip) {
 		if (attr == hwmon_chip_update_interval) {
-			if (temp < 156)
-				mask = TMP108_CONVRATE_16HZ;
-			else if (temp < 625)
-				mask = TMP108_CONVRATE_4HZ;
-			else if (temp < 2500)
-				mask = TMP108_CONVRATE_1HZ;
-			else
-				mask = TMP108_CONVRATE_0P25HZ;
+			index = find_closest_descending(temp, tmp108->sample_times,
+							tmp108->n_sample_times);
 			return regmap_update_bits(tmp108->regmap,
 						  TMP108_REG_CONF,
 						  TMP108_CONF_CONVRATE_MASK,
-						  mask);
+						  FIELD_PREP(TMP108_CONF_CONVRATE_FLD, index));
 		}
 		return -EOPNOTSUPP;
 	}
@@ -251,6 +251,8 @@ static int tmp108_write(struct device *dev, enum hwmon_sensor_types type,
 static umode_t tmp108_is_visible(const void *data, enum hwmon_sensor_types type,
 				 u32 attr, int channel)
 {
+	const struct tmp108 *tmp108 = data;
+
 	if (type == hwmon_chip && attr == hwmon_chip_update_interval)
 		return 0644;

@@ -264,8 +266,11 @@ static umode_t tmp108_is_visible(const void *data, enum hwmon_sensor_types type,
 		return 0444;
 	case hwmon_temp_min:
 	case hwmon_temp_max:
+		return 0644;
 	case hwmon_temp_min_hyst:
 	case hwmon_temp_max_hyst:
+		if (tmp108->hw_id == P3T1035_ID)
+			return 0;
 		return 0644;
 	default:
 		return 0;
@@ -311,6 +316,106 @@ static bool tmp108_is_volatile_reg(struct device *dev, unsigned int reg)
 	return reg == TMP108_REG_TEMP || reg == TMP108_REG_CONF;
 }

+static int tmp108_i2c_reg_read(void *context, unsigned int reg, unsigned int *val)
+{
+	struct i2c_client *client = context;
+	struct tmp108 *tmp108 = i2c_get_clientdata(client);
+	int ret;
+
+	if (reg == TMP108_REG_CONF && !tmp108->config_reg_16bits) {
+		ret = i2c_smbus_read_byte_data(client, TMP108_REG_CONF);
+		if (ret < 0)
+			return ret;
+		*val = ret << 8;
+		return 0;
+	}
+
+	ret = i2c_smbus_read_word_swapped(client, reg);
+	if (ret < 0)
+		return ret;
+	*val = ret;
+	return 0;
+}
+
+static int tmp108_i2c_reg_write(void *context, unsigned int reg, unsigned int val)
+{
+	struct i2c_client *client = context;
+	struct tmp108 *tmp108 = i2c_get_clientdata(client);
+
+	if (reg == TMP108_REG_CONF && !tmp108->config_reg_16bits)
+		return i2c_smbus_write_byte_data(client, reg, val >> 8);
+	return i2c_smbus_write_word_swapped(client, reg, val);
+}
+
+static const struct regmap_bus tmp108_i2c_regmap_bus = {
+	.reg_read = tmp108_i2c_reg_read,
+	.reg_write = tmp108_i2c_reg_write,
+};
+
+static int tmp108_i3c_reg_read(void *context, unsigned int reg, unsigned int *val)
+{
+	struct i3c_device *i3cdev = context;
+	struct tmp108 *tmp108 = i3cdev_get_drvdata(i3cdev);
+	u8 reg_buf[1], val_buf[2];
+	struct i3c_xfer xfers[] = {
+		{
+			.rnw = false,
+			.len = 1,
+			.data.out = reg_buf,
+		},
+		{
+			.rnw = true,
+			.len = 2,
+			.data.in = val_buf,
+		},
+	};
+	int ret;
+
+	reg_buf[0] = reg;
+
+	if (reg == TMP108_REG_CONF && !tmp108->config_reg_16bits)
+		xfers[1].len--;
+
+	ret = i3c_device_do_xfers(i3cdev, xfers, 2, I3C_SDR);
+	if (ret < 0)
+		return ret;
+
+	*val = val_buf[0] << 8;
+	if (reg != TMP108_REG_CONF || tmp108->config_reg_16bits)
+		*val |= val_buf[1];
+
+	return 0;
+}
+
+static int tmp108_i3c_reg_write(void *context, unsigned int reg, unsigned int val)
+{
+	struct i3c_device *i3cdev = context;
+	struct tmp108 *tmp108 = i3cdev_get_drvdata(i3cdev);
+	u8 val_buf[3];
+	struct i3c_xfer xfers[] = {
+		{
+			.rnw = false,
+			.len = 3,
+			.data.out = val_buf,
+		},
+	};
+
+	val_buf[0] = reg;
+	val_buf[1] = (val >> 8) & 0xff;
+
+	if (reg == TMP108_REG_CONF && !tmp108->config_reg_16bits)
+		xfers[0].len--;
+	else
+		val_buf[2] = val & 0xff;
+
+	return i3c_device_do_xfers(i3cdev, xfers, 1, I3C_SDR);
+}
+
+static const struct regmap_bus tmp108_i3c_regmap_bus = {
+	.reg_read = tmp108_i3c_reg_read,
+	.reg_write = tmp108_i3c_reg_write,
+};
+
 static const struct regmap_config tmp108_regmap_config = {
 	.reg_bits = 8,
 	.val_bits = 16,
@@ -323,7 +428,8 @@ static const struct regmap_config tmp108_regmap_config = {
 	.use_single_write = true,
 };

-static int tmp108_common_probe(struct device *dev, struct regmap *regmap, char *name)
+static int tmp108_common_probe(struct device *dev, struct regmap *regmap, char *name,
+			       enum tmp108_hw_id hw_id)
 {
 	struct device *hwmon_dev;
 	struct tmp108 *tmp108;
@@ -340,6 +446,15 @@ static int tmp108_common_probe(struct device *dev, struct regmap *regmap, char *

 	dev_set_drvdata(dev, tmp108);
 	tmp108->regmap = regmap;
+	tmp108->hw_id = hw_id;
+	tmp108->config_reg_16bits = (hw_id == P3T1035_ID) ? false : true;
+	if (hw_id == P3T1035_ID) {
+		tmp108->sample_times = p3t1035_sample_times;
+		tmp108->n_sample_times = ARRAY_SIZE(p3t1035_sample_times);
+	} else {
+		tmp108->sample_times = tmp108_sample_times;
+		tmp108->n_sample_times = ARRAY_SIZE(tmp108_sample_times);
+	}

 	err = regmap_read(tmp108->regmap, TMP108_REG_CONF, &config);
 	if (err < 0) {
@@ -351,7 +466,6 @@ static int tmp108_common_probe(struct device *dev, struct regmap *regmap, char *
 	/* Only continuous mode is supported. */
 	config &= ~TMP108_CONF_MODE_MASK;
 	config |= TMP108_MODE_CONTINUOUS;
-
 	/* Only comparator mode is supported. */
 	config &= ~TMP108_CONF_TM;

@@ -384,17 +498,20 @@ static int tmp108_probe(struct i2c_client *client)
 {
 	struct device *dev = &client->dev;
 	struct regmap *regmap;
+	enum tmp108_hw_id hw_id;

 	if (!i2c_check_functionality(client->adapter,
-				     I2C_FUNC_SMBUS_WORD_DATA))
+				     I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA))
 		return dev_err_probe(dev, -ENODEV,
 				     "adapter doesn't support SMBus word transactions\n");

-	regmap = devm_regmap_init_i2c(client, &tmp108_regmap_config);
+	regmap = devm_regmap_init(dev, &tmp108_i2c_regmap_bus, client, &tmp108_regmap_config);
 	if (IS_ERR(regmap))
 		return dev_err_probe(dev, PTR_ERR(regmap), "regmap init failed");

-	return tmp108_common_probe(dev, regmap, client->name);
+	hw_id = (unsigned long)i2c_get_match_data(client);
+
+	return tmp108_common_probe(dev, regmap, client->name, hw_id);
 }

 static int tmp108_suspend(struct device *dev)
@@ -420,15 +537,17 @@ static int tmp108_resume(struct device *dev)
 static DEFINE_SIMPLE_DEV_PM_OPS(tmp108_dev_pm_ops, tmp108_suspend, tmp108_resume);

 static const struct i2c_device_id tmp108_i2c_ids[] = {
-	{ "p3t1085" },
-	{ "tmp108" },
-	{ }
+	{ "p3t1035", P3T1035_ID },
+	{ "p3t1085", P3T1085_ID },
+	{ "tmp108", TMP108_ID },
+	{}
 };
 MODULE_DEVICE_TABLE(i2c, tmp108_i2c_ids);

 static const struct of_device_id tmp108_of_ids[] = {
-	{ .compatible = "nxp,p3t1085", },
-	{ .compatible = "ti,tmp108", },
+	{ .compatible = "nxp,p3t1035", .data = (void *)(uintptr_t)P3T1035_ID },
+	{ .compatible = "nxp,p3t1085", .data = (void *)(uintptr_t)P3T1085_ID },
+	{ .compatible = "ti,tmp108", .data = (void *)(uintptr_t)TMP108_ID },
 	{}
 };
 MODULE_DEVICE_TABLE(of, tmp108_of_ids);
@@ -444,7 +563,8 @@ static struct i2c_driver tmp108_driver = {
 };

 static const struct i3c_device_id p3t1085_i3c_ids[] = {
-	I3C_DEVICE(0x011b, 0x1529, NULL),
+	I3C_DEVICE(0x011B, 0x1529, (void *)P3T1085_ID),
+	I3C_DEVICE(0x011B, 0x152B, (void *)P3T1035_ID),
 	{}
 };
 MODULE_DEVICE_TABLE(i3c, p3t1085_i3c_ids);
@@ -453,13 +573,16 @@ static int p3t1085_i3c_probe(struct i3c_device *i3cdev)
 {
 	struct device *dev = i3cdev_to_dev(i3cdev);
 	struct regmap *regmap;
+	const struct i3c_device_id *id;

-	regmap = devm_regmap_init_i3c(i3cdev, &tmp108_regmap_config);
+	regmap = devm_regmap_init(dev, &tmp108_i3c_regmap_bus, i3cdev, &tmp108_regmap_config);
 	if (IS_ERR(regmap))
 		return dev_err_probe(dev, PTR_ERR(regmap),
 				     "Failed to register i3c regmap\n");

-	return tmp108_common_probe(dev, regmap, "p3t1085_i3c");
+	id = i3c_device_match_id(i3cdev, p3t1085_i3c_ids);
+
+	return tmp108_common_probe(dev, regmap, "p3t1085_i3c", (unsigned long)id->data);
 }

 static struct i3c_driver p3t1085_driver = {
--
2.34.1

[PATCH v4 3/3] hwmon: (tmp108) Add P3T1035 and P3T2030 support

[Mayank Mahajan]

Update the hwmon driver documentation for sensors: P3T1035 and P3T2030.

Signed-off-by: Mayank Mahajan <mayankmahajan.x@nxp.com>
---
V1 -> V2:
- No changes in v2.
V2 -> V3:
- No changes in v3.
V3 -> V4:
- No changes in v4.

 Documentation/hwmon/tmp108.rst | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/Documentation/hwmon/tmp108.rst b/Documentation/hwmon/tmp108.rst
index bc4941d98268..c218ea333dd6 100644
--- a/Documentation/hwmon/tmp108.rst
+++ b/Documentation/hwmon/tmp108.rst
@@ -3,6 +3,15 @@ Kernel driver tmp108

 Supported chips:

+  * NXP P3T1035
+
+    Prefix: 'p3t1035'
+
+    Addresses scanned: none
+
+    Datasheet: https://www.nxp.com/docs/en/data-sheet/P3T1035XUK_P3T2030XUK.pdf
+
+
   * NXP P3T1085

     Prefix: 'p3t1085'
@@ -11,6 +20,14 @@ Supported chips:

     Datasheet: https://www.nxp.com/docs/en/data-sheet/P3T1085UK.pdf

+  * NXP P3T2030
+
+    Prefix: 'p3t2030'
+
+    Addresses scanned: none
+
+    Datasheet: https://www.nxp.com/docs/en/data-sheet/P3T1035XUK_P3T2030XUK.pdf
+
   * Texas Instruments TMP108

     Prefix: 'tmp108'
--
2.34.1

Re: [PATCH v4 1/3] dt-bindings: hwmon: ti,tmp108: Add P3T1035,P3T2030

[Krzysztof Kozlowski]

On 16/01/2026 12:35, Mayank Mahajan wrote:
> Document the NXP P3T1035 and P3T2030 compatibles in TMP108.
> 
> Signed-off-by: Mayank Mahajan <mayankmahajan.x@nxp.com>

Read carefully replies you receive.

<form letter>
This is a friendly reminder during the review process.

It looks like you received a tag and forgot to add it.

If you do not know the process, here is a short explanation:
Please add Acked-by/Reviewed-by/Tested-by tags when posting new versions
of patchset, under or above your Signed-off-by tag, unless patch changed
significantly (e.g. new properties added to the DT bindings). Tag is
"received", when provided in a message replied to you on the mailing
list. Tools like b4 can help here. However, there's no need to repost
patches *only* to add the tags. The upstream maintainer will do that for
tags received on the version they apply.

Please read:
https://elixir.bootlin.com/linux/v6.12-rc3/source/Documentation/process/submitting-patches.rst#L577

If a tag was not added on purpose, please state why and what changed.
</form letter>

Best regards,
Krzysztof

Re: [PATCH v4 2/3] hwmon: (tmp108) Add support for P3T1035 and P3T2030

[kernel test robot]

Hi Mayank,

kernel test robot noticed the following build errors:

[auto build test ERROR on 983d014aafb14ee5e4915465bf8948e8f3a723b5]

url:    https://github.com/intel-lab-lkp/linux/commits/Mayank-Mahajan/hwmon-tmp108-Add-support-for-P3T1035-and-P3T2030/20260116-193800
base:   983d014aafb14ee5e4915465bf8948e8f3a723b5
patch link:    https://lore.kernel.org/r/20260116113554.986-2-mayankmahajan.x%40nxp.com
patch subject: [PATCH v4 2/3] hwmon: (tmp108) Add support for P3T1035 and P3T2030
config: i386-buildonly-randconfig-004-20260117 (https://download.01.org/0day-ci/archive/20260117/202601170516.uQw9DKHB-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260117/202601170516.uQw9DKHB-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202601170516.uQw9DKHB-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/hwmon/tmp108.c:123:33: error: call to undeclared function 'FIELD_GET'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     123 |                         *temp = tmp108->sample_times[FIELD_GET(TMP108_CONF_CONVRATE_FLD, regval)];
         |                                                      ^
>> drivers/hwmon/tmp108.c:210:9: error: call to undeclared function 'FIELD_PREP'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     210 |                                                   FIELD_PREP(TMP108_CONF_CONVRATE_FLD, index));
         |                                                   ^
   2 errors generated.


vim +/FIELD_GET +123 drivers/hwmon/tmp108.c

   109	
   110	static int tmp108_read(struct device *dev, enum hwmon_sensor_types type,
   111			       u32 attr, int channel, long *temp)
   112	{
   113		struct tmp108 *tmp108 = dev_get_drvdata(dev);
   114		unsigned int regval;
   115		int err, hyst;
   116	
   117		if (type == hwmon_chip) {
   118			if (attr == hwmon_chip_update_interval) {
   119				err = regmap_read(tmp108->regmap, TMP108_REG_CONF,
   120						  &regval);
   121				if (err < 0)
   122					return err;
 > 123				*temp = tmp108->sample_times[FIELD_GET(TMP108_CONF_CONVRATE_FLD, regval)];
   124				return 0;
   125			}
   126			return -EOPNOTSUPP;
   127		}
   128	
   129		switch (attr) {
   130		case hwmon_temp_input:
   131			/* Is it too early to return a conversion ? */
   132			if (time_before(jiffies, tmp108->ready_time)) {
   133				dev_dbg(dev, "%s: Conversion not ready yet..\n",
   134					__func__);
   135				return -EAGAIN;
   136			}
   137			err = regmap_read(tmp108->regmap, TMP108_REG_TEMP, &regval);
   138			if (err < 0)
   139				return err;
   140			*temp = tmp108_temp_reg_to_mC(regval);
   141			break;
   142		case hwmon_temp_min:
   143		case hwmon_temp_max:
   144			err = regmap_read(tmp108->regmap, attr == hwmon_temp_min ?
   145					  TMP108_REG_TLOW : TMP108_REG_THIGH, &regval);
   146			if (err < 0)
   147				return err;
   148			*temp = tmp108_temp_reg_to_mC(regval);
   149			break;
   150		case hwmon_temp_min_alarm:
   151		case hwmon_temp_max_alarm:
   152			err = regmap_read(tmp108->regmap, TMP108_REG_CONF, &regval);
   153			if (err < 0)
   154				return err;
   155			*temp = !!(regval & (attr == hwmon_temp_min_alarm ?
   156					     TMP108_CONF_FL : TMP108_CONF_FH));
   157			break;
   158		case hwmon_temp_min_hyst:
   159		case hwmon_temp_max_hyst:
   160			err = regmap_read(tmp108->regmap, TMP108_REG_CONF, &regval);
   161			if (err < 0)
   162				return err;
   163			switch (regval & TMP108_CONF_HYSTERESIS_MASK) {
   164			case TMP108_HYSTERESIS_0C:
   165			default:
   166				hyst = 0;
   167				break;
   168			case TMP108_HYSTERESIS_1C:
   169				hyst = 1000;
   170				break;
   171			case TMP108_HYSTERESIS_2C:
   172				hyst = 2000;
   173				break;
   174			case TMP108_HYSTERESIS_4C:
   175				hyst = 4000;
   176				break;
   177			}
   178			err = regmap_read(tmp108->regmap, attr == hwmon_temp_min_hyst ?
   179					  TMP108_REG_TLOW : TMP108_REG_THIGH, &regval);
   180			if (err < 0)
   181				return err;
   182			*temp = tmp108_temp_reg_to_mC(regval);
   183			if (attr == hwmon_temp_min_hyst)
   184				*temp += hyst;
   185			else
   186				*temp -= hyst;
   187			break;
   188		default:
   189			return -EOPNOTSUPP;
   190		}
   191	
   192		return 0;
   193	}
   194	
   195	static int tmp108_write(struct device *dev, enum hwmon_sensor_types type,
   196				u32 attr, int channel, long temp)
   197	{
   198		struct tmp108 *tmp108 = dev_get_drvdata(dev);
   199		u32 regval, mask;
   200		u8 index;
   201		int err;
   202	
   203		if (type == hwmon_chip) {
   204			if (attr == hwmon_chip_update_interval) {
   205				index = find_closest_descending(temp, tmp108->sample_times,
   206								tmp108->n_sample_times);
   207				return regmap_update_bits(tmp108->regmap,
   208							  TMP108_REG_CONF,
   209							  TMP108_CONF_CONVRATE_MASK,
 > 210							  FIELD_PREP(TMP108_CONF_CONVRATE_FLD, index));
   211			}
   212			return -EOPNOTSUPP;
   213		}
   214	
   215		switch (attr) {
   216		case hwmon_temp_min:
   217		case hwmon_temp_max:
   218			temp = clamp_val(temp, TMP108_TEMP_MIN_MC, TMP108_TEMP_MAX_MC);
   219			return regmap_write(tmp108->regmap,
   220					    attr == hwmon_temp_min ?
   221						TMP108_REG_TLOW : TMP108_REG_THIGH,
   222					    tmp108_mC_to_temp_reg(temp));
   223		case hwmon_temp_min_hyst:
   224		case hwmon_temp_max_hyst:
   225			temp = clamp_val(temp, TMP108_TEMP_MIN_MC, TMP108_TEMP_MAX_MC);
   226			err = regmap_read(tmp108->regmap,
   227					  attr == hwmon_temp_min_hyst ?
   228						TMP108_REG_TLOW : TMP108_REG_THIGH,
   229					  &regval);
   230			if (err < 0)
   231				return err;
   232			if (attr == hwmon_temp_min_hyst)
   233				temp -= tmp108_temp_reg_to_mC(regval);
   234			else
   235				temp = tmp108_temp_reg_to_mC(regval) - temp;
   236			if (temp < 500)
   237				mask = TMP108_HYSTERESIS_0C;
   238			else if (temp < 1500)
   239				mask = TMP108_HYSTERESIS_1C;
   240			else if (temp < 3000)
   241				mask = TMP108_HYSTERESIS_2C;
   242			else
   243				mask = TMP108_HYSTERESIS_4C;
   244			return regmap_update_bits(tmp108->regmap, TMP108_REG_CONF,
   245						  TMP108_CONF_HYSTERESIS_MASK, mask);
   246		default:
   247			return -EOPNOTSUPP;
   248		}
   249	}
   250	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v4 2/3] hwmon: (tmp108) Add support for P3T1035 and P3T2030

[Frank Li]

On Fri, Jan 16, 2026 at 05:05:53PM +0530, Mayank Mahajan wrote:
> Add support for the P3T1035 & P3T2030 temperature sensor. While mostly
> compatible with the TMP108, P3T1035 uses an 8-bit configuration register
> instead of the 16-bit layout used by TMP108. Updated driver to handle
> this difference during configuration read/write.
>
> Signed-off-by: Mayank Mahajan <mayankmahajan.x@nxp.com>
> ---
> V1 -> V2:
> - Disabled hysteresis in is_visible function for P3T1035.
> - Added tables for conversion rate similar to the LM75 driver.
> - Implemented different bus access depending on the chip being used.
>    - Removed regmap for 8 bits; now we are using one regmap as before.
>    - Added read and write functions for i2c and i3c for use with regmap.
>    - Mapped the 8-bit configuration register to a 16 bit value for P3T1035.
> V2 -> V3:
> - Remove changes not relevant to adding a new device in the driver.
> - Address warnings due to incorrect usage of casting operations.
> - Remove the usage of P3T2030 as it's functionally identical to P3T1035.
> V3 -> V4:
> - Add GENMASK for getting mask for conversion rates.
> - Add static arrays for containing sample times for different sensors.
> - Remove redundant code such as checking for NULL pointer in probe.
> - Improve readability by removing double negation.
> - Remove type cast where not required; make reg_buf & val_buf local.
>
>  drivers/hwmon/Kconfig  |   2 +-
>  drivers/hwmon/tmp108.c | 203 +++++++++++++++++++++++++++++++++--------
>  2 files changed, 164 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 157678b821fc..31969bddc812 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -2398,7 +2398,7 @@ config SENSORS_TMP108
>  	select REGMAP_I3C if I3C
>  	help
>  	  If you say yes here you get support for Texas Instruments TMP108
> -	  sensor chips and NXP P3T1085.
> +	  sensor chips, NXP temperature sensors P3T1035, P3T1085 and P3T2030.
>
>  	  This driver can also be built as a module. If so, the module
>  	  will be called tmp108.
> diff --git a/drivers/hwmon/tmp108.c b/drivers/hwmon/tmp108.c
> index 60a237cbedbc..d308e2aed18a 100644
> --- a/drivers/hwmon/tmp108.c
> +++ b/drivers/hwmon/tmp108.c
> @@ -17,9 +17,16 @@
>  #include <linux/regmap.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/slab.h>
> +#include <linux/util_macros.h>
>
>  #define	DRIVER_NAME "tmp108"
>
> +enum tmp108_hw_id {
> +	P3T1035_ID,		/* For sensors p3t1035 and p3t2030 */
> +	P3T1085_ID,
> +	TMP108_ID,
> +};
> +
>  #define	TMP108_REG_TEMP		0x00
>  #define	TMP108_REG_CONF		0x01
>  #define	TMP108_REG_TLOW		0x02
> @@ -61,6 +68,7 @@
>  #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_CONVRATE_SHIFT		13
>
>  #define TMP108_CONF_HYSTERESIS_MASK	(TMP108_CONF_HYS0|TMP108_CONF_HYS1)
>  #define TMP108_HYSTERESIS_0C		0x0000
> @@ -70,12 +78,23 @@
>
>  #define TMP108_CONVERSION_TIME_MS	30	/* in milli-seconds */
>
> +#define TMP108_CONF_CR0_POS		13
> +#define TMP108_CONF_CR1_POS		14
> +#define TMP108_CONF_CONVRATE_FLD	GENMASK(TMP108_CONF_CR1_POS, TMP108_CONF_CR0_POS)
> +
>  struct tmp108 {
> -	struct regmap *regmap;
> -	u16 orig_config;
> -	unsigned long ready_time;
> +	struct regmap		*regmap;
> +	u16			orig_config;
> +	unsigned long		ready_time;

don't mix format change in this patch.
Now prefer orignial format, just one space between type and field.

> +	enum tmp108_hw_id	hw_id;
> +	bool			config_reg_16bits;
> +	ushort			*sample_times;
> +	size_t			n_sample_times;
>  };
>
> +ushort p3t1035_sample_times[] = {4000, 1000, 250, 125};
> +ushort tmp108_sample_times[] = {4000, 1000, 250, 63};
> +
>  /* convert 12-bit TMP108 register value to milliCelsius */
>  static inline int tmp108_temp_reg_to_mC(s16 val)
>  {
> @@ -101,21 +120,7 @@ static int tmp108_read(struct device *dev, enum hwmon_sensor_types type,
>  					  &regval);
>  			if (err < 0)
>  				return err;
> -			switch (regval & TMP108_CONF_CONVRATE_MASK) {
> -			case TMP108_CONVRATE_0P25HZ:
> -			default:
> -				*temp = 4000;
> -				break;
> -			case TMP108_CONVRATE_1HZ:
> -				*temp = 1000;
> -				break;
> -			case TMP108_CONVRATE_4HZ:
> -				*temp = 250;
> -				break;
> -			case TMP108_CONVRATE_16HZ:
> -				*temp = 63;
> -				break;
> -			}
> +			*temp = tmp108->sample_times[FIELD_GET(TMP108_CONF_CONVRATE_FLD, regval)];

This code optimation need seperate patch.

>  			return 0;
>  		}
>  		return -EOPNOTSUPP;
> @@ -192,22 +197,17 @@ static int tmp108_write(struct device *dev, enum hwmon_sensor_types type,
>  {
>  	struct tmp108 *tmp108 = dev_get_drvdata(dev);
>  	u32 regval, mask;
> +	u8 index;
>  	int err;
>
>  	if (type == hwmon_chip) {
>  		if (attr == hwmon_chip_update_interval) {
> -			if (temp < 156)
> -				mask = TMP108_CONVRATE_16HZ;
> -			else if (temp < 625)
> -				mask = TMP108_CONVRATE_4HZ;
> -			else if (temp < 2500)
> -				mask = TMP108_CONVRATE_1HZ;
> -			else
> -				mask = TMP108_CONVRATE_0P25HZ;
> +			index = find_closest_descending(temp, tmp108->sample_times,
> +							tmp108->n_sample_times);

Need seperate patch for the code cleanup.

>  			return regmap_update_bits(tmp108->regmap,
>  						  TMP108_REG_CONF,
>  						  TMP108_CONF_CONVRATE_MASK,
> -						  mask);
> +						  FIELD_PREP(TMP108_CONF_CONVRATE_FLD, index));
>  		}
>  		return -EOPNOTSUPP;
>  	}
> @@ -251,6 +251,8 @@ static int tmp108_write(struct device *dev, enum hwmon_sensor_types type,
>  static umode_t tmp108_is_visible(const void *data, enum hwmon_sensor_types type,
>  				 u32 attr, int channel)
>  {
> +	const struct tmp108 *tmp108 = data;
> +
>  	if (type == hwmon_chip && attr == hwmon_chip_update_interval)
>  		return 0644;
>
> @@ -264,8 +266,11 @@ static umode_t tmp108_is_visible(const void *data, enum hwmon_sensor_types type,
>  		return 0444;
>  	case hwmon_temp_min:
>  	case hwmon_temp_max:
> +		return 0644;
>  	case hwmon_temp_min_hyst:
>  	case hwmon_temp_max_hyst:
> +		if (tmp108->hw_id == P3T1035_ID)
> +			return 0;
>  		return 0644;
>  	default:
>  		return 0;
> @@ -311,6 +316,106 @@ static bool tmp108_is_volatile_reg(struct device *dev, unsigned int reg)
>  	return reg == TMP108_REG_TEMP || reg == TMP108_REG_CONF;
>  }
>
> +static int tmp108_i2c_reg_read(void *context, unsigned int reg, unsigned int *val)
> +{
> +	struct i2c_client *client = context;
> +	struct tmp108 *tmp108 = i2c_get_clientdata(client);
> +	int ret;
> +
> +	if (reg == TMP108_REG_CONF && !tmp108->config_reg_16bits) {
> +		ret = i2c_smbus_read_byte_data(client, TMP108_REG_CONF);
> +		if (ret < 0)
> +			return ret;
> +		*val = ret << 8;
> +		return 0;
> +	}
> +
> +	ret = i2c_smbus_read_word_swapped(client, reg);
> +	if (ret < 0)
> +		return ret;
> +	*val = ret;
> +	return 0;
> +}
> +
> +static int tmp108_i2c_reg_write(void *context, unsigned int reg, unsigned int val)
> +{
> +	struct i2c_client *client = context;
> +	struct tmp108 *tmp108 = i2c_get_clientdata(client);
> +
> +	if (reg == TMP108_REG_CONF && !tmp108->config_reg_16bits)
> +		return i2c_smbus_write_byte_data(client, reg, val >> 8);
> +	return i2c_smbus_write_word_swapped(client, reg, val);
> +}
> +
> +static const struct regmap_bus tmp108_i2c_regmap_bus = {
> +	.reg_read = tmp108_i2c_reg_read,
> +	.reg_write = tmp108_i2c_reg_write,
> +};
> +
> +static int tmp108_i3c_reg_read(void *context, unsigned int reg, unsigned int *val)
> +{
> +	struct i3c_device *i3cdev = context;
> +	struct tmp108 *tmp108 = i3cdev_get_drvdata(i3cdev);
> +	u8 reg_buf[1], val_buf[2];
> +	struct i3c_xfer xfers[] = {
> +		{
> +			.rnw = false,
> +			.len = 1,
> +			.data.out = reg_buf,
> +		},
> +		{
> +			.rnw = true,
> +			.len = 2,
> +			.data.in = val_buf,
> +		},
> +	};
> +	int ret;
> +
> +	reg_buf[0] = reg;
> +
> +	if (reg == TMP108_REG_CONF && !tmp108->config_reg_16bits)
> +		xfers[1].len--;
> +
> +	ret = i3c_device_do_xfers(i3cdev, xfers, 2, I3C_SDR);
> +	if (ret < 0)
> +		return ret;
> +
> +	*val = val_buf[0] << 8;
> +	if (reg != TMP108_REG_CONF || tmp108->config_reg_16bits)
> +		*val |= val_buf[1];
> +
> +	return 0;
> +}
> +
> +static int tmp108_i3c_reg_write(void *context, unsigned int reg, unsigned int val)
> +{
> +	struct i3c_device *i3cdev = context;
> +	struct tmp108 *tmp108 = i3cdev_get_drvdata(i3cdev);
> +	u8 val_buf[3];
> +	struct i3c_xfer xfers[] = {
> +		{
> +			.rnw = false,
> +			.len = 3,
> +			.data.out = val_buf,
> +		},
> +	};
> +
> +	val_buf[0] = reg;
> +	val_buf[1] = (val >> 8) & 0xff;
> +
> +	if (reg == TMP108_REG_CONF && !tmp108->config_reg_16bits)
> +		xfers[0].len--;
> +	else
> +		val_buf[2] = val & 0xff;
> +
> +	return i3c_device_do_xfers(i3cdev, xfers, 1, I3C_SDR);
> +}
> +
> +static const struct regmap_bus tmp108_i3c_regmap_bus = {
> +	.reg_read = tmp108_i3c_reg_read,
> +	.reg_write = tmp108_i3c_reg_write,
> +};
> +
>  static const struct regmap_config tmp108_regmap_config = {
>  	.reg_bits = 8,
>  	.val_bits = 16,
> @@ -323,7 +428,8 @@ static const struct regmap_config tmp108_regmap_config = {
>  	.use_single_write = true,
>  };
>
> -static int tmp108_common_probe(struct device *dev, struct regmap *regmap, char *name)
> +static int tmp108_common_probe(struct device *dev, struct regmap *regmap, char *name,
> +			       enum tmp108_hw_id hw_id)
>  {
>  	struct device *hwmon_dev;
>  	struct tmp108 *tmp108;
> @@ -340,6 +446,15 @@ static int tmp108_common_probe(struct device *dev, struct regmap *regmap, char *
>
>  	dev_set_drvdata(dev, tmp108);
>  	tmp108->regmap = regmap;
> +	tmp108->hw_id = hw_id;
> +	tmp108->config_reg_16bits = (hw_id == P3T1035_ID) ? false : true;
> +	if (hw_id == P3T1035_ID) {
> +		tmp108->sample_times = p3t1035_sample_times;
> +		tmp108->n_sample_times = ARRAY_SIZE(p3t1035_sample_times);
> +	} else {
> +		tmp108->sample_times = tmp108_sample_times;
> +		tmp108->n_sample_times = ARRAY_SIZE(tmp108_sample_times);
> +	}
>
>  	err = regmap_read(tmp108->regmap, TMP108_REG_CONF, &config);
>  	if (err < 0) {
> @@ -351,7 +466,6 @@ static int tmp108_common_probe(struct device *dev, struct regmap *regmap, char *
>  	/* Only continuous mode is supported. */
>  	config &= ~TMP108_CONF_MODE_MASK;
>  	config |= TMP108_MODE_CONTINUOUS;
> -
>  	/* Only comparator mode is supported. */
>  	config &= ~TMP108_CONF_TM;
>
> @@ -384,17 +498,20 @@ static int tmp108_probe(struct i2c_client *client)
>  {
>  	struct device *dev = &client->dev;
>  	struct regmap *regmap;
> +	enum tmp108_hw_id hw_id;
>
>  	if (!i2c_check_functionality(client->adapter,
> -				     I2C_FUNC_SMBUS_WORD_DATA))
> +				     I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA))
>  		return dev_err_probe(dev, -ENODEV,
>  				     "adapter doesn't support SMBus word transactions\n");
>
> -	regmap = devm_regmap_init_i2c(client, &tmp108_regmap_config);
> +	regmap = devm_regmap_init(dev, &tmp108_i2c_regmap_bus, client, &tmp108_regmap_config);
>  	if (IS_ERR(regmap))
>  		return dev_err_probe(dev, PTR_ERR(regmap), "regmap init failed");
>
> -	return tmp108_common_probe(dev, regmap, client->name);
> +	hw_id = (unsigned long)i2c_get_match_data(client);
> +
> +	return tmp108_common_probe(dev, regmap, client->name, hw_id);
>  }
>
>  static int tmp108_suspend(struct device *dev)
> @@ -420,15 +537,17 @@ static int tmp108_resume(struct device *dev)
>  static DEFINE_SIMPLE_DEV_PM_OPS(tmp108_dev_pm_ops, tmp108_suspend, tmp108_resume);
>
>  static const struct i2c_device_id tmp108_i2c_ids[] = {
> -	{ "p3t1085" },
> -	{ "tmp108" },
> -	{ }
> +	{ "p3t1035", P3T1035_ID },
> +	{ "p3t1085", P3T1085_ID },
> +	{ "tmp108", TMP108_ID },
> +	{}
>  };
>  MODULE_DEVICE_TABLE(i2c, tmp108_i2c_ids);
>
>  static const struct of_device_id tmp108_of_ids[] = {
> -	{ .compatible = "nxp,p3t1085", },
> -	{ .compatible = "ti,tmp108", },
> +	{ .compatible = "nxp,p3t1035", .data = (void *)(uintptr_t)P3T1035_ID },
> +	{ .compatible = "nxp,p3t1085", .data = (void *)(uintptr_t)P3T1085_ID },
> +	{ .compatible = "ti,tmp108", .data = (void *)(uintptr_t)TMP108_ID },

Do not use device ID, define struct drvdata

	struct tmp108_drvdata
	{
		.samples = p3t1035_sample_times;
		.reg_width = 16,
		...
	}

Frank
>  	{}
>  };
>  MODULE_DEVICE_TABLE(of, tmp108_of_ids);
> @@ -444,7 +563,8 @@ static struct i2c_driver tmp108_driver = {
>  };
>
>  static const struct i3c_device_id p3t1085_i3c_ids[] = {
> -	I3C_DEVICE(0x011b, 0x1529, NULL),
> +	I3C_DEVICE(0x011B, 0x1529, (void *)P3T1085_ID),
> +	I3C_DEVICE(0x011B, 0x152B, (void *)P3T1035_ID),
>  	{}
>  };
>  MODULE_DEVICE_TABLE(i3c, p3t1085_i3c_ids);
> @@ -453,13 +573,16 @@ static int p3t1085_i3c_probe(struct i3c_device *i3cdev)
>  {
>  	struct device *dev = i3cdev_to_dev(i3cdev);
>  	struct regmap *regmap;
> +	const struct i3c_device_id *id;
>
> -	regmap = devm_regmap_init_i3c(i3cdev, &tmp108_regmap_config);
> +	regmap = devm_regmap_init(dev, &tmp108_i3c_regmap_bus, i3cdev, &tmp108_regmap_config);
>  	if (IS_ERR(regmap))
>  		return dev_err_probe(dev, PTR_ERR(regmap),
>  				     "Failed to register i3c regmap\n");
>
> -	return tmp108_common_probe(dev, regmap, "p3t1085_i3c");
> +	id = i3c_device_match_id(i3cdev, p3t1085_i3c_ids);
> +
> +	return tmp108_common_probe(dev, regmap, "p3t1085_i3c", (unsigned long)id->data);
>  }
>
>  static struct i3c_driver p3t1085_driver = {
> --
> 2.34.1

Re: [PATCH v4 2/3] hwmon: (tmp108) Add support for P3T1035 and P3T2030

[Guenter Roeck]

On 1/16/26 14:28, Frank Li wrote:
> On Fri, Jan 16, 2026 at 05:05:53PM +0530, Mayank Mahajan wrote:
>> Add support for the P3T1035 & P3T2030 temperature sensor. While mostly
>> compatible with the TMP108, P3T1035 uses an 8-bit configuration register
>> instead of the 16-bit layout used by TMP108. Updated driver to handle
>> this difference during configuration read/write.
>>
>> Signed-off-by: Mayank Mahajan <mayankmahajan.x@nxp.com>
>> ---
>> V1 -> V2:
>> - Disabled hysteresis in is_visible function for P3T1035.
>> - Added tables for conversion rate similar to the LM75 driver.
>> - Implemented different bus access depending on the chip being used.
>>     - Removed regmap for 8 bits; now we are using one regmap as before.
>>     - Added read and write functions for i2c and i3c for use with regmap.
>>     - Mapped the 8-bit configuration register to a 16 bit value for P3T1035.
>> V2 -> V3:
>> - Remove changes not relevant to adding a new device in the driver.
>> - Address warnings due to incorrect usage of casting operations.
>> - Remove the usage of P3T2030 as it's functionally identical to P3T1035.
>> V3 -> V4:
>> - Add GENMASK for getting mask for conversion rates.
>> - Add static arrays for containing sample times for different sensors.
>> - Remove redundant code such as checking for NULL pointer in probe.
>> - Improve readability by removing double negation.
>> - Remove type cast where not required; make reg_buf & val_buf local.
>>
>>   drivers/hwmon/Kconfig  |   2 +-
>>   drivers/hwmon/tmp108.c | 203 +++++++++++++++++++++++++++++++++--------
>>   2 files changed, 164 insertions(+), 41 deletions(-)
>>
>> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
>> index 157678b821fc..31969bddc812 100644
>> --- a/drivers/hwmon/Kconfig
>> +++ b/drivers/hwmon/Kconfig
>> @@ -2398,7 +2398,7 @@ config SENSORS_TMP108
>>   	select REGMAP_I3C if I3C
>>   	help
>>   	  If you say yes here you get support for Texas Instruments TMP108
>> -	  sensor chips and NXP P3T1085.
>> +	  sensor chips, NXP temperature sensors P3T1035, P3T1085 and P3T2030.
>>
>>   	  This driver can also be built as a module. If so, the module
>>   	  will be called tmp108.
>> diff --git a/drivers/hwmon/tmp108.c b/drivers/hwmon/tmp108.c
>> index 60a237cbedbc..d308e2aed18a 100644
>> --- a/drivers/hwmon/tmp108.c
>> +++ b/drivers/hwmon/tmp108.c
>> @@ -17,9 +17,16 @@
>>   #include <linux/regmap.h>
>>   #include <linux/regulator/consumer.h>
>>   #include <linux/slab.h>
>> +#include <linux/util_macros.h>
>>
>>   #define	DRIVER_NAME "tmp108"
>>
>> +enum tmp108_hw_id {
>> +	P3T1035_ID,		/* For sensors p3t1035 and p3t2030 */
>> +	P3T1085_ID,
>> +	TMP108_ID,
>> +};
>> +
>>   #define	TMP108_REG_TEMP		0x00
>>   #define	TMP108_REG_CONF		0x01
>>   #define	TMP108_REG_TLOW		0x02
>> @@ -61,6 +68,7 @@
>>   #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_CONVRATE_SHIFT		13
>>
>>   #define TMP108_CONF_HYSTERESIS_MASK	(TMP108_CONF_HYS0|TMP108_CONF_HYS1)
>>   #define TMP108_HYSTERESIS_0C		0x0000
>> @@ -70,12 +78,23 @@
>>
>>   #define TMP108_CONVERSION_TIME_MS	30	/* in milli-seconds */
>>
>> +#define TMP108_CONF_CR0_POS		13
>> +#define TMP108_CONF_CR1_POS		14
>> +#define TMP108_CONF_CONVRATE_FLD	GENMASK(TMP108_CONF_CR1_POS, TMP108_CONF_CR0_POS)
>> +
>>   struct tmp108 {
>> -	struct regmap *regmap;
>> -	u16 orig_config;
>> -	unsigned long ready_time;
>> +	struct regmap		*regmap;
>> +	u16			orig_config;
>> +	unsigned long		ready_time;
> 
> don't mix format change in this patch.
> Now prefer orignial format, just one space between type and field.
> 
>> +	enum tmp108_hw_id	hw_id;
>> +	bool			config_reg_16bits;
>> +	ushort			*sample_times;
>> +	size_t			n_sample_times;
>>   };
>>
>> +ushort p3t1035_sample_times[] = {4000, 1000, 250, 125};
>> +ushort tmp108_sample_times[] = {4000, 1000, 250, 63};
>> +
>>   /* convert 12-bit TMP108 register value to milliCelsius */
>>   static inline int tmp108_temp_reg_to_mC(s16 val)
>>   {
>> @@ -101,21 +120,7 @@ static int tmp108_read(struct device *dev, enum hwmon_sensor_types type,
>>   					  &regval);
>>   			if (err < 0)
>>   				return err;
>> -			switch (regval & TMP108_CONF_CONVRATE_MASK) {
>> -			case TMP108_CONVRATE_0P25HZ:
>> -			default:
>> -				*temp = 4000;
>> -				break;
>> -			case TMP108_CONVRATE_1HZ:
>> -				*temp = 1000;
>> -				break;
>> -			case TMP108_CONVRATE_4HZ:
>> -				*temp = 250;
>> -				break;
>> -			case TMP108_CONVRATE_16HZ:
>> -				*temp = 63;
>> -				break;
>> -			}
>> +			*temp = tmp108->sample_times[FIELD_GET(TMP108_CONF_CONVRATE_FLD, regval)];
> 
> This code optimation need seperate patch.
> 
No, There are now two ranges, not just one.

>>   			return 0;
>>   		}
>>   		return -EOPNOTSUPP;
>> @@ -192,22 +197,17 @@ static int tmp108_write(struct device *dev, enum hwmon_sensor_types type,
>>   {
>>   	struct tmp108 *tmp108 = dev_get_drvdata(dev);
>>   	u32 regval, mask;
>> +	u8 index;
>>   	int err;
>>
>>   	if (type == hwmon_chip) {
>>   		if (attr == hwmon_chip_update_interval) {
>> -			if (temp < 156)
>> -				mask = TMP108_CONVRATE_16HZ;
>> -			else if (temp < 625)
>> -				mask = TMP108_CONVRATE_4HZ;
>> -			else if (temp < 2500)
>> -				mask = TMP108_CONVRATE_1HZ;
>> -			else
>> -				mask = TMP108_CONVRATE_0P25HZ;
>> +			index = find_closest_descending(temp, tmp108->sample_times,
>> +							tmp108->n_sample_times);
> 
> Need seperate patch for the code cleanup.
> 

No. There are now two ranges, not just one. That is not a code cleanup,
it is a change necessary to support multiple sets of ranges.

Guenter

>>   			return regmap_update_bits(tmp108->regmap,
>>   						  TMP108_REG_CONF,
>>   						  TMP108_CONF_CONVRATE_MASK,
>> -						  mask);
>> +						  FIELD_PREP(TMP108_CONF_CONVRATE_FLD, index));
>>   		}
>>   		return -EOPNOTSUPP;
>>   	}
>> @@ -251,6 +251,8 @@ static int tmp108_write(struct device *dev, enum hwmon_sensor_types type,
>>   static umode_t tmp108_is_visible(const void *data, enum hwmon_sensor_types type,
>>   				 u32 attr, int channel)
>>   {
>> +	const struct tmp108 *tmp108 = data;
>> +
>>   	if (type == hwmon_chip && attr == hwmon_chip_update_interval)
>>   		return 0644;
>>
>> @@ -264,8 +266,11 @@ static umode_t tmp108_is_visible(const void *data, enum hwmon_sensor_types type,
>>   		return 0444;
>>   	case hwmon_temp_min:
>>   	case hwmon_temp_max:
>> +		return 0644;
>>   	case hwmon_temp_min_hyst:
>>   	case hwmon_temp_max_hyst:
>> +		if (tmp108->hw_id == P3T1035_ID)
>> +			return 0;
>>   		return 0644;
>>   	default:
>>   		return 0;
>> @@ -311,6 +316,106 @@ static bool tmp108_is_volatile_reg(struct device *dev, unsigned int reg)
>>   	return reg == TMP108_REG_TEMP || reg == TMP108_REG_CONF;
>>   }
>>
>> +static int tmp108_i2c_reg_read(void *context, unsigned int reg, unsigned int *val)
>> +{
>> +	struct i2c_client *client = context;
>> +	struct tmp108 *tmp108 = i2c_get_clientdata(client);
>> +	int ret;
>> +
>> +	if (reg == TMP108_REG_CONF && !tmp108->config_reg_16bits) {
>> +		ret = i2c_smbus_read_byte_data(client, TMP108_REG_CONF);
>> +		if (ret < 0)
>> +			return ret;
>> +		*val = ret << 8;
>> +		return 0;
>> +	}
>> +
>> +	ret = i2c_smbus_read_word_swapped(client, reg);
>> +	if (ret < 0)
>> +		return ret;
>> +	*val = ret;
>> +	return 0;
>> +}
>> +
>> +static int tmp108_i2c_reg_write(void *context, unsigned int reg, unsigned int val)
>> +{
>> +	struct i2c_client *client = context;
>> +	struct tmp108 *tmp108 = i2c_get_clientdata(client);
>> +
>> +	if (reg == TMP108_REG_CONF && !tmp108->config_reg_16bits)
>> +		return i2c_smbus_write_byte_data(client, reg, val >> 8);
>> +	return i2c_smbus_write_word_swapped(client, reg, val);
>> +}
>> +
>> +static const struct regmap_bus tmp108_i2c_regmap_bus = {
>> +	.reg_read = tmp108_i2c_reg_read,
>> +	.reg_write = tmp108_i2c_reg_write,
>> +};
>> +
>> +static int tmp108_i3c_reg_read(void *context, unsigned int reg, unsigned int *val)
>> +{
>> +	struct i3c_device *i3cdev = context;
>> +	struct tmp108 *tmp108 = i3cdev_get_drvdata(i3cdev);
>> +	u8 reg_buf[1], val_buf[2];
>> +	struct i3c_xfer xfers[] = {
>> +		{
>> +			.rnw = false,
>> +			.len = 1,
>> +			.data.out = reg_buf,
>> +		},
>> +		{
>> +			.rnw = true,
>> +			.len = 2,
>> +			.data.in = val_buf,
>> +		},
>> +	};
>> +	int ret;
>> +
>> +	reg_buf[0] = reg;
>> +
>> +	if (reg == TMP108_REG_CONF && !tmp108->config_reg_16bits)
>> +		xfers[1].len--;
>> +
>> +	ret = i3c_device_do_xfers(i3cdev, xfers, 2, I3C_SDR);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	*val = val_buf[0] << 8;
>> +	if (reg != TMP108_REG_CONF || tmp108->config_reg_16bits)
>> +		*val |= val_buf[1];
>> +
>> +	return 0;
>> +}
>> +
>> +static int tmp108_i3c_reg_write(void *context, unsigned int reg, unsigned int val)
>> +{
>> +	struct i3c_device *i3cdev = context;
>> +	struct tmp108 *tmp108 = i3cdev_get_drvdata(i3cdev);
>> +	u8 val_buf[3];
>> +	struct i3c_xfer xfers[] = {
>> +		{
>> +			.rnw = false,
>> +			.len = 3,
>> +			.data.out = val_buf,
>> +		},
>> +	};
>> +
>> +	val_buf[0] = reg;
>> +	val_buf[1] = (val >> 8) & 0xff;
>> +
>> +	if (reg == TMP108_REG_CONF && !tmp108->config_reg_16bits)
>> +		xfers[0].len--;
>> +	else
>> +		val_buf[2] = val & 0xff;
>> +
>> +	return i3c_device_do_xfers(i3cdev, xfers, 1, I3C_SDR);
>> +}
>> +
>> +static const struct regmap_bus tmp108_i3c_regmap_bus = {
>> +	.reg_read = tmp108_i3c_reg_read,
>> +	.reg_write = tmp108_i3c_reg_write,
>> +};
>> +
>>   static const struct regmap_config tmp108_regmap_config = {
>>   	.reg_bits = 8,
>>   	.val_bits = 16,
>> @@ -323,7 +428,8 @@ static const struct regmap_config tmp108_regmap_config = {
>>   	.use_single_write = true,
>>   };
>>
>> -static int tmp108_common_probe(struct device *dev, struct regmap *regmap, char *name)
>> +static int tmp108_common_probe(struct device *dev, struct regmap *regmap, char *name,
>> +			       enum tmp108_hw_id hw_id)
>>   {
>>   	struct device *hwmon_dev;
>>   	struct tmp108 *tmp108;
>> @@ -340,6 +446,15 @@ static int tmp108_common_probe(struct device *dev, struct regmap *regmap, char *
>>
>>   	dev_set_drvdata(dev, tmp108);
>>   	tmp108->regmap = regmap;
>> +	tmp108->hw_id = hw_id;
>> +	tmp108->config_reg_16bits = (hw_id == P3T1035_ID) ? false : true;
>> +	if (hw_id == P3T1035_ID) {
>> +		tmp108->sample_times = p3t1035_sample_times;
>> +		tmp108->n_sample_times = ARRAY_SIZE(p3t1035_sample_times);
>> +	} else {
>> +		tmp108->sample_times = tmp108_sample_times;
>> +		tmp108->n_sample_times = ARRAY_SIZE(tmp108_sample_times);
>> +	}
>>
>>   	err = regmap_read(tmp108->regmap, TMP108_REG_CONF, &config);
>>   	if (err < 0) {
>> @@ -351,7 +466,6 @@ static int tmp108_common_probe(struct device *dev, struct regmap *regmap, char *
>>   	/* Only continuous mode is supported. */
>>   	config &= ~TMP108_CONF_MODE_MASK;
>>   	config |= TMP108_MODE_CONTINUOUS;
>> -
>>   	/* Only comparator mode is supported. */
>>   	config &= ~TMP108_CONF_TM;
>>
>> @@ -384,17 +498,20 @@ static int tmp108_probe(struct i2c_client *client)
>>   {
>>   	struct device *dev = &client->dev;
>>   	struct regmap *regmap;
>> +	enum tmp108_hw_id hw_id;
>>
>>   	if (!i2c_check_functionality(client->adapter,
>> -				     I2C_FUNC_SMBUS_WORD_DATA))
>> +				     I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA))
>>   		return dev_err_probe(dev, -ENODEV,
>>   				     "adapter doesn't support SMBus word transactions\n");
>>
>> -	regmap = devm_regmap_init_i2c(client, &tmp108_regmap_config);
>> +	regmap = devm_regmap_init(dev, &tmp108_i2c_regmap_bus, client, &tmp108_regmap_config);
>>   	if (IS_ERR(regmap))
>>   		return dev_err_probe(dev, PTR_ERR(regmap), "regmap init failed");
>>
>> -	return tmp108_common_probe(dev, regmap, client->name);
>> +	hw_id = (unsigned long)i2c_get_match_data(client);
>> +
>> +	return tmp108_common_probe(dev, regmap, client->name, hw_id);
>>   }
>>
>>   static int tmp108_suspend(struct device *dev)
>> @@ -420,15 +537,17 @@ static int tmp108_resume(struct device *dev)
>>   static DEFINE_SIMPLE_DEV_PM_OPS(tmp108_dev_pm_ops, tmp108_suspend, tmp108_resume);
>>
>>   static const struct i2c_device_id tmp108_i2c_ids[] = {
>> -	{ "p3t1085" },
>> -	{ "tmp108" },
>> -	{ }
>> +	{ "p3t1035", P3T1035_ID },
>> +	{ "p3t1085", P3T1085_ID },
>> +	{ "tmp108", TMP108_ID },
>> +	{}
>>   };
>>   MODULE_DEVICE_TABLE(i2c, tmp108_i2c_ids);
>>
>>   static const struct of_device_id tmp108_of_ids[] = {
>> -	{ .compatible = "nxp,p3t1085", },
>> -	{ .compatible = "ti,tmp108", },
>> +	{ .compatible = "nxp,p3t1035", .data = (void *)(uintptr_t)P3T1035_ID },
>> +	{ .compatible = "nxp,p3t1085", .data = (void *)(uintptr_t)P3T1085_ID },
>> +	{ .compatible = "ti,tmp108", .data = (void *)(uintptr_t)TMP108_ID },
> 
> Do not use device ID, define struct drvdata
> 
> 	struct tmp108_drvdata
> 	{
> 		.samples = p3t1035_sample_times;
> 		.reg_width = 16,
> 		...
> 	}
> 
> Frank
>>   	{}
>>   };
>>   MODULE_DEVICE_TABLE(of, tmp108_of_ids);
>> @@ -444,7 +563,8 @@ static struct i2c_driver tmp108_driver = {
>>   };
>>
>>   static const struct i3c_device_id p3t1085_i3c_ids[] = {
>> -	I3C_DEVICE(0x011b, 0x1529, NULL),
>> +	I3C_DEVICE(0x011B, 0x1529, (void *)P3T1085_ID),
>> +	I3C_DEVICE(0x011B, 0x152B, (void *)P3T1035_ID),
>>   	{}
>>   };
>>   MODULE_DEVICE_TABLE(i3c, p3t1085_i3c_ids);
>> @@ -453,13 +573,16 @@ static int p3t1085_i3c_probe(struct i3c_device *i3cdev)
>>   {
>>   	struct device *dev = i3cdev_to_dev(i3cdev);
>>   	struct regmap *regmap;
>> +	const struct i3c_device_id *id;
>>
>> -	regmap = devm_regmap_init_i3c(i3cdev, &tmp108_regmap_config);
>> +	regmap = devm_regmap_init(dev, &tmp108_i3c_regmap_bus, i3cdev, &tmp108_regmap_config);
>>   	if (IS_ERR(regmap))
>>   		return dev_err_probe(dev, PTR_ERR(regmap),
>>   				     "Failed to register i3c regmap\n");
>>
>> -	return tmp108_common_probe(dev, regmap, "p3t1085_i3c");
>> +	id = i3c_device_match_id(i3cdev, p3t1085_i3c_ids);
>> +
>> +	return tmp108_common_probe(dev, regmap, "p3t1085_i3c", (unsigned long)id->data);
>>   }
>>
>>   static struct i3c_driver p3t1085_driver = {
>> --
>> 2.34.1

Re: [PATCH v4 2/3] hwmon: (tmp108) Add support for P3T1035 and P3T2030

[Frank Li]

On Fri, Jan 16, 2026 at 04:36:59PM -0800, Guenter Roeck wrote:
> On 1/16/26 14:28, Frank Li wrote:
> > On Fri, Jan 16, 2026 at 05:05:53PM +0530, Mayank Mahajan wrote:
> > > Add support for the P3T1035 & P3T2030 temperature sensor. While mostly
> > > compatible with the TMP108, P3T1035 uses an 8-bit configuration register
> > > instead of the 16-bit layout used by TMP108. Updated driver to handle
> > > this difference during configuration read/write.
> > >
> > > Signed-off-by: Mayank Mahajan <mayankmahajan.x@nxp.com>
> > > ---
> > > V1 -> V2:
> > > - Disabled hysteresis in is_visible function for P3T1035.
> > > - Added tables for conversion rate similar to the LM75 driver.
> > > - Implemented different bus access depending on the chip being used.
> > >     - Removed regmap for 8 bits; now we are using one regmap as before.
> > >     - Added read and write functions for i2c and i3c for use with regmap.
> > >     - Mapped the 8-bit configuration register to a 16 bit value for P3T1035.
> > > V2 -> V3:
> > > - Remove changes not relevant to adding a new device in the driver.
> > > - Address warnings due to incorrect usage of casting operations.
> > > - Remove the usage of P3T2030 as it's functionally identical to P3T1035.
> > > V3 -> V4:
> > > - Add GENMASK for getting mask for conversion rates.
> > > - Add static arrays for containing sample times for different sensors.
> > > - Remove redundant code such as checking for NULL pointer in probe.
> > > - Improve readability by removing double negation.
> > > - Remove type cast where not required; make reg_buf & val_buf local.
> > >
> > >   drivers/hwmon/Kconfig  |   2 +-
> > >   drivers/hwmon/tmp108.c | 203 +++++++++++++++++++++++++++++++++--------
> > >   2 files changed, 164 insertions(+), 41 deletions(-)
> > >
> > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > > index 157678b821fc..31969bddc812 100644
> > > --- a/drivers/hwmon/Kconfig
> > > +++ b/drivers/hwmon/Kconfig
> > > @@ -2398,7 +2398,7 @@ config SENSORS_TMP108
> > >   	select REGMAP_I3C if I3C
> > >   	help
> > >   	  If you say yes here you get support for Texas Instruments TMP108
> > > -	  sensor chips and NXP P3T1085.
> > > +	  sensor chips, NXP temperature sensors P3T1035, P3T1085 and P3T2030.
> > >
> > >   	  This driver can also be built as a module. If so, the module
> > >   	  will be called tmp108.
> > > diff --git a/drivers/hwmon/tmp108.c b/drivers/hwmon/tmp108.c
> > > index 60a237cbedbc..d308e2aed18a 100644
> > > --- a/drivers/hwmon/tmp108.c
> > > +++ b/drivers/hwmon/tmp108.c
> > > @@ -17,9 +17,16 @@
> > >   #include <linux/regmap.h>
> > >   #include <linux/regulator/consumer.h>
> > >   #include <linux/slab.h>
> > > +#include <linux/util_macros.h>
> > >
> > >   #define	DRIVER_NAME "tmp108"
> > >
> > > +enum tmp108_hw_id {
> > > +	P3T1035_ID,		/* For sensors p3t1035 and p3t2030 */
> > > +	P3T1085_ID,
> > > +	TMP108_ID,
> > > +};
> > > +
> > >   #define	TMP108_REG_TEMP		0x00
> > >   #define	TMP108_REG_CONF		0x01
> > >   #define	TMP108_REG_TLOW		0x02
> > > @@ -61,6 +68,7 @@
> > >   #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_CONVRATE_SHIFT		13
> > >
> > >   #define TMP108_CONF_HYSTERESIS_MASK	(TMP108_CONF_HYS0|TMP108_CONF_HYS1)
> > >   #define TMP108_HYSTERESIS_0C		0x0000
> > > @@ -70,12 +78,23 @@
> > >
> > >   #define TMP108_CONVERSION_TIME_MS	30	/* in milli-seconds */
> > >
> > > +#define TMP108_CONF_CR0_POS		13
> > > +#define TMP108_CONF_CR1_POS		14
> > > +#define TMP108_CONF_CONVRATE_FLD	GENMASK(TMP108_CONF_CR1_POS, TMP108_CONF_CR0_POS)
> > > +
> > >   struct tmp108 {
> > > -	struct regmap *regmap;
> > > -	u16 orig_config;
> > > -	unsigned long ready_time;
> > > +	struct regmap		*regmap;
> > > +	u16			orig_config;
> > > +	unsigned long		ready_time;
> >
> > don't mix format change in this patch.
> > Now prefer orignial format, just one space between type and field.
> >
> > > +	enum tmp108_hw_id	hw_id;
> > > +	bool			config_reg_16bits;
> > > +	ushort			*sample_times;
> > > +	size_t			n_sample_times;
> > >   };
> > >
> > > +ushort p3t1035_sample_times[] = {4000, 1000, 250, 125};
> > > +ushort tmp108_sample_times[] = {4000, 1000, 250, 63};
> > > +
> > >   /* convert 12-bit TMP108 register value to milliCelsius */
> > >   static inline int tmp108_temp_reg_to_mC(s16 val)
> > >   {
> > > @@ -101,21 +120,7 @@ static int tmp108_read(struct device *dev, enum hwmon_sensor_types type,
> > >   					  &regval);
> > >   			if (err < 0)
> > >   				return err;
> > > -			switch (regval & TMP108_CONF_CONVRATE_MASK) {
> > > -			case TMP108_CONVRATE_0P25HZ:
> > > -			default:
> > > -				*temp = 4000;
> > > -				break;
> > > -			case TMP108_CONVRATE_1HZ:
> > > -				*temp = 1000;
> > > -				break;
> > > -			case TMP108_CONVRATE_4HZ:
> > > -				*temp = 250;
> > > -				break;
> > > -			case TMP108_CONVRATE_16HZ:
> > > -				*temp = 63;
> > > -				break;
> > > -			}
> > > +			*temp = tmp108->sample_times[FIELD_GET(TMP108_CONF_CONVRATE_FLD, regval)];
> >
> > This code optimation need seperate patch.
> >
> No, There are now two ranges, not just one.

Previous code use swtich case to handle sample. now convert to use a map
array tmp108_sample_times[].

My means use a small patch convert existed switch case to map array firstly
, then add second range, which make code easy to review.

>
> > >   			return 0;
> > >   		}
> > >   		return -EOPNOTSUPP;
> > > @@ -192,22 +197,17 @@ static int tmp108_write(struct device *dev, enum hwmon_sensor_types type,
> > >   {
> > >   	struct tmp108 *tmp108 = dev_get_drvdata(dev);
> > >   	u32 regval, mask;
> > > +	u8 index;
> > >   	int err;
> > >
> > >   	if (type == hwmon_chip) {
> > >   		if (attr == hwmon_chip_update_interval) {
> > > -			if (temp < 156)
> > > -				mask = TMP108_CONVRATE_16HZ;
> > > -			else if (temp < 625)
> > > -				mask = TMP108_CONVRATE_4HZ;
> > > -			else if (temp < 2500)
> > > -				mask = TMP108_CONVRATE_1HZ;
> > > -			else
> > > -				mask = TMP108_CONVRATE_0P25HZ;
> > > +			index = find_closest_descending(temp, tmp108->sample_times,
> > > +							tmp108->n_sample_times);
> >
> > Need seperate patch for the code cleanup.
> >
>
> No. There are now two ranges, not just one. That is not a code cleanup,
> it is a change necessary to support multiple sets of ranges.


The smae here, this code just convert if else to helper function
find_closest_descending(), which is not related work, which add new
part support.

Frank
>
> Guenter
>
> > >   			return regmap_update_bits(tmp108->regmap,
> > >   						  TMP108_REG_CONF,
> > >   						  TMP108_CONF_CONVRATE_MASK,
> > > -						  mask);
> > > +						  FIELD_PREP(TMP108_CONF_CONVRATE_FLD, index));
> > >   		}
> > >   		return -EOPNOTSUPP;
> > >   	}
> > > @@ -251,6 +251,8 @@ static int tmp108_write(struct device *dev, enum hwmon_sensor_types type,
> > >   static umode_t tmp108_is_visible(const void *data, enum hwmon_sensor_types type,
> > >   				 u32 attr, int channel)
> > >   {
> > > +	const struct tmp108 *tmp108 = data;
> > > +
> > >   	if (type == hwmon_chip && attr == hwmon_chip_update_interval)
> > >   		return 0644;
> > >
> > > @@ -264,8 +266,11 @@ static umode_t tmp108_is_visible(const void *data, enum hwmon_sensor_types type,
> > >   		return 0444;
> > >   	case hwmon_temp_min:
> > >   	case hwmon_temp_max:
> > > +		return 0644;
> > >   	case hwmon_temp_min_hyst:
> > >   	case hwmon_temp_max_hyst:
> > > +		if (tmp108->hw_id == P3T1035_ID)
> > > +			return 0;
> > >   		return 0644;
> > >   	default:
> > >   		return 0;
> > > @@ -311,6 +316,106 @@ static bool tmp108_is_volatile_reg(struct device *dev, unsigned int reg)
> > >   	return reg == TMP108_REG_TEMP || reg == TMP108_REG_CONF;
> > >   }
> > >
> > > +static int tmp108_i2c_reg_read(void *context, unsigned int reg, unsigned int *val)
> > > +{
> > > +	struct i2c_client *client = context;
> > > +	struct tmp108 *tmp108 = i2c_get_clientdata(client);
> > > +	int ret;
> > > +
> > > +	if (reg == TMP108_REG_CONF && !tmp108->config_reg_16bits) {
> > > +		ret = i2c_smbus_read_byte_data(client, TMP108_REG_CONF);
> > > +		if (ret < 0)
> > > +			return ret;
> > > +		*val = ret << 8;
> > > +		return 0;
> > > +	}
> > > +
> > > +	ret = i2c_smbus_read_word_swapped(client, reg);
> > > +	if (ret < 0)
> > > +		return ret;
> > > +	*val = ret;
> > > +	return 0;
> > > +}
> > > +
> > > +static int tmp108_i2c_reg_write(void *context, unsigned int reg, unsigned int val)
> > > +{
> > > +	struct i2c_client *client = context;
> > > +	struct tmp108 *tmp108 = i2c_get_clientdata(client);
> > > +
> > > +	if (reg == TMP108_REG_CONF && !tmp108->config_reg_16bits)
> > > +		return i2c_smbus_write_byte_data(client, reg, val >> 8);
> > > +	return i2c_smbus_write_word_swapped(client, reg, val);
> > > +}
> > > +
> > > +static const struct regmap_bus tmp108_i2c_regmap_bus = {
> > > +	.reg_read = tmp108_i2c_reg_read,
> > > +	.reg_write = tmp108_i2c_reg_write,
> > > +};
> > > +
> > > +static int tmp108_i3c_reg_read(void *context, unsigned int reg, unsigned int *val)
> > > +{
> > > +	struct i3c_device *i3cdev = context;
> > > +	struct tmp108 *tmp108 = i3cdev_get_drvdata(i3cdev);
> > > +	u8 reg_buf[1], val_buf[2];
> > > +	struct i3c_xfer xfers[] = {
> > > +		{
> > > +			.rnw = false,
> > > +			.len = 1,
> > > +			.data.out = reg_buf,
> > > +		},
> > > +		{
> > > +			.rnw = true,
> > > +			.len = 2,
> > > +			.data.in = val_buf,
> > > +		},
> > > +	};
> > > +	int ret;
> > > +
> > > +	reg_buf[0] = reg;
> > > +
> > > +	if (reg == TMP108_REG_CONF && !tmp108->config_reg_16bits)
> > > +		xfers[1].len--;
> > > +
> > > +	ret = i3c_device_do_xfers(i3cdev, xfers, 2, I3C_SDR);
> > > +	if (ret < 0)
> > > +		return ret;
> > > +
> > > +	*val = val_buf[0] << 8;
> > > +	if (reg != TMP108_REG_CONF || tmp108->config_reg_16bits)
> > > +		*val |= val_buf[1];
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static int tmp108_i3c_reg_write(void *context, unsigned int reg, unsigned int val)
> > > +{
> > > +	struct i3c_device *i3cdev = context;
> > > +	struct tmp108 *tmp108 = i3cdev_get_drvdata(i3cdev);
> > > +	u8 val_buf[3];
> > > +	struct i3c_xfer xfers[] = {
> > > +		{
> > > +			.rnw = false,
> > > +			.len = 3,
> > > +			.data.out = val_buf,
> > > +		},
> > > +	};
> > > +
> > > +	val_buf[0] = reg;
> > > +	val_buf[1] = (val >> 8) & 0xff;
> > > +
> > > +	if (reg == TMP108_REG_CONF && !tmp108->config_reg_16bits)
> > > +		xfers[0].len--;
> > > +	else
> > > +		val_buf[2] = val & 0xff;
> > > +
> > > +	return i3c_device_do_xfers(i3cdev, xfers, 1, I3C_SDR);
> > > +}
> > > +
> > > +static const struct regmap_bus tmp108_i3c_regmap_bus = {
> > > +	.reg_read = tmp108_i3c_reg_read,
> > > +	.reg_write = tmp108_i3c_reg_write,
> > > +};
> > > +
> > >   static const struct regmap_config tmp108_regmap_config = {
> > >   	.reg_bits = 8,
> > >   	.val_bits = 16,
> > > @@ -323,7 +428,8 @@ static const struct regmap_config tmp108_regmap_config = {
> > >   	.use_single_write = true,
> > >   };
> > >
> > > -static int tmp108_common_probe(struct device *dev, struct regmap *regmap, char *name)
> > > +static int tmp108_common_probe(struct device *dev, struct regmap *regmap, char *name,
> > > +			       enum tmp108_hw_id hw_id)
> > >   {
> > >   	struct device *hwmon_dev;
> > >   	struct tmp108 *tmp108;
> > > @@ -340,6 +446,15 @@ static int tmp108_common_probe(struct device *dev, struct regmap *regmap, char *
> > >
> > >   	dev_set_drvdata(dev, tmp108);
> > >   	tmp108->regmap = regmap;
> > > +	tmp108->hw_id = hw_id;
> > > +	tmp108->config_reg_16bits = (hw_id == P3T1035_ID) ? false : true;
> > > +	if (hw_id == P3T1035_ID) {
> > > +		tmp108->sample_times = p3t1035_sample_times;
> > > +		tmp108->n_sample_times = ARRAY_SIZE(p3t1035_sample_times);
> > > +	} else {
> > > +		tmp108->sample_times = tmp108_sample_times;
> > > +		tmp108->n_sample_times = ARRAY_SIZE(tmp108_sample_times);
> > > +	}
> > >
> > >   	err = regmap_read(tmp108->regmap, TMP108_REG_CONF, &config);
> > >   	if (err < 0) {
> > > @@ -351,7 +466,6 @@ static int tmp108_common_probe(struct device *dev, struct regmap *regmap, char *
> > >   	/* Only continuous mode is supported. */
> > >   	config &= ~TMP108_CONF_MODE_MASK;
> > >   	config |= TMP108_MODE_CONTINUOUS;
> > > -
> > >   	/* Only comparator mode is supported. */
> > >   	config &= ~TMP108_CONF_TM;
> > >
> > > @@ -384,17 +498,20 @@ static int tmp108_probe(struct i2c_client *client)
> > >   {
> > >   	struct device *dev = &client->dev;
> > >   	struct regmap *regmap;
> > > +	enum tmp108_hw_id hw_id;
> > >
> > >   	if (!i2c_check_functionality(client->adapter,
> > > -				     I2C_FUNC_SMBUS_WORD_DATA))
> > > +				     I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA))
> > >   		return dev_err_probe(dev, -ENODEV,
> > >   				     "adapter doesn't support SMBus word transactions\n");
> > >
> > > -	regmap = devm_regmap_init_i2c(client, &tmp108_regmap_config);
> > > +	regmap = devm_regmap_init(dev, &tmp108_i2c_regmap_bus, client, &tmp108_regmap_config);
> > >   	if (IS_ERR(regmap))
> > >   		return dev_err_probe(dev, PTR_ERR(regmap), "regmap init failed");
> > >
> > > -	return tmp108_common_probe(dev, regmap, client->name);
> > > +	hw_id = (unsigned long)i2c_get_match_data(client);
> > > +
> > > +	return tmp108_common_probe(dev, regmap, client->name, hw_id);
> > >   }
> > >
> > >   static int tmp108_suspend(struct device *dev)
> > > @@ -420,15 +537,17 @@ static int tmp108_resume(struct device *dev)
> > >   static DEFINE_SIMPLE_DEV_PM_OPS(tmp108_dev_pm_ops, tmp108_suspend, tmp108_resume);
> > >
> > >   static const struct i2c_device_id tmp108_i2c_ids[] = {
> > > -	{ "p3t1085" },
> > > -	{ "tmp108" },
> > > -	{ }
> > > +	{ "p3t1035", P3T1035_ID },
> > > +	{ "p3t1085", P3T1085_ID },
> > > +	{ "tmp108", TMP108_ID },
> > > +	{}
> > >   };
> > >   MODULE_DEVICE_TABLE(i2c, tmp108_i2c_ids);
> > >
> > >   static const struct of_device_id tmp108_of_ids[] = {
> > > -	{ .compatible = "nxp,p3t1085", },
> > > -	{ .compatible = "ti,tmp108", },
> > > +	{ .compatible = "nxp,p3t1035", .data = (void *)(uintptr_t)P3T1035_ID },
> > > +	{ .compatible = "nxp,p3t1085", .data = (void *)(uintptr_t)P3T1085_ID },
> > > +	{ .compatible = "ti,tmp108", .data = (void *)(uintptr_t)TMP108_ID },
> >
> > Do not use device ID, define struct drvdata
> >
> > 	struct tmp108_drvdata
> > 	{
> > 		.samples = p3t1035_sample_times;
> > 		.reg_width = 16,
> > 		...
> > 	}
> >
> > Frank
> > >   	{}
> > >   };
> > >   MODULE_DEVICE_TABLE(of, tmp108_of_ids);
> > > @@ -444,7 +563,8 @@ static struct i2c_driver tmp108_driver = {
> > >   };
> > >
> > >   static const struct i3c_device_id p3t1085_i3c_ids[] = {
> > > -	I3C_DEVICE(0x011b, 0x1529, NULL),
> > > +	I3C_DEVICE(0x011B, 0x1529, (void *)P3T1085_ID),
> > > +	I3C_DEVICE(0x011B, 0x152B, (void *)P3T1035_ID),
> > >   	{}
> > >   };
> > >   MODULE_DEVICE_TABLE(i3c, p3t1085_i3c_ids);
> > > @@ -453,13 +573,16 @@ static int p3t1085_i3c_probe(struct i3c_device *i3cdev)
> > >   {
> > >   	struct device *dev = i3cdev_to_dev(i3cdev);
> > >   	struct regmap *regmap;
> > > +	const struct i3c_device_id *id;
> > >
> > > -	regmap = devm_regmap_init_i3c(i3cdev, &tmp108_regmap_config);
> > > +	regmap = devm_regmap_init(dev, &tmp108_i3c_regmap_bus, i3cdev, &tmp108_regmap_config);
> > >   	if (IS_ERR(regmap))
> > >   		return dev_err_probe(dev, PTR_ERR(regmap),
> > >   				     "Failed to register i3c regmap\n");
> > >
> > > -	return tmp108_common_probe(dev, regmap, "p3t1085_i3c");
> > > +	id = i3c_device_match_id(i3cdev, p3t1085_i3c_ids);
> > > +
> > > +	return tmp108_common_probe(dev, regmap, "p3t1085_i3c", (unsigned long)id->data);
> > >   }
> > >
> > >   static struct i3c_driver p3t1085_driver = {
> > > --
> > > 2.34.1
>

Re: [PATCH v4 2/3] hwmon: (tmp108) Add support for P3T1035 and P3T2030

[Guenter Roeck]

Frank,

On 1/18/26 09:30, Frank Li wrote:
...
>>>> @@ -101,21 +120,7 @@ static int tmp108_read(struct device *dev, enum hwmon_sensor_types type,
>>>>    					  &regval);
>>>>    			if (err < 0)
>>>>    				return err;
>>>> -			switch (regval & TMP108_CONF_CONVRATE_MASK) {
>>>> -			case TMP108_CONVRATE_0P25HZ:
>>>> -			default:
>>>> -				*temp = 4000;
>>>> -				break;
>>>> -			case TMP108_CONVRATE_1HZ:
>>>> -				*temp = 1000;
>>>> -				break;
>>>> -			case TMP108_CONVRATE_4HZ:
>>>> -				*temp = 250;
>>>> -				break;
>>>> -			case TMP108_CONVRATE_16HZ:
>>>> -				*temp = 63;
>>>> -				break;
>>>> -			}
>>>> +			*temp = tmp108->sample_times[FIELD_GET(TMP108_CONF_CONVRATE_FLD, regval)];
>>>
>>> This code optimation need seperate patch.
>>>
>> No, There are now two ranges, not just one.
> 
> Previous code use swtich case to handle sample. now convert to use a map
> array tmp108_sample_times[].
> 
> My means use a small patch convert existed switch case to map array firstly
> , then add second range, which make code easy to review.
> 

You are not signed up as reviewer of this driver. I am, and I don't want to see
a separate patch just for this because it means extra work for me.

Thanks,
Guenter