* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.