AW: [PATCH 2/2] hwmon: (lm75) Support active-high alert polarity

AW: [PATCH 2/2] hwmon: (lm75) Support active-high alert polarity

[markus.stockhausen]

Hi bot,

> Von: sashiko-bot@kernel.org <sashiko-bot@kernel.org> 
> Gesendet: Freitag, 1. Mai 2026 14:46
> An: Markus Stockhausen <markus.stockhausen@gmx.de>
> Cc: linux-hwmon@vger.kernel.org
> Betreff: Re: [PATCH 2/2] hwmon: (lm75) Support active-high alert polarity
>
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [High] The active-high alert polarity configuration is completely ignored by the hardware because it is not added to the register update mask.
> - [High] LM75_ALERT_POLARITY_HIGH_16_BIT targets the wrong byte of the configuration register.
> - [High] Changing the AS6200 default from active-high to active-low breaks devicetree backward compatibility.

unsure if answering to you helps both of us further. So my 2 cents only shortly:

- Issue 1: Existing definitions for tmp1XX devices do not not obey "clr_mask is superset of set_mask".
- Issue 2: AS6200 datasheet states BIT(10) is polarity. Thus .set_mask = 0x94C0 can only align one way.
- Issue 3: Please advise. 

Best regards.

Markus

Re: AW: [PATCH 2/2] hwmon: (lm75) Support active-high alert polarity

[Guenter Roeck]

On 5/1/26 07:06, markus.stockhausen@gmx.de wrote:
> Hi bot,
> 
>> Von: sashiko-bot@kernel.org <sashiko-bot@kernel.org>
>> Gesendet: Freitag, 1. Mai 2026 14:46
>> An: Markus Stockhausen <markus.stockhausen@gmx.de>
>> Cc: linux-hwmon@vger.kernel.org
>> Betreff: Re: [PATCH 2/2] hwmon: (lm75) Support active-high alert polarity
>>
>> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>> - [High] The active-high alert polarity configuration is completely ignored by the hardware because it is not added to the register update mask.
>> - [High] LM75_ALERT_POLARITY_HIGH_16_BIT targets the wrong byte of the configuration register.
>> - [High] Changing the AS6200 default from active-high to active-low breaks devicetree backward compatibility.
> 
> unsure if answering to you helps both of us further. So my 2 cents only shortly:
> 
> - Issue 1: Existing definitions for tmp1XX devices do not not obey "clr_mask is superset of set_mask".

I think you are saying that this is a pre-existing bug and that set_mask is effectively
ignored. Is that a reason not to fix the problem ?

> - Issue 2: AS6200 datasheet states BIT(10) is polarity. Thus .set_mask = 0x94C0 can only align one way.

The reverse bit order of that register _is_ a bit confusing.

> - Issue 3: Please advise.

Above you are saying that the set mask is for all practical purposes
ignored. As far as I can say this is correct, meaning as6200 (and TMP112)
will default to its chip default, meaning the bit will be 0 anyway.

Looking into the AS6200 datasheet, changing this is not straightforward:
Setting the polarity bit to high also changes the value of the "alert"
bit, meaning the bit will be 1 if there is _no_ alarm. In other words,
this only works accidentally right now, and it does not really matter
if the default is changed. On the contrary, changing the default would result
in unexpected behavior since the alarm attribute would report alarms when
there is none.

So this will need separate patches:
1) Fix set_mask and alert handling for AS6200 to be low active
    and to report the alarm correctly even if it is high active
    (xor config register bit 5 and 10 (translated to 2 and 13)
    when reporting the alarm).
2) Fix lm75_write_config() to add set_mask to clr_mask to ensure
    that the bits which are supposed to be set are actually set.

Then add this series on top of it.

Thanks,
Guenter

AW: AW: [PATCH 2/2] hwmon: (lm75) Support active-high alert polarity

[markus.stockhausen]

> Von: Guenter Roeck <groeck7@gmail.com> Im Auftrag von Guenter Roeck
> Gesendet: Freitag, 1. Mai 2026 19:38
> Betreff: Re: AW: [PATCH 2/2] hwmon: (lm75) Support active-high alert polarity
> ...
>>> - [High] The active-high alert polarity configuration is completely ignored by the hardware because it is not added to the register update mask.
>>> - [High] LM75_ALERT_POLARITY_HIGH_16_BIT targets the wrong byte of the configuration register.
>>> - [High] Changing the AS6200 default from active-high to active-low breaks devicetree backward compatibility.
>> 
>> unsure if answering to you helps both of us further. So my 2 cents only shortly:
>> 
>> - Issue 1: Existing definitions for tmp1XX devices do not not obey "clr_mask is superset of set_mask".
>
> I think you are saying that this is a pre-existing bug and that set_mask is effectively
> ignored. Is that a reason not to fix the problem ?
> ...
> 2) Fix lm75_write_config() to add set_mask to clr_mask to ensure
>     that the bits which are supposed to be set are actually set.

I did not expect a bug in the current driver and was baffled by the bots 
response. From your feedback that qualifies a "Fixes" tag. Am I right to 
assume that 6da24a2 ("hwmon: (lm75) Hide register size differences in 
regmap access functions") is the commit to blame?

Markus

Re: AW: AW: [PATCH 2/2] hwmon: (lm75) Support active-high alert polarity

[Guenter Roeck]

On 5/1/26 12:00, markus.stockhausen@gmx.de wrote:
>> Von: Guenter Roeck <groeck7@gmail.com> Im Auftrag von Guenter Roeck
>> Gesendet: Freitag, 1. Mai 2026 19:38
>> Betreff: Re: AW: [PATCH 2/2] hwmon: (lm75) Support active-high alert polarity
>> ...
>>>> - [High] The active-high alert polarity configuration is completely ignored by the hardware because it is not added to the register update mask.
>>>> - [High] LM75_ALERT_POLARITY_HIGH_16_BIT targets the wrong byte of the configuration register.
>>>> - [High] Changing the AS6200 default from active-high to active-low breaks devicetree backward compatibility.
>>>
>>> unsure if answering to you helps both of us further. So my 2 cents only shortly:
>>>
>>> - Issue 1: Existing definitions for tmp1XX devices do not not obey "clr_mask is superset of set_mask".
>>
>> I think you are saying that this is a pre-existing bug and that set_mask is effectively
>> ignored. Is that a reason not to fix the problem ?
>> ...
>> 2) Fix lm75_write_config() to add set_mask to clr_mask to ensure
>>      that the bits which are supposed to be set are actually set.
> 
> I did not expect a bug in the current driver and was baffled by the bots
> response. From your feedback that qualifies a "Fixes" tag. Am I right to
> assume that 6da24a2 ("hwmon: (lm75) Hide register size differences in
> regmap access functions") is the commit to blame?
> 

Correct.

Thanks,
Guenter

Re: AW: [PATCH 2/2] hwmon: (lm75) Support active-high alert polarity

[Guenter Roeck]

On 5/1/26 10:37, Guenter Roeck wrote:
> On 5/1/26 07:06, markus.stockhausen@gmx.de wrote:
>> Hi bot,
>>
>>> Von: sashiko-bot@kernel.org <sashiko-bot@kernel.org>
>>> Gesendet: Freitag, 1. Mai 2026 14:46
>>> An: Markus Stockhausen <markus.stockhausen@gmx.de>
>>> Cc: linux-hwmon@vger.kernel.org
>>> Betreff: Re: [PATCH 2/2] hwmon: (lm75) Support active-high alert polarity
>>>
>>> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>>> - [High] The active-high alert polarity configuration is completely ignored by the hardware because it is not added to the register update mask.
>>> - [High] LM75_ALERT_POLARITY_HIGH_16_BIT targets the wrong byte of the configuration register.
>>> - [High] Changing the AS6200 default from active-high to active-low breaks devicetree backward compatibility.
>>
>> unsure if answering to you helps both of us further. So my 2 cents only shortly:
>>
>> - Issue 1: Existing definitions for tmp1XX devices do not not obey "clr_mask is superset of set_mask".
> 
> I think you are saying that this is a pre-existing bug and that set_mask is effectively
> ignored. Is that a reason not to fix the problem ?
> 
>> - Issue 2: AS6200 datasheet states BIT(10) is polarity. Thus .set_mask = 0x94C0 can only align one way.
> 
> The reverse bit order of that register _is_ a bit confusing.
> 
>> - Issue 3: Please advise.
> 
> Above you are saying that the set mask is for all practical purposes
> ignored. As far as I can say this is correct, meaning as6200 (and TMP112)
> will default to its chip default, meaning the bit will be 0 anyway.
> 
> Looking into the AS6200 datasheet, changing this is not straightforward:
> Setting the polarity bit to high also changes the value of the "alert"
> bit, meaning the bit will be 1 if there is _no_ alarm. In other words,
> this only works accidentally right now, and it does not really matter
> if the default is changed. On the contrary, changing the default would result
> in unexpected behavior since the alarm attribute would report alarms when
> there is none.
> 
> So this will need separate patches:
> 1) Fix set_mask and alert handling for AS6200 to be low active
>     and to report the alarm correctly even if it is high active
>     (xor config register bit 5 and 10 (translated to 2 and 13)
>     when reporting the alarm).
> 2) Fix lm75_write_config() to add set_mask to clr_mask to ensure
>     that the bits which are supposed to be set are actually set.
> 

Turns out the default value for AS6200 has more problems. It is set to
0x94c0 which claims to be "8 sample/s, 4 CF, positive polarity".

0xc0 reflects the samples/s. So far so good. However, 0x94 is wrong.
It sets the single shot bit, effectively disabling the sensor, and
it does not set the CF bits as advertised. Please change the default
to 0x10c0 which reflects the intent more closely, except for the
negative polarity.

Thanks,
Guenter

> Then add this series on top of it.
> 
> Thanks,
> Guenter
> 
> 

AW: AW: [PATCH 2/2] hwmon: (lm75) Support active-high alert polarity

[markus.stockhausen]

> Von: Guenter Roeck <groeck7@gmail.com> Im Auftrag von Guenter Roeck
> Gesendet: Freitag, 1. Mai 2026 22:14
> Betreff: Re: AW: [PATCH 2/2] hwmon: (lm75) Support active-high alert polarity
>  
> > So this will need separate patches:
> > 1) Fix set_mask and alert handling for AS6200 to be low active
> >     and to report the alarm correctly even if it is high active
> >     (xor config register bit 5 and 10 (translated to 2 and 13)
> >     when reporting the alarm).
> > 2) Fix lm75_write_config() to add set_mask to clr_mask to ensure
> >     that the bits which are supposed to be set are actually set.
> > 
>
> Turns out the default value for AS6200 has more problems. It is set to
> 0x94c0 which claims to be "8 sample/s, 4 CF, positive polarity".
>
> 0xc0 reflects the samples/s. So far so good. However, 0x94 is wrong.
> It sets the single shot bit, effectively disabling the sensor, and
> it does not set the CF bits as advertised. Please change the default
> to 0x10c0 which reflects the intent more closely, except for the
> negative polarity.

Understood. I still need advise about xor during alarm report. 
Do you talk about this code?

  case as6200:
  case tmp112:
-  *val = (regval >> 13) & 0x1;
+  *val = ((regval >> 13) & 0x1) ^ 0x1;

Mabye totally stupid but this bug and the deep dive follow up
hits my brain a little bit too hard.

Thanks.

Markus

Re: AW: AW: [PATCH 2/2] hwmon: (lm75) Support active-high alert polarity

[Guenter Roeck]

On 5/1/26 13:42, markus.stockhausen@gmx.de wrote:
>> Von: Guenter Roeck <groeck7@gmail.com> Im Auftrag von Guenter Roeck
>> Gesendet: Freitag, 1. Mai 2026 22:14
>> Betreff: Re: AW: [PATCH 2/2] hwmon: (lm75) Support active-high alert polarity
>>   
>>> So this will need separate patches:
>>> 1) Fix set_mask and alert handling for AS6200 to be low active
>>>      and to report the alarm correctly even if it is high active
>>>      (xor config register bit 5 and 10 (translated to 2 and 13)
>>>      when reporting the alarm).
>>> 2) Fix lm75_write_config() to add set_mask to clr_mask to ensure
>>>      that the bits which are supposed to be set are actually set.
>>>
>>
>> Turns out the default value for AS6200 has more problems. It is set to
>> 0x94c0 which claims to be "8 sample/s, 4 CF, positive polarity".
>>
>> 0xc0 reflects the samples/s. So far so good. However, 0x94 is wrong.
>> It sets the single shot bit, effectively disabling the sensor, and
>> it does not set the CF bits as advertised. Please change the default
>> to 0x10c0 which reflects the intent more closely, except for the
>> negative polarity.
> 
> Understood. I still need advise about xor during alarm report.
> Do you talk about this code?
> 
>    case as6200:
>    case tmp112:
> -  *val = (regval >> 13) & 0x1;
> +  *val = ((regval >> 13) & 0x1) ^ 0x1;
> 
No, you'll need to xor bit 2 (the polarity bit) and 13 (the alert bit).

Something like
	alarm = !!(regval & BIT(13)) ^ !!(regval & BIT(2))

> Mabye totally stupid but this bug and the deep dive follow up
> hits my brain a little bit too hard.
> 
:-)

Cheers,
Guenter