| Message ID | 20200605224403.181015-3-sebastian.reichel@collabora.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | misc. gpio-charger patches | expand |
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
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>
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 >
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");
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(-)