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

[sashiko-bot@kernel.org]

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