Linux Hardware Monitor development
 help / color / mirror / Atom feed
From: Guenter Roeck <linux@roeck-us.net>
To: markus.stockhausen@gmx.de, sashiko@lists.linux.dev
Cc: linux-hwmon@vger.kernel.org
Subject: Re: AW: [PATCH 2/2] hwmon: (lm75) Support active-high alert polarity
Date: Fri, 1 May 2026 10:37:51 -0700	[thread overview]
Message-ID: <8776fc88-364e-4ac5-b30c-8a706716d8ff@roeck-us.net> (raw)
In-Reply-To: <039601dcd973$bf191230$3d4b3690$@gmx.de>

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


  reply	other threads:[~2026-05-01 17:37 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-01 12:05 [PATCH 0/2] Support active-high alert polarity for LM75 Markus Stockhausen
2026-05-01 12:05 ` [PATCH 1/2] dt-bindings: hwmon: lm75: Add lm75,alert-polarity-active-high property Markus Stockhausen
2026-05-01 12:24   ` sashiko-bot
2026-05-02 17:37     ` AW: " markus.stockhausen
2026-05-02 18:10       ` Guenter Roeck
2026-05-03 18:02   ` Conor Dooley
2026-05-01 12:05 ` [PATCH 2/2] hwmon: (lm75) Support active-high alert polarity Markus Stockhausen
2026-05-01 12:45   ` sashiko-bot
2026-05-01 14:06     ` AW: " markus.stockhausen
2026-05-01 17:37       ` Guenter Roeck [this message]
2026-05-01 19:00         ` AW: " markus.stockhausen
2026-05-01 19:24           ` Guenter Roeck
2026-05-01 20:14         ` Guenter Roeck
2026-05-01 20:42           ` AW: " markus.stockhausen
2026-05-01 21:19             ` Guenter Roeck
2026-05-01 17:05   ` Conor Dooley
2026-05-01 19:32     ` Guenter Roeck
2026-05-03 18:00       ` Conor Dooley

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8776fc88-364e-4ac5-b30c-8a706716d8ff@roeck-us.net \
    --to=linux@roeck-us.net \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=markus.stockhausen@gmx.de \
    --cc=sashiko@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox