linux-kernel - Re: [PATCH v2 3/3] dt-bindings: hwmon: Add bindings for max31732

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <105cae8a-ba03-ea60-70f2-8a307a26ad14@linaro.org>
Date:   Thu, 29 Dec 2022 17:39:30 +0100
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Guenter Roeck <linux@...ck-us.net>
Cc:     Sinan Divarci <Sinan.Divarci@...log.com>, jdelvare@...e.com,
        robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org,
        linux-hwmon@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/3] dt-bindings: hwmon: Add bindings for max31732

On 29/12/2022 16:52, Guenter Roeck wrote:
>>> +  adi,alarm1-interrupt-mode:
>>> +    description: |
>>> +      Enables the ALARM1 output to function in interrupt mode.
>>> +      Default ALARM1 output function is comparator mode.
>>
>> Why this is a property of DT/hardware? Don't encode policy in DT.
>>
> 
> I would not call this "policy". Normally it is an implementation
> question or decision, since interrupts behave differently depending
> on the mode. Impact is difficult to see, though, since the chip
> documentation is not available to the public.

Some more background would be useful here from the author...

> 
>>> +    type: boolean
>>> +
>>> +  adi,alarm2-interrupt-mode:
>>> +    description: |
>>> +      Enables the ALARM2 output to function in interrupt mode.
>>> +      Default ALARM2 output function is comparator mode.
>>
>> Same question.
>>
>>> +    type: boolean
>>> +
>>> +  adi,alarm1-fault-queue:
>>> +    description: The number of consecutive faults required to assert ALARM1.
>>
>> Same question - why this number differs with hardware?
>>
> 
> Noisier hardware will require more samples to avoid spurious faults.
> Trade-off is speed of reporting a fault. Normally the board designer
> would determine a value which is low enough to avoid spurious faults.
> 
> Note that the chip (according to patch 2/3) supports resistance
> cancellation as well as beta compensation, which are also board specific.
> I don't have access to the datasheet, so I don't know for sure if those
> are configurable (typically they are). If they are configurable, I would
> expect to see respective properties.

If that's the argument behind property, then it's fine.

Best regards,
Krzysztof