[PATCHv2,2/6] power: supply: gpio-charger: Make gpios optional
diff mbox series

Message ID 20200605224403.181015-3-sebastian.reichel@collabora.com
State New
Headers show
Series
  • misc. gpio-charger patches
Related show

Commit Message

Sebastian Reichel June 5, 2020, 10:43 p.m. UTC
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(-)

Comments

Linus Walleij June 10, 2020, 9:44 a.m. UTC | #1
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
Rob Herring June 15, 2020, 5:50 p.m. UTC | #2
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>
Sebastian Reichel June 19, 2020, 4:29 p.m. UTC | #3
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
>

Patch
diff mbox series

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");