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

On 5/1/26 07:06, [email protected] wrote:
> Hi bot,
> 
>> Von: [email protected] <[email protected]>
>> Gesendet: Freitag, 1. Mai 2026 14:46
>> An: Markus Stockhausen <[email protected]>
>> Cc: [email protected]
>> 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