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

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

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