diff mbox series

[1/2] dt-bindings: power: supply: gpio-charger: add support for default charge current limit

Message ID 20241211-default-charge-current-limit-v1-1-7819ba06ee2a@liebherr.com
State New
Headers show
Series [1/2] dt-bindings: power: supply: gpio-charger: add support for default charge current limit | expand

Commit Message

Dimitri Fedrau via B4 Relay Dec. 11, 2024, 7:29 a.m. UTC
From: Dimitri Fedrau <dimitri.fedrau@liebherr.com>

Add binding for default charge current limit.

Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
---
 Documentation/devicetree/bindings/power/supply/gpio-charger.yaml | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Krzysztof Kozlowski Dec. 13, 2024, 11 a.m. UTC | #1
On Wed, Dec 11, 2024 at 08:29:09AM +0100, Dimitri Fedrau wrote:
> Add binding for default charge current limit.

Why?

> 
> Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
> ---
>  Documentation/devicetree/bindings/power/supply/gpio-charger.yaml | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/power/supply/gpio-charger.yaml b/Documentation/devicetree/bindings/power/supply/gpio-charger.yaml
> index 89f8e2bcb2d7836c6a4308aff51721bd83fa3ba1..545fdd7133daf67b5bc238c5af26d0cbd8b44eae 100644
> --- a/Documentation/devicetree/bindings/power/supply/gpio-charger.yaml
> +++ b/Documentation/devicetree/bindings/power/supply/gpio-charger.yaml
> @@ -58,6 +58,10 @@ properties:
>              charge-current-limit-gpios property. Bit 1 second to last
>              GPIO and so on.
>  
> +  charge-current-limit-default:

Use standard property suffixes - see other bindings how they define
charge current.
git grep charge -- Documentation/devicetree/bindings/power/supply/

But what I don't get is why GPIO chager needs it, since this is
non-configurable for GPIO charger.

You have entire commit msg or property description to explain such
things.

Best regards,
Krzysztof
Dimitri Fedrau Dec. 13, 2024, 2:19 p.m. UTC | #2
Am Fri, Dec 13, 2024 at 12:00:46PM +0100 schrieb Krzysztof Kozlowski:
> On Wed, Dec 11, 2024 at 08:29:09AM +0100, Dimitri Fedrau wrote:
> > Add binding for default charge current limit.
> 
> Why?
> 
See below.

> > 
> > Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
> > ---
> >  Documentation/devicetree/bindings/power/supply/gpio-charger.yaml | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/power/supply/gpio-charger.yaml b/Documentation/devicetree/bindings/power/supply/gpio-charger.yaml
> > index 89f8e2bcb2d7836c6a4308aff51721bd83fa3ba1..545fdd7133daf67b5bc238c5af26d0cbd8b44eae 100644
> > --- a/Documentation/devicetree/bindings/power/supply/gpio-charger.yaml
> > +++ b/Documentation/devicetree/bindings/power/supply/gpio-charger.yaml
> > @@ -58,6 +58,10 @@ properties:
> >              charge-current-limit-gpios property. Bit 1 second to last
> >              GPIO and so on.
> >  
> > +  charge-current-limit-default:
> 
> Use standard property suffixes - see other bindings how they define
> charge current.
> git grep charge -- Documentation/devicetree/bindings/power/supply/
>
Will fix it, thanks for the hint.

> But what I don't get is why GPIO chager needs it, since this is
> non-configurable for GPIO charger.
> 
With properties charge-current-limit-gpios and charge-current-limit-mapping
one can define charge current limits in uA using up to 32 GPIOs. At the
moment the driver defaults to smallest current limitation for safety
reasons. When disabling charging should be possible as in the example,
the charger defaults to non-charging. By having a default the charge
current limit can be setup on probe and charging is enabled.

> You have entire commit msg or property description to explain such
> things.
> 
Will explain it in more detail.

Best regards,
Dimitri Fedrau
Krzysztof Kozlowski Dec. 13, 2024, 2:32 p.m. UTC | #3
On 13/12/2024 15:19, Dimitri Fedrau wrote:
> 
>> But what I don't get is why GPIO chager needs it, since this is
>> non-configurable for GPIO charger.
>>
> With properties charge-current-limit-gpios and charge-current-limit-mapping
> one can define charge current limits in uA using up to 32 GPIOs. At the
> moment the driver defaults to smallest current limitation for safety
> reasons. When disabling charging should be possible as in the example,
> the charger defaults to non-charging. By having a default the charge
> current limit can be setup on probe and charging is enabled.

OK, the commit msg should explain the intention and real use case you
are solving here. Plus you miss the dependency - this property depends
on charge-current-limit-mapping.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/power/supply/gpio-charger.yaml b/Documentation/devicetree/bindings/power/supply/gpio-charger.yaml
index 89f8e2bcb2d7836c6a4308aff51721bd83fa3ba1..545fdd7133daf67b5bc238c5af26d0cbd8b44eae 100644
--- a/Documentation/devicetree/bindings/power/supply/gpio-charger.yaml
+++ b/Documentation/devicetree/bindings/power/supply/gpio-charger.yaml
@@ -58,6 +58,10 @@  properties:
             charge-current-limit-gpios property. Bit 1 second to last
             GPIO and so on.
 
+  charge-current-limit-default:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Default charge current limit in uA.
+
 required:
   - compatible