[PATCHv2 0/6] misc. gpio-charger patches

[PATCHv2 0/6] misc. gpio-charger patches

[Sebastian Reichel]

Hi,

This is v2 of a patchset for gpio-charger. The patches are
mostly unrelated to each other, but have some dependencies.

Merge plan is:

Assuming there are no huge blockers, the plan is to merge
patches 1-3 through power-supply subsystem for 5.9. The
ARM patches can go via ARM subsystem for 5.9 and the final
patch can simply be postponed for 5.10.

Changelog since PATCHv1:
 * Fixed the YAML binding as suggested by Rob
 * Implemented the suggestions from Emil
 * Split making "gpios" optional into separate patch, which
   simplifies reviewing and follows "one change per patch"
   style
 * Add two new patches converting platform data users to
   use GPIO descriptor tables
 * Add final patch removing gpio from platform data

-- Sebastian

Sebastian Reichel (6):
  dt-bindings: power: supply: gpio-charger: convert to yaml
  power: supply: gpio-charger: Make gpios optional
  power: supply: gpio-charger: add charge-current-limit feature
  ARM: pxa: Use GPIO descriptor for gpio-charger
  ARM: sa1100: Use GPIO descriptor for gpio-charger
  power: supply: gpio-charger: drop legacy GPIO support

 .../bindings/power/supply/gpio-charger.txt    |  31 ---
 .../bindings/power/supply/gpio-charger.yaml   |  94 ++++++++
 arch/arm/mach-pxa/tosa.c                      |  24 +--
 arch/arm/mach-sa1100/collie.c                 |  11 +-
 drivers/power/supply/gpio-charger.c           | 200 ++++++++++++++----
 include/linux/power/gpio-charger.h            |   5 -
 6 files changed, 275 insertions(+), 90 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/power/supply/gpio-charger.txt
 create mode 100644 Documentation/devicetree/bindings/power/supply/gpio-charger.yaml

-- 
2.26.2

[PATCHv2 1/6] dt-bindings: power: supply: gpio-charger: convert to yaml

[Sebastian Reichel]

Convert the gpio-charger bindings from text format to
new YAML based representation.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 .../bindings/power/supply/gpio-charger.txt    | 31 ----------
 .../bindings/power/supply/gpio-charger.yaml   | 58 +++++++++++++++++++
 2 files changed, 58 insertions(+), 31 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/power/supply/gpio-charger.txt
 create mode 100644 Documentation/devicetree/bindings/power/supply/gpio-charger.yaml

diff --git a/Documentation/devicetree/bindings/power/supply/gpio-charger.txt b/Documentation/devicetree/bindings/power/supply/gpio-charger.txt
deleted file mode 100644
index 0fb33b2c62a6..000000000000
--- a/Documentation/devicetree/bindings/power/supply/gpio-charger.txt
+++ /dev/null
@@ -1,31 +0,0 @@
-gpio-charger
-
-Required properties :
- - compatible : "gpio-charger"
- - gpios : GPIO indicating the charger presence.
-   See GPIO binding in bindings/gpio/gpio.txt .
- - charger-type : power supply type, one of
-     unknown
-     battery
-     ups
-     mains
-     usb-sdp (USB standard downstream port)
-     usb-dcp (USB dedicated charging port)
-     usb-cdp (USB charging downstream port)
-     usb-aca (USB accessory charger adapter)
-
-Optional properties:
- - charge-status-gpios: GPIO indicating whether a battery is charging.
-
-Example:
-
-	usb_charger: charger {
-		compatible = "gpio-charger";
-		charger-type = "usb-sdp";
-		gpios = <&gpd 28 GPIO_ACTIVE_LOW>;
-		charge-status-gpios = <&gpc 27 GPIO_ACTIVE_LOW>;
-	};
-
-	battery {
-		power-supplies = <&usb_charger>;
-	};
diff --git a/Documentation/devicetree/bindings/power/supply/gpio-charger.yaml b/Documentation/devicetree/bindings/power/supply/gpio-charger.yaml
new file mode 100644
index 000000000000..78b167c62ab1
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/gpio-charger.yaml
@@ -0,0 +1,58 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/supply/gpio-charger.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: simple battery chargers only communicating through GPIOs
+
+maintainers:
+  - Sebastian Reichel <sre@kernel.org>
+
+description:
+  This binding is for all chargers, which are working more or less
+  autonomously, only providing some status GPIOs and possibly some
+  GPIOs for limited control over the charging process.
+
+properties:
+  compatible:
+    const: gpio-charger
+
+  charger-type:
+    enum:
+      - unknown
+      - battery
+      - ups
+      - mains
+      - usb-sdp                   # USB standard downstream port
+      - usb-dcp                   # USB dedicated charging port
+      - usb-cdp                   # USB charging downstream port
+      - usb-aca                   # USB accessory charger adapter
+    description:
+      Type of the charger, e.g. "mains" for a wall charger.
+
+  gpios:
+    maxItems: 1
+    description: GPIO indicating the charger presence
+
+  charge-status-gpios:
+    maxItems: 1
+    description: GPIO indicating the charging status
+
+required:
+  - compatible
+  - gpios
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    charger {
+      compatible = "gpio-charger";
+      charger-type = "usb-sdp";
+
+      gpios = <&gpd 28 GPIO_ACTIVE_LOW>;
+      charge-status-gpios = <&gpc 27 GPIO_ACTIVE_LOW>;
+    };
-- 
2.26.2

Re: [PATCHv2 1/6] dt-bindings: power: supply: gpio-charger: convert to yaml

[Linus Walleij]

On Sat, Jun 6, 2020 at 12:44 AM Sebastian Reichel
<sebastian.reichel@collabora.com> wrote:

> Convert the gpio-charger bindings from text format to
> new YAML based representation.
>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>

Looks good to me, if it passes the checks:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCHv2 1/6] dt-bindings: power: supply: gpio-charger: convert to yaml

[Rob Herring]

On Sat, 06 Jun 2020 00:43:58 +0200, Sebastian Reichel wrote:
> Convert the gpio-charger bindings from text format to
> new YAML based representation.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>  .../bindings/power/supply/gpio-charger.txt    | 31 ----------
>  .../bindings/power/supply/gpio-charger.yaml   | 58 +++++++++++++++++++
>  2 files changed, 58 insertions(+), 31 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/power/supply/gpio-charger.txt
>  create mode 100644 Documentation/devicetree/bindings/power/supply/gpio-charger.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCHv2 1/6] dt-bindings: power: supply: gpio-charger: convert to yaml

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 3880 bytes --]

Hi,

On Sat, Jun 06, 2020 at 12:43:58AM +0200, Sebastian Reichel wrote:
> Convert the gpio-charger bindings from text format to
> new YAML based representation.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---

Thanks, queued.

-- Sebastian

>  .../bindings/power/supply/gpio-charger.txt    | 31 ----------
>  .../bindings/power/supply/gpio-charger.yaml   | 58 +++++++++++++++++++
>  2 files changed, 58 insertions(+), 31 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/power/supply/gpio-charger.txt
>  create mode 100644 Documentation/devicetree/bindings/power/supply/gpio-charger.yaml
> 
> diff --git a/Documentation/devicetree/bindings/power/supply/gpio-charger.txt b/Documentation/devicetree/bindings/power/supply/gpio-charger.txt
> deleted file mode 100644
> index 0fb33b2c62a6..000000000000
> --- a/Documentation/devicetree/bindings/power/supply/gpio-charger.txt
> +++ /dev/null
> @@ -1,31 +0,0 @@
> -gpio-charger
> -
> -Required properties :
> - - compatible : "gpio-charger"
> - - gpios : GPIO indicating the charger presence.
> -   See GPIO binding in bindings/gpio/gpio.txt .
> - - charger-type : power supply type, one of
> -     unknown
> -     battery
> -     ups
> -     mains
> -     usb-sdp (USB standard downstream port)
> -     usb-dcp (USB dedicated charging port)
> -     usb-cdp (USB charging downstream port)
> -     usb-aca (USB accessory charger adapter)
> -
> -Optional properties:
> - - charge-status-gpios: GPIO indicating whether a battery is charging.
> -
> -Example:
> -
> -	usb_charger: charger {
> -		compatible = "gpio-charger";
> -		charger-type = "usb-sdp";
> -		gpios = <&gpd 28 GPIO_ACTIVE_LOW>;
> -		charge-status-gpios = <&gpc 27 GPIO_ACTIVE_LOW>;
> -	};
> -
> -	battery {
> -		power-supplies = <&usb_charger>;
> -	};
> diff --git a/Documentation/devicetree/bindings/power/supply/gpio-charger.yaml b/Documentation/devicetree/bindings/power/supply/gpio-charger.yaml
> new file mode 100644
> index 000000000000..78b167c62ab1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/gpio-charger.yaml
> @@ -0,0 +1,58 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/power/supply/gpio-charger.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: simple battery chargers only communicating through GPIOs
> +
> +maintainers:
> +  - Sebastian Reichel <sre@kernel.org>
> +
> +description:
> +  This binding is for all chargers, which are working more or less
> +  autonomously, only providing some status GPIOs and possibly some
> +  GPIOs for limited control over the charging process.
> +
> +properties:
> +  compatible:
> +    const: gpio-charger
> +
> +  charger-type:
> +    enum:
> +      - unknown
> +      - battery
> +      - ups
> +      - mains
> +      - usb-sdp                   # USB standard downstream port
> +      - usb-dcp                   # USB dedicated charging port
> +      - usb-cdp                   # USB charging downstream port
> +      - usb-aca                   # USB accessory charger adapter
> +    description:
> +      Type of the charger, e.g. "mains" for a wall charger.
> +
> +  gpios:
> +    maxItems: 1
> +    description: GPIO indicating the charger presence
> +
> +  charge-status-gpios:
> +    maxItems: 1
> +    description: GPIO indicating the charging status
> +
> +required:
> +  - compatible
> +  - gpios
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    charger {
> +      compatible = "gpio-charger";
> +      charger-type = "usb-sdp";
> +
> +      gpios = <&gpd 28 GPIO_ACTIVE_LOW>;
> +      charge-status-gpios = <&gpc 27 GPIO_ACTIVE_LOW>;
> +    };
> -- 
> 2.26.2
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCHv2 2/6] power: supply: gpio-charger: Make gpios optional

[Sebastian Reichel]

While strongly recommended, not all devices have a gpio to
detect if the charger is connected. This moves the 'gpios'
from required to optional section.

This also modifies error handling for the GPIO a bit: We
no longer fallback to pdata, if a GPIO is specified using
GPIO descriptor tables. This is a bit cleaner and does
not have any real impact: There are only two mainline pdata
users (arm/mach-sa1100/collie.c, arm/mach-pxa/tosa.c) and
none of them specify the GPIO via gpiod descriptor tables.
Once both have been converted the driver's support for
specifying GPIOs numbers in pdata will be dropped.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 .../bindings/power/supply/gpio-charger.yaml   |  7 +++-
 drivers/power/supply/gpio-charger.c           | 38 ++++++++++++-------
 2 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/Documentation/devicetree/bindings/power/supply/gpio-charger.yaml b/Documentation/devicetree/bindings/power/supply/gpio-charger.yaml
index 78b167c62ab1..30eabbb14ef3 100644
--- a/Documentation/devicetree/bindings/power/supply/gpio-charger.yaml
+++ b/Documentation/devicetree/bindings/power/supply/gpio-charger.yaml
@@ -41,7 +41,12 @@ properties:
 
 required:
   - compatible
-  - gpios
+
+anyOf:
+  - required:
+    - gpios
+  - required:
+    - charge-status-gpios
 
 additionalProperties: false
 
diff --git a/drivers/power/supply/gpio-charger.c b/drivers/power/supply/gpio-charger.c
index 1b959c7f8b0e..875735d50716 100644
--- a/drivers/power/supply/gpio-charger.c
+++ b/drivers/power/supply/gpio-charger.c
@@ -112,9 +112,14 @@ static int gpio_charger_get_irq(struct device *dev, void *dev_id,
 	return irq;
 }
 
+/*
+ * The entries will be overwritten by driver's probe routine depending
+ * on the available features. This list ensures, that the array is big
+ * enough for all optional features.
+ */
 static enum power_supply_property gpio_charger_properties[] = {
 	POWER_SUPPLY_PROP_ONLINE,
-	POWER_SUPPLY_PROP_STATUS /* Must always be last in the array. */
+	POWER_SUPPLY_PROP_STATUS,
 };
 
 static int gpio_charger_probe(struct platform_device *pdev)
@@ -128,6 +133,7 @@ static int gpio_charger_probe(struct platform_device *pdev)
 	int charge_status_irq;
 	unsigned long flags;
 	int ret;
+	int num_props = 0;
 
 	if (!pdata && !dev->of_node) {
 		dev_err(dev, "No platform data\n");
@@ -142,13 +148,13 @@ static int gpio_charger_probe(struct platform_device *pdev)
 	 * This will fetch a GPIO descriptor from device tree, ACPI or
 	 * boardfile descriptor tables. It's good to try this first.
 	 */
-	gpio_charger->gpiod = devm_gpiod_get(dev, NULL, GPIOD_IN);
+	gpio_charger->gpiod = devm_gpiod_get_optional(dev, NULL, GPIOD_IN);
 
 	/*
-	 * If this fails and we're not using device tree, try the
-	 * legacy platform data method.
+	 * Fallback to legacy platform data method, if no GPIO is specified
+	 * using boardfile descriptor tables.
 	 */
-	if (IS_ERR(gpio_charger->gpiod) && !dev->of_node) {
+	if (!gpio_charger->gpiod && pdata) {
 		/* Non-DT: use legacy GPIO numbers */
 		if (!gpio_is_valid(pdata->gpio)) {
 			dev_err(dev, "Invalid gpio pin in pdata\n");
@@ -173,17 +179,23 @@ static int gpio_charger_probe(struct platform_device *pdev)
 		return PTR_ERR(gpio_charger->gpiod);
 	}
 
+	if (gpio_charger->gpiod) {
+		gpio_charger_properties[num_props] = POWER_SUPPLY_PROP_ONLINE;
+		num_props++;
+	}
+
 	charge_status = devm_gpiod_get_optional(dev, "charge-status", GPIOD_IN);
-	gpio_charger->charge_status = charge_status;
-	if (IS_ERR(gpio_charger->charge_status))
-		return PTR_ERR(gpio_charger->charge_status);
+	if (IS_ERR(charge_status))
+		return PTR_ERR(charge_status);
+	if (charge_status) {
+		gpio_charger->charge_status = charge_status;
+		gpio_charger_properties[num_props] = POWER_SUPPLY_PROP_STATUS;
+		num_props++;
+	}
 
 	charger_desc = &gpio_charger->charger_desc;
 	charger_desc->properties = gpio_charger_properties;
-	charger_desc->num_properties = ARRAY_SIZE(gpio_charger_properties);
-	/* Remove POWER_SUPPLY_PROP_STATUS from the supported properties. */
-	if (!gpio_charger->charge_status)
-		charger_desc->num_properties -= 1;
+	charger_desc->num_properties = num_props;
 	charger_desc->get_property = gpio_charger_get_property;
 
 	psy_cfg.of_node = dev->of_node;
@@ -269,6 +281,6 @@ static struct platform_driver gpio_charger_driver = {
 module_platform_driver(gpio_charger_driver);
 
 MODULE_AUTHOR("Lars-Peter Clausen <lars@metafoo.de>");
-MODULE_DESCRIPTION("Driver for chargers which report their online status through a GPIO");
+MODULE_DESCRIPTION("Driver for chargers only communicating via GPIO(s)");
 MODULE_LICENSE("GPL");
 MODULE_ALIAS("platform:gpio-charger");
-- 
2.26.2

Re: [PATCHv2 2/6] power: supply: gpio-charger: Make gpios optional

[Linus Walleij]

On Sat, Jun 6, 2020 at 12:44 AM Sebastian Reichel
<sebastian.reichel@collabora.com> wrote:

> While strongly recommended, not all devices have a gpio to
> detect if the charger is connected. This moves the 'gpios'
> from required to optional section.
>
> This also modifies error handling for the GPIO a bit: We
> no longer fallback to pdata, if a GPIO is specified using
> GPIO descriptor tables. This is a bit cleaner and does
> not have any real impact: There are only two mainline pdata
> users (arm/mach-sa1100/collie.c, arm/mach-pxa/tosa.c) and
> none of them specify the GPIO via gpiod descriptor tables.
> Once both have been converted the driver's support for
> specifying GPIOs numbers in pdata will be dropped.
>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCHv2 2/6] power: supply: gpio-charger: Make gpios optional

[Rob Herring]

On Sat, 06 Jun 2020 00:43:59 +0200, Sebastian Reichel wrote:
> While strongly recommended, not all devices have a gpio to
> detect if the charger is connected. This moves the 'gpios'
> from required to optional section.
> 
> This also modifies error handling for the GPIO a bit: We
> no longer fallback to pdata, if a GPIO is specified using
> GPIO descriptor tables. This is a bit cleaner and does
> not have any real impact: There are only two mainline pdata
> users (arm/mach-sa1100/collie.c, arm/mach-pxa/tosa.c) and
> none of them specify the GPIO via gpiod descriptor tables.
> Once both have been converted the driver's support for
> specifying GPIOs numbers in pdata will be dropped.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>  .../bindings/power/supply/gpio-charger.yaml   |  7 +++-
>  drivers/power/supply/gpio-charger.c           | 38 ++++++++++++-------
>  2 files changed, 31 insertions(+), 14 deletions(-)
> 

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCHv2 2/6] power: supply: gpio-charger: Make gpios optional

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 5195 bytes --]

Hi,

On Sat, Jun 06, 2020 at 12:43:59AM +0200, Sebastian Reichel wrote:
> While strongly recommended, not all devices have a gpio to
> detect if the charger is connected. This moves the 'gpios'
> from required to optional section.
> 
> This also modifies error handling for the GPIO a bit: We
> no longer fallback to pdata, if a GPIO is specified using
> GPIO descriptor tables. This is a bit cleaner and does
> not have any real impact: There are only two mainline pdata
> users (arm/mach-sa1100/collie.c, arm/mach-pxa/tosa.c) and
> none of them specify the GPIO via gpiod descriptor tables.
> Once both have been converted the driver's support for
> specifying GPIOs numbers in pdata will be dropped.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---

Thanks, queued.

-- Sebastian

>  .../bindings/power/supply/gpio-charger.yaml   |  7 +++-
>  drivers/power/supply/gpio-charger.c           | 38 ++++++++++++-------
>  2 files changed, 31 insertions(+), 14 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/power/supply/gpio-charger.yaml b/Documentation/devicetree/bindings/power/supply/gpio-charger.yaml
> index 78b167c62ab1..30eabbb14ef3 100644
> --- a/Documentation/devicetree/bindings/power/supply/gpio-charger.yaml
> +++ b/Documentation/devicetree/bindings/power/supply/gpio-charger.yaml
> @@ -41,7 +41,12 @@ properties:
>  
>  required:
>    - compatible
> -  - gpios
> +
> +anyOf:
> +  - required:
> +    - gpios
> +  - required:
> +    - charge-status-gpios
>  
>  additionalProperties: false
>  
> diff --git a/drivers/power/supply/gpio-charger.c b/drivers/power/supply/gpio-charger.c
> index 1b959c7f8b0e..875735d50716 100644
> --- a/drivers/power/supply/gpio-charger.c
> +++ b/drivers/power/supply/gpio-charger.c
> @@ -112,9 +112,14 @@ static int gpio_charger_get_irq(struct device *dev, void *dev_id,
>  	return irq;
>  }
>  
> +/*
> + * The entries will be overwritten by driver's probe routine depending
> + * on the available features. This list ensures, that the array is big
> + * enough for all optional features.
> + */
>  static enum power_supply_property gpio_charger_properties[] = {
>  	POWER_SUPPLY_PROP_ONLINE,
> -	POWER_SUPPLY_PROP_STATUS /* Must always be last in the array. */
> +	POWER_SUPPLY_PROP_STATUS,
>  };
>  
>  static int gpio_charger_probe(struct platform_device *pdev)
> @@ -128,6 +133,7 @@ static int gpio_charger_probe(struct platform_device *pdev)
>  	int charge_status_irq;
>  	unsigned long flags;
>  	int ret;
> +	int num_props = 0;
>  
>  	if (!pdata && !dev->of_node) {
>  		dev_err(dev, "No platform data\n");
> @@ -142,13 +148,13 @@ static int gpio_charger_probe(struct platform_device *pdev)
>  	 * This will fetch a GPIO descriptor from device tree, ACPI or
>  	 * boardfile descriptor tables. It's good to try this first.
>  	 */
> -	gpio_charger->gpiod = devm_gpiod_get(dev, NULL, GPIOD_IN);
> +	gpio_charger->gpiod = devm_gpiod_get_optional(dev, NULL, GPIOD_IN);
>  
>  	/*
> -	 * If this fails and we're not using device tree, try the
> -	 * legacy platform data method.
> +	 * Fallback to legacy platform data method, if no GPIO is specified
> +	 * using boardfile descriptor tables.
>  	 */
> -	if (IS_ERR(gpio_charger->gpiod) && !dev->of_node) {
> +	if (!gpio_charger->gpiod && pdata) {
>  		/* Non-DT: use legacy GPIO numbers */
>  		if (!gpio_is_valid(pdata->gpio)) {
>  			dev_err(dev, "Invalid gpio pin in pdata\n");
> @@ -173,17 +179,23 @@ static int gpio_charger_probe(struct platform_device *pdev)
>  		return PTR_ERR(gpio_charger->gpiod);
>  	}
>  
> +	if (gpio_charger->gpiod) {
> +		gpio_charger_properties[num_props] = POWER_SUPPLY_PROP_ONLINE;
> +		num_props++;
> +	}
> +
>  	charge_status = devm_gpiod_get_optional(dev, "charge-status", GPIOD_IN);
> -	gpio_charger->charge_status = charge_status;
> -	if (IS_ERR(gpio_charger->charge_status))
> -		return PTR_ERR(gpio_charger->charge_status);
> +	if (IS_ERR(charge_status))
> +		return PTR_ERR(charge_status);
> +	if (charge_status) {
> +		gpio_charger->charge_status = charge_status;
> +		gpio_charger_properties[num_props] = POWER_SUPPLY_PROP_STATUS;
> +		num_props++;
> +	}
>  
>  	charger_desc = &gpio_charger->charger_desc;
>  	charger_desc->properties = gpio_charger_properties;
> -	charger_desc->num_properties = ARRAY_SIZE(gpio_charger_properties);
> -	/* Remove POWER_SUPPLY_PROP_STATUS from the supported properties. */
> -	if (!gpio_charger->charge_status)
> -		charger_desc->num_properties -= 1;
> +	charger_desc->num_properties = num_props;
>  	charger_desc->get_property = gpio_charger_get_property;
>  
>  	psy_cfg.of_node = dev->of_node;
> @@ -269,6 +281,6 @@ static struct platform_driver gpio_charger_driver = {
>  module_platform_driver(gpio_charger_driver);
>  
>  MODULE_AUTHOR("Lars-Peter Clausen <lars@metafoo.de>");
> -MODULE_DESCRIPTION("Driver for chargers which report their online status through a GPIO");
> +MODULE_DESCRIPTION("Driver for chargers only communicating via GPIO(s)");
>  MODULE_LICENSE("GPL");
>  MODULE_ALIAS("platform:gpio-charger");
> -- 
> 2.26.2
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Sebastian Reichel]

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

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 .../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
+      this order). The touples must be provided in descending order of the
+      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.
+
 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
     };
diff --git a/drivers/power/supply/gpio-charger.c b/drivers/power/supply/gpio-charger.c
index 875735d50716..74fc664c01e3 100644
--- a/drivers/power/supply/gpio-charger.c
+++ b/drivers/power/supply/gpio-charger.c
@@ -18,7 +18,13 @@
 
 #include <linux/power/gpio-charger.h>
 
+struct gpio_mapping {
+	u32 limit_ua;
+	u32 gpiodata;
+} __packed;
+
 struct gpio_charger {
+	struct device *dev;
 	unsigned int irq;
 	unsigned int charge_status_irq;
 	bool wakeup_enabled;
@@ -27,6 +33,11 @@ struct gpio_charger {
 	struct power_supply_desc charger_desc;
 	struct gpio_desc *gpiod;
 	struct gpio_desc *charge_status;
+
+	struct gpio_descs *current_limit_gpios;
+	struct gpio_mapping *current_limit_map;
+	u32 current_limit_map_size;
+	u32 charge_current_limit;
 };
 
 static irqreturn_t gpio_charger_irq(int irq, void *devid)
@@ -43,6 +54,35 @@ static inline struct gpio_charger *psy_to_gpio_charger(struct power_supply *psy)
 	return power_supply_get_drvdata(psy);
 }
 
+static int set_charge_current_limit(struct gpio_charger *gpio_charger, int val)
+{
+	struct gpio_mapping mapping;
+	int ndescs = gpio_charger->current_limit_gpios->ndescs;
+	struct gpio_desc **gpios = gpio_charger->current_limit_gpios->desc;
+	int i;
+
+	if (!gpio_charger->current_limit_map_size)
+		return -EINVAL;
+
+	for (i = 0; i < gpio_charger->current_limit_map_size; i++) {
+		if (gpio_charger->current_limit_map[i].limit_ua <= val)
+			break;
+	}
+	mapping = gpio_charger->current_limit_map[i];
+
+	for (i = 0; i < ndescs; i++) {
+		bool val = (mapping.gpiodata >> i) & 1;
+		gpiod_set_value_cansleep(gpios[ndescs-i-1], val);
+	}
+
+	gpio_charger->charge_current_limit = mapping.limit_ua;
+
+	dev_dbg(gpio_charger->dev, "set charge current limit to %d (requested: %d)\n",
+		gpio_charger->charge_current_limit, val);
+
+	return 0;
+}
+
 static int gpio_charger_get_property(struct power_supply *psy,
 		enum power_supply_property psp, union power_supply_propval *val)
 {
@@ -58,6 +98,9 @@ static int gpio_charger_get_property(struct power_supply *psy,
 		else
 			val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
 		break;
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX:
+		val->intval = gpio_charger->charge_current_limit;
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -65,6 +108,34 @@ static int gpio_charger_get_property(struct power_supply *psy,
 	return 0;
 }
 
+static int gpio_charger_set_property(struct power_supply *psy,
+	enum power_supply_property psp, const union power_supply_propval *val)
+{
+	struct gpio_charger *gpio_charger = psy_to_gpio_charger(psy);
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX:
+		return set_charge_current_limit(gpio_charger, val->intval);
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int gpio_charger_property_is_writeable(struct power_supply *psy,
+					      enum power_supply_property psp)
+{
+	switch (psp) {
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX:
+		return 1;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
 static enum power_supply_type gpio_charger_get_type(struct device *dev)
 {
 	const char *chargetype;
@@ -112,6 +183,61 @@ static int gpio_charger_get_irq(struct device *dev, void *dev_id,
 	return irq;
 }
 
+static int init_charge_current_limit(struct device *dev,
+				    struct gpio_charger *gpio_charger)
+{
+	int i, len;
+	u32 cur_limit = U32_MAX;
+
+	gpio_charger->current_limit_gpios = devm_gpiod_get_array_optional(dev,
+		"charge-current-limit", GPIOD_OUT_LOW);
+	if (IS_ERR(gpio_charger->current_limit_gpios)) {
+		dev_err(dev, "error getting current-limit GPIOs\n");
+		return PTR_ERR(gpio_charger->current_limit_gpios);
+	}
+
+	if (!gpio_charger->current_limit_gpios)
+		return 0;
+
+	len = device_property_read_u32_array(dev, "charge-current-limit-mapping",
+		NULL, 0);
+	if (len < 0)
+		return len;
+
+	if (len == 0 || len % 2) {
+		dev_err(dev, "invalid charge-current-limit-mapping length\n");
+		return -EINVAL;
+	}
+
+	gpio_charger->current_limit_map = devm_kmalloc_array(dev,
+		len / 2, sizeof(*gpio_charger->current_limit_map), GFP_KERNEL);
+	if (!gpio_charger->current_limit_map)
+		return -ENOMEM;
+
+	gpio_charger->current_limit_map_size = len / 2;
+
+	len = device_property_read_u32_array(dev, "charge-current-limit-mapping",
+		(u32*) gpio_charger->current_limit_map, len);
+	if (len < 0)
+		return len;
+
+	for (i=0; i < gpio_charger->current_limit_map_size; i++) {
+		if (gpio_charger->current_limit_map[i].limit_ua > cur_limit) {
+			dev_err(dev, "charge-current-limit-mapping not sorted by current in descending order\n");
+			return -EINVAL;
+		}
+
+		cur_limit = gpio_charger->current_limit_map[i].limit_ua;
+	}
+
+	/* default to smallest current limitation for safety reasons */
+	len = gpio_charger->current_limit_map_size - 1;
+	set_charge_current_limit(gpio_charger,
+		gpio_charger->current_limit_map[len].limit_ua);
+
+	return 0;
+}
+
 /*
  * The entries will be overwritten by driver's probe routine depending
  * on the available features. This list ensures, that the array is big
@@ -120,6 +246,7 @@ static int gpio_charger_get_irq(struct device *dev, void *dev_id,
 static enum power_supply_property gpio_charger_properties[] = {
 	POWER_SUPPLY_PROP_ONLINE,
 	POWER_SUPPLY_PROP_STATUS,
+	POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX,
 };
 
 static int gpio_charger_probe(struct platform_device *pdev)
@@ -143,6 +270,7 @@ static int gpio_charger_probe(struct platform_device *pdev)
 	gpio_charger = devm_kzalloc(dev, sizeof(*gpio_charger), GFP_KERNEL);
 	if (!gpio_charger)
 		return -ENOMEM;
+	gpio_charger->dev = dev;
 
 	/*
 	 * This will fetch a GPIO descriptor from device tree, ACPI or
@@ -193,10 +321,22 @@ static int gpio_charger_probe(struct platform_device *pdev)
 		num_props++;
 	}
 
+	ret = init_charge_current_limit(dev, gpio_charger);
+	if (ret < 0)
+		return ret;
+	if (gpio_charger->current_limit_map) {
+		gpio_charger_properties[num_props] =
+			POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX;
+		num_props++;
+	}
+
 	charger_desc = &gpio_charger->charger_desc;
 	charger_desc->properties = gpio_charger_properties;
 	charger_desc->num_properties = num_props;
 	charger_desc->get_property = gpio_charger_get_property;
+	charger_desc->set_property = gpio_charger_set_property;
+	charger_desc->property_is_writeable =
+					gpio_charger_property_is_writeable;
 
 	psy_cfg.of_node = dev->of_node;
 	psy_cfg.drv_data = gpio_charger;
-- 
2.26.2

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

[Linus Walleij]

On Sat, Jun 6, 2020 at 12:44 AM Sebastian Reichel
<sebastian.reichel@collabora.com> wrote:

> Add new charge-current-limit feature to gpio-charger.
>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>

That's really neat and useful.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

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

[Rob Herring]

On Sat, Jun 06, 2020 at 12:44:00AM +0200, Sebastian Reichel wrote:
> Add new charge-current-limit feature to gpio-charger.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>  .../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/

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

and here.

> +      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.

> +
>  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
>      };

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

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 3210 bytes --]

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:
> > Add new charge-current-limit feature to gpio-charger.
> > 
> > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> > ---
> >  .../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/
> 
> > +      this order). The touples must be provided in descending order of the
> 
> and here.

Ack.

> > +      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

> > +
> >  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
> >      };

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCHv2 4/6] ARM: pxa: Use GPIO descriptor for gpio-charger

[Sebastian Reichel]

Provide AC detect GPIO via gpiod table instead of
legacy platform data so that legacy GPIO support
can be removed from the driver.

Also remove useless IRQ resource, which is not
used by the driver.

Due to lack of hardware this has only been compile
tested.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 arch/arm/mach-pxa/tosa.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/arch/arm/mach-pxa/tosa.c b/arch/arm/mach-pxa/tosa.c
index 3d2c108e911e..e4da2b4c5055 100644
--- a/arch/arm/mach-pxa/tosa.c
+++ b/arch/arm/mach-pxa/tosa.c
@@ -369,6 +369,14 @@ static struct pxaficp_platform_data tosa_ficp_platform_data = {
 /*
  * Tosa AC IN
  */
+static struct gpiod_lookup_table tosa_charger_gpiod_table = {
+	.dev_id = "gpio-charger",
+	.table = {
+		GPIO_LOOKUP("gpio-pxa", TOSA_GPIO_AC_IN, NULL, GPIO_ACTIVE_LOW),
+		{},
+	},
+};
+
 static char *tosa_ac_supplied_to[] = {
 	"main-battery",
 	"backup-battery",
@@ -378,29 +386,14 @@ static char *tosa_ac_supplied_to[] = {
 static struct gpio_charger_platform_data tosa_power_data = {
 	.name			= "charger",
 	.type			= POWER_SUPPLY_TYPE_MAINS,
-	.gpio			= TOSA_GPIO_AC_IN,
-	.gpio_active_low	= 1,
 	.supplied_to		= tosa_ac_supplied_to,
 	.num_supplicants	= ARRAY_SIZE(tosa_ac_supplied_to),
 };
 
-static struct resource tosa_power_resource[] = {
-	{
-		.name		= "ac",
-		.start		= PXA_GPIO_TO_IRQ(TOSA_GPIO_AC_IN),
-		.end		= PXA_GPIO_TO_IRQ(TOSA_GPIO_AC_IN),
-		.flags		= IORESOURCE_IRQ |
-				  IORESOURCE_IRQ_HIGHEDGE |
-				  IORESOURCE_IRQ_LOWEDGE,
-	},
-};
-
 static struct platform_device tosa_power_device = {
 	.name			= "gpio-charger",
 	.id			= -1,
 	.dev.platform_data	= &tosa_power_data,
-	.resource		= tosa_power_resource,
-	.num_resources		= ARRAY_SIZE(tosa_power_resource),
 };
 
 /*
@@ -950,6 +943,7 @@ static void __init tosa_init(void)
 
 	clk_add_alias("CLK_CK3P6MI", tc6393xb_device.name, "GPIO11_CLK", NULL);
 
+	gpiod_add_lookup_table(&tosa_charger_gpiod_table);
 	gpiod_add_lookup_table(&tosa_udc_gpiod_table);
 	platform_add_devices(devices, ARRAY_SIZE(devices));
 }
-- 
2.26.2

Re: [PATCHv2 4/6] ARM: pxa: Use GPIO descriptor for gpio-charger

[Linus Walleij]

On Sat, Jun 6, 2020 at 12:44 AM Sebastian Reichel
<sebastian.reichel@collabora.com> wrote:

> Provide AC detect GPIO via gpiod table instead of
> legacy platform data so that legacy GPIO support
> can be removed from the driver.
>
> Also remove useless IRQ resource, which is not
> used by the driver.
>
> Due to lack of hardware this has only been compile
> tested.
>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Thanks for going the extra mile and doing this deep clean-up.

Yours,
Linus Walleij

[PATCHv2 5/6] ARM: sa1100: Use GPIO descriptor for gpio-charger

[Sebastian Reichel]

Provide AC detect GPIO via gpiod table instead of
legacy platform data so that legacy GPIO support
can be removed from the driver.

Due to lack of hardware this has only been compile
tested.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 arch/arm/mach-sa1100/collie.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-sa1100/collie.c b/arch/arm/mach-sa1100/collie.c
index 3cc2b71e16f0..3e871a3db3b0 100644
--- a/arch/arm/mach-sa1100/collie.c
+++ b/arch/arm/mach-sa1100/collie.c
@@ -30,6 +30,7 @@
 #include <linux/gpio_keys.h>
 #include <linux/input.h>
 #include <linux/gpio.h>
+#include <linux/gpio/machine.h>
 #include <linux/power/gpio-charger.h>
 
 #include <video/sa1100fb.h>
@@ -131,6 +132,14 @@ static struct irda_platform_data collie_ir_data = {
 /*
  * Collie AC IN
  */
+static struct gpiod_lookup_table collie_charger_gpiod_table = {
+	.dev_id = "gpio-charger",
+	.table = {
+		GPIO_LOOKUP("gpio", COLLIE_GPIO_AC_IN, NULL, GPIO_ACTIVE_HIGH),
+		{},
+	},
+};
+
 static char *collie_ac_supplied_to[] = {
 	"main-battery",
 	"backup-battery",
@@ -140,7 +149,6 @@ static char *collie_ac_supplied_to[] = {
 static struct gpio_charger_platform_data collie_power_data = {
 	.name			= "charger",
 	.type			= POWER_SUPPLY_TYPE_MAINS,
-	.gpio			= COLLIE_GPIO_AC_IN,
 	.supplied_to		= collie_ac_supplied_to,
 	.num_supplicants	= ARRAY_SIZE(collie_ac_supplied_to),
 };
@@ -386,6 +394,7 @@ static void __init collie_init(void)
 
 	platform_scoop_config = &collie_pcmcia_config;
 
+	gpiod_add_lookup_table(&collie_charger_gpiod_table);
 	ret = platform_add_devices(devices, ARRAY_SIZE(devices));
 	if (ret) {
 		printk(KERN_WARNING "collie: Unable to register LoCoMo device\n");
-- 
2.26.2

Re: [PATCHv2 5/6] ARM: sa1100: Use GPIO descriptor for gpio-charger

[Linus Walleij]

On Sat, Jun 6, 2020 at 12:44 AM Sebastian Reichel
<sebastian.reichel@collabora.com> wrote:

> Provide AC detect GPIO via gpiod table instead of
> legacy platform data so that legacy GPIO support
> can be removed from the driver.
>
> Due to lack of hardware this has only been compile
> tested.
>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCHv2 6/6] power: supply: gpio-charger: drop legacy GPIO support

[Sebastian Reichel]

All board files have been converted to use boardfile GPIO
descriptor tables, so GPIO support can be removed from
platform data.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/power/supply/gpio-charger.c | 30 +----------------------------
 include/linux/power/gpio-charger.h  |  5 -----
 2 files changed, 1 insertion(+), 34 deletions(-)

diff --git a/drivers/power/supply/gpio-charger.c b/drivers/power/supply/gpio-charger.c
index 74fc664c01e3..8c9f69f3d13e 100644
--- a/drivers/power/supply/gpio-charger.c
+++ b/drivers/power/supply/gpio-charger.c
@@ -258,7 +258,6 @@ static int gpio_charger_probe(struct platform_device *pdev)
 	struct power_supply_desc *charger_desc;
 	struct gpio_desc *charge_status;
 	int charge_status_irq;
-	unsigned long flags;
 	int ret;
 	int num_props = 0;
 
@@ -272,41 +271,14 @@ static int gpio_charger_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	gpio_charger->dev = dev;
 
-	/*
-	 * This will fetch a GPIO descriptor from device tree, ACPI or
-	 * boardfile descriptor tables. It's good to try this first.
-	 */
 	gpio_charger->gpiod = devm_gpiod_get_optional(dev, NULL, GPIOD_IN);
-
-	/*
-	 * Fallback to legacy platform data method, if no GPIO is specified
-	 * using boardfile descriptor tables.
-	 */
-	if (!gpio_charger->gpiod && pdata) {
-		/* Non-DT: use legacy GPIO numbers */
-		if (!gpio_is_valid(pdata->gpio)) {
-			dev_err(dev, "Invalid gpio pin in pdata\n");
-			return -EINVAL;
-		}
-		flags = GPIOF_IN;
-		if (pdata->gpio_active_low)
-			flags |= GPIOF_ACTIVE_LOW;
-		ret = devm_gpio_request_one(dev, pdata->gpio, flags,
-					    dev_name(dev));
-		if (ret) {
-			dev_err(dev, "Failed to request gpio pin: %d\n", ret);
-			return ret;
-		}
-		/* Then convert this to gpiod for now */
-		gpio_charger->gpiod = gpio_to_desc(pdata->gpio);
-	} else if (IS_ERR(gpio_charger->gpiod)) {
+	if (IS_ERR(gpio_charger->gpiod)) {
 		/* Just try again if this happens */
 		if (PTR_ERR(gpio_charger->gpiod) == -EPROBE_DEFER)
 			return -EPROBE_DEFER;
 		dev_err(dev, "error getting GPIO descriptor\n");
 		return PTR_ERR(gpio_charger->gpiod);
 	}
-
 	if (gpio_charger->gpiod) {
 		gpio_charger_properties[num_props] = POWER_SUPPLY_PROP_ONLINE;
 		num_props++;
diff --git a/include/linux/power/gpio-charger.h b/include/linux/power/gpio-charger.h
index 5a5a8de98181..3081391e93c9 100644
--- a/include/linux/power/gpio-charger.h
+++ b/include/linux/power/gpio-charger.h
@@ -13,8 +13,6 @@
  * struct gpio_charger_platform_data - platform_data for gpio_charger devices
  * @name:		Name for the chargers power_supply device
  * @type:		Type of the charger
- * @gpio:		GPIO which is used to indicate the chargers status
- * @gpio_active_low:	Should be set to 1 if the GPIO is active low otherwise 0
  * @supplied_to:	Array of battery names to which this chargers supplies power
  * @num_supplicants:	Number of entries in the supplied_to array
  */
@@ -22,9 +20,6 @@ struct gpio_charger_platform_data {
 	const char *name;
 	enum power_supply_type type;
 
-	int gpio;
-	int gpio_active_low;
-
 	char **supplied_to;
 	size_t num_supplicants;
 };
-- 
2.26.2

Re: [PATCHv2 6/6] power: supply: gpio-charger: drop legacy GPIO support

[Linus Walleij]

On Sat, Jun 6, 2020 at 12:44 AM Sebastian Reichel
<sebastian.reichel@collabora.com> wrote:

> All board files have been converted to use boardfile GPIO
> descriptor tables, so GPIO support can be removed from
> platform data.
>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>

Thanks so much for doing this!
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij