* [PATCH v3 0/2] hwmon: (lm75) Fix AS6200 and config register handling
@ 2026-05-02 17:32 Markus Stockhausen
2026-05-02 17:32 ` [PATCH v3 1/2] hwmon: (lm75) Fix AS6200 setup and alarm handling Markus Stockhausen
2026-05-02 17:32 ` [PATCH v3 2/2] hwmon: (lm75) Fix configuration register writes Markus Stockhausen
0 siblings, 2 replies; 7+ messages in thread
From: Markus Stockhausen @ 2026-05-02 17:32 UTC (permalink / raw)
To: linux, linux-hwmon, wsa+renesas, alkuor; +Cc: 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>
---
Changes in v3
- Adapt hwmon_temp_alarm comparison in lm75_read() to check for equality
of bits 2 and 13
Changes in v2
- Adapt hwmon_temp_alarm comparison in lm75_read() to check for inequality
of bits 2 and 13.
- Reword AS6200 fix to make clear how single shot configuration works
according to the datasheet. Add explanation that config register is little
endian.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v3 1/2] hwmon: (lm75) Fix AS6200 setup and alarm handling
2026-05-02 17:32 [PATCH v3 0/2] hwmon: (lm75) Fix AS6200 and config register handling Markus Stockhausen
@ 2026-05-02 17:32 ` Markus Stockhausen
2026-05-02 17:58 ` sashiko-bot
2026-05-02 18:13 ` Guenter Roeck
2026-05-02 17:32 ` [PATCH v3 2/2] hwmon: (lm75) Fix configuration register writes Markus Stockhausen
1 sibling, 2 replies; 7+ messages in thread
From: Markus Stockhausen @ 2026-05-02 17:32 UTC (permalink / raw)
To: linux, linux-hwmon, wsa+renesas, alkuor; +Cc: 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 mode (bit 15). According to the datasheet "The device
features a single shot measurement mode if the device is in sleep
mode (SM=1)". This is quite contradictionary.
- 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 all sensors with "wrong" masks like the AS6200 are not
initialized as intended.
Fix that by
- Change the set_mask to 0x10c0 to reflect the current active-low
setup properly and to drive the sensor in continous mode. This
takes into account that the config register is little endian and
the first byte sent to the chip is the LSB.
- Adapt the alarm handling so it can report the alarm correctly
even if it is high active. This is done by comparing 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 in standard active-low mode.
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..7efcef7768cd 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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v3 2/2] hwmon: (lm75) Fix configuration register writes.
2026-05-02 17:32 [PATCH v3 0/2] hwmon: (lm75) Fix AS6200 and config register handling Markus Stockhausen
2026-05-02 17:32 ` [PATCH v3 1/2] hwmon: (lm75) Fix AS6200 setup and alarm handling Markus Stockhausen
@ 2026-05-02 17:32 ` Markus Stockhausen
2026-05-02 18:14 ` Guenter Roeck
2026-05-02 18:19 ` sashiko-bot
1 sibling, 2 replies; 7+ messages in thread
From: Markus Stockhausen @ 2026-05-02 17:32 UTC (permalink / raw)
To: linux, linux-hwmon, wsa+renesas, alkuor; +Cc: 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 7efcef7768cd..8b74cc314196 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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3 1/2] hwmon: (lm75) Fix AS6200 setup and alarm handling
2026-05-02 17:32 ` [PATCH v3 1/2] hwmon: (lm75) Fix AS6200 setup and alarm handling Markus Stockhausen
@ 2026-05-02 17:58 ` sashiko-bot
2026-05-02 18:13 ` Guenter Roeck
1 sibling, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-05-02 17:58 UTC (permalink / raw)
To: Markus Stockhausen; +Cc: linux-hwmon
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incorrect Endianness Conversion / Bit Manipulation in Configuration Mask
--
commit 8a922153b759cd33365a5d701e970e65199083f1
Author: Markus Stockhausen <markus.stockhausen@gmx.de>
hwmon: (lm75) Fix AS6200 setup and alarm handling
The initialization of the AS6200 has shortcomings regarding the conversion
mode and polarity. This patch updates the set_mask to 0x10C0 to fix the
configuration and adapts the alarm handling logic to properly report alarms.
> - Change the set_mask to 0x10c0 to reflect the current active-low
> setup properly and to drive the sensor in continous mode. This
> takes into account that the config register is little endian and
> the first byte sent to the chip is the LSB.
Is the assumption that the config register is little-endian correct here?
The AS6200 is a big-endian sensor (it expects its MSB first). However, the
driver writes the 16-bit config register using i2c_smbus_write_word_data(),
which sends the integer's lower byte first.
Because of this, the lower byte of the driver's set_mask becomes the sensor's
MSB, and the upper byte becomes the sensor's LSB.
> diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
> index f1a1e5b888f6..7efcef7768cd 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 */
With the new mask set to 0x10C0, the lower byte is still 0xC0. This means
0xC0 is still transmitted first and becomes the MSB on the sensor, so bit 7
of the MSB (bit 15 of the hardware register) remains set. Will this keep the
sensor in Single-Shot mode instead of continuous mode?
This logic issue appears to persist even after the subsequent patch in the
series (0da1c5266106) fixes the regmap_update_bits() mask application bug.
Additionally, does changing the upper byte from 0x94 to 0x10 unintentionally
scramble the lower configuration bits on the sensor?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260502173207.3567876-1-markus.stockhausen@gmx.de?part=1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 1/2] hwmon: (lm75) Fix AS6200 setup and alarm handling
2026-05-02 17:32 ` [PATCH v3 1/2] hwmon: (lm75) Fix AS6200 setup and alarm handling Markus Stockhausen
2026-05-02 17:58 ` sashiko-bot
@ 2026-05-02 18:13 ` Guenter Roeck
1 sibling, 0 replies; 7+ messages in thread
From: Guenter Roeck @ 2026-05-02 18:13 UTC (permalink / raw)
To: Markus Stockhausen; +Cc: linux-hwmon, wsa+renesas, alkuor
On Sat, May 02, 2026 at 07:32:06PM +0200, 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 mode (bit 15). According to the datasheet "The device
> features a single shot measurement mode if the device is in sleep
> mode (SM=1)". This is quite contradictionary.
> - 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 all sensors with "wrong" masks like the AS6200 are not
> initialized as intended.
>
> Fix that by
>
> - Change the set_mask to 0x10c0 to reflect the current active-low
> setup properly and to drive the sensor in continous mode. This
> takes into account that the config register is little endian and
> the first byte sent to the chip is the LSB.
> - Adapt the alarm handling so it can report the alarm correctly
> even if it is high active. This is done by comparing 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 in standard active-low mode.
>
> Fixes: 4b6358e1fe46 ("hwmon: (lm75) Add AMS AS6200 temperature sensor")
Note: This is how the "Fixes" tag is supposed to look like.
> Suggested-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
Applied.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 2/2] hwmon: (lm75) Fix configuration register writes.
2026-05-02 17:32 ` [PATCH v3 2/2] hwmon: (lm75) Fix configuration register writes Markus Stockhausen
@ 2026-05-02 18:14 ` Guenter Roeck
2026-05-02 18:19 ` sashiko-bot
1 sibling, 0 replies; 7+ messages in thread
From: Guenter Roeck @ 2026-05-02 18:14 UTC (permalink / raw)
To: Markus Stockhausen; +Cc: linux-hwmon, wsa+renesas, alkuor
On Sat, May 02, 2026 at 07:32:07PM +0200, Markus Stockhausen wrote:
> 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: 6da24a25f766 ("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>
Applied, after fixing the Fixes: tag as seen above.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 2/2] hwmon: (lm75) Fix configuration register writes.
2026-05-02 17:32 ` [PATCH v3 2/2] hwmon: (lm75) Fix configuration register writes Markus Stockhausen
2026-05-02 18:14 ` Guenter Roeck
@ 2026-05-02 18:19 ` sashiko-bot
1 sibling, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-05-02 18:19 UTC (permalink / raw)
To: Markus Stockhausen; +Cc: linux-hwmon
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Applying this fix without correcting the 16-bit sensor masks causes `tmp112` and `as6200` to be severely misconfigured during probe.
--
commit 0da1c5266106ec86191304203df47a352a002b92
Author: Markus Stockhausen <markus.stockhausen@gmx.de>
hwmon: (lm75) Fix configuration register writes.
This commit fixes a bug in lm75_write_config() where set mask bits not
defined in the clear mask were being dropped. It ensures both masks are
properly combined for the regmap update.
> diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
> index 7efcef7768cd..8b74cc314196 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);
Will applying this fix without correcting the 16-bit sensor masks cause
sensors like tmp112 and as6200 to be misconfigured during probe?
I2C temperature sensors transmit and receive the MSB first, but
i2c_smbus_read_word_data() and i2c_smbus_write_word_data() read and write the
LSB first. This causes the driver's memory representation of the configuration
register to be byte-swapped, with the sensor's physical MSB in the lower 8
bits and the physical LSB in the upper 8 bits.
The set_mask for tmp112 (0x60C0) and as6200 (0x10C0) are defined assuming
native byte order. Prior to this patch, the logical bug in
lm75_write_config() silently dropped the set_mask bits because they were not
in clr_mask, leaving the sensors at their power-on defaults.
By fixing the helper, 0x60C0 is actively written to the byte-swapped register
value. Due to the byte-swap, the 0xC0 byte is applied to the lower 8 bits
(sensor MSB) and 0x60 to the upper 8 bits (sensor LSB).
For tmp112, the device receives 0xC0 as its MSB and 0x60 as its LSB. Does
this configure the sensor to 11-bit resolution and 1 Hz sample rate instead
of the intended 12-bit, 8 Hz configuration?
Could the endianness of the masks for 16-bit devices be fixed simultaneously
to avoid this regression?
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260502173207.3567876-1-markus.stockhausen@gmx.de?part=2
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-05-02 18:19 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-02 17:32 [PATCH v3 0/2] hwmon: (lm75) Fix AS6200 and config register handling Markus Stockhausen
2026-05-02 17:32 ` [PATCH v3 1/2] hwmon: (lm75) Fix AS6200 setup and alarm handling Markus Stockhausen
2026-05-02 17:58 ` sashiko-bot
2026-05-02 18:13 ` Guenter Roeck
2026-05-02 17:32 ` [PATCH v3 2/2] hwmon: (lm75) Fix configuration register writes Markus Stockhausen
2026-05-02 18:14 ` Guenter Roeck
2026-05-02 18:19 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox