linux-kernel - Re: [PATCH 1/2] dt-bindings: hwmon: ti,ina2xx: Add alert-polarity property

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: <76dd5c0e-cc67-4ad1-8733-d8efdb8a172b@roeck-us.net>
Date: Wed, 29 May 2024 07:01:57 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Krzysztof Kozlowski <krzk@...nel.org>, Amna Waseem
 <Amna.Waseem@...s.com>, Jean Delvare <jdelvare@...e.com>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>
Cc: linux-hwmon@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, kernel@...s.com
Subject: Re: [PATCH 1/2] dt-bindings: hwmon: ti,ina2xx: Add alert-polarity
 property

On 5/29/24 00:07, Krzysztof Kozlowski wrote:
> On 29/05/2024 08:07, Amna Waseem wrote:
>> Add a property to the binding to configure the Alert Polarity.
>> Alert pin is asserted based on the value of Alert Polarity bit of
>> Mask/Enable register. It is by default 0 which means Alert pin is
>> configured to be active low. To configure it to active high, set
>> alert-polarity property value to 1.
>>
>> Signed-off-by: Amna Waseem <Amna.Waseem@...s.com>
>> ---
>>   Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml b/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
>> index df86c2c92037..a3f0fd71fcc6 100644
>> --- a/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
>> +++ b/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
>> @@ -66,6 +66,14 @@ properties:
>>       description: phandle to the regulator that provides the VS supply typically
>>         in range from 2.7 V to 5.5 V.
>>   
>> +  alert-polarity:
> 
> Missing vendor prefix.
> 

Are you sure you want a vendor prefix here ? Reason for asking is that
many hardware monitoring chips have configurable alert or interrupt polarity,
only the name is different. Some examples are the JC42.4 standard ("event
polarity"), adt7410/adt7420 "interrupt polarity", MAX31827 ("alarm polarity"),
or DS1621 ("output polarity"). We even have a vendor property, "adi,alarm-pol",
used for MAX31827.

Secondary problem is that not all chips of the series support this
configuration. INA209 has a configurable "warning polarity", but the
warning pin and the smbus alert pin are two different pins.
INA219 and INA220 do not have alert or interrupt output pins.
Only INA226, INA230, INA231, INA238, and INA260 support configurable
alert polarity.

Thanks,
Guenter

>> +    description: |
> 
> Do not need '|' unless you need to preserve formatting.
> 
>> +      Alert polarity bit value of Mask/Enable register. Alert pin is asserted
>> +      based on the value of Alert polarity Bit. Default value is active low.
>> +      0 selects active low, 1 selects active high.
> 
> Just use string, easier to read. But for sure do not introduce different
> values than we already have - GPIO HIGH is 0, not 1.
> 
> 
> 
> Best regards,
> Krzysztof
>