| Message ID | 1435316357-26606-1-git-send-email-hdegoede@redhat.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
W dniu 26.06.2015 o 19:59, Hans de Goede pisze: > This adds a driver for the usb power_supply bits of the axp20x PMICs. > > I initially started writing my own driver, before coming aware of > Bruno Prémont's excellent earlier RFC with a driver for this. > > My driver was lacking CURRENT_MAX and VOLTAGE_MIN support Bruno's > drvier has, so I've copied the code for those from his driver. > > Note that the AC-power-supply and battery charger bits will need separate > drivers. Each one needs its own devictree child-node so that other > devicetree nodes can reference the right power-supply, and thus each one > will get its own mfd-cell / platform_device and platform-driver. > > Cc: Bruno Prémont <bonbons@linux-vserver.org> > Acked-by: Lee Jones <lee.jones@linaro.org> > Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > Changes in v2: > -Split out the dt-bindings documentation into a separate patch > -Renamed axp20x_read_16bit to axp20x_read_variable_width > -Use better local variable names inside axp20x_read_variable_width > Changes in v3: > -Add Bruno's S-o-b Hi, Two minor comments below, the driver itself looks good. > --- > drivers/power/Kconfig | 7 ++ > drivers/power/Makefile | 1 + > drivers/power/axp20x_usb_power.c | 243 +++++++++++++++++++++++++++++++++++++++ > include/linux/mfd/axp20x.h | 24 ++++ > 4 files changed, 275 insertions(+) > create mode 100644 drivers/power/axp20x_usb_power.c > > diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig > index 4091fb0..1fee60c 100644 > --- a/drivers/power/Kconfig > +++ b/drivers/power/Kconfig > @@ -439,6 +439,13 @@ config BATTERY_RT5033 > The fuelgauge calculates and determines the battery state of charge > according to battery open circuit voltage. > > +config AXP20X_POWER > + tristate "AXP20x power supply driver" > + depends on MFD_AXP20X > + help > + This driver provides support for the power supply features of > + AXP20x PMIC. > + > source "drivers/power/reset/Kconfig" > > endif # POWER_SUPPLY > diff --git a/drivers/power/Makefile b/drivers/power/Makefile > index b7b0181..ae0f27d 100644 > --- a/drivers/power/Makefile > +++ b/drivers/power/Makefile > @@ -9,6 +9,7 @@ obj-$(CONFIG_GENERIC_ADC_BATTERY) += generic-adc-battery.o > > obj-$(CONFIG_PDA_POWER) += pda_power.o > obj-$(CONFIG_APM_POWER) += apm_power.o > +obj-$(CONFIG_AXP20X_POWER) += axp20x_usb_power.o > obj-$(CONFIG_MAX8925_POWER) += max8925_power.o > obj-$(CONFIG_WM831X_BACKUP) += wm831x_backup.o > obj-$(CONFIG_WM831X_POWER) += wm831x_power.o > diff --git a/drivers/power/axp20x_usb_power.c b/drivers/power/axp20x_usb_power.c > new file mode 100644 > index 0000000..09f388e > --- /dev/null > +++ b/drivers/power/axp20x_usb_power.c > @@ -0,0 +1,243 @@ > +/* > + * AXP20x PMIC USB power supply status driver > + * > + * Copyright (C) 2015 Hans de Goede <hdegoede@redhat.com> > + * Copyright (C) 2014 Bruno Prémont <bonbons@linux-vserver.org> > + * > + * 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/platform_device.h> > +#include <linux/power_supply.h> > +#include <linux/regmap.h> > +#include <linux/slab.h> > + > +#define DRVNAME "axp20x-usb-power-supply" > + > +#define AXP20X_PWR_STATUS_VBUS_PRESENT BIT(5) > +#define AXP20X_PWR_STATUS_VBUS_USED BIT(4) > + > +#define AXP20X_USB_STATUS_VBUS_VALID BIT(2) > + > +#define AXP20X_VBUS_VHOLD_uV(b) (4000000 + (((b) >> 3) & 7) * 100000) > +#define AXP20X_VBUS_CLIMIT_MASK 3 > +#define AXP20X_VBUC_CLIMIT_900mA 0 > +#define AXP20X_VBUC_CLIMIT_500mA 1 > +#define AXP20X_VBUC_CLIMIT_100mA 2 > +#define AXP20X_VBUC_CLIMIT_NONE 3 > + > +#define AXP20X_ADC_EN1_VBUS_CURR BIT(2) > +#define AXP20X_ADC_EN1_VBUS_VOLT BIT(3) > + > +#define AXP20X_VBUS_MON_VBUS_VALID BIT(3) > + > +struct axp20x_usb_power { > + struct regmap *regmap; > + struct power_supply *supply; > +}; > + > +static irqreturn_t axp20x_usb_power_irq(int irq, void *devid) > +{ > + struct axp20x_usb_power *power = devid; > + > + power_supply_changed(power->supply); > + > + return IRQ_HANDLED; > +} > + > +static int axp20x_usb_power_get_property(struct power_supply *psy, > + enum power_supply_property psp, union power_supply_propval *val) > +{ > + struct axp20x_usb_power *power = power_supply_get_drvdata(psy); > + unsigned int input, v; > + int r; Here and in probe function - it is strictly personal but I don't like the 'r' variable. I got used to 'err' or 'ret' which are more descriptive to me. 'r' may look like 'resource' for example. > + > + switch (psp) { > + case POWER_SUPPLY_PROP_VOLTAGE_MIN: > + r = regmap_read(power->regmap, AXP20X_VBUS_IPSOUT_MGMT, &v); > + if (r) > + return r; > + > + val->intval = AXP20X_VBUS_VHOLD_uV(v); > + return 0; > + case POWER_SUPPLY_PROP_VOLTAGE_NOW: > + r = axp20x_read_variable_width(power->regmap, > + AXP20X_VBUS_V_ADC_H, 12); > + if (r < 0) > + return r; > + > + val->intval = r * 1700; /* 1 step = 1.7 mV */ > + return 0; > + case POWER_SUPPLY_PROP_CURRENT_MAX: > + r = regmap_read(power->regmap, AXP20X_VBUS_IPSOUT_MGMT, &v); > + if (r) > + return r; > + > + switch (v & AXP20X_VBUS_CLIMIT_MASK) { > + case AXP20X_VBUC_CLIMIT_100mA: > + val->intval = 100000; > + break; > + case AXP20X_VBUC_CLIMIT_500mA: > + val->intval = 500000; > + break; > + case AXP20X_VBUC_CLIMIT_900mA: > + val->intval = 900000; > + break; > + case AXP20X_VBUC_CLIMIT_NONE: > + val->intval = -1; > + break; > + } > + return 0; > + case POWER_SUPPLY_PROP_CURRENT_NOW: > + r = axp20x_read_variable_width(power->regmap, > + AXP20X_VBUS_I_ADC_H, 12); > + if (r < 0) > + return r; > + > + val->intval = r * 375; /* 1 step = 0.375 mA */ > + return 0; > + default: > + break; > + } > + > + /* All the properties below need the input-status reg value */ > + r = regmap_read(power->regmap, AXP20X_PWR_INPUT_STATUS, &input); > + if (r) > + return r; > + > + switch (psp) { > + case POWER_SUPPLY_PROP_HEALTH: > + if (!(input & AXP20X_PWR_STATUS_VBUS_PRESENT)) { > + val->intval = POWER_SUPPLY_HEALTH_UNKNOWN; > + break; > + } > + > + r = regmap_read(power->regmap, AXP20X_USB_OTG_STATUS, &v); > + if (r) > + return r; > + > + if (!(v & AXP20X_USB_STATUS_VBUS_VALID)) { > + val->intval = POWER_SUPPLY_HEALTH_UNSPEC_FAILURE; > + break; > + } > + > + val->intval = POWER_SUPPLY_HEALTH_GOOD; > + break; > + case POWER_SUPPLY_PROP_PRESENT: > + val->intval = !!(input & AXP20X_PWR_STATUS_VBUS_PRESENT); > + break; > + case POWER_SUPPLY_PROP_ONLINE: > + val->intval = !!(input & AXP20X_PWR_STATUS_VBUS_USED); > + break; > + default: > + return -EINVAL; > + } > + > + return 0; > +} > + > +static enum power_supply_property axp20x_usb_power_properties[] = { > + POWER_SUPPLY_PROP_HEALTH, > + POWER_SUPPLY_PROP_PRESENT, > + POWER_SUPPLY_PROP_ONLINE, > + POWER_SUPPLY_PROP_VOLTAGE_MIN, > + POWER_SUPPLY_PROP_VOLTAGE_NOW, > + POWER_SUPPLY_PROP_CURRENT_MAX, > + POWER_SUPPLY_PROP_CURRENT_NOW, > +}; > + > +static const struct power_supply_desc axp20x_usb_power_desc = { > + .name = "axp20x-usb", > + .type = POWER_SUPPLY_TYPE_USB, > + .properties = axp20x_usb_power_properties, > + .num_properties = ARRAY_SIZE(axp20x_usb_power_properties), > + .get_property = axp20x_usb_power_get_property, > +}; > + > +static int axp20x_usb_power_probe(struct platform_device *pdev) > +{ > + struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent); > + struct power_supply_config psy_cfg = {}; > + struct axp20x_usb_power *power; > + static const char * const irq_names[] = { "VBUS_PLUGIN", > + "VBUS_REMOVAL", "VBUS_VALID", "VBUS_NOT_VALID" }; > + int i, irq, r; > + > + if (!of_device_is_available(pdev->dev.of_node)) > + return -ENODEV; > + > + power = devm_kzalloc(&pdev->dev, sizeof(*power), GFP_KERNEL); > + if (!power) > + return -ENOMEM; > + > + power->regmap = axp20x->regmap; > + > + /* Enable vbus valid checking */ > + r = regmap_update_bits(power->regmap, AXP20X_VBUS_MON, > + AXP20X_VBUS_MON_VBUS_VALID, AXP20X_VBUS_MON_VBUS_VALID); > + if (r) > + return r; > + > + /* Enable vbus voltage and current measurement */ > + r = regmap_update_bits(power->regmap, AXP20X_ADC_EN1, > + AXP20X_ADC_EN1_VBUS_CURR | AXP20X_ADC_EN1_VBUS_VOLT, > + AXP20X_ADC_EN1_VBUS_CURR | AXP20X_ADC_EN1_VBUS_VOLT); > + if (r) > + return r; > + > + psy_cfg.of_node = pdev->dev.of_node; > + psy_cfg.drv_data = power; > + > + power->supply = devm_power_supply_register(&pdev->dev, > + &axp20x_usb_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; i < ARRAY_SIZE(irq_names); 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); > + r = devm_request_any_context_irq(&pdev->dev, irq, > + axp20x_usb_power_irq, 0, DRVNAME, power); > + if (r < 0) > + dev_warn(&pdev->dev, "Error requesting %s IRQ: %d\n", > + irq_names[i], r); > + } > + > + return 0; > +} > + > +static const struct of_device_id axp20x_usb_power_match[] = { > + { .compatible = "x-powers,axp202-usb-power-supply" }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, axp20x_usb_power_match); > + > +static struct platform_driver axp20x_usb_power_driver = { > + .probe = axp20x_usb_power_probe, > + .driver = { > + .name = DRVNAME, > + .of_match_table = axp20x_usb_power_match, > + }, > +}; > + > +module_platform_driver(axp20x_usb_power_driver); > + > +MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com>"); > +MODULE_DESCRIPTION("AXP20x PMIC USB power supply status driver"); > +MODULE_LICENSE("GPL"); > diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h > index 6d8b39a..8ec996e 100644 > --- a/include/linux/mfd/axp20x.h > +++ b/include/linux/mfd/axp20x.h > @@ -11,6 +11,8 @@ > #ifndef __LINUX_MFD_AXP20X_H > #define __LINUX_MFD_AXP20X_H > > +#include <linux/regmap.h> > + > enum { > AXP202_ID = 0, > AXP209_ID, > @@ -372,4 +374,26 @@ struct axp288_extcon_pdata { > struct gpio_desc *gpio_mux_cntl; > }; > > +/* generic helper function for reading 9-16 bit wide regs */ > +static inline int axp20x_read_variable_width(struct regmap *regmap, > + unsigned int reg, unsigned int width) > +{ > + unsigned int reg_val, result; > + int err; > + > + err = regmap_read(regmap, reg, ®_val); > + if (err) > + return err; > + > + result = reg_val << (width - 8); > + > + err = regmap_read(regmap, reg + 1, ®_val); > + if (err) > + return err; > + > + result |= reg_val; > + > + return result; > +} > + > #endif /* __LINUX_MFD_AXP20X_H */ Is this static inline function used outside of drivers/power/axp20x_usb_power.c? Looks like it isn't, but maybe you have a plan for it? If not, then it shouldn't be in header file. Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, 27 Jun 2015 14:28:40 Krzysztof Kozlowski wrote: > W dniu 26.06.2015 o 19:59, Hans de Goede pisze: > > This adds a driver for the usb power_supply bits of the axp20x PMICs. > > > > I initially started writing my own driver, before coming aware of > > Bruno Prémont's excellent earlier RFC with a driver for this. > > > > My driver was lacking CURRENT_MAX and VOLTAGE_MIN support Bruno's > > drvier has, so I've copied the code for those from his driver. > > > > Note that the AC-power-supply and battery charger bits will need separate > > drivers. Each one needs its own devictree child-node so that other > > devicetree nodes can reference the right power-supply, and thus each one > > will get its own mfd-cell / platform_device and platform-driver. > > > > Cc: Bruno Prémont <bonbons@linux-vserver.org> > > Acked-by: Lee Jones <lee.jones@linaro.org> > > Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org> > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > > --- > > Changes in v2: > > -Split out the dt-bindings documentation into a separate patch > > -Renamed axp20x_read_16bit to axp20x_read_variable_width > > -Use better local variable names inside axp20x_read_variable_width > > Changes in v3: > > -Add Bruno's S-o-b > > Hi, > > Two minor comments below, the driver itself looks good. > > > > --- > > drivers/power/Kconfig | 7 ++ > > drivers/power/Makefile | 1 + > > drivers/power/axp20x_usb_power.c | 243 +++++++++++++++++++++++++++++++++++++++ > > include/linux/mfd/axp20x.h | 24 ++++ > > 4 files changed, 275 insertions(+) > > create mode 100644 drivers/power/axp20x_usb_power.c > > > > diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig > > index 4091fb0..1fee60c 100644 > > --- a/drivers/power/Kconfig > > +++ b/drivers/power/Kconfig > > @@ -439,6 +439,13 @@ config BATTERY_RT5033 > > The fuelgauge calculates and determines the battery state of charge > > according to battery open circuit voltage. > > > > +config AXP20X_POWER > > + tristate "AXP20x power supply driver" > > + depends on MFD_AXP20X > > + help > > + This driver provides support for the power supply features of > > + AXP20x PMIC. > > + > > source "drivers/power/reset/Kconfig" > > > > endif # POWER_SUPPLY > > diff --git a/drivers/power/Makefile b/drivers/power/Makefile > > index b7b0181..ae0f27d 100644 > > --- a/drivers/power/Makefile > > +++ b/drivers/power/Makefile > > @@ -9,6 +9,7 @@ obj-$(CONFIG_GENERIC_ADC_BATTERY) += generic-adc-battery.o > > > > obj-$(CONFIG_PDA_POWER) += pda_power.o > > obj-$(CONFIG_APM_POWER) += apm_power.o > > +obj-$(CONFIG_AXP20X_POWER) += axp20x_usb_power.o > > obj-$(CONFIG_MAX8925_POWER) += max8925_power.o > > obj-$(CONFIG_WM831X_BACKUP) += wm831x_backup.o > > obj-$(CONFIG_WM831X_POWER) += wm831x_power.o > > diff --git a/drivers/power/axp20x_usb_power.c b/drivers/power/axp20x_usb_power.c > > new file mode 100644 > > index 0000000..09f388e > > --- /dev/null > > +++ b/drivers/power/axp20x_usb_power.c > > @@ -0,0 +1,243 @@ > > +/* > > + * AXP20x PMIC USB power supply status driver > > + * > > + * Copyright (C) 2015 Hans de Goede <hdegoede@redhat.com> > > + * Copyright (C) 2014 Bruno Prémont <bonbons@linux-vserver.org> > > + * > > + * 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/platform_device.h> > > +#include <linux/power_supply.h> > > +#include <linux/regmap.h> > > +#include <linux/slab.h> > > + > > +#define DRVNAME "axp20x-usb-power-supply" > > + > > +#define AXP20X_PWR_STATUS_VBUS_PRESENT BIT(5) > > +#define AXP20X_PWR_STATUS_VBUS_USED BIT(4) > > + > > +#define AXP20X_USB_STATUS_VBUS_VALID BIT(2) > > + > > +#define AXP20X_VBUS_VHOLD_uV(b) (4000000 + (((b) >> 3) & 7) * 100000) > > +#define AXP20X_VBUS_CLIMIT_MASK 3 > > +#define AXP20X_VBUC_CLIMIT_900mA 0 > > +#define AXP20X_VBUC_CLIMIT_500mA 1 > > +#define AXP20X_VBUC_CLIMIT_100mA 2 > > +#define AXP20X_VBUC_CLIMIT_NONE 3 > > + > > +#define AXP20X_ADC_EN1_VBUS_CURR BIT(2) > > +#define AXP20X_ADC_EN1_VBUS_VOLT BIT(3) > > + > > +#define AXP20X_VBUS_MON_VBUS_VALID BIT(3) > > + > > +struct axp20x_usb_power { > > + struct regmap *regmap; > > + struct power_supply *supply; > > +}; > > + > > +static irqreturn_t axp20x_usb_power_irq(int irq, void *devid) > > +{ > > + struct axp20x_usb_power *power = devid; > > + > > + power_supply_changed(power->supply); > > + > > + return IRQ_HANDLED; > > +} > > + > > +static int axp20x_usb_power_get_property(struct power_supply *psy, > > + enum power_supply_property psp, union power_supply_propval *val) > > +{ > > + struct axp20x_usb_power *power = power_supply_get_drvdata(psy); > > + unsigned int input, v; > > + int r; > > Here and in probe function - it is strictly personal but I don't like > the 'r' variable. I got used to 'err' or 'ret' which are more > descriptive to me. 'r' may look like 'resource' for example. 'ret' might work but doesn't it imply return value for this function instead of return value from a called function? > > + > > + switch (psp) { > > + case POWER_SUPPLY_PROP_VOLTAGE_MIN: > > + r = regmap_read(power->regmap, AXP20X_VBUS_IPSOUT_MGMT, &v); > > + if (r) > > + return r; > > + > > + val->intval = AXP20X_VBUS_VHOLD_uV(v); > > + return 0; > > + case POWER_SUPPLY_PROP_VOLTAGE_NOW: > > + r = axp20x_read_variable_width(power->regmap, > > + AXP20X_VBUS_V_ADC_H, 12); > > + if (r < 0) > > + return r; > > + > > + val->intval = r * 1700; /* 1 step = 1.7 mV */ > > + return 0; > > + case POWER_SUPPLY_PROP_CURRENT_MAX: > > + r = regmap_read(power->regmap, AXP20X_VBUS_IPSOUT_MGMT, &v); > > + if (r) > > + return r; > > + > > + switch (v & AXP20X_VBUS_CLIMIT_MASK) { > > + case AXP20X_VBUC_CLIMIT_100mA: > > + val->intval = 100000; > > + break; > > + case AXP20X_VBUC_CLIMIT_500mA: > > + val->intval = 500000; > > + break; > > + case AXP20X_VBUC_CLIMIT_900mA: > > + val->intval = 900000; > > + break; > > + case AXP20X_VBUC_CLIMIT_NONE: > > + val->intval = -1; > > + break; > > + } > > + return 0; > > + case POWER_SUPPLY_PROP_CURRENT_NOW: > > + r = axp20x_read_variable_width(power->regmap, > > + AXP20X_VBUS_I_ADC_H, 12); > > + if (r < 0) > > + return r; > > + > > + val->intval = r * 375; /* 1 step = 0.375 mA */ > > + return 0; > > + default: > > + break; > > + } > > + > > + /* All the properties below need the input-status reg value */ > > + r = regmap_read(power->regmap, AXP20X_PWR_INPUT_STATUS, &input); > > + if (r) > > + return r; > > + > > + switch (psp) { > > + case POWER_SUPPLY_PROP_HEALTH: > > + if (!(input & AXP20X_PWR_STATUS_VBUS_PRESENT)) { > > + val->intval = POWER_SUPPLY_HEALTH_UNKNOWN; > > + break; > > + } > > + > > + r = regmap_read(power->regmap, AXP20X_USB_OTG_STATUS, &v); > > + if (r) > > + return r; > > + > > + if (!(v & AXP20X_USB_STATUS_VBUS_VALID)) { > > + val->intval = POWER_SUPPLY_HEALTH_UNSPEC_FAILURE; > > + break; > > + } > > + > > + val->intval = POWER_SUPPLY_HEALTH_GOOD; > > + break; > > + case POWER_SUPPLY_PROP_PRESENT: > > + val->intval = !!(input & AXP20X_PWR_STATUS_VBUS_PRESENT); > > + break; > > + case POWER_SUPPLY_PROP_ONLINE: > > + val->intval = !!(input & AXP20X_PWR_STATUS_VBUS_USED); > > + break; > > + default: > > + return -EINVAL; > > + } > > + > > + return 0; > > +} > > + > > +static enum power_supply_property axp20x_usb_power_properties[] = { > > + POWER_SUPPLY_PROP_HEALTH, > > + POWER_SUPPLY_PROP_PRESENT, > > + POWER_SUPPLY_PROP_ONLINE, > > + POWER_SUPPLY_PROP_VOLTAGE_MIN, > > + POWER_SUPPLY_PROP_VOLTAGE_NOW, > > + POWER_SUPPLY_PROP_CURRENT_MAX, > > + POWER_SUPPLY_PROP_CURRENT_NOW, > > +}; > > + > > +static const struct power_supply_desc axp20x_usb_power_desc = { > > + .name = "axp20x-usb", > > + .type = POWER_SUPPLY_TYPE_USB, > > + .properties = axp20x_usb_power_properties, > > + .num_properties = ARRAY_SIZE(axp20x_usb_power_properties), > > + .get_property = axp20x_usb_power_get_property, > > +}; > > + > > +static int axp20x_usb_power_probe(struct platform_device *pdev) > > +{ > > + struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent); > > + struct power_supply_config psy_cfg = {}; > > + struct axp20x_usb_power *power; > > + static const char * const irq_names[] = { "VBUS_PLUGIN", > > + "VBUS_REMOVAL", "VBUS_VALID", "VBUS_NOT_VALID" }; > > + int i, irq, r; > > + > > + if (!of_device_is_available(pdev->dev.of_node)) > > + return -ENODEV; > > + > > + power = devm_kzalloc(&pdev->dev, sizeof(*power), GFP_KERNEL); > > + if (!power) > > + return -ENOMEM; > > + > > + power->regmap = axp20x->regmap; > > + > > + /* Enable vbus valid checking */ > > + r = regmap_update_bits(power->regmap, AXP20X_VBUS_MON, > > + AXP20X_VBUS_MON_VBUS_VALID, AXP20X_VBUS_MON_VBUS_VALID); > > + if (r) > > + return r; > > + > > + /* Enable vbus voltage and current measurement */ > > + r = regmap_update_bits(power->regmap, AXP20X_ADC_EN1, > > + AXP20X_ADC_EN1_VBUS_CURR | AXP20X_ADC_EN1_VBUS_VOLT, > > + AXP20X_ADC_EN1_VBUS_CURR | AXP20X_ADC_EN1_VBUS_VOLT); > > + if (r) > > + return r; > > + > > + psy_cfg.of_node = pdev->dev.of_node; > > + psy_cfg.drv_data = power; > > + > > + power->supply = devm_power_supply_register(&pdev->dev, > > + &axp20x_usb_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; i < ARRAY_SIZE(irq_names); 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); > > + r = devm_request_any_context_irq(&pdev->dev, irq, > > + axp20x_usb_power_irq, 0, DRVNAME, power); > > + if (r < 0) > > + dev_warn(&pdev->dev, "Error requesting %s IRQ: %d\n", > > + irq_names[i], r); > > + } > > + > > + return 0; > > +} > > + > > +static const struct of_device_id axp20x_usb_power_match[] = { > > + { .compatible = "x-powers,axp202-usb-power-supply" }, > > + { } > > +}; > > +MODULE_DEVICE_TABLE(of, axp20x_usb_power_match); > > + > > +static struct platform_driver axp20x_usb_power_driver = { > > + .probe = axp20x_usb_power_probe, > > + .driver = { > > + .name = DRVNAME, > > + .of_match_table = axp20x_usb_power_match, > > + }, > > +}; > > + > > +module_platform_driver(axp20x_usb_power_driver); > > + > > +MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com>"); > > +MODULE_DESCRIPTION("AXP20x PMIC USB power supply status driver"); > > +MODULE_LICENSE("GPL"); > > diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h > > index 6d8b39a..8ec996e 100644 > > --- a/include/linux/mfd/axp20x.h > > +++ b/include/linux/mfd/axp20x.h > > @@ -11,6 +11,8 @@ > > #ifndef __LINUX_MFD_AXP20X_H > > #define __LINUX_MFD_AXP20X_H > > > > +#include <linux/regmap.h> > > + > > enum { > > AXP202_ID = 0, > > AXP209_ID, > > @@ -372,4 +374,26 @@ struct axp288_extcon_pdata { > > struct gpio_desc *gpio_mux_cntl; > > }; > > > > +/* generic helper function for reading 9-16 bit wide regs */ > > +static inline int axp20x_read_variable_width(struct regmap *regmap, > > + unsigned int reg, unsigned int width) > > +{ > > + unsigned int reg_val, result; > > + int err; > > + > > + err = regmap_read(regmap, reg, ®_val); > > + if (err) > > + return err; > > + > > + result = reg_val << (width - 8); > > + > > + err = regmap_read(regmap, reg + 1, ®_val); > > + if (err) > > + return err; > > + > > + result |= reg_val; > > + > > + return result; > > +} > > + > > #endif /* __LINUX_MFD_AXP20X_H */ > > Is this static inline function used outside of > drivers/power/axp20x_usb_power.c? Looks like it isn't, but maybe you > have a plan for it? If not, then it shouldn't be in header file. See comments for first iteration of this patch: | | This is only used twice and only in one file. | | | | Do you know of any other uses of this call that will be upstreamed | | shortly? | | Yes I plan to write drivers for the other 3 power_supply class | cells (ac-power, battery-charger, rtc-bat-charger) in the axp209, | and most of those need the same helper which I why I put it here. > Best regards, > Krzysztof Best regards, Bruno -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
W dniu 28.06.2015 o 05:25, Bruno Prémont pisze: > On Sat, 27 Jun 2015 14:28:40 Krzysztof Kozlowski wrote: >> W dniu 26.06.2015 o 19:59, Hans de Goede pisze: >>> This adds a driver for the usb power_supply bits of the axp20x PMICs. >>> >>> I initially started writing my own driver, before coming aware of >>> Bruno Prémont's excellent earlier RFC with a driver for this. >>> >>> My driver was lacking CURRENT_MAX and VOLTAGE_MIN support Bruno's >>> drvier has, so I've copied the code for those from his driver. >>> >>> Note that the AC-power-supply and battery charger bits will need separate >>> drivers. Each one needs its own devictree child-node so that other >>> devicetree nodes can reference the right power-supply, and thus each one >>> will get its own mfd-cell / platform_device and platform-driver. >>> >>> Cc: Bruno Prémont <bonbons@linux-vserver.org> >>> Acked-by: Lee Jones <lee.jones@linaro.org> >>> Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org> >>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >>> --- >>> Changes in v2: >>> -Split out the dt-bindings documentation into a separate patch >>> -Renamed axp20x_read_16bit to axp20x_read_variable_width >>> -Use better local variable names inside axp20x_read_variable_width >>> Changes in v3: >>> -Add Bruno's S-o-b >> >> Hi, >> >> Two minor comments below, the driver itself looks good. >> >> >>> --- >>> drivers/power/Kconfig | 7 ++ >>> drivers/power/Makefile | 1 + >>> drivers/power/axp20x_usb_power.c | 243 +++++++++++++++++++++++++++++++++++++++ >>> include/linux/mfd/axp20x.h | 24 ++++ >>> 4 files changed, 275 insertions(+) >>> create mode 100644 drivers/power/axp20x_usb_power.c >>> >>> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig >>> index 4091fb0..1fee60c 100644 >>> --- a/drivers/power/Kconfig >>> +++ b/drivers/power/Kconfig >>> @@ -439,6 +439,13 @@ config BATTERY_RT5033 >>> The fuelgauge calculates and determines the battery state of charge >>> according to battery open circuit voltage. >>> >>> +config AXP20X_POWER >>> + tristate "AXP20x power supply driver" >>> + depends on MFD_AXP20X >>> + help >>> + This driver provides support for the power supply features of >>> + AXP20x PMIC. >>> + >>> source "drivers/power/reset/Kconfig" >>> >>> endif # POWER_SUPPLY >>> diff --git a/drivers/power/Makefile b/drivers/power/Makefile >>> index b7b0181..ae0f27d 100644 >>> --- a/drivers/power/Makefile >>> +++ b/drivers/power/Makefile >>> @@ -9,6 +9,7 @@ obj-$(CONFIG_GENERIC_ADC_BATTERY) += generic-adc-battery.o >>> >>> obj-$(CONFIG_PDA_POWER) += pda_power.o >>> obj-$(CONFIG_APM_POWER) += apm_power.o >>> +obj-$(CONFIG_AXP20X_POWER) += axp20x_usb_power.o >>> obj-$(CONFIG_MAX8925_POWER) += max8925_power.o >>> obj-$(CONFIG_WM831X_BACKUP) += wm831x_backup.o >>> obj-$(CONFIG_WM831X_POWER) += wm831x_power.o >>> diff --git a/drivers/power/axp20x_usb_power.c b/drivers/power/axp20x_usb_power.c >>> new file mode 100644 >>> index 0000000..09f388e >>> --- /dev/null >>> +++ b/drivers/power/axp20x_usb_power.c >>> @@ -0,0 +1,243 @@ >>> +/* >>> + * AXP20x PMIC USB power supply status driver >>> + * >>> + * Copyright (C) 2015 Hans de Goede <hdegoede@redhat.com> >>> + * Copyright (C) 2014 Bruno Prémont <bonbons@linux-vserver.org> >>> + * >>> + * 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/platform_device.h> >>> +#include <linux/power_supply.h> >>> +#include <linux/regmap.h> >>> +#include <linux/slab.h> >>> + >>> +#define DRVNAME "axp20x-usb-power-supply" >>> + >>> +#define AXP20X_PWR_STATUS_VBUS_PRESENT BIT(5) >>> +#define AXP20X_PWR_STATUS_VBUS_USED BIT(4) >>> + >>> +#define AXP20X_USB_STATUS_VBUS_VALID BIT(2) >>> + >>> +#define AXP20X_VBUS_VHOLD_uV(b) (4000000 + (((b) >> 3) & 7) * 100000) >>> +#define AXP20X_VBUS_CLIMIT_MASK 3 >>> +#define AXP20X_VBUC_CLIMIT_900mA 0 >>> +#define AXP20X_VBUC_CLIMIT_500mA 1 >>> +#define AXP20X_VBUC_CLIMIT_100mA 2 >>> +#define AXP20X_VBUC_CLIMIT_NONE 3 >>> + >>> +#define AXP20X_ADC_EN1_VBUS_CURR BIT(2) >>> +#define AXP20X_ADC_EN1_VBUS_VOLT BIT(3) >>> + >>> +#define AXP20X_VBUS_MON_VBUS_VALID BIT(3) >>> + >>> +struct axp20x_usb_power { >>> + struct regmap *regmap; >>> + struct power_supply *supply; >>> +}; >>> + >>> +static irqreturn_t axp20x_usb_power_irq(int irq, void *devid) >>> +{ >>> + struct axp20x_usb_power *power = devid; >>> + >>> + power_supply_changed(power->supply); >>> + >>> + return IRQ_HANDLED; >>> +} >>> + >>> +static int axp20x_usb_power_get_property(struct power_supply *psy, >>> + enum power_supply_property psp, union power_supply_propval *val) >>> +{ >>> + struct axp20x_usb_power *power = power_supply_get_drvdata(psy); >>> + unsigned int input, v; >>> + int r; >> >> Here and in probe function - it is strictly personal but I don't like >> the 'r' variable. I got used to 'err' or 'ret' which are more >> descriptive to me. 'r' may look like 'resource' for example. > > 'ret' might work but doesn't it imply return value for this function > instead of return value from a called function? A little, but in this case it is indeed a return value from called function. 'err' would fine as well. > >>> + >>> + switch (psp) { >>> + case POWER_SUPPLY_PROP_VOLTAGE_MIN: >>> + r = regmap_read(power->regmap, AXP20X_VBUS_IPSOUT_MGMT, &v); >>> + if (r) >>> + return r; >>> + >>> + val->intval = AXP20X_VBUS_VHOLD_uV(v); >>> + return 0; >>> + case POWER_SUPPLY_PROP_VOLTAGE_NOW: >>> + r = axp20x_read_variable_width(power->regmap, >>> + AXP20X_VBUS_V_ADC_H, 12); >>> + if (r < 0) >>> + return r; >>> + >>> + val->intval = r * 1700; /* 1 step = 1.7 mV */ >>> + return 0; >>> + case POWER_SUPPLY_PROP_CURRENT_MAX: >>> + r = regmap_read(power->regmap, AXP20X_VBUS_IPSOUT_MGMT, &v); >>> + if (r) >>> + return r; >>> + >>> + switch (v & AXP20X_VBUS_CLIMIT_MASK) { >>> + case AXP20X_VBUC_CLIMIT_100mA: >>> + val->intval = 100000; >>> + break; >>> + case AXP20X_VBUC_CLIMIT_500mA: >>> + val->intval = 500000; >>> + break; >>> + case AXP20X_VBUC_CLIMIT_900mA: >>> + val->intval = 900000; >>> + break; >>> + case AXP20X_VBUC_CLIMIT_NONE: >>> + val->intval = -1; >>> + break; >>> + } >>> + return 0; >>> + case POWER_SUPPLY_PROP_CURRENT_NOW: >>> + r = axp20x_read_variable_width(power->regmap, >>> + AXP20X_VBUS_I_ADC_H, 12); >>> + if (r < 0) >>> + return r; >>> + >>> + val->intval = r * 375; /* 1 step = 0.375 mA */ >>> + return 0; >>> + default: >>> + break; >>> + } >>> + >>> + /* All the properties below need the input-status reg value */ >>> + r = regmap_read(power->regmap, AXP20X_PWR_INPUT_STATUS, &input); >>> + if (r) >>> + return r; >>> + >>> + switch (psp) { >>> + case POWER_SUPPLY_PROP_HEALTH: >>> + if (!(input & AXP20X_PWR_STATUS_VBUS_PRESENT)) { >>> + val->intval = POWER_SUPPLY_HEALTH_UNKNOWN; >>> + break; >>> + } >>> + >>> + r = regmap_read(power->regmap, AXP20X_USB_OTG_STATUS, &v); >>> + if (r) >>> + return r; >>> + >>> + if (!(v & AXP20X_USB_STATUS_VBUS_VALID)) { >>> + val->intval = POWER_SUPPLY_HEALTH_UNSPEC_FAILURE; >>> + break; >>> + } >>> + >>> + val->intval = POWER_SUPPLY_HEALTH_GOOD; >>> + break; >>> + case POWER_SUPPLY_PROP_PRESENT: >>> + val->intval = !!(input & AXP20X_PWR_STATUS_VBUS_PRESENT); >>> + break; >>> + case POWER_SUPPLY_PROP_ONLINE: >>> + val->intval = !!(input & AXP20X_PWR_STATUS_VBUS_USED); >>> + break; >>> + default: >>> + return -EINVAL; >>> + } >>> + >>> + return 0; >>> +} >>> + >>> +static enum power_supply_property axp20x_usb_power_properties[] = { >>> + POWER_SUPPLY_PROP_HEALTH, >>> + POWER_SUPPLY_PROP_PRESENT, >>> + POWER_SUPPLY_PROP_ONLINE, >>> + POWER_SUPPLY_PROP_VOLTAGE_MIN, >>> + POWER_SUPPLY_PROP_VOLTAGE_NOW, >>> + POWER_SUPPLY_PROP_CURRENT_MAX, >>> + POWER_SUPPLY_PROP_CURRENT_NOW, >>> +}; >>> + >>> +static const struct power_supply_desc axp20x_usb_power_desc = { >>> + .name = "axp20x-usb", >>> + .type = POWER_SUPPLY_TYPE_USB, >>> + .properties = axp20x_usb_power_properties, >>> + .num_properties = ARRAY_SIZE(axp20x_usb_power_properties), >>> + .get_property = axp20x_usb_power_get_property, >>> +}; >>> + >>> +static int axp20x_usb_power_probe(struct platform_device *pdev) >>> +{ >>> + struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent); >>> + struct power_supply_config psy_cfg = {}; >>> + struct axp20x_usb_power *power; >>> + static const char * const irq_names[] = { "VBUS_PLUGIN", >>> + "VBUS_REMOVAL", "VBUS_VALID", "VBUS_NOT_VALID" }; >>> + int i, irq, r; >>> + >>> + if (!of_device_is_available(pdev->dev.of_node)) >>> + return -ENODEV; >>> + >>> + power = devm_kzalloc(&pdev->dev, sizeof(*power), GFP_KERNEL); >>> + if (!power) >>> + return -ENOMEM; >>> + >>> + power->regmap = axp20x->regmap; >>> + >>> + /* Enable vbus valid checking */ >>> + r = regmap_update_bits(power->regmap, AXP20X_VBUS_MON, >>> + AXP20X_VBUS_MON_VBUS_VALID, AXP20X_VBUS_MON_VBUS_VALID); >>> + if (r) >>> + return r; >>> + >>> + /* Enable vbus voltage and current measurement */ >>> + r = regmap_update_bits(power->regmap, AXP20X_ADC_EN1, >>> + AXP20X_ADC_EN1_VBUS_CURR | AXP20X_ADC_EN1_VBUS_VOLT, >>> + AXP20X_ADC_EN1_VBUS_CURR | AXP20X_ADC_EN1_VBUS_VOLT); >>> + if (r) >>> + return r; >>> + >>> + psy_cfg.of_node = pdev->dev.of_node; >>> + psy_cfg.drv_data = power; >>> + >>> + power->supply = devm_power_supply_register(&pdev->dev, >>> + &axp20x_usb_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; i < ARRAY_SIZE(irq_names); 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); >>> + r = devm_request_any_context_irq(&pdev->dev, irq, >>> + axp20x_usb_power_irq, 0, DRVNAME, power); >>> + if (r < 0) >>> + dev_warn(&pdev->dev, "Error requesting %s IRQ: %d\n", >>> + irq_names[i], r); >>> + } >>> + >>> + return 0; >>> +} >>> + >>> +static const struct of_device_id axp20x_usb_power_match[] = { >>> + { .compatible = "x-powers,axp202-usb-power-supply" }, >>> + { } >>> +}; >>> +MODULE_DEVICE_TABLE(of, axp20x_usb_power_match); >>> + >>> +static struct platform_driver axp20x_usb_power_driver = { >>> + .probe = axp20x_usb_power_probe, >>> + .driver = { >>> + .name = DRVNAME, >>> + .of_match_table = axp20x_usb_power_match, >>> + }, >>> +}; >>> + >>> +module_platform_driver(axp20x_usb_power_driver); >>> + >>> +MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com>"); >>> +MODULE_DESCRIPTION("AXP20x PMIC USB power supply status driver"); >>> +MODULE_LICENSE("GPL"); >>> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h >>> index 6d8b39a..8ec996e 100644 >>> --- a/include/linux/mfd/axp20x.h >>> +++ b/include/linux/mfd/axp20x.h >>> @@ -11,6 +11,8 @@ >>> #ifndef __LINUX_MFD_AXP20X_H >>> #define __LINUX_MFD_AXP20X_H >>> >>> +#include <linux/regmap.h> >>> + >>> enum { >>> AXP202_ID = 0, >>> AXP209_ID, >>> @@ -372,4 +374,26 @@ struct axp288_extcon_pdata { >>> struct gpio_desc *gpio_mux_cntl; >>> }; >>> >>> +/* generic helper function for reading 9-16 bit wide regs */ >>> +static inline int axp20x_read_variable_width(struct regmap *regmap, >>> + unsigned int reg, unsigned int width) >>> +{ >>> + unsigned int reg_val, result; >>> + int err; >>> + >>> + err = regmap_read(regmap, reg, ®_val); >>> + if (err) >>> + return err; >>> + >>> + result = reg_val << (width - 8); >>> + >>> + err = regmap_read(regmap, reg + 1, ®_val); >>> + if (err) >>> + return err; >>> + >>> + result |= reg_val; >>> + >>> + return result; >>> +} >>> + >>> #endif /* __LINUX_MFD_AXP20X_H */ >> >> Is this static inline function used outside of >> drivers/power/axp20x_usb_power.c? Looks like it isn't, but maybe you >> have a plan for it? If not, then it shouldn't be in header file. > > See comments for first iteration of this patch: > | | This is only used twice and only in one file. > | | > | | Do you know of any other uses of this call that will be upstreamed > | | shortly? > | > | Yes I plan to write drivers for the other 3 power_supply class > | cells (ac-power, battery-charger, rtc-bat-charger) in the axp209, > | and most of those need the same helper which I why I put it here. Thanks for explanation. Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jun 26, 2015 at 12:59:15PM +0200, Hans de Goede wrote: > From: Bruno Prémont <bonbons@linux-vserver.org> > > Add an extra set of registers which is necessary tu support the PMICs > battery charger function, and mark registers which contain status bits, > gpio status, and adc readings as volatile. > > Cc: Bruno Prémont <bonbons@linux-vserver.org> > Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com> Thanks! Maxime
On Fri, Jun 26, 2015 at 12:59:16PM +0200, Hans de Goede wrote: > Add a cell for the usb power_supply part of the axp20x PMICs. > > Note that this cell is only for the usb power_supply part and not the > ac-power / battery-charger / rtc-backup-bat-charger bits. > > Depending on the board each of those must be enabled / disabled separately > in devicetree as most boards do not use all 4. So in dt each one needs its > own child-node of the axp20x node. Another reason for using separate child > nodes for each is so that other devicetree nodes can have a power-supply > property with a phandle referencing a node representing a single > power-supply. > > The decision to use a separate devicetree node for each is reflected on > the kernel side by each getting its own mfd-cell / platform_device and > platform-driver. > > Note this commit also makes some whitespace changes to the intialization > of existing cells in axp20x_cells, these are pure whitespace changes, > functionally nothing changes. > > Cc: Bruno Prémont <bonbons@linux-vserver.org> > Acked-by: Lee Jones <lee.jones@linaro.org> > Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com> Thanks! Maxime
Hi, On Fri, Jun 26, 2015 at 12:59:17PM +0200, Hans de Goede wrote: > This adds a driver for the usb power_supply bits of the axp20x PMICs. > > I initially started writing my own driver, before coming aware of > Bruno Prémont's excellent earlier RFC with a driver for this. > > My driver was lacking CURRENT_MAX and VOLTAGE_MIN support Bruno's > drvier has, so I've copied the code for those from his driver. > > Note that the AC-power-supply and battery charger bits will need separate > drivers. Each one needs its own devictree child-node so that other > devicetree nodes can reference the right power-supply, and thus each one > will get its own mfd-cell / platform_device and platform-driver. > > Cc: Bruno Prémont <bonbons@linux-vserver.org> > Acked-by: Lee Jones <lee.jones@linaro.org> > Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > Changes in v2: > -Split out the dt-bindings documentation into a separate patch > -Renamed axp20x_read_16bit to axp20x_read_variable_width > -Use better local variable names inside axp20x_read_variable_width > Changes in v3: > -Add Bruno's S-o-b > --- > drivers/power/Kconfig | 7 ++ > drivers/power/Makefile | 1 + > drivers/power/axp20x_usb_power.c | 243 +++++++++++++++++++++++++++++++++++++++ > include/linux/mfd/axp20x.h | 24 ++++ > 4 files changed, 275 insertions(+) > create mode 100644 drivers/power/axp20x_usb_power.c > > diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig > index 4091fb0..1fee60c 100644 > --- a/drivers/power/Kconfig > +++ b/drivers/power/Kconfig > @@ -439,6 +439,13 @@ config BATTERY_RT5033 > The fuelgauge calculates and determines the battery state of charge > according to battery open circuit voltage. > > +config AXP20X_POWER > + tristate "AXP20x power supply driver" > + depends on MFD_AXP20X > + help > + This driver provides support for the power supply features of > + AXP20x PMIC. > + > source "drivers/power/reset/Kconfig" > > endif # POWER_SUPPLY > diff --git a/drivers/power/Makefile b/drivers/power/Makefile > index b7b0181..ae0f27d 100644 > --- a/drivers/power/Makefile > +++ b/drivers/power/Makefile > @@ -9,6 +9,7 @@ obj-$(CONFIG_GENERIC_ADC_BATTERY) += generic-adc-battery.o > > obj-$(CONFIG_PDA_POWER) += pda_power.o > obj-$(CONFIG_APM_POWER) += apm_power.o > +obj-$(CONFIG_AXP20X_POWER) += axp20x_usb_power.o > obj-$(CONFIG_MAX8925_POWER) += max8925_power.o > obj-$(CONFIG_WM831X_BACKUP) += wm831x_backup.o > obj-$(CONFIG_WM831X_POWER) += wm831x_power.o > diff --git a/drivers/power/axp20x_usb_power.c b/drivers/power/axp20x_usb_power.c > new file mode 100644 > index 0000000..09f388e > --- /dev/null > +++ b/drivers/power/axp20x_usb_power.c > @@ -0,0 +1,243 @@ > +/* > + * AXP20x PMIC USB power supply status driver > + * > + * Copyright (C) 2015 Hans de Goede <hdegoede@redhat.com> > + * Copyright (C) 2014 Bruno Prémont <bonbons@linux-vserver.org> > + * > + * 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/platform_device.h> > +#include <linux/power_supply.h> > +#include <linux/regmap.h> > +#include <linux/slab.h> > + > +#define DRVNAME "axp20x-usb-power-supply" > + > +#define AXP20X_PWR_STATUS_VBUS_PRESENT BIT(5) > +#define AXP20X_PWR_STATUS_VBUS_USED BIT(4) > + > +#define AXP20X_USB_STATUS_VBUS_VALID BIT(2) > + > +#define AXP20X_VBUS_VHOLD_uV(b) (4000000 + (((b) >> 3) & 7) * 100000) > +#define AXP20X_VBUS_CLIMIT_MASK 3 > +#define AXP20X_VBUC_CLIMIT_900mA 0 > +#define AXP20X_VBUC_CLIMIT_500mA 1 > +#define AXP20X_VBUC_CLIMIT_100mA 2 > +#define AXP20X_VBUC_CLIMIT_NONE 3 > + > +#define AXP20X_ADC_EN1_VBUS_CURR BIT(2) > +#define AXP20X_ADC_EN1_VBUS_VOLT BIT(3) > + > +#define AXP20X_VBUS_MON_VBUS_VALID BIT(3) > + > +struct axp20x_usb_power { > + struct regmap *regmap; > + struct power_supply *supply; > +}; > + > +static irqreturn_t axp20x_usb_power_irq(int irq, void *devid) > +{ > + struct axp20x_usb_power *power = devid; > + > + power_supply_changed(power->supply); > + > + return IRQ_HANDLED; > +} > + > +static int axp20x_usb_power_get_property(struct power_supply *psy, > + enum power_supply_property psp, union power_supply_propval *val) > +{ > + struct axp20x_usb_power *power = power_supply_get_drvdata(psy); > + unsigned int input, v; > + int r; > + > + switch (psp) { > + case POWER_SUPPLY_PROP_VOLTAGE_MIN: > + r = regmap_read(power->regmap, AXP20X_VBUS_IPSOUT_MGMT, &v); > + if (r) > + return r; > + > + val->intval = AXP20X_VBUS_VHOLD_uV(v); > + return 0; > + case POWER_SUPPLY_PROP_VOLTAGE_NOW: > + r = axp20x_read_variable_width(power->regmap, > + AXP20X_VBUS_V_ADC_H, 12); > + if (r < 0) > + return r; > + > + val->intval = r * 1700; /* 1 step = 1.7 mV */ Eventually, as we will have more users (the various power supplies, the TS pin?), we should have an IIO driver for the internal ADC. Do you know how close it is from the AXP288's ? > + return 0; > + case POWER_SUPPLY_PROP_CURRENT_MAX: > + r = regmap_read(power->regmap, AXP20X_VBUS_IPSOUT_MGMT, &v); > + if (r) > + return r; > + > + switch (v & AXP20X_VBUS_CLIMIT_MASK) { > + case AXP20X_VBUC_CLIMIT_100mA: > + val->intval = 100000; > + break; > + case AXP20X_VBUC_CLIMIT_500mA: > + val->intval = 500000; > + break; > + case AXP20X_VBUC_CLIMIT_900mA: > + val->intval = 900000; > + break; > + case AXP20X_VBUC_CLIMIT_NONE: > + val->intval = -1; > + break; > + } > + return 0; > + case POWER_SUPPLY_PROP_CURRENT_NOW: > + r = axp20x_read_variable_width(power->regmap, > + AXP20X_VBUS_I_ADC_H, 12); > + if (r < 0) > + return r; > + > + val->intval = r * 375; /* 1 step = 0.375 mA */ And here too. > + return 0; > + default: > + break; > + } > + > + /* All the properties below need the input-status reg value */ > + r = regmap_read(power->regmap, AXP20X_PWR_INPUT_STATUS, &input); > + if (r) > + return r; > + > + switch (psp) { > + case POWER_SUPPLY_PROP_HEALTH: > + if (!(input & AXP20X_PWR_STATUS_VBUS_PRESENT)) { > + val->intval = POWER_SUPPLY_HEALTH_UNKNOWN; > + break; > + } > + > + r = regmap_read(power->regmap, AXP20X_USB_OTG_STATUS, &v); > + if (r) > + return r; > + > + if (!(v & AXP20X_USB_STATUS_VBUS_VALID)) { > + val->intval = POWER_SUPPLY_HEALTH_UNSPEC_FAILURE; > + break; > + } > + > + val->intval = POWER_SUPPLY_HEALTH_GOOD; > + break; > + case POWER_SUPPLY_PROP_PRESENT: > + val->intval = !!(input & AXP20X_PWR_STATUS_VBUS_PRESENT); > + break; > + case POWER_SUPPLY_PROP_ONLINE: > + val->intval = !!(input & AXP20X_PWR_STATUS_VBUS_USED); > + break; > + default: > + return -EINVAL; > + } > + > + return 0; > +} > + > +static enum power_supply_property axp20x_usb_power_properties[] = { > + POWER_SUPPLY_PROP_HEALTH, > + POWER_SUPPLY_PROP_PRESENT, > + POWER_SUPPLY_PROP_ONLINE, > + POWER_SUPPLY_PROP_VOLTAGE_MIN, > + POWER_SUPPLY_PROP_VOLTAGE_NOW, > + POWER_SUPPLY_PROP_CURRENT_MAX, > + POWER_SUPPLY_PROP_CURRENT_NOW, > +}; > + > +static const struct power_supply_desc axp20x_usb_power_desc = { > + .name = "axp20x-usb", > + .type = POWER_SUPPLY_TYPE_USB, > + .properties = axp20x_usb_power_properties, > + .num_properties = ARRAY_SIZE(axp20x_usb_power_properties), > + .get_property = axp20x_usb_power_get_property, > +}; > + > +static int axp20x_usb_power_probe(struct platform_device *pdev) > +{ > + struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent); > + struct power_supply_config psy_cfg = {}; > + struct axp20x_usb_power *power; > + static const char * const irq_names[] = { "VBUS_PLUGIN", > + "VBUS_REMOVAL", "VBUS_VALID", "VBUS_NOT_VALID" }; > + int i, irq, r; > + > + if (!of_device_is_available(pdev->dev.of_node)) > + return -ENODEV; That looks odd. Is it even going to be probed in the first place? > + > + power = devm_kzalloc(&pdev->dev, sizeof(*power), GFP_KERNEL); > + if (!power) > + return -ENOMEM; > + > + power->regmap = axp20x->regmap; It should probably be wise to test that the axp20x pointer is not NULL. > + > + /* Enable vbus valid checking */ > + r = regmap_update_bits(power->regmap, AXP20X_VBUS_MON, > + AXP20X_VBUS_MON_VBUS_VALID, AXP20X_VBUS_MON_VBUS_VALID); > + if (r) > + return r; > + > + /* Enable vbus voltage and current measurement */ > + r = regmap_update_bits(power->regmap, AXP20X_ADC_EN1, > + AXP20X_ADC_EN1_VBUS_CURR | AXP20X_ADC_EN1_VBUS_VOLT, > + AXP20X_ADC_EN1_VBUS_CURR | AXP20X_ADC_EN1_VBUS_VOLT); > + if (r) > + return r; > + > + psy_cfg.of_node = pdev->dev.of_node; > + psy_cfg.drv_data = power; > + > + power->supply = devm_power_supply_register(&pdev->dev, > + &axp20x_usb_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; i < ARRAY_SIZE(irq_names); 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); > + r = devm_request_any_context_irq(&pdev->dev, irq, > + axp20x_usb_power_irq, 0, DRVNAME, power); > + if (r < 0) > + dev_warn(&pdev->dev, "Error requesting %s IRQ: %d\n", > + irq_names[i], r); > + } > + > + return 0; > +} > + > +static const struct of_device_id axp20x_usb_power_match[] = { > + { .compatible = "x-powers,axp202-usb-power-supply" }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, axp20x_usb_power_match); > + > +static struct platform_driver axp20x_usb_power_driver = { > + .probe = axp20x_usb_power_probe, > + .driver = { > + .name = DRVNAME, > + .of_match_table = axp20x_usb_power_match, > + }, > +}; > + > +module_platform_driver(axp20x_usb_power_driver); > + > +MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com>"); > +MODULE_DESCRIPTION("AXP20x PMIC USB power supply status driver"); > +MODULE_LICENSE("GPL"); > diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h > index 6d8b39a..8ec996e 100644 > --- a/include/linux/mfd/axp20x.h > +++ b/include/linux/mfd/axp20x.h > @@ -11,6 +11,8 @@ > #ifndef __LINUX_MFD_AXP20X_H > #define __LINUX_MFD_AXP20X_H > > +#include <linux/regmap.h> > + > enum { > AXP202_ID = 0, > AXP209_ID, > @@ -372,4 +374,26 @@ struct axp288_extcon_pdata { > struct gpio_desc *gpio_mux_cntl; > }; > > +/* generic helper function for reading 9-16 bit wide regs */ > +static inline int axp20x_read_variable_width(struct regmap *regmap, > + unsigned int reg, unsigned int width) > +{ > + unsigned int reg_val, result; > + int err; > + > + err = regmap_read(regmap, reg, ®_val); > + if (err) > + return err; > + > + result = reg_val << (width - 8); > + > + err = regmap_read(regmap, reg + 1, ®_val); > + if (err) > + return err; > + > + result |= reg_val; > + > + return result; > +} > + > #endif /* __LINUX_MFD_AXP20X_H */ > -- > 2.4.3 > Looks good otherwise, thanks! Maxime
Hi, On 06-07-15 12:40, Maxime Ripard wrote: > Hi, > > On Fri, Jun 26, 2015 at 12:59:17PM +0200, Hans de Goede wrote: >> This adds a driver for the usb power_supply bits of the axp20x PMICs. >> >> I initially started writing my own driver, before coming aware of >> Bruno Prémont's excellent earlier RFC with a driver for this. >> >> My driver was lacking CURRENT_MAX and VOLTAGE_MIN support Bruno's >> drvier has, so I've copied the code for those from his driver. >> >> Note that the AC-power-supply and battery charger bits will need separate >> drivers. Each one needs its own devictree child-node so that other >> devicetree nodes can reference the right power-supply, and thus each one >> will get its own mfd-cell / platform_device and platform-driver. >> >> Cc: Bruno Prémont <bonbons@linux-vserver.org> >> Acked-by: Lee Jones <lee.jones@linaro.org> >> Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> Changes in v2: >> -Split out the dt-bindings documentation into a separate patch >> -Renamed axp20x_read_16bit to axp20x_read_variable_width >> -Use better local variable names inside axp20x_read_variable_width >> Changes in v3: >> -Add Bruno's S-o-b >> --- >> drivers/power/Kconfig | 7 ++ >> drivers/power/Makefile | 1 + >> drivers/power/axp20x_usb_power.c | 243 +++++++++++++++++++++++++++++++++++++++ >> include/linux/mfd/axp20x.h | 24 ++++ >> 4 files changed, 275 insertions(+) >> create mode 100644 drivers/power/axp20x_usb_power.c >> >> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig >> index 4091fb0..1fee60c 100644 >> --- a/drivers/power/Kconfig >> +++ b/drivers/power/Kconfig >> @@ -439,6 +439,13 @@ config BATTERY_RT5033 >> The fuelgauge calculates and determines the battery state of charge >> according to battery open circuit voltage. >> >> +config AXP20X_POWER >> + tristate "AXP20x power supply driver" >> + depends on MFD_AXP20X >> + help >> + This driver provides support for the power supply features of >> + AXP20x PMIC. >> + >> source "drivers/power/reset/Kconfig" >> >> endif # POWER_SUPPLY >> diff --git a/drivers/power/Makefile b/drivers/power/Makefile >> index b7b0181..ae0f27d 100644 >> --- a/drivers/power/Makefile >> +++ b/drivers/power/Makefile >> @@ -9,6 +9,7 @@ obj-$(CONFIG_GENERIC_ADC_BATTERY) += generic-adc-battery.o >> >> obj-$(CONFIG_PDA_POWER) += pda_power.o >> obj-$(CONFIG_APM_POWER) += apm_power.o >> +obj-$(CONFIG_AXP20X_POWER) += axp20x_usb_power.o >> obj-$(CONFIG_MAX8925_POWER) += max8925_power.o >> obj-$(CONFIG_WM831X_BACKUP) += wm831x_backup.o >> obj-$(CONFIG_WM831X_POWER) += wm831x_power.o >> diff --git a/drivers/power/axp20x_usb_power.c b/drivers/power/axp20x_usb_power.c >> new file mode 100644 >> index 0000000..09f388e >> --- /dev/null >> +++ b/drivers/power/axp20x_usb_power.c >> @@ -0,0 +1,243 @@ >> +/* >> + * AXP20x PMIC USB power supply status driver >> + * >> + * Copyright (C) 2015 Hans de Goede <hdegoede@redhat.com> >> + * Copyright (C) 2014 Bruno Prémont <bonbons@linux-vserver.org> >> + * >> + * 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/platform_device.h> >> +#include <linux/power_supply.h> >> +#include <linux/regmap.h> >> +#include <linux/slab.h> >> + >> +#define DRVNAME "axp20x-usb-power-supply" >> + >> +#define AXP20X_PWR_STATUS_VBUS_PRESENT BIT(5) >> +#define AXP20X_PWR_STATUS_VBUS_USED BIT(4) >> + >> +#define AXP20X_USB_STATUS_VBUS_VALID BIT(2) >> + >> +#define AXP20X_VBUS_VHOLD_uV(b) (4000000 + (((b) >> 3) & 7) * 100000) >> +#define AXP20X_VBUS_CLIMIT_MASK 3 >> +#define AXP20X_VBUC_CLIMIT_900mA 0 >> +#define AXP20X_VBUC_CLIMIT_500mA 1 >> +#define AXP20X_VBUC_CLIMIT_100mA 2 >> +#define AXP20X_VBUC_CLIMIT_NONE 3 >> + >> +#define AXP20X_ADC_EN1_VBUS_CURR BIT(2) >> +#define AXP20X_ADC_EN1_VBUS_VOLT BIT(3) >> + >> +#define AXP20X_VBUS_MON_VBUS_VALID BIT(3) >> + >> +struct axp20x_usb_power { >> + struct regmap *regmap; >> + struct power_supply *supply; >> +}; >> + >> +static irqreturn_t axp20x_usb_power_irq(int irq, void *devid) >> +{ >> + struct axp20x_usb_power *power = devid; >> + >> + power_supply_changed(power->supply); >> + >> + return IRQ_HANDLED; >> +} >> + >> +static int axp20x_usb_power_get_property(struct power_supply *psy, >> + enum power_supply_property psp, union power_supply_propval *val) >> +{ >> + struct axp20x_usb_power *power = power_supply_get_drvdata(psy); >> + unsigned int input, v; >> + int r; >> + >> + switch (psp) { >> + case POWER_SUPPLY_PROP_VOLTAGE_MIN: >> + r = regmap_read(power->regmap, AXP20X_VBUS_IPSOUT_MGMT, &v); >> + if (r) >> + return r; >> + >> + val->intval = AXP20X_VBUS_VHOLD_uV(v); >> + return 0; >> + case POWER_SUPPLY_PROP_VOLTAGE_NOW: >> + r = axp20x_read_variable_width(power->regmap, >> + AXP20X_VBUS_V_ADC_H, 12); >> + if (r < 0) >> + return r; >> + >> + val->intval = r * 1700; /* 1 step = 1.7 mV */ > > Eventually, as we will have more users (the various power supplies, > the TS pin?), we should have an IIO driver for the internal ADC. IMHO an IIO driver is only useful in case the TS or gpio pins are used in ADC mode, and sofar we've not encountered any boards using that, In my expereince the IIO framework is overly complicated and usually gets in the way more then it helps. > Do you know how close it is from the AXP288's ? No I've not compared the 2. >> + return 0; >> + case POWER_SUPPLY_PROP_CURRENT_MAX: >> + r = regmap_read(power->regmap, AXP20X_VBUS_IPSOUT_MGMT, &v); >> + if (r) >> + return r; >> + >> + switch (v & AXP20X_VBUS_CLIMIT_MASK) { >> + case AXP20X_VBUC_CLIMIT_100mA: >> + val->intval = 100000; >> + break; >> + case AXP20X_VBUC_CLIMIT_500mA: >> + val->intval = 500000; >> + break; >> + case AXP20X_VBUC_CLIMIT_900mA: >> + val->intval = 900000; >> + break; >> + case AXP20X_VBUC_CLIMIT_NONE: >> + val->intval = -1; >> + break; >> + } >> + return 0; >> + case POWER_SUPPLY_PROP_CURRENT_NOW: >> + r = axp20x_read_variable_width(power->regmap, >> + AXP20X_VBUS_I_ADC_H, 12); >> + if (r < 0) >> + return r; >> + >> + val->intval = r * 375; /* 1 step = 0.375 mA */ > > And here too. > >> + return 0; >> + default: >> + break; >> + } >> + >> + /* All the properties below need the input-status reg value */ >> + r = regmap_read(power->regmap, AXP20X_PWR_INPUT_STATUS, &input); >> + if (r) >> + return r; >> + >> + switch (psp) { >> + case POWER_SUPPLY_PROP_HEALTH: >> + if (!(input & AXP20X_PWR_STATUS_VBUS_PRESENT)) { >> + val->intval = POWER_SUPPLY_HEALTH_UNKNOWN; >> + break; >> + } >> + >> + r = regmap_read(power->regmap, AXP20X_USB_OTG_STATUS, &v); >> + if (r) >> + return r; >> + >> + if (!(v & AXP20X_USB_STATUS_VBUS_VALID)) { >> + val->intval = POWER_SUPPLY_HEALTH_UNSPEC_FAILURE; >> + break; >> + } >> + >> + val->intval = POWER_SUPPLY_HEALTH_GOOD; >> + break; >> + case POWER_SUPPLY_PROP_PRESENT: >> + val->intval = !!(input & AXP20X_PWR_STATUS_VBUS_PRESENT); >> + break; >> + case POWER_SUPPLY_PROP_ONLINE: >> + val->intval = !!(input & AXP20X_PWR_STATUS_VBUS_USED); >> + break; >> + default: >> + return -EINVAL; >> + } >> + >> + return 0; >> +} >> + >> +static enum power_supply_property axp20x_usb_power_properties[] = { >> + POWER_SUPPLY_PROP_HEALTH, >> + POWER_SUPPLY_PROP_PRESENT, >> + POWER_SUPPLY_PROP_ONLINE, >> + POWER_SUPPLY_PROP_VOLTAGE_MIN, >> + POWER_SUPPLY_PROP_VOLTAGE_NOW, >> + POWER_SUPPLY_PROP_CURRENT_MAX, >> + POWER_SUPPLY_PROP_CURRENT_NOW, >> +}; >> + >> +static const struct power_supply_desc axp20x_usb_power_desc = { >> + .name = "axp20x-usb", >> + .type = POWER_SUPPLY_TYPE_USB, >> + .properties = axp20x_usb_power_properties, >> + .num_properties = ARRAY_SIZE(axp20x_usb_power_properties), >> + .get_property = axp20x_usb_power_get_property, >> +}; >> + >> +static int axp20x_usb_power_probe(struct platform_device *pdev) >> +{ >> + struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent); >> + struct power_supply_config psy_cfg = {}; >> + struct axp20x_usb_power *power; >> + static const char * const irq_names[] = { "VBUS_PLUGIN", >> + "VBUS_REMOVAL", "VBUS_VALID", "VBUS_NOT_VALID" }; >> + int i, irq, r; >> + >> + if (!of_device_is_available(pdev->dev.of_node)) >> + return -ENODEV; > > That looks odd. Is it even going to be probed in the first place? Yes, the mfd framework will instantiate all cells listed for an mfd device without checking there are compatible child-nodes present in the dtb. >> + >> + power = devm_kzalloc(&pdev->dev, sizeof(*power), GFP_KERNEL); >> + if (!power) >> + return -ENOMEM; >> + >> + power->regmap = axp20x->regmap; > > It should probably be wise to test that the axp20x pointer is not > NULL. The platform device we are probing has been instantiated by the mfd framework after setting the parent device driver ptr, so this will never be not NULL. >> + >> + /* Enable vbus valid checking */ >> + r = regmap_update_bits(power->regmap, AXP20X_VBUS_MON, >> + AXP20X_VBUS_MON_VBUS_VALID, AXP20X_VBUS_MON_VBUS_VALID); >> + if (r) >> + return r; >> + >> + /* Enable vbus voltage and current measurement */ >> + r = regmap_update_bits(power->regmap, AXP20X_ADC_EN1, >> + AXP20X_ADC_EN1_VBUS_CURR | AXP20X_ADC_EN1_VBUS_VOLT, >> + AXP20X_ADC_EN1_VBUS_CURR | AXP20X_ADC_EN1_VBUS_VOLT); >> + if (r) >> + return r; >> + >> + psy_cfg.of_node = pdev->dev.of_node; >> + psy_cfg.drv_data = power; >> + >> + power->supply = devm_power_supply_register(&pdev->dev, >> + &axp20x_usb_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; i < ARRAY_SIZE(irq_names); 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); >> + r = devm_request_any_context_irq(&pdev->dev, irq, >> + axp20x_usb_power_irq, 0, DRVNAME, power); >> + if (r < 0) >> + dev_warn(&pdev->dev, "Error requesting %s IRQ: %d\n", >> + irq_names[i], r); >> + } >> + >> + return 0; >> +} >> + >> +static const struct of_device_id axp20x_usb_power_match[] = { >> + { .compatible = "x-powers,axp202-usb-power-supply" }, >> + { } >> +}; >> +MODULE_DEVICE_TABLE(of, axp20x_usb_power_match); >> + >> +static struct platform_driver axp20x_usb_power_driver = { >> + .probe = axp20x_usb_power_probe, >> + .driver = { >> + .name = DRVNAME, >> + .of_match_table = axp20x_usb_power_match, >> + }, >> +}; >> + >> +module_platform_driver(axp20x_usb_power_driver); >> + >> +MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com>"); >> +MODULE_DESCRIPTION("AXP20x PMIC USB power supply status driver"); >> +MODULE_LICENSE("GPL"); >> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h >> index 6d8b39a..8ec996e 100644 >> --- a/include/linux/mfd/axp20x.h >> +++ b/include/linux/mfd/axp20x.h >> @@ -11,6 +11,8 @@ >> #ifndef __LINUX_MFD_AXP20X_H >> #define __LINUX_MFD_AXP20X_H >> >> +#include <linux/regmap.h> >> + >> enum { >> AXP202_ID = 0, >> AXP209_ID, >> @@ -372,4 +374,26 @@ struct axp288_extcon_pdata { >> struct gpio_desc *gpio_mux_cntl; >> }; >> >> +/* generic helper function for reading 9-16 bit wide regs */ >> +static inline int axp20x_read_variable_width(struct regmap *regmap, >> + unsigned int reg, unsigned int width) >> +{ >> + unsigned int reg_val, result; >> + int err; >> + >> + err = regmap_read(regmap, reg, ®_val); >> + if (err) >> + return err; >> + >> + result = reg_val << (width - 8); >> + >> + err = regmap_read(regmap, reg + 1, ®_val); >> + if (err) >> + return err; >> + >> + result |= reg_val; >> + >> + return result; >> +} >> + >> #endif /* __LINUX_MFD_AXP20X_H */ >> -- >> 2.4.3 Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 26 Jun 2015, Hans de Goede wrote: > From: Bruno Prémont <bonbons@linux-vserver.org> > > Add an extra set of registers which is necessary tu support the PMICs > battery charger function, and mark registers which contain status bits, > gpio status, and adc readings as volatile. > > Cc: Bruno Prémont <bonbons@linux-vserver.org> > Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > Changes in v2: > -Add a AXP20X_OCV_MAX define > Changes in v3: > -Add Bruno's S-o-b > --- > drivers/mfd/axp20x.c | 8 +++++++- > include/linux/mfd/axp20x.h | 6 ++++++ > 2 files changed, 13 insertions(+), 1 deletion(-) For my own reference: Acked-by: Lee Jones <lee.jones@linaro.org> Are these two patches tied to any other in the set, or can I apply them separately? > diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c > index 6df9155..f9a3c2d 100644 > --- a/drivers/mfd/axp20x.c > +++ b/drivers/mfd/axp20x.c > @@ -39,10 +39,16 @@ static const char * const axp20x_model_names[] = { > static const struct regmap_range axp20x_writeable_ranges[] = { > regmap_reg_range(AXP20X_DATACACHE(0), AXP20X_IRQ5_STATE), > regmap_reg_range(AXP20X_DCDC_MODE, AXP20X_FG_RES), > + regmap_reg_range(AXP20X_RDC_H, AXP20X_OCV(15)), > }; > > static const struct regmap_range axp20x_volatile_ranges[] = { > + regmap_reg_range(AXP20X_PWR_INPUT_STATUS, AXP20X_USB_OTG_STATUS), > + regmap_reg_range(AXP20X_CHRG_CTRL1, AXP20X_CHRG_CTRL2), > regmap_reg_range(AXP20X_IRQ1_EN, AXP20X_IRQ5_STATE), > + regmap_reg_range(AXP20X_ACIN_V_ADC_H, AXP20X_IPSOUT_V_HIGH_L), > + regmap_reg_range(AXP20X_GPIO20_SS, AXP20X_GPIO3_CTRL), > + regmap_reg_range(AXP20X_FG_RES, AXP20X_RDC_L), > }; > > static const struct regmap_access_table axp20x_writeable_table = { > @@ -159,7 +165,7 @@ static const struct regmap_config axp20x_regmap_config = { > .val_bits = 8, > .wr_table = &axp20x_writeable_table, > .volatile_table = &axp20x_volatile_table, > - .max_register = AXP20X_FG_RES, > + .max_register = AXP20X_OCV(AXP20X_OCV_MAX), > .cache_type = REGCACHE_RBTREE, > }; > > diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h > index 5275423..6d8b39a 100644 > --- a/include/linux/mfd/axp20x.h > +++ b/include/linux/mfd/axp20x.h > @@ -151,6 +151,12 @@ enum { > #define AXP20X_CC_CTRL 0xb8 > #define AXP20X_FG_RES 0xb9 > > +/* OCV */ > +#define AXP20X_RDC_H 0xba > +#define AXP20X_RDC_L 0xbb > +#define AXP20X_OCV(m) (0xc0 + (m)) > +#define AXP20X_OCV_MAX 0xf > + > /* AXP22X specific registers */ > #define AXP22X_BATLOW_THRES1 0xe6 >
On Fri, 26 Jun 2015, Hans de Goede wrote: > Add a cell for the usb power_supply part of the axp20x PMICs. > > Note that this cell is only for the usb power_supply part and not the > ac-power / battery-charger / rtc-backup-bat-charger bits. > > Depending on the board each of those must be enabled / disabled separately > in devicetree as most boards do not use all 4. So in dt each one needs its > own child-node of the axp20x node. Another reason for using separate child > nodes for each is so that other devicetree nodes can have a power-supply > property with a phandle referencing a node representing a single > power-supply. > > The decision to use a separate devicetree node for each is reflected on > the kernel side by each getting its own mfd-cell / platform_device and > platform-driver. > > Note this commit also makes some whitespace changes to the intialization > of existing cells in axp20x_cells, these are pure whitespace changes, > functionally nothing changes. > > Cc: Bruno Prémont <bonbons@linux-vserver.org> > Acked-by: Lee Jones <lee.jones@linaro.org> > Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > Changes in v2: > -Use DEFINE_RES_IRQ_NAMED > -Change indentation of axp20x_cells initializers to avoid line wrapping > Changes in v3: > -Improve commit message > -Add Bruno's S-o-b > --- > drivers/mfd/axp20x.c | 20 ++++++++++++++++---- > 1 file changed, 16 insertions(+), 4 deletions(-) For my own reference: Acked-by: Lee Jones <lee.jones@linaro.org> Are these two patches tied to any other in the set, or can I apply them separately? > diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c > index f9a3c2d..ca4a604 100644 > --- a/drivers/mfd/axp20x.c > +++ b/drivers/mfd/axp20x.c > @@ -113,6 +113,13 @@ static struct resource axp20x_pek_resources[] = { > }, > }; > > +static struct resource axp20x_usb_power_supply_resources[] = { > + DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_VBUS_PLUGIN, "VBUS_PLUGIN"), > + DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_VBUS_REMOVAL, "VBUS_REMOVAL"), > + DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_VBUS_VALID, "VBUS_VALID"), > + DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_VBUS_NOT_VALID, "VBUS_NOT_VALID"), > +}; > + > static struct resource axp22x_pek_resources[] = { > { > .name = "PEK_DBR", > @@ -363,11 +370,16 @@ static const struct regmap_irq_chip axp288_regmap_irq_chip = { > > static struct mfd_cell axp20x_cells[] = { > { > - .name = "axp20x-pek", > - .num_resources = ARRAY_SIZE(axp20x_pek_resources), > - .resources = axp20x_pek_resources, > + .name = "axp20x-pek", > + .num_resources = ARRAY_SIZE(axp20x_pek_resources), > + .resources = axp20x_pek_resources, > }, { > - .name = "axp20x-regulator", > + .name = "axp20x-regulator", > + }, { > + .name = "axp20x-usb-power-supply", > + .of_compatible = "x-powers,axp202-usb-power-supply", > + .num_resources = ARRAY_SIZE(axp20x_usb_power_supply_resources), > + .resources = axp20x_usb_power_supply_resources, > }, > }; >
Hi, On 07-07-15 09:01, Lee Jones wrote: > On Fri, 26 Jun 2015, Hans de Goede wrote: > >> Add a cell for the usb power_supply part of the axp20x PMICs. >> >> Note that this cell is only for the usb power_supply part and not the >> ac-power / battery-charger / rtc-backup-bat-charger bits. >> >> Depending on the board each of those must be enabled / disabled separately >> in devicetree as most boards do not use all 4. So in dt each one needs its >> own child-node of the axp20x node. Another reason for using separate child >> nodes for each is so that other devicetree nodes can have a power-supply >> property with a phandle referencing a node representing a single >> power-supply. >> >> The decision to use a separate devicetree node for each is reflected on >> the kernel side by each getting its own mfd-cell / platform_device and >> platform-driver. >> >> Note this commit also makes some whitespace changes to the intialization >> of existing cells in axp20x_cells, these are pure whitespace changes, >> functionally nothing changes. >> >> Cc: Bruno Prémont <bonbons@linux-vserver.org> >> Acked-by: Lee Jones <lee.jones@linaro.org> >> Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> Changes in v2: >> -Use DEFINE_RES_IRQ_NAMED >> -Change indentation of axp20x_cells initializers to avoid line wrapping >> Changes in v3: >> -Improve commit message >> -Add Bruno's S-o-b >> --- >> drivers/mfd/axp20x.c | 20 ++++++++++++++++---- >> 1 file changed, 16 insertions(+), 4 deletions(-) > > For my own reference: > Acked-by: Lee Jones <lee.jones@linaro.org> > > Are these two patches tied to any other in the set, or can I apply > them separately? You can apply them seperately, thanks. I'm wondering who will take care of upstreaming the first patch of this set: "ARM: dts: Add binding documentation for AXP20x pmic usb power supply" If you can take that one too that would be great. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 07 Jul 2015, Hans de Goede wrote: > On 07-07-15 09:01, Lee Jones wrote: > >On Fri, 26 Jun 2015, Hans de Goede wrote: > > > >>Add a cell for the usb power_supply part of the axp20x PMICs. > >> > >>Note that this cell is only for the usb power_supply part and not the > >>ac-power / battery-charger / rtc-backup-bat-charger bits. > >> > >>Depending on the board each of those must be enabled / disabled separately > >>in devicetree as most boards do not use all 4. So in dt each one needs its > >>own child-node of the axp20x node. Another reason for using separate child > >>nodes for each is so that other devicetree nodes can have a power-supply > >>property with a phandle referencing a node representing a single > >>power-supply. > >> > >>The decision to use a separate devicetree node for each is reflected on > >>the kernel side by each getting its own mfd-cell / platform_device and > >>platform-driver. > >> > >>Note this commit also makes some whitespace changes to the intialization > >>of existing cells in axp20x_cells, these are pure whitespace changes, > >>functionally nothing changes. > >> > >>Cc: Bruno Prémont <bonbons@linux-vserver.org> > >>Acked-by: Lee Jones <lee.jones@linaro.org> > >>Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org> > >>Signed-off-by: Hans de Goede <hdegoede@redhat.com> > >>--- > >>Changes in v2: > >>-Use DEFINE_RES_IRQ_NAMED > >>-Change indentation of axp20x_cells initializers to avoid line wrapping > >>Changes in v3: > >>-Improve commit message > >>-Add Bruno's S-o-b > >>--- > >> drivers/mfd/axp20x.c | 20 ++++++++++++++++---- > >> 1 file changed, 16 insertions(+), 4 deletions(-) > > > >For my own reference: > > Acked-by: Lee Jones <lee.jones@linaro.org> > > > >Are these two patches tied to any other in the set, or can I apply > >them separately? > > You can apply them seperately, thanks. > > I'm wondering who will take care of upstreaming the first patch of this > set: "ARM: dts: Add binding documentation for AXP20x pmic usb power supply" > > If you can take that one too that would be great. I can do, but I need an Ack from these guys: POWER SUPPLY CLASS/SUBSYSTEM and DRIVERS M: Sebastian Reichel <sre@kernel.org> M: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> M: David Woodhouse <dwmw2@infradead.org> L: linux-pm@vger.kernel.org T: git git://git.infradead.org/battery-2.6.git S: Maintained F: include/linux/power_supply.h F: drivers/power/ ... failing that, they should take it.
Hi, On Fri, Jun 26, 2015 at 12:59:14PM +0200, Hans de Goede wrote: > Add binding documentation for the usb power supply part of the AXP20x pmic. > [...] > +Example: > + > [...] > + > + usb_power_supply: usb_power_supply { ^^^^^^^^^^^^^^^^ use usb-power-supply here. > + compatible = "x-powers,axp202-usb-power-supply"; > + }; > +}; -- Sebastian
Hi, On Fri, Jun 26, 2015 at 12:59:17PM +0200, Hans de Goede wrote: > This adds a driver for the usb power_supply bits of the axp20x PMICs. > > I initially started writing my own driver, before coming aware of > Bruno Prémont's excellent earlier RFC with a driver for this. > > My driver was lacking CURRENT_MAX and VOLTAGE_MIN support Bruno's > drvier has, so I've copied the code for those from his driver. > > Note that the AC-power-supply and battery charger bits will need separate > drivers. Each one needs its own devictree child-node so that other > devicetree nodes can reference the right power-supply, and thus each one > will get its own mfd-cell / platform_device and platform-driver. Once the other comments have been taken care of, the driver looks ok to me. -- Sebastian
Hi, On 24-07-15 16:59, Sebastian Reichel wrote: > Hi, > > On Fri, Jun 26, 2015 at 12:59:14PM +0200, Hans de Goede wrote: >> Add binding documentation for the usb power supply part of the AXP20x pmic. >> [...] >> +Example: >> + >> [...] >> + >> + usb_power_supply: usb_power_supply { > ^^^^^^^^^^^^^^^^ > use usb-power-supply here. Ok, fixed for the upcoming v4 of the power-supply part of this patch set. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On 24-07-15 17:10, Sebastian Reichel wrote: > Hi, > > On Fri, Jun 26, 2015 at 12:59:17PM +0200, Hans de Goede wrote: >> This adds a driver for the usb power_supply bits of the axp20x PMICs. >> >> I initially started writing my own driver, before coming aware of >> Bruno Prémont's excellent earlier RFC with a driver for this. >> >> My driver was lacking CURRENT_MAX and VOLTAGE_MIN support Bruno's >> drvier has, so I've copied the code for those from his driver. >> >> Note that the AC-power-supply and battery charger bits will need separate >> drivers. Each one needs its own devictree child-node so that other >> devicetree nodes can reference the right power-supply, and thus each one >> will get its own mfd-cell / platform_device and platform-driver. > > Once the other comments have been taken care of, the driver looks ok > to me. Ok, I will do a v4 of the power-supply part of this patch set, with the various remarks on this patch fixed. Thanks & Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Sebastian, On 24-07-15 17:10, Sebastian Reichel wrote: > Hi, > > On Fri, Jun 26, 2015 at 12:59:17PM +0200, Hans de Goede wrote: >> This adds a driver for the usb power_supply bits of the axp20x PMICs. >> >> I initially started writing my own driver, before coming aware of >> Bruno Prémont's excellent earlier RFC with a driver for this. >> >> My driver was lacking CURRENT_MAX and VOLTAGE_MIN support Bruno's >> drvier has, so I've copied the code for those from his driver. >> >> Note that the AC-power-supply and battery charger bits will need separate >> drivers. Each one needs its own devictree child-node so that other >> devicetree nodes can reference the right power-supply, and thus each one >> will get its own mfd-cell / platform_device and platform-driver. > > Once the other comments have been taken care of, the driver looks ok > to me. I send out a v4 taking care of the other comments a while back, any news on this? Anything I need todo from my side to get things moving? Thanks & Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/power_supply/axp20x_usb_power.txt b/Documentation/devicetree/bindings/power_supply/axp20x_usb_power.txt new file mode 100644 index 0000000..a84d801 --- /dev/null +++ b/Documentation/devicetree/bindings/power_supply/axp20x_usb_power.txt @@ -0,0 +1,34 @@ +AXP20x USB power supply + +Required Properties: +-compatible: "x-powers,axp202-usb-power-supply" + +This node is a subnode of the axp20x PMIC. + +Example: + +axp209: pmic@34 { + compatible = "x-powers,axp209"; + reg = <0x34>; + interrupt-parent = <&nmi_intc>; + interrupts = <0 IRQ_TYPE_LEVEL_LOW>; + interrupt-controller; + #interrupt-cells = <1>; + + regulators { + x-powers,dcdc-freq = <1500>; + + vdd_cpu: dcdc2 { + regulator-always-on; + regulator-min-microvolt = <1000000>; + regulator-max-microvolt = <1450000>; + regulator-name = "vdd-cpu"; + }; + + ... + }; + + usb_power_supply: usb_power_supply { + compatible = "x-powers,axp202-usb-power-supply"; + }; +};
Add binding documentation for the usb power supply part of the AXP20x pmic. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- Changes in v2: -Split out into a separate patch from the actual driver commit --- .../bindings/power_supply/axp20x_usb_power.txt | 34 ++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 Documentation/devicetree/bindings/power_supply/axp20x_usb_power.txt