* [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
* [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 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
* 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