* [PATCH 0/2] max8903: Add device tree support and logic fixup @ 2016-06-02 6:44 chris 2016-06-02 6:44 ` [PATCH 1/2] max8903: adds support for initiation via device tree chris 2016-06-02 6:44 ` [PATCH 2/2] max8903: cleans up confusing relationship between dc_valid, dok and dcm chris 0 siblings, 2 replies; 68+ messages in thread From: chris @ 2016-06-02 6:44 UTC (permalink / raw) To: linux-kernel, linux-pm; +Cc: Chris Lapa From: Chris Lapa <chris@lapa.com.au> This patch set adds device tree support for the MAX8903 battery charger and also cleans up the logic with the dc_valid, dok and dcm pins. I verified these patches work on a board I have here, which uses the DC power side (not the USB portition) of the MAX8903. Chris Lapa (2): max8903: adds support for initiation via device tree. max8903: cleans up confusing relationship between dc_valid, dok and dcm. .../devicetree/bindings/power/max8903-charger.txt | 28 ++ drivers/power/max8903_charger.c | 287 ++++++++++++++++----- include/linux/power/max8903_charger.h | 6 +- 3 files changed, 250 insertions(+), 71 deletions(-) create mode 100644 Documentation/devicetree/bindings/power/max8903-charger.txt -- 1.9.1 ^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH 1/2] max8903: adds support for initiation via device tree. 2016-06-02 6:44 [PATCH 0/2] max8903: Add device tree support and logic fixup chris @ 2016-06-02 6:44 ` chris 2016-06-09 10:35 ` Krzysztof Kozlowski 2016-06-02 6:44 ` [PATCH 2/2] max8903: cleans up confusing relationship between dc_valid, dok and dcm chris 1 sibling, 1 reply; 68+ messages in thread From: chris @ 2016-06-02 6:44 UTC (permalink / raw) To: linux-kernel, linux-pm; +Cc: Chris Lapa From: Chris Lapa <chris@lapa.com.au> This commit also adds requesting gpio's via devm_gpio_request() to ensure the gpio is available for usage by the driver. Signed-off-by: Chris Lapa <chris@lapa.com.au> --- .../devicetree/bindings/power/max8903-charger.txt | 28 ++ drivers/power/max8903_charger.c | 281 ++++++++++++++++----- 2 files changed, 250 insertions(+), 59 deletions(-) create mode 100644 Documentation/devicetree/bindings/power/max8903-charger.txt diff --git a/Documentation/devicetree/bindings/power/max8903-charger.txt b/Documentation/devicetree/bindings/power/max8903-charger.txt new file mode 100644 index 0000000..7207731 --- /dev/null +++ b/Documentation/devicetree/bindings/power/max8903-charger.txt @@ -0,0 +1,28 @@ +Maxim Semiconductor MAX8903 Battery Charger bindings + +Required properties: +- compatible: "max8903-charger" for MAX8903 Battery Charger +- dc_valid: + - dok: DC power OK pin +- usb_valid: + - uok: USB power OK pin + +Optional properties: +- cen: Charge enable pin +- chg: Charger status pin +- flt: Fault pin +- dcm: Current limit mode setting (DC or USB) +- usus: USB suspend pin + + +Example: + + max8903-charger { + compatible = "max8903-charger"; + dok = <&gpio2 3 GPIO_ACTIVE_LOW>; + flt = <&gpio2 2 GPIO_ACTIVE_LOW>; + chg = <&gpio3 15 GPIO_ACTIVE_LOW>; + cen = <&gpio2 5 GPIO_ACTIVE_LOW>; + dc_valid; + status = "okay"; + }; diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c index 17876ca..1989c10 100644 --- a/drivers/power/max8903_charger.c +++ b/drivers/power/max8903_charger.c @@ -23,13 +23,16 @@ #include <linux/gpio.h> #include <linux/interrupt.h> #include <linux/module.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/of_gpio.h> #include <linux/slab.h> #include <linux/power_supply.h> #include <linux/platform_device.h> #include <linux/power/max8903_charger.h> struct max8903_data { - struct max8903_pdata pdata; + struct max8903_pdata *pdata; struct device *dev; struct power_supply *psy; struct power_supply_desc psy_desc; @@ -53,8 +56,8 @@ static int max8903_get_property(struct power_supply *psy, switch (psp) { case POWER_SUPPLY_PROP_STATUS: val->intval = POWER_SUPPLY_STATUS_UNKNOWN; - if (data->pdata.chg) { - if (gpio_get_value(data->pdata.chg) == 0) + if (data->pdata->chg) { + if (gpio_get_value(data->pdata->chg) == 0) val->intval = POWER_SUPPLY_STATUS_CHARGING; else if (data->usb_in || data->ta_in) val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING; @@ -75,13 +78,14 @@ static int max8903_get_property(struct power_supply *psy, default: return -EINVAL; } + return 0; } static irqreturn_t max8903_dcin(int irq, void *_data) { struct max8903_data *data = _data; - struct max8903_pdata *pdata = &data->pdata; + struct max8903_pdata *pdata = data->pdata; bool ta_in; enum power_supply_type old_type; @@ -122,7 +126,7 @@ static irqreturn_t max8903_dcin(int irq, void *_data) static irqreturn_t max8903_usbin(int irq, void *_data) { struct max8903_data *data = _data; - struct max8903_pdata *pdata = &data->pdata; + struct max8903_pdata *pdata = data->pdata; bool usb_in; enum power_supply_type old_type; @@ -161,7 +165,7 @@ static irqreturn_t max8903_usbin(int irq, void *_data) static irqreturn_t max8903_fault(int irq, void *_data) { struct max8903_data *data = _data; - struct max8903_pdata *pdata = &data->pdata; + struct max8903_pdata *pdata = data->pdata; bool fault; fault = gpio_get_value(pdata->flt) ? false : true; @@ -179,34 +183,135 @@ static irqreturn_t max8903_fault(int irq, void *_data) return IRQ_HANDLED; } +static struct max8903_pdata *max8903_parse_dt_data( + struct device *dev) +{ + struct device_node *of_node = dev->of_node; + struct max8903_pdata *pdata = NULL; + + if (!of_node) { + return pdata; + } + + pdata = devm_kzalloc(dev, sizeof(struct max8903_pdata), + GFP_KERNEL); + if (!pdata) { + return pdata; + } + + if (of_get_property(of_node, "dc_valid", NULL)) { + pdata->dc_valid = true; + } + + if (of_get_property(of_node, "usb_valid", NULL)) { + pdata->usb_valid = true; + } + + pdata->cen = of_get_named_gpio(of_node, "cen", 0); + if (!gpio_is_valid(pdata->cen)) { + pdata->cen = 0; + } + + pdata->chg = of_get_named_gpio(of_node, "chg", 0); + if (!gpio_is_valid(pdata->chg)) { + pdata->chg = 0; + } + + pdata->flt = of_get_named_gpio(of_node, "flt", 0); + if (!gpio_is_valid(pdata->flt)) { + pdata->flt = 0; + } + + pdata->usus = of_get_named_gpio(of_node, "usus", 0); + if (!gpio_is_valid(pdata->usus)) { + pdata->usus = 0; + } + + pdata->dcm = of_get_named_gpio(of_node, "dcm", 0); + if (!gpio_is_valid(pdata->dcm)) { + pdata->dcm = 0; + } + + pdata->dok = of_get_named_gpio(of_node, "dok", 0); + if (!gpio_is_valid(pdata->dok)) { + pdata->dok = 0; + } + + pdata->uok = of_get_named_gpio(of_node, "uok", 0); + if (!gpio_is_valid(pdata->uok)) { + pdata->uok = 0; + } + + return pdata; +} + static int max8903_probe(struct platform_device *pdev) { - struct max8903_data *data; + struct max8903_data *charger; struct device *dev = &pdev->dev; - struct max8903_pdata *pdata = pdev->dev.platform_data; struct power_supply_config psy_cfg = {}; int ret = 0; int gpio; int ta_in = 0; int usb_in = 0; - data = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL); - if (data == NULL) { + charger = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL); + if (charger == NULL) { dev_err(dev, "Cannot allocate memory.\n"); return -ENOMEM; } - memcpy(&data->pdata, pdata, sizeof(struct max8903_pdata)); - data->dev = dev; - platform_set_drvdata(pdev, data); - if (pdata->dc_valid == false && pdata->usb_valid == false) { + charger->pdata = pdev->dev.platform_data; + if (IS_ENABLED(CONFIG_OF) && !charger->pdata && dev->of_node) { + charger->pdata = max8903_parse_dt_data(dev); + if (!charger->pdata) + return -EINVAL; + } + + charger->dev = dev; + + platform_set_drvdata(pdev, charger); + + charger->fault = false; + charger->ta_in = ta_in; + charger->usb_in = usb_in; + + charger->psy_desc.name = "max8903_charger"; + charger->psy_desc.type = (ta_in) ? POWER_SUPPLY_TYPE_MAINS : + ((usb_in) ? POWER_SUPPLY_TYPE_USB : + POWER_SUPPLY_TYPE_BATTERY); + charger->psy_desc.get_property = max8903_get_property; + charger->psy_desc.properties = max8903_charger_props; + charger->psy_desc.num_properties = ARRAY_SIZE(max8903_charger_props); + + if (charger->pdata->dc_valid == false && charger->pdata->usb_valid == false) { dev_err(dev, "No valid power sources.\n"); return -EINVAL; } - if (pdata->dc_valid) { - if (pdata->dok && gpio_is_valid(pdata->dok) && - pdata->dcm && gpio_is_valid(pdata->dcm)) { + if (charger->pdata->dc_valid) { + if (charger->pdata->dok && gpio_is_valid(charger->pdata->dok) && + charger->pdata->dcm && gpio_is_valid(charger->pdata->dcm)) { + ret = devm_gpio_request(dev, + charger->pdata->dok, + charger->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for dok: %d err %d\n", + charger->pdata->dok, ret); + return -EINVAL; + } + + ret = devm_gpio_request(dev, + charger->pdata->dcm, + charger->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for dcm: %d err %d\n", + charger->pdata->dcm, ret); + return -EINVAL; + } + gpio = pdata->dok; /* PULL_UPed Interrupt */ ta_in = gpio_get_value(gpio) ? 0 : 1; @@ -219,18 +324,38 @@ static int max8903_probe(struct platform_device *pdev) } } else { if (pdata->dcm) { - if (gpio_is_valid(pdata->dcm)) + if (gpio_is_valid(pdata->dcm)) { + ret = devm_gpio_request(dev, + charger->pdata->dcm, + charger->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for dcm: %d err %d\n", + charger->pdata->dcm, ret); + return -EINVAL; + } + gpio_set_value(pdata->dcm, 0); - else { + } else { dev_err(dev, "Invalid pin: dcm.\n"); return -EINVAL; } } } - if (pdata->usb_valid) { - if (pdata->uok && gpio_is_valid(pdata->uok)) { - gpio = pdata->uok; + if (charger->pdata->usb_valid) { + if (gpio_is_valid(charger->pdata->uok)) { + ret = devm_gpio_request(dev, + charger->pdata->uok, + charger->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for uok: %d err %d\n", + charger->pdata->uok, ret); + return -EINVAL; + } + + gpio = charger->pdata->uok; usb_in = gpio_get_value(gpio) ? 0 : 1; } else { dev_err(dev, "When USB is wired, UOK should be wired." @@ -239,91 +364,122 @@ static int max8903_probe(struct platform_device *pdev) } } - if (pdata->cen) { - if (gpio_is_valid(pdata->cen)) { - gpio_set_value(pdata->cen, (ta_in || usb_in) ? 0 : 1); + if (charger->pdata->cen) { + if (gpio_is_valid(charger->pdata->cen)) { + ret = devm_gpio_request(dev, + charger->pdata->cen, + charger->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for cen: %d err %d\n", + charger->pdata->cen, ret); + return -EINVAL; + } + + gpio_set_value(charger->pdata->cen, (ta_in || usb_in) ? 0 : 1); } else { dev_err(dev, "Invalid pin: cen.\n"); return -EINVAL; } } - if (pdata->chg) { - if (!gpio_is_valid(pdata->chg)) { + if (charger->pdata->chg) { + if (gpio_is_valid(charger->pdata->chg)) { + ret = devm_gpio_request(dev, + charger->pdata->chg, + charger->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for chg: %d err %d\n", + charger->pdata->chg, ret); + return -EINVAL; + } + } else { dev_err(dev, "Invalid pin: chg.\n"); return -EINVAL; } } - if (pdata->flt) { - if (!gpio_is_valid(pdata->flt)) { + if (charger->pdata->flt) { + if (gpio_is_valid(charger->pdata->flt)) { + ret = devm_gpio_request(dev, + charger->pdata->flt, + charger->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for flt: %d err %d\n", + charger->pdata->flt, ret); + return -EINVAL; + } + } else { dev_err(dev, "Invalid pin: flt.\n"); return -EINVAL; } } - if (pdata->usus) { - if (!gpio_is_valid(pdata->usus)) { + if (charger->pdata->usus) { + if (gpio_is_valid(charger->pdata->usus)) { + ret = devm_gpio_request(dev, + charger->pdata->usus, + charger->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for usus: %d err %d\n", + charger->pdata->usus, ret); + return -EINVAL; + } + } else { dev_err(dev, "Invalid pin: usus.\n"); return -EINVAL; } } - data->fault = false; - data->ta_in = ta_in; - data->usb_in = usb_in; - - data->psy_desc.name = "max8903_charger"; - data->psy_desc.type = (ta_in) ? POWER_SUPPLY_TYPE_MAINS : - ((usb_in) ? POWER_SUPPLY_TYPE_USB : - POWER_SUPPLY_TYPE_BATTERY); - data->psy_desc.get_property = max8903_get_property; - data->psy_desc.properties = max8903_charger_props; - data->psy_desc.num_properties = ARRAY_SIZE(max8903_charger_props); - - psy_cfg.drv_data = data; + psy_cfg.supplied_to = NULL; + psy_cfg.num_supplicants = 0; + psy_cfg.of_node = dev->of_node; + psy_cfg.drv_data = charger; - data->psy = devm_power_supply_register(dev, &data->psy_desc, &psy_cfg); - if (IS_ERR(data->psy)) { + charger->psy = devm_power_supply_register(dev, &charger->psy_desc, &psy_cfg); + if (IS_ERR(charger->psy)) { dev_err(dev, "failed: power supply register.\n"); - return PTR_ERR(data->psy); + return PTR_ERR(charger->psy); } - if (pdata->dc_valid) { - ret = devm_request_threaded_irq(dev, gpio_to_irq(pdata->dok), + if (charger->pdata->dc_valid) { + ret = devm_request_threaded_irq(dev, gpio_to_irq(charger->pdata->dok), NULL, max8903_dcin, IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING | IRQF_ONESHOT, - "MAX8903 DC IN", data); + "MAX8903 DC IN", charger); if (ret) { dev_err(dev, "Cannot request irq %d for DC (%d)\n", - gpio_to_irq(pdata->dok), ret); + gpio_to_irq(charger->pdata->dok), ret); return ret; } } - if (pdata->usb_valid) { - ret = devm_request_threaded_irq(dev, gpio_to_irq(pdata->uok), + if (charger->pdata->usb_valid) { + ret = devm_request_threaded_irq(dev, gpio_to_irq(charger->pdata->uok), NULL, max8903_usbin, IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING | IRQF_ONESHOT, - "MAX8903 USB IN", data); + "MAX8903 USB IN", charger); if (ret) { dev_err(dev, "Cannot request irq %d for USB (%d)\n", - gpio_to_irq(pdata->uok), ret); + gpio_to_irq(charger->pdata->uok), ret); return ret; } } - if (pdata->flt) { - ret = devm_request_threaded_irq(dev, gpio_to_irq(pdata->flt), + if (charger->pdata->flt) { + ret = devm_request_threaded_irq(dev, gpio_to_irq(charger->pdata->flt), NULL, max8903_fault, IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING | IRQF_ONESHOT, - "MAX8903 Fault", data); + "MAX8903 Fault", charger); if (ret) { dev_err(dev, "Cannot request irq %d for Fault (%d)\n", - gpio_to_irq(pdata->flt), ret); + gpio_to_irq(charger->pdata->flt), ret); return ret; } } @@ -331,10 +487,17 @@ static int max8903_probe(struct platform_device *pdev) return 0; } +static const struct of_device_id max8903_match_ids[] = { + { .compatible = "max8903-charger", }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, max8903_match_ids); + static struct platform_driver max8903_driver = { .probe = max8903_probe, .driver = { .name = "max8903-charger", + .of_match_table = max8903_match_ids }, }; -- 1.9.1 ^ permalink raw reply related [flat|nested] 68+ messages in thread
* Re: [PATCH 1/2] max8903: adds support for initiation via device tree. 2016-06-02 6:44 ` [PATCH 1/2] max8903: adds support for initiation via device tree chris @ 2016-06-09 10:35 ` Krzysztof Kozlowski 2016-06-10 5:35 ` Chris Lapa 0 siblings, 1 reply; 68+ messages in thread From: Krzysztof Kozlowski @ 2016-06-09 10:35 UTC (permalink / raw) To: chris; +Cc: linux-kernel, linux-pm Hi, Thanks for your contribution. Few comments below: On Thu, Jun 2, 2016 at 8:44 AM, <chris@lapa.com.au> wrote: > From: Chris Lapa <chris@lapa.com.au> > > This commit also adds requesting gpio's via devm_gpio_request() to ensure > the gpio is available for usage by the driver. > > Signed-off-by: Chris Lapa <chris@lapa.com.au> > --- > .../devicetree/bindings/power/max8903-charger.txt | 28 ++ > drivers/power/max8903_charger.c | 281 ++++++++++++++++----- > 2 files changed, 250 insertions(+), 59 deletions(-) > create mode 100644 Documentation/devicetree/bindings/power/max8903-charger.txt Please put the bindings documentation in separate, first patch. > > diff --git a/Documentation/devicetree/bindings/power/max8903-charger.txt b/Documentation/devicetree/bindings/power/max8903-charger.txt > new file mode 100644 > index 0000000..7207731 > --- /dev/null > +++ b/Documentation/devicetree/bindings/power/max8903-charger.txt > @@ -0,0 +1,28 @@ > +Maxim Semiconductor MAX8903 Battery Charger bindings > + > +Required properties: > +- compatible: "max8903-charger" for MAX8903 Battery Charger Needs a 'maxim,' prefix. > +- dc_valid: > + - dok: DC power OK pin > +- usb_valid: > + - uok: USB power OK pin I don't understand the explanation of them - dok/uok. What do you want to say here? > + > +Optional properties: > +- cen: Charge enable pin > +- chg: Charger status pin > +- flt: Fault pin > +- dcm: Current limit mode setting (DC or USB) > +- usus: USB suspend pin Each gpio should be suffixed with '-gpios' (see Documentation/devicetree/bindings/gpio/gpio.txt). > + > + > +Example: > + > + max8903-charger { > + compatible = "max8903-charger"; > + dok = <&gpio2 3 GPIO_ACTIVE_LOW>; > + flt = <&gpio2 2 GPIO_ACTIVE_LOW>; > + chg = <&gpio3 15 GPIO_ACTIVE_LOW>; > + cen = <&gpio2 5 GPIO_ACTIVE_LOW>; > + dc_valid; > + status = "okay"; > + }; > diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c > index 17876ca..1989c10 100644 > --- a/drivers/power/max8903_charger.c > +++ b/drivers/power/max8903_charger.c > @@ -23,13 +23,16 @@ > #include <linux/gpio.h> > #include <linux/interrupt.h> > #include <linux/module.h> > +#include <linux/of.h> > +#include <linux/of_device.h> > +#include <linux/of_gpio.h> > #include <linux/slab.h> > #include <linux/power_supply.h> > #include <linux/platform_device.h> > #include <linux/power/max8903_charger.h> > > struct max8903_data { > - struct max8903_pdata pdata; > + struct max8903_pdata *pdata; > struct device *dev; > struct power_supply *psy; > struct power_supply_desc psy_desc; > @@ -53,8 +56,8 @@ static int max8903_get_property(struct power_supply *psy, > switch (psp) { > case POWER_SUPPLY_PROP_STATUS: > val->intval = POWER_SUPPLY_STATUS_UNKNOWN; > - if (data->pdata.chg) { > - if (gpio_get_value(data->pdata.chg) == 0) > + if (data->pdata->chg) { > + if (gpio_get_value(data->pdata->chg) == 0) > val->intval = POWER_SUPPLY_STATUS_CHARGING; > else if (data->usb_in || data->ta_in) > val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING; > @@ -75,13 +78,14 @@ static int max8903_get_property(struct power_supply *psy, > default: > return -EINVAL; > } > + > return 0; > } > > static irqreturn_t max8903_dcin(int irq, void *_data) > { > struct max8903_data *data = _data; > - struct max8903_pdata *pdata = &data->pdata; > + struct max8903_pdata *pdata = data->pdata; > bool ta_in; > enum power_supply_type old_type; > > @@ -122,7 +126,7 @@ static irqreturn_t max8903_dcin(int irq, void *_data) > static irqreturn_t max8903_usbin(int irq, void *_data) > { > struct max8903_data *data = _data; > - struct max8903_pdata *pdata = &data->pdata; > + struct max8903_pdata *pdata = data->pdata; > bool usb_in; > enum power_supply_type old_type; > > @@ -161,7 +165,7 @@ static irqreturn_t max8903_usbin(int irq, void *_data) > static irqreturn_t max8903_fault(int irq, void *_data) > { > struct max8903_data *data = _data; > - struct max8903_pdata *pdata = &data->pdata; > + struct max8903_pdata *pdata = data->pdata; > bool fault; > > fault = gpio_get_value(pdata->flt) ? false : true; > @@ -179,34 +183,135 @@ static irqreturn_t max8903_fault(int irq, void *_data) > return IRQ_HANDLED; > } > > +static struct max8903_pdata *max8903_parse_dt_data( > + struct device *dev) > +{ > + struct device_node *of_node = dev->of_node; > + struct max8903_pdata *pdata = NULL; > + > + if (!of_node) { > + return pdata; > + } Run a scripts/checkpatch.pl. In general the {} are not needed for single statements. > + > + pdata = devm_kzalloc(dev, sizeof(struct max8903_pdata), > + GFP_KERNEL); > + if (!pdata) { > + return pdata; ditto > + } > + > + if (of_get_property(of_node, "dc_valid", NULL)) { > + pdata->dc_valid = true; > + } > + > + if (of_get_property(of_node, "usb_valid", NULL)) { > + pdata->usb_valid = true; > + } > + > + pdata->cen = of_get_named_gpio(of_node, "cen", 0); > + if (!gpio_is_valid(pdata->cen)) { > + pdata->cen = 0; > + } > + > + pdata->chg = of_get_named_gpio(of_node, "chg", 0); > + if (!gpio_is_valid(pdata->chg)) { > + pdata->chg = 0; > + } > + > + pdata->flt = of_get_named_gpio(of_node, "flt", 0); > + if (!gpio_is_valid(pdata->flt)) { > + pdata->flt = 0; > + } > + > + pdata->usus = of_get_named_gpio(of_node, "usus", 0); > + if (!gpio_is_valid(pdata->usus)) { > + pdata->usus = 0; > + } > + > + pdata->dcm = of_get_named_gpio(of_node, "dcm", 0); > + if (!gpio_is_valid(pdata->dcm)) { > + pdata->dcm = 0; > + } > + > + pdata->dok = of_get_named_gpio(of_node, "dok", 0); > + if (!gpio_is_valid(pdata->dok)) { > + pdata->dok = 0; > + } > + > + pdata->uok = of_get_named_gpio(of_node, "uok", 0); > + if (!gpio_is_valid(pdata->uok)) { > + pdata->uok = 0; > + } > + > + return pdata; > +} > + > static int max8903_probe(struct platform_device *pdev) > { > - struct max8903_data *data; > + struct max8903_data *charger; > struct device *dev = &pdev->dev; > - struct max8903_pdata *pdata = pdev->dev.platform_data; > struct power_supply_config psy_cfg = {}; > int ret = 0; > int gpio; > int ta_in = 0; > int usb_in = 0; > > - data = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL); > - if (data == NULL) { > + charger = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL); > + if (charger == NULL) { > dev_err(dev, "Cannot allocate memory.\n"); In a separate patch - you can get rid of error message. I didn't perform a thorough review. First fix some easy ones. :) Best regards, Krzysztof ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 1/2] max8903: adds support for initiation via device tree. 2016-06-09 10:35 ` Krzysztof Kozlowski @ 2016-06-10 5:35 ` Chris Lapa 2016-06-10 6:13 ` Krzysztof Kozlowski 0 siblings, 1 reply; 68+ messages in thread From: Chris Lapa @ 2016-06-10 5:35 UTC (permalink / raw) To: Krzysztof Kozlowski; +Cc: linux-kernel, linux-pm Hi Krzysztof, Thanks for the review. I'm working on those changes now. However just so I know for the future. Why no error checking on devm_kzalloc() result? Looking through the source for devm_kzalloc() it looks like NULL isn't caught anywhere else. Thanks, Chris On 9/06/2016 8:35 PM, Krzysztof Kozlowski wrote: > Hi, > > Thanks for your contribution. Few comments below: > > On Thu, Jun 2, 2016 at 8:44 AM, <chris@lapa.com.au> wrote: >> From: Chris Lapa <chris@lapa.com.au> >> >> This commit also adds requesting gpio's via devm_gpio_request() to ensure >> the gpio is available for usage by the driver. >> >> Signed-off-by: Chris Lapa <chris@lapa.com.au> >> --- >> .../devicetree/bindings/power/max8903-charger.txt | 28 ++ >> drivers/power/max8903_charger.c | 281 ++++++++++++++++----- >> 2 files changed, 250 insertions(+), 59 deletions(-) >> create mode 100644 Documentation/devicetree/bindings/power/max8903-charger.txt > > Please put the bindings documentation in separate, first patch. > >> >> diff --git a/Documentation/devicetree/bindings/power/max8903-charger.txt b/Documentation/devicetree/bindings/power/max8903-charger.txt >> new file mode 100644 >> index 0000000..7207731 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/power/max8903-charger.txt >> @@ -0,0 +1,28 @@ >> +Maxim Semiconductor MAX8903 Battery Charger bindings >> + >> +Required properties: >> +- compatible: "max8903-charger" for MAX8903 Battery Charger > > Needs a 'maxim,' prefix. > >> +- dc_valid: >> + - dok: DC power OK pin >> +- usb_valid: >> + - uok: USB power OK pin > > I don't understand the explanation of them - dok/uok. What do you want > to say here? > >> + >> +Optional properties: >> +- cen: Charge enable pin >> +- chg: Charger status pin >> +- flt: Fault pin >> +- dcm: Current limit mode setting (DC or USB) >> +- usus: USB suspend pin > > Each gpio should be suffixed with '-gpios' (see > Documentation/devicetree/bindings/gpio/gpio.txt). > >> + >> + >> +Example: >> + >> + max8903-charger { >> + compatible = "max8903-charger"; >> + dok = <&gpio2 3 GPIO_ACTIVE_LOW>; >> + flt = <&gpio2 2 GPIO_ACTIVE_LOW>; >> + chg = <&gpio3 15 GPIO_ACTIVE_LOW>; >> + cen = <&gpio2 5 GPIO_ACTIVE_LOW>; >> + dc_valid; >> + status = "okay"; >> + }; >> diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c >> index 17876ca..1989c10 100644 >> --- a/drivers/power/max8903_charger.c >> +++ b/drivers/power/max8903_charger.c >> @@ -23,13 +23,16 @@ >> #include <linux/gpio.h> >> #include <linux/interrupt.h> >> #include <linux/module.h> >> +#include <linux/of.h> >> +#include <linux/of_device.h> >> +#include <linux/of_gpio.h> >> #include <linux/slab.h> >> #include <linux/power_supply.h> >> #include <linux/platform_device.h> >> #include <linux/power/max8903_charger.h> >> >> struct max8903_data { >> - struct max8903_pdata pdata; >> + struct max8903_pdata *pdata; >> struct device *dev; >> struct power_supply *psy; >> struct power_supply_desc psy_desc; >> @@ -53,8 +56,8 @@ static int max8903_get_property(struct power_supply *psy, >> switch (psp) { >> case POWER_SUPPLY_PROP_STATUS: >> val->intval = POWER_SUPPLY_STATUS_UNKNOWN; >> - if (data->pdata.chg) { >> - if (gpio_get_value(data->pdata.chg) == 0) >> + if (data->pdata->chg) { >> + if (gpio_get_value(data->pdata->chg) == 0) >> val->intval = POWER_SUPPLY_STATUS_CHARGING; >> else if (data->usb_in || data->ta_in) >> val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING; >> @@ -75,13 +78,14 @@ static int max8903_get_property(struct power_supply *psy, >> default: >> return -EINVAL; >> } >> + >> return 0; >> } >> >> static irqreturn_t max8903_dcin(int irq, void *_data) >> { >> struct max8903_data *data = _data; >> - struct max8903_pdata *pdata = &data->pdata; >> + struct max8903_pdata *pdata = data->pdata; >> bool ta_in; >> enum power_supply_type old_type; >> >> @@ -122,7 +126,7 @@ static irqreturn_t max8903_dcin(int irq, void *_data) >> static irqreturn_t max8903_usbin(int irq, void *_data) >> { >> struct max8903_data *data = _data; >> - struct max8903_pdata *pdata = &data->pdata; >> + struct max8903_pdata *pdata = data->pdata; >> bool usb_in; >> enum power_supply_type old_type; >> >> @@ -161,7 +165,7 @@ static irqreturn_t max8903_usbin(int irq, void *_data) >> static irqreturn_t max8903_fault(int irq, void *_data) >> { >> struct max8903_data *data = _data; >> - struct max8903_pdata *pdata = &data->pdata; >> + struct max8903_pdata *pdata = data->pdata; >> bool fault; >> >> fault = gpio_get_value(pdata->flt) ? false : true; >> @@ -179,34 +183,135 @@ static irqreturn_t max8903_fault(int irq, void *_data) >> return IRQ_HANDLED; >> } >> >> +static struct max8903_pdata *max8903_parse_dt_data( >> + struct device *dev) >> +{ >> + struct device_node *of_node = dev->of_node; >> + struct max8903_pdata *pdata = NULL; >> + >> + if (!of_node) { >> + return pdata; >> + } > > Run a scripts/checkpatch.pl. In general the {} are not needed for > single statements. > >> + >> + pdata = devm_kzalloc(dev, sizeof(struct max8903_pdata), >> + GFP_KERNEL); >> + if (!pdata) { >> + return pdata; > > ditto > >> + } >> + >> + if (of_get_property(of_node, "dc_valid", NULL)) { >> + pdata->dc_valid = true; >> + } >> + >> + if (of_get_property(of_node, "usb_valid", NULL)) { >> + pdata->usb_valid = true; >> + } >> + >> + pdata->cen = of_get_named_gpio(of_node, "cen", 0); >> + if (!gpio_is_valid(pdata->cen)) { >> + pdata->cen = 0; >> + } >> + >> + pdata->chg = of_get_named_gpio(of_node, "chg", 0); >> + if (!gpio_is_valid(pdata->chg)) { >> + pdata->chg = 0; >> + } >> + >> + pdata->flt = of_get_named_gpio(of_node, "flt", 0); >> + if (!gpio_is_valid(pdata->flt)) { >> + pdata->flt = 0; >> + } >> + >> + pdata->usus = of_get_named_gpio(of_node, "usus", 0); >> + if (!gpio_is_valid(pdata->usus)) { >> + pdata->usus = 0; >> + } >> + >> + pdata->dcm = of_get_named_gpio(of_node, "dcm", 0); >> + if (!gpio_is_valid(pdata->dcm)) { >> + pdata->dcm = 0; >> + } >> + >> + pdata->dok = of_get_named_gpio(of_node, "dok", 0); >> + if (!gpio_is_valid(pdata->dok)) { >> + pdata->dok = 0; >> + } >> + >> + pdata->uok = of_get_named_gpio(of_node, "uok", 0); >> + if (!gpio_is_valid(pdata->uok)) { >> + pdata->uok = 0; >> + } >> + >> + return pdata; >> +} >> + >> static int max8903_probe(struct platform_device *pdev) >> { >> - struct max8903_data *data; >> + struct max8903_data *charger; >> struct device *dev = &pdev->dev; >> - struct max8903_pdata *pdata = pdev->dev.platform_data; >> struct power_supply_config psy_cfg = {}; >> int ret = 0; >> int gpio; >> int ta_in = 0; >> int usb_in = 0; >> >> - data = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL); >> - if (data == NULL) { >> + charger = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL); >> + if (charger == NULL) { >> dev_err(dev, "Cannot allocate memory.\n"); > > In a separate patch - you can get rid of error message. > > I didn't perform a thorough review. First fix some easy ones. :) > > Best regards, > Krzysztof > ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 1/2] max8903: adds support for initiation via device tree. 2016-06-10 5:35 ` Chris Lapa @ 2016-06-10 6:13 ` Krzysztof Kozlowski 2016-06-10 10:23 ` Krzysztof Kozlowski 0 siblings, 1 reply; 68+ messages in thread From: Krzysztof Kozlowski @ 2016-06-10 6:13 UTC (permalink / raw) To: chris; +Cc: linux-kernel, linux-pm On 06/10/2016 07:35 AM, Chris Lapa wrote: > Hi Krzysztof, > > Thanks for the review. I'm working on those changes now. > > However just so I know for the future. Why no error checking on > devm_kzalloc() result? Looking through the source for devm_kzalloc() it > looks like NULL isn't caught anywhere else. Error checking is necessary. Just do not print the error message. The kernel core will print one with full back trace. if (charger == NULL) return -ENOMEM; Best regards, Krzysztof ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 1/2] max8903: adds support for initiation via device tree. 2016-06-10 6:13 ` Krzysztof Kozlowski @ 2016-06-10 10:23 ` Krzysztof Kozlowski 0 siblings, 0 replies; 68+ messages in thread From: Krzysztof Kozlowski @ 2016-06-10 10:23 UTC (permalink / raw) To: chris; +Cc: linux-kernel, linux-pm On 06/10/2016 08:13 AM, Krzysztof Kozlowski wrote: > On 06/10/2016 07:35 AM, Chris Lapa wrote: >> Hi Krzysztof, >> >> Thanks for the review. I'm working on those changes now. >> >> However just so I know for the future. Why no error checking on >> devm_kzalloc() result? Looking through the source for devm_kzalloc() it >> looks like NULL isn't caught anywhere else. > > Error checking is necessary. Just do not print the error message. The > kernel core will print one with full back trace. > > if (charger == NULL) > return -ENOMEM; ... and while at it just convert it to simpler: if (!charger) return -ENOMEM; BR, Krzysztof ^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH 2/2] max8903: cleans up confusing relationship between dc_valid, dok and dcm. 2016-06-02 6:44 [PATCH 0/2] max8903: Add device tree support and logic fixup chris 2016-06-02 6:44 ` [PATCH 1/2] max8903: adds support for initiation via device tree chris @ 2016-06-02 6:44 ` chris 2016-06-10 12:32 ` [PATCH v2 0/4] max8903: Add device tree support and logic fixup Chris Lapa ` (3 more replies) 1 sibling, 4 replies; 68+ messages in thread From: chris @ 2016-06-02 6:44 UTC (permalink / raw) To: linux-kernel, linux-pm; +Cc: Chris Lapa From: Chris Lapa <chris@lapa.com.au> The max8903_charger.h file indicated that dcm and dok were not optional when dc_valid is set. It makes sense to have dok as a compulsory pin when dc_valid is given. However dcm can be optionally wired to a fixed level especially when the circuit is configured for dc power exclusively. The previous implementation already allowed for this somewhat, however no error was given if dok wasn't given whilst dc_valid was. The new implementation enforces dok presence when dc_valid is given. Whilst allowing dcm to be optional. Signed-off-by: Chris Lapa <chris@lapa.com.au> --- drivers/power/max8903_charger.c | 40 ++++++++++++----------------------- include/linux/power/max8903_charger.h | 6 +++--- 2 files changed, 17 insertions(+), 29 deletions(-) diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c index 1989c10..d3c09f9 100644 --- a/drivers/power/max8903_charger.c +++ b/drivers/power/max8903_charger.c @@ -290,8 +290,7 @@ static int max8903_probe(struct platform_device *pdev) } if (charger->pdata->dc_valid) { - if (charger->pdata->dok && gpio_is_valid(charger->pdata->dok) && - charger->pdata->dcm && gpio_is_valid(charger->pdata->dcm)) { + if (charger->pdata->dok && gpio_is_valid(charger->pdata->dok)) { ret = devm_gpio_request(dev, charger->pdata->dok, charger->psy_desc.name); @@ -302,6 +301,17 @@ static int max8903_probe(struct platform_device *pdev) return -EINVAL; } + gpio = charger->pdata->dok; /* PULL_UPed Interrupt */ + ta_in = gpio_get_value(gpio) ? 0 : 1; + } else { + dev_err(dev, "When DC is wired, DOK should" + " be wired as well.\n"); + return -EINVAL; + } + } + + if (charger->pdata->dcm) { + if (gpio_is_valid(charger->pdata->dcm)) { ret = devm_gpio_request(dev, charger->pdata->dcm, charger->psy_desc.name); @@ -312,35 +322,13 @@ static int max8903_probe(struct platform_device *pdev) return -EINVAL; } - gpio = pdata->dok; /* PULL_UPed Interrupt */ - ta_in = gpio_get_value(gpio) ? 0 : 1; - gpio = pdata->dcm; /* Output */ + gpio = charger->pdata->dcm; /* Output */ gpio_set_value(gpio, ta_in); } else { - dev_err(dev, "When DC is wired, DOK and DCM should" - " be wired as well.\n"); + dev_err(dev, "Invalid pin: dcm.\n"); return -EINVAL; } - } else { - if (pdata->dcm) { - if (gpio_is_valid(pdata->dcm)) { - ret = devm_gpio_request(dev, - charger->pdata->dcm, - charger->psy_desc.name); - if (ret) { - dev_err(dev, - "Failed GPIO request for dcm: %d err %d\n", - charger->pdata->dcm, ret); - return -EINVAL; - } - - gpio_set_value(pdata->dcm, 0); - } else { - dev_err(dev, "Invalid pin: dcm.\n"); - return -EINVAL; - } - } } if (charger->pdata->usb_valid) { diff --git a/include/linux/power/max8903_charger.h b/include/linux/power/max8903_charger.h index 24f51db..89d3f1c 100644 --- a/include/linux/power/max8903_charger.h +++ b/include/linux/power/max8903_charger.h @@ -26,8 +26,8 @@ struct max8903_pdata { /* * GPIOs - * cen, chg, flt, and usus are optional. - * dok, dcm, and uok are not optional depending on the status of + * cen, chg, flt, dcm and usus are optional. + * dok and uok are not optional depending on the status of * dc_valid and usb_valid. */ int cen; /* Charger Enable input */ @@ -41,7 +41,7 @@ struct max8903_pdata { /* * DC(Adapter/TA) is wired * When dc_valid is true, - * dok and dcm should be valid. + * dok should be valid. * * At least one of dc_valid or usb_valid should be true. */ -- 1.9.1 ^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH v2 0/4] max8903: Add device tree support and logic fixup 2016-06-02 6:44 ` [PATCH 2/2] max8903: cleans up confusing relationship between dc_valid, dok and dcm chris @ 2016-06-10 12:32 ` Chris Lapa 2016-06-10 12:32 ` [PATCH v2 1/4] max8903: adds documentation for device tree bindings Chris Lapa ` (3 more replies) 2016-06-17 5:00 ` [PATCH v3 0/7] max8903: Add device tree support and misc fixes Chris Lapa ` (2 subsequent siblings) 3 siblings, 4 replies; 68+ messages in thread From: Chris Lapa @ 2016-06-10 12:32 UTC (permalink / raw) To: k.kozlowski; +Cc: devicetree, linux-kernel, linux-pm, Chris Lapa From: Chris Lapa <chris@lapa.com.au> This patch set adds device tree support for the MAX8903 battery charger and also cleans up the logic with the dc_valid, dok and dcm pins. I verified these patches work on a board I have here, which uses the DC power side (not the USB portition) of the MAX8903. Chris Lapa (4): max8903: adds documentation for device tree bindings. max8903: adds support for initiation via device tree. max8903: cleans up confusing relationship between dc_valid, dok and dcm. max8903: remove unnecessary malloc failed message print out. .../devicetree/bindings/power/max8903-charger.txt | 30 +++ drivers/power/max8903_charger.c | 284 ++++++++++++++++----- include/linux/power/max8903_charger.h | 6 +- 3 files changed, 248 insertions(+), 72 deletions(-) create mode 100644 Documentation/devicetree/bindings/power/max8903-charger.txt -- 1.9.1 ^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH v2 1/4] max8903: adds documentation for device tree bindings. 2016-06-10 12:32 ` [PATCH v2 0/4] max8903: Add device tree support and logic fixup Chris Lapa @ 2016-06-10 12:32 ` Chris Lapa 2016-06-10 13:51 ` Krzysztof Kozlowski 2016-06-10 12:32 ` [PATCH v2 2/4] max8903: adds support for initiation via device tree Chris Lapa ` (2 subsequent siblings) 3 siblings, 1 reply; 68+ messages in thread From: Chris Lapa @ 2016-06-10 12:32 UTC (permalink / raw) To: k.kozlowski; +Cc: devicetree, linux-kernel, linux-pm, Chris Lapa From: Chris Lapa <chris@lapa.com.au> Signed-off-by: Chris Lapa <chris@lapa.com.au> --- .../devicetree/bindings/power/max8903-charger.txt | 30 ++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/max8903-charger.txt diff --git a/Documentation/devicetree/bindings/power/max8903-charger.txt b/Documentation/devicetree/bindings/power/max8903-charger.txt new file mode 100644 index 0000000..e0b5366 --- /dev/null +++ b/Documentation/devicetree/bindings/power/max8903-charger.txt @@ -0,0 +1,30 @@ +Maxim Semiconductor MAX8903 Battery Charger bindings + +Required properties: +- compatible: "maxim,max8903-charger" for MAX8903 Battery Charger +- dc_valid: Specifies that the DC portion of the MAX8903 has been connected up + and that dok-gpios should be specified + - dok-gpios: Valid DC power has been detected +- usb_valid: Specifies that the USB portion of the MAX8903 has been connected up + and that uok-gpios should be specified + - uok-gpios: Valid USB power has been detected + +Optional properties: +- cen-gpios: Charge enable pin +- chg-gpios: Charger status pin +- flt-gpios: Fault pin +- dcm-gpios: Current limit mode setting (DC or USB) +- usus-gpios: USB suspend pin + + +Example: + + max8903-charger { + compatible = "maxim,max8903-charger"; + dok-gpios = <&gpio2 3 GPIO_ACTIVE_LOW>; + flt-gpios = <&gpio2 2 GPIO_ACTIVE_LOW>; + chg-gpios = <&gpio3 15 GPIO_ACTIVE_LOW>; + cen-gpios = <&gpio2 5 GPIO_ACTIVE_LOW>; + dc_valid; + status = "okay"; + }; -- 1.9.1 ^ permalink raw reply related [flat|nested] 68+ messages in thread
* Re: [PATCH v2 1/4] max8903: adds documentation for device tree bindings. 2016-06-10 12:32 ` [PATCH v2 1/4] max8903: adds documentation for device tree bindings Chris Lapa @ 2016-06-10 13:51 ` Krzysztof Kozlowski 2016-06-14 1:54 ` Chris Lapa 0 siblings, 1 reply; 68+ messages in thread From: Krzysztof Kozlowski @ 2016-06-10 13:51 UTC (permalink / raw) To: Chris Lapa; +Cc: devicetree, linux-kernel, linux-pm On 06/10/2016 02:32 PM, Chris Lapa wrote: > From: Chris Lapa <chris@lapa.com.au> > > Signed-off-by: Chris Lapa <chris@lapa.com.au> > --- > .../devicetree/bindings/power/max8903-charger.txt | 30 ++++++++++++++++++++++ > 1 file changed, 30 insertions(+) > create mode 100644 Documentation/devicetree/bindings/power/max8903-charger.txt You again skipped all of the maintainers. Your patch won't be applied. You need to send the patch to the appropriate person so he/she could apply it. >From this patch: Rob Herring <robh+dt@kernel.org> (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,commit_signer:9/19=47%) Mark Rutland <mark.rutland@arm.com> (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS) >From power suppyy tree: Sebastian Reichel <sre@kernel.org> (maintainer:POWER SUPPLY CLASS/SUBSYSTEM and DRIVERS) Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> (maintainer:POWER SUPPLY CLASS/SUBSYSTEM and DRIVERS) David Woodhouse <dwmw2@infradead.org> (maintainer:POWER SUPPLY CLASS/SUBSYSTEM and DRIVERS) > diff --git a/Documentation/devicetree/bindings/power/max8903-charger.txt b/Documentation/devicetree/bindings/power/max8903-charger.txt > new file mode 100644 > index 0000000..e0b5366 > --- /dev/null > +++ b/Documentation/devicetree/bindings/power/max8903-charger.txt > @@ -0,0 +1,30 @@ > +Maxim Semiconductor MAX8903 Battery Charger bindings > + > +Required properties: > +- compatible: "maxim,max8903-charger" for MAX8903 Battery Charger > +- dc_valid: Specifies that the DC portion of the MAX8903 has been connected up > + and that dok-gpios should be specified You don't need the dc_valid nor the usb_valid. If the dok-gpios are present - use DC power. If the uok-gpios - USB power. If both or none: print error. BTW, It would be nice if send also the user of this bindings - DTS/DTSI. Best regards, Krzysztof > + - dok-gpios: Valid DC power has been detected > +- usb_valid: Specifies that the USB portion of the MAX8903 has been connected up > + and that uok-gpios should be specified > + - uok-gpios: Valid USB power has been detected > + > +Optional properties: > +- cen-gpios: Charge enable pin > +- chg-gpios: Charger status pin > +- flt-gpios: Fault pin > +- dcm-gpios: Current limit mode setting (DC or USB) > +- usus-gpios: USB suspend pin > + > + > +Example: > + > + max8903-charger { > + compatible = "maxim,max8903-charger"; > + dok-gpios = <&gpio2 3 GPIO_ACTIVE_LOW>; > + flt-gpios = <&gpio2 2 GPIO_ACTIVE_LOW>; > + chg-gpios = <&gpio3 15 GPIO_ACTIVE_LOW>; > + cen-gpios = <&gpio2 5 GPIO_ACTIVE_LOW>; > + dc_valid; > + status = "okay"; > + }; > ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH v2 1/4] max8903: adds documentation for device tree bindings. 2016-06-10 13:51 ` Krzysztof Kozlowski @ 2016-06-14 1:54 ` Chris Lapa 2016-06-16 6:35 ` Krzysztof Kozlowski 0 siblings, 1 reply; 68+ messages in thread From: Chris Lapa @ 2016-06-14 1:54 UTC (permalink / raw) To: Krzysztof Kozlowski; +Cc: devicetree, linux-kernel, linux-pm On 10/06/2016 11:51 PM, Krzysztof Kozlowski wrote: > On 06/10/2016 02:32 PM, Chris Lapa wrote: >> From: Chris Lapa <chris@lapa.com.au> >> >> Signed-off-by: Chris Lapa <chris@lapa.com.au> >> --- >> .../devicetree/bindings/power/max8903-charger.txt | 30 ++++++++++++++++++++++ >> 1 file changed, 30 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/power/max8903-charger.txt > > You again skipped all of the maintainers. Your patch won't be applied. > You need to send the patch to the appropriate person so he/she could > apply it. > > From this patch: > Rob Herring <robh+dt@kernel.org> (maintainer:OPEN FIRMWARE AND FLATTENED > DEVICE TREE BINDINGS,commit_signer:9/19=47%) > Mark Rutland <mark.rutland@arm.com> (maintainer:OPEN FIRMWARE AND > FLATTENED DEVICE TREE BINDINGS) > > From power suppyy tree: > Sebastian Reichel <sre@kernel.org> (maintainer:POWER SUPPLY > CLASS/SUBSYSTEM and DRIVERS) > Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> (maintainer:POWER SUPPLY > CLASS/SUBSYSTEM and DRIVERS) > David Woodhouse <dwmw2@infradead.org> (maintainer:POWER SUPPLY > CLASS/SUBSYSTEM and DRIVERS) > > >> diff --git a/Documentation/devicetree/bindings/power/max8903-charger.txt b/Documentation/devicetree/bindings/power/max8903-charger.txt >> new file mode 100644 >> index 0000000..e0b5366 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/power/max8903-charger.txt >> @@ -0,0 +1,30 @@ >> +Maxim Semiconductor MAX8903 Battery Charger bindings >> + >> +Required properties: >> +- compatible: "maxim,max8903-charger" for MAX8903 Battery Charger >> +- dc_valid: Specifies that the DC portion of the MAX8903 has been connected up >> + and that dok-gpios should be specified > > You don't need the dc_valid nor the usb_valid. If the dok-gpios are > present - use DC power. If the uok-gpios - USB power. > I did think that as well, but I didn't want to break backwards compatibility. > If both or none: print error. Its valid to have both connected. > > BTW, It would be nice if send also the user of this bindings - DTS/DTSI. I'm not sure I understand what you mean here? > > Best regards, > Krzysztof > >> + - dok-gpios: Valid DC power has been detected >> +- usb_valid: Specifies that the USB portion of the MAX8903 has been connected up >> + and that uok-gpios should be specified >> + - uok-gpios: Valid USB power has been detected >> + >> +Optional properties: >> +- cen-gpios: Charge enable pin >> +- chg-gpios: Charger status pin >> +- flt-gpios: Fault pin >> +- dcm-gpios: Current limit mode setting (DC or USB) >> +- usus-gpios: USB suspend pin >> + >> + >> +Example: >> + >> + max8903-charger { >> + compatible = "maxim,max8903-charger"; >> + dok-gpios = <&gpio2 3 GPIO_ACTIVE_LOW>; >> + flt-gpios = <&gpio2 2 GPIO_ACTIVE_LOW>; >> + chg-gpios = <&gpio3 15 GPIO_ACTIVE_LOW>; >> + cen-gpios = <&gpio2 5 GPIO_ACTIVE_LOW>; >> + dc_valid; >> + status = "okay"; >> + }; >> > ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH v2 1/4] max8903: adds documentation for device tree bindings. 2016-06-14 1:54 ` Chris Lapa @ 2016-06-16 6:35 ` Krzysztof Kozlowski 2016-06-16 6:48 ` Chris Lapa 0 siblings, 1 reply; 68+ messages in thread From: Krzysztof Kozlowski @ 2016-06-16 6:35 UTC (permalink / raw) To: chris; +Cc: devicetree, linux-kernel, linux-pm On 06/14/2016 03:54 AM, Chris Lapa wrote: > On 10/06/2016 11:51 PM, Krzysztof Kozlowski wrote: >> On 06/10/2016 02:32 PM, Chris Lapa wrote: >>> From: Chris Lapa <chris@lapa.com.au> >>> >>> Signed-off-by: Chris Lapa <chris@lapa.com.au> >>> --- >>> .../devicetree/bindings/power/max8903-charger.txt | 30 >>> ++++++++++++++++++++++ >>> 1 file changed, 30 insertions(+) >>> create mode 100644 >>> Documentation/devicetree/bindings/power/max8903-charger.txt >> >> You again skipped all of the maintainers. Your patch won't be applied. >> You need to send the patch to the appropriate person so he/she could >> apply it. >> >> From this patch: >> Rob Herring <robh+dt@kernel.org> (maintainer:OPEN FIRMWARE AND FLATTENED >> DEVICE TREE BINDINGS,commit_signer:9/19=47%) >> Mark Rutland <mark.rutland@arm.com> (maintainer:OPEN FIRMWARE AND >> FLATTENED DEVICE TREE BINDINGS) >> >> From power suppyy tree: >> Sebastian Reichel <sre@kernel.org> (maintainer:POWER SUPPLY >> CLASS/SUBSYSTEM and DRIVERS) >> Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> (maintainer:POWER SUPPLY >> CLASS/SUBSYSTEM and DRIVERS) >> David Woodhouse <dwmw2@infradead.org> (maintainer:POWER SUPPLY >> CLASS/SUBSYSTEM and DRIVERS) >> >> >>> diff --git >>> a/Documentation/devicetree/bindings/power/max8903-charger.txt >>> b/Documentation/devicetree/bindings/power/max8903-charger.txt >>> new file mode 100644 >>> index 0000000..e0b5366 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/power/max8903-charger.txt >>> @@ -0,0 +1,30 @@ >>> +Maxim Semiconductor MAX8903 Battery Charger bindings >>> + >>> +Required properties: >>> +- compatible: "maxim,max8903-charger" for MAX8903 Battery Charger >>> +- dc_valid: Specifies that the DC portion of the MAX8903 has been >>> connected up >>> + and that dok-gpios should be specified >> >> You don't need the dc_valid nor the usb_valid. If the dok-gpios are >> present - use DC power. If the uok-gpios - USB power. >> > I did think that as well, but I didn't want to break backwards > compatibility. What do you mean by backwards compatibility? The driver did not support DeviceTree before so it is not about DT compatibility. If you think about platform data then it is different: how you match DT into driver's structure is up to you. > >> If both or none: print error. > Its valid to have both connected. Ah, okay. Just an example. > >> >> BTW, It would be nice if send also the user of this bindings - DTS/DTSI. > I'm not sure I understand what you mean here? I mean I would like to see the DTS file which is using this device and these bindings. It is not necessary but it would be nice. Best regards, Krzysztof > >> >> Best regards, >> Krzysztof >> >>> + - dok-gpios: Valid DC power has been detected >>> +- usb_valid: Specifies that the USB portion of the MAX8903 has been >>> connected up >>> + and that uok-gpios should be specified >>> + - uok-gpios: Valid USB power has been detected >>> + >>> +Optional properties: >>> +- cen-gpios: Charge enable pin >>> +- chg-gpios: Charger status pin >>> +- flt-gpios: Fault pin >>> +- dcm-gpios: Current limit mode setting (DC or USB) >>> +- usus-gpios: USB suspend pin >>> + >>> + >>> +Example: >>> + >>> + max8903-charger { >>> + compatible = "maxim,max8903-charger"; >>> + dok-gpios = <&gpio2 3 GPIO_ACTIVE_LOW>; >>> + flt-gpios = <&gpio2 2 GPIO_ACTIVE_LOW>; >>> + chg-gpios = <&gpio3 15 GPIO_ACTIVE_LOW>; >>> + cen-gpios = <&gpio2 5 GPIO_ACTIVE_LOW>; >>> + dc_valid; >>> + status = "okay"; >>> + }; >>> >> > > > ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH v2 1/4] max8903: adds documentation for device tree bindings. 2016-06-16 6:35 ` Krzysztof Kozlowski @ 2016-06-16 6:48 ` Chris Lapa 0 siblings, 0 replies; 68+ messages in thread From: Chris Lapa @ 2016-06-16 6:48 UTC (permalink / raw) To: Krzysztof Kozlowski; +Cc: devicetree, linux-kernel, linux-pm On 16/06/2016 4:35 PM, Krzysztof Kozlowski wrote: > On 06/14/2016 03:54 AM, Chris Lapa wrote: >> On 10/06/2016 11:51 PM, Krzysztof Kozlowski wrote: >>> On 06/10/2016 02:32 PM, Chris Lapa wrote: >>>> From: Chris Lapa <chris@lapa.com.au> >>>> >>>> Signed-off-by: Chris Lapa <chris@lapa.com.au> >>>> --- >>>> .../devicetree/bindings/power/max8903-charger.txt | 30 >>>> ++++++++++++++++++++++ >>>> 1 file changed, 30 insertions(+) >>>> create mode 100644 >>>> Documentation/devicetree/bindings/power/max8903-charger.txt >>> >>> You again skipped all of the maintainers. Your patch won't be applied. >>> You need to send the patch to the appropriate person so he/she could >>> apply it. >>> >>> From this patch: >>> Rob Herring <robh+dt@kernel.org> (maintainer:OPEN FIRMWARE AND FLATTENED >>> DEVICE TREE BINDINGS,commit_signer:9/19=47%) >>> Mark Rutland <mark.rutland@arm.com> (maintainer:OPEN FIRMWARE AND >>> FLATTENED DEVICE TREE BINDINGS) >>> >>> From power suppyy tree: >>> Sebastian Reichel <sre@kernel.org> (maintainer:POWER SUPPLY >>> CLASS/SUBSYSTEM and DRIVERS) >>> Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> (maintainer:POWER SUPPLY >>> CLASS/SUBSYSTEM and DRIVERS) >>> David Woodhouse <dwmw2@infradead.org> (maintainer:POWER SUPPLY >>> CLASS/SUBSYSTEM and DRIVERS) >>> >>> >>>> diff --git >>>> a/Documentation/devicetree/bindings/power/max8903-charger.txt >>>> b/Documentation/devicetree/bindings/power/max8903-charger.txt >>>> new file mode 100644 >>>> index 0000000..e0b5366 >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/power/max8903-charger.txt >>>> @@ -0,0 +1,30 @@ >>>> +Maxim Semiconductor MAX8903 Battery Charger bindings >>>> + >>>> +Required properties: >>>> +- compatible: "maxim,max8903-charger" for MAX8903 Battery Charger >>>> +- dc_valid: Specifies that the DC portion of the MAX8903 has been >>>> connected up >>>> + and that dok-gpios should be specified >>> >>> You don't need the dc_valid nor the usb_valid. If the dok-gpios are >>> present - use DC power. If the uok-gpios - USB power. >>> >> I did think that as well, but I didn't want to break backwards >> compatibility. > > What do you mean by backwards compatibility? The driver did not support > DeviceTree before so it is not about DT compatibility. If you think > about platform data then it is different: how you match DT into driver's > structure is up to you. Ah okay, I misunderstood you here. I thought you meant remove usb_valid and dc_valid from the platform data. However determining usb and dc validity from the gpios set in the DT does make more sense. I will do that! > > >> >>> If both or none: print error. >> Its valid to have both connected. > > Ah, okay. Just an example. > >> >>> >>> BTW, It would be nice if send also the user of this bindings - DTS/DTSI. >> I'm not sure I understand what you mean here? The DTSI file used is almost identical to the copy I included in the bindings document. Other then some extra pinmux stuff. Thanks, Chris > > I mean I would like to see the DTS file which is using this device and > these bindings. It is not necessary but it would be nice. > > Best regards, > Krzysztof > >> >>> >>> Best regards, >>> Krzysztof >>> >>>> + - dok-gpios: Valid DC power has been detected >>>> +- usb_valid: Specifies that the USB portion of the MAX8903 has been >>>> connected up >>>> + and that uok-gpios should be specified >>>> + - uok-gpios: Valid USB power has been detected >>>> + >>>> +Optional properties: >>>> +- cen-gpios: Charge enable pin >>>> +- chg-gpios: Charger status pin >>>> +- flt-gpios: Fault pin >>>> +- dcm-gpios: Current limit mode setting (DC or USB) >>>> +- usus-gpios: USB suspend pin >>>> + >>>> + >>>> +Example: >>>> + >>>> + max8903-charger { >>>> + compatible = "maxim,max8903-charger"; >>>> + dok-gpios = <&gpio2 3 GPIO_ACTIVE_LOW>; >>>> + flt-gpios = <&gpio2 2 GPIO_ACTIVE_LOW>; >>>> + chg-gpios = <&gpio3 15 GPIO_ACTIVE_LOW>; >>>> + cen-gpios = <&gpio2 5 GPIO_ACTIVE_LOW>; >>>> + dc_valid; >>>> + status = "okay"; >>>> + }; >>>> >>> >> >> >> > ^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH v2 2/4] max8903: adds support for initiation via device tree. 2016-06-10 12:32 ` [PATCH v2 0/4] max8903: Add device tree support and logic fixup Chris Lapa 2016-06-10 12:32 ` [PATCH v2 1/4] max8903: adds documentation for device tree bindings Chris Lapa @ 2016-06-10 12:32 ` Chris Lapa 2016-06-10 14:02 ` Krzysztof Kozlowski 2016-06-10 12:32 ` [PATCH v2 3/4] max8903: cleans up confusing relationship between dc_valid, dok and dcm Chris Lapa 2016-06-10 12:32 ` [PATCH v2 4/4] max8903: remove unnecessary malloc failed message print out Chris Lapa 3 siblings, 1 reply; 68+ messages in thread From: Chris Lapa @ 2016-06-10 12:32 UTC (permalink / raw) To: k.kozlowski; +Cc: devicetree, linux-kernel, linux-pm, Chris Lapa From: Chris Lapa <chris@lapa.com.au> This commit also adds requesting gpio's via devm_gpio_request() to ensure the gpio is available for usage by the driver. Signed-off-by: Chris Lapa <chris@lapa.com.au> --- drivers/power/max8903_charger.c | 288 +++++++++++++++++++++++++++++++--------- 1 file changed, 225 insertions(+), 63 deletions(-) diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c index 17876ca..d7544c8 100644 --- a/drivers/power/max8903_charger.c +++ b/drivers/power/max8903_charger.c @@ -23,13 +23,16 @@ #include <linux/gpio.h> #include <linux/interrupt.h> #include <linux/module.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/of_gpio.h> #include <linux/slab.h> #include <linux/power_supply.h> #include <linux/platform_device.h> #include <linux/power/max8903_charger.h> struct max8903_data { - struct max8903_pdata pdata; + struct max8903_pdata *pdata; struct device *dev; struct power_supply *psy; struct power_supply_desc psy_desc; @@ -53,8 +56,8 @@ static int max8903_get_property(struct power_supply *psy, switch (psp) { case POWER_SUPPLY_PROP_STATUS: val->intval = POWER_SUPPLY_STATUS_UNKNOWN; - if (data->pdata.chg) { - if (gpio_get_value(data->pdata.chg) == 0) + if (data->pdata->chg) { + if (gpio_get_value(data->pdata->chg) == 0) val->intval = POWER_SUPPLY_STATUS_CHARGING; else if (data->usb_in || data->ta_in) val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING; @@ -75,13 +78,14 @@ static int max8903_get_property(struct power_supply *psy, default: return -EINVAL; } + return 0; } static irqreturn_t max8903_dcin(int irq, void *_data) { struct max8903_data *data = _data; - struct max8903_pdata *pdata = &data->pdata; + struct max8903_pdata *pdata = data->pdata; bool ta_in; enum power_supply_type old_type; @@ -122,7 +126,7 @@ static irqreturn_t max8903_dcin(int irq, void *_data) static irqreturn_t max8903_usbin(int irq, void *_data) { struct max8903_data *data = _data; - struct max8903_pdata *pdata = &data->pdata; + struct max8903_pdata *pdata = data->pdata; bool usb_in; enum power_supply_type old_type; @@ -161,7 +165,7 @@ static irqreturn_t max8903_usbin(int irq, void *_data) static irqreturn_t max8903_fault(int irq, void *_data) { struct max8903_data *data = _data; - struct max8903_pdata *pdata = &data->pdata; + struct max8903_pdata *pdata = data->pdata; bool fault; fault = gpio_get_value(pdata->flt) ? false : true; @@ -179,38 +183,132 @@ static irqreturn_t max8903_fault(int irq, void *_data) return IRQ_HANDLED; } +static struct max8903_pdata *max8903_parse_dt_data( + struct device *dev) +{ + struct device_node *of_node = dev->of_node; + struct max8903_pdata *pdata = NULL; + + if (!of_node) + return pdata; + + + pdata = devm_kzalloc(dev, sizeof(struct max8903_pdata), + GFP_KERNEL); + if (!pdata) + return pdata; + + if (of_get_property(of_node, "dc_valid", NULL)) + pdata->dc_valid = true; + + if (of_get_property(of_node, "usb_valid", NULL)) + pdata->usb_valid = true; + + pdata->cen = of_get_named_gpio(of_node, "cen-gpios", 0); + if (!gpio_is_valid(pdata->cen)) + pdata->cen = 0; + + pdata->chg = of_get_named_gpio(of_node, "chg-gpios", 0); + if (!gpio_is_valid(pdata->chg)) + pdata->chg = 0; + + pdata->flt = of_get_named_gpio(of_node, "flt-gpios", 0); + if (!gpio_is_valid(pdata->flt)) + pdata->flt = 0; + + pdata->usus = of_get_named_gpio(of_node, "usus-gpios", 0); + if (!gpio_is_valid(pdata->usus)) + pdata->usus = 0; + + pdata->dcm = of_get_named_gpio(of_node, "dcm-gpios", 0); + if (!gpio_is_valid(pdata->dcm)) + pdata->dcm = 0; + + pdata->dok = of_get_named_gpio(of_node, "dok-gpios", 0); + if (!gpio_is_valid(pdata->dok)) + pdata->dok = 0; + + pdata->uok = of_get_named_gpio(of_node, "uok-gpios", 0); + if (!gpio_is_valid(pdata->uok)) + pdata->uok = 0; + + return pdata; +} + static int max8903_probe(struct platform_device *pdev) { - struct max8903_data *data; + struct max8903_data *charger; struct device *dev = &pdev->dev; - struct max8903_pdata *pdata = pdev->dev.platform_data; struct power_supply_config psy_cfg = {}; int ret = 0; int gpio; int ta_in = 0; int usb_in = 0; - data = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL); - if (data == NULL) { + charger = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL); + if (charger == NULL) { dev_err(dev, "Cannot allocate memory.\n"); return -ENOMEM; } - memcpy(&data->pdata, pdata, sizeof(struct max8903_pdata)); - data->dev = dev; - platform_set_drvdata(pdev, data); - if (pdata->dc_valid == false && pdata->usb_valid == false) { + charger->pdata = pdev->dev.platform_data; + if (IS_ENABLED(CONFIG_OF) && !charger->pdata && dev->of_node) { + charger->pdata = max8903_parse_dt_data(dev); + if (!charger->pdata) + return -EINVAL; + } + + charger->dev = dev; + + platform_set_drvdata(pdev, charger); + + charger->fault = false; + charger->ta_in = ta_in; + charger->usb_in = usb_in; + + charger->psy_desc.name = "max8903_charger"; + charger->psy_desc.type = (ta_in) ? POWER_SUPPLY_TYPE_MAINS : + ((usb_in) ? POWER_SUPPLY_TYPE_USB : + POWER_SUPPLY_TYPE_BATTERY); + charger->psy_desc.get_property = max8903_get_property; + charger->psy_desc.properties = max8903_charger_props; + charger->psy_desc.num_properties = ARRAY_SIZE(max8903_charger_props); + + if (charger->pdata->dc_valid == false && + charger->pdata->usb_valid == false) { dev_err(dev, "No valid power sources.\n"); return -EINVAL; } - if (pdata->dc_valid) { - if (pdata->dok && gpio_is_valid(pdata->dok) && - pdata->dcm && gpio_is_valid(pdata->dcm)) { - gpio = pdata->dok; /* PULL_UPed Interrupt */ + if (charger->pdata->dc_valid) { + if (charger->pdata->dok && + gpio_is_valid(charger->pdata->dok) && + charger->pdata->dcm && + gpio_is_valid(charger->pdata->dcm)) { + ret = devm_gpio_request(dev, + charger->pdata->dok, + charger->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for dok: %d err %d\n", + charger->pdata->dok, ret); + return -EINVAL; + } + + ret = devm_gpio_request(dev, + charger->pdata->dcm, + charger->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for dcm: %d err %d\n", + charger->pdata->dcm, ret); + return -EINVAL; + } + + gpio = charger->pdata->dok; /* PULL_UPed Interrupt */ ta_in = gpio_get_value(gpio) ? 0 : 1; - gpio = pdata->dcm; /* Output */ + gpio = charger->pdata->dcm; /* Output */ gpio_set_value(gpio, ta_in); } else { dev_err(dev, "When DC is wired, DOK and DCM should" @@ -218,19 +316,39 @@ static int max8903_probe(struct platform_device *pdev) return -EINVAL; } } else { - if (pdata->dcm) { - if (gpio_is_valid(pdata->dcm)) - gpio_set_value(pdata->dcm, 0); - else { + if (charger->pdata->dcm) { + if (gpio_is_valid(charger->pdata->dcm)) { + ret = devm_gpio_request(dev, + charger->pdata->dcm, + charger->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for dcm: %d err %d\n", + charger->pdata->dcm, ret); + return -EINVAL; + } + + gpio_set_value(charger->pdata->dcm, 0); + } else { dev_err(dev, "Invalid pin: dcm.\n"); return -EINVAL; } } } - if (pdata->usb_valid) { - if (pdata->uok && gpio_is_valid(pdata->uok)) { - gpio = pdata->uok; + if (charger->pdata->usb_valid) { + if (gpio_is_valid(charger->pdata->uok)) { + ret = devm_gpio_request(dev, + charger->pdata->uok, + charger->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for uok: %d err %d\n", + charger->pdata->uok, ret); + return -EINVAL; + } + + gpio = charger->pdata->uok; usb_in = gpio_get_value(gpio) ? 0 : 1; } else { dev_err(dev, "When USB is wired, UOK should be wired." @@ -239,91 +357,128 @@ static int max8903_probe(struct platform_device *pdev) } } - if (pdata->cen) { - if (gpio_is_valid(pdata->cen)) { - gpio_set_value(pdata->cen, (ta_in || usb_in) ? 0 : 1); + if (charger->pdata->cen) { + if (gpio_is_valid(charger->pdata->cen)) { + ret = devm_gpio_request(dev, + charger->pdata->cen, + charger->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for cen: %d err %d\n", + charger->pdata->cen, ret); + return -EINVAL; + } + + gpio_set_value(charger->pdata->cen, + (ta_in || usb_in) ? 0 : 1); } else { dev_err(dev, "Invalid pin: cen.\n"); return -EINVAL; } } - if (pdata->chg) { - if (!gpio_is_valid(pdata->chg)) { + if (charger->pdata->chg) { + if (gpio_is_valid(charger->pdata->chg)) { + ret = devm_gpio_request(dev, + charger->pdata->chg, + charger->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for chg: %d err %d\n", + charger->pdata->chg, ret); + return -EINVAL; + } + } else { dev_err(dev, "Invalid pin: chg.\n"); return -EINVAL; } } - if (pdata->flt) { - if (!gpio_is_valid(pdata->flt)) { + if (charger->pdata->flt) { + if (gpio_is_valid(charger->pdata->flt)) { + ret = devm_gpio_request(dev, + charger->pdata->flt, + charger->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for flt: %d err %d\n", + charger->pdata->flt, ret); + return -EINVAL; + } + } else { dev_err(dev, "Invalid pin: flt.\n"); return -EINVAL; } } - if (pdata->usus) { - if (!gpio_is_valid(pdata->usus)) { + if (charger->pdata->usus) { + if (gpio_is_valid(charger->pdata->usus)) { + ret = devm_gpio_request(dev, + charger->pdata->usus, + charger->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for usus: %d err %d\n", + charger->pdata->usus, ret); + return -EINVAL; + } + } else { dev_err(dev, "Invalid pin: usus.\n"); return -EINVAL; } } - data->fault = false; - data->ta_in = ta_in; - data->usb_in = usb_in; + psy_cfg.supplied_to = NULL; + psy_cfg.num_supplicants = 0; + psy_cfg.of_node = dev->of_node; + psy_cfg.drv_data = charger; - data->psy_desc.name = "max8903_charger"; - data->psy_desc.type = (ta_in) ? POWER_SUPPLY_TYPE_MAINS : - ((usb_in) ? POWER_SUPPLY_TYPE_USB : - POWER_SUPPLY_TYPE_BATTERY); - data->psy_desc.get_property = max8903_get_property; - data->psy_desc.properties = max8903_charger_props; - data->psy_desc.num_properties = ARRAY_SIZE(max8903_charger_props); - - psy_cfg.drv_data = data; - - data->psy = devm_power_supply_register(dev, &data->psy_desc, &psy_cfg); - if (IS_ERR(data->psy)) { + charger->psy = devm_power_supply_register(dev, + &charger->psy_desc, + &psy_cfg); + if (IS_ERR(charger->psy)) { dev_err(dev, "failed: power supply register.\n"); - return PTR_ERR(data->psy); + return PTR_ERR(charger->psy); } - if (pdata->dc_valid) { - ret = devm_request_threaded_irq(dev, gpio_to_irq(pdata->dok), + if (charger->pdata->dc_valid) { + ret = devm_request_threaded_irq(dev, + gpio_to_irq(charger->pdata->dok), NULL, max8903_dcin, IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING | IRQF_ONESHOT, - "MAX8903 DC IN", data); + "MAX8903 DC IN", charger); if (ret) { dev_err(dev, "Cannot request irq %d for DC (%d)\n", - gpio_to_irq(pdata->dok), ret); + gpio_to_irq(charger->pdata->dok), ret); return ret; } } - if (pdata->usb_valid) { - ret = devm_request_threaded_irq(dev, gpio_to_irq(pdata->uok), + if (charger->pdata->usb_valid) { + ret = devm_request_threaded_irq(dev, + gpio_to_irq(charger->pdata->uok), NULL, max8903_usbin, IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING | IRQF_ONESHOT, - "MAX8903 USB IN", data); + "MAX8903 USB IN", charger); if (ret) { dev_err(dev, "Cannot request irq %d for USB (%d)\n", - gpio_to_irq(pdata->uok), ret); + gpio_to_irq(charger->pdata->uok), ret); return ret; } } - if (pdata->flt) { - ret = devm_request_threaded_irq(dev, gpio_to_irq(pdata->flt), + if (charger->pdata->flt) { + ret = devm_request_threaded_irq(dev, + gpio_to_irq(charger->pdata->flt), NULL, max8903_fault, IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING | IRQF_ONESHOT, - "MAX8903 Fault", data); + "MAX8903 Fault", charger); if (ret) { dev_err(dev, "Cannot request irq %d for Fault (%d)\n", - gpio_to_irq(pdata->flt), ret); + gpio_to_irq(charger->pdata->flt), ret); return ret; } } @@ -331,10 +486,17 @@ static int max8903_probe(struct platform_device *pdev) return 0; } +static const struct of_device_id max8903_match_ids[] = { + { .compatible = "maxim,max8903-charger", }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, max8903_match_ids); + static struct platform_driver max8903_driver = { .probe = max8903_probe, .driver = { .name = "max8903-charger", + .of_match_table = max8903_match_ids }, }; -- 1.9.1 ^ permalink raw reply related [flat|nested] 68+ messages in thread
* Re: [PATCH v2 2/4] max8903: adds support for initiation via device tree. 2016-06-10 12:32 ` [PATCH v2 2/4] max8903: adds support for initiation via device tree Chris Lapa @ 2016-06-10 14:02 ` Krzysztof Kozlowski 2016-06-14 1:50 ` Chris Lapa 0 siblings, 1 reply; 68+ messages in thread From: Krzysztof Kozlowski @ 2016-06-10 14:02 UTC (permalink / raw) To: Chris Lapa; +Cc: devicetree, linux-kernel, linux-pm On 06/10/2016 02:32 PM, Chris Lapa wrote: > From: Chris Lapa <chris@lapa.com.au> > > This commit also adds requesting gpio's via devm_gpio_request() to ensure > the gpio is available for usage by the driver. > > Signed-off-by: Chris Lapa <chris@lapa.com.au> > --- > drivers/power/max8903_charger.c | 288 +++++++++++++++++++++++++++++++--------- > 1 file changed, 225 insertions(+), 63 deletions(-) > > diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c > index 17876ca..d7544c8 100644 > --- a/drivers/power/max8903_charger.c > +++ b/drivers/power/max8903_charger.c > @@ -23,13 +23,16 @@ > #include <linux/gpio.h> > #include <linux/interrupt.h> > #include <linux/module.h> > +#include <linux/of.h> > +#include <linux/of_device.h> > +#include <linux/of_gpio.h> > #include <linux/slab.h> > #include <linux/power_supply.h> > #include <linux/platform_device.h> > #include <linux/power/max8903_charger.h> > > struct max8903_data { > - struct max8903_pdata pdata; > + struct max8903_pdata *pdata; Please split the conversion to '*pdata' to separate patch. It obfuscates a lot the patch so it is difficult to find the DT changes. > struct device *dev; > struct power_supply *psy; > struct power_supply_desc psy_desc; > @@ -53,8 +56,8 @@ static int max8903_get_property(struct power_supply *psy, > switch (psp) { > case POWER_SUPPLY_PROP_STATUS: > val->intval = POWER_SUPPLY_STATUS_UNKNOWN; > - if (data->pdata.chg) { > - if (gpio_get_value(data->pdata.chg) == 0) > + if (data->pdata->chg) { > + if (gpio_get_value(data->pdata->chg) == 0) > val->intval = POWER_SUPPLY_STATUS_CHARGING; > else if (data->usb_in || data->ta_in) > val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING; > @@ -75,13 +78,14 @@ static int max8903_get_property(struct power_supply *psy, > default: > return -EINVAL; > } > + > return 0; > } > > static irqreturn_t max8903_dcin(int irq, void *_data) > { > struct max8903_data *data = _data; > - struct max8903_pdata *pdata = &data->pdata; > + struct max8903_pdata *pdata = data->pdata; > bool ta_in; > enum power_supply_type old_type; > > @@ -122,7 +126,7 @@ static irqreturn_t max8903_dcin(int irq, void *_data) > static irqreturn_t max8903_usbin(int irq, void *_data) > { > struct max8903_data *data = _data; > - struct max8903_pdata *pdata = &data->pdata; > + struct max8903_pdata *pdata = data->pdata; > bool usb_in; > enum power_supply_type old_type; > > @@ -161,7 +165,7 @@ static irqreturn_t max8903_usbin(int irq, void *_data) > static irqreturn_t max8903_fault(int irq, void *_data) > { > struct max8903_data *data = _data; > - struct max8903_pdata *pdata = &data->pdata; > + struct max8903_pdata *pdata = data->pdata; > bool fault; > > fault = gpio_get_value(pdata->flt) ? false : true; > @@ -179,38 +183,132 @@ static irqreturn_t max8903_fault(int irq, void *_data) > return IRQ_HANDLED; > } > > +static struct max8903_pdata *max8903_parse_dt_data( > + struct device *dev) > +{ > + struct device_node *of_node = dev->of_node; > + struct max8903_pdata *pdata = NULL; > + > + if (!of_node) > + return pdata; > + Unnecessary blank line. Just "return NULL", it is easier to read such code (no need to double check if pdata was initialized or not). > + > + pdata = devm_kzalloc(dev, sizeof(struct max8903_pdata), sizeof(*pdata) > + GFP_KERNEL); > + if (!pdata) > + return pdata; Ditto, return NULL. > + > + if (of_get_property(of_node, "dc_valid", NULL)) > + pdata->dc_valid = true; > + > + if (of_get_property(of_node, "usb_valid", NULL)) > + pdata->usb_valid = true; > + > + pdata->cen = of_get_named_gpio(of_node, "cen-gpios", 0); > + if (!gpio_is_valid(pdata->cen)) > + pdata->cen = 0; 0 could be a valid GPIO so probably you want: pdata->cen = -EINVAL; > + > + pdata->chg = of_get_named_gpio(of_node, "chg-gpios", 0); > + if (!gpio_is_valid(pdata->chg)) > + pdata->chg = 0; > + > + pdata->flt = of_get_named_gpio(of_node, "flt-gpios", 0); > + if (!gpio_is_valid(pdata->flt)) > + pdata->flt = 0; > + > + pdata->usus = of_get_named_gpio(of_node, "usus-gpios", 0); > + if (!gpio_is_valid(pdata->usus)) > + pdata->usus = 0; > + > + pdata->dcm = of_get_named_gpio(of_node, "dcm-gpios", 0); > + if (!gpio_is_valid(pdata->dcm)) > + pdata->dcm = 0; > + > + pdata->dok = of_get_named_gpio(of_node, "dok-gpios", 0); > + if (!gpio_is_valid(pdata->dok)) > + pdata->dok = 0; > + > + pdata->uok = of_get_named_gpio(of_node, "uok-gpios", 0); > + if (!gpio_is_valid(pdata->uok)) > + pdata->uok = 0; > + > + return pdata; > +} > + > static int max8903_probe(struct platform_device *pdev) > { > - struct max8903_data *data; > + struct max8903_data *charger; > struct device *dev = &pdev->dev; > - struct max8903_pdata *pdata = pdev->dev.platform_data; > struct power_supply_config psy_cfg = {}; > int ret = 0; > int gpio; > int ta_in = 0; > int usb_in = 0; > > - data = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL); > - if (data == NULL) { > + charger = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL); > + if (charger == NULL) { > dev_err(dev, "Cannot allocate memory.\n"); > return -ENOMEM; > } > - memcpy(&data->pdata, pdata, sizeof(struct max8903_pdata)); > - data->dev = dev; > - platform_set_drvdata(pdev, data); > > - if (pdata->dc_valid == false && pdata->usb_valid == false) { > + charger->pdata = pdev->dev.platform_data; > + if (IS_ENABLED(CONFIG_OF) && !charger->pdata && dev->of_node) { > + charger->pdata = max8903_parse_dt_data(dev); > + if (!charger->pdata) > + return -EINVAL; > + } > + > + charger->dev = dev; > + > + platform_set_drvdata(pdev, charger); > + > + charger->fault = false; > + charger->ta_in = ta_in; > + charger->usb_in = usb_in; > + > + charger->psy_desc.name = "max8903_charger"; > + charger->psy_desc.type = (ta_in) ? POWER_SUPPLY_TYPE_MAINS : > + ((usb_in) ? POWER_SUPPLY_TYPE_USB : > + POWER_SUPPLY_TYPE_BATTERY); > + charger->psy_desc.get_property = max8903_get_property; > + charger->psy_desc.properties = max8903_charger_props; > + charger->psy_desc.num_properties = ARRAY_SIZE(max8903_charger_props); > + > + if (charger->pdata->dc_valid == false && > + charger->pdata->usb_valid == false) { > dev_err(dev, "No valid power sources.\n"); > return -EINVAL; > } > > - if (pdata->dc_valid) { > - if (pdata->dok && gpio_is_valid(pdata->dok) && > - pdata->dcm && gpio_is_valid(pdata->dcm)) { > - gpio = pdata->dok; /* PULL_UPed Interrupt */ > + if (charger->pdata->dc_valid) { > + if (charger->pdata->dok && > + gpio_is_valid(charger->pdata->dok) && > + charger->pdata->dcm && > + gpio_is_valid(charger->pdata->dcm)) { > + ret = devm_gpio_request(dev, > + charger->pdata->dok, > + charger->psy_desc.name); > + if (ret) { > + dev_err(dev, > + "Failed GPIO request for dok: %d err %d\n", > + charger->pdata->dok, ret); > + return -EINVAL; > + } > + > + ret = devm_gpio_request(dev, > + charger->pdata->dcm, > + charger->psy_desc.name); > + if (ret) { > + dev_err(dev, > + "Failed GPIO request for dcm: %d err %d\n", > + charger->pdata->dcm, ret); > + return -EINVAL; > + } > + > + gpio = charger->pdata->dok; /* PULL_UPed Interrupt */ > ta_in = gpio_get_value(gpio) ? 0 : 1; > > - gpio = pdata->dcm; /* Output */ > + gpio = charger->pdata->dcm; /* Output */ > gpio_set_value(gpio, ta_in); > } else { > dev_err(dev, "When DC is wired, DOK and DCM should" > @@ -218,19 +316,39 @@ static int max8903_probe(struct platform_device *pdev) > return -EINVAL; > } > } else { > - if (pdata->dcm) { > - if (gpio_is_valid(pdata->dcm)) > - gpio_set_value(pdata->dcm, 0); > - else { > + if (charger->pdata->dcm) { > + if (gpio_is_valid(charger->pdata->dcm)) { > + ret = devm_gpio_request(dev, > + charger->pdata->dcm, > + charger->psy_desc.name); > + if (ret) { > + dev_err(dev, > + "Failed GPIO request for dcm: %d err %d\n", > + charger->pdata->dcm, ret); > + return -EINVAL; > + } > + > + gpio_set_value(charger->pdata->dcm, 0); > + } else { > dev_err(dev, "Invalid pin: dcm.\n"); > return -EINVAL; > } > } > } > > - if (pdata->usb_valid) { > - if (pdata->uok && gpio_is_valid(pdata->uok)) { > - gpio = pdata->uok; > + if (charger->pdata->usb_valid) { > + if (gpio_is_valid(charger->pdata->uok)) { > + ret = devm_gpio_request(dev, > + charger->pdata->uok, > + charger->psy_desc.name); > + if (ret) { > + dev_err(dev, > + "Failed GPIO request for uok: %d err %d\n", > + charger->pdata->uok, ret); > + return -EINVAL; > + } > + > + gpio = charger->pdata->uok; > usb_in = gpio_get_value(gpio) ? 0 : 1; > } else { > dev_err(dev, "When USB is wired, UOK should be wired." > @@ -239,91 +357,128 @@ static int max8903_probe(struct platform_device *pdev) > } > } > > - if (pdata->cen) { > - if (gpio_is_valid(pdata->cen)) { > - gpio_set_value(pdata->cen, (ta_in || usb_in) ? 0 : 1); > + if (charger->pdata->cen) { > + if (gpio_is_valid(charger->pdata->cen)) { > + ret = devm_gpio_request(dev, > + charger->pdata->cen, > + charger->psy_desc.name); > + if (ret) { > + dev_err(dev, > + "Failed GPIO request for cen: %d err %d\n", > + charger->pdata->cen, ret); > + return -EINVAL; > + } > + > + gpio_set_value(charger->pdata->cen, > + (ta_in || usb_in) ? 0 : 1); > } else { > dev_err(dev, "Invalid pin: cen.\n"); > return -EINVAL; > } > } > > - if (pdata->chg) { > - if (!gpio_is_valid(pdata->chg)) { > + if (charger->pdata->chg) { > + if (gpio_is_valid(charger->pdata->chg)) { > + ret = devm_gpio_request(dev, > + charger->pdata->chg, > + charger->psy_desc.name); > + if (ret) { > + dev_err(dev, > + "Failed GPIO request for chg: %d err %d\n", > + charger->pdata->chg, ret); > + return -EINVAL; > + } > + } else { > dev_err(dev, "Invalid pin: chg.\n"); > return -EINVAL; > } > } > > - if (pdata->flt) { > - if (!gpio_is_valid(pdata->flt)) { > + if (charger->pdata->flt) { > + if (gpio_is_valid(charger->pdata->flt)) { > + ret = devm_gpio_request(dev, > + charger->pdata->flt, > + charger->psy_desc.name); > + if (ret) { > + dev_err(dev, > + "Failed GPIO request for flt: %d err %d\n", > + charger->pdata->flt, ret); > + return -EINVAL; > + } > + } else { > dev_err(dev, "Invalid pin: flt.\n"); > return -EINVAL; > } > } > > - if (pdata->usus) { > - if (!gpio_is_valid(pdata->usus)) { > + if (charger->pdata->usus) { > + if (gpio_is_valid(charger->pdata->usus)) { > + ret = devm_gpio_request(dev, > + charger->pdata->usus, > + charger->psy_desc.name); > + if (ret) { > + dev_err(dev, > + "Failed GPIO request for usus: %d err %d\n", > + charger->pdata->usus, ret); > + return -EINVAL; > + } > + } else { > dev_err(dev, "Invalid pin: usus.\n"); > return -EINVAL; > } > } > > - data->fault = false; > - data->ta_in = ta_in; > - data->usb_in = usb_in; > + psy_cfg.supplied_to = NULL; > + psy_cfg.num_supplicants = 0; Why? This is already set to 0. Additionally this does not look needded for conversion to DT. Please split out all unrelated changes to separate patches. It could be organized as: patch #1: Document DT bindings. patch #2: Some fix needed. patch #3: Some other cleanup patch #4: Store pointer to pdata instead of copying it patch #5: Add support for Device Tree Best regards, Krzysztof ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH v2 2/4] max8903: adds support for initiation via device tree. 2016-06-10 14:02 ` Krzysztof Kozlowski @ 2016-06-14 1:50 ` Chris Lapa 0 siblings, 0 replies; 68+ messages in thread From: Chris Lapa @ 2016-06-14 1:50 UTC (permalink / raw) To: Krzysztof Kozlowski; +Cc: devicetree, linux-kernel, linux-pm Hi Krzysztof, Appreciate the feedback, working on the fixups you pointed out now. I'll ensure I send v3 of the patches to the actual maintainers and not just the list. Still getting my head around the lists mechanics :) Thanks, Chris: On 11/06/2016 12:02 AM, Krzysztof Kozlowski wrote: > On 06/10/2016 02:32 PM, Chris Lapa wrote: >> From: Chris Lapa <chris@lapa.com.au> >> >> This commit also adds requesting gpio's via devm_gpio_request() to ensure >> the gpio is available for usage by the driver. >> >> Signed-off-by: Chris Lapa <chris@lapa.com.au> >> --- >> drivers/power/max8903_charger.c | 288 +++++++++++++++++++++++++++++++--------- >> 1 file changed, 225 insertions(+), 63 deletions(-) >> >> diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c >> index 17876ca..d7544c8 100644 >> --- a/drivers/power/max8903_charger.c >> +++ b/drivers/power/max8903_charger.c >> @@ -23,13 +23,16 @@ >> #include <linux/gpio.h> >> #include <linux/interrupt.h> >> #include <linux/module.h> >> +#include <linux/of.h> >> +#include <linux/of_device.h> >> +#include <linux/of_gpio.h> >> #include <linux/slab.h> >> #include <linux/power_supply.h> >> #include <linux/platform_device.h> >> #include <linux/power/max8903_charger.h> >> >> struct max8903_data { >> - struct max8903_pdata pdata; >> + struct max8903_pdata *pdata; > > Please split the conversion to '*pdata' to separate patch. It obfuscates > a lot the patch so it is difficult to find the DT changes. > >> struct device *dev; >> struct power_supply *psy; >> struct power_supply_desc psy_desc; >> @@ -53,8 +56,8 @@ static int max8903_get_property(struct power_supply *psy, >> switch (psp) { >> case POWER_SUPPLY_PROP_STATUS: >> val->intval = POWER_SUPPLY_STATUS_UNKNOWN; >> - if (data->pdata.chg) { >> - if (gpio_get_value(data->pdata.chg) == 0) >> + if (data->pdata->chg) { >> + if (gpio_get_value(data->pdata->chg) == 0) >> val->intval = POWER_SUPPLY_STATUS_CHARGING; >> else if (data->usb_in || data->ta_in) >> val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING; >> @@ -75,13 +78,14 @@ static int max8903_get_property(struct power_supply *psy, >> default: >> return -EINVAL; >> } >> + >> return 0; >> } >> >> static irqreturn_t max8903_dcin(int irq, void *_data) >> { >> struct max8903_data *data = _data; >> - struct max8903_pdata *pdata = &data->pdata; >> + struct max8903_pdata *pdata = data->pdata; >> bool ta_in; >> enum power_supply_type old_type; >> >> @@ -122,7 +126,7 @@ static irqreturn_t max8903_dcin(int irq, void *_data) >> static irqreturn_t max8903_usbin(int irq, void *_data) >> { >> struct max8903_data *data = _data; >> - struct max8903_pdata *pdata = &data->pdata; >> + struct max8903_pdata *pdata = data->pdata; >> bool usb_in; >> enum power_supply_type old_type; >> >> @@ -161,7 +165,7 @@ static irqreturn_t max8903_usbin(int irq, void *_data) >> static irqreturn_t max8903_fault(int irq, void *_data) >> { >> struct max8903_data *data = _data; >> - struct max8903_pdata *pdata = &data->pdata; >> + struct max8903_pdata *pdata = data->pdata; >> bool fault; >> >> fault = gpio_get_value(pdata->flt) ? false : true; >> @@ -179,38 +183,132 @@ static irqreturn_t max8903_fault(int irq, void *_data) >> return IRQ_HANDLED; >> } >> >> +static struct max8903_pdata *max8903_parse_dt_data( >> + struct device *dev) >> +{ >> + struct device_node *of_node = dev->of_node; >> + struct max8903_pdata *pdata = NULL; >> + >> + if (!of_node) >> + return pdata; >> + > > Unnecessary blank line. > > Just "return NULL", it is easier to read such code (no need to double > check if pdata was initialized or not). > >> + >> + pdata = devm_kzalloc(dev, sizeof(struct max8903_pdata), > > sizeof(*pdata) > >> + GFP_KERNEL); >> + if (!pdata) >> + return pdata; > > Ditto, return NULL. > >> + >> + if (of_get_property(of_node, "dc_valid", NULL)) >> + pdata->dc_valid = true; >> + >> + if (of_get_property(of_node, "usb_valid", NULL)) >> + pdata->usb_valid = true; >> + >> + pdata->cen = of_get_named_gpio(of_node, "cen-gpios", 0); >> + if (!gpio_is_valid(pdata->cen)) >> + pdata->cen = 0; > > 0 could be a valid GPIO so probably you want: > pdata->cen = -EINVAL; > >> + >> + pdata->chg = of_get_named_gpio(of_node, "chg-gpios", 0); >> + if (!gpio_is_valid(pdata->chg)) >> + pdata->chg = 0; >> + >> + pdata->flt = of_get_named_gpio(of_node, "flt-gpios", 0); >> + if (!gpio_is_valid(pdata->flt)) >> + pdata->flt = 0; >> + >> + pdata->usus = of_get_named_gpio(of_node, "usus-gpios", 0); >> + if (!gpio_is_valid(pdata->usus)) >> + pdata->usus = 0; >> + >> + pdata->dcm = of_get_named_gpio(of_node, "dcm-gpios", 0); >> + if (!gpio_is_valid(pdata->dcm)) >> + pdata->dcm = 0; >> + >> + pdata->dok = of_get_named_gpio(of_node, "dok-gpios", 0); >> + if (!gpio_is_valid(pdata->dok)) >> + pdata->dok = 0; >> + >> + pdata->uok = of_get_named_gpio(of_node, "uok-gpios", 0); >> + if (!gpio_is_valid(pdata->uok)) >> + pdata->uok = 0; >> + >> + return pdata; >> +} >> + >> static int max8903_probe(struct platform_device *pdev) >> { >> - struct max8903_data *data; >> + struct max8903_data *charger; >> struct device *dev = &pdev->dev; >> - struct max8903_pdata *pdata = pdev->dev.platform_data; >> struct power_supply_config psy_cfg = {}; >> int ret = 0; >> int gpio; >> int ta_in = 0; >> int usb_in = 0; >> >> - data = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL); >> - if (data == NULL) { >> + charger = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL); >> + if (charger == NULL) { >> dev_err(dev, "Cannot allocate memory.\n"); >> return -ENOMEM; >> } >> - memcpy(&data->pdata, pdata, sizeof(struct max8903_pdata)); >> - data->dev = dev; >> - platform_set_drvdata(pdev, data); >> >> - if (pdata->dc_valid == false && pdata->usb_valid == false) { >> + charger->pdata = pdev->dev.platform_data; >> + if (IS_ENABLED(CONFIG_OF) && !charger->pdata && dev->of_node) { >> + charger->pdata = max8903_parse_dt_data(dev); >> + if (!charger->pdata) >> + return -EINVAL; >> + } >> + >> + charger->dev = dev; >> + >> + platform_set_drvdata(pdev, charger); >> + >> + charger->fault = false; >> + charger->ta_in = ta_in; >> + charger->usb_in = usb_in; >> + >> + charger->psy_desc.name = "max8903_charger"; >> + charger->psy_desc.type = (ta_in) ? POWER_SUPPLY_TYPE_MAINS : >> + ((usb_in) ? POWER_SUPPLY_TYPE_USB : >> + POWER_SUPPLY_TYPE_BATTERY); >> + charger->psy_desc.get_property = max8903_get_property; >> + charger->psy_desc.properties = max8903_charger_props; >> + charger->psy_desc.num_properties = ARRAY_SIZE(max8903_charger_props); >> + >> + if (charger->pdata->dc_valid == false && >> + charger->pdata->usb_valid == false) { >> dev_err(dev, "No valid power sources.\n"); >> return -EINVAL; >> } >> >> - if (pdata->dc_valid) { >> - if (pdata->dok && gpio_is_valid(pdata->dok) && >> - pdata->dcm && gpio_is_valid(pdata->dcm)) { >> - gpio = pdata->dok; /* PULL_UPed Interrupt */ >> + if (charger->pdata->dc_valid) { >> + if (charger->pdata->dok && >> + gpio_is_valid(charger->pdata->dok) && >> + charger->pdata->dcm && >> + gpio_is_valid(charger->pdata->dcm)) { >> + ret = devm_gpio_request(dev, >> + charger->pdata->dok, >> + charger->psy_desc.name); >> + if (ret) { >> + dev_err(dev, >> + "Failed GPIO request for dok: %d err %d\n", >> + charger->pdata->dok, ret); >> + return -EINVAL; >> + } >> + >> + ret = devm_gpio_request(dev, >> + charger->pdata->dcm, >> + charger->psy_desc.name); >> + if (ret) { >> + dev_err(dev, >> + "Failed GPIO request for dcm: %d err %d\n", >> + charger->pdata->dcm, ret); >> + return -EINVAL; >> + } >> + >> + gpio = charger->pdata->dok; /* PULL_UPed Interrupt */ >> ta_in = gpio_get_value(gpio) ? 0 : 1; >> >> - gpio = pdata->dcm; /* Output */ >> + gpio = charger->pdata->dcm; /* Output */ >> gpio_set_value(gpio, ta_in); >> } else { >> dev_err(dev, "When DC is wired, DOK and DCM should" >> @@ -218,19 +316,39 @@ static int max8903_probe(struct platform_device *pdev) >> return -EINVAL; >> } >> } else { >> - if (pdata->dcm) { >> - if (gpio_is_valid(pdata->dcm)) >> - gpio_set_value(pdata->dcm, 0); >> - else { >> + if (charger->pdata->dcm) { >> + if (gpio_is_valid(charger->pdata->dcm)) { >> + ret = devm_gpio_request(dev, >> + charger->pdata->dcm, >> + charger->psy_desc.name); >> + if (ret) { >> + dev_err(dev, >> + "Failed GPIO request for dcm: %d err %d\n", >> + charger->pdata->dcm, ret); >> + return -EINVAL; >> + } >> + >> + gpio_set_value(charger->pdata->dcm, 0); >> + } else { >> dev_err(dev, "Invalid pin: dcm.\n"); >> return -EINVAL; >> } >> } >> } >> >> - if (pdata->usb_valid) { >> - if (pdata->uok && gpio_is_valid(pdata->uok)) { >> - gpio = pdata->uok; >> + if (charger->pdata->usb_valid) { >> + if (gpio_is_valid(charger->pdata->uok)) { >> + ret = devm_gpio_request(dev, >> + charger->pdata->uok, >> + charger->psy_desc.name); >> + if (ret) { >> + dev_err(dev, >> + "Failed GPIO request for uok: %d err %d\n", >> + charger->pdata->uok, ret); >> + return -EINVAL; >> + } >> + >> + gpio = charger->pdata->uok; >> usb_in = gpio_get_value(gpio) ? 0 : 1; >> } else { >> dev_err(dev, "When USB is wired, UOK should be wired." >> @@ -239,91 +357,128 @@ static int max8903_probe(struct platform_device *pdev) >> } >> } >> >> - if (pdata->cen) { >> - if (gpio_is_valid(pdata->cen)) { >> - gpio_set_value(pdata->cen, (ta_in || usb_in) ? 0 : 1); >> + if (charger->pdata->cen) { >> + if (gpio_is_valid(charger->pdata->cen)) { >> + ret = devm_gpio_request(dev, >> + charger->pdata->cen, >> + charger->psy_desc.name); >> + if (ret) { >> + dev_err(dev, >> + "Failed GPIO request for cen: %d err %d\n", >> + charger->pdata->cen, ret); >> + return -EINVAL; >> + } >> + >> + gpio_set_value(charger->pdata->cen, >> + (ta_in || usb_in) ? 0 : 1); >> } else { >> dev_err(dev, "Invalid pin: cen.\n"); >> return -EINVAL; >> } >> } >> >> - if (pdata->chg) { >> - if (!gpio_is_valid(pdata->chg)) { >> + if (charger->pdata->chg) { >> + if (gpio_is_valid(charger->pdata->chg)) { >> + ret = devm_gpio_request(dev, >> + charger->pdata->chg, >> + charger->psy_desc.name); >> + if (ret) { >> + dev_err(dev, >> + "Failed GPIO request for chg: %d err %d\n", >> + charger->pdata->chg, ret); >> + return -EINVAL; >> + } >> + } else { >> dev_err(dev, "Invalid pin: chg.\n"); >> return -EINVAL; >> } >> } >> >> - if (pdata->flt) { >> - if (!gpio_is_valid(pdata->flt)) { >> + if (charger->pdata->flt) { >> + if (gpio_is_valid(charger->pdata->flt)) { >> + ret = devm_gpio_request(dev, >> + charger->pdata->flt, >> + charger->psy_desc.name); >> + if (ret) { >> + dev_err(dev, >> + "Failed GPIO request for flt: %d err %d\n", >> + charger->pdata->flt, ret); >> + return -EINVAL; >> + } >> + } else { >> dev_err(dev, "Invalid pin: flt.\n"); >> return -EINVAL; >> } >> } >> >> - if (pdata->usus) { >> - if (!gpio_is_valid(pdata->usus)) { >> + if (charger->pdata->usus) { >> + if (gpio_is_valid(charger->pdata->usus)) { >> + ret = devm_gpio_request(dev, >> + charger->pdata->usus, >> + charger->psy_desc.name); >> + if (ret) { >> + dev_err(dev, >> + "Failed GPIO request for usus: %d err %d\n", >> + charger->pdata->usus, ret); >> + return -EINVAL; >> + } >> + } else { >> dev_err(dev, "Invalid pin: usus.\n"); >> return -EINVAL; >> } >> } >> >> - data->fault = false; >> - data->ta_in = ta_in; >> - data->usb_in = usb_in; >> + psy_cfg.supplied_to = NULL; >> + psy_cfg.num_supplicants = 0; > > Why? This is already set to 0. Additionally this does not look needded > for conversion to DT. Please split out all unrelated changes to separate > patches. It could be organized as: > patch #1: Document DT bindings. > patch #2: Some fix needed. > patch #3: Some other cleanup > patch #4: Store pointer to pdata instead of copying it > patch #5: Add support for Device Tree > > Best regards, > Krzysztof > ^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH v2 3/4] max8903: cleans up confusing relationship between dc_valid, dok and dcm. 2016-06-10 12:32 ` [PATCH v2 0/4] max8903: Add device tree support and logic fixup Chris Lapa 2016-06-10 12:32 ` [PATCH v2 1/4] max8903: adds documentation for device tree bindings Chris Lapa 2016-06-10 12:32 ` [PATCH v2 2/4] max8903: adds support for initiation via device tree Chris Lapa @ 2016-06-10 12:32 ` Chris Lapa 2016-06-10 12:32 ` [PATCH v2 4/4] max8903: remove unnecessary malloc failed message print out Chris Lapa 3 siblings, 0 replies; 68+ messages in thread From: Chris Lapa @ 2016-06-10 12:32 UTC (permalink / raw) To: k.kozlowski; +Cc: devicetree, linux-kernel, linux-pm, Chris Lapa From: Chris Lapa <chris@lapa.com.au> The max8903_charger.h file indicated that dcm and dok were not optional when dc_valid is set. It makes sense to have dok as a compulsory pin when dc_valid is given. However dcm can be optionally wired to a fixed level especially when the circuit is configured for dc power exclusively. The previous implementation already allowed for this somewhat, however no error was given if dok wasn't given whilst dc_valid was. The new implementation enforces dok presence when dc_valid is given. Whilst allowing dcm to be optional. Signed-off-by: Chris Lapa <chris@lapa.com.au> --- drivers/power/max8903_charger.c | 40 ++++++++++++----------------------- include/linux/power/max8903_charger.h | 6 +++--- 2 files changed, 16 insertions(+), 30 deletions(-) diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c index d7544c8..20d4b55 100644 --- a/drivers/power/max8903_charger.c +++ b/drivers/power/max8903_charger.c @@ -282,9 +282,7 @@ static int max8903_probe(struct platform_device *pdev) if (charger->pdata->dc_valid) { if (charger->pdata->dok && - gpio_is_valid(charger->pdata->dok) && - charger->pdata->dcm && - gpio_is_valid(charger->pdata->dcm)) { + gpio_is_valid(charger->pdata->dok)) { ret = devm_gpio_request(dev, charger->pdata->dok, charger->psy_desc.name); @@ -295,6 +293,17 @@ static int max8903_probe(struct platform_device *pdev) return -EINVAL; } + gpio = charger->pdata->dok; /* PULL_UPed Interrupt */ + ta_in = gpio_get_value(gpio) ? 0 : 1; + } else { + dev_err(dev, "When DC is wired, DOK should" + " be wired as well.\n"); + return -EINVAL; + } + } + + if (charger->pdata->dcm) { + if (gpio_is_valid(charger->pdata->dcm)) { ret = devm_gpio_request(dev, charger->pdata->dcm, charger->psy_desc.name); @@ -305,35 +314,12 @@ static int max8903_probe(struct platform_device *pdev) return -EINVAL; } - gpio = charger->pdata->dok; /* PULL_UPed Interrupt */ - ta_in = gpio_get_value(gpio) ? 0 : 1; - gpio = charger->pdata->dcm; /* Output */ gpio_set_value(gpio, ta_in); } else { - dev_err(dev, "When DC is wired, DOK and DCM should" - " be wired as well.\n"); + dev_err(dev, "Invalid pin: dcm.\n"); return -EINVAL; } - } else { - if (charger->pdata->dcm) { - if (gpio_is_valid(charger->pdata->dcm)) { - ret = devm_gpio_request(dev, - charger->pdata->dcm, - charger->psy_desc.name); - if (ret) { - dev_err(dev, - "Failed GPIO request for dcm: %d err %d\n", - charger->pdata->dcm, ret); - return -EINVAL; - } - - gpio_set_value(charger->pdata->dcm, 0); - } else { - dev_err(dev, "Invalid pin: dcm.\n"); - return -EINVAL; - } - } } if (charger->pdata->usb_valid) { diff --git a/include/linux/power/max8903_charger.h b/include/linux/power/max8903_charger.h index 24f51db..89d3f1c 100644 --- a/include/linux/power/max8903_charger.h +++ b/include/linux/power/max8903_charger.h @@ -26,8 +26,8 @@ struct max8903_pdata { /* * GPIOs - * cen, chg, flt, and usus are optional. - * dok, dcm, and uok are not optional depending on the status of + * cen, chg, flt, dcm and usus are optional. + * dok and uok are not optional depending on the status of * dc_valid and usb_valid. */ int cen; /* Charger Enable input */ @@ -41,7 +41,7 @@ struct max8903_pdata { /* * DC(Adapter/TA) is wired * When dc_valid is true, - * dok and dcm should be valid. + * dok should be valid. * * At least one of dc_valid or usb_valid should be true. */ -- 1.9.1 ^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH v2 4/4] max8903: remove unnecessary malloc failed message print out. 2016-06-10 12:32 ` [PATCH v2 0/4] max8903: Add device tree support and logic fixup Chris Lapa ` (2 preceding siblings ...) 2016-06-10 12:32 ` [PATCH v2 3/4] max8903: cleans up confusing relationship between dc_valid, dok and dcm Chris Lapa @ 2016-06-10 12:32 ` Chris Lapa 2016-06-10 14:08 ` Krzysztof Kozlowski 3 siblings, 1 reply; 68+ messages in thread From: Chris Lapa @ 2016-06-10 12:32 UTC (permalink / raw) To: k.kozlowski; +Cc: devicetree, linux-kernel, linux-pm, Chris Lapa From: Chris Lapa <chris@lapa.com.au> Signed-off-by: Chris Lapa <chris@lapa.com.au> --- drivers/power/max8903_charger.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c index 20d4b55..d60f9b2 100644 --- a/drivers/power/max8903_charger.c +++ b/drivers/power/max8903_charger.c @@ -246,10 +246,8 @@ static int max8903_probe(struct platform_device *pdev) int usb_in = 0; charger = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL); - if (charger == NULL) { - dev_err(dev, "Cannot allocate memory.\n"); + if (!charger) return -ENOMEM; - } charger->pdata = pdev->dev.platform_data; if (IS_ENABLED(CONFIG_OF) && !charger->pdata && dev->of_node) { -- 1.9.1 ^ permalink raw reply related [flat|nested] 68+ messages in thread
* Re: [PATCH v2 4/4] max8903: remove unnecessary malloc failed message print out. 2016-06-10 12:32 ` [PATCH v2 4/4] max8903: remove unnecessary malloc failed message print out Chris Lapa @ 2016-06-10 14:08 ` Krzysztof Kozlowski 0 siblings, 0 replies; 68+ messages in thread From: Krzysztof Kozlowski @ 2016-06-10 14:08 UTC (permalink / raw) To: Chris Lapa; +Cc: devicetree, linux-kernel, linux-pm On 06/10/2016 02:32 PM, Chris Lapa wrote: > From: Chris Lapa <chris@lapa.com.au> Here and in all other patches (like #1) you need to write something. The commit title could be very short (even non-sentence) and here a little bit longer. E.g.: Title: power: max8903: Remove pointless 'out of memory' error message Body: Remove the error message of memory allocation failure because it is printed by core. Best regards, Krzysztof > Signed-off-by: Chris Lapa <chris@lapa.com.au> > --- > drivers/power/max8903_charger.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c > index 20d4b55..d60f9b2 100644 > --- a/drivers/power/max8903_charger.c > +++ b/drivers/power/max8903_charger.c > @@ -246,10 +246,8 @@ static int max8903_probe(struct platform_device *pdev) > int usb_in = 0; > > charger = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL); > - if (charger == NULL) { > - dev_err(dev, "Cannot allocate memory.\n"); > + if (!charger) > return -ENOMEM; > - } > > charger->pdata = pdev->dev.platform_data; > if (IS_ENABLED(CONFIG_OF) && !charger->pdata && dev->of_node) { > ^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH v3 0/7] max8903: Add device tree support and misc fixes 2016-06-02 6:44 ` [PATCH 2/2] max8903: cleans up confusing relationship between dc_valid, dok and dcm chris 2016-06-10 12:32 ` [PATCH v2 0/4] max8903: Add device tree support and logic fixup Chris Lapa @ 2016-06-17 5:00 ` Chris Lapa 2016-06-17 5:00 ` [PATCH v3 1/7] max8903: adds documentation for device tree bindings Chris Lapa ` (6 more replies) 2016-06-19 22:27 ` [PATCH v4 0/7] max8903: Add device tree support and misc fixes Chris Lapa 2016-06-24 2:26 ` [PATCH v5 0/7] max8903: Add device tree support and misc fixes Chris Lapa 3 siblings, 7 replies; 68+ messages in thread From: Chris Lapa @ 2016-06-17 5:00 UTC (permalink / raw) To: k.kozlowski, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm, Chris Lapa From: Chris Lapa <chris@lapa.com.au> This patch set adds device tree support for the MAX8903 battery charger. It also cleans up logic with dc_valid, dok and dcm pins as well as fixing up validity checking of gpios. I verified these patches work on a board I have here, which uses the DC power side (not the USB portition) of the MAX8903. Changes v2 -> v3: * Seperate requesting of gpio's into its own commit * Fixed up validity checking of GPIO's * Remove dc_valid and usb_valid from device tree * Remove some unncessary init to psy_cfg.num_supplicants and psy_cfg.supplied_to * Reorder patches so device tree implementation is final patch Changes v1 -> v2: * Seperate DT bindings documentation into its own commit * Add maxim prefix to DT compatible field * Add gpios suffix to gpio's in DT * Remove malloc failed error message Chris Lapa (7): max8903: adds documentation for device tree bindings. max8903: store pointer to pdata instead of copying it. max8903: cleans up confusing relationship between dc_valid, dok and dcm. max8903: adds requesting of gpios. max8903: removes non zero validity checks on gpios. max8903: remove unnecessary 'out of memory' error message. max8903: adds support for initiation via device tree. .../devicetree/bindings/power/max8903-charger.txt | 25 ++ drivers/power/max8903_charger.c | 278 +++++++++++++++------ include/linux/power/max8903_charger.h | 6 +- 3 files changed, 227 insertions(+), 82 deletions(-) create mode 100644 Documentation/devicetree/bindings/power/max8903-charger.txt -- 1.9.1 ^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH v3 1/7] max8903: adds documentation for device tree bindings. 2016-06-17 5:00 ` [PATCH v3 0/7] max8903: Add device tree support and misc fixes Chris Lapa @ 2016-06-17 5:00 ` Chris Lapa 2016-06-17 6:15 ` Krzysztof Kozlowski 2016-06-20 13:22 ` Rob Herring 2016-06-17 5:00 ` [PATCH v3 2/7] max8903: store pointer to pdata instead of copying it Chris Lapa ` (5 subsequent siblings) 6 siblings, 2 replies; 68+ messages in thread From: Chris Lapa @ 2016-06-17 5:00 UTC (permalink / raw) To: k.kozlowski, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm, Chris Lapa From: Chris Lapa <chris@lapa.com.au> Signed-off-by: Chris Lapa <chris@lapa.com.au> --- .../devicetree/bindings/power/max8903-charger.txt | 25 ++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/max8903-charger.txt diff --git a/Documentation/devicetree/bindings/power/max8903-charger.txt b/Documentation/devicetree/bindings/power/max8903-charger.txt new file mode 100644 index 0000000..aea1dd2a --- /dev/null +++ b/Documentation/devicetree/bindings/power/max8903-charger.txt @@ -0,0 +1,25 @@ +Maxim Semiconductor MAX8903 Battery Charger bindings + +Required properties: +- compatible: "maxim,max8903-charger" for MAX8903 Battery Charger +- dok-gpios: Valid DC power has been detected, optional if uok-gpios is provided +- uok-gpios: Valid USB power has been detected, optional if dok-gpios is provided + +Optional properties: +- cen-gpios: Charge enable pin +- chg-gpios: Charger status pin +- flt-gpios: Fault pin +- dcm-gpios: Current limit mode setting (DC or USB) +- usus-gpios: USB suspend pin + + +Example: + + max8903-charger { + compatible = "maxim,max8903-charger"; + dok-gpios = <&gpio2 3 GPIO_ACTIVE_LOW>; + flt-gpios = <&gpio2 2 GPIO_ACTIVE_LOW>; + chg-gpios = <&gpio3 15 GPIO_ACTIVE_LOW>; + cen-gpios = <&gpio2 5 GPIO_ACTIVE_LOW>; + status = "okay"; + }; -- 1.9.1 ^ permalink raw reply related [flat|nested] 68+ messages in thread
* Re: [PATCH v3 1/7] max8903: adds documentation for device tree bindings. 2016-06-17 5:00 ` [PATCH v3 1/7] max8903: adds documentation for device tree bindings Chris Lapa @ 2016-06-17 6:15 ` Krzysztof Kozlowski 2016-06-20 13:22 ` Rob Herring 1 sibling, 0 replies; 68+ messages in thread From: Krzysztof Kozlowski @ 2016-06-17 6:15 UTC (permalink / raw) To: Chris Lapa, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm On 06/17/2016 07:00 AM, Chris Lapa wrote: > From: Chris Lapa <chris@lapa.com.au> > > Signed-off-by: Chris Lapa <chris@lapa.com.au> > --- > .../devicetree/bindings/power/max8903-charger.txt | 25 ++++++++++++++++++++++ > 1 file changed, 25 insertions(+) > create mode 100644 Documentation/devicetree/bindings/power/max8903-charger.txt Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> Best regards, Krzysztof ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH v3 1/7] max8903: adds documentation for device tree bindings. 2016-06-17 5:00 ` [PATCH v3 1/7] max8903: adds documentation for device tree bindings Chris Lapa 2016-06-17 6:15 ` Krzysztof Kozlowski @ 2016-06-20 13:22 ` Rob Herring 2016-06-20 23:43 ` Chris Lapa 1 sibling, 1 reply; 68+ messages in thread From: Rob Herring @ 2016-06-20 13:22 UTC (permalink / raw) To: Chris Lapa Cc: k.kozlowski, dwmw2, dbaryshkov, sre, mark.rutland, devicetree, linux-kernel, linux-pm On Fri, Jun 17, 2016 at 03:00:20PM +1000, Chris Lapa wrote: > From: Chris Lapa <chris@lapa.com.au> > > Signed-off-by: Chris Lapa <chris@lapa.com.au> > --- > .../devicetree/bindings/power/max8903-charger.txt | 25 ++++++++++++++++++++++ > 1 file changed, 25 insertions(+) > create mode 100644 Documentation/devicetree/bindings/power/max8903-charger.txt > > diff --git a/Documentation/devicetree/bindings/power/max8903-charger.txt b/Documentation/devicetree/bindings/power/max8903-charger.txt > new file mode 100644 > index 0000000..aea1dd2a > --- /dev/null > +++ b/Documentation/devicetree/bindings/power/max8903-charger.txt > @@ -0,0 +1,25 @@ > +Maxim Semiconductor MAX8903 Battery Charger bindings > + > +Required properties: > +- compatible: "maxim,max8903-charger" for MAX8903 Battery Charger Drop "charger" as that is implied by the chip. > +- dok-gpios: Valid DC power has been detected, optional if uok-gpios is provided > +- uok-gpios: Valid USB power has been detected, optional if dok-gpios is provided > + > +Optional properties: > +- cen-gpios: Charge enable pin > +- chg-gpios: Charger status pin > +- flt-gpios: Fault pin > +- dcm-gpios: Current limit mode setting (DC or USB) > +- usus-gpios: USB suspend pin Need to state direction and active state for all of these. ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH v3 1/7] max8903: adds documentation for device tree bindings. 2016-06-20 13:22 ` Rob Herring @ 2016-06-20 23:43 ` Chris Lapa 0 siblings, 0 replies; 68+ messages in thread From: Chris Lapa @ 2016-06-20 23:43 UTC (permalink / raw) To: Rob Herring Cc: k.kozlowski, dwmw2, dbaryshkov, sre, mark.rutland, devicetree, linux-kernel, linux-pm On 20/06/2016 11:22 PM, Rob Herring wrote: > On Fri, Jun 17, 2016 at 03:00:20PM +1000, Chris Lapa wrote: >> From: Chris Lapa <chris@lapa.com.au> >> >> Signed-off-by: Chris Lapa <chris@lapa.com.au> >> --- >> .../devicetree/bindings/power/max8903-charger.txt | 25 ++++++++++++++++++++++ >> 1 file changed, 25 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/power/max8903-charger.txt >> >> diff --git a/Documentation/devicetree/bindings/power/max8903-charger.txt b/Documentation/devicetree/bindings/power/max8903-charger.txt >> new file mode 100644 >> index 0000000..aea1dd2a >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/power/max8903-charger.txt >> @@ -0,0 +1,25 @@ >> +Maxim Semiconductor MAX8903 Battery Charger bindings >> + >> +Required properties: >> +- compatible: "maxim,max8903-charger" for MAX8903 Battery Charger > > Drop "charger" as that is implied by the chip. Just for clarification, you mean: compatible: "maxim,max8903" for MAX8903 Battery Charger Thanks, Chris > >> +- dok-gpios: Valid DC power has been detected, optional if uok-gpios is provided >> +- uok-gpios: Valid USB power has been detected, optional if dok-gpios is provided >> + >> +Optional properties: >> +- cen-gpios: Charge enable pin >> +- chg-gpios: Charger status pin >> +- flt-gpios: Fault pin >> +- dcm-gpios: Current limit mode setting (DC or USB) >> +- usus-gpios: USB suspend pin > > Need to state direction and active state for all of these. > ^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH v3 2/7] max8903: store pointer to pdata instead of copying it. 2016-06-17 5:00 ` [PATCH v3 0/7] max8903: Add device tree support and misc fixes Chris Lapa 2016-06-17 5:00 ` [PATCH v3 1/7] max8903: adds documentation for device tree bindings Chris Lapa @ 2016-06-17 5:00 ` Chris Lapa 2016-06-17 6:14 ` Krzysztof Kozlowski 2016-06-17 5:00 ` [PATCH v3 3/7] max8903: cleans up confusing relationship between dc_valid, dok and dcm Chris Lapa ` (4 subsequent siblings) 6 siblings, 1 reply; 68+ messages in thread From: Chris Lapa @ 2016-06-17 5:00 UTC (permalink / raw) To: k.kozlowski, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm, Chris Lapa From: Chris Lapa <chris@lapa.com.au> Stores pointer to pdata because it easily allows pdata to reference either platform data or in the future device tree data. Signed-off-by: Chris Lapa <chris@lapa.com.au> --- drivers/power/max8903_charger.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c index 17876ca..0a5b0e1 100644 --- a/drivers/power/max8903_charger.c +++ b/drivers/power/max8903_charger.c @@ -29,7 +29,7 @@ #include <linux/power/max8903_charger.h> struct max8903_data { - struct max8903_pdata pdata; + struct max8903_pdata *pdata; struct device *dev; struct power_supply *psy; struct power_supply_desc psy_desc; @@ -53,8 +53,8 @@ static int max8903_get_property(struct power_supply *psy, switch (psp) { case POWER_SUPPLY_PROP_STATUS: val->intval = POWER_SUPPLY_STATUS_UNKNOWN; - if (data->pdata.chg) { - if (gpio_get_value(data->pdata.chg) == 0) + if (data->pdata->chg) { + if (gpio_get_value(data->pdata->chg) == 0) val->intval = POWER_SUPPLY_STATUS_CHARGING; else if (data->usb_in || data->ta_in) val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING; @@ -81,7 +81,7 @@ static int max8903_get_property(struct power_supply *psy, static irqreturn_t max8903_dcin(int irq, void *_data) { struct max8903_data *data = _data; - struct max8903_pdata *pdata = &data->pdata; + struct max8903_pdata *pdata = data->pdata; bool ta_in; enum power_supply_type old_type; @@ -122,7 +122,7 @@ static irqreturn_t max8903_dcin(int irq, void *_data) static irqreturn_t max8903_usbin(int irq, void *_data) { struct max8903_data *data = _data; - struct max8903_pdata *pdata = &data->pdata; + struct max8903_pdata *pdata = data->pdata; bool usb_in; enum power_supply_type old_type; @@ -161,7 +161,7 @@ static irqreturn_t max8903_usbin(int irq, void *_data) static irqreturn_t max8903_fault(int irq, void *_data) { struct max8903_data *data = _data; - struct max8903_pdata *pdata = &data->pdata; + struct max8903_pdata *pdata = data->pdata; bool fault; fault = gpio_get_value(pdata->flt) ? false : true; @@ -190,12 +190,18 @@ static int max8903_probe(struct platform_device *pdev) int ta_in = 0; int usb_in = 0; + if (pdata == NULL) { + dev_err(dev, "No platform data.\n"); + return -EINVAL; + } + data = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL); if (data == NULL) { dev_err(dev, "Cannot allocate memory.\n"); return -ENOMEM; } - memcpy(&data->pdata, pdata, sizeof(struct max8903_pdata)); + + data->pdata = pdev->dev.platform_data; data->dev = dev; platform_set_drvdata(pdev, data); -- 1.9.1 ^ permalink raw reply related [flat|nested] 68+ messages in thread
* Re: [PATCH v3 2/7] max8903: store pointer to pdata instead of copying it. 2016-06-17 5:00 ` [PATCH v3 2/7] max8903: store pointer to pdata instead of copying it Chris Lapa @ 2016-06-17 6:14 ` Krzysztof Kozlowski 0 siblings, 0 replies; 68+ messages in thread From: Krzysztof Kozlowski @ 2016-06-17 6:14 UTC (permalink / raw) To: Chris Lapa, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm On 06/17/2016 07:00 AM, Chris Lapa wrote: > From: Chris Lapa <chris@lapa.com.au> > > Stores pointer to pdata because it easily allows pdata to reference > either platform data or in the future device tree data. > > Signed-off-by: Chris Lapa <chris@lapa.com.au> > --- > drivers/power/max8903_charger.c | 20 +++++++++++++------- > 1 file changed, 13 insertions(+), 7 deletions(-) Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> Best regards, Krzysztof ^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH v3 3/7] max8903: cleans up confusing relationship between dc_valid, dok and dcm. 2016-06-17 5:00 ` [PATCH v3 0/7] max8903: Add device tree support and misc fixes Chris Lapa 2016-06-17 5:00 ` [PATCH v3 1/7] max8903: adds documentation for device tree bindings Chris Lapa 2016-06-17 5:00 ` [PATCH v3 2/7] max8903: store pointer to pdata instead of copying it Chris Lapa @ 2016-06-17 5:00 ` Chris Lapa 2016-06-17 6:23 ` Krzysztof Kozlowski 2016-06-17 6:26 ` Krzysztof Kozlowski 2016-06-17 5:00 ` [PATCH v3 4/7] max8903: adds requesting of gpios Chris Lapa ` (3 subsequent siblings) 6 siblings, 2 replies; 68+ messages in thread From: Chris Lapa @ 2016-06-17 5:00 UTC (permalink / raw) To: k.kozlowski, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm, Chris Lapa From: Chris Lapa <chris@lapa.com.au> The max8903_charger.h file indicated that dcm and dok were not optional when dc_valid is set. It makes sense to have dok as a compulsory pin when dc_valid is given. However dcm can be optionally wired to a fixed level especially when the circuit is configured for dc power exclusively. The previous implementation already allowed for this somewhat, however no error was given if dok wasn't given whilst dc_valid was. The new implementation enforces dok presence when dc_valid is given. Whilst allowing dcm to be optional. Signed-off-by: Chris Lapa <chris@lapa.com.au> --- drivers/power/max8903_charger.c | 23 ++++++++++------------- include/linux/power/max8903_charger.h | 6 +++--- 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c index 0a5b0e1..dbd911c4 100644 --- a/drivers/power/max8903_charger.c +++ b/drivers/power/max8903_charger.c @@ -211,27 +211,24 @@ static int max8903_probe(struct platform_device *pdev) } if (pdata->dc_valid) { - if (pdata->dok && gpio_is_valid(pdata->dok) && - pdata->dcm && gpio_is_valid(pdata->dcm)) { + if (pdata->dok && gpio_is_valid(pdata->dok)) { gpio = pdata->dok; /* PULL_UPed Interrupt */ ta_in = gpio_get_value(gpio) ? 0 : 1; + } else { + dev_err(dev, "When DC is wired, DOK should" + " be wired as well.\n"); + return -EINVAL; + } + } + if (pdata->dcm) { + if (gpio_is_valid(pdata->dcm)) { gpio = pdata->dcm; /* Output */ gpio_set_value(gpio, ta_in); } else { - dev_err(dev, "When DC is wired, DOK and DCM should" - " be wired as well.\n"); + dev_err(dev, "Invalid pin: dcm.\n"); return -EINVAL; } - } else { - if (pdata->dcm) { - if (gpio_is_valid(pdata->dcm)) - gpio_set_value(pdata->dcm, 0); - else { - dev_err(dev, "Invalid pin: dcm.\n"); - return -EINVAL; - } - } } if (pdata->usb_valid) { diff --git a/include/linux/power/max8903_charger.h b/include/linux/power/max8903_charger.h index 24f51db..89d3f1c 100644 --- a/include/linux/power/max8903_charger.h +++ b/include/linux/power/max8903_charger.h @@ -26,8 +26,8 @@ struct max8903_pdata { /* * GPIOs - * cen, chg, flt, and usus are optional. - * dok, dcm, and uok are not optional depending on the status of + * cen, chg, flt, dcm and usus are optional. + * dok and uok are not optional depending on the status of * dc_valid and usb_valid. */ int cen; /* Charger Enable input */ @@ -41,7 +41,7 @@ struct max8903_pdata { /* * DC(Adapter/TA) is wired * When dc_valid is true, - * dok and dcm should be valid. + * dok should be valid. * * At least one of dc_valid or usb_valid should be true. */ -- 1.9.1 ^ permalink raw reply related [flat|nested] 68+ messages in thread
* Re: [PATCH v3 3/7] max8903: cleans up confusing relationship between dc_valid, dok and dcm. 2016-06-17 5:00 ` [PATCH v3 3/7] max8903: cleans up confusing relationship between dc_valid, dok and dcm Chris Lapa @ 2016-06-17 6:23 ` Krzysztof Kozlowski 2016-06-17 6:26 ` Krzysztof Kozlowski 1 sibling, 0 replies; 68+ messages in thread From: Krzysztof Kozlowski @ 2016-06-17 6:23 UTC (permalink / raw) To: Chris Lapa, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm On 06/17/2016 07:00 AM, Chris Lapa wrote: > From: Chris Lapa <chris@lapa.com.au> > > The max8903_charger.h file indicated that dcm and dok were not optional > when dc_valid is set. > > It makes sense to have dok as a compulsory pin when dc_valid is given. > However dcm can be optionally wired to a fixed level especially when the > circuit is configured for dc power exclusively. > > The previous implementation already allowed for this somewhat, however no > error was given if dok wasn't given whilst dc_valid was. > > The new implementation enforces dok presence when dc_valid is given. Whilst > allowing dcm to be optional. > > Signed-off-by: Chris Lapa <chris@lapa.com.au> > --- > drivers/power/max8903_charger.c | 23 ++++++++++------------- > include/linux/power/max8903_charger.h | 6 +++--- > 2 files changed, 13 insertions(+), 16 deletions(-) Code looks correct: Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> Best regards, Krzysztof ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH v3 3/7] max8903: cleans up confusing relationship between dc_valid, dok and dcm. 2016-06-17 5:00 ` [PATCH v3 3/7] max8903: cleans up confusing relationship between dc_valid, dok and dcm Chris Lapa 2016-06-17 6:23 ` Krzysztof Kozlowski @ 2016-06-17 6:26 ` Krzysztof Kozlowski 2016-06-17 6:28 ` Chris Lapa 2016-06-19 13:48 ` Chris Lapa 1 sibling, 2 replies; 68+ messages in thread From: Krzysztof Kozlowski @ 2016-06-17 6:26 UTC (permalink / raw) To: Chris Lapa, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm On 06/17/2016 07:00 AM, Chris Lapa wrote: > From: Chris Lapa <chris@lapa.com.au> > > The max8903_charger.h file indicated that dcm and dok were not optional > when dc_valid is set. > > It makes sense to have dok as a compulsory pin when dc_valid is given. > However dcm can be optionally wired to a fixed level especially when the > circuit is configured for dc power exclusively. > > The previous implementation already allowed for this somewhat, however no > error was given if dok wasn't given whilst dc_valid was. > > The new implementation enforces dok presence when dc_valid is given. Whilst > allowing dcm to be optional. > > Signed-off-by: Chris Lapa <chris@lapa.com.au> > --- > drivers/power/max8903_charger.c | 23 ++++++++++------------- > include/linux/power/max8903_charger.h | 6 +++--- > 2 files changed, 13 insertions(+), 16 deletions(-) > > diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c > index 0a5b0e1..dbd911c4 100644 > --- a/drivers/power/max8903_charger.c > +++ b/drivers/power/max8903_charger.c > @@ -211,27 +211,24 @@ static int max8903_probe(struct platform_device *pdev) > } > > if (pdata->dc_valid) { > - if (pdata->dok && gpio_is_valid(pdata->dok) && > - pdata->dcm && gpio_is_valid(pdata->dcm)) { > + if (pdata->dok && gpio_is_valid(pdata->dok)) { > gpio = pdata->dok; /* PULL_UPed Interrupt */ > ta_in = gpio_get_value(gpio) ? 0 : 1; > + } else { > + dev_err(dev, "When DC is wired, DOK should" > + " be wired as well.\n"); Just found one nit. Don't split the strings. dev_err(dev, "When DC is wired, DOK should be wired as well.\n"); Best regards, Krzysztof ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH v3 3/7] max8903: cleans up confusing relationship between dc_valid, dok and dcm. 2016-06-17 6:26 ` Krzysztof Kozlowski @ 2016-06-17 6:28 ` Chris Lapa 2016-06-17 6:42 ` Krzysztof Kozlowski 2016-06-19 13:48 ` Chris Lapa 1 sibling, 1 reply; 68+ messages in thread From: Chris Lapa @ 2016-06-17 6:28 UTC (permalink / raw) To: Krzysztof Kozlowski, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm On 17/06/2016 4:26 PM, Krzysztof Kozlowski wrote: > On 06/17/2016 07:00 AM, Chris Lapa wrote: >> From: Chris Lapa <chris@lapa.com.au> >> >> The max8903_charger.h file indicated that dcm and dok were not optional >> when dc_valid is set. >> >> It makes sense to have dok as a compulsory pin when dc_valid is given. >> However dcm can be optionally wired to a fixed level especially when the >> circuit is configured for dc power exclusively. >> >> The previous implementation already allowed for this somewhat, however no >> error was given if dok wasn't given whilst dc_valid was. >> >> The new implementation enforces dok presence when dc_valid is given. Whilst >> allowing dcm to be optional. >> >> Signed-off-by: Chris Lapa <chris@lapa.com.au> >> --- >> drivers/power/max8903_charger.c | 23 ++++++++++------------- >> include/linux/power/max8903_charger.h | 6 +++--- >> 2 files changed, 13 insertions(+), 16 deletions(-) >> >> diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c >> index 0a5b0e1..dbd911c4 100644 >> --- a/drivers/power/max8903_charger.c >> +++ b/drivers/power/max8903_charger.c >> @@ -211,27 +211,24 @@ static int max8903_probe(struct platform_device *pdev) >> } >> >> if (pdata->dc_valid) { >> - if (pdata->dok && gpio_is_valid(pdata->dok) && >> - pdata->dcm && gpio_is_valid(pdata->dcm)) { >> + if (pdata->dok && gpio_is_valid(pdata->dok)) { >> gpio = pdata->dok; /* PULL_UPed Interrupt */ >> ta_in = gpio_get_value(gpio) ? 0 : 1; >> + } else { >> + dev_err(dev, "When DC is wired, DOK should" >> + " be wired as well.\n"); > > Just found one nit. Don't split the strings. > dev_err(dev, > "When DC is wired, DOK should be wired as well.\n"); I saw that one as well when I ran checkpatch, however I thought if I changed it then I would get a warning about the line being > 80 chars. So wasn't sure which direction to go. Thanks Chris > > Best regards, > Krzysztof > ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH v3 3/7] max8903: cleans up confusing relationship between dc_valid, dok and dcm. 2016-06-17 6:28 ` Chris Lapa @ 2016-06-17 6:42 ` Krzysztof Kozlowski 0 siblings, 0 replies; 68+ messages in thread From: Krzysztof Kozlowski @ 2016-06-17 6:42 UTC (permalink / raw) To: chris, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm On 06/17/2016 08:28 AM, Chris Lapa wrote: > On 17/06/2016 4:26 PM, Krzysztof Kozlowski wrote: >> On 06/17/2016 07:00 AM, Chris Lapa wrote: >>> From: Chris Lapa <chris@lapa.com.au> >>> >>> The max8903_charger.h file indicated that dcm and dok were not optional >>> when dc_valid is set. >>> >>> It makes sense to have dok as a compulsory pin when dc_valid is given. >>> However dcm can be optionally wired to a fixed level especially when the >>> circuit is configured for dc power exclusively. >>> >>> The previous implementation already allowed for this somewhat, >>> however no >>> error was given if dok wasn't given whilst dc_valid was. >>> >>> The new implementation enforces dok presence when dc_valid is given. >>> Whilst >>> allowing dcm to be optional. >>> >>> Signed-off-by: Chris Lapa <chris@lapa.com.au> >>> --- >>> drivers/power/max8903_charger.c | 23 ++++++++++------------- >>> include/linux/power/max8903_charger.h | 6 +++--- >>> 2 files changed, 13 insertions(+), 16 deletions(-) >>> >>> diff --git a/drivers/power/max8903_charger.c >>> b/drivers/power/max8903_charger.c >>> index 0a5b0e1..dbd911c4 100644 >>> --- a/drivers/power/max8903_charger.c >>> +++ b/drivers/power/max8903_charger.c >>> @@ -211,27 +211,24 @@ static int max8903_probe(struct platform_device >>> *pdev) >>> } >>> >>> if (pdata->dc_valid) { >>> - if (pdata->dok && gpio_is_valid(pdata->dok) && >>> - pdata->dcm && gpio_is_valid(pdata->dcm)) { >>> + if (pdata->dok && gpio_is_valid(pdata->dok)) { >>> gpio = pdata->dok; /* PULL_UPed Interrupt */ >>> ta_in = gpio_get_value(gpio) ? 0 : 1; >>> + } else { >>> + dev_err(dev, "When DC is wired, DOK should" >>> + " be wired as well.\n"); >> >> Just found one nit. Don't split the strings. >> dev_err(dev, >> "When DC is wired, DOK should be wired as well.\n"); > I saw that one as well when I ran checkpatch, however I thought if I > changed it then I would get a warning about the line being > 80 chars. > So wasn't sure which direction to go. Checkpatch shouldn't complain on strings so if you move the string to next line, it should be ok. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH v3 3/7] max8903: cleans up confusing relationship between dc_valid, dok and dcm. 2016-06-17 6:26 ` Krzysztof Kozlowski 2016-06-17 6:28 ` Chris Lapa @ 2016-06-19 13:48 ` Chris Lapa 1 sibling, 0 replies; 68+ messages in thread From: Chris Lapa @ 2016-06-19 13:48 UTC (permalink / raw) To: Krzysztof Kozlowski, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm On 17/06/2016 4:26 PM, Krzysztof Kozlowski wrote: > On 06/17/2016 07:00 AM, Chris Lapa wrote: >> From: Chris Lapa <chris@lapa.com.au> >> >> The max8903_charger.h file indicated that dcm and dok were not optional >> when dc_valid is set. >> >> It makes sense to have dok as a compulsory pin when dc_valid is given. >> However dcm can be optionally wired to a fixed level especially when the >> circuit is configured for dc power exclusively. >> >> The previous implementation already allowed for this somewhat, however no >> error was given if dok wasn't given whilst dc_valid was. >> >> The new implementation enforces dok presence when dc_valid is given. Whilst >> allowing dcm to be optional. >> >> Signed-off-by: Chris Lapa <chris@lapa.com.au> >> --- >> drivers/power/max8903_charger.c | 23 ++++++++++------------- >> include/linux/power/max8903_charger.h | 6 +++--- >> 2 files changed, 13 insertions(+), 16 deletions(-) >> >> diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c >> index 0a5b0e1..dbd911c4 100644 >> --- a/drivers/power/max8903_charger.c >> +++ b/drivers/power/max8903_charger.c >> @@ -211,27 +211,24 @@ static int max8903_probe(struct platform_device *pdev) >> } >> >> if (pdata->dc_valid) { >> - if (pdata->dok && gpio_is_valid(pdata->dok) && >> - pdata->dcm && gpio_is_valid(pdata->dcm)) { >> + if (pdata->dok && gpio_is_valid(pdata->dok)) { >> gpio = pdata->dok; /* PULL_UPed Interrupt */ >> ta_in = gpio_get_value(gpio) ? 0 : 1; >> + } else { >> + dev_err(dev, "When DC is wired, DOK should" >> + " be wired as well.\n"); > > Just found one nit. Don't split the strings. > dev_err(dev, > "When DC is wired, DOK should be wired as well.\n"); > > Best regards, > Krzysztof > Fixed in v4 Thanks, Chris ^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH v3 4/7] max8903: adds requesting of gpios. 2016-06-17 5:00 ` [PATCH v3 0/7] max8903: Add device tree support and misc fixes Chris Lapa ` (2 preceding siblings ...) 2016-06-17 5:00 ` [PATCH v3 3/7] max8903: cleans up confusing relationship between dc_valid, dok and dcm Chris Lapa @ 2016-06-17 5:00 ` Chris Lapa 2016-06-17 6:30 ` Krzysztof Kozlowski 2016-06-17 5:00 ` [PATCH v3 5/7] max8903: removes non zero validity checks on gpios Chris Lapa ` (2 subsequent siblings) 6 siblings, 1 reply; 68+ messages in thread From: Chris Lapa @ 2016-06-17 5:00 UTC (permalink / raw) To: k.kozlowski, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm, Chris Lapa From: Chris Lapa <chris@lapa.com.au> This change ensures all gpios are available for the driver to use. Signed-off-by: Chris Lapa <chris@lapa.com.au> --- drivers/power/max8903_charger.c | 79 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 70 insertions(+), 9 deletions(-) diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c index dbd911c4..c068efe 100644 --- a/drivers/power/max8903_charger.c +++ b/drivers/power/max8903_charger.c @@ -212,6 +212,16 @@ static int max8903_probe(struct platform_device *pdev) if (pdata->dc_valid) { if (pdata->dok && gpio_is_valid(pdata->dok)) { + ret = devm_gpio_request(dev, + pdata->dok, + data->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for dok: %d err %d\n", + pdata->dok, ret); + return -EINVAL; + } + gpio = pdata->dok; /* PULL_UPed Interrupt */ ta_in = gpio_get_value(gpio) ? 0 : 1; } else { @@ -223,6 +233,16 @@ static int max8903_probe(struct platform_device *pdev) if (pdata->dcm) { if (gpio_is_valid(pdata->dcm)) { + ret = devm_gpio_request(dev, + pdata->dcm, + data->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for dcm: %d err %d\n", + pdata->dcm, ret); + return -EINVAL; + } + gpio = pdata->dcm; /* Output */ gpio_set_value(gpio, ta_in); } else { @@ -233,6 +253,16 @@ static int max8903_probe(struct platform_device *pdev) if (pdata->usb_valid) { if (pdata->uok && gpio_is_valid(pdata->uok)) { + ret = devm_gpio_request(dev, + pdata->uok, + data->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for uok: %d err %d\n", + pdata->uok, ret); + return -EINVAL; + } + gpio = pdata->uok; usb_in = gpio_get_value(gpio) ? 0 : 1; } else { @@ -244,6 +274,16 @@ static int max8903_probe(struct platform_device *pdev) if (pdata->cen) { if (gpio_is_valid(pdata->cen)) { + ret = devm_gpio_request(dev, + pdata->cen, + data->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for cen: %d err %d\n", + pdata->cen, ret); + return -EINVAL; + } + gpio_set_value(pdata->cen, (ta_in || usb_in) ? 0 : 1); } else { dev_err(dev, "Invalid pin: cen.\n"); @@ -252,23 +292,44 @@ static int max8903_probe(struct platform_device *pdev) } if (pdata->chg) { - if (!gpio_is_valid(pdata->chg)) { - dev_err(dev, "Invalid pin: chg.\n"); - return -EINVAL; + if (gpio_is_valid(pdata->chg)) { + ret = devm_gpio_request(dev, + pdata->chg, + data->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for chg: %d err %d\n", + pdata->chg, ret); + return -EINVAL; + } } } if (pdata->flt) { - if (!gpio_is_valid(pdata->flt)) { - dev_err(dev, "Invalid pin: flt.\n"); - return -EINVAL; + if (gpio_is_valid(pdata->flt)) { + ret = devm_gpio_request(dev, + pdata->flt, + data->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for flt: %d err %d\n", + pdata->flt, ret); + return -EINVAL; + } } } if (pdata->usus) { - if (!gpio_is_valid(pdata->usus)) { - dev_err(dev, "Invalid pin: usus.\n"); - return -EINVAL; + if (gpio_is_valid(pdata->usus)) { + ret = devm_gpio_request(dev, + pdata->usus, + data->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for usus: %d err %d\n", + pdata->usus, ret); + return -EINVAL; + } } } -- 1.9.1 ^ permalink raw reply related [flat|nested] 68+ messages in thread
* Re: [PATCH v3 4/7] max8903: adds requesting of gpios. 2016-06-17 5:00 ` [PATCH v3 4/7] max8903: adds requesting of gpios Chris Lapa @ 2016-06-17 6:30 ` Krzysztof Kozlowski 2016-06-19 13:51 ` Chris Lapa 0 siblings, 1 reply; 68+ messages in thread From: Krzysztof Kozlowski @ 2016-06-17 6:30 UTC (permalink / raw) To: Chris Lapa, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm On 06/17/2016 07:00 AM, Chris Lapa wrote: > From: Chris Lapa <chris@lapa.com.au> > > This change ensures all gpios are available for the driver to use. > > Signed-off-by: Chris Lapa <chris@lapa.com.au> > --- > drivers/power/max8903_charger.c | 79 ++++++++++++++++++++++++++++++++++++----- > 1 file changed, 70 insertions(+), 9 deletions(-) > > diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c > index dbd911c4..c068efe 100644 > --- a/drivers/power/max8903_charger.c > +++ b/drivers/power/max8903_charger.c > @@ -212,6 +212,16 @@ static int max8903_probe(struct platform_device *pdev) > > if (pdata->dc_valid) { > if (pdata->dok && gpio_is_valid(pdata->dok)) { > + ret = devm_gpio_request(dev, > + pdata->dok, No need to split these two arguments. They fit in 80 chars. > + data->psy_desc.name); > + if (ret) { > + dev_err(dev, > + "Failed GPIO request for dok: %d err %d\n", > + pdata->dok, ret); Indentation here is wrong. These should be aligned to first argument (dev). > + return -EINVAL; Return 'ret'. All these three comments apply also to code below. Anyway the probe function is getting bigger and more difficult to read. Consider factoring out some code to a separate function. For example all the GPIO handling stuff could be factored. Best regards, Krzysztof > + } > + > gpio = pdata->dok; /* PULL_UPed Interrupt */ > ta_in = gpio_get_value(gpio) ? 0 : 1; > } else { > @@ -223,6 +233,16 @@ static int max8903_probe(struct platform_device *pdev) > > if (pdata->dcm) { > if (gpio_is_valid(pdata->dcm)) { > + ret = devm_gpio_request(dev, > + pdata->dcm, > + data->psy_desc.name); > + if (ret) { > + dev_err(dev, > + "Failed GPIO request for dcm: %d err %d\n", > + pdata->dcm, ret); > + return -EINVAL; > + } > + > gpio = pdata->dcm; /* Output */ > gpio_set_value(gpio, ta_in); > } else { > @@ -233,6 +253,16 @@ static int max8903_probe(struct platform_device *pdev) > > if (pdata->usb_valid) { > if (pdata->uok && gpio_is_valid(pdata->uok)) { > + ret = devm_gpio_request(dev, > + pdata->uok, > + data->psy_desc.name); > + if (ret) { > + dev_err(dev, > + "Failed GPIO request for uok: %d err %d\n", > + pdata->uok, ret); > + return -EINVAL; > + } > + > gpio = pdata->uok; > usb_in = gpio_get_value(gpio) ? 0 : 1; > } else { > @@ -244,6 +274,16 @@ static int max8903_probe(struct platform_device *pdev) > > if (pdata->cen) { > if (gpio_is_valid(pdata->cen)) { > + ret = devm_gpio_request(dev, > + pdata->cen, > + data->psy_desc.name); > + if (ret) { > + dev_err(dev, > + "Failed GPIO request for cen: %d err %d\n", > + pdata->cen, ret); > + return -EINVAL; > + } > + > gpio_set_value(pdata->cen, (ta_in || usb_in) ? 0 : 1); > } else { > dev_err(dev, "Invalid pin: cen.\n"); > @@ -252,23 +292,44 @@ static int max8903_probe(struct platform_device *pdev) > } > > if (pdata->chg) { > - if (!gpio_is_valid(pdata->chg)) { > - dev_err(dev, "Invalid pin: chg.\n"); > - return -EINVAL; > + if (gpio_is_valid(pdata->chg)) { > + ret = devm_gpio_request(dev, > + pdata->chg, > + data->psy_desc.name); > + if (ret) { > + dev_err(dev, > + "Failed GPIO request for chg: %d err %d\n", > + pdata->chg, ret); > + return -EINVAL; > + } > } > } > > if (pdata->flt) { > - if (!gpio_is_valid(pdata->flt)) { > - dev_err(dev, "Invalid pin: flt.\n"); > - return -EINVAL; > + if (gpio_is_valid(pdata->flt)) { > + ret = devm_gpio_request(dev, > + pdata->flt, > + data->psy_desc.name); > + if (ret) { > + dev_err(dev, > + "Failed GPIO request for flt: %d err %d\n", > + pdata->flt, ret); > + return -EINVAL; > + } > } > } > > if (pdata->usus) { > - if (!gpio_is_valid(pdata->usus)) { > - dev_err(dev, "Invalid pin: usus.\n"); > - return -EINVAL; > + if (gpio_is_valid(pdata->usus)) { > + ret = devm_gpio_request(dev, > + pdata->usus, > + data->psy_desc.name); > + if (ret) { > + dev_err(dev, > + "Failed GPIO request for usus: %d err %d\n", > + pdata->usus, ret); > + return -EINVAL; > + } > } > } > > ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH v3 4/7] max8903: adds requesting of gpios. 2016-06-17 6:30 ` Krzysztof Kozlowski @ 2016-06-19 13:51 ` Chris Lapa 0 siblings, 0 replies; 68+ messages in thread From: Chris Lapa @ 2016-06-19 13:51 UTC (permalink / raw) To: Krzysztof Kozlowski, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm On 17/06/2016 4:30 PM, Krzysztof Kozlowski wrote: > On 06/17/2016 07:00 AM, Chris Lapa wrote: >> From: Chris Lapa <chris@lapa.com.au> >> >> This change ensures all gpios are available for the driver to use. >> >> Signed-off-by: Chris Lapa <chris@lapa.com.au> >> --- >> drivers/power/max8903_charger.c | 79 ++++++++++++++++++++++++++++++++++++----- >> 1 file changed, 70 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c >> index dbd911c4..c068efe 100644 >> --- a/drivers/power/max8903_charger.c >> +++ b/drivers/power/max8903_charger.c >> @@ -212,6 +212,16 @@ static int max8903_probe(struct platform_device *pdev) >> >> if (pdata->dc_valid) { >> if (pdata->dok && gpio_is_valid(pdata->dok)) { >> + ret = devm_gpio_request(dev, >> + pdata->dok, > > No need to split these two arguments. They fit in 80 chars. > >> + data->psy_desc.name); >> + if (ret) { >> + dev_err(dev, >> + "Failed GPIO request for dok: %d err %d\n", >> + pdata->dok, ret); > > Indentation here is wrong. These should be aligned to first argument (dev). > >> + return -EINVAL; > > Return 'ret'. > > All these three comments apply also to code below. > > Anyway the probe function is getting bigger and more difficult to read. > Consider factoring out some code to a separate function. For example all > the GPIO handling stuff could be factored. > > Best regards, > Krzysztof Good idea, I've split the gpio setup code into a function named: max8903_setup_gpios() Also fixed the other issues you mentioned above. Thanks, Chris > >> + } >> + >> gpio = pdata->dok; /* PULL_UPed Interrupt */ >> ta_in = gpio_get_value(gpio) ? 0 : 1; >> } else { >> @@ -223,6 +233,16 @@ static int max8903_probe(struct platform_device *pdev) >> >> if (pdata->dcm) { >> if (gpio_is_valid(pdata->dcm)) { >> + ret = devm_gpio_request(dev, >> + pdata->dcm, >> + data->psy_desc.name); >> + if (ret) { >> + dev_err(dev, >> + "Failed GPIO request for dcm: %d err %d\n", >> + pdata->dcm, ret); >> + return -EINVAL; >> + } >> + >> gpio = pdata->dcm; /* Output */ >> gpio_set_value(gpio, ta_in); >> } else { >> @@ -233,6 +253,16 @@ static int max8903_probe(struct platform_device *pdev) >> >> if (pdata->usb_valid) { >> if (pdata->uok && gpio_is_valid(pdata->uok)) { >> + ret = devm_gpio_request(dev, >> + pdata->uok, >> + data->psy_desc.name); >> + if (ret) { >> + dev_err(dev, >> + "Failed GPIO request for uok: %d err %d\n", >> + pdata->uok, ret); >> + return -EINVAL; >> + } >> + >> gpio = pdata->uok; >> usb_in = gpio_get_value(gpio) ? 0 : 1; >> } else { >> @@ -244,6 +274,16 @@ static int max8903_probe(struct platform_device *pdev) >> >> if (pdata->cen) { >> if (gpio_is_valid(pdata->cen)) { >> + ret = devm_gpio_request(dev, >> + pdata->cen, >> + data->psy_desc.name); >> + if (ret) { >> + dev_err(dev, >> + "Failed GPIO request for cen: %d err %d\n", >> + pdata->cen, ret); >> + return -EINVAL; >> + } >> + >> gpio_set_value(pdata->cen, (ta_in || usb_in) ? 0 : 1); >> } else { >> dev_err(dev, "Invalid pin: cen.\n"); >> @@ -252,23 +292,44 @@ static int max8903_probe(struct platform_device *pdev) >> } >> >> if (pdata->chg) { >> - if (!gpio_is_valid(pdata->chg)) { >> - dev_err(dev, "Invalid pin: chg.\n"); >> - return -EINVAL; >> + if (gpio_is_valid(pdata->chg)) { >> + ret = devm_gpio_request(dev, >> + pdata->chg, >> + data->psy_desc.name); >> + if (ret) { >> + dev_err(dev, >> + "Failed GPIO request for chg: %d err %d\n", >> + pdata->chg, ret); >> + return -EINVAL; >> + } >> } >> } >> >> if (pdata->flt) { >> - if (!gpio_is_valid(pdata->flt)) { >> - dev_err(dev, "Invalid pin: flt.\n"); >> - return -EINVAL; >> + if (gpio_is_valid(pdata->flt)) { >> + ret = devm_gpio_request(dev, >> + pdata->flt, >> + data->psy_desc.name); >> + if (ret) { >> + dev_err(dev, >> + "Failed GPIO request for flt: %d err %d\n", >> + pdata->flt, ret); >> + return -EINVAL; >> + } >> } >> } >> >> if (pdata->usus) { >> - if (!gpio_is_valid(pdata->usus)) { >> - dev_err(dev, "Invalid pin: usus.\n"); >> - return -EINVAL; >> + if (gpio_is_valid(pdata->usus)) { >> + ret = devm_gpio_request(dev, >> + pdata->usus, >> + data->psy_desc.name); >> + if (ret) { >> + dev_err(dev, >> + "Failed GPIO request for usus: %d err %d\n", >> + pdata->usus, ret); >> + return -EINVAL; >> + } >> } >> } >> >> > ^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH v3 5/7] max8903: removes non zero validity checks on gpios. 2016-06-17 5:00 ` [PATCH v3 0/7] max8903: Add device tree support and misc fixes Chris Lapa ` (3 preceding siblings ...) 2016-06-17 5:00 ` [PATCH v3 4/7] max8903: adds requesting of gpios Chris Lapa @ 2016-06-17 5:00 ` Chris Lapa 2016-06-17 6:35 ` Krzysztof Kozlowski 2016-06-17 5:00 ` [PATCH v3 6/7] max8903: remove unnecessary 'out of memory' error message Chris Lapa 2016-06-17 5:00 ` [PATCH v3 7/7] max8903: adds support for initiation via device tree Chris Lapa 6 siblings, 1 reply; 68+ messages in thread From: Chris Lapa @ 2016-06-17 5:00 UTC (permalink / raw) To: k.kozlowski, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm, Chris Lapa From: Chris Lapa <chris@lapa.com.au> Prior to this commit a zero gpio was treated as invalid. Whereas gpio_is_valid() will treat a zero gpio as valid. This commit removes the confusion and explicity uses gpio_is_valid() throughout. Which in turn results in several of the error messages becoming redundant and thus removed. Signed-off-by: Chris Lapa <chris@lapa.com.au> --- drivers/power/max8903_charger.c | 125 ++++++++++++++++++---------------------- 1 file changed, 55 insertions(+), 70 deletions(-) diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c index c068efe..bfb81a2 100644 --- a/drivers/power/max8903_charger.c +++ b/drivers/power/max8903_charger.c @@ -53,7 +53,7 @@ static int max8903_get_property(struct power_supply *psy, switch (psp) { case POWER_SUPPLY_PROP_STATUS: val->intval = POWER_SUPPLY_STATUS_UNKNOWN; - if (data->pdata->chg) { + if (gpio_is_valid(data->pdata->chg)) { if (gpio_get_value(data->pdata->chg) == 0) val->intval = POWER_SUPPLY_STATUS_CHARGING; else if (data->usb_in || data->ta_in) @@ -93,11 +93,11 @@ static irqreturn_t max8903_dcin(int irq, void *_data) data->ta_in = ta_in; /* Set Current-Limit-Mode 1:DC 0:USB */ - if (pdata->dcm) + if (gpio_is_valid(pdata->dcm)) gpio_set_value(pdata->dcm, ta_in ? 1 : 0); /* Charger Enable / Disable (cen is negated) */ - if (pdata->cen) + if (gpio_is_valid(pdata->cen)) gpio_set_value(pdata->cen, ta_in ? 0 : (data->usb_in ? 0 : 1)); @@ -136,7 +136,7 @@ static irqreturn_t max8903_usbin(int irq, void *_data) /* Do not touch Current-Limit-Mode */ /* Charger Enable / Disable (cen is negated) */ - if (pdata->cen) + if (gpio_is_valid(pdata->cen)) gpio_set_value(pdata->cen, usb_in ? 0 : (data->ta_in ? 0 : 1)); @@ -211,7 +211,7 @@ static int max8903_probe(struct platform_device *pdev) } if (pdata->dc_valid) { - if (pdata->dok && gpio_is_valid(pdata->dok)) { + if (gpio_is_valid(pdata->dok)) { ret = devm_gpio_request(dev, pdata->dok, data->psy_desc.name); @@ -231,28 +231,23 @@ static int max8903_probe(struct platform_device *pdev) } } - if (pdata->dcm) { - if (gpio_is_valid(pdata->dcm)) { - ret = devm_gpio_request(dev, - pdata->dcm, - data->psy_desc.name); - if (ret) { - dev_err(dev, - "Failed GPIO request for dcm: %d err %d\n", - pdata->dcm, ret); - return -EINVAL; - } - - gpio = pdata->dcm; /* Output */ - gpio_set_value(gpio, ta_in); - } else { - dev_err(dev, "Invalid pin: dcm.\n"); + if (gpio_is_valid(pdata->dcm)) { + ret = devm_gpio_request(dev, + pdata->dcm, + data->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for dcm: %d err %d\n", + pdata->dcm, ret); return -EINVAL; } + + gpio = pdata->dcm; /* Output */ + gpio_set_value(gpio, ta_in); } if (pdata->usb_valid) { - if (pdata->uok && gpio_is_valid(pdata->uok)) { + if (gpio_is_valid(pdata->uok)) { ret = devm_gpio_request(dev, pdata->uok, data->psy_desc.name); @@ -272,64 +267,54 @@ static int max8903_probe(struct platform_device *pdev) } } - if (pdata->cen) { - if (gpio_is_valid(pdata->cen)) { - ret = devm_gpio_request(dev, - pdata->cen, - data->psy_desc.name); - if (ret) { - dev_err(dev, - "Failed GPIO request for cen: %d err %d\n", - pdata->cen, ret); - return -EINVAL; - } - gpio_set_value(pdata->cen, (ta_in || usb_in) ? 0 : 1); - } else { - dev_err(dev, "Invalid pin: cen.\n"); + if (gpio_is_valid(pdata->cen)) { + ret = devm_gpio_request(dev, + pdata->cen, + data->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for cen: %d err %d\n", + pdata->cen, ret); return -EINVAL; } + + gpio_set_value(pdata->cen, (ta_in || usb_in) ? 0 : 1); } - if (pdata->chg) { - if (gpio_is_valid(pdata->chg)) { - ret = devm_gpio_request(dev, - pdata->chg, - data->psy_desc.name); - if (ret) { - dev_err(dev, - "Failed GPIO request for chg: %d err %d\n", - pdata->chg, ret); - return -EINVAL; - } + if (gpio_is_valid(pdata->chg)) { + ret = devm_gpio_request(dev, + pdata->chg, + data->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for chg: %d err %d\n", + pdata->chg, ret); + return -EINVAL; } } - if (pdata->flt) { - if (gpio_is_valid(pdata->flt)) { - ret = devm_gpio_request(dev, - pdata->flt, - data->psy_desc.name); - if (ret) { - dev_err(dev, - "Failed GPIO request for flt: %d err %d\n", - pdata->flt, ret); - return -EINVAL; - } + if (gpio_is_valid(pdata->flt)) { + ret = devm_gpio_request(dev, + pdata->flt, + data->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for flt: %d err %d\n", + pdata->flt, ret); + return -EINVAL; } } - if (pdata->usus) { - if (gpio_is_valid(pdata->usus)) { - ret = devm_gpio_request(dev, - pdata->usus, - data->psy_desc.name); - if (ret) { - dev_err(dev, - "Failed GPIO request for usus: %d err %d\n", - pdata->usus, ret); - return -EINVAL; - } + if (gpio_is_valid(pdata->usus)) { + ret = devm_gpio_request(dev, + pdata->usus, + data->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for usus: %d err %d\n", + pdata->usus, ret); + return -EINVAL; } } @@ -379,7 +364,7 @@ static int max8903_probe(struct platform_device *pdev) } } - if (pdata->flt) { + if (gpio_is_valid(pdata->flt)) { ret = devm_request_threaded_irq(dev, gpio_to_irq(pdata->flt), NULL, max8903_fault, IRQF_TRIGGER_FALLING | -- 1.9.1 ^ permalink raw reply related [flat|nested] 68+ messages in thread
* Re: [PATCH v3 5/7] max8903: removes non zero validity checks on gpios. 2016-06-17 5:00 ` [PATCH v3 5/7] max8903: removes non zero validity checks on gpios Chris Lapa @ 2016-06-17 6:35 ` Krzysztof Kozlowski 0 siblings, 0 replies; 68+ messages in thread From: Krzysztof Kozlowski @ 2016-06-17 6:35 UTC (permalink / raw) To: Chris Lapa, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm On 06/17/2016 07:00 AM, Chris Lapa wrote: > From: Chris Lapa <chris@lapa.com.au> > > Prior to this commit a zero gpio was treated as invalid. Whereas > gpio_is_valid() will treat a zero gpio as valid. > > This commit removes the confusion and explicity uses gpio_is_valid() s/explicity/explicitly/ > throughout. Which in turn results in several of the error messages becoming > redundant and thus removed. > > Signed-off-by: Chris Lapa <chris@lapa.com.au> > --- > drivers/power/max8903_charger.c | 125 ++++++++++++++++++---------------------- > 1 file changed, 55 insertions(+), 70 deletions(-) Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> Best regards, Krzysztof ^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH v3 6/7] max8903: remove unnecessary 'out of memory' error message. 2016-06-17 5:00 ` [PATCH v3 0/7] max8903: Add device tree support and misc fixes Chris Lapa ` (4 preceding siblings ...) 2016-06-17 5:00 ` [PATCH v3 5/7] max8903: removes non zero validity checks on gpios Chris Lapa @ 2016-06-17 5:00 ` Chris Lapa 2016-06-17 6:35 ` Krzysztof Kozlowski 2016-06-17 5:00 ` [PATCH v3 7/7] max8903: adds support for initiation via device tree Chris Lapa 6 siblings, 1 reply; 68+ messages in thread From: Chris Lapa @ 2016-06-17 5:00 UTC (permalink / raw) To: k.kozlowski, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm, Chris Lapa From: Chris Lapa <chris@lapa.com.au> Remove the 'out of memory' error message as it is printed by the core. Signed-off-by: Chris Lapa <chris@lapa.com.au> --- drivers/power/max8903_charger.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c index bfb81a2..5ddc667 100644 --- a/drivers/power/max8903_charger.c +++ b/drivers/power/max8903_charger.c @@ -196,10 +196,8 @@ static int max8903_probe(struct platform_device *pdev) } data = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL); - if (data == NULL) { - dev_err(dev, "Cannot allocate memory.\n"); + if (!data) return -ENOMEM; - } data->pdata = pdev->dev.platform_data; data->dev = dev; -- 1.9.1 ^ permalink raw reply related [flat|nested] 68+ messages in thread
* Re: [PATCH v3 6/7] max8903: remove unnecessary 'out of memory' error message. 2016-06-17 5:00 ` [PATCH v3 6/7] max8903: remove unnecessary 'out of memory' error message Chris Lapa @ 2016-06-17 6:35 ` Krzysztof Kozlowski 0 siblings, 0 replies; 68+ messages in thread From: Krzysztof Kozlowski @ 2016-06-17 6:35 UTC (permalink / raw) To: Chris Lapa, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm On 06/17/2016 07:00 AM, Chris Lapa wrote: > From: Chris Lapa <chris@lapa.com.au> > > Remove the 'out of memory' error message as it is printed by the core. > > Signed-off-by: Chris Lapa <chris@lapa.com.au> > --- > drivers/power/max8903_charger.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> Best regards, Krzysztof ^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH v3 7/7] max8903: adds support for initiation via device tree. 2016-06-17 5:00 ` [PATCH v3 0/7] max8903: Add device tree support and misc fixes Chris Lapa ` (5 preceding siblings ...) 2016-06-17 5:00 ` [PATCH v3 6/7] max8903: remove unnecessary 'out of memory' error message Chris Lapa @ 2016-06-17 5:00 ` Chris Lapa 2016-06-17 6:41 ` Krzysztof Kozlowski 6 siblings, 1 reply; 68+ messages in thread From: Chris Lapa @ 2016-06-17 5:00 UTC (permalink / raw) To: k.kozlowski, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm, Chris Lapa From: Chris Lapa <chris@lapa.com.au> Adds support for device tree to setup a max8903 battery charger. DC and USB validity are determined by looking the presence of the dok and uok gpios. Signed-off-by: Chris Lapa <chris@lapa.com.au> --- drivers/power/max8903_charger.c | 217 +++++++++++++++++++++++++++------------- 1 file changed, 145 insertions(+), 72 deletions(-) diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c index 5ddc667..3c59213 100644 --- a/drivers/power/max8903_charger.c +++ b/drivers/power/max8903_charger.c @@ -23,6 +23,9 @@ #include <linux/gpio.h> #include <linux/interrupt.h> #include <linux/module.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/of_gpio.h> #include <linux/slab.h> #include <linux/power_supply.h> #include <linux/platform_device.h> @@ -75,6 +78,7 @@ static int max8903_get_property(struct power_supply *psy, default: return -EINVAL; } + return 0; } @@ -179,48 +183,116 @@ static irqreturn_t max8903_fault(int irq, void *_data) return IRQ_HANDLED; } +static struct max8903_pdata *max8903_parse_dt_data( + struct device *dev) +{ + struct device_node *of_node = dev->of_node; + struct max8903_pdata *pdata = NULL; + + if (!of_node) + return NULL; + + pdata = devm_kzalloc(dev, sizeof(*pdata), + GFP_KERNEL); + if (!pdata) + return NULL; + + pdata->dc_valid = false; + pdata->usb_valid = false; + + pdata->cen = of_get_named_gpio(of_node, "cen-gpios", 0); + if (!gpio_is_valid(pdata->cen)) + pdata->cen = -EINVAL; + + pdata->chg = of_get_named_gpio(of_node, "chg-gpios", 0); + if (!gpio_is_valid(pdata->chg)) + pdata->chg = -EINVAL; + + pdata->flt = of_get_named_gpio(of_node, "flt-gpios", 0); + if (!gpio_is_valid(pdata->flt)) + pdata->flt = -EINVAL; + + pdata->usus = of_get_named_gpio(of_node, "usus-gpios", 0); + if (!gpio_is_valid(pdata->usus)) + pdata->usus = -EINVAL; + + pdata->dcm = of_get_named_gpio(of_node, "dcm-gpios", 0); + if (!gpio_is_valid(pdata->dcm)) + pdata->dcm = -EINVAL; + + pdata->dok = of_get_named_gpio(of_node, "dok-gpios", 0); + if (!gpio_is_valid(pdata->dok)) + pdata->dok = -EINVAL; + else + pdata->dc_valid = true; + + pdata->uok = of_get_named_gpio(of_node, "uok-gpios", 0); + if (!gpio_is_valid(pdata->uok)) + pdata->uok = -EINVAL; + else + pdata->usb_valid = true; + + return pdata; +} + static int max8903_probe(struct platform_device *pdev) { - struct max8903_data *data; + struct max8903_data *charger; struct device *dev = &pdev->dev; - struct max8903_pdata *pdata = pdev->dev.platform_data; struct power_supply_config psy_cfg = {}; int ret = 0; int gpio; int ta_in = 0; int usb_in = 0; - if (pdata == NULL) { + charger = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL); + if (!charger) + return -ENOMEM; + + charger->pdata = pdev->dev.platform_data; + if (IS_ENABLED(CONFIG_OF) && !charger->pdata && dev->of_node) + charger->pdata = max8903_parse_dt_data(dev); + + if (!charger->pdata) { dev_err(dev, "No platform data.\n"); return -EINVAL; } - data = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL); - if (!data) - return -ENOMEM; + charger->dev = dev; - data->pdata = pdev->dev.platform_data; - data->dev = dev; - platform_set_drvdata(pdev, data); + charger->fault = false; + charger->ta_in = ta_in; + charger->usb_in = usb_in; - if (pdata->dc_valid == false && pdata->usb_valid == false) { + charger->psy_desc.name = "max8903_charger"; + charger->psy_desc.type = (ta_in) ? POWER_SUPPLY_TYPE_MAINS : + ((usb_in) ? POWER_SUPPLY_TYPE_USB : + POWER_SUPPLY_TYPE_BATTERY); + charger->psy_desc.get_property = max8903_get_property; + charger->psy_desc.properties = max8903_charger_props; + charger->psy_desc.num_properties = ARRAY_SIZE(max8903_charger_props); + + platform_set_drvdata(pdev, charger); + + if (charger->pdata->dc_valid == false && charger->pdata->usb_valid == false) { dev_err(dev, "No valid power sources.\n"); return -EINVAL; } - if (pdata->dc_valid) { - if (gpio_is_valid(pdata->dok)) { + + if (charger->pdata->dc_valid) { + if (gpio_is_valid(charger->pdata->dok)) { ret = devm_gpio_request(dev, - pdata->dok, - data->psy_desc.name); + charger->pdata->dok, + charger->psy_desc.name); if (ret) { dev_err(dev, "Failed GPIO request for dok: %d err %d\n", - pdata->dok, ret); + charger->pdata->dok, ret); return -EINVAL; } - gpio = pdata->dok; /* PULL_UPed Interrupt */ + gpio = charger->pdata->dok; /* PULL_UPed Interrupt */ ta_in = gpio_get_value(gpio) ? 0 : 1; } else { dev_err(dev, "When DC is wired, DOK should" @@ -229,34 +301,34 @@ static int max8903_probe(struct platform_device *pdev) } } - if (gpio_is_valid(pdata->dcm)) { + if (gpio_is_valid(charger->pdata->dcm)) { ret = devm_gpio_request(dev, - pdata->dcm, - data->psy_desc.name); + charger->pdata->dcm, + charger->psy_desc.name); if (ret) { dev_err(dev, "Failed GPIO request for dcm: %d err %d\n", - pdata->dcm, ret); + charger->pdata->dcm, ret); return -EINVAL; } - gpio = pdata->dcm; /* Output */ + gpio = charger->pdata->dcm; /* Output */ gpio_set_value(gpio, ta_in); } - if (pdata->usb_valid) { - if (gpio_is_valid(pdata->uok)) { + if (charger->pdata->usb_valid) { + if (gpio_is_valid(charger->pdata->uok)) { ret = devm_gpio_request(dev, - pdata->uok, - data->psy_desc.name); + charger->pdata->uok, + charger->psy_desc.name); if (ret) { dev_err(dev, "Failed GPIO request for uok: %d err %d\n", - pdata->uok, ret); + charger->pdata->uok, ret); return -EINVAL; } - gpio = pdata->uok; + gpio = charger->pdata->uok; usb_in = gpio_get_value(gpio) ? 0 : 1; } else { dev_err(dev, "When USB is wired, UOK should be wired." @@ -266,111 +338,105 @@ static int max8903_probe(struct platform_device *pdev) } - if (gpio_is_valid(pdata->cen)) { + if (gpio_is_valid(charger->pdata->cen)) { ret = devm_gpio_request(dev, - pdata->cen, - data->psy_desc.name); + charger->pdata->cen, + charger->psy_desc.name); if (ret) { dev_err(dev, "Failed GPIO request for cen: %d err %d\n", - pdata->cen, ret); + charger->pdata->cen, ret); return -EINVAL; } - gpio_set_value(pdata->cen, (ta_in || usb_in) ? 0 : 1); + gpio_set_value(charger->pdata->cen, (ta_in || usb_in) ? 0 : 1); } - if (gpio_is_valid(pdata->chg)) { + if (gpio_is_valid(charger->pdata->chg)) { ret = devm_gpio_request(dev, - pdata->chg, - data->psy_desc.name); + charger->pdata->chg, + charger->psy_desc.name); if (ret) { dev_err(dev, "Failed GPIO request for chg: %d err %d\n", - pdata->chg, ret); + charger->pdata->chg, ret); return -EINVAL; } } - if (gpio_is_valid(pdata->flt)) { + if (gpio_is_valid(charger->pdata->flt)) { ret = devm_gpio_request(dev, - pdata->flt, - data->psy_desc.name); + charger->pdata->flt, + charger->psy_desc.name); if (ret) { dev_err(dev, "Failed GPIO request for flt: %d err %d\n", - pdata->flt, ret); + charger->pdata->flt, ret); return -EINVAL; } } - if (gpio_is_valid(pdata->usus)) { + if (gpio_is_valid(charger->pdata->usus)) { ret = devm_gpio_request(dev, - pdata->usus, - data->psy_desc.name); + charger->pdata->usus, + charger->psy_desc.name); if (ret) { dev_err(dev, "Failed GPIO request for usus: %d err %d\n", - pdata->usus, ret); + charger->pdata->usus, ret); return -EINVAL; } } - data->fault = false; - data->ta_in = ta_in; - data->usb_in = usb_in; - - data->psy_desc.name = "max8903_charger"; - data->psy_desc.type = (ta_in) ? POWER_SUPPLY_TYPE_MAINS : - ((usb_in) ? POWER_SUPPLY_TYPE_USB : - POWER_SUPPLY_TYPE_BATTERY); - data->psy_desc.get_property = max8903_get_property; - data->psy_desc.properties = max8903_charger_props; - data->psy_desc.num_properties = ARRAY_SIZE(max8903_charger_props); - - psy_cfg.drv_data = data; + psy_cfg.of_node = dev->of_node; + psy_cfg.drv_data = charger; - data->psy = devm_power_supply_register(dev, &data->psy_desc, &psy_cfg); - if (IS_ERR(data->psy)) { + charger->psy = devm_power_supply_register(dev, + &charger->psy_desc, + &psy_cfg); + if (IS_ERR(charger->psy)) { dev_err(dev, "failed: power supply register.\n"); - return PTR_ERR(data->psy); + return PTR_ERR(charger->psy); } - if (pdata->dc_valid) { - ret = devm_request_threaded_irq(dev, gpio_to_irq(pdata->dok), + if (charger->pdata->dc_valid) { + ret = devm_request_threaded_irq(dev, + gpio_to_irq(charger->pdata->dok), NULL, max8903_dcin, IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING | IRQF_ONESHOT, - "MAX8903 DC IN", data); + "MAX8903 DC IN", charger); if (ret) { dev_err(dev, "Cannot request irq %d for DC (%d)\n", - gpio_to_irq(pdata->dok), ret); + gpio_to_irq(charger->pdata->dok), ret); return ret; } } - if (pdata->usb_valid) { - ret = devm_request_threaded_irq(dev, gpio_to_irq(pdata->uok), + if (charger->pdata->usb_valid) { + ret = devm_request_threaded_irq(dev, + gpio_to_irq(charger->pdata->uok), NULL, max8903_usbin, IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING | IRQF_ONESHOT, - "MAX8903 USB IN", data); + "MAX8903 USB IN", charger); if (ret) { dev_err(dev, "Cannot request irq %d for USB (%d)\n", - gpio_to_irq(pdata->uok), ret); + gpio_to_irq(charger->pdata->uok), ret); return ret; } } - if (gpio_is_valid(pdata->flt)) { - ret = devm_request_threaded_irq(dev, gpio_to_irq(pdata->flt), + if (gpio_is_valid(charger->pdata->flt)) { + ret = devm_request_threaded_irq(dev, + gpio_to_irq(charger->pdata->flt), NULL, max8903_fault, IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING | IRQF_ONESHOT, - "MAX8903 Fault", data); + "MAX8903 Fault", charger); if (ret) { dev_err(dev, "Cannot request irq %d for Fault (%d)\n", - gpio_to_irq(pdata->flt), ret); + gpio_to_irq(charger->pdata->flt), ret); return ret; } } @@ -378,10 +444,17 @@ static int max8903_probe(struct platform_device *pdev) return 0; } +static const struct of_device_id max8903_match_ids[] = { + { .compatible = "maxim,max8903-charger", }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, max8903_match_ids); + static struct platform_driver max8903_driver = { .probe = max8903_probe, .driver = { .name = "max8903-charger", + .of_match_table = max8903_match_ids }, }; -- 1.9.1 ^ permalink raw reply related [flat|nested] 68+ messages in thread
* Re: [PATCH v3 7/7] max8903: adds support for initiation via device tree. 2016-06-17 5:00 ` [PATCH v3 7/7] max8903: adds support for initiation via device tree Chris Lapa @ 2016-06-17 6:41 ` Krzysztof Kozlowski 2016-06-19 13:52 ` Chris Lapa 0 siblings, 1 reply; 68+ messages in thread From: Krzysztof Kozlowski @ 2016-06-17 6:41 UTC (permalink / raw) To: Chris Lapa, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm On 06/17/2016 07:00 AM, Chris Lapa wrote: > From: Chris Lapa <chris@lapa.com.au> > > Adds support for device tree to setup a max8903 battery charger. DC and USB > validity are determined by looking the presence of the dok and uok gpios. > > Signed-off-by: Chris Lapa <chris@lapa.com.au> > --- > drivers/power/max8903_charger.c | 217 +++++++++++++++++++++++++++------------- > 1 file changed, 145 insertions(+), 72 deletions(-) > > diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c > index 5ddc667..3c59213 100644 > --- a/drivers/power/max8903_charger.c > +++ b/drivers/power/max8903_charger.c > @@ -23,6 +23,9 @@ > #include <linux/gpio.h> > #include <linux/interrupt.h> > #include <linux/module.h> > +#include <linux/of.h> > +#include <linux/of_device.h> > +#include <linux/of_gpio.h> > #include <linux/slab.h> > #include <linux/power_supply.h> > #include <linux/platform_device.h> > @@ -75,6 +78,7 @@ static int max8903_get_property(struct power_supply *psy, > default: > return -EINVAL; > } > + > return 0; > } > > @@ -179,48 +183,116 @@ static irqreturn_t max8903_fault(int irq, void *_data) > return IRQ_HANDLED; > } > > +static struct max8903_pdata *max8903_parse_dt_data( > + struct device *dev) > +{ > + struct device_node *of_node = dev->of_node; Common naming convention for device_node is just 'np': struct device_node *np = dev->of_node; It is shorter and already spread in the kernel. > + struct max8903_pdata *pdata = NULL; > + > + if (!of_node) > + return NULL; > + > + pdata = devm_kzalloc(dev, sizeof(*pdata), > + GFP_KERNEL); No need to break the line. > + if (!pdata) > + return NULL; > + > + pdata->dc_valid = false; > + pdata->usb_valid = false; > + > + pdata->cen = of_get_named_gpio(of_node, "cen-gpios", 0); > + if (!gpio_is_valid(pdata->cen)) > + pdata->cen = -EINVAL; > + > + pdata->chg = of_get_named_gpio(of_node, "chg-gpios", 0); > + if (!gpio_is_valid(pdata->chg)) > + pdata->chg = -EINVAL; > + > + pdata->flt = of_get_named_gpio(of_node, "flt-gpios", 0); > + if (!gpio_is_valid(pdata->flt)) > + pdata->flt = -EINVAL; > + > + pdata->usus = of_get_named_gpio(of_node, "usus-gpios", 0); > + if (!gpio_is_valid(pdata->usus)) > + pdata->usus = -EINVAL; > + > + pdata->dcm = of_get_named_gpio(of_node, "dcm-gpios", 0); > + if (!gpio_is_valid(pdata->dcm)) > + pdata->dcm = -EINVAL; > + > + pdata->dok = of_get_named_gpio(of_node, "dok-gpios", 0); > + if (!gpio_is_valid(pdata->dok)) > + pdata->dok = -EINVAL; > + else > + pdata->dc_valid = true; > + > + pdata->uok = of_get_named_gpio(of_node, "uok-gpios", 0); > + if (!gpio_is_valid(pdata->uok)) > + pdata->uok = -EINVAL; > + else > + pdata->usb_valid = true; > + > + return pdata; > +} > + > static int max8903_probe(struct platform_device *pdev) > { > - struct max8903_data *data; > + struct max8903_data *charger; > struct device *dev = &pdev->dev; > - struct max8903_pdata *pdata = pdev->dev.platform_data; > struct power_supply_config psy_cfg = {}; > int ret = 0; > int gpio; > int ta_in = 0; > int usb_in = 0; > > - if (pdata == NULL) { > + charger = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL); > + if (!charger) > + return -ENOMEM; > + > + charger->pdata = pdev->dev.platform_data; > + if (IS_ENABLED(CONFIG_OF) && !charger->pdata && dev->of_node) > + charger->pdata = max8903_parse_dt_data(dev); > + > + if (!charger->pdata) { > dev_err(dev, "No platform data.\n"); > return -EINVAL; > } > > - data = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL); > - if (!data) > - return -ENOMEM; > + charger->dev = dev; > > - data->pdata = pdev->dev.platform_data; > - data->dev = dev; > - platform_set_drvdata(pdev, data); > + charger->fault = false; > + charger->ta_in = ta_in; > + charger->usb_in = usb_in; > > - if (pdata->dc_valid == false && pdata->usb_valid == false) { > + charger->psy_desc.name = "max8903_charger"; > + charger->psy_desc.type = (ta_in) ? POWER_SUPPLY_TYPE_MAINS : > + ((usb_in) ? POWER_SUPPLY_TYPE_USB : > + POWER_SUPPLY_TYPE_BATTERY); > + charger->psy_desc.get_property = max8903_get_property; > + charger->psy_desc.properties = max8903_charger_props; > + charger->psy_desc.num_properties = ARRAY_SIZE(max8903_charger_props); > + > + platform_set_drvdata(pdev, charger); > + > + if (charger->pdata->dc_valid == false && charger->pdata->usb_valid == false) { > dev_err(dev, "No valid power sources.\n"); > return -EINVAL; > } > > - if (pdata->dc_valid) { > - if (gpio_is_valid(pdata->dok)) { > + > + if (charger->pdata->dc_valid) { > + if (gpio_is_valid(charger->pdata->dok)) { > ret = devm_gpio_request(dev, > - pdata->dok, > - data->psy_desc.name); > + charger->pdata->dok, > + charger->psy_desc.name); > if (ret) { > dev_err(dev, > "Failed GPIO request for dok: %d err %d\n", > - pdata->dok, ret); > + charger->pdata->dok, ret); I pointed the weird indentation in 4/7. Just a reminder: after fixing it in 4/7 you also have to adjust it in consecutive patches. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH v3 7/7] max8903: adds support for initiation via device tree. 2016-06-17 6:41 ` Krzysztof Kozlowski @ 2016-06-19 13:52 ` Chris Lapa 0 siblings, 0 replies; 68+ messages in thread From: Chris Lapa @ 2016-06-19 13:52 UTC (permalink / raw) To: Krzysztof Kozlowski, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm On 17/06/2016 4:41 PM, Krzysztof Kozlowski wrote: > On 06/17/2016 07:00 AM, Chris Lapa wrote: >> From: Chris Lapa <chris@lapa.com.au> >> >> Adds support for device tree to setup a max8903 battery charger. DC and USB >> validity are determined by looking the presence of the dok and uok gpios. >> >> Signed-off-by: Chris Lapa <chris@lapa.com.au> >> --- >> drivers/power/max8903_charger.c | 217 +++++++++++++++++++++++++++------------- >> 1 file changed, 145 insertions(+), 72 deletions(-) >> >> diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c >> index 5ddc667..3c59213 100644 >> --- a/drivers/power/max8903_charger.c >> +++ b/drivers/power/max8903_charger.c >> @@ -23,6 +23,9 @@ >> #include <linux/gpio.h> >> #include <linux/interrupt.h> >> #include <linux/module.h> >> +#include <linux/of.h> >> +#include <linux/of_device.h> >> +#include <linux/of_gpio.h> >> #include <linux/slab.h> >> #include <linux/power_supply.h> >> #include <linux/platform_device.h> >> @@ -75,6 +78,7 @@ static int max8903_get_property(struct power_supply *psy, >> default: >> return -EINVAL; >> } >> + >> return 0; >> } >> >> @@ -179,48 +183,116 @@ static irqreturn_t max8903_fault(int irq, void *_data) >> return IRQ_HANDLED; >> } >> >> +static struct max8903_pdata *max8903_parse_dt_data( >> + struct device *dev) >> +{ >> + struct device_node *of_node = dev->of_node; > > Common naming convention for device_node is just 'np': > struct device_node *np = dev->of_node; > > It is shorter and already spread in the kernel. > >> + struct max8903_pdata *pdata = NULL; >> + >> + if (!of_node) >> + return NULL; >> + >> + pdata = devm_kzalloc(dev, sizeof(*pdata), >> + GFP_KERNEL); > > No need to break the line. > >> + if (!pdata) >> + return NULL; >> + >> + pdata->dc_valid = false; >> + pdata->usb_valid = false; >> + >> + pdata->cen = of_get_named_gpio(of_node, "cen-gpios", 0); >> + if (!gpio_is_valid(pdata->cen)) >> + pdata->cen = -EINVAL; >> + >> + pdata->chg = of_get_named_gpio(of_node, "chg-gpios", 0); >> + if (!gpio_is_valid(pdata->chg)) >> + pdata->chg = -EINVAL; >> + >> + pdata->flt = of_get_named_gpio(of_node, "flt-gpios", 0); >> + if (!gpio_is_valid(pdata->flt)) >> + pdata->flt = -EINVAL; >> + >> + pdata->usus = of_get_named_gpio(of_node, "usus-gpios", 0); >> + if (!gpio_is_valid(pdata->usus)) >> + pdata->usus = -EINVAL; >> + >> + pdata->dcm = of_get_named_gpio(of_node, "dcm-gpios", 0); >> + if (!gpio_is_valid(pdata->dcm)) >> + pdata->dcm = -EINVAL; >> + >> + pdata->dok = of_get_named_gpio(of_node, "dok-gpios", 0); >> + if (!gpio_is_valid(pdata->dok)) >> + pdata->dok = -EINVAL; >> + else >> + pdata->dc_valid = true; >> + >> + pdata->uok = of_get_named_gpio(of_node, "uok-gpios", 0); >> + if (!gpio_is_valid(pdata->uok)) >> + pdata->uok = -EINVAL; >> + else >> + pdata->usb_valid = true; >> + >> + return pdata; >> +} >> + >> static int max8903_probe(struct platform_device *pdev) >> { >> - struct max8903_data *data; >> + struct max8903_data *charger; >> struct device *dev = &pdev->dev; >> - struct max8903_pdata *pdata = pdev->dev.platform_data; >> struct power_supply_config psy_cfg = {}; >> int ret = 0; >> int gpio; >> int ta_in = 0; >> int usb_in = 0; >> >> - if (pdata == NULL) { >> + charger = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL); >> + if (!charger) >> + return -ENOMEM; >> + >> + charger->pdata = pdev->dev.platform_data; >> + if (IS_ENABLED(CONFIG_OF) && !charger->pdata && dev->of_node) >> + charger->pdata = max8903_parse_dt_data(dev); >> + >> + if (!charger->pdata) { >> dev_err(dev, "No platform data.\n"); >> return -EINVAL; >> } >> >> - data = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL); >> - if (!data) >> - return -ENOMEM; >> + charger->dev = dev; >> >> - data->pdata = pdev->dev.platform_data; >> - data->dev = dev; >> - platform_set_drvdata(pdev, data); >> + charger->fault = false; >> + charger->ta_in = ta_in; >> + charger->usb_in = usb_in; >> >> - if (pdata->dc_valid == false && pdata->usb_valid == false) { >> + charger->psy_desc.name = "max8903_charger"; >> + charger->psy_desc.type = (ta_in) ? POWER_SUPPLY_TYPE_MAINS : >> + ((usb_in) ? POWER_SUPPLY_TYPE_USB : >> + POWER_SUPPLY_TYPE_BATTERY); >> + charger->psy_desc.get_property = max8903_get_property; >> + charger->psy_desc.properties = max8903_charger_props; >> + charger->psy_desc.num_properties = ARRAY_SIZE(max8903_charger_props); >> + >> + platform_set_drvdata(pdev, charger); >> + >> + if (charger->pdata->dc_valid == false && charger->pdata->usb_valid == false) { >> dev_err(dev, "No valid power sources.\n"); >> return -EINVAL; >> } >> >> - if (pdata->dc_valid) { >> - if (gpio_is_valid(pdata->dok)) { >> + >> + if (charger->pdata->dc_valid) { >> + if (gpio_is_valid(charger->pdata->dok)) { >> ret = devm_gpio_request(dev, >> - pdata->dok, >> - data->psy_desc.name); >> + charger->pdata->dok, >> + charger->psy_desc.name); >> if (ret) { >> dev_err(dev, >> "Failed GPIO request for dok: %d err %d\n", >> - pdata->dok, ret); >> + charger->pdata->dok, ret); > > > I pointed the weird indentation in 4/7. Just a reminder: after fixing it > in 4/7 you also have to adjust it in consecutive patches. > > Best regards, > Krzysztof > Fixed in v4. Will send later once I have a chance to test on the hardware. Thanks, Chris ^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH v4 0/7] max8903: Add device tree support and misc fixes 2016-06-02 6:44 ` [PATCH 2/2] max8903: cleans up confusing relationship between dc_valid, dok and dcm chris 2016-06-10 12:32 ` [PATCH v2 0/4] max8903: Add device tree support and logic fixup Chris Lapa 2016-06-17 5:00 ` [PATCH v3 0/7] max8903: Add device tree support and misc fixes Chris Lapa @ 2016-06-19 22:27 ` Chris Lapa 2016-06-19 22:27 ` [PATCH v4 1/7] max8903: adds documentation for device tree bindings Chris Lapa ` (6 more replies) 2016-06-24 2:26 ` [PATCH v5 0/7] max8903: Add device tree support and misc fixes Chris Lapa 3 siblings, 7 replies; 68+ messages in thread From: Chris Lapa @ 2016-06-19 22:27 UTC (permalink / raw) To: k.kozlowski, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm, Chris Lapa From: Chris Lapa <chris@lapa.com.au> This patch set adds device tree support for the MAX8903 battery charger. It also cleans up logic with dc_valid, dok and dcm pins as well as fixing up validity checking of gpios. I verified these patches work on a board I have here, which uses the DC power side (not the USB portition) of the MAX8903. Changes v3 -> v4: * Fixed formatting, such as multiline strings and indentation mistakes * Moved gpio setup code into max8903_setup_gpios() in 3/7 * Fixed typo in 5/7 * Renamed of_node to np in 7/7 Changes v2 -> v3: * Separate requesting of gpio's into its own commit * Fixed up validity checking of GPIO's * Remove dc_valid and usb_valid from device tree * Remove some unncessary init to psy_cfg.num_supplicants and psy_cfg.supplied_to * Reorder patches so device tree implementation is final patch Changes v1 -> v2: * Separate DT bindings documentation into its own commit * Add maxim prefix to DT compatible field * Add gpios suffix to gpio's in DT * Remove malloc failed error message Chris Lapa (7): max8903: adds documentation for device tree bindings. max8903: store pointer to pdata instead of copying it. max8903: cleans up confusing relationship between dc_valid, dok and dcm. max8903: adds requesting of gpios. max8903: removes non zero validity checks on gpios. max8903: remove unnecessary 'out of memory' error message. max8903: adds support for initiation via device tree .../devicetree/bindings/power/max8903-charger.txt | 25 +++ drivers/power/max8903_charger.c | 239 +++++++++++++++------ include/linux/power/max8903_charger.h | 6 +- 3 files changed, 204 insertions(+), 66 deletions(-) create mode 100644 Documentation/devicetree/bindings/power/max8903-charger.txt -- 1.9.1 ^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH v4 1/7] max8903: adds documentation for device tree bindings. 2016-06-19 22:27 ` [PATCH v4 0/7] max8903: Add device tree support and misc fixes Chris Lapa @ 2016-06-19 22:27 ` Chris Lapa 2016-06-20 12:17 ` Krzysztof Kozlowski 2016-06-19 22:27 ` [PATCH v4 2/7] max8903: store pointer to pdata instead of copying it Chris Lapa ` (5 subsequent siblings) 6 siblings, 1 reply; 68+ messages in thread From: Chris Lapa @ 2016-06-19 22:27 UTC (permalink / raw) To: k.kozlowski, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm, Chris Lapa From: Chris Lapa <chris@lapa.com.au> Signed-off-by: Chris Lapa <chris@lapa.com.au> --- .../devicetree/bindings/power/max8903-charger.txt | 25 ++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/max8903-charger.txt diff --git a/Documentation/devicetree/bindings/power/max8903-charger.txt b/Documentation/devicetree/bindings/power/max8903-charger.txt new file mode 100644 index 0000000..aea1dd2a --- /dev/null +++ b/Documentation/devicetree/bindings/power/max8903-charger.txt @@ -0,0 +1,25 @@ +Maxim Semiconductor MAX8903 Battery Charger bindings + +Required properties: +- compatible: "maxim,max8903-charger" for MAX8903 Battery Charger +- dok-gpios: Valid DC power has been detected, optional if uok-gpios is provided +- uok-gpios: Valid USB power has been detected, optional if dok-gpios is provided + +Optional properties: +- cen-gpios: Charge enable pin +- chg-gpios: Charger status pin +- flt-gpios: Fault pin +- dcm-gpios: Current limit mode setting (DC or USB) +- usus-gpios: USB suspend pin + + +Example: + + max8903-charger { + compatible = "maxim,max8903-charger"; + dok-gpios = <&gpio2 3 GPIO_ACTIVE_LOW>; + flt-gpios = <&gpio2 2 GPIO_ACTIVE_LOW>; + chg-gpios = <&gpio3 15 GPIO_ACTIVE_LOW>; + cen-gpios = <&gpio2 5 GPIO_ACTIVE_LOW>; + status = "okay"; + }; -- 1.9.1 ^ permalink raw reply related [flat|nested] 68+ messages in thread
* Re: [PATCH v4 1/7] max8903: adds documentation for device tree bindings. 2016-06-19 22:27 ` [PATCH v4 1/7] max8903: adds documentation for device tree bindings Chris Lapa @ 2016-06-20 12:17 ` Krzysztof Kozlowski 0 siblings, 0 replies; 68+ messages in thread From: Krzysztof Kozlowski @ 2016-06-20 12:17 UTC (permalink / raw) To: Chris Lapa, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm On 06/20/2016 12:27 AM, Chris Lapa wrote: > From: Chris Lapa <chris@lapa.com.au> > > Signed-off-by: Chris Lapa <chris@lapa.com.au> I already reviewed this patch. When resubmitting please include gathered tags (reviewed, acked, tested) after your signed-off-by. Just put them in chronological order: Signed-off-by: Chris Lapa <chris@lapa.com.au> Reviewed-by: ... ... etc For the record: Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> Best regards, Krzysztof ^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH v4 2/7] max8903: store pointer to pdata instead of copying it. 2016-06-19 22:27 ` [PATCH v4 0/7] max8903: Add device tree support and misc fixes Chris Lapa 2016-06-19 22:27 ` [PATCH v4 1/7] max8903: adds documentation for device tree bindings Chris Lapa @ 2016-06-19 22:27 ` Chris Lapa 2016-06-20 12:18 ` Krzysztof Kozlowski 2016-06-19 22:27 ` [PATCH v4 3/7] max8903: cleans up confusing relationship between dc_valid, dok and dcm Chris Lapa ` (4 subsequent siblings) 6 siblings, 1 reply; 68+ messages in thread From: Chris Lapa @ 2016-06-19 22:27 UTC (permalink / raw) To: k.kozlowski, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm, Chris Lapa From: Chris Lapa <chris@lapa.com.au> Stores pointer to pdata because it easily allows pdata to reference either platform data or in the future device tree data. Signed-off-by: Chris Lapa <chris@lapa.com.au> --- drivers/power/max8903_charger.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c index 17876ca..0a5b0e1 100644 --- a/drivers/power/max8903_charger.c +++ b/drivers/power/max8903_charger.c @@ -29,7 +29,7 @@ #include <linux/power/max8903_charger.h> struct max8903_data { - struct max8903_pdata pdata; + struct max8903_pdata *pdata; struct device *dev; struct power_supply *psy; struct power_supply_desc psy_desc; @@ -53,8 +53,8 @@ static int max8903_get_property(struct power_supply *psy, switch (psp) { case POWER_SUPPLY_PROP_STATUS: val->intval = POWER_SUPPLY_STATUS_UNKNOWN; - if (data->pdata.chg) { - if (gpio_get_value(data->pdata.chg) == 0) + if (data->pdata->chg) { + if (gpio_get_value(data->pdata->chg) == 0) val->intval = POWER_SUPPLY_STATUS_CHARGING; else if (data->usb_in || data->ta_in) val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING; @@ -81,7 +81,7 @@ static int max8903_get_property(struct power_supply *psy, static irqreturn_t max8903_dcin(int irq, void *_data) { struct max8903_data *data = _data; - struct max8903_pdata *pdata = &data->pdata; + struct max8903_pdata *pdata = data->pdata; bool ta_in; enum power_supply_type old_type; @@ -122,7 +122,7 @@ static irqreturn_t max8903_dcin(int irq, void *_data) static irqreturn_t max8903_usbin(int irq, void *_data) { struct max8903_data *data = _data; - struct max8903_pdata *pdata = &data->pdata; + struct max8903_pdata *pdata = data->pdata; bool usb_in; enum power_supply_type old_type; @@ -161,7 +161,7 @@ static irqreturn_t max8903_usbin(int irq, void *_data) static irqreturn_t max8903_fault(int irq, void *_data) { struct max8903_data *data = _data; - struct max8903_pdata *pdata = &data->pdata; + struct max8903_pdata *pdata = data->pdata; bool fault; fault = gpio_get_value(pdata->flt) ? false : true; @@ -190,12 +190,18 @@ static int max8903_probe(struct platform_device *pdev) int ta_in = 0; int usb_in = 0; + if (pdata == NULL) { + dev_err(dev, "No platform data.\n"); + return -EINVAL; + } + data = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL); if (data == NULL) { dev_err(dev, "Cannot allocate memory.\n"); return -ENOMEM; } - memcpy(&data->pdata, pdata, sizeof(struct max8903_pdata)); + + data->pdata = pdev->dev.platform_data; data->dev = dev; platform_set_drvdata(pdev, data); -- 1.9.1 ^ permalink raw reply related [flat|nested] 68+ messages in thread
* Re: [PATCH v4 2/7] max8903: store pointer to pdata instead of copying it. 2016-06-19 22:27 ` [PATCH v4 2/7] max8903: store pointer to pdata instead of copying it Chris Lapa @ 2016-06-20 12:18 ` Krzysztof Kozlowski 0 siblings, 0 replies; 68+ messages in thread From: Krzysztof Kozlowski @ 2016-06-20 12:18 UTC (permalink / raw) To: Chris Lapa, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm On 06/20/2016 12:27 AM, Chris Lapa wrote: > From: Chris Lapa <chris@lapa.com.au> > > Stores pointer to pdata because it easily allows pdata to reference > either platform data or in the future device tree data. > > Signed-off-by: Chris Lapa <chris@lapa.com.au> > --- > drivers/power/max8903_charger.c | 20 +++++++++++++------- > 1 file changed, 13 insertions(+), 7 deletions(-) The same as in #1. For the record: Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> BR. Krzysztof ^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH v4 3/7] max8903: cleans up confusing relationship between dc_valid, dok and dcm. 2016-06-19 22:27 ` [PATCH v4 0/7] max8903: Add device tree support and misc fixes Chris Lapa 2016-06-19 22:27 ` [PATCH v4 1/7] max8903: adds documentation for device tree bindings Chris Lapa 2016-06-19 22:27 ` [PATCH v4 2/7] max8903: store pointer to pdata instead of copying it Chris Lapa @ 2016-06-19 22:27 ` Chris Lapa 2016-06-20 12:18 ` Krzysztof Kozlowski 2016-06-19 22:27 ` [PATCH v4 4/7] max8903: adds requesting of gpios Chris Lapa ` (3 subsequent siblings) 6 siblings, 1 reply; 68+ messages in thread From: Chris Lapa @ 2016-06-19 22:27 UTC (permalink / raw) To: k.kozlowski, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm, Chris Lapa From: Chris Lapa <chris@lapa.com.au> The max8903_charger.h file indicated that dcm and dok were not optional when dc_valid is set. It makes sense to have dok as a compulsory pin when dc_valid is given. However dcm can be optionally wired to a fixed level especially when the circuit is configured for dc power exclusively. The previous implementation already allowed for this somewhat, however no error was given if dok wasn't given whilst dc_valid was. The new implementation enforces dok presence when dc_valid is given. Whilst allowing dcm to be optional. Signed-off-by: Chris Lapa <chris@lapa.com.au> --- drivers/power/max8903_charger.c | 22 +++++++++------------- include/linux/power/max8903_charger.h | 6 +++--- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c index 0a5b0e1..6ec705f 100644 --- a/drivers/power/max8903_charger.c +++ b/drivers/power/max8903_charger.c @@ -211,27 +211,23 @@ static int max8903_probe(struct platform_device *pdev) } if (pdata->dc_valid) { - if (pdata->dok && gpio_is_valid(pdata->dok) && - pdata->dcm && gpio_is_valid(pdata->dcm)) { + if (pdata->dok && gpio_is_valid(pdata->dok)) { gpio = pdata->dok; /* PULL_UPed Interrupt */ ta_in = gpio_get_value(gpio) ? 0 : 1; + } else { + dev_err(dev, "When DC is wired, DOK should be wired as well.\n"); + return -EINVAL; + } + } + if (pdata->dcm) { + if (gpio_is_valid(pdata->dcm)) { gpio = pdata->dcm; /* Output */ gpio_set_value(gpio, ta_in); } else { - dev_err(dev, "When DC is wired, DOK and DCM should" - " be wired as well.\n"); + dev_err(dev, "Invalid pin: dcm.\n"); return -EINVAL; } - } else { - if (pdata->dcm) { - if (gpio_is_valid(pdata->dcm)) - gpio_set_value(pdata->dcm, 0); - else { - dev_err(dev, "Invalid pin: dcm.\n"); - return -EINVAL; - } - } } if (pdata->usb_valid) { diff --git a/include/linux/power/max8903_charger.h b/include/linux/power/max8903_charger.h index 24f51db..89d3f1c 100644 --- a/include/linux/power/max8903_charger.h +++ b/include/linux/power/max8903_charger.h @@ -26,8 +26,8 @@ struct max8903_pdata { /* * GPIOs - * cen, chg, flt, and usus are optional. - * dok, dcm, and uok are not optional depending on the status of + * cen, chg, flt, dcm and usus are optional. + * dok and uok are not optional depending on the status of * dc_valid and usb_valid. */ int cen; /* Charger Enable input */ @@ -41,7 +41,7 @@ struct max8903_pdata { /* * DC(Adapter/TA) is wired * When dc_valid is true, - * dok and dcm should be valid. + * dok should be valid. * * At least one of dc_valid or usb_valid should be true. */ -- 1.9.1 ^ permalink raw reply related [flat|nested] 68+ messages in thread
* Re: [PATCH v4 3/7] max8903: cleans up confusing relationship between dc_valid, dok and dcm. 2016-06-19 22:27 ` [PATCH v4 3/7] max8903: cleans up confusing relationship between dc_valid, dok and dcm Chris Lapa @ 2016-06-20 12:18 ` Krzysztof Kozlowski 0 siblings, 0 replies; 68+ messages in thread From: Krzysztof Kozlowski @ 2016-06-20 12:18 UTC (permalink / raw) To: Chris Lapa, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm On 06/20/2016 12:27 AM, Chris Lapa wrote: > From: Chris Lapa <chris@lapa.com.au> > > The max8903_charger.h file indicated that dcm and dok were not optional > when dc_valid is set. > > It makes sense to have dok as a compulsory pin when dc_valid is given. > However dcm can be optionally wired to a fixed level especially when the > circuit is configured for dc power exclusively. > > The previous implementation already allowed for this somewhat, however no > error was given if dok wasn't given whilst dc_valid was. > > The new implementation enforces dok presence when dc_valid is given. Whilst > allowing dcm to be optional. > > Signed-off-by: Chris Lapa <chris@lapa.com.au> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> BR. Krzysztof ^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH v4 4/7] max8903: adds requesting of gpios. 2016-06-19 22:27 ` [PATCH v4 0/7] max8903: Add device tree support and misc fixes Chris Lapa ` (2 preceding siblings ...) 2016-06-19 22:27 ` [PATCH v4 3/7] max8903: cleans up confusing relationship between dc_valid, dok and dcm Chris Lapa @ 2016-06-19 22:27 ` Chris Lapa 2016-06-20 12:20 ` Krzysztof Kozlowski 2016-06-19 22:27 ` [PATCH v4 5/7] max8903: removes non zero validity checks on gpios Chris Lapa ` (2 subsequent siblings) 6 siblings, 1 reply; 68+ messages in thread From: Chris Lapa @ 2016-06-19 22:27 UTC (permalink / raw) To: k.kozlowski, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm, Chris Lapa From: Chris Lapa <chris@lapa.com.au> This change ensures all gpios are available for the driver to use and also splits off gpio setup into its own function for readability. Signed-off-by: Chris Lapa <chris@lapa.com.au> --- drivers/power/max8903_charger.c | 136 ++++++++++++++++++++++++++++++---------- 1 file changed, 102 insertions(+), 34 deletions(-) diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c index 6ec705f..3f35593 100644 --- a/drivers/power/max8903_charger.c +++ b/drivers/power/max8903_charger.c @@ -179,39 +179,27 @@ static irqreturn_t max8903_fault(int irq, void *_data) return IRQ_HANDLED; } -static int max8903_probe(struct platform_device *pdev) +static int max8903_setup_gpios(struct platform_device *pdev) { - struct max8903_data *data; + struct max8903_data *data = platform_get_drvdata(pdev); struct device *dev = &pdev->dev; struct max8903_pdata *pdata = pdev->dev.platform_data; - struct power_supply_config psy_cfg = {}; int ret = 0; int gpio; int ta_in = 0; int usb_in = 0; - if (pdata == NULL) { - dev_err(dev, "No platform data.\n"); - return -EINVAL; - } - - data = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL); - if (data == NULL) { - dev_err(dev, "Cannot allocate memory.\n"); - return -ENOMEM; - } - - data->pdata = pdev->dev.platform_data; - data->dev = dev; - platform_set_drvdata(pdev, data); - - if (pdata->dc_valid == false && pdata->usb_valid == false) { - dev_err(dev, "No valid power sources.\n"); - return -EINVAL; - } - if (pdata->dc_valid) { if (pdata->dok && gpio_is_valid(pdata->dok)) { + ret = devm_gpio_request(dev, pdata->dok, + data->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for dok: %d err %d\n", + pdata->dok, ret); + return ret; + } + gpio = pdata->dok; /* PULL_UPed Interrupt */ ta_in = gpio_get_value(gpio) ? 0 : 1; } else { @@ -222,6 +210,15 @@ static int max8903_probe(struct platform_device *pdev) if (pdata->dcm) { if (gpio_is_valid(pdata->dcm)) { + ret = devm_gpio_request(dev, pdata->dcm, + data->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for dcm: %d err %d\n", + pdata->dcm, ret); + return ret; + } + gpio = pdata->dcm; /* Output */ gpio_set_value(gpio, ta_in); } else { @@ -232,6 +229,15 @@ static int max8903_probe(struct platform_device *pdev) if (pdata->usb_valid) { if (pdata->uok && gpio_is_valid(pdata->uok)) { + ret = devm_gpio_request(dev, pdata->uok, + data->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for uok: %d err %d\n", + pdata->uok, ret); + return ret; + } + gpio = pdata->uok; usb_in = gpio_get_value(gpio) ? 0 : 1; } else { @@ -243,6 +249,15 @@ static int max8903_probe(struct platform_device *pdev) if (pdata->cen) { if (gpio_is_valid(pdata->cen)) { + ret = devm_gpio_request(dev, pdata->cen, + data->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for cen: %d err %d\n", + pdata->cen, ret); + return ret; + } + gpio_set_value(pdata->cen, (ta_in || usb_in) ? 0 : 1); } else { dev_err(dev, "Invalid pin: cen.\n"); @@ -251,23 +266,41 @@ static int max8903_probe(struct platform_device *pdev) } if (pdata->chg) { - if (!gpio_is_valid(pdata->chg)) { - dev_err(dev, "Invalid pin: chg.\n"); - return -EINVAL; + if (gpio_is_valid(pdata->chg)) { + ret = devm_gpio_request(dev, pdata->chg, + data->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for chg: %d err %d\n", + pdata->chg, ret); + return ret; + } } } if (pdata->flt) { - if (!gpio_is_valid(pdata->flt)) { - dev_err(dev, "Invalid pin: flt.\n"); - return -EINVAL; + if (gpio_is_valid(pdata->flt)) { + ret = devm_gpio_request(dev, pdata->flt, + data->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for flt: %d err %d\n", + pdata->flt, ret); + return ret; + } } } if (pdata->usus) { - if (!gpio_is_valid(pdata->usus)) { - dev_err(dev, "Invalid pin: usus.\n"); - return -EINVAL; + if (gpio_is_valid(pdata->usus)) { + ret = devm_gpio_request(dev, pdata->usus, + data->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for usus: %d err %d\n", + pdata->usus, ret); + return ret; + } } } @@ -275,9 +308,44 @@ static int max8903_probe(struct platform_device *pdev) data->ta_in = ta_in; data->usb_in = usb_in; + return 0; +} + +static int max8903_probe(struct platform_device *pdev) +{ + struct max8903_data *data; + struct device *dev = &pdev->dev; + struct max8903_pdata *pdata = pdev->dev.platform_data; + struct power_supply_config psy_cfg = {}; + int ret = 0; + + if (pdata == NULL) { + dev_err(dev, "No platform data.\n"); + return -EINVAL; + } + + data = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL); + if (data == NULL) { + dev_err(dev, "Cannot allocate memory.\n"); + return -ENOMEM; + } + + data->pdata = pdev->dev.platform_data; + data->dev = dev; + platform_set_drvdata(pdev, data); + + if (pdata->dc_valid == false && pdata->usb_valid == false) { + dev_err(dev, "No valid power sources.\n"); + return -EINVAL; + } + + ret = max8903_setup_gpios(pdev); + if (ret) + return ret; + data->psy_desc.name = "max8903_charger"; - data->psy_desc.type = (ta_in) ? POWER_SUPPLY_TYPE_MAINS : - ((usb_in) ? POWER_SUPPLY_TYPE_USB : + data->psy_desc.type = (data->ta_in) ? POWER_SUPPLY_TYPE_MAINS : + ((data->usb_in) ? POWER_SUPPLY_TYPE_USB : POWER_SUPPLY_TYPE_BATTERY); data->psy_desc.get_property = max8903_get_property; data->psy_desc.properties = max8903_charger_props; -- 1.9.1 ^ permalink raw reply related [flat|nested] 68+ messages in thread
* Re: [PATCH v4 4/7] max8903: adds requesting of gpios. 2016-06-19 22:27 ` [PATCH v4 4/7] max8903: adds requesting of gpios Chris Lapa @ 2016-06-20 12:20 ` Krzysztof Kozlowski 0 siblings, 0 replies; 68+ messages in thread From: Krzysztof Kozlowski @ 2016-06-20 12:20 UTC (permalink / raw) To: Chris Lapa, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm On 06/20/2016 12:27 AM, Chris Lapa wrote: > From: Chris Lapa <chris@lapa.com.au> > > This change ensures all gpios are available for the driver to use and also > splits off gpio setup into its own function for readability. > > Signed-off-by: Chris Lapa <chris@lapa.com.au> > --- > drivers/power/max8903_charger.c | 136 ++++++++++++++++++++++++++++++---------- > 1 file changed, 102 insertions(+), 34 deletions(-) > Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> Best regards, Krzysztof ^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH v4 5/7] max8903: removes non zero validity checks on gpios. 2016-06-19 22:27 ` [PATCH v4 0/7] max8903: Add device tree support and misc fixes Chris Lapa ` (3 preceding siblings ...) 2016-06-19 22:27 ` [PATCH v4 4/7] max8903: adds requesting of gpios Chris Lapa @ 2016-06-19 22:27 ` Chris Lapa 2016-06-20 12:21 ` Krzysztof Kozlowski 2016-06-19 22:27 ` [PATCH v4 6/7] max8903: remove unnecessary 'out of memory' error message Chris Lapa 2016-06-19 22:27 ` [PATCH v4 7/7] max8903: adds support for initiation via device tree Chris Lapa 6 siblings, 1 reply; 68+ messages in thread From: Chris Lapa @ 2016-06-19 22:27 UTC (permalink / raw) To: k.kozlowski, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm, Chris Lapa From: Chris Lapa <chris@lapa.com.au> Prior to this commit a zero gpio was treated as invalid. Whereas gpio_is_valid() will treat a zero gpio as valid. This commit removes the confusion and explicitly uses gpio_is_valid() throughout. Which in turn results in several of the error messages becoming redundant and thus removed. Signed-off-by: Chris Lapa <chris@lapa.com.au> --- drivers/power/max8903_charger.c | 115 ++++++++++++++++------------------------ 1 file changed, 47 insertions(+), 68 deletions(-) diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c index 3f35593..643a87a 100644 --- a/drivers/power/max8903_charger.c +++ b/drivers/power/max8903_charger.c @@ -53,7 +53,7 @@ static int max8903_get_property(struct power_supply *psy, switch (psp) { case POWER_SUPPLY_PROP_STATUS: val->intval = POWER_SUPPLY_STATUS_UNKNOWN; - if (data->pdata->chg) { + if (gpio_is_valid(data->pdata->chg)) { if (gpio_get_value(data->pdata->chg) == 0) val->intval = POWER_SUPPLY_STATUS_CHARGING; else if (data->usb_in || data->ta_in) @@ -93,11 +93,11 @@ static irqreturn_t max8903_dcin(int irq, void *_data) data->ta_in = ta_in; /* Set Current-Limit-Mode 1:DC 0:USB */ - if (pdata->dcm) + if (gpio_is_valid(pdata->dcm)) gpio_set_value(pdata->dcm, ta_in ? 1 : 0); /* Charger Enable / Disable (cen is negated) */ - if (pdata->cen) + if (gpio_is_valid(pdata->cen)) gpio_set_value(pdata->cen, ta_in ? 0 : (data->usb_in ? 0 : 1)); @@ -136,7 +136,7 @@ static irqreturn_t max8903_usbin(int irq, void *_data) /* Do not touch Current-Limit-Mode */ /* Charger Enable / Disable (cen is negated) */ - if (pdata->cen) + if (gpio_is_valid(pdata->cen)) gpio_set_value(pdata->cen, usb_in ? 0 : (data->ta_in ? 0 : 1)); @@ -190,7 +190,7 @@ static int max8903_setup_gpios(struct platform_device *pdev) int usb_in = 0; if (pdata->dc_valid) { - if (pdata->dok && gpio_is_valid(pdata->dok)) { + if (gpio_is_valid(pdata->dok)) { ret = devm_gpio_request(dev, pdata->dok, data->psy_desc.name); if (ret) { @@ -208,27 +208,21 @@ static int max8903_setup_gpios(struct platform_device *pdev) } } - if (pdata->dcm) { - if (gpio_is_valid(pdata->dcm)) { - ret = devm_gpio_request(dev, pdata->dcm, - data->psy_desc.name); - if (ret) { - dev_err(dev, - "Failed GPIO request for dcm: %d err %d\n", - pdata->dcm, ret); - return ret; - } - - gpio = pdata->dcm; /* Output */ - gpio_set_value(gpio, ta_in); - } else { - dev_err(dev, "Invalid pin: dcm.\n"); - return -EINVAL; + if (gpio_is_valid(pdata->dcm)) { + ret = devm_gpio_request(dev, pdata->dcm, data->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for dcm: %d err %d\n", + pdata->dcm, ret); + return ret; } + + gpio = pdata->dcm; /* Output */ + gpio_set_value(gpio, ta_in); } if (pdata->usb_valid) { - if (pdata->uok && gpio_is_valid(pdata->uok)) { + if (gpio_is_valid(pdata->uok)) { ret = devm_gpio_request(dev, pdata->uok, data->psy_desc.name); if (ret) { @@ -247,60 +241,45 @@ static int max8903_setup_gpios(struct platform_device *pdev) } } - if (pdata->cen) { - if (gpio_is_valid(pdata->cen)) { - ret = devm_gpio_request(dev, pdata->cen, - data->psy_desc.name); - if (ret) { - dev_err(dev, - "Failed GPIO request for cen: %d err %d\n", - pdata->cen, ret); - return ret; - } - - gpio_set_value(pdata->cen, (ta_in || usb_in) ? 0 : 1); - } else { - dev_err(dev, "Invalid pin: cen.\n"); - return -EINVAL; + if (gpio_is_valid(pdata->cen)) { + ret = devm_gpio_request(dev, pdata->cen, data->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for cen: %d err %d\n", + pdata->cen, ret); + return ret; } + + gpio_set_value(pdata->cen, (ta_in || usb_in) ? 0 : 1); } - if (pdata->chg) { - if (gpio_is_valid(pdata->chg)) { - ret = devm_gpio_request(dev, pdata->chg, - data->psy_desc.name); - if (ret) { - dev_err(dev, - "Failed GPIO request for chg: %d err %d\n", - pdata->chg, ret); - return ret; - } + if (gpio_is_valid(pdata->chg)) { + ret = devm_gpio_request(dev, pdata->chg, data->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for chg: %d err %d\n", + pdata->chg, ret); + return ret; } } - if (pdata->flt) { - if (gpio_is_valid(pdata->flt)) { - ret = devm_gpio_request(dev, pdata->flt, - data->psy_desc.name); - if (ret) { - dev_err(dev, - "Failed GPIO request for flt: %d err %d\n", - pdata->flt, ret); - return ret; - } + if (gpio_is_valid(pdata->flt)) { + ret = devm_gpio_request(dev, pdata->flt, data->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for flt: %d err %d\n", + pdata->flt, ret); + return ret; } } - if (pdata->usus) { - if (gpio_is_valid(pdata->usus)) { - ret = devm_gpio_request(dev, pdata->usus, - data->psy_desc.name); - if (ret) { - dev_err(dev, - "Failed GPIO request for usus: %d err %d\n", - pdata->usus, ret); - return ret; - } + if (gpio_is_valid(pdata->usus)) { + ret = devm_gpio_request(dev, pdata->usus, data->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for usus: %d err %d\n", + pdata->usus, ret); + return ret; } } @@ -385,7 +364,7 @@ static int max8903_probe(struct platform_device *pdev) } } - if (pdata->flt) { + if (gpio_is_valid(pdata->flt)) { ret = devm_request_threaded_irq(dev, gpio_to_irq(pdata->flt), NULL, max8903_fault, IRQF_TRIGGER_FALLING | -- 1.9.1 ^ permalink raw reply related [flat|nested] 68+ messages in thread
* Re: [PATCH v4 5/7] max8903: removes non zero validity checks on gpios. 2016-06-19 22:27 ` [PATCH v4 5/7] max8903: removes non zero validity checks on gpios Chris Lapa @ 2016-06-20 12:21 ` Krzysztof Kozlowski 0 siblings, 0 replies; 68+ messages in thread From: Krzysztof Kozlowski @ 2016-06-20 12:21 UTC (permalink / raw) To: Chris Lapa, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm On 06/20/2016 12:27 AM, Chris Lapa wrote: > From: Chris Lapa <chris@lapa.com.au> > > Prior to this commit a zero gpio was treated as invalid. Whereas > gpio_is_valid() will treat a zero gpio as valid. > > This commit removes the confusion and explicitly uses gpio_is_valid() > throughout. Which in turn results in several of the error messages becoming > redundant and thus removed. > > Signed-off-by: Chris Lapa <chris@lapa.com.au> The same as in #1, I already reviewed it. For the record: Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> BR. Krzysztof ^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH v4 6/7] max8903: remove unnecessary 'out of memory' error message. 2016-06-19 22:27 ` [PATCH v4 0/7] max8903: Add device tree support and misc fixes Chris Lapa ` (4 preceding siblings ...) 2016-06-19 22:27 ` [PATCH v4 5/7] max8903: removes non zero validity checks on gpios Chris Lapa @ 2016-06-19 22:27 ` Chris Lapa 2016-06-20 12:22 ` Krzysztof Kozlowski 2016-06-19 22:27 ` [PATCH v4 7/7] max8903: adds support for initiation via device tree Chris Lapa 6 siblings, 1 reply; 68+ messages in thread From: Chris Lapa @ 2016-06-19 22:27 UTC (permalink / raw) To: k.kozlowski, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm, Chris Lapa From: Chris Lapa <chris@lapa.com.au> Remove the 'out of memory' error message as it is printed by the core. Signed-off-by: Chris Lapa <chris@lapa.com.au> --- drivers/power/max8903_charger.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c index 643a87a..9453bbf 100644 --- a/drivers/power/max8903_charger.c +++ b/drivers/power/max8903_charger.c @@ -304,10 +304,8 @@ static int max8903_probe(struct platform_device *pdev) } data = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL); - if (data == NULL) { - dev_err(dev, "Cannot allocate memory.\n"); + if (!data) return -ENOMEM; - } data->pdata = pdev->dev.platform_data; data->dev = dev; -- 1.9.1 ^ permalink raw reply related [flat|nested] 68+ messages in thread
* Re: [PATCH v4 6/7] max8903: remove unnecessary 'out of memory' error message. 2016-06-19 22:27 ` [PATCH v4 6/7] max8903: remove unnecessary 'out of memory' error message Chris Lapa @ 2016-06-20 12:22 ` Krzysztof Kozlowski 0 siblings, 0 replies; 68+ messages in thread From: Krzysztof Kozlowski @ 2016-06-20 12:22 UTC (permalink / raw) To: Chris Lapa, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm On 06/20/2016 12:27 AM, Chris Lapa wrote: > From: Chris Lapa <chris@lapa.com.au> > > Remove the 'out of memory' error message as it is printed by the core. > > Signed-off-by: Chris Lapa <chris@lapa.com.au> The same as in #1, I already reviewed it. For the record: Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> BR. Krzysztof ^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH v4 7/7] max8903: adds support for initiation via device tree 2016-06-19 22:27 ` [PATCH v4 0/7] max8903: Add device tree support and misc fixes Chris Lapa ` (5 preceding siblings ...) 2016-06-19 22:27 ` [PATCH v4 6/7] max8903: remove unnecessary 'out of memory' error message Chris Lapa @ 2016-06-19 22:27 ` Chris Lapa 2016-06-20 12:23 ` Krzysztof Kozlowski 6 siblings, 1 reply; 68+ messages in thread From: Chris Lapa @ 2016-06-19 22:27 UTC (permalink / raw) To: k.kozlowski, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm, Chris Lapa From: Chris Lapa <chris@lapa.com.au> Adds support for device tree to setup a max8903 battery charger. DC and USB validity are determined by looking the presence of the dok and uok gpios. Signed-off-by: Chris Lapa <chris@lapa.com.au> --- drivers/power/max8903_charger.c | 78 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 72 insertions(+), 6 deletions(-) diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c index 9453bbf..47e3929 100644 --- a/drivers/power/max8903_charger.c +++ b/drivers/power/max8903_charger.c @@ -23,6 +23,9 @@ #include <linux/gpio.h> #include <linux/interrupt.h> #include <linux/module.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/of_gpio.h> #include <linux/slab.h> #include <linux/power_supply.h> #include <linux/platform_device.h> @@ -75,6 +78,7 @@ static int max8903_get_property(struct power_supply *psy, default: return -EINVAL; } + return 0; } @@ -179,6 +183,56 @@ static irqreturn_t max8903_fault(int irq, void *_data) return IRQ_HANDLED; } +static struct max8903_pdata *max8903_parse_dt_data(struct device *dev) +{ + struct device_node *np = dev->of_node; + struct max8903_pdata *pdata = NULL; + + if (!np) + return NULL; + + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); + if (!pdata) + return NULL; + + pdata->dc_valid = false; + pdata->usb_valid = false; + + pdata->cen = of_get_named_gpio(np, "cen-gpios", 0); + if (!gpio_is_valid(pdata->cen)) + pdata->cen = -EINVAL; + + pdata->chg = of_get_named_gpio(np, "chg-gpios", 0); + if (!gpio_is_valid(pdata->chg)) + pdata->chg = -EINVAL; + + pdata->flt = of_get_named_gpio(np, "flt-gpios", 0); + if (!gpio_is_valid(pdata->flt)) + pdata->flt = -EINVAL; + + pdata->usus = of_get_named_gpio(np, "usus-gpios", 0); + if (!gpio_is_valid(pdata->usus)) + pdata->usus = -EINVAL; + + pdata->dcm = of_get_named_gpio(np, "dcm-gpios", 0); + if (!gpio_is_valid(pdata->dcm)) + pdata->dcm = -EINVAL; + + pdata->dok = of_get_named_gpio(np, "dok-gpios", 0); + if (!gpio_is_valid(pdata->dok)) + pdata->dok = -EINVAL; + else + pdata->dc_valid = true; + + pdata->uok = of_get_named_gpio(np, "uok-gpios", 0); + if (!gpio_is_valid(pdata->uok)) + pdata->uok = -EINVAL; + else + pdata->usb_valid = true; + + return pdata; +} + static int max8903_setup_gpios(struct platform_device *pdev) { struct max8903_data *data = platform_get_drvdata(pdev); @@ -298,16 +352,20 @@ static int max8903_probe(struct platform_device *pdev) struct power_supply_config psy_cfg = {}; int ret = 0; - if (pdata == NULL) { - dev_err(dev, "No platform data.\n"); - return -EINVAL; - } - data = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL); if (!data) return -ENOMEM; - data->pdata = pdev->dev.platform_data; + if (IS_ENABLED(CONFIG_OF) && !pdata && dev->of_node) + pdata = max8903_parse_dt_data(dev); + + if (!pdata) { + dev_err(dev, "No platform data.\n"); + return -EINVAL; + } + + pdev->dev.platform_data = pdata; + data->pdata = pdata; data->dev = dev; platform_set_drvdata(pdev, data); @@ -328,6 +386,7 @@ static int max8903_probe(struct platform_device *pdev) data->psy_desc.properties = max8903_charger_props; data->psy_desc.num_properties = ARRAY_SIZE(max8903_charger_props); + psy_cfg.of_node = dev->of_node; psy_cfg.drv_data = data; data->psy = devm_power_supply_register(dev, &data->psy_desc, &psy_cfg); @@ -378,10 +437,17 @@ static int max8903_probe(struct platform_device *pdev) return 0; } +static const struct of_device_id max8903_match_ids[] = { + { .compatible = "maxim,max8903-charger", }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, max8903_match_ids); + static struct platform_driver max8903_driver = { .probe = max8903_probe, .driver = { .name = "max8903-charger", + .of_match_table = max8903_match_ids }, }; -- 1.9.1 ^ permalink raw reply related [flat|nested] 68+ messages in thread
* Re: [PATCH v4 7/7] max8903: adds support for initiation via device tree 2016-06-19 22:27 ` [PATCH v4 7/7] max8903: adds support for initiation via device tree Chris Lapa @ 2016-06-20 12:23 ` Krzysztof Kozlowski 0 siblings, 0 replies; 68+ messages in thread From: Krzysztof Kozlowski @ 2016-06-20 12:23 UTC (permalink / raw) To: Chris Lapa, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm On 06/20/2016 12:27 AM, Chris Lapa wrote: > From: Chris Lapa <chris@lapa.com.au> > > Adds support for device tree to setup a max8903 battery charger. DC and USB > validity are determined by looking the presence of the dok and uok gpios. > > Signed-off-by: Chris Lapa <chris@lapa.com.au> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> BR, Krzysztof ^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH v5 0/7] max8903: Add device tree support and misc fixes 2016-06-02 6:44 ` [PATCH 2/2] max8903: cleans up confusing relationship between dc_valid, dok and dcm chris ` (2 preceding siblings ...) 2016-06-19 22:27 ` [PATCH v4 0/7] max8903: Add device tree support and misc fixes Chris Lapa @ 2016-06-24 2:26 ` Chris Lapa 2016-06-24 2:26 ` [PATCH v5 1/7] max8903: adds documentation for device tree bindings Chris Lapa ` (7 more replies) 3 siblings, 8 replies; 68+ messages in thread From: Chris Lapa @ 2016-06-24 2:26 UTC (permalink / raw) To: k.kozlowski, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm, Chris Lapa From: Chris Lapa <chris@lapa.com.au> This patch set adds device tree support for the MAX8903 battery charger. It also cleans up logic with dc_valid, dok and dcm pins as well as fixing up validity checking of gpios. I verified these patches work on a board I have here, which uses the DC power side (not the USB portition) of the MAX8903. Changes v4 -> v5: * Changes compatible field from max8903_charger to max8903 in 1/7 and 7/7 * Improves DT documentation to include direction and state for each gpio Changes v3 -> v4: * Fixed formatting, such as multiline strings and indentation mistakes * Moved gpio setup code into max8903_setup_gpios() in 3/7 * Fixed typo in 5/7 * Renamed of_node to np in 7/7 Changes v2 -> v3: * Separate requesting of gpio's into its own commit * Fixed up validity checking of GPIO's * Remove dc_valid and usb_valid from device tree * Remove some unncessary init to psy_cfg.num_supplicants and psy_cfg.supplied_to * Reorder patches so device tree implementation is final patch Changes v1 -> v2: * Separate DT bindings documentation into its own commit * Add maxim prefix to DT compatible field * Add gpios suffix to gpio's in DT * Remove malloc failed error message Chris Lapa (7): max8903: adds documentation for device tree bindings. max8903: store pointer to pdata instead of copying it. max8903: cleans up confusing relationship between dc_valid, dok and dcm. max8903: adds requesting of gpios. max8903: removes non zero validity checks on gpios. max8903: remove unnecessary 'out of memory' error message. max8903: adds support for initiation via device tree .../devicetree/bindings/power/max8903-charger.txt | 25 +++ arch/arm/boot/dts/dairytest-servo.dtsi | 36 ++++ drivers/power/max8903_charger.c | 239 +++++++++++++++------ include/linux/power/max8903_charger.h | 6 +- 4 files changed, 240 insertions(+), 66 deletions(-) create mode 100644 Documentation/devicetree/bindings/power/max8903-charger.txt create mode 100644 arch/arm/boot/dts/dairytest-servo.dtsi -- 1.9.1 ^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH v5 1/7] max8903: adds documentation for device tree bindings. 2016-06-24 2:26 ` [PATCH v5 0/7] max8903: Add device tree support and misc fixes Chris Lapa @ 2016-06-24 2:26 ` Chris Lapa 2016-06-28 20:55 ` Rob Herring 2016-06-24 2:26 ` [PATCH v5 2/7] max8903: store pointer to pdata instead of copying it Chris Lapa ` (6 subsequent siblings) 7 siblings, 1 reply; 68+ messages in thread From: Chris Lapa @ 2016-06-24 2:26 UTC (permalink / raw) To: k.kozlowski, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm, Chris Lapa From: Chris Lapa <chris@lapa.com.au> Signed-off-by: Chris Lapa <chris@lapa.com.au> --- .../devicetree/bindings/power/max8903-charger.txt | 25 +++++++++++++++ arch/arm/boot/dts/dairytest-servo.dtsi | 36 ++++++++++++++++++++++ 2 files changed, 61 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/max8903-charger.txt create mode 100644 arch/arm/boot/dts/dairytest-servo.dtsi diff --git a/Documentation/devicetree/bindings/power/max8903-charger.txt b/Documentation/devicetree/bindings/power/max8903-charger.txt new file mode 100644 index 0000000..f0f4e12 --- /dev/null +++ b/Documentation/devicetree/bindings/power/max8903-charger.txt @@ -0,0 +1,25 @@ +Maxim Semiconductor MAX8903 Battery Charger bindings + +Required properties: +- compatible: "maxim,max8903" for MAX8903 Battery Charger +- dok-gpios: Valid DC power has been detected (active low, input), optional if uok-gpios is provided +- uok-gpios: Valid USB power has been detected (active low, input), optional if dok-gpios is provided + +Optional properties: +- cen-gpios: Charge enable pin (active low, output) +- chg-gpios: Charger status pin (active low, input) +- flt-gpios: Fault pin (active low, output) +- dcm-gpios: Current limit mode setting (DC=1 or USB=0, output) +- usus-gpios: USB suspend pin (active high, output) + + +Example: + + max8903-charger { + compatible = "maxim,max8903"; + dok-gpios = <&gpio2 3 GPIO_ACTIVE_LOW>; + flt-gpios = <&gpio2 2 GPIO_ACTIVE_LOW>; + chg-gpios = <&gpio3 15 GPIO_ACTIVE_LOW>; + cen-gpios = <&gpio2 5 GPIO_ACTIVE_LOW>; + status = "okay"; + }; diff --git a/arch/arm/boot/dts/dairytest-servo.dtsi b/arch/arm/boot/dts/dairytest-servo.dtsi new file mode 100644 index 0000000..3fbe81d --- /dev/null +++ b/arch/arm/boot/dts/dairytest-servo.dtsi @@ -0,0 +1,36 @@ +#include <dt-bindings/board/am335x-bbw-bbb-base.h> +#include <dt-bindings/gpio/gpio.h> +#include <dt-bindings/pinctrl/am33xx.h> + +&am33xx_pinmux { + servo_power_pin: pinmux_servo_power_pin{ + pinctrl-single,pins = < + BONE_P9_27 (PIN_OUTPUT| MUX_MODE7) /* ZCZ(C13) - gpio3_19, OUTPUT | MODE7 */ + >; + }; + + ehrpwm2_pin: pinmux_ehrpwm2_pin{ + pinctrl-single,pins = < + BONE_P8_19 (PIN_OUTPUT | MUX_MODE4) /* ZCZ(R1) - ehrpwm2a, OUTPUT | MODE3 */ + >; + }; +}; + +/ { + servo_power { + status = "okay"; + compatible = "pinctrl-single"; + pinctrl-names = "default"; + pinctrl-0 = <&servo_power_pin>; + }; +}; + +&epwmss2 { + status = "okay"; +}; + +&ehrpwm2 { + pinctrl-names = "default"; + pinctrl-0 = <&ehrpwm2_pin>; + status = "okay"; +}; -- 1.9.1 ^ permalink raw reply related [flat|nested] 68+ messages in thread
* Re: [PATCH v5 1/7] max8903: adds documentation for device tree bindings. 2016-06-24 2:26 ` [PATCH v5 1/7] max8903: adds documentation for device tree bindings Chris Lapa @ 2016-06-28 20:55 ` Rob Herring 0 siblings, 0 replies; 68+ messages in thread From: Rob Herring @ 2016-06-28 20:55 UTC (permalink / raw) To: Chris Lapa Cc: k.kozlowski, dwmw2, dbaryshkov, sre, mark.rutland, devicetree, linux-kernel, linux-pm On Fri, Jun 24, 2016 at 12:26:06PM +1000, Chris Lapa wrote: > From: Chris Lapa <chris@lapa.com.au> > > Signed-off-by: Chris Lapa <chris@lapa.com.au> > --- > .../devicetree/bindings/power/max8903-charger.txt | 25 +++++++++++++++ > arch/arm/boot/dts/dairytest-servo.dtsi | 36 ++++++++++++++++++++++ Binding looks fine, but this dtsi is unrelated changed. > 2 files changed, 61 insertions(+) > create mode 100644 Documentation/devicetree/bindings/power/max8903-charger.txt > create mode 100644 arch/arm/boot/dts/dairytest-servo.dtsi ^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH v5 2/7] max8903: store pointer to pdata instead of copying it. 2016-06-24 2:26 ` [PATCH v5 0/7] max8903: Add device tree support and misc fixes Chris Lapa 2016-06-24 2:26 ` [PATCH v5 1/7] max8903: adds documentation for device tree bindings Chris Lapa @ 2016-06-24 2:26 ` Chris Lapa 2016-06-24 2:26 ` [PATCH v5 3/7] max8903: cleans up confusing relationship between dc_valid, dok and dcm Chris Lapa ` (5 subsequent siblings) 7 siblings, 0 replies; 68+ messages in thread From: Chris Lapa @ 2016-06-24 2:26 UTC (permalink / raw) To: k.kozlowski, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm, Chris Lapa From: Chris Lapa <chris@lapa.com.au> Stores pointer to pdata because it easily allows pdata to reference either platform data or in the future device tree data. Signed-off-by: Chris Lapa <chris@lapa.com.au> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> --- drivers/power/max8903_charger.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c index 17876ca..0a5b0e1 100644 --- a/drivers/power/max8903_charger.c +++ b/drivers/power/max8903_charger.c @@ -29,7 +29,7 @@ #include <linux/power/max8903_charger.h> struct max8903_data { - struct max8903_pdata pdata; + struct max8903_pdata *pdata; struct device *dev; struct power_supply *psy; struct power_supply_desc psy_desc; @@ -53,8 +53,8 @@ static int max8903_get_property(struct power_supply *psy, switch (psp) { case POWER_SUPPLY_PROP_STATUS: val->intval = POWER_SUPPLY_STATUS_UNKNOWN; - if (data->pdata.chg) { - if (gpio_get_value(data->pdata.chg) == 0) + if (data->pdata->chg) { + if (gpio_get_value(data->pdata->chg) == 0) val->intval = POWER_SUPPLY_STATUS_CHARGING; else if (data->usb_in || data->ta_in) val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING; @@ -81,7 +81,7 @@ static int max8903_get_property(struct power_supply *psy, static irqreturn_t max8903_dcin(int irq, void *_data) { struct max8903_data *data = _data; - struct max8903_pdata *pdata = &data->pdata; + struct max8903_pdata *pdata = data->pdata; bool ta_in; enum power_supply_type old_type; @@ -122,7 +122,7 @@ static irqreturn_t max8903_dcin(int irq, void *_data) static irqreturn_t max8903_usbin(int irq, void *_data) { struct max8903_data *data = _data; - struct max8903_pdata *pdata = &data->pdata; + struct max8903_pdata *pdata = data->pdata; bool usb_in; enum power_supply_type old_type; @@ -161,7 +161,7 @@ static irqreturn_t max8903_usbin(int irq, void *_data) static irqreturn_t max8903_fault(int irq, void *_data) { struct max8903_data *data = _data; - struct max8903_pdata *pdata = &data->pdata; + struct max8903_pdata *pdata = data->pdata; bool fault; fault = gpio_get_value(pdata->flt) ? false : true; @@ -190,12 +190,18 @@ static int max8903_probe(struct platform_device *pdev) int ta_in = 0; int usb_in = 0; + if (pdata == NULL) { + dev_err(dev, "No platform data.\n"); + return -EINVAL; + } + data = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL); if (data == NULL) { dev_err(dev, "Cannot allocate memory.\n"); return -ENOMEM; } - memcpy(&data->pdata, pdata, sizeof(struct max8903_pdata)); + + data->pdata = pdev->dev.platform_data; data->dev = dev; platform_set_drvdata(pdev, data); -- 1.9.1 ^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH v5 3/7] max8903: cleans up confusing relationship between dc_valid, dok and dcm. 2016-06-24 2:26 ` [PATCH v5 0/7] max8903: Add device tree support and misc fixes Chris Lapa 2016-06-24 2:26 ` [PATCH v5 1/7] max8903: adds documentation for device tree bindings Chris Lapa 2016-06-24 2:26 ` [PATCH v5 2/7] max8903: store pointer to pdata instead of copying it Chris Lapa @ 2016-06-24 2:26 ` Chris Lapa 2016-06-24 2:26 ` [PATCH v5 4/7] max8903: adds requesting of gpios Chris Lapa ` (4 subsequent siblings) 7 siblings, 0 replies; 68+ messages in thread From: Chris Lapa @ 2016-06-24 2:26 UTC (permalink / raw) To: k.kozlowski, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm, Chris Lapa From: Chris Lapa <chris@lapa.com.au> The max8903_charger.h file indicated that dcm and dok were not optional when dc_valid is set. It makes sense to have dok as a compulsory pin when dc_valid is given. However dcm can be optionally wired to a fixed level especially when the circuit is configured for dc power exclusively. The previous implementation already allowed for this somewhat, however no error was given if dok wasn't given whilst dc_valid was. The new implementation enforces dok presence when dc_valid is given. Whilst allowing dcm to be optional. Signed-off-by: Chris Lapa <chris@lapa.com.au> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> --- drivers/power/max8903_charger.c | 22 +++++++++------------- include/linux/power/max8903_charger.h | 6 +++--- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c index 0a5b0e1..6ec705f 100644 --- a/drivers/power/max8903_charger.c +++ b/drivers/power/max8903_charger.c @@ -211,27 +211,23 @@ static int max8903_probe(struct platform_device *pdev) } if (pdata->dc_valid) { - if (pdata->dok && gpio_is_valid(pdata->dok) && - pdata->dcm && gpio_is_valid(pdata->dcm)) { + if (pdata->dok && gpio_is_valid(pdata->dok)) { gpio = pdata->dok; /* PULL_UPed Interrupt */ ta_in = gpio_get_value(gpio) ? 0 : 1; + } else { + dev_err(dev, "When DC is wired, DOK should be wired as well.\n"); + return -EINVAL; + } + } + if (pdata->dcm) { + if (gpio_is_valid(pdata->dcm)) { gpio = pdata->dcm; /* Output */ gpio_set_value(gpio, ta_in); } else { - dev_err(dev, "When DC is wired, DOK and DCM should" - " be wired as well.\n"); + dev_err(dev, "Invalid pin: dcm.\n"); return -EINVAL; } - } else { - if (pdata->dcm) { - if (gpio_is_valid(pdata->dcm)) - gpio_set_value(pdata->dcm, 0); - else { - dev_err(dev, "Invalid pin: dcm.\n"); - return -EINVAL; - } - } } if (pdata->usb_valid) { diff --git a/include/linux/power/max8903_charger.h b/include/linux/power/max8903_charger.h index 24f51db..89d3f1c 100644 --- a/include/linux/power/max8903_charger.h +++ b/include/linux/power/max8903_charger.h @@ -26,8 +26,8 @@ struct max8903_pdata { /* * GPIOs - * cen, chg, flt, and usus are optional. - * dok, dcm, and uok are not optional depending on the status of + * cen, chg, flt, dcm and usus are optional. + * dok and uok are not optional depending on the status of * dc_valid and usb_valid. */ int cen; /* Charger Enable input */ @@ -41,7 +41,7 @@ struct max8903_pdata { /* * DC(Adapter/TA) is wired * When dc_valid is true, - * dok and dcm should be valid. + * dok should be valid. * * At least one of dc_valid or usb_valid should be true. */ -- 1.9.1 ^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH v5 4/7] max8903: adds requesting of gpios. 2016-06-24 2:26 ` [PATCH v5 0/7] max8903: Add device tree support and misc fixes Chris Lapa ` (2 preceding siblings ...) 2016-06-24 2:26 ` [PATCH v5 3/7] max8903: cleans up confusing relationship between dc_valid, dok and dcm Chris Lapa @ 2016-06-24 2:26 ` Chris Lapa 2016-06-24 2:26 ` [PATCH v5 5/7] max8903: removes non zero validity checks on gpios Chris Lapa ` (3 subsequent siblings) 7 siblings, 0 replies; 68+ messages in thread From: Chris Lapa @ 2016-06-24 2:26 UTC (permalink / raw) To: k.kozlowski, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm, Chris Lapa From: Chris Lapa <chris@lapa.com.au> This change ensures all gpios are available for the driver to use and also splits off gpio setup into its own function for readability. Signed-off-by: Chris Lapa <chris@lapa.com.au> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> --- drivers/power/max8903_charger.c | 136 ++++++++++++++++++++++++++++++---------- 1 file changed, 102 insertions(+), 34 deletions(-) diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c index 6ec705f..3f35593 100644 --- a/drivers/power/max8903_charger.c +++ b/drivers/power/max8903_charger.c @@ -179,39 +179,27 @@ static irqreturn_t max8903_fault(int irq, void *_data) return IRQ_HANDLED; } -static int max8903_probe(struct platform_device *pdev) +static int max8903_setup_gpios(struct platform_device *pdev) { - struct max8903_data *data; + struct max8903_data *data = platform_get_drvdata(pdev); struct device *dev = &pdev->dev; struct max8903_pdata *pdata = pdev->dev.platform_data; - struct power_supply_config psy_cfg = {}; int ret = 0; int gpio; int ta_in = 0; int usb_in = 0; - if (pdata == NULL) { - dev_err(dev, "No platform data.\n"); - return -EINVAL; - } - - data = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL); - if (data == NULL) { - dev_err(dev, "Cannot allocate memory.\n"); - return -ENOMEM; - } - - data->pdata = pdev->dev.platform_data; - data->dev = dev; - platform_set_drvdata(pdev, data); - - if (pdata->dc_valid == false && pdata->usb_valid == false) { - dev_err(dev, "No valid power sources.\n"); - return -EINVAL; - } - if (pdata->dc_valid) { if (pdata->dok && gpio_is_valid(pdata->dok)) { + ret = devm_gpio_request(dev, pdata->dok, + data->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for dok: %d err %d\n", + pdata->dok, ret); + return ret; + } + gpio = pdata->dok; /* PULL_UPed Interrupt */ ta_in = gpio_get_value(gpio) ? 0 : 1; } else { @@ -222,6 +210,15 @@ static int max8903_probe(struct platform_device *pdev) if (pdata->dcm) { if (gpio_is_valid(pdata->dcm)) { + ret = devm_gpio_request(dev, pdata->dcm, + data->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for dcm: %d err %d\n", + pdata->dcm, ret); + return ret; + } + gpio = pdata->dcm; /* Output */ gpio_set_value(gpio, ta_in); } else { @@ -232,6 +229,15 @@ static int max8903_probe(struct platform_device *pdev) if (pdata->usb_valid) { if (pdata->uok && gpio_is_valid(pdata->uok)) { + ret = devm_gpio_request(dev, pdata->uok, + data->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for uok: %d err %d\n", + pdata->uok, ret); + return ret; + } + gpio = pdata->uok; usb_in = gpio_get_value(gpio) ? 0 : 1; } else { @@ -243,6 +249,15 @@ static int max8903_probe(struct platform_device *pdev) if (pdata->cen) { if (gpio_is_valid(pdata->cen)) { + ret = devm_gpio_request(dev, pdata->cen, + data->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for cen: %d err %d\n", + pdata->cen, ret); + return ret; + } + gpio_set_value(pdata->cen, (ta_in || usb_in) ? 0 : 1); } else { dev_err(dev, "Invalid pin: cen.\n"); @@ -251,23 +266,41 @@ static int max8903_probe(struct platform_device *pdev) } if (pdata->chg) { - if (!gpio_is_valid(pdata->chg)) { - dev_err(dev, "Invalid pin: chg.\n"); - return -EINVAL; + if (gpio_is_valid(pdata->chg)) { + ret = devm_gpio_request(dev, pdata->chg, + data->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for chg: %d err %d\n", + pdata->chg, ret); + return ret; + } } } if (pdata->flt) { - if (!gpio_is_valid(pdata->flt)) { - dev_err(dev, "Invalid pin: flt.\n"); - return -EINVAL; + if (gpio_is_valid(pdata->flt)) { + ret = devm_gpio_request(dev, pdata->flt, + data->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for flt: %d err %d\n", + pdata->flt, ret); + return ret; + } } } if (pdata->usus) { - if (!gpio_is_valid(pdata->usus)) { - dev_err(dev, "Invalid pin: usus.\n"); - return -EINVAL; + if (gpio_is_valid(pdata->usus)) { + ret = devm_gpio_request(dev, pdata->usus, + data->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for usus: %d err %d\n", + pdata->usus, ret); + return ret; + } } } @@ -275,9 +308,44 @@ static int max8903_probe(struct platform_device *pdev) data->ta_in = ta_in; data->usb_in = usb_in; + return 0; +} + +static int max8903_probe(struct platform_device *pdev) +{ + struct max8903_data *data; + struct device *dev = &pdev->dev; + struct max8903_pdata *pdata = pdev->dev.platform_data; + struct power_supply_config psy_cfg = {}; + int ret = 0; + + if (pdata == NULL) { + dev_err(dev, "No platform data.\n"); + return -EINVAL; + } + + data = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL); + if (data == NULL) { + dev_err(dev, "Cannot allocate memory.\n"); + return -ENOMEM; + } + + data->pdata = pdev->dev.platform_data; + data->dev = dev; + platform_set_drvdata(pdev, data); + + if (pdata->dc_valid == false && pdata->usb_valid == false) { + dev_err(dev, "No valid power sources.\n"); + return -EINVAL; + } + + ret = max8903_setup_gpios(pdev); + if (ret) + return ret; + data->psy_desc.name = "max8903_charger"; - data->psy_desc.type = (ta_in) ? POWER_SUPPLY_TYPE_MAINS : - ((usb_in) ? POWER_SUPPLY_TYPE_USB : + data->psy_desc.type = (data->ta_in) ? POWER_SUPPLY_TYPE_MAINS : + ((data->usb_in) ? POWER_SUPPLY_TYPE_USB : POWER_SUPPLY_TYPE_BATTERY); data->psy_desc.get_property = max8903_get_property; data->psy_desc.properties = max8903_charger_props; -- 1.9.1 ^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH v5 5/7] max8903: removes non zero validity checks on gpios. 2016-06-24 2:26 ` [PATCH v5 0/7] max8903: Add device tree support and misc fixes Chris Lapa ` (3 preceding siblings ...) 2016-06-24 2:26 ` [PATCH v5 4/7] max8903: adds requesting of gpios Chris Lapa @ 2016-06-24 2:26 ` Chris Lapa 2016-06-24 2:26 ` [PATCH v5 6/7] max8903: remove unnecessary 'out of memory' error message Chris Lapa ` (2 subsequent siblings) 7 siblings, 0 replies; 68+ messages in thread From: Chris Lapa @ 2016-06-24 2:26 UTC (permalink / raw) To: k.kozlowski, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm, Chris Lapa From: Chris Lapa <chris@lapa.com.au> Prior to this commit a zero gpio was treated as invalid. Whereas gpio_is_valid() will treat a zero gpio as valid. This commit removes the confusion and explicitly uses gpio_is_valid() throughout. Which in turn results in several of the error messages becoming redundant and thus removed. Signed-off-by: Chris Lapa <chris@lapa.com.au> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> --- drivers/power/max8903_charger.c | 115 ++++++++++++++++------------------------ 1 file changed, 47 insertions(+), 68 deletions(-) diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c index 3f35593..643a87a 100644 --- a/drivers/power/max8903_charger.c +++ b/drivers/power/max8903_charger.c @@ -53,7 +53,7 @@ static int max8903_get_property(struct power_supply *psy, switch (psp) { case POWER_SUPPLY_PROP_STATUS: val->intval = POWER_SUPPLY_STATUS_UNKNOWN; - if (data->pdata->chg) { + if (gpio_is_valid(data->pdata->chg)) { if (gpio_get_value(data->pdata->chg) == 0) val->intval = POWER_SUPPLY_STATUS_CHARGING; else if (data->usb_in || data->ta_in) @@ -93,11 +93,11 @@ static irqreturn_t max8903_dcin(int irq, void *_data) data->ta_in = ta_in; /* Set Current-Limit-Mode 1:DC 0:USB */ - if (pdata->dcm) + if (gpio_is_valid(pdata->dcm)) gpio_set_value(pdata->dcm, ta_in ? 1 : 0); /* Charger Enable / Disable (cen is negated) */ - if (pdata->cen) + if (gpio_is_valid(pdata->cen)) gpio_set_value(pdata->cen, ta_in ? 0 : (data->usb_in ? 0 : 1)); @@ -136,7 +136,7 @@ static irqreturn_t max8903_usbin(int irq, void *_data) /* Do not touch Current-Limit-Mode */ /* Charger Enable / Disable (cen is negated) */ - if (pdata->cen) + if (gpio_is_valid(pdata->cen)) gpio_set_value(pdata->cen, usb_in ? 0 : (data->ta_in ? 0 : 1)); @@ -190,7 +190,7 @@ static int max8903_setup_gpios(struct platform_device *pdev) int usb_in = 0; if (pdata->dc_valid) { - if (pdata->dok && gpio_is_valid(pdata->dok)) { + if (gpio_is_valid(pdata->dok)) { ret = devm_gpio_request(dev, pdata->dok, data->psy_desc.name); if (ret) { @@ -208,27 +208,21 @@ static int max8903_setup_gpios(struct platform_device *pdev) } } - if (pdata->dcm) { - if (gpio_is_valid(pdata->dcm)) { - ret = devm_gpio_request(dev, pdata->dcm, - data->psy_desc.name); - if (ret) { - dev_err(dev, - "Failed GPIO request for dcm: %d err %d\n", - pdata->dcm, ret); - return ret; - } - - gpio = pdata->dcm; /* Output */ - gpio_set_value(gpio, ta_in); - } else { - dev_err(dev, "Invalid pin: dcm.\n"); - return -EINVAL; + if (gpio_is_valid(pdata->dcm)) { + ret = devm_gpio_request(dev, pdata->dcm, data->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for dcm: %d err %d\n", + pdata->dcm, ret); + return ret; } + + gpio = pdata->dcm; /* Output */ + gpio_set_value(gpio, ta_in); } if (pdata->usb_valid) { - if (pdata->uok && gpio_is_valid(pdata->uok)) { + if (gpio_is_valid(pdata->uok)) { ret = devm_gpio_request(dev, pdata->uok, data->psy_desc.name); if (ret) { @@ -247,60 +241,45 @@ static int max8903_setup_gpios(struct platform_device *pdev) } } - if (pdata->cen) { - if (gpio_is_valid(pdata->cen)) { - ret = devm_gpio_request(dev, pdata->cen, - data->psy_desc.name); - if (ret) { - dev_err(dev, - "Failed GPIO request for cen: %d err %d\n", - pdata->cen, ret); - return ret; - } - - gpio_set_value(pdata->cen, (ta_in || usb_in) ? 0 : 1); - } else { - dev_err(dev, "Invalid pin: cen.\n"); - return -EINVAL; + if (gpio_is_valid(pdata->cen)) { + ret = devm_gpio_request(dev, pdata->cen, data->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for cen: %d err %d\n", + pdata->cen, ret); + return ret; } + + gpio_set_value(pdata->cen, (ta_in || usb_in) ? 0 : 1); } - if (pdata->chg) { - if (gpio_is_valid(pdata->chg)) { - ret = devm_gpio_request(dev, pdata->chg, - data->psy_desc.name); - if (ret) { - dev_err(dev, - "Failed GPIO request for chg: %d err %d\n", - pdata->chg, ret); - return ret; - } + if (gpio_is_valid(pdata->chg)) { + ret = devm_gpio_request(dev, pdata->chg, data->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for chg: %d err %d\n", + pdata->chg, ret); + return ret; } } - if (pdata->flt) { - if (gpio_is_valid(pdata->flt)) { - ret = devm_gpio_request(dev, pdata->flt, - data->psy_desc.name); - if (ret) { - dev_err(dev, - "Failed GPIO request for flt: %d err %d\n", - pdata->flt, ret); - return ret; - } + if (gpio_is_valid(pdata->flt)) { + ret = devm_gpio_request(dev, pdata->flt, data->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for flt: %d err %d\n", + pdata->flt, ret); + return ret; } } - if (pdata->usus) { - if (gpio_is_valid(pdata->usus)) { - ret = devm_gpio_request(dev, pdata->usus, - data->psy_desc.name); - if (ret) { - dev_err(dev, - "Failed GPIO request for usus: %d err %d\n", - pdata->usus, ret); - return ret; - } + if (gpio_is_valid(pdata->usus)) { + ret = devm_gpio_request(dev, pdata->usus, data->psy_desc.name); + if (ret) { + dev_err(dev, + "Failed GPIO request for usus: %d err %d\n", + pdata->usus, ret); + return ret; } } @@ -385,7 +364,7 @@ static int max8903_probe(struct platform_device *pdev) } } - if (pdata->flt) { + if (gpio_is_valid(pdata->flt)) { ret = devm_request_threaded_irq(dev, gpio_to_irq(pdata->flt), NULL, max8903_fault, IRQF_TRIGGER_FALLING | -- 1.9.1 ^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH v5 6/7] max8903: remove unnecessary 'out of memory' error message. 2016-06-24 2:26 ` [PATCH v5 0/7] max8903: Add device tree support and misc fixes Chris Lapa ` (4 preceding siblings ...) 2016-06-24 2:26 ` [PATCH v5 5/7] max8903: removes non zero validity checks on gpios Chris Lapa @ 2016-06-24 2:26 ` Chris Lapa 2016-06-24 2:26 ` [PATCH v5 7/7] max8903: adds support for initiation via device tree Chris Lapa 2016-06-28 18:16 ` [PATCH v5 0/7] max8903: Add device tree support and misc fixes Sebastian Reichel 7 siblings, 0 replies; 68+ messages in thread From: Chris Lapa @ 2016-06-24 2:26 UTC (permalink / raw) To: k.kozlowski, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm, Chris Lapa From: Chris Lapa <chris@lapa.com.au> Remove the 'out of memory' error message as it is printed by the core. Signed-off-by: Chris Lapa <chris@lapa.com.au> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> --- drivers/power/max8903_charger.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c index 643a87a..9453bbf 100644 --- a/drivers/power/max8903_charger.c +++ b/drivers/power/max8903_charger.c @@ -304,10 +304,8 @@ static int max8903_probe(struct platform_device *pdev) } data = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL); - if (data == NULL) { - dev_err(dev, "Cannot allocate memory.\n"); + if (!data) return -ENOMEM; - } data->pdata = pdev->dev.platform_data; data->dev = dev; -- 1.9.1 ^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH v5 7/7] max8903: adds support for initiation via device tree 2016-06-24 2:26 ` [PATCH v5 0/7] max8903: Add device tree support and misc fixes Chris Lapa ` (5 preceding siblings ...) 2016-06-24 2:26 ` [PATCH v5 6/7] max8903: remove unnecessary 'out of memory' error message Chris Lapa @ 2016-06-24 2:26 ` Chris Lapa 2016-06-28 18:16 ` [PATCH v5 0/7] max8903: Add device tree support and misc fixes Sebastian Reichel 7 siblings, 0 replies; 68+ messages in thread From: Chris Lapa @ 2016-06-24 2:26 UTC (permalink / raw) To: k.kozlowski, dwmw2, dbaryshkov, sre, mark.rutland, robh+dt Cc: devicetree, linux-kernel, linux-pm, Chris Lapa From: Chris Lapa <chris@lapa.com.au> Adds support for device tree to setup a max8903 battery charger. DC and USB validity are determined by looking the presence of the dok and uok gpios. Signed-off-by: Chris Lapa <chris@lapa.com.au> --- drivers/power/max8903_charger.c | 78 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 72 insertions(+), 6 deletions(-) diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c index 9453bbf..fdc73d6 100644 --- a/drivers/power/max8903_charger.c +++ b/drivers/power/max8903_charger.c @@ -23,6 +23,9 @@ #include <linux/gpio.h> #include <linux/interrupt.h> #include <linux/module.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/of_gpio.h> #include <linux/slab.h> #include <linux/power_supply.h> #include <linux/platform_device.h> @@ -75,6 +78,7 @@ static int max8903_get_property(struct power_supply *psy, default: return -EINVAL; } + return 0; } @@ -179,6 +183,56 @@ static irqreturn_t max8903_fault(int irq, void *_data) return IRQ_HANDLED; } +static struct max8903_pdata *max8903_parse_dt_data(struct device *dev) +{ + struct device_node *np = dev->of_node; + struct max8903_pdata *pdata = NULL; + + if (!np) + return NULL; + + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); + if (!pdata) + return NULL; + + pdata->dc_valid = false; + pdata->usb_valid = false; + + pdata->cen = of_get_named_gpio(np, "cen-gpios", 0); + if (!gpio_is_valid(pdata->cen)) + pdata->cen = -EINVAL; + + pdata->chg = of_get_named_gpio(np, "chg-gpios", 0); + if (!gpio_is_valid(pdata->chg)) + pdata->chg = -EINVAL; + + pdata->flt = of_get_named_gpio(np, "flt-gpios", 0); + if (!gpio_is_valid(pdata->flt)) + pdata->flt = -EINVAL; + + pdata->usus = of_get_named_gpio(np, "usus-gpios", 0); + if (!gpio_is_valid(pdata->usus)) + pdata->usus = -EINVAL; + + pdata->dcm = of_get_named_gpio(np, "dcm-gpios", 0); + if (!gpio_is_valid(pdata->dcm)) + pdata->dcm = -EINVAL; + + pdata->dok = of_get_named_gpio(np, "dok-gpios", 0); + if (!gpio_is_valid(pdata->dok)) + pdata->dok = -EINVAL; + else + pdata->dc_valid = true; + + pdata->uok = of_get_named_gpio(np, "uok-gpios", 0); + if (!gpio_is_valid(pdata->uok)) + pdata->uok = -EINVAL; + else + pdata->usb_valid = true; + + return pdata; +} + static int max8903_setup_gpios(struct platform_device *pdev) { struct max8903_data *data = platform_get_drvdata(pdev); @@ -298,16 +352,20 @@ static int max8903_probe(struct platform_device *pdev) struct power_supply_config psy_cfg = {}; int ret = 0; - if (pdata == NULL) { - dev_err(dev, "No platform data.\n"); - return -EINVAL; - } - data = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL); if (!data) return -ENOMEM; - data->pdata = pdev->dev.platform_data; + if (IS_ENABLED(CONFIG_OF) && !pdata && dev->of_node) + pdata = max8903_parse_dt_data(dev); + + if (!pdata) { + dev_err(dev, "No platform data.\n"); + return -EINVAL; + } + + pdev->dev.platform_data = pdata; + data->pdata = pdata; data->dev = dev; platform_set_drvdata(pdev, data); @@ -328,6 +386,7 @@ static int max8903_probe(struct platform_device *pdev) data->psy_desc.properties = max8903_charger_props; data->psy_desc.num_properties = ARRAY_SIZE(max8903_charger_props); + psy_cfg.of_node = dev->of_node; psy_cfg.drv_data = data; data->psy = devm_power_supply_register(dev, &data->psy_desc, &psy_cfg); @@ -378,10 +437,17 @@ static int max8903_probe(struct platform_device *pdev) return 0; } +static const struct of_device_id max8903_match_ids[] = { + { .compatible = "maxim,max8903", }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, max8903_match_ids); + static struct platform_driver max8903_driver = { .probe = max8903_probe, .driver = { .name = "max8903-charger", + .of_match_table = max8903_match_ids }, }; -- 1.9.1 ^ permalink raw reply related [flat|nested] 68+ messages in thread
* Re: [PATCH v5 0/7] max8903: Add device tree support and misc fixes 2016-06-24 2:26 ` [PATCH v5 0/7] max8903: Add device tree support and misc fixes Chris Lapa ` (6 preceding siblings ...) 2016-06-24 2:26 ` [PATCH v5 7/7] max8903: adds support for initiation via device tree Chris Lapa @ 2016-06-28 18:16 ` Sebastian Reichel 2016-06-28 23:05 ` Chris Lapa 7 siblings, 1 reply; 68+ messages in thread From: Sebastian Reichel @ 2016-06-28 18:16 UTC (permalink / raw) To: Chris Lapa Cc: k.kozlowski, dwmw2, dbaryshkov, mark.rutland, robh+dt, devicetree, linux-kernel, linux-pm [-- Attachment #1: Type: text/plain, Size: 661 bytes --] Hi Chris, On Fri, Jun 24, 2016 at 12:26:05PM +1000, Chris Lapa wrote: > This patch set adds device tree support for the MAX8903 battery charger. > It also cleans up logic with dc_valid, dok and dcm pins as well as > fixing up validity checking of gpios. > > I verified these patches work on a board I have here, which uses the > DC power side (not the USB portition) of the MAX8903. I queued this into my for-next branch, except for the dtsi file, which I dropped from your first patch. Please submit it separately to the correct maintainers. [...] > arch/arm/boot/dts/dairytest-servo.dtsi | 36 ++++ [...] -- Sebastian [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH v5 0/7] max8903: Add device tree support and misc fixes 2016-06-28 18:16 ` [PATCH v5 0/7] max8903: Add device tree support and misc fixes Sebastian Reichel @ 2016-06-28 23:05 ` Chris Lapa 0 siblings, 0 replies; 68+ messages in thread From: Chris Lapa @ 2016-06-28 23:05 UTC (permalink / raw) To: Sebastian Reichel Cc: k.kozlowski, dwmw2, dbaryshkov, mark.rutland, robh+dt, devicetree, linux-kernel, linux-pm Hi Sebastian, Sorry about the extra dtsi file, I accidentally included it in the v5 patch set (wasn't in <= v4). Thanks, Chris On 29/06/2016 4:16 AM, Sebastian Reichel wrote: > Hi Chris, > > On Fri, Jun 24, 2016 at 12:26:05PM +1000, Chris Lapa wrote: >> This patch set adds device tree support for the MAX8903 battery charger. >> It also cleans up logic with dc_valid, dok and dcm pins as well as >> fixing up validity checking of gpios. >> >> I verified these patches work on a board I have here, which uses the >> DC power side (not the USB portition) of the MAX8903. > > I queued this into my for-next branch, except for the dtsi file, > which I dropped from your first patch. Please submit it separately > to the correct maintainers. > > [...] > >> arch/arm/boot/dts/dairytest-servo.dtsi | 36 ++++ > > [...] > > -- Sebastian > ^ permalink raw reply [flat|nested] 68+ messages in thread
end of thread, other threads:[~2016-06-28 23:06 UTC | newest] Thread overview: 68+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-06-02 6:44 [PATCH 0/2] max8903: Add device tree support and logic fixup chris 2016-06-02 6:44 ` [PATCH 1/2] max8903: adds support for initiation via device tree chris 2016-06-09 10:35 ` Krzysztof Kozlowski 2016-06-10 5:35 ` Chris Lapa 2016-06-10 6:13 ` Krzysztof Kozlowski 2016-06-10 10:23 ` Krzysztof Kozlowski 2016-06-02 6:44 ` [PATCH 2/2] max8903: cleans up confusing relationship between dc_valid, dok and dcm chris 2016-06-10 12:32 ` [PATCH v2 0/4] max8903: Add device tree support and logic fixup Chris Lapa 2016-06-10 12:32 ` [PATCH v2 1/4] max8903: adds documentation for device tree bindings Chris Lapa 2016-06-10 13:51 ` Krzysztof Kozlowski 2016-06-14 1:54 ` Chris Lapa 2016-06-16 6:35 ` Krzysztof Kozlowski 2016-06-16 6:48 ` Chris Lapa 2016-06-10 12:32 ` [PATCH v2 2/4] max8903: adds support for initiation via device tree Chris Lapa 2016-06-10 14:02 ` Krzysztof Kozlowski 2016-06-14 1:50 ` Chris Lapa 2016-06-10 12:32 ` [PATCH v2 3/4] max8903: cleans up confusing relationship between dc_valid, dok and dcm Chris Lapa 2016-06-10 12:32 ` [PATCH v2 4/4] max8903: remove unnecessary malloc failed message print out Chris Lapa 2016-06-10 14:08 ` Krzysztof Kozlowski 2016-06-17 5:00 ` [PATCH v3 0/7] max8903: Add device tree support and misc fixes Chris Lapa 2016-06-17 5:00 ` [PATCH v3 1/7] max8903: adds documentation for device tree bindings Chris Lapa 2016-06-17 6:15 ` Krzysztof Kozlowski 2016-06-20 13:22 ` Rob Herring 2016-06-20 23:43 ` Chris Lapa 2016-06-17 5:00 ` [PATCH v3 2/7] max8903: store pointer to pdata instead of copying it Chris Lapa 2016-06-17 6:14 ` Krzysztof Kozlowski 2016-06-17 5:00 ` [PATCH v3 3/7] max8903: cleans up confusing relationship between dc_valid, dok and dcm Chris Lapa 2016-06-17 6:23 ` Krzysztof Kozlowski 2016-06-17 6:26 ` Krzysztof Kozlowski 2016-06-17 6:28 ` Chris Lapa 2016-06-17 6:42 ` Krzysztof Kozlowski 2016-06-19 13:48 ` Chris Lapa 2016-06-17 5:00 ` [PATCH v3 4/7] max8903: adds requesting of gpios Chris Lapa 2016-06-17 6:30 ` Krzysztof Kozlowski 2016-06-19 13:51 ` Chris Lapa 2016-06-17 5:00 ` [PATCH v3 5/7] max8903: removes non zero validity checks on gpios Chris Lapa 2016-06-17 6:35 ` Krzysztof Kozlowski 2016-06-17 5:00 ` [PATCH v3 6/7] max8903: remove unnecessary 'out of memory' error message Chris Lapa 2016-06-17 6:35 ` Krzysztof Kozlowski 2016-06-17 5:00 ` [PATCH v3 7/7] max8903: adds support for initiation via device tree Chris Lapa 2016-06-17 6:41 ` Krzysztof Kozlowski 2016-06-19 13:52 ` Chris Lapa 2016-06-19 22:27 ` [PATCH v4 0/7] max8903: Add device tree support and misc fixes Chris Lapa 2016-06-19 22:27 ` [PATCH v4 1/7] max8903: adds documentation for device tree bindings Chris Lapa 2016-06-20 12:17 ` Krzysztof Kozlowski 2016-06-19 22:27 ` [PATCH v4 2/7] max8903: store pointer to pdata instead of copying it Chris Lapa 2016-06-20 12:18 ` Krzysztof Kozlowski 2016-06-19 22:27 ` [PATCH v4 3/7] max8903: cleans up confusing relationship between dc_valid, dok and dcm Chris Lapa 2016-06-20 12:18 ` Krzysztof Kozlowski 2016-06-19 22:27 ` [PATCH v4 4/7] max8903: adds requesting of gpios Chris Lapa 2016-06-20 12:20 ` Krzysztof Kozlowski 2016-06-19 22:27 ` [PATCH v4 5/7] max8903: removes non zero validity checks on gpios Chris Lapa 2016-06-20 12:21 ` Krzysztof Kozlowski 2016-06-19 22:27 ` [PATCH v4 6/7] max8903: remove unnecessary 'out of memory' error message Chris Lapa 2016-06-20 12:22 ` Krzysztof Kozlowski 2016-06-19 22:27 ` [PATCH v4 7/7] max8903: adds support for initiation via device tree Chris Lapa 2016-06-20 12:23 ` Krzysztof Kozlowski 2016-06-24 2:26 ` [PATCH v5 0/7] max8903: Add device tree support and misc fixes Chris Lapa 2016-06-24 2:26 ` [PATCH v5 1/7] max8903: adds documentation for device tree bindings Chris Lapa 2016-06-28 20:55 ` Rob Herring 2016-06-24 2:26 ` [PATCH v5 2/7] max8903: store pointer to pdata instead of copying it Chris Lapa 2016-06-24 2:26 ` [PATCH v5 3/7] max8903: cleans up confusing relationship between dc_valid, dok and dcm Chris Lapa 2016-06-24 2:26 ` [PATCH v5 4/7] max8903: adds requesting of gpios Chris Lapa 2016-06-24 2:26 ` [PATCH v5 5/7] max8903: removes non zero validity checks on gpios Chris Lapa 2016-06-24 2:26 ` [PATCH v5 6/7] max8903: remove unnecessary 'out of memory' error message Chris Lapa 2016-06-24 2:26 ` [PATCH v5 7/7] max8903: adds support for initiation via device tree Chris Lapa 2016-06-28 18:16 ` [PATCH v5 0/7] max8903: Add device tree support and misc fixes Sebastian Reichel 2016-06-28 23:05 ` Chris Lapa
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).