[PATCH v3 0/3] hwmon: (lm75) add MAX31875 support

[PATCH v3 0/3] hwmon: (lm75) add MAX31875 support

[John Erasmus Mari Geronimo]

Add support for the Analog Devices MAX31875 I2C temperature sensor by
extending the existing lm75 driver.

The MAX31875 provides a 16-bit configuration register, selectable
resolution of 8, 9, 10, or 12 bits, conversion rates of 125 ms, 250 ms,
1 s, or 4 s, and an alarm output.

Changes since v2:
- dt-bindings: expand the commit message to explain why the MAX31875
  needs its own compatible string instead of reusing an existing
  lm75-family one, and pick up Conor Dooley's Acked-by given
  conditional on that fix.
- driver: drop the bogus .resolutions table, which incorrectly tied
  resolution to the sample-time index even though the two are
  independently configurable on this chip (Guenter Roeck).
- driver: use the BIT() idiom for the alarm bit read to match the
  rest of the file (Guenter Roeck).

Changes since v1:
- Rework the implementation to use the existing lm75 driver instead of
  the previously selected max31827 base driver.
- Add the MAX31875 Device Tree binding, documentation, and driver
  configuration for resolution, conversion rate, and alarm reporting.

John Erasmus Mari Geronimo (3):
  dt-bindings: hwmon: lm75: add MAX31875
  hwmon: (lm75) add MAX31875 documentation
  hwmon: (lm75) add MAX31875 support

 .../devicetree/bindings/hwmon/lm75.yaml       |  1 +
 Documentation/hwmon/lm75.rst                  |  6 +++--
 drivers/hwmon/Kconfig                         |  2 +-
 drivers/hwmon/lm75.c                          | 24 +++++++++++++++++++
 4 files changed, 30 insertions(+), 3 deletions(-)

-- 
2.34.1

[PATCH v3 1/3] dt-bindings: hwmon: lm75: add MAX31875

[John Erasmus Mari Geronimo]

Add MAX31875 compatible string to lm75 dt-bindings.

The MAX31875 cannot reuse an existing lm75-family compatible such as
adi,adt75: it has a 16-bit configuration register (instead of 8-bit),
four independently selectable conversion rates (125 ms, 250 ms, 1 s,
4 s) and four independently selectable resolutions (8, 9, 10, 12
bits), and reports its alarm through a different configuration
register bit. These differences require dedicated handling in the
lm75 driver, so a distinct compatible string is needed to select it.

Signed-off-by: John Erasmus Mari Geronimo <johnerasmusmari.geronimo@analog.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---
 Documentation/devicetree/bindings/hwmon/lm75.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/hwmon/lm75.yaml b/Documentation/devicetree/bindings/hwmon/lm75.yaml
index b48bf3fd721fe..d56f3ec299105 100644
--- a/Documentation/devicetree/bindings/hwmon/lm75.yaml
+++ b/Documentation/devicetree/bindings/hwmon/lm75.yaml
@@ -14,6 +14,7 @@ properties:
   compatible:
     enum:
       - adi,adt75
+      - adi,max31875
       - ams,as6200
       - atmel,at30ts74
       - dallas,ds1775
-- 
2.34.1

[PATCH v3 2/3] hwmon: (lm75) add MAX31875 documentation

[John Erasmus Mari Geronimo]

Document the Maxim MAX31875 I2C temperature sensor as a supported
chip in the lm75 driver documentation.

Signed-off-by: John Erasmus Mari Geronimo <johnerasmusmari.geronimo@analog.com>
---
 Documentation/hwmon/lm75.rst | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/hwmon/lm75.rst b/Documentation/hwmon/lm75.rst
index ca46754e028b3..54a6a75e8a71b 100644
--- a/Documentation/hwmon/lm75.rst
+++ b/Documentation/hwmon/lm75.rst
@@ -66,9 +66,9 @@ Supported chips:
 
 	       http://www.microchip.com/
 
-  * Analog Devices ADT75
+  * Analog Devices ADT75, MAX31875
 
-    Prefix: 'adt75'
+    Prefix: 'adt75', 'max31875'
 
     Addresses scanned: none
 
@@ -76,6 +76,8 @@ Supported chips:
 
 	       https://www.analog.com/adt75
 
+	       https://www.analog.com/max31875
+
   * ST Microelectronics STDS75
 
     Prefix: 'stds75'
-- 
2.34.1

[PATCH v3 3/3] hwmon: (lm75) add MAX31875 support

[John Erasmus Mari Geronimo]

Add support for the Maxim MAX31875 I2C temperature sensor by
extending the lm75 driver. The MAX31875 features a 16-bit
configuration register, configurable resolution (8, 9, 10, or
12 bits), and selectable conversion rates (125ms, 250ms, 1s, 4s).

Suggested-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: John Erasmus Mari Geronimo <johnerasmusmari.geronimo@analog.com>
---
 drivers/hwmon/Kconfig |  2 +-
 drivers/hwmon/lm75.c  | 24 ++++++++++++++++++++++++
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index e4c4f2b09732d..c68b6c3c2f866 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1525,7 +1525,7 @@ config SENSORS_LM75
 	  If you say yes here you get support for one common type of
 	  temperature sensor chip, with models including:
 
-		- Analog Devices ADT75
+		- Analog Devices ADT75, MAX31875
 		- Atmel (now Microchip) AT30TS74
 		- Dallas Semiconductor DS75, DS1775 and DS7505
 		- Global Mixed-mode Technology (GMT) G751
diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
index 104149a03bad0..2fc5659a3d6bf 100644
--- a/drivers/hwmon/lm75.c
+++ b/drivers/hwmon/lm75.c
@@ -39,6 +39,7 @@ enum lm75_type {		/* keep sorted in alphabetical order */
 	max6625,
 	max6626,
 	max31725,
+	max31875,
 	mcp980x,
 	p3t1750,
 	p3t1755,
@@ -223,6 +224,14 @@ static const struct lm75_params device_params[] = {
 		.default_resolution = 16,
 		.default_sample_time = MSEC_PER_SEC / 20,
 	},
+	[max31875] = {
+		.config_reg_16bits = true,
+		.default_resolution = 10,
+		.default_sample_time = 4000,
+		.num_sample_times = 4,
+		.sample_times = (unsigned int []){ 125, 250, 1000, 4000 },
+		.alarm = true,
+	},
 	[tcn75] = {
 		.default_resolution = 9,
 		.default_sample_time = MSEC_PER_SEC / 18,
@@ -421,6 +430,9 @@ static int lm75_read(struct device *dev, enum hwmon_sensor_types type,
 			case tmp112:
 				*val = !!(regval & BIT(13)) == !!(regval & BIT(2));
 				break;
+			case max31875:
+				*val = !!(regval & BIT(7));
+				break;
 			default:
 				return -EINVAL;
 			}
@@ -495,6 +507,13 @@ static int lm75_update_interval(struct device *dev, long val)
 			return err;
 		data->sample_time = data->params->sample_times[index];
 		break;
+	case max31875:
+		err = regmap_update_bits(data->regmap, LM75_REG_CONF,
+					 0x0600, (3 - index) << 9);
+		if (err < 0)
+			return err;
+		data->sample_time = data->params->sample_times[index];
+		break;
 	case pct2075:
 		err = regmap_write(data->regmap, PCT2075_REG_IDLE, index + 1);
 		if (err)
@@ -848,6 +867,7 @@ static const struct i2c_device_id lm75_i2c_ids[] = {
 	{ .name = "max6626", .driver_data = max6626 },
 	{ .name = "max31725", .driver_data = max31725 },
 	{ .name = "max31726", .driver_data = max31725 },
+	{ .name = "max31875", .driver_data = max31875 },
 	{ .name = "mcp980x", .driver_data = mcp980x },
 	{ .name = "p3t1750", .driver_data = p3t1750 },
 	{ .name = "p3t1755", .driver_data = p3t1755 },
@@ -905,6 +925,10 @@ static const struct of_device_id lm75_of_match[] = {
 		.compatible = "adi,adt75",
 		.data = (void *)adt75
 	},
+	{
+		.compatible = "adi,max31875",
+		.data = (void *)max31875
+	},
 	{
 		.compatible = "ams,as6200",
 		.data = (void *)as6200
-- 
2.34.1

Re: [PATCH v3 1/3] dt-bindings: hwmon: lm75: add MAX31875

[sashiko-bot]

> Add MAX31875 compatible string to lm75 dt-bindings.
> 
> The MAX31875 cannot reuse an existing lm75-family compatible such as
> adi,adt75: it has a 16-bit configuration register (instead of 8-bit),
> four independently selectable conversion rates (125 ms, 250 ms, 1 s,
> 4 s) and four independently selectable resolutions (8, 9, 10, 12
> bits), and reports its alarm through a different configuration
> register bit. These differences require dedicated handling in the
> lm75 driver, so a distinct compatible string is needed to select it.
> 
> Signed-off-by: John Erasmus Mari Geronimo <johnerasmusmari.geronimo@analog.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009115617.1976-1-johnerasmusmari.geronimo@analog.com?part=1

Re: [PATCH v3 2/3] hwmon: (lm75) add MAX31875 documentation

[sashiko-bot]

> Document the Maxim MAX31875 I2C temperature sensor as a supported
> chip in the lm75 driver documentation.
> 
> Signed-off-by: John Erasmus Mari Geronimo <johnerasmusmari.geronimo@analog.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009115617.1976-1-johnerasmusmari.geronimo@analog.com?part=2

Re: [PATCH v3 3/3] hwmon: (lm75) add MAX31875 support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] hwmon: lm75: unconditional 16-bit polarity mask corrupts max31875 conversion rate
- [Medium] hwmon: lm75: Fix incorrect bit mask for MAX31875 conversion rate
- [Medium] hwmon: lm75: max31875 state desynchronization on probe
--

commit 5136e5422ce7bd39a8f47ef8a45e61f3dcdb4cf8
Author: John Erasmus Mari Geronimo <johnerasmusmari.geronimo@analog.com>

hwmon: (lm75) add MAX31875 support

This commit adds support for the Maxim MAX31875 I2C temperature sensor
by extending the lm75 driver. The new device supports a 16-bit
configuration register with configurable resolutions and selectable
conversion rates.

> diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
> index 2d2d752aeac9d..cda28b9b093c1 100644
> --- a/drivers/hwmon/lm75.c
> +++ b/drivers/hwmon/lm75.c

[ ... ]

> @@ -222,6 +223,14 @@ static const struct lm75_params device_params[] = {
>  		.default_resolution = 16,
>  		.default_sample_time = MSEC_PER_SEC / 20,
>  	},
> +	[max31875] = {
> +		.config_reg_16bits = true,
> +		.default_resolution = 10,
> +		.default_sample_time = 4000,
> +		.num_sample_times = 4,
> +		.sample_times = (unsigned int []){ 125, 250, 1000, 4000 },
> +		.alarm = true,
> +	},

[Severity: High]
Does setting .config_reg_16bits to true for max31875 cause a regression
where the conversion rate is corrupted during initialization?

In lm75_generic_probe(), the polarity mask is unconditionally set to BIT(10)
for 16-bit chips:

    if (!data->params->config_reg_16bits)
        pol_mask = LM75_ALERT_POLARITY_HIGH_8_BIT;
    else
        pol_mask = LM75_ALERT_POLARITY_HIGH_16_BIT;

This mask is then applied to the configuration register:

    clr_mask = data->params->clr_mask | pol_mask;
    ...
    err = lm75_write_config(data, set_mask, clr_mask);

Because the max31875 conversion rate mask includes BIT(10) and BIT(9),
could probing the max31875 forcefully clear or set the MSB of its
conversion rate, causing mutual corruption between the alarm polarity and
conversion rate settings?

[Severity: Medium]
Since .set_mask and .clr_mask are not defined here, does this result in a
state desynchronization on probe?

In lm75_generic_probe(), the software state is unconditionally initialized to
the defaults:

    data->sample_time = data->params->default_sample_time;
    data->resolution = data->params->default_resolution;

But since the configuration masks are 0, lm75_write_config() fails to
enforce these defaults on the hardware.

If a bootloader or prior system state configured the conversion rate or
resolution differently, will the driver state become desynchronized from the
actual hardware state?

[ ... ]

> @@ -494,6 +506,13 @@ static int lm75_update_interval(struct device *dev, long val)
>  			return err;
>  		data->sample_time = data->params->sample_times[index];
>  		break;
> +	case max31875:
> +		err = regmap_update_bits(data->regmap, LM75_REG_CONF,
> +					 0x0600, (3 - index) << 9);

[Severity: Medium]
Is the bit mask for the MAX31875 conversion rate correct here?

Because lm75_i2c_reg_read() uses i2c_smbus_read_word_data() for 16-bit
configuration registers, the chip's MSB is placed in the lower 8 bits of the
returned value to maintain compatibility with 8-bit standard LM75 features.

This means D10 and D9 map to bits 2 and 1 of regval.

Using the mask 0x0600 and shifting by 9 modifies bits 10 and 9 of regval,
which alters bits D2 and D1 of the chip's LSB instead of setting the
conversion rate.

Should the mask be 0x0006 and the shift be 1?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009115617.1976-1-johnerasmusmari.geronimo@analog.com?part=3