[PATCH 0/2] hwmon: (lm75) Fix AS6200 setup and alarm handling

[PATCH 0/2] hwmon: (lm75) Fix AS6200 setup and alarm handling

[Markus Stockhausen]

This series fixes two issues in the LM75 driver. 

1. The AMS AS6200 is not initialized as documented in the device-add-commit.
2. The lm75_write_config() skips bits during initialization.

Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>

[PATCH 1/2] hwmon: (lm75) Fix AS6200 setup and alarm handling

[Markus Stockhausen]

The initialization of the AS6200 has two shortcomings

- The device-add-commit states "Conversion mode: continuous" but the
  the lm75_params structure uses set_mask = 0x94c0. This activates
  single shot (bit 15) that effectly disables the sensor
- It is the only device that activates polarity active-high (bit 10)

All this is paired with a undefined clear mask bug in function
lm75_write_config() that was introduced with a later refactoring
commit.

[as6200] = {
	.config_reg_16bits = true,
	.set_mask = 0x94C0,
        -> .clr_mask not defined here
	.default_resolution = 12,
...
static inline int lm75_write_config(struct lm75_data *data, u16 set_mask,
				    u16 clr_mask)
{
	return regmap_update_bits(data->regmap, LM75_REG_CONF,
				  clr_mask | LM75_SHUTDOWN, set_mask);
}

regmap_update_bits() requires clr_mask to be a superset of set_mask.
So basically AS6200 and TMP112 sensors are not initialized as intended
and AS6200 is effectively driven in polarity active-low.

Fix that by

- changing the set_mask to 0x10c0 to reflect the current active-low
  setup properly and to drive the sensor in continous mode
- adapting the alarm handling so it can report the alarm correctly
  even if it is high active. This is done by xoring config register
  bit 5 and 10 (translated to 2 and 13).

This commit does not introduce any ABI breakage as the mutliple bugs
effectly drive the AS6200 and TMP112 useless.

Fixes 4b6358e ("hwmon: (lm75) Add AMS AS6200 temperature sensor")
Suggested-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
---
 drivers/hwmon/lm75.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
index f1a1e5b888f6..4fd4288ae502 100644
--- a/drivers/hwmon/lm75.c
+++ b/drivers/hwmon/lm75.c
@@ -137,7 +137,7 @@ static const struct lm75_params device_params[] = {
 	},
 	[as6200] = {
 		.config_reg_16bits = true,
-		.set_mask = 0x94C0,	/* 8 sample/s, 4 CF, positive polarity */
+		.set_mask = 0x10C0,	/* 8 sample/s, 4 CF */
 		.default_resolution = 12,
 		.default_sample_time = 125,
 		.num_sample_times = 4,
@@ -416,7 +416,7 @@ static int lm75_read(struct device *dev, enum hwmon_sensor_types type,
 			switch (data->kind) {
 			case as6200:
 			case tmp112:
-				*val = (regval >> 13) & 0x1;
+				*val = !!(regval & BIT(13)) ^ !!(regval & BIT(2));
 				break;
 			default:
 				return -EINVAL;
-- 
2.54.0

[PATCH 2/2] hwmon: (lm75) Fix configuration register writes.

[Markus Stockhausen]

Sensors configurations are defined by set and clear masks. These
do not follow a consistent "clear mask is a superset of set mask"
rule. This relaxed definition breaks lm75_write_config()

static inline int lm75_write_config(struct lm75_data *data, u16 set_mask,
				    u16 clr_mask)
{
	return regmap_update_bits(data->regmap, LM75_REG_CONF,
				  clr_mask | LM75_SHUTDOWN, set_mask);
}

Basically all bits from set_mask that are not defined in clr_mask are
dropped. Fix that by enhancing the helper to always combine clr_mask
and set_mask into the mask bits of regmap_update_bits().

Fixes: 6da24a2 ("hwmon: (lm75) Hide register size differences in regmap access functions")
Suggested-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
---
 drivers/hwmon/lm75.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
index 4fd4288ae502..c601bb494dad 100644
--- a/drivers/hwmon/lm75.c
+++ b/drivers/hwmon/lm75.c
@@ -353,7 +353,7 @@ static inline int lm75_write_config(struct lm75_data *data, u16 set_mask,
 				    u16 clr_mask)
 {
 	return regmap_update_bits(data->regmap, LM75_REG_CONF,
-				  clr_mask | LM75_SHUTDOWN, set_mask);
+				  clr_mask | set_mask | LM75_SHUTDOWN, set_mask);
 }
 
 static irqreturn_t lm75_alarm_handler(int irq, void *private)
-- 
2.54.0

Re: [PATCH 1/2] hwmon: (lm75) Fix AS6200 setup and alarm handling

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The updated set_mask of 0x10C0 is byte-swapped over I2C and will force the sensor into single-shot mode instead of the intended continuous mode.
- [High] The new alarm reporting logic is mathematically inverted, violating the hwmon ABI by returning 0 during an active alarm.
--

commit bef39cd430ab022cea5921c7c9e935e0fe939e92
Author: Markus Stockhausen <markus.stockhausen@gmx.de>

hwmon: (lm75) Fix AS6200 setup and alarm handling

This commit attempts to fix the AS6200 initialization by changing the set
mask to enable continuous conversion mode. It also modifies the alarm
handling logic with the intention of correctly reporting alarms regardless
of the configured polarity.

> diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
> index f1a1e5b888f6..4fd4288ae502 100644
> --- a/drivers/hwmon/lm75.c
> +++ b/drivers/hwmon/lm75.c
> @@ -137,7 +137,7 @@ static const struct lm75_params device_params[] = {
>  	},
>  	[as6200] = {
>  		.config_reg_16bits = true,
> -		.set_mask = 0x94C0,	/* 8 sample/s, 4 CF, positive polarity */
> +		.set_mask = 0x10C0,	/* 8 sample/s, 4 CF */

The intent described in the commit message is to clear bit 15 to disable
single-shot mode. However, won't this new mask still force the sensor into
single-shot mode over the wire?

The LM75 driver writes LM75_REG_CONF using i2c_smbus_write_word_data(),
which transmits data in little-endian order. The AS6200 is a big-endian I2C
sensor. When 0x10C0 is transmitted, the lower byte (0xC0) is sent first and
interpreted by the device as the MSB. Since 0xC0 is 11000000 in binary,
bit 15 remains set to 1.

[ ... ]

> @@ -416,7 +416,7 @@ static int lm75_read(struct device *dev, enum hwmon_sensor_types type,
>  			switch (data->kind) {
>  			case as6200:
>  			case tmp112:
> -				*val = (regval >> 13) & 0x1;
> +				*val = !!(regval & BIT(13)) ^ !!(regval & BIT(2));

Does this logic correctly map to the hwmon ABI, which expects a value of 1
during an active alarm?

According to the sensor datasheets, the AL bit reflects the ALERT pin
directly. 

If POL=0 (active low), the pin is low on alarm (AL=0), so 0 ^ 0 = 0.
If POL=1 (active high), the pin is high on alarm (AL=1), so 1 ^ 1 = 0.

In both cases, it appears this expression evaluates to 0 during an active
alarm and 1 during normal operation, inverting the expected hwmon behavior.

Additionally, does hardcoding BIT(13) and BIT(2) solidify an underlying
I2C byte-swapping behavior directly into the logic rather than fixing the
root cause?

>  				break;
>  			default:
>  				return -EINVAL;
>  			}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260502094524.3358193-1-markus.stockhausen@gmx.de?part=1

Re: [PATCH 1/2] hwmon: (lm75) Fix AS6200 setup and alarm handling

[Guenter Roeck]

On 5/2/26 03:10, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] The updated set_mask of 0x10C0 is byte-swapped over I2C and will force the sensor into single-shot mode instead of the intended continuous mode.
> - [High] The new alarm reporting logic is mathematically inverted, violating the hwmon ABI by returning 0 during an active alarm.
> --
> 
> commit bef39cd430ab022cea5921c7c9e935e0fe939e92
> Author: Markus Stockhausen <markus.stockhausen@gmx.de>
> 
> hwmon: (lm75) Fix AS6200 setup and alarm handling
> 
> This commit attempts to fix the AS6200 initialization by changing the set
> mask to enable continuous conversion mode. It also modifies the alarm
> handling logic with the intention of correctly reporting alarms regardless
> of the configured polarity.
> 
>> diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
>> index f1a1e5b888f6..4fd4288ae502 100644
>> --- a/drivers/hwmon/lm75.c
>> +++ b/drivers/hwmon/lm75.c
>> @@ -137,7 +137,7 @@ static const struct lm75_params device_params[] = {
>>   	},
>>   	[as6200] = {
>>   		.config_reg_16bits = true,
>> -		.set_mask = 0x94C0,	/* 8 sample/s, 4 CF, positive polarity */
>> +		.set_mask = 0x10C0,	/* 8 sample/s, 4 CF */
> 
> The intent described in the commit message is to clear bit 15 to disable
> single-shot mode. However, won't this new mask still force the sensor into
> single-shot mode over the wire?
> 
> The LM75 driver writes LM75_REG_CONF using i2c_smbus_write_word_data(),
> which transmits data in little-endian order. The AS6200 is a big-endian I2C
> sensor. When 0x10C0 is transmitted, the lower byte (0xC0) is sent first and
> interpreted by the device as the MSB. Since 0xC0 is 11000000 in binary,
> bit 15 remains set to 1.
> 

... except for the config register, which is little endian, and the first
byte sent to the chip is the LSB. No idea how to tell that to the agent.

Either case, I ended up ordering an evaluation board for as6200 and samples
for TMP112, so we'll see how this works for real once I get that.

> [ ... ]
> 
>> @@ -416,7 +416,7 @@ static int lm75_read(struct device *dev, enum hwmon_sensor_types type,
>>   			switch (data->kind) {
>>   			case as6200:
>>   			case tmp112:
>> -				*val = (regval >> 13) & 0x1;
>> +				*val = !!(regval & BIT(13)) ^ !!(regval & BIT(2));
> 
> Does this logic correctly map to the hwmon ABI, which expects a value of 1
> during an active alarm?
> 
> According to the sensor datasheets, the AL bit reflects the ALERT pin
> directly.
> 
> If POL=0 (active low), the pin is low on alarm (AL=0), so 0 ^ 0 = 0.
> If POL=1 (active high), the pin is high on alarm (AL=1), so 1 ^ 1 = 0.
> 
> In both cases, it appears this expression evaluates to 0 during an active
> alarm and 1 during normal operation, inverting the expected hwmon behavior.
> 

I had another look at the datasheet. Turns out the agent is correct. The
bits must be _different_ for the alarm to be active. So this has to be
something like
	!!(regval & BIT(13)) != !!(regval & BIT(2));

Sorry for that.

> Additionally, does hardcoding BIT(13) and BIT(2) solidify an underlying
> I2C byte-swapping behavior directly into the logic rather than fixing the
> root cause?
> 
We can live with that.

Thanks,
Guenter

Re: [PATCH 1/2] hwmon: (lm75) Fix AS6200 setup and alarm handling

[Guenter Roeck]

On 5/2/26 02:45, Markus Stockhausen wrote:
> The initialization of the AS6200 has two shortcomings
> 
> - The device-add-commit states "Conversion mode: continuous" but the
>    the lm75_params structure uses set_mask = 0x94c0. This activates
>    single shot (bit 15) that effectly disables the sensor

Actually, according to the datasheet, the single-shot bit is only
relevant in shutdown mode and is supposed to trigger a single shot
conversion in that case. So setting it doesn't disable the sensor,
but it is either pointless or has unknown effect.

Sorry again.

Guenter

AW: [PATCH 1/2] hwmon: (lm75) Fix AS6200 setup and alarm handling

[markus.stockhausen]

> Von: Guenter Roeck <groeck7@gmail.com> Im Auftrag von Guenter Roeck
> Gesendet: Samstag, 2. Mai 2026 16:45
> An: sashiko@lists.linux.dev; Markus Stockhausen <markus.stockhausen@gmx.de>
> Cc: linux-hwmon@vger.kernel.org
> Betreff: Re: [PATCH 1/2] hwmon: (lm75) Fix AS6200 setup and alarm handling
> 
> ...
> Either case, I ended up ordering an evaluation board for as6200 and samples
> for TMP112, so we'll see how this works for real once I get that.

Looking at this again that is a good idea. device_params says 8 bit sampling (aka 
2 consecutive bits = 1) is in bits 6/7

	[as6200] = {
		.config_reg_16bits = true,
		.set_mask = 0x10C0,	/* 8 sample/s, 4 CF */

	[tmp112] = {
		.config_reg_16bits = true,
		.set_mask = 0x60C0,	/* 12-bit mode, 8 samples / second */

But lm75_update_interval() uses bits 14/15.

	case tmp112:
	case as6200:
		err = regmap_update_bits(data->regmap, LM75_REG_CONF,
					 0xc000, (3 - index) << 14);

This confuses me and maybe the bot ...

Markus