* [PATCH v3 1/3] dt-bindings: hwmon: ti,tmp108: Add P3T1035,P3T2030
@ 2026-01-15 11:14 Mayank Mahajan
2026-01-15 11:14 ` [PATCH v3 2/3] hwmon: (tmp108) Add support for P3T1035 and P3T2030 Mayank Mahajan
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Mayank Mahajan @ 2026-01-15 11:14 UTC (permalink / raw)
To: linux, corbet, robh, krzk+dt, conor+dt, linux-hwmon, devicetree,
linux-doc, linux-kernel
Cc: priyanka.jain, vikash.bansal, 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.
.../devicetree/bindings/hwmon/ti,tmp108.yaml | 24 ++++++++++++-------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/Documentation/devicetree/bindings/hwmon/ti,tmp108.yaml b/Documentation/devicetree/bindings/hwmon/ti,tmp108.yaml
index a6f9319e068d..1f540c623de6 100644
--- a/Documentation/devicetree/bindings/hwmon/ti,tmp108.yaml
+++ b/Documentation/devicetree/bindings/hwmon/ti,tmp108.yaml
@@ -4,27 +4,35 @@
$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
+
+ P3T2030 is functionally identical to P3T1035. Hence, device tree nodes that
+ use the P3T2030 must provide a fallback compatible string of "nxp,p3t1035"
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:
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v3 2/3] hwmon: (tmp108) Add support for P3T1035 and P3T2030 2026-01-15 11:14 [PATCH v3 1/3] dt-bindings: hwmon: ti,tmp108: Add P3T1035,P3T2030 Mayank Mahajan @ 2026-01-15 11:14 ` Mayank Mahajan 2026-01-16 4:31 ` kernel test robot 2026-01-16 6:11 ` Guenter Roeck 2026-01-15 11:14 ` [PATCH v3 3/3] hwmon: (tmp108) Add P3T1035 and P3T2030 support Mayank Mahajan 2026-01-16 7:22 ` [PATCH v3 1/3] dt-bindings: hwmon: ti,tmp108: Add P3T1035,P3T2030 Krzysztof Kozlowski 2 siblings, 2 replies; 6+ messages in thread From: Mayank Mahajan @ 2026-01-15 11:14 UTC (permalink / raw) To: linux, corbet, robh, krzk+dt, conor+dt, linux-hwmon, devicetree, linux-doc, linux-kernel Cc: priyanka.jain, vikash.bansal, 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. drivers/hwmon/Kconfig | 2 +- drivers/hwmon/tmp108.c | 227 +++++++++++++++++++++++++++++++++-------- 2 files changed, 186 insertions(+), 43 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..38a2203c3bd9 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 @@ -71,11 +79,21 @@ #define TMP108_CONVERSION_TIME_MS 30 /* in milli-seconds */ 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; + u8 reg_buf[1]; + u8 val_buf[3]; + unsigned int sample_times[4]; }; +static const u16 tmp108_sample_set_masks[] = { 3 << TMP108_CONVRATE_SHIFT, + 2 << TMP108_CONVRATE_SHIFT, + 1 << TMP108_CONVRATE_SHIFT, + 0 << TMP108_CONVRATE_SHIFT }; + /* convert 12-bit TMP108 register value to milliCelsius */ static inline int tmp108_temp_reg_to_mC(s16 val) { @@ -94,6 +112,8 @@ static int tmp108_read(struct device *dev, enum hwmon_sensor_types type, struct tmp108 *tmp108 = dev_get_drvdata(dev); unsigned int regval; int err, hyst; + u16 conv_rate; + u8 index; if (type == hwmon_chip) { if (attr == hwmon_chip_update_interval) { @@ -101,21 +121,10 @@ static int tmp108_read(struct device *dev, enum hwmon_sensor_types type, ®val); 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; - } + conv_rate = regval & TMP108_CONF_CONVRATE_MASK; + index = find_closest_descending(conv_rate, tmp108_sample_set_masks, + (int)ARRAY_SIZE(tmp108_sample_set_masks)); + *temp = tmp108->sample_times[index]; return 0; } return -EOPNOTSUPP; @@ -192,22 +201,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(temp, tmp108->sample_times, + (int)ARRAY_SIZE(tmp108->sample_times)); return regmap_update_bits(tmp108->regmap, TMP108_REG_CONF, TMP108_CONF_CONVRATE_MASK, - mask); + tmp108_sample_set_masks[index]); } return -EOPNOTSUPP; } @@ -251,6 +255,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 +270,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 +320,105 @@ 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); + else + ret = i2c_smbus_read_word_swapped(client, reg); + + if (ret < 0) + return ret; + + if (reg == TMP108_REG_CONF && !tmp108->config_reg_16bits) + *val = ret << 8; + else + *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); + struct i3c_xfer xfers[] = { + { + .rnw = false, + .len = 1, + .data.out = tmp108->reg_buf, + }, + { + .rnw = true, + .len = 2, + .data.in = tmp108->val_buf, + }, + }; + int ret; + + tmp108->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 = tmp108->val_buf[0] << 8; + if (!(reg == TMP108_REG_CONF && !tmp108->config_reg_16bits)) + *val |= tmp108->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); + struct i3c_xfer xfers[] = { + { + .rnw = false, + .len = 3, + .data.out = tmp108->val_buf, + }, + }; + + tmp108->val_buf[0] = reg; + tmp108->val_buf[1] = (val >> 8) & 0xff; + + if (reg == TMP108_REG_CONF && !tmp108->config_reg_16bits) + xfers[0].len--; + else + tmp108->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 +431,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 +449,14 @@ 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) + memcpy(tmp108->sample_times, (unsigned int[]){ 125, 250, 1000, 4000 }, + sizeof(tmp108->sample_times)); + else + memcpy(tmp108->sample_times, (unsigned int[]){ 63, 250, 1000, 4000 }, + sizeof(tmp108->sample_times)); err = regmap_read(tmp108->regmap, TMP108_REG_CONF, &config); if (err < 0) { @@ -351,7 +468,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 +500,33 @@ static int tmp108_probe(struct i2c_client *client) { struct device *dev = &client->dev; struct regmap *regmap; + enum tmp108_hw_id hw_id; + const void *of_data; 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); + /* Prefer OF match data (DT-first systems) */ + of_data = device_get_match_data(&client->dev); + if (of_data) { + hw_id = (unsigned long)of_data; + } else { + /* Fall back to legacy I2C ID table */ + const struct i2c_device_id *id = i2c_client_get_device_id(client); + + if (!id) { + return dev_err_probe(dev, -ENODEV, "No matching device ID for i2c device\n"); + } + hw_id = (unsigned long)id->driver_data; + } + + return tmp108_common_probe(dev, regmap, client->name, hw_id); } static int tmp108_suspend(struct device *dev) @@ -420,16 +552,18 @@ 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 }, + { /* sentinel */ }, }; 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 }, + { /* sentinel */ }, }; MODULE_DEVICE_TABLE(of, tmp108_of_ids); @@ -444,8 +578,9 @@ 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), + { /* sentinel */ }, }; MODULE_DEVICE_TABLE(i3c, p3t1085_i3c_ids); @@ -453,13 +588,21 @@ 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; + enum tmp108_hw_id hw_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); + if (!id) { + return dev_err_probe(dev, -ENODEV, "No matching device ID for i3c device\n"); + } + hw_id = (enum tmp108_hw_id)(uintptr_t)id->data; + + return tmp108_common_probe(dev, regmap, "p3t1085_i3c", hw_id); } static struct i3c_driver p3t1085_driver = { -- 2.34.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/3] hwmon: (tmp108) Add support for P3T1035 and P3T2030 2026-01-15 11:14 ` [PATCH v3 2/3] hwmon: (tmp108) Add support for P3T1035 and P3T2030 Mayank Mahajan @ 2026-01-16 4:31 ` kernel test robot 2026-01-16 6:11 ` Guenter Roeck 1 sibling, 0 replies; 6+ messages in thread From: kernel test robot @ 2026-01-16 4:31 UTC (permalink / raw) To: Mayank Mahajan, linux, corbet, robh, krzk+dt, conor+dt, linux-hwmon, devicetree, linux-doc, linux-kernel Cc: oe-kbuild-all, priyanka.jain, vikash.bansal, Mayank Mahajan Hi Mayank, kernel test robot noticed the following build warnings: [auto build test WARNING on groeck-staging/hwmon-next] [also build test WARNING on linus/master v6.19-rc5] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Mayank-Mahajan/hwmon-tmp108-Add-support-for-P3T1035-and-P3T2030/20260115-191549 base: https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next patch link: https://lore.kernel.org/r/20260115111418.1851-2-mayankmahajan.x%40nxp.com patch subject: [PATCH v3 2/3] hwmon: (tmp108) Add support for P3T1035 and P3T2030 config: xtensa-randconfig-r122-20260116 (https://download.01.org/0day-ci/archive/20260116/202601161234.jWOgBbs8-lkp@intel.com/config) compiler: xtensa-linux-gcc (GCC) 8.5.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260116/202601161234.jWOgBbs8-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/202601161234.jWOgBbs8-lkp@intel.com/ sparse warnings: (new ones prefixed by >>) drivers/hwmon/tmp108.c: note: in included file (through arch/xtensa/include/asm/bitops.h, include/linux/bitops.h, include/linux/log2.h, ...): arch/xtensa/include/asm/processor.h:105:2: sparse: sparse: Unsupported xtensa ABI arch/xtensa/include/asm/processor.h:135:2: sparse: sparse: Unsupported Xtensa ABI >> drivers/hwmon/tmp108.c:455:17: sparse: sparse: macro "memcpy" passed 6 arguments, but takes just 3 drivers/hwmon/tmp108.c:458:17: sparse: sparse: macro "memcpy" passed 6 arguments, but takes just 3 drivers/hwmon/tmp108.c:618:1: sparse: sparse: bad integer constant expression drivers/hwmon/tmp108.c:618:1: sparse: sparse: static assertion failed: "MODULE_INFO(author, ...) contains embedded NUL byte" drivers/hwmon/tmp108.c:619:1: sparse: sparse: bad integer constant expression drivers/hwmon/tmp108.c:619:1: sparse: sparse: static assertion failed: "MODULE_INFO(description, ...) contains embedded NUL byte" drivers/hwmon/tmp108.c:620:1: sparse: sparse: bad integer constant expression drivers/hwmon/tmp108.c:620:1: sparse: sparse: static assertion failed: "MODULE_INFO(license, ...) contains embedded NUL byte" vim +/memcpy +455 drivers/hwmon/tmp108.c 433 434 static int tmp108_common_probe(struct device *dev, struct regmap *regmap, char *name, 435 enum tmp108_hw_id hw_id) 436 { 437 struct device *hwmon_dev; 438 struct tmp108 *tmp108; 439 u32 config; 440 int err; 441 442 err = devm_regulator_get_enable(dev, "vcc"); 443 if (err) 444 return dev_err_probe(dev, err, "Failed to enable regulator\n"); 445 446 tmp108 = devm_kzalloc(dev, sizeof(*tmp108), GFP_KERNEL); 447 if (!tmp108) 448 return -ENOMEM; 449 450 dev_set_drvdata(dev, tmp108); 451 tmp108->regmap = regmap; 452 tmp108->hw_id = hw_id; 453 tmp108->config_reg_16bits = (hw_id == P3T1035_ID) ? false : true; 454 if (hw_id == P3T1035_ID) > 455 memcpy(tmp108->sample_times, (unsigned int[]){ 125, 250, 1000, 4000 }, 456 sizeof(tmp108->sample_times)); 457 else 458 memcpy(tmp108->sample_times, (unsigned int[]){ 63, 250, 1000, 4000 }, 459 sizeof(tmp108->sample_times)); 460 461 err = regmap_read(tmp108->regmap, TMP108_REG_CONF, &config); 462 if (err < 0) { 463 dev_err(dev, "error reading config register: %d", err); 464 return err; 465 } 466 tmp108->orig_config = config; 467 468 /* Only continuous mode is supported. */ 469 config &= ~TMP108_CONF_MODE_MASK; 470 config |= TMP108_MODE_CONTINUOUS; 471 /* Only comparator mode is supported. */ 472 config &= ~TMP108_CONF_TM; 473 474 err = regmap_write(tmp108->regmap, TMP108_REG_CONF, config); 475 if (err < 0) { 476 dev_err(dev, "error writing config register: %d", err); 477 return err; 478 } 479 480 tmp108->ready_time = jiffies; 481 if ((tmp108->orig_config & TMP108_CONF_MODE_MASK) == 482 TMP108_MODE_SHUTDOWN) 483 tmp108->ready_time += 484 msecs_to_jiffies(TMP108_CONVERSION_TIME_MS); 485 486 err = devm_add_action_or_reset(dev, tmp108_restore_config, tmp108); 487 if (err) { 488 dev_err(dev, "add action or reset failed: %d", err); 489 return err; 490 } 491 492 hwmon_dev = devm_hwmon_device_register_with_info(dev, name, 493 tmp108, 494 &tmp108_chip_info, 495 NULL); 496 return PTR_ERR_OR_ZERO(hwmon_dev); 497 } 498 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/3] hwmon: (tmp108) Add support for P3T1035 and P3T2030 2026-01-15 11:14 ` [PATCH v3 2/3] hwmon: (tmp108) Add support for P3T1035 and P3T2030 Mayank Mahajan 2026-01-16 4:31 ` kernel test robot @ 2026-01-16 6:11 ` Guenter Roeck 1 sibling, 0 replies; 6+ messages in thread From: Guenter Roeck @ 2026-01-16 6:11 UTC (permalink / raw) To: Mayank Mahajan, corbet, robh, krzk+dt, conor+dt, linux-hwmon, devicetree, linux-doc, linux-kernel Cc: priyanka.jain, vikash.bansal On 1/15/26 03:14, 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. > > drivers/hwmon/Kconfig | 2 +- > drivers/hwmon/tmp108.c | 227 +++++++++++++++++++++++++++++++++-------- > 2 files changed, 186 insertions(+), 43 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..38a2203c3bd9 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 > @@ -71,11 +79,21 @@ > #define TMP108_CONVERSION_TIME_MS 30 /* in milli-seconds */ > > 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; > + u8 reg_buf[1]; > + u8 val_buf[3]; > + unsigned int sample_times[4]; > }; > > +static const u16 tmp108_sample_set_masks[] = { 3 << TMP108_CONVRATE_SHIFT, > + 2 << TMP108_CONVRATE_SHIFT, > + 1 << TMP108_CONVRATE_SHIFT, > + 0 << TMP108_CONVRATE_SHIFT }; > + Unnecessary. See below. > /* convert 12-bit TMP108 register value to milliCelsius */ > static inline int tmp108_temp_reg_to_mC(s16 val) > { > @@ -94,6 +112,8 @@ static int tmp108_read(struct device *dev, enum hwmon_sensor_types type, > struct tmp108 *tmp108 = dev_get_drvdata(dev); > unsigned int regval; > int err, hyst; > + u16 conv_rate; > + u8 index; > > if (type == hwmon_chip) { > if (attr == hwmon_chip_update_interval) { > @@ -101,21 +121,10 @@ static int tmp108_read(struct device *dev, enum hwmon_sensor_types type, > ®val); > 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; > - } > + conv_rate = regval & TMP108_CONF_CONVRATE_MASK; > + index = find_closest_descending(conv_rate, tmp108_sample_set_masks, > + (int)ARRAY_SIZE(tmp108_sample_set_masks)); > + *temp = tmp108->sample_times[index]; (regval & TMP108_CONF_CONVRATE_MASK) >> TMP108_CONVRATE_SHIFT, or alternatively FIELD_GET(TMP108_CONF_CONVRATE_MASK, regval), yields 0..3. With a sample_times array of { 4000, 1000, 250, 63 } or { 4000, 1000, 250, 125 }, the code above could simply be *temp = tmp108->sample_times[FIELD_GET(TMP108_CONF_CONVRATE_MASK, regval)]; which would both be easier to understand and much simpler. > return 0; > } > return -EOPNOTSUPP; > @@ -192,22 +201,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(temp, tmp108->sample_times, > + (int)ARRAY_SIZE(tmp108->sample_times)); I don't see why the type cast would be needed. Other users of find_closest() don't need it either. > return regmap_update_bits(tmp108->regmap, > TMP108_REG_CONF, > TMP108_CONF_CONVRATE_MASK, > - mask); > + tmp108_sample_set_masks[index]); Use GENMASK(). > } > return -EOPNOTSUPP; > } > @@ -251,6 +255,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 +270,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 +320,105 @@ 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); > + else > + ret = i2c_smbus_read_word_swapped(client, reg); > + > + if (ret < 0) > + return ret; > + > + if (reg == TMP108_REG_CONF && !tmp108->config_reg_16bits) > + *val = ret << 8; > + else > + *val = ret; This evaluates reg and tmp108->config_reg_16bits twice. Try 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; instead to reduce runtime overhead. > + > + 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); > + struct i3c_xfer xfers[] = { > + { > + .rnw = false, > + .len = 1, > + .data.out = tmp108->reg_buf, > + }, > + { > + .rnw = true, > + .len = 2, > + .data.in = tmp108->val_buf, What is the point of having reg_buf and val_buf allocated instead of just using local variables/arrays ? > + }, > + }; > + int ret; > + > + tmp108->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 = tmp108->val_buf[0] << 8; > + if (!(reg == TMP108_REG_CONF && !tmp108->config_reg_16bits)) Please refrain from using double negations. if (reg != TMP108_REG_CONF || tmp108->config_reg_16bits) is much easier to understand. > + *val |= tmp108->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); > + struct i3c_xfer xfers[] = { > + { > + .rnw = false, > + .len = 3, > + .data.out = tmp108->val_buf, > + }, > + }; > + > + tmp108->val_buf[0] = reg; > + tmp108->val_buf[1] = (val >> 8) & 0xff; > + > + if (reg == TMP108_REG_CONF && !tmp108->config_reg_16bits) > + xfers[0].len--; > + else > + tmp108->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 +431,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 +449,14 @@ 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) > + memcpy(tmp108->sample_times, (unsigned int[]){ 125, 250, 1000, 4000 }, > + sizeof(tmp108->sample_times)); > + else > + memcpy(tmp108->sample_times, (unsigned int[]){ 63, 250, 1000, 4000 }, > + sizeof(tmp108->sample_times)); You'd think that the repeated 0-day complaints have an effect. Just make tmp108->sample_times a pointer and create two ushort arrays where the values match the index values. struct tmp108 { ushort *sample_times; }; ushort p3t_1035_sample_times[] = {4000, 1000, 250, 125}; ushort tmp108_sample_times[] = {4000, 1000, 250, 125}; if (hw_id == P3T1035_ID) tmp108->sample_times = p3t_1035_sample_times; else tmp108->sample_times = tmp108_sample_times; Something like tmp108->sample_times = (ushort []) {4000, 1000, 250, 125}; might work as well, but I did not test it. The memcpy is really unnecessary here. > > err = regmap_read(tmp108->regmap, TMP108_REG_CONF, &config); > if (err < 0) { > @@ -351,7 +468,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 +500,33 @@ static int tmp108_probe(struct i2c_client *client) > { > struct device *dev = &client->dev; > struct regmap *regmap; > + enum tmp108_hw_id hw_id; > + const void *of_data; > > 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); > + /* Prefer OF match data (DT-first systems) */ > + of_data = device_get_match_data(&client->dev); > + if (of_data) { > + hw_id = (unsigned long)of_data; > + } else { > + /* Fall back to legacy I2C ID table */ > + const struct i2c_device_id *id = i2c_client_get_device_id(client); > + > + if (!id) { > + return dev_err_probe(dev, -ENODEV, "No matching device ID for i2c device\n"); > + } > + hw_id = (unsigned long)id->driver_data; > + } That complexity is unnecessary. Just use i2c_get_match_data(). > + > + return tmp108_common_probe(dev, regmap, client->name, hw_id); > } > > static int tmp108_suspend(struct device *dev) > @@ -420,16 +552,18 @@ 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 }, > + { /* sentinel */ }, > }; > 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 }, > + { /* sentinel */ }, > }; > MODULE_DEVICE_TABLE(of, tmp108_of_ids); > > @@ -444,8 +578,9 @@ 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), > + { /* sentinel */ }, I know that some people like that comment, and I accept it for new drivers. I do _not_ accept it being changed in existing drivers. > }; > MODULE_DEVICE_TABLE(i3c, p3t1085_i3c_ids); > > @@ -453,13 +588,21 @@ 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; > + enum tmp108_hw_id hw_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); > + if (!id) { > + return dev_err_probe(dev, -ENODEV, "No matching device ID for i3c device\n"); > + } Unnecessary error check since the id already matches or the function would not have been called. > + hw_id = (enum tmp108_hw_id)(uintptr_t)id->data; > + > + return tmp108_common_probe(dev, regmap, "p3t1085_i3c", hw_id); > } > > static struct i3c_driver p3t1085_driver = { > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 3/3] hwmon: (tmp108) Add P3T1035 and P3T2030 support 2026-01-15 11:14 [PATCH v3 1/3] dt-bindings: hwmon: ti,tmp108: Add P3T1035,P3T2030 Mayank Mahajan 2026-01-15 11:14 ` [PATCH v3 2/3] hwmon: (tmp108) Add support for P3T1035 and P3T2030 Mayank Mahajan @ 2026-01-15 11:14 ` Mayank Mahajan 2026-01-16 7:22 ` [PATCH v3 1/3] dt-bindings: hwmon: ti,tmp108: Add P3T1035,P3T2030 Krzysztof Kozlowski 2 siblings, 0 replies; 6+ messages in thread From: Mayank Mahajan @ 2026-01-15 11:14 UTC (permalink / raw) To: linux, corbet, robh, krzk+dt, conor+dt, linux-hwmon, devicetree, linux-doc, linux-kernel Cc: priyanka.jain, vikash.bansal, 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. 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/3] dt-bindings: hwmon: ti,tmp108: Add P3T1035,P3T2030 2026-01-15 11:14 [PATCH v3 1/3] dt-bindings: hwmon: ti,tmp108: Add P3T1035,P3T2030 Mayank Mahajan 2026-01-15 11:14 ` [PATCH v3 2/3] hwmon: (tmp108) Add support for P3T1035 and P3T2030 Mayank Mahajan 2026-01-15 11:14 ` [PATCH v3 3/3] hwmon: (tmp108) Add P3T1035 and P3T2030 support Mayank Mahajan @ 2026-01-16 7:22 ` Krzysztof Kozlowski 2 siblings, 0 replies; 6+ messages in thread From: Krzysztof Kozlowski @ 2026-01-16 7:22 UTC (permalink / raw) To: Mayank Mahajan, linux, corbet, robh, krzk+dt, conor+dt, linux-hwmon, devicetree, linux-doc, linux-kernel Cc: priyanka.jain, vikash.bansal On 15/01/2026 12:14, Mayank Mahajan wrote: > 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. > > .../devicetree/bindings/hwmon/ti,tmp108.yaml | 24 ++++++++++++------- > 1 file changed, 16 insertions(+), 8 deletions(-) > > diff --git a/Documentation/devicetree/bindings/hwmon/ti,tmp108.yaml b/Documentation/devicetree/bindings/hwmon/ti,tmp108.yaml > index a6f9319e068d..1f540c623de6 100644 > --- a/Documentation/devicetree/bindings/hwmon/ti,tmp108.yaml > +++ b/Documentation/devicetree/bindings/hwmon/ti,tmp108.yaml > @@ -4,27 +4,35 @@ > $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 > + > + P3T2030 is functionally identical to P3T1035. Hence, device tree nodes that > + use the P3T2030 must provide a fallback compatible string of "nxp,p3t1035" Drop the sentence. Schema already tells that. Never repeat the schema in free text. It's like adding comments to code: ptr = kzalloc() /* Check for null pointer after allocation */ if (!ptr) return -ENOMEM This is worse coding. Write concise, clearly readable code. With this fixed: Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com> Best regards, Krzysztof ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-01-16 7:22 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-01-15 11:14 [PATCH v3 1/3] dt-bindings: hwmon: ti,tmp108: Add P3T1035,P3T2030 Mayank Mahajan 2026-01-15 11:14 ` [PATCH v3 2/3] hwmon: (tmp108) Add support for P3T1035 and P3T2030 Mayank Mahajan 2026-01-16 4:31 ` kernel test robot 2026-01-16 6:11 ` Guenter Roeck 2026-01-15 11:14 ` [PATCH v3 3/3] hwmon: (tmp108) Add P3T1035 and P3T2030 support Mayank Mahajan 2026-01-16 7:22 ` [PATCH v3 1/3] dt-bindings: hwmon: ti,tmp108: Add P3T1035,P3T2030 Krzysztof Kozlowski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox