| Message ID | 20170102163723.7939-9-quentin.schulz@free-electrons.com |
|---|---|
| State | Superseded |
| Headers | show |
On 02/01/17 11:37, Quentin Schulz wrote: > The X-Powers AXP20X and AXP22X PMICs expose the status of AC power > supply. > > Moreover, the AXP20X can also expose the current current and voltage > values of the AC power supply. > > This adds the driver which exposes the status of the AC power supply of > the AXP20X and AXP22X PMICs. > > Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com> For the IIO bits Acked-by: Jonathan Cameron <jic23@kernel.org> Trivial comment inline. > --- > drivers/power/supply/Kconfig | 12 ++ > drivers/power/supply/Makefile | 1 + > drivers/power/supply/axp20x_ac_power.c | 251 +++++++++++++++++++++++++++++++++ > 3 files changed, 264 insertions(+) > create mode 100644 drivers/power/supply/axp20x_ac_power.c > > diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig > index 76806a0..c552b4b 100644 > --- a/drivers/power/supply/Kconfig > +++ b/drivers/power/supply/Kconfig > @@ -214,6 +214,18 @@ config BATTERY_DA9150 > This driver can also be built as a module. If so, the module will be > called da9150-fg. > > +config CHARGER_AXP20X > + tristate "X-Powers AXP20X and AXP22X AC power supply driver" > + depends on MFD_AXP20X > + depends on AXP20X_ADC > + depends on IIO > + help > + Say Y here to enable support for X-Powers AXP20X and AXP22X PMICs' AC > + power supply. > + > + This driver can also be built as a module. If so, the module will be > + called axp20x_ac_power. > + > config AXP288_CHARGER > tristate "X-Powers AXP288 Charger" > depends on MFD_AXP20X && EXTCON_AXP288 > diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile > index 36c599d..7d22417 100644 > --- a/drivers/power/supply/Makefile > +++ b/drivers/power/supply/Makefile > @@ -18,6 +18,7 @@ obj-$(CONFIG_TEST_POWER) += test_power.o > > obj-$(CONFIG_BATTERY_88PM860X) += 88pm860x_battery.o > obj-$(CONFIG_BATTERY_ACT8945A) += act8945a_charger.o > +obj-$(CONFIG_CHARGER_AXP20X) += axp20x_ac_power.o > obj-$(CONFIG_BATTERY_DS2760) += ds2760_battery.o > obj-$(CONFIG_BATTERY_DS2780) += ds2780_battery.o > obj-$(CONFIG_BATTERY_DS2781) += ds2781_battery.o > diff --git a/drivers/power/supply/axp20x_ac_power.c b/drivers/power/supply/axp20x_ac_power.c > new file mode 100644 > index 0000000..d7bc25c > --- /dev/null > +++ b/drivers/power/supply/axp20x_ac_power.c > @@ -0,0 +1,251 @@ > +/* > + * AXP20X and AXP22X PMICs' ACIN power supply driver > + * > + * Copyright (C) 2016 Free Electrons > + * Quentin Schulz <quentin.schulz@free-electrons.com> > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License as published by the > + * Free Software Foundation; either version 2 of the License, or (at your > + * option) any later version. > + */ > + > +#include <linux/device.h> > +#include <linux/init.h> > +#include <linux/interrupt.h> > +#include <linux/kernel.h> > +#include <linux/mfd/axp20x.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/of_device.h> > +#include <linux/platform_device.h> > +#include <linux/power_supply.h> > +#include <linux/regmap.h> > +#include <linux/slab.h> > +#include <linux/iio/consumer.h> > + > +#define AXP20X_PWR_STATUS_ACIN_PRESENT BIT(7) > +#define AXP20X_PWR_STATUS_ACIN_AVAIL BIT(6) > + > +#define DRVNAME "axp20x-ac-power-supply" > + > +struct axp20x_ac_power { > + struct device_node *np; > + struct regmap *regmap; > + struct power_supply *supply; > + int axp20x_id; > + struct iio_channel *acin_v; > + struct iio_channel *acin_i; > +}; > + > +static irqreturn_t axp20x_ac_power_irq(int irq, void *devid) > +{ > + struct axp20x_ac_power *power = devid; > + > + power_supply_changed(power->supply); > + > + return IRQ_HANDLED; > +} > + > +static int axp20x_ac_power_get_property(struct power_supply *psy, > + enum power_supply_property psp, > + union power_supply_propval *val) > +{ > + struct axp20x_ac_power *power = power_supply_get_drvdata(psy); > + int ret, reg; > + > + switch (psp) { > + case POWER_SUPPLY_PROP_HEALTH: > + ret = regmap_read(power->regmap, AXP20X_PWR_INPUT_STATUS, ®); > + if (ret) > + return ret; > + > + if (reg & AXP20X_PWR_STATUS_ACIN_PRESENT) { > + val->intval = POWER_SUPPLY_HEALTH_GOOD; > + return 0; > + } > + > + val->intval = POWER_SUPPLY_HEALTH_UNKNOWN; > + return 0; > + > + case POWER_SUPPLY_PROP_PRESENT: > + ret = regmap_read(power->regmap, AXP20X_PWR_INPUT_STATUS, ®); > + if (ret) > + return ret; > + > + val->intval = !!(reg & AXP20X_PWR_STATUS_ACIN_PRESENT); > + return 0; > + > + case POWER_SUPPLY_PROP_ONLINE: > + ret = regmap_read(power->regmap, AXP20X_PWR_INPUT_STATUS, ®); > + if (ret) > + return ret; > + > + val->intval = !!(reg & AXP20X_PWR_STATUS_ACIN_AVAIL); > + return 0; > + > + case POWER_SUPPLY_PROP_VOLTAGE_NOW: > + ret = iio_read_channel_processed(power->acin_v, &val->intval); > + if (ret) > + return ret; > + > + /* > + * IIO framework gives mV but Power Supply framework gives µV. > + */ single line comment syntax throughout or we'll have to face a patch 'fixing' it. > + val->intval *= 1000; > + > + return 0; > + > + case POWER_SUPPLY_PROP_CURRENT_NOW: > + ret = iio_read_channel_processed(power->acin_i, &val->intval); > + if (ret) > + return ret; > + > + /* > + * IIO framework gives mV but Power Supply framework gives µV. > + */ > + val->intval *= 1000; > + > + return 0; > + > + default: > + return -EINVAL; > + } > + > + return -EINVAL; > +} > + > +static enum power_supply_property axp20x_ac_power_properties[] = { > + POWER_SUPPLY_PROP_HEALTH, > + POWER_SUPPLY_PROP_PRESENT, > + POWER_SUPPLY_PROP_ONLINE, > + POWER_SUPPLY_PROP_VOLTAGE_NOW, > + POWER_SUPPLY_PROP_CURRENT_NOW, > +}; > + > +static enum power_supply_property axp22x_ac_power_properties[] = { > + POWER_SUPPLY_PROP_HEALTH, > + POWER_SUPPLY_PROP_PRESENT, > + POWER_SUPPLY_PROP_ONLINE, > +}; > + > +static const struct power_supply_desc axp20x_ac_power_desc = { > + .name = "axp20x-ac", > + .type = POWER_SUPPLY_TYPE_MAINS, > + .properties = axp20x_ac_power_properties, > + .num_properties = ARRAY_SIZE(axp20x_ac_power_properties), > + .get_property = axp20x_ac_power_get_property, > +}; > + > +static const struct power_supply_desc axp22x_ac_power_desc = { > + .name = "axp22x-ac", > + .type = POWER_SUPPLY_TYPE_MAINS, > + .properties = axp22x_ac_power_properties, > + .num_properties = ARRAY_SIZE(axp22x_ac_power_properties), > + .get_property = axp20x_ac_power_get_property, > +}; > + > +static int axp20x_ac_power_probe(struct platform_device *pdev) > +{ > + struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent); > + struct power_supply_config psy_cfg = {}; > + struct axp20x_ac_power *power; > + static const char * const axp20x_irq_names[] = { "ACIN_PLUGIN", > + "ACIN_REMOVAL", NULL }; > + static const char * const *irq_names; > + const struct power_supply_desc *ac_power_desc; > + int i, irq, ret; > + > + if (!of_device_is_available(pdev->dev.of_node)) > + return -ENODEV; > + > + if (!axp20x) { > + dev_err(&pdev->dev, "Parent drvdata not set\n"); > + return -EINVAL; > + } > + > + power = devm_kzalloc(&pdev->dev, sizeof(*power), GFP_KERNEL); > + if (!power) > + return -ENOMEM; > + > + power->axp20x_id = (int)of_device_get_match_data(&pdev->dev); > + > + irq_names = axp20x_irq_names; > + > + if (power->axp20x_id == AXP202_ID) { > + ac_power_desc = &axp20x_ac_power_desc; > + > + power->acin_v = devm_iio_channel_get(&pdev->dev, "acin_v"); > + if (IS_ERR(power->acin_v)) { > + if (PTR_ERR(power->acin_v) == -ENODEV) > + return -EPROBE_DEFER; > + return PTR_ERR(power->acin_v); > + } > + > + power->acin_i = devm_iio_channel_get(&pdev->dev, "acin_i"); > + if (IS_ERR(power->acin_i)) { > + if (PTR_ERR(power->acin_i) == -ENODEV) > + return -EPROBE_DEFER; > + return PTR_ERR(power->acin_i); > + } > + } else { > + ac_power_desc = &axp22x_ac_power_desc; > + } > + > + power->np = pdev->dev.of_node; > + power->regmap = axp20x->regmap; > + > + platform_set_drvdata(pdev, power); > + > + psy_cfg.of_node = pdev->dev.of_node; > + psy_cfg.drv_data = power; > + > + power->supply = devm_power_supply_register(&pdev->dev, ac_power_desc, > + &psy_cfg); > + if (IS_ERR(power->supply)) > + return PTR_ERR(power->supply); > + > + /* Request irqs after registering, as irqs may trigger immediately */ > + for (i = 0; irq_names[i]; i++) { > + irq = platform_get_irq_byname(pdev, irq_names[i]); > + if (irq < 0) { > + dev_warn(&pdev->dev, "No IRQ for %s: %d\n", > + irq_names[i], irq); > + continue; > + } > + irq = regmap_irq_get_virq(axp20x->regmap_irqc, irq); > + ret = devm_request_any_context_irq(&pdev->dev, irq, > + axp20x_ac_power_irq, 0, > + DRVNAME, power); > + if (ret < 0) > + dev_warn(&pdev->dev, "Error requesting %s IRQ: %d\n", > + irq_names[i], ret); > + } > + > + return 0; > +} > + > +static const struct of_device_id axp20x_ac_power_match[] = { > + { > + .compatible = "x-powers,axp202-ac-power-supply", > + .data = (void *)AXP202_ID, > + }, { > + .compatible = "x-powers,axp221-ac-power-supply", > + .data = (void *)AXP221_ID, > + }, { /* sentinel */ } > +}; > +MODULE_DEVICE_TABLE(of, axp20x_ac_power_match); > + > +static struct platform_driver axp20x_ac_power_driver = { > + .probe = axp20x_ac_power_probe, > + .driver = { > + .name = DRVNAME, > + .of_match_table = axp20x_ac_power_match, > + }, > +}; > + > +module_platform_driver(axp20x_ac_power_driver); > + > +MODULE_AUTHOR("Quentin Schulz <quentin.schulz@free-electrons.com>"); > +MODULE_DESCRIPTION("AXP20X and AXP22X PMICs' AC power supply driver"); > +MODULE_LICENSE("GPL"); >
Hi Jonathan, On 07/01/2017 20:31, Jonathan Cameron wrote: > On 02/01/17 11:37, Quentin Schulz wrote: [...] >> + /* >> + * IIO framework gives mV but Power Supply framework gives µV. >> + */ > single line comment syntax throughout or we'll have to face a patch 'fixing' it. I actually had to make it a multiple lines comment to make checkpatch happy. Maybe there is something with the encoding of the µ symbol? I'll retry with a u instead and let you know. [...] Thanks, Quentin
Hi Quentin, The driver looks mostly fine. I do have a two comments, though. On Mon, Jan 02, 2017 at 05:37:08PM +0100, Quentin Schulz wrote: > [...] > > +static int axp20x_ac_power_probe(struct platform_device *pdev) > +{ > + static const char * const axp20x_irq_names[] = { "ACIN_PLUGIN", > + "ACIN_REMOVAL", NULL }; > > + static const char * const *irq_names; > + const struct power_supply_desc *ac_power_desc; > + int i, irq, ret; > + > + if (!of_device_is_available(pdev->dev.of_node)) > + return -ENODEV; > + > + if (!axp20x) { > + dev_err(&pdev->dev, "Parent drvdata not set\n"); > + return -EINVAL; > + } axp20x will no longer be needed after implementing below comments. > [...] > + irq_names = axp20x_irq_names; Just rename axp20x_irq_names into irq_names, since its only used here. > [...] > > + power->np = pdev->dev.of_node; This can be dropped, it's not used at all. > + power->regmap = axp20x->regmap; power->regmap = dev_get_regmap(pdev->dev.parent, NULL); > [...] > + /* Request irqs after registering, as irqs may trigger immediately */ > + for (i = 0; irq_names[i]; i++) { > + irq = platform_get_irq_byname(pdev, irq_names[i]); > + if (irq < 0) { > + dev_warn(&pdev->dev, "No IRQ for %s: %d\n", > + irq_names[i], irq); > + continue; > + } > + irq = regmap_irq_get_virq(axp20x->regmap_irqc, irq); The mapping should actually happen in the mfd driver, so that the platform resource contains a valid irq. > + ret = devm_request_any_context_irq(&pdev->dev, irq, > + axp20x_ac_power_irq, 0, > + DRVNAME, power); > + if (ret < 0) > + dev_warn(&pdev->dev, "Error requesting %s IRQ: %d\n", > + irq_names[i], ret); > + } -- Sebastian _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Sebastian, On 17/01/2017 04:00, Sebastian Reichel wrote: > Hi Quentin, > > The driver looks mostly fine. I do have a two comments, though. > > On Mon, Jan 02, 2017 at 05:37:08PM +0100, Quentin Schulz wrote: >> [...] >> >> +static int axp20x_ac_power_probe(struct platform_device *pdev) >> +{ >> + static const char * const axp20x_irq_names[] = { "ACIN_PLUGIN", >> + "ACIN_REMOVAL", NULL }; >> >> + static const char * const *irq_names; >> + const struct power_supply_desc *ac_power_desc; >> + int i, irq, ret; >> + >> + if (!of_device_is_available(pdev->dev.of_node)) >> + return -ENODEV; >> + >> + if (!axp20x) { >> + dev_err(&pdev->dev, "Parent drvdata not set\n"); >> + return -EINVAL; >> + } > > axp20x will no longer be needed after implementing below > comments. > >> [...] >> + irq_names = axp20x_irq_names; > > Just rename axp20x_irq_names into irq_names, since its only used > here. > >> [...] >> >> + power->np = pdev->dev.of_node; > > This can be dropped, it's not used at all. > >> + power->regmap = axp20x->regmap; > > power->regmap = dev_get_regmap(pdev->dev.parent, NULL); > ACK on everything above. >> [...] >> + /* Request irqs after registering, as irqs may trigger immediately */ >> + for (i = 0; irq_names[i]; i++) { >> + irq = platform_get_irq_byname(pdev, irq_names[i]); >> + if (irq < 0) { >> + dev_warn(&pdev->dev, "No IRQ for %s: %d\n", >> + irq_names[i], irq); >> + continue; >> + } >> + irq = regmap_irq_get_virq(axp20x->regmap_irqc, irq); > > The mapping should actually happen in the mfd driver, so that > the platform resource contains a valid irq. > I've come with this solution: ------------------------------------------------------------------------ ------------------------------------------------------------------------ However, this implies that all cells added by the mfd driver which are requesting irqs will need to be changed in the same commit to remove the regmap_irq_get_virq calls. If we don't modify the drivers, they will purely fail to request the irqs. The impacted drivers are the following: - drivers/extcon/extcon-axp288.c - drivers/input/misc/axp20x-pek.c - drivers/power/supply/axp20x_usb_power.c - drivers/power/supply/axp288_charger.c - drivers/power/supply/axp288_fuel_gauge.c Is it really worth to do such a cleanup? I'm assuming that impacting four different subsystems at the same time might require a bit of time to make the patch into the kernel. I don't see also another way than doing one single patch for all changes since the changes in the mfd driver will break all aforementioned drivers. Thanks, Quentin -- Quentin Schulz, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kerneldiff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c index 012c064..117eacb 100644 --- a/drivers/mfd/axp20x.c +++ b/drivers/mfd/axp20x.c @@ -882,7 +882,7 @@ EXPORT_SYMBOL(axp20x_match_device); int axp20x_device_probe(struct axp20x_dev *axp20x) { - int ret; + int ret, irq_base; ret = regmap_add_irq_chip(axp20x->regmap, axp20x->irq, IRQF_ONESHOT | IRQF_SHARED, -1, @@ -893,8 +893,9 @@ int axp20x_device_probe(struct axp20x_dev *axp20x) return ret; } + irq_base = regmap_irq_chip_get_base(axp20x->regmap_irqc); ret = mfd_add_devices(axp20x->dev, -1, axp20x->cells, - axp20x->nr_cells, NULL, 0, NULL); + axp20x->nr_cells, NULL, irq_base, NULL); if (ret) { dev_err(axp20x->dev, "failed to add MFD devices: %d\n", ret);
On Thu, Jan 26, 2017 at 02:32:21PM +0100, Quentin Schulz wrote: > I've come with this solution: > > ------------------------------------------------------------------------ > diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c > index 012c064..117eacb 100644 > --- a/drivers/mfd/axp20x.c > +++ b/drivers/mfd/axp20x.c > @@ -882,7 +882,7 @@ EXPORT_SYMBOL(axp20x_match_device); > > int axp20x_device_probe(struct axp20x_dev *axp20x) > { > - int ret; > + int ret, irq_base; > > ret = regmap_add_irq_chip(axp20x->regmap, axp20x->irq, > IRQF_ONESHOT | IRQF_SHARED, -1, > @@ -893,8 +893,9 @@ int axp20x_device_probe(struct axp20x_dev *axp20x) > return ret; > } > > + irq_base = regmap_irq_chip_get_base(axp20x->regmap_irqc); > ret = mfd_add_devices(axp20x->dev, -1, axp20x->cells, > - axp20x->nr_cells, NULL, 0, NULL); > + axp20x->nr_cells, NULL, irq_base, NULL); > > if (ret) { > dev_err(axp20x->dev, "failed to add MFD devices: %d\n", ret); > ------------------------------------------------------------------------ > > However, this implies that all cells added by the mfd driver which are > requesting irqs will need to be changed in the same commit to remove the > regmap_irq_get_virq calls. If we don't modify the drivers, they will > purely fail to request the irqs. > > The impacted drivers are the following: > > - drivers/extcon/extcon-axp288.c > - drivers/input/misc/axp20x-pek.c > - drivers/power/supply/axp20x_usb_power.c > - drivers/power/supply/axp288_charger.c > - drivers/power/supply/axp288_fuel_gauge.c > > Is it really worth to do such a cleanup? Yes. The current behaviour goes against what everyone is expecting from the API. > I'm assuming that impacting four different subsystems at the same > time might require a bit of time to make the patch into the > kernel. I don't see also another way than doing one single patch for > all changes since the changes in the mfd driver will break all > aforementioned drivers. However, I think that can be fixed in a later, independant serie. This serie is quite big already and this has been long overdue, so I'd really like not to delay it once again because of a dependency on a cross-tree cleanup. Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 27/01/17 08:20, Maxime Ripard wrote: > On Thu, Jan 26, 2017 at 02:32:21PM +0100, Quentin Schulz wrote: >> I've come with this solution: >> >> ------------------------------------------------------------------------ >> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c >> index 012c064..117eacb 100644 >> --- a/drivers/mfd/axp20x.c >> +++ b/drivers/mfd/axp20x.c >> @@ -882,7 +882,7 @@ EXPORT_SYMBOL(axp20x_match_device); >> >> int axp20x_device_probe(struct axp20x_dev *axp20x) >> { >> - int ret; >> + int ret, irq_base; >> >> ret = regmap_add_irq_chip(axp20x->regmap, axp20x->irq, >> IRQF_ONESHOT | IRQF_SHARED, -1, >> @@ -893,8 +893,9 @@ int axp20x_device_probe(struct axp20x_dev *axp20x) >> return ret; >> } >> >> + irq_base = regmap_irq_chip_get_base(axp20x->regmap_irqc); >> ret = mfd_add_devices(axp20x->dev, -1, axp20x->cells, >> - axp20x->nr_cells, NULL, 0, NULL); >> + axp20x->nr_cells, NULL, irq_base, NULL); >> >> if (ret) { >> dev_err(axp20x->dev, "failed to add MFD devices: %d\n", ret); >> ------------------------------------------------------------------------ >> >> However, this implies that all cells added by the mfd driver which are >> requesting irqs will need to be changed in the same commit to remove the >> regmap_irq_get_virq calls. If we don't modify the drivers, they will >> purely fail to request the irqs. >> >> The impacted drivers are the following: >> >> - drivers/extcon/extcon-axp288.c >> - drivers/input/misc/axp20x-pek.c >> - drivers/power/supply/axp20x_usb_power.c >> - drivers/power/supply/axp288_charger.c >> - drivers/power/supply/axp288_fuel_gauge.c >> >> Is it really worth to do such a cleanup? > > Yes. The current behaviour goes against what everyone is expecting > from the API. > >> I'm assuming that impacting four different subsystems at the same >> time might require a bit of time to make the patch into the >> kernel. I don't see also another way than doing one single patch for >> all changes since the changes in the mfd driver will break all >> aforementioned drivers. > > However, I think that can be fixed in a later, independant serie. This > serie is quite big already and this has been long overdue, so I'd > really like not to delay it once again because of a dependency on a > cross-tree cleanup. It's not that cross tree really. Lining up this level of change to go through an immutable branch pulled into each of the relevant trees isn't too hard to arrange. Jonathan > > Maxime > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi, On Sat, Jan 28, 2017 at 02:30:22PM +0000, Jonathan Cameron wrote: > On 27/01/17 08:20, Maxime Ripard wrote: > > On Thu, Jan 26, 2017 at 02:32:21PM +0100, Quentin Schulz wrote: > >> I've come with this solution: > >> > >> ------------------------------------------------------------------------ > >> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c > >> index 012c064..117eacb 100644 > >> --- a/drivers/mfd/axp20x.c > >> +++ b/drivers/mfd/axp20x.c > >> @@ -882,7 +882,7 @@ EXPORT_SYMBOL(axp20x_match_device); > >> > >> int axp20x_device_probe(struct axp20x_dev *axp20x) > >> { > >> - int ret; > >> + int ret, irq_base; > >> > >> ret = regmap_add_irq_chip(axp20x->regmap, axp20x->irq, > >> IRQF_ONESHOT | IRQF_SHARED, -1, > >> @@ -893,8 +893,9 @@ int axp20x_device_probe(struct axp20x_dev *axp20x) > >> return ret; > >> } > >> > >> + irq_base = regmap_irq_chip_get_base(axp20x->regmap_irqc); > >> ret = mfd_add_devices(axp20x->dev, -1, axp20x->cells, > >> - axp20x->nr_cells, NULL, 0, NULL); > >> + axp20x->nr_cells, NULL, irq_base, NULL); > >> > >> if (ret) { > >> dev_err(axp20x->dev, "failed to add MFD devices: %d\n", ret); > >> ------------------------------------------------------------------------ > >> > >> However, this implies that all cells added by the mfd driver which are > >> requesting irqs will need to be changed in the same commit to remove the > >> regmap_irq_get_virq calls. If we don't modify the drivers, they will > >> purely fail to request the irqs. > >> > >> The impacted drivers are the following: > >> > >> - drivers/extcon/extcon-axp288.c > >> - drivers/input/misc/axp20x-pek.c > >> - drivers/power/supply/axp20x_usb_power.c > >> - drivers/power/supply/axp288_charger.c > >> - drivers/power/supply/axp288_fuel_gauge.c So mostly power-supply, which is affected by this series anyways. Only input & extcon are added. > >> Is it really worth to do such a cleanup? > > > > Yes. The current behaviour goes against what everyone is expecting > > from the API. > > > >> I'm assuming that impacting four different subsystems at the same > >> time might require a bit of time to make the patch into the > >> kernel. I don't see also another way than doing one single patch for > >> all changes since the changes in the mfd driver will break all > >> aforementioned drivers. > > > > However, I think that can be fixed in a later, independant serie. This > > serie is quite big already and this has been long overdue, so I'd > > really like not to delay it once again because of a dependency on a > > cross-tree cleanup. > > It's not that cross tree really. Lining up this level of change to > go through an immutable branch pulled into each of the relevant trees > isn't too hard to arrange. I'm fine with doing this as a separate series, if it follows directly behind this one. Mainlined drivers tend to be used as template for new ones and this invalid IRQ resources are a bad example. -- Sebastian _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig index 76806a0..c552b4b 100644 --- a/drivers/power/supply/Kconfig +++ b/drivers/power/supply/Kconfig @@ -214,6 +214,18 @@ config BATTERY_DA9150 This driver can also be built as a module. If so, the module will be called da9150-fg. +config CHARGER_AXP20X + tristate "X-Powers AXP20X and AXP22X AC power supply driver" + depends on MFD_AXP20X + depends on AXP20X_ADC + depends on IIO + help + Say Y here to enable support for X-Powers AXP20X and AXP22X PMICs' AC + power supply. + + This driver can also be built as a module. If so, the module will be + called axp20x_ac_power. + config AXP288_CHARGER tristate "X-Powers AXP288 Charger" depends on MFD_AXP20X && EXTCON_AXP288 diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile index 36c599d..7d22417 100644 --- a/drivers/power/supply/Makefile +++ b/drivers/power/supply/Makefile @@ -18,6 +18,7 @@ obj-$(CONFIG_TEST_POWER) += test_power.o obj-$(CONFIG_BATTERY_88PM860X) += 88pm860x_battery.o obj-$(CONFIG_BATTERY_ACT8945A) += act8945a_charger.o +obj-$(CONFIG_CHARGER_AXP20X) += axp20x_ac_power.o obj-$(CONFIG_BATTERY_DS2760) += ds2760_battery.o obj-$(CONFIG_BATTERY_DS2780) += ds2780_battery.o obj-$(CONFIG_BATTERY_DS2781) += ds2781_battery.o diff --git a/drivers/power/supply/axp20x_ac_power.c b/drivers/power/supply/axp20x_ac_power.c new file mode 100644 index 0000000..d7bc25c --- /dev/null +++ b/drivers/power/supply/axp20x_ac_power.c @@ -0,0 +1,251 @@ +/* + * AXP20X and AXP22X PMICs' ACIN power supply driver + * + * Copyright (C) 2016 Free Electrons + * Quentin Schulz <quentin.schulz@free-electrons.com> + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. + */ + +#include <linux/device.h> +#include <linux/init.h> +#include <linux/interrupt.h> +#include <linux/kernel.h> +#include <linux/mfd/axp20x.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/platform_device.h> +#include <linux/power_supply.h> +#include <linux/regmap.h> +#include <linux/slab.h> +#include <linux/iio/consumer.h> + +#define AXP20X_PWR_STATUS_ACIN_PRESENT BIT(7) +#define AXP20X_PWR_STATUS_ACIN_AVAIL BIT(6) + +#define DRVNAME "axp20x-ac-power-supply" + +struct axp20x_ac_power { + struct device_node *np; + struct regmap *regmap; + struct power_supply *supply; + int axp20x_id; + struct iio_channel *acin_v; + struct iio_channel *acin_i; +}; + +static irqreturn_t axp20x_ac_power_irq(int irq, void *devid) +{ + struct axp20x_ac_power *power = devid; + + power_supply_changed(power->supply); + + return IRQ_HANDLED; +} + +static int axp20x_ac_power_get_property(struct power_supply *psy, + enum power_supply_property psp, + union power_supply_propval *val) +{ + struct axp20x_ac_power *power = power_supply_get_drvdata(psy); + int ret, reg; + + switch (psp) { + case POWER_SUPPLY_PROP_HEALTH: + ret = regmap_read(power->regmap, AXP20X_PWR_INPUT_STATUS, ®); + if (ret) + return ret; + + if (reg & AXP20X_PWR_STATUS_ACIN_PRESENT) { + val->intval = POWER_SUPPLY_HEALTH_GOOD; + return 0; + } + + val->intval = POWER_SUPPLY_HEALTH_UNKNOWN; + return 0; + + case POWER_SUPPLY_PROP_PRESENT: + ret = regmap_read(power->regmap, AXP20X_PWR_INPUT_STATUS, ®); + if (ret) + return ret; + + val->intval = !!(reg & AXP20X_PWR_STATUS_ACIN_PRESENT); + return 0; + + case POWER_SUPPLY_PROP_ONLINE: + ret = regmap_read(power->regmap, AXP20X_PWR_INPUT_STATUS, ®); + if (ret) + return ret; + + val->intval = !!(reg & AXP20X_PWR_STATUS_ACIN_AVAIL); + return 0; + + case POWER_SUPPLY_PROP_VOLTAGE_NOW: + ret = iio_read_channel_processed(power->acin_v, &val->intval); + if (ret) + return ret; + + /* + * IIO framework gives mV but Power Supply framework gives µV. + */ + val->intval *= 1000; + + return 0; + + case POWER_SUPPLY_PROP_CURRENT_NOW: + ret = iio_read_channel_processed(power->acin_i, &val->intval); + if (ret) + return ret; + + /* + * IIO framework gives mV but Power Supply framework gives µV. + */ + val->intval *= 1000; + + return 0; + + default: + return -EINVAL; + } + + return -EINVAL; +} + +static enum power_supply_property axp20x_ac_power_properties[] = { + POWER_SUPPLY_PROP_HEALTH, + POWER_SUPPLY_PROP_PRESENT, + POWER_SUPPLY_PROP_ONLINE, + POWER_SUPPLY_PROP_VOLTAGE_NOW, + POWER_SUPPLY_PROP_CURRENT_NOW, +}; + +static enum power_supply_property axp22x_ac_power_properties[] = { + POWER_SUPPLY_PROP_HEALTH, + POWER_SUPPLY_PROP_PRESENT, + POWER_SUPPLY_PROP_ONLINE, +}; + +static const struct power_supply_desc axp20x_ac_power_desc = { + .name = "axp20x-ac", + .type = POWER_SUPPLY_TYPE_MAINS, + .properties = axp20x_ac_power_properties, + .num_properties = ARRAY_SIZE(axp20x_ac_power_properties), + .get_property = axp20x_ac_power_get_property, +}; + +static const struct power_supply_desc axp22x_ac_power_desc = { + .name = "axp22x-ac", + .type = POWER_SUPPLY_TYPE_MAINS, + .properties = axp22x_ac_power_properties, + .num_properties = ARRAY_SIZE(axp22x_ac_power_properties), + .get_property = axp20x_ac_power_get_property, +}; + +static int axp20x_ac_power_probe(struct platform_device *pdev) +{ + struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent); + struct power_supply_config psy_cfg = {}; + struct axp20x_ac_power *power; + static const char * const axp20x_irq_names[] = { "ACIN_PLUGIN", + "ACIN_REMOVAL", NULL }; + static const char * const *irq_names; + const struct power_supply_desc *ac_power_desc; + int i, irq, ret; + + if (!of_device_is_available(pdev->dev.of_node)) + return -ENODEV; + + if (!axp20x) { + dev_err(&pdev->dev, "Parent drvdata not set\n"); + return -EINVAL; + } + + power = devm_kzalloc(&pdev->dev, sizeof(*power), GFP_KERNEL); + if (!power) + return -ENOMEM; + + power->axp20x_id = (int)of_device_get_match_data(&pdev->dev); + + irq_names = axp20x_irq_names; + + if (power->axp20x_id == AXP202_ID) { + ac_power_desc = &axp20x_ac_power_desc; + + power->acin_v = devm_iio_channel_get(&pdev->dev, "acin_v"); + if (IS_ERR(power->acin_v)) { + if (PTR_ERR(power->acin_v) == -ENODEV) + return -EPROBE_DEFER; + return PTR_ERR(power->acin_v); + } + + power->acin_i = devm_iio_channel_get(&pdev->dev, "acin_i"); + if (IS_ERR(power->acin_i)) { + if (PTR_ERR(power->acin_i) == -ENODEV) + return -EPROBE_DEFER; + return PTR_ERR(power->acin_i); + } + } else { + ac_power_desc = &axp22x_ac_power_desc; + } + + power->np = pdev->dev.of_node; + power->regmap = axp20x->regmap; + + platform_set_drvdata(pdev, power); + + psy_cfg.of_node = pdev->dev.of_node; + psy_cfg.drv_data = power; + + power->supply = devm_power_supply_register(&pdev->dev, ac_power_desc, + &psy_cfg); + if (IS_ERR(power->supply)) + return PTR_ERR(power->supply); + + /* Request irqs after registering, as irqs may trigger immediately */ + for (i = 0; irq_names[i]; i++) { + irq = platform_get_irq_byname(pdev, irq_names[i]); + if (irq < 0) { + dev_warn(&pdev->dev, "No IRQ for %s: %d\n", + irq_names[i], irq); + continue; + } + irq = regmap_irq_get_virq(axp20x->regmap_irqc, irq); + ret = devm_request_any_context_irq(&pdev->dev, irq, + axp20x_ac_power_irq, 0, + DRVNAME, power); + if (ret < 0) + dev_warn(&pdev->dev, "Error requesting %s IRQ: %d\n", + irq_names[i], ret); + } + + return 0; +} + +static const struct of_device_id axp20x_ac_power_match[] = { + { + .compatible = "x-powers,axp202-ac-power-supply", + .data = (void *)AXP202_ID, + }, { + .compatible = "x-powers,axp221-ac-power-supply", + .data = (void *)AXP221_ID, + }, { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, axp20x_ac_power_match); + +static struct platform_driver axp20x_ac_power_driver = { + .probe = axp20x_ac_power_probe, + .driver = { + .name = DRVNAME, + .of_match_table = axp20x_ac_power_match, + }, +}; + +module_platform_driver(axp20x_ac_power_driver); + +MODULE_AUTHOR("Quentin Schulz <quentin.schulz@free-electrons.com>"); +MODULE_DESCRIPTION("AXP20X and AXP22X PMICs' AC power supply driver"); +MODULE_LICENSE("GPL");
The X-Powers AXP20X and AXP22X PMICs expose the status of AC power supply. Moreover, the AXP20X can also expose the current current and voltage values of the AC power supply. This adds the driver which exposes the status of the AC power supply of the AXP20X and AXP22X PMICs. Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com> --- drivers/power/supply/Kconfig | 12 ++ drivers/power/supply/Makefile | 1 + drivers/power/supply/axp20x_ac_power.c | 251 +++++++++++++++++++++++++++++++++ 3 files changed, 264 insertions(+) create mode 100644 drivers/power/supply/axp20x_ac_power.c