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

Groups > linux.kernel > #1426273 > unrolled thread

[PATCH v4 0/7] max8903: Add device tree support and misc fixes

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 v4 0/7] max8903: Add device tree support and misc fixes Chris Lapa <chris@lapa.com.au> - 2016-06-20 09:30 +0200
    [PATCH v4 6/7] max8903: remove unnecessary 'out of memory' error message. Chris Lapa <chris@lapa.com.au> - 2016-06-20 09:30 +0200
      Re: [PATCH v4 6/7] max8903: remove unnecessary 'out of memory' error  message. Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2016-06-20 14:30 +0200
    [PATCH v4 4/7] max8903: adds requesting of gpios. Chris Lapa <chris@lapa.com.au> - 2016-06-20 09:30 +0200
      Re: [PATCH v4 4/7] max8903: adds requesting of gpios. Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2016-06-20 14:30 +0200
    [PATCH v4 7/7] max8903: adds support for initiation via device tree Chris Lapa <chris@lapa.com.au> - 2016-06-20 09:30 +0200
      Re: [PATCH v4 7/7] max8903: adds support for initiation via device tree Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2016-06-20 14:30 +0200
    [PATCH v4 2/7] max8903: store pointer to pdata instead of copying it. Chris Lapa <chris@lapa.com.au> - 2016-06-20 09:30 +0200
      Re: [PATCH v4 2/7] max8903: store pointer to pdata instead of copying  it. Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2016-06-20 14:30 +0200
    [PATCH v4 3/7] max8903: cleans up confusing relationship between dc_valid, dok and dcm. Chris Lapa <chris@lapa.com.au> - 2016-06-20 10:10 +0200
      Re: [PATCH v4 3/7] max8903: cleans up confusing relationship between  dc_valid, dok and dcm. Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2016-06-20 14:30 +0200
    [PATCH v4 5/7] max8903: removes non zero validity checks on gpios. Chris Lapa <chris@lapa.com.au> - 2016-06-20 10:10 +0200
      Re: [PATCH v4 5/7] max8903: removes non zero validity checks on gpios. Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2016-06-20 14:30 +0200

#1426273 — [PATCH v4 0/7] max8903: Add device tree support and misc fixes

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

[toc] | [next] | [standalone]

#1426276 — [PATCH v4 6/7] max8903: remove unnecessary 'out of memory' error message.

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

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

#1426553 — Re: [PATCH v4 6/7] max8903: remove unnecessary 'out of memory' error message.

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

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

#1426277 — [PATCH v4 4/7] max8903: adds requesting of gpios.

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

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

#1426541 — Re: [PATCH v4 4/7] max8903: adds requesting of gpios.

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

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

#1426278 — [PATCH v4 7/7] max8903: adds support for initiation via device tree

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

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

#1426545 — Re: [PATCH v4 7/7] max8903: adds support for initiation via device tree

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

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

#1426281 — [PATCH v4 2/7] max8903: store pointer to pdata instead of copying it.

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

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

#1426534 — Re: [PATCH v4 2/7] max8903: store pointer to pdata instead of copying it.

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

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

#1426302 — [PATCH v4 3/7] max8903: cleans up confusing relationship between dc_valid, dok and dcm.

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

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

#1426537 — Re: [PATCH v4 3/7] max8903: cleans up confusing relationship between dc_valid, dok and dcm.

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

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

#1426303 — [PATCH v4 5/7] max8903: removes non zero validity checks on gpios.

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

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

#1426554 — Re: [PATCH v4 5/7] max8903: removes non zero validity checks on gpios.

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web