All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] max8903: Add device tree support and logic fixup
@ 2016-06-07  4:02 chris
  2016-06-07  4:02 ` [PATCH 1/2] max8903: adds support for initiation via device tree chris
  2016-06-07  4:02 ` [PATCH 2/2] max8903: cleans up confusing relationship between dc_valid, dok and dcm chris
  0 siblings, 2 replies; 4+ messages in thread
From: chris @ 2016-06-07  4:02 UTC (permalink / raw)
  To: linux-pm, dwmw2, dbaryshkov, sre; +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] 4+ messages in thread

* [PATCH 1/2] max8903: adds support for initiation via device tree.
  2016-06-07  4:02 [PATCH 0/2] max8903: Add device tree support and logic fixup chris
@ 2016-06-07  4:02 ` chris
  2016-06-07  4:02 ` [PATCH 2/2] max8903: cleans up confusing relationship between dc_valid, dok and dcm chris
  1 sibling, 0 replies; 4+ messages in thread
From: chris @ 2016-06-07  4:02 UTC (permalink / raw)
  To: linux-pm, dwmw2, dbaryshkov, sre; +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] 4+ messages in thread

* [PATCH 2/2] max8903: cleans up confusing relationship between dc_valid, dok and dcm.
  2016-06-07  4:02 [PATCH 0/2] max8903: Add device tree support and logic fixup chris
  2016-06-07  4:02 ` [PATCH 1/2] max8903: adds support for initiation via device tree chris
@ 2016-06-07  4:02 ` chris
  1 sibling, 0 replies; 4+ messages in thread
From: chris @ 2016-06-07  4:02 UTC (permalink / raw)
  To: linux-pm, dwmw2, dbaryshkov, sre; +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] 4+ 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 ` chris
  0 siblings, 0 replies; 4+ 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] 4+ messages in thread

end of thread, other threads:[~2016-06-07  4:02 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-07  4:02 [PATCH 0/2] max8903: Add device tree support and logic fixup chris
2016-06-07  4:02 ` [PATCH 1/2] max8903: adds support for initiation via device tree chris
2016-06-07  4:02 ` [PATCH 2/2] max8903: cleans up confusing relationship between dc_valid, dok and dcm chris
  -- strict thread matches above, loose matches on Subject: below --
2016-06-02  6:44 [PATCH 0/2] max8903: Add device tree support and logic fixup chris
2016-06-02  6:44 ` [PATCH 2/2] max8903: cleans up confusing relationship between dc_valid, dok and dcm chris

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.