* [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
* 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
* [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 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