diff mbox series

[v4,1/2] dt-bindings: power: supply: Add STC3117 Fuel Gauge

Message ID 20241127151953.29550-2-bhavin.sharma@siliconsignals.io
State New
Headers show
Series power: supply: Add STC3117 Fuel Gauge | expand

Commit Message

Bhavin Sharma Nov. 27, 2024, 3:19 p.m. UTC
The STC3117 provides a simple fuel gauge via I2C.
Add a DT schema to describe how to set it up in the device tree.

Signed-off-by: Hardevsinh Palaniya <hardevsinh.palaniya@siliconsignals.io>
Signed-off-by: Bhavin Sharma <bhavin.sharma@siliconsignals.io>
---
 .../bindings/power/supply/st,stc3117.yaml     | 53 +++++++++++++++++++
 1 file changed, 53 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/st,stc3117.yaml

Comments

Krzysztof Kozlowski Nov. 28, 2024, 8:33 a.m. UTC | #1
On 27/11/2024 19:22, Krzysztof Kozlowski wrote:
> On 27/11/2024 16:19, Bhavin Sharma wrote:
>> +
>> +allOf:
>> +  - $ref: power-supply.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - st,stc3117
>> +
>> +  reg:
>> +    maxItems: 1
> 
> I asked you some questions on v2, then on v3 and no responses.
> 
> You implemented some changes but still did not answer my question. I am
> not going to ask again, obviously expecting different result on the same
> makes little sense.
> 
> No ack from me.
> 

You responded privately - I am not going to do any talks under NDA. I
also do not provide some sort of personal support service. Keep *ALL*
discussions public.

Explaining what you asked:

Some of these are from monitored-battery. Sense resistor should be
separate property. But different question is about missing resources,
like supplies (VCC) and interrupts. Just look at datasheet.

Best regards,
Krzysztof
Bhavin Sharma Nov. 28, 2024, 8:41 a.m. UTC | #2
Hi Krzysztof,
 
Thank you for your review.
 
> On 27/11/2024 16:19, Bhavin Sharma wrote:
> > +
> > +allOf:
> > +  - $ref: power-supply.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - st,stc3117
> > +
> > +  reg:
> > +    maxItems: 1
>
> I asked you some questions on v2, then on v3 and no responses.
 
I sincerely apologize for not addressing your questions in versions 2 and 3 
of the patch.
 
Regarding the battery configuration, we need the following information:
- Battery capacity
- Battery impedance
- OCV curve
- Minimum and maximum voltage
- Current sense register
 
Currently, I have set the battery capacity and impedance directly in the 
driver as default values. However, I understand these should ideally be 
defined in the device tree source (DTS). I will update the DTS accordingly
to include these parameters.
 
Additionally, since the OCV curve, minimum/maximum voltage, and sense
register values are fixed, I would like your opinion on whether these should
also be defined in the DTS or if it is acceptable to keep them in the driver itself.
 
Once again, I apologize for the oversight and thank you for your understanding.
 
Best regards,
Bhavin
Bhavin Sharma Nov. 28, 2024, 9:22 a.m. UTC | #3
Hi Krzysztof,

>>> +  reg:
>>> +    maxItems: 1
>>
>> I asked you some questions on v2, then on v3 and no responses.
>>
>> You implemented some changes but still did not answer my question. I am
>> not going to ask again, obviously expecting different result on the same
>> makes little sense.
>>
>> No ack from me.
>>
>
> You responded privately - I am not going to do any talks under NDA. I
> also do not provide some sort of personal support service. Keep *ALL*
> discussions public.

My apologies, that was unintentional.

> Explaining what you asked:
>
> Some of these are from monitored-battery. Sense resistor should be
> separate property. But different question is about missing resources,
> like supplies (VCC) and interrupts. Just look at datasheet.

since battery itself serves as the power supply for fuel gauge no additional supplies are required. 

Regarding interrupts yes, datasheet specifies an alarm pin that signals low State of Charge or
Voltage conditions. However, I haven’t incorporated interrupts in the driver to handle this. Should I
list the alarm pin as a resource in the device tree (or relevant configuration) even if it’s not 
implemented in the driver yet?

Best Regards,
Bhavin
Krzysztof Kozlowski Nov. 28, 2024, 10:08 a.m. UTC | #4
On 28/11/2024 10:22, Bhavin Sharma wrote:
> 
>> Explaining what you asked:
>>
>> Some of these are from monitored-battery. Sense resistor should be
>> separate property. But different question is about missing resources,
>> like supplies (VCC) and interrupts. Just look at datasheet.
> 
> since battery itself serves as the power supply for fuel gauge no additional supplies are required. 
> 
> Regarding interrupts yes, datasheet specifies an alarm pin that signals low State of Charge or
> Voltage conditions. However, I haven’t incorporated interrupts in the driver to handle this. Should I
> list the alarm pin as a resource in the device tree (or relevant configuration) even if it’s not 
> implemented in the driver yet?

Yes, bindings should be complete.
Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/power/supply/st,stc3117.yaml b/Documentation/devicetree/bindings/power/supply/st,stc3117.yaml
new file mode 100644
index 000000000000..06e53534ad76
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/st,stc3117.yaml
@@ -0,0 +1,53 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/supply/st,stc3117.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: STMicroelectronics STC3117 Fuel Gauge Unit Power Supply
+
+maintainers:
+  - Bhavin Sharma <bhavin.sharma@siliconsignals.io>
+  - Hardevsinh Palaniya <hardevsinh.palaniya@siliconsignals.io>
+
+description: |
+  The STC3117 includes the STMicroelectronics OptimGauge algorithm.
+  It provides accurate battery state-of-charge (SOC) monitoring, tracks
+  battery parameter changes with operation conditions, temperature,
+  and aging, and allows the application to get a battery state-of-health
+  (SOH) indication.
+
+  An alarm output signals low SOC or low voltage conditions and also
+  indicates fault conditions like a missing or swapped battery.
+
+  Datasheet is available at
+  https://www.st.com/resource/en/datasheet/stc3117.pdf
+
+allOf:
+  - $ref: power-supply.yaml#
+
+properties:
+  compatible:
+    enum:
+      - st,stc3117
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      battery@70 {
+        compatible = "st,stc3117";
+        reg = <0x70>;
+      };
+    };