Re: [PATCH 1/8] dt-binding:ti,ina3221:Add SQ52210

[Wenliang Yan <wenliang202407@163.com>]

At 2025-11-11 16:17:26, "Krzysztof Kozlowski" <krzk@kernel.org> wrote:
>On 11/11/2025 09:05, Wenliang Yan wrote:
>> Add a compatible string for sq52210, sq52210 is forward compatible
>> with INA3221 and add alert register to implement four additional
>> alert function.
>> 
>> Signed-off-by: Wenliang Yan <wenliang202407@163.com>
>
>
>Please use subject prefixes matching the subsystem. You can get them for
>example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
>your patch is touching. For bindings, the preferred subjects are
>explained here:
>https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters
>
>It is never "dt-binding:". Also, all spaces are gone. Just look at `git
>log`.
>

I will change the subject to "dt-bindings: hwmon: ti,ina3221: Add SQ52210" before the next submission.

>
>> ---
>>  .../devicetree/bindings/hwmon/ti,ina3221.yaml    | 16 +++++++++++++++-
>>  1 file changed, 15 insertions(+), 1 deletion(-)
>> 
>> diff --git a/Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml b/Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml
>> index 5f10f1207d69..0fae82ca3ee1 100644
>> --- a/Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml
>> +++ b/Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml
>> @@ -12,7 +12,9 @@ maintainers:
>>  
>>  properties:
>>    compatible:
>> -    const: ti,ina3221
>> +    enum:
>> +      - silergy,sq52210
>> +      - ti,ina3221
>>  
>>    reg:
>>      maxItems: 1
>> @@ -77,6 +79,18 @@ patternProperties:
>>            exclude specific channels from the summation control function.
>>          type: boolean
>>  
>> +      alert-type:
>
>Not a generic property, missing type and vendor prefix.
>
>This clearly was not tested.
>
>
>> +        description: |
>> +          The SQ52210 features a configurable alert function with four
>> +          types: SUL, BOL, BUL, and POL. Each channel can be configured to
>> +          select one of these types to enable the alert function. This alert
>> +          function can operate concurrently with both Critical and Warning
>> +          functions.
>> +
>> +          The configuration must use numerical values 0 through 3,
>
>
>Don't repeat constraints in free form text.
>
>> +          0 corresponds to SUL, 1 to BOL, 2 to BUL, and 3 to POL.
>> +        enum: [ 0, 1, 2, 3 ]
>
>No, use string enum instead.
>
>Anyway, does not look like DT property. Why would alert type be set per
>board? Why I cannot change the alert during runtime?
>
>

You are right, my previous consideration was problematic. Indeed, there is
no need to set the alert type at the board level. I will modify this
content and test it before the next submission.


Thanks,
Wenlaing Yan