[PATCH v2 1/8] dt-bindings: hwmon: ti,ina3221: Add SQ52210

Wenliang Yan posted 8 patches 1 week, 6 days ago
There is a newer version of this series
[PATCH v2 1/8] dt-bindings: hwmon: ti,ina3221: Add SQ52210
Posted by Wenliang Yan 1 week, 6 days ago
Add a compatible string for sq52210. The sq52210 is forward compatible
with INA3221 and incorporates alert registers to implement four
additional alert functions, all of which require no board-level
configuration.

Signed-off-by: Wenliang Yan <wenliang202407@163.com>
---
 Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml b/Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml
index 5f10f1207d69..3bdd8dfb7a36 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
-- 
2.17.1
Re: [PATCH v2 1/8] dt-bindings: hwmon: ti,ina3221: Add SQ52210
Posted by Krzysztof Kozlowski 1 week, 5 days ago
On Tue, Nov 18, 2025 at 07:51:41AM -0500, Wenliang Yan wrote:
> Add a compatible string for sq52210. The sq52210 is forward compatible

forward?

> with INA3221 and incorporates alert registers to implement four

But this suggests opposite.

Your driver changes confirm that even more - it is not forward
compatible. And in other way why wouldn't compatibility be expressed in
the bindings?

Best regards,
Krzysztof
Re: [PATCH v2 1/8] dt-bindings: hwmon: ti,ina3221: Add SQ52210
Posted by Wenliang Yan 1 week, 5 days ago
At 2025-11-19 15:22:38, "Krzysztof Kozlowski" <krzk@kernel.org> wrote:
>On Tue, Nov 18, 2025 at 07:51:41AM -0500, Wenliang Yan wrote:
>> Add a compatible string for sq52210. The sq52210 is forward compatible
>
>forward?
>
>> with INA3221 and incorporates alert registers to implement four
>
>But this suggests opposite.
>
>Your driver changes confirm that even more - it is not forward
>compatible. And in other way why wouldn't compatibility be expressed in
>the bindings?
>
>Best regards,
>Krzysztof

Perhaps my use of "forward" was inaccurate. I only meant to express that
at the hardware level, the SQ52210 contains all the registers and
functions of the INA3221, and builds upon them by adding current, power,
and alert registers. However, these additional registers don't require
adding more specific properties in the binding file.
Are you suggesting that I'm missing the description of SQ52210's
characteristics in the documentation?


Thanks,
Wenlaing Yan
Re: [PATCH v2 1/8] dt-bindings: hwmon: ti,ina3221: Add SQ52210
Posted by Krzysztof Kozlowski 1 week, 5 days ago
On 19/11/2025 10:06, Wenliang Yan wrote:
> At 2025-11-19 15:22:38, "Krzysztof Kozlowski" <krzk@kernel.org> wrote:
>> On Tue, Nov 18, 2025 at 07:51:41AM -0500, Wenliang Yan wrote:
>>> Add a compatible string for sq52210. The sq52210 is forward compatible
>>
>> forward?
>>
>>> with INA3221 and incorporates alert registers to implement four
>>
>> But this suggests opposite.
>>
>> Your driver changes confirm that even more - it is not forward
>> compatible. And in other way why wouldn't compatibility be expressed in
>> the bindings?
>>
>> Best regards,
>> Krzysztof
> 
> Perhaps my use of "forward" was inaccurate. I only meant to express that
> at the hardware level, the SQ52210 contains all the registers and
> functions of the INA3221, and builds upon them by adding current, power,
> and alert registers. However, these additional registers don't require
> adding more specific properties in the binding file.
> Are you suggesting that I'm missing the description of SQ52210's
> characteristics in the documentation?

This is backwards compatibility and if that's the case - driver can bind
via old compatible and work correctly with previous functionality - why
not expressing it in the bindings as compatible devices? See writing
bindings.

Best regards,
Krzysztof
Re: [PATCH v2 1/8] dt-bindings: hwmon: ti,ina3221: Add SQ52210
Posted by Wenliang Yan 1 week, 4 days ago
At 2025-11-19 17:17:18, "Krzysztof Kozlowski" <krzk@kernel.org> wrote:
>On 19/11/2025 10:06, Wenliang Yan wrote:
>> At 2025-11-19 15:22:38, "Krzysztof Kozlowski" <krzk@kernel.org> wrote:
>>> On Tue, Nov 18, 2025 at 07:51:41AM -0500, Wenliang Yan wrote:
>>>> Add a compatible string for sq52210. The sq52210 is forward compatible
>>>
>>> forward?
>>>
>>>> with INA3221 and incorporates alert registers to implement four
>>>
>>> But this suggests opposite.
>>>
>>> Your driver changes confirm that even more - it is not forward
>>> compatible. And in other way why wouldn't compatibility be expressed in
>>> the bindings?
>>>
>>> Best regards,
>>> Krzysztof
>> 
>> Perhaps my use of "forward" was inaccurate. I only meant to express that
>> at the hardware level, the SQ52210 contains all the registers and
>> functions of the INA3221, and builds upon them by adding current, power,
>> and alert registers. However, these additional registers don't require
>> adding more specific properties in the binding file.
>> Are you suggesting that I'm missing the description of SQ52210's
>> characteristics in the documentation?
>
>This is backwards compatibility and if that's the case - driver can bind
>via old compatible and work correctly with previous functionality - why
>not expressing it in the bindings as compatible devices? See writing
>bindings.
>
>Best regards,
>Krzysztof

Okay, I will use oneOf to express the compatibility relationship in the
v3 version.

Thanks,
Wenlaing Yan