Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1411907 > unrolled thread

[PATCH 2/2] max8903: cleans up confusing relationship between dc_valid, dok and dcm.

Started bychris@lapa.com.au
First post2016-06-02 08:50 +0200
Last post2016-06-10 14:40 +0200
Articles 9 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 2/2] max8903: cleans up confusing relationship between dc_valid, dok and dcm. chris@lapa.com.au - 2016-06-02 08:50 +0200
    [PATCH v2 4/4] max8903: remove unnecessary malloc failed message print out. Chris Lapa <chris@lapa.com.au> - 2016-06-10 14:40 +0200
      Re: [PATCH v2 4/4] max8903: remove unnecessary malloc failed message  print out. Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2016-06-10 16:10 +0200
    [PATCH v2 1/4] max8903: adds documentation for device tree bindings. Chris Lapa <chris@lapa.com.au> - 2016-06-10 14:40 +0200
      Re: [PATCH v2 1/4] max8903: adds documentation for device tree  bindings. Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2016-06-10 16:00 +0200
    [PATCH v2 2/4] max8903: adds support for initiation via device tree. Chris Lapa <chris@lapa.com.au> - 2016-06-10 14:40 +0200
      Re: [PATCH v2 2/4] max8903: adds support for initiation via device  tree. Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2016-06-10 16:10 +0200
    [PATCH v2 3/4] max8903: cleans up confusing relationship between dc_valid, dok and dcm. Chris Lapa <chris@lapa.com.au> - 2016-06-10 14:40 +0200
    [PATCH v2 0/4]  max8903: Add device tree support and logic fixup Chris Lapa <chris@lapa.com.au> - 2016-06-10 14:40 +0200

#1411907 — [PATCH 2/2] max8903: cleans up confusing relationship between dc_valid, dok and dcm.

Fromchris@lapa.com.au
Date2016-06-02 08:50 +0200
Subject[PATCH 2/2] max8903: cleans up confusing relationship between dc_valid, dok and dcm.
Message-ID<rFuC5-2sz-13@gated-at.bofh.it>
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

[toc] | [next] | [standalone]

#1419340 — [PATCH v2 4/4] max8903: remove unnecessary malloc failed message print out.

FromChris Lapa <chris@lapa.com.au>
Date2016-06-10 14:40 +0200
Subject[PATCH v2 4/4] max8903: remove unnecessary malloc failed message print out.
Message-ID<rItTb-4zv-5@gated-at.bofh.it>
In reply to#1411907
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

[toc] | [prev] | [next] | [standalone]

#1419449 — Re: [PATCH v2 4/4] max8903: remove unnecessary malloc failed message print out.

FromKrzysztof Kozlowski <k.kozlowski@samsung.com>
Date2016-06-10 16:10 +0200
SubjectRe: [PATCH v2 4/4] max8903: remove unnecessary malloc failed message print out.
Message-ID<rIvih-5yi-3@gated-at.bofh.it>
In reply to#1419340
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) {
>

[toc] | [prev] | [next] | [standalone]

#1419346 — [PATCH v2 1/4] max8903: adds documentation for device tree bindings.

FromChris Lapa <chris@lapa.com.au>
Date2016-06-10 14:40 +0200
Subject[PATCH v2 1/4] max8903: adds documentation for device tree bindings.
Message-ID<rItTb-4zv-15@gated-at.bofh.it>
In reply to#1411907
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

[toc] | [prev] | [next] | [standalone]

#1419445 — Re: [PATCH v2 1/4] max8903: adds documentation for device tree bindings.

FromKrzysztof Kozlowski <k.kozlowski@samsung.com>
Date2016-06-10 16:00 +0200
SubjectRe: [PATCH v2 1/4] max8903: adds documentation for device tree bindings.
Message-ID<rIv8C-5fJ-33@gated-at.bofh.it>
In reply to#1419346
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";
> +	};
>

[toc] | [prev] | [next] | [standalone]

#1419347 — [PATCH v2 2/4] max8903: adds support for initiation via device tree.

FromChris Lapa <chris@lapa.com.au>
Date2016-06-10 14:40 +0200
Subject[PATCH v2 2/4] max8903: adds support for initiation via device tree.
Message-ID<rItTb-4zv-21@gated-at.bofh.it>
In reply to#1411907
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

[toc] | [prev] | [next] | [standalone]

#1419450 — Re: [PATCH v2 2/4] max8903: adds support for initiation via device tree.

FromKrzysztof Kozlowski <k.kozlowski@samsung.com>
Date2016-06-10 16:10 +0200
SubjectRe: [PATCH v2 2/4] max8903: adds support for initiation via device tree.
Message-ID<rIvih-5yi-21@gated-at.bofh.it>
In reply to#1419347
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

[toc] | [prev] | [next] | [standalone]

#1419348 — [PATCH v2 3/4] max8903: cleans up confusing relationship between dc_valid, dok and dcm.

FromChris Lapa <chris@lapa.com.au>
Date2016-06-10 14:40 +0200
Subject[PATCH v2 3/4] max8903: cleans up confusing relationship between dc_valid, dok and dcm.
Message-ID<rItTb-4zv-23@gated-at.bofh.it>
In reply to#1411907
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

[toc] | [prev] | [next] | [standalone]

#1419349 — [PATCH v2 0/4] max8903: Add device tree support and logic fixup

FromChris Lapa <chris@lapa.com.au>
Date2016-06-10 14:40 +0200
Subject[PATCH v2 0/4] max8903: Add device tree support and logic fixup
Message-ID<rItTb-4zv-7@gated-at.bofh.it>
In reply to#1411907
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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web