Re: [PATCHv2 3/6] power: supply: gpio-charger: add charge-current-limit feature

Hi,

On Mon, Jun 15, 2020 at 11:58:44AM -0600, Rob Herring wrote:

On Sat, Jun 06, 2020 at 12:44:00AM +0200, Sebastian Reichel wrote:

quoted

Add new charge-current-limit feature to gpio-charger.

Signed-off-by: Sebastian Reichel <redacted>
---
 .../bindings/power/supply/gpio-charger.yaml   |  31 ++++
 drivers/power/supply/gpio-charger.c           | 140 ++++++++++++++++++
 2 files changed, 171 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/supply/gpio-charger.yaml b/Documentation/devicetree/bindings/power/supply/gpio-charger.yaml
index 30eabbb14ef3..e11cfdc68a51 100644
--- a/Documentation/devicetree/bindings/power/supply/gpio-charger.yaml
+++ b/Documentation/devicetree/bindings/power/supply/gpio-charger.yaml

@@ -39,6 +39,25 @@ properties:
     maxItems: 1
     description: GPIO indicating the charging status
 
+  charge-current-limit-gpios:
+    minItems: 1
+    maxItems: 32
+    description: GPIOs used for current limiting
+
+  charge-current-limit-mapping:
+    description: List of touples with current in uA and a GPIO bitmap (in

s/touples/tuples/

quoted

+      this order). The touples must be provided in descending order of the

and here.

Ack.

quoted

+      current limit.
+    $ref: /schemas/types.yaml#/definitions/uint32-matrix
+    items:
+      items:
+        - description:
+            Current limit in uA
+        - description:
+            Encoded GPIO setting. Bit 0 represents last GPIO from the
+            charge-current-limit-gpios property. Bit 1 second to last
+            GPIO and so on.

Seems a bit odd that bit N doesn't represent index N of the gpios.

I was looking at it from a graphical POV (i.e. "last" bit represents
last element):

list = <element3, element2, element1, element0>;
bits = 0b1011; // element3, 1 and 0

Basically when writing it the order is the same at the cost of list
index being reverse of bit index. But I do not really mind the order.
If people think its better the other way around I can swap it.

-- Sebastian

quoted

+
 required:
   - compatible
 

@@ -47,6 +66,12 @@ anyOf:
     - gpios
   - required:
     - charge-status-gpios
+  - required:
+    - charge-current-limit-gpios
+
+dependencies:
+  charge-current-limit-gpios: [ charge-current-limit-mapping ]
+  charge-current-limit-mapping: [ charge-current-limit-gpios ]
 
 additionalProperties: false
 

@@ -60,4 +85,10 @@ examples:
 
       gpios = <&gpd 28 GPIO_ACTIVE_LOW>;
       charge-status-gpios = <&gpc 27 GPIO_ACTIVE_LOW>;
+
+      charge-current-limit-gpios = <&gpioA 11 GPIO_ACTIVE_HIGH>,
+                                   <&gpioA 12 GPIO_ACTIVE_HIGH>;
+      charge-current-limit-mapping = <2500000 0x00>, // 2.5 A => both GPIOs low
+                                     <700000 0x01>, // 700 mA => GPIO A.12 high
+                                     <0 0x02>; // 0 mA => GPIO A.11 high
     };