[PATCH v2 0/8] hwmon: (ina3221) Various improvement and add support for SQ52210

[PATCH v2 0/8] hwmon: (ina3221) Various improvement and add support for SQ52210

[Wenliang Yan]

Revised based on feedback from the previous version:

1.Adjusted the description in dt-bindings: only added a compatible string
for sq52210.

2.Modified the register read/write value conversion function, refined the
calculation logic for each register, and reduced unnecessary function
calls.

3.Restructured the patch content for easier review.

---
v1: https://lore.kernel.org/linux-hwmon/20251111080546.32421-1-wenliang202407@163.com/

Wenliang Yan (8):
  dt-bindings: hwmon: ti,ina3221: Add SQ52210
  hwmon: (ina3221) Add support for SQ52210
  hwmon: (ina3221) Pre-calculate current and power LSB
  hwmon: (ina3221) Support alert configuration
  hwmon: (ina3221) Introduce power attribute and alert characteristics
  hwmon: (ina3221) Support for writing alert limit values and modify
    the 'ina3221_read_value' function.
  hwmon: (ina3221) Support write/read functions for 'power' attribute
  hwmon: (ina3221) Modify write/read functions for 'in' and 'curr'
    attribute

 .../devicetree/bindings/hwmon/ti,ina3221.yaml |   4 +-
 Documentation/hwmon/ina3221.rst               |  24 +
 drivers/hwmon/ina3221.c                       | 520 +++++++++++++++++-
 3 files changed, 533 insertions(+), 15 deletions(-)

-- 
2.17.1

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

[Wenliang Yan]

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

[Krzysztof Kozlowski]

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

[Wenliang Yan]

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

[Krzysztof Kozlowski]

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

[Wenliang Yan]

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