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

Groups > linux.kernel > #1430305 > unrolled thread

[PATCH v5 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 v5 0/7] max8903: Add device tree support and misc fixes Chris Lapa <chris@lapa.com.au> - 2016-06-24 04:30 +0200
    [PATCH v5 5/7] max8903: removes non zero validity checks on gpios. Chris Lapa <chris@lapa.com.au> - 2016-06-24 04:30 +0200
    [PATCH v5 7/7] max8903: adds support for initiation via device tree Chris Lapa <chris@lapa.com.au> - 2016-06-24 04:30 +0200
    [PATCH v5 1/7] max8903: adds documentation for device tree bindings. Chris Lapa <chris@lapa.com.au> - 2016-06-24 04:30 +0200
      Re: [PATCH v5 1/7] max8903: adds documentation for device tree  bindings. Rob Herring <robh@kernel.org> - 2016-06-28 23:00 +0200
    Re: [PATCH v5 0/7] max8903: Add device tree support and misc fixes Sebastian Reichel <sre@kernel.org> - 2016-06-28 20:20 +0200
      Re: [PATCH v5 0/7] max8903: Add device tree support and misc fixes Chris Lapa <chris@lapa.com.au> - 2016-06-29 01:10 +0200

#1430305 — [PATCH v5 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 v4 -> v5:
 * Changes compatible field from max8903_charger to max8903 in 1/7 and 7/7
 * Improves DT documentation to include direction and state for each gpio

Changes v3 -> v4:
 * Fixed formatting, such as multiline strings and indentation mistakes
 * Moved gpio setup code into max8903_setup_gpios() in 3/7
 * Fixed typo in 5/7
 * Renamed of_node to np in 7/7

Changes v2 -> v3:
 * Separate requesting of gpio's into its own commit
 * Fixed up validity checking of GPIO's
 * Remove dc_valid and usb_valid from device tree
 * Remove some unncessary init to psy_cfg.num_supplicants and psy_cfg.supplied_to
 * Reorder patches so device tree implementation is final patch

Changes v1 -> v2:
 * Separate DT bindings documentation into its own commit
 * Add maxim prefix to DT compatible field
 * Add gpios suffix to gpio's in DT
 * Remove malloc failed error message


Chris Lapa (7):
  max8903: adds documentation for device tree bindings.
  max8903: store pointer to pdata instead of copying it.
  max8903: cleans up confusing relationship between dc_valid, dok and
    dcm.
  max8903: adds requesting of gpios.
  max8903: removes non zero validity checks on gpios.
  max8903: remove unnecessary 'out of memory' error message.
  max8903: adds support for initiation via device tree

 .../devicetree/bindings/power/max8903-charger.txt  |  25 +++
 arch/arm/boot/dts/dairytest-servo.dtsi             |  36 ++++
 drivers/power/max8903_charger.c                    | 239 +++++++++++++++------
 include/linux/power/max8903_charger.h              |   6 +-
 4 files changed, 240 insertions(+), 66 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power/max8903-charger.txt
 create mode 100644 arch/arm/boot/dts/dairytest-servo.dtsi

-- 
1.9.1

[toc] | [next] | [standalone]

#1430306 — [PATCH v5 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>

Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/power/max8903_charger.c | 115 ++++++++++++++++------------------------
 1 file changed, 47 insertions(+), 68 deletions(-)

diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c
index 3f35593..643a87a 100644
--- a/drivers/power/max8903_charger.c
+++ b/drivers/power/max8903_charger.c
@@ -53,7 +53,7 @@ static int max8903_get_property(struct power_supply *psy,
 	switch (psp) {
 	case POWER_SUPPLY_PROP_STATUS:
 		val->intval = POWER_SUPPLY_STATUS_UNKNOWN;
-		if (data->pdata->chg) {
+		if (gpio_is_valid(data->pdata->chg)) {
 			if (gpio_get_value(data->pdata->chg) == 0)
 				val->intval = POWER_SUPPLY_STATUS_CHARGING;
 			else if (data->usb_in || data->ta_in)
@@ -93,11 +93,11 @@ static irqreturn_t max8903_dcin(int irq, void *_data)
 	data->ta_in = ta_in;
 
 	/* Set Current-Limit-Mode 1:DC 0:USB */
-	if (pdata->dcm)
+	if (gpio_is_valid(pdata->dcm))
 		gpio_set_value(pdata->dcm, ta_in ? 1 : 0);
 
 	/* Charger Enable / Disable (cen is negated) */
-	if (pdata->cen)
+	if (gpio_is_valid(pdata->cen))
 		gpio_set_value(pdata->cen, ta_in ? 0 :
 				(data->usb_in ? 0 : 1));
 
@@ -136,7 +136,7 @@ static irqreturn_t max8903_usbin(int irq, void *_data)
 	/* Do not touch Current-Limit-Mode */
 
 	/* Charger Enable / Disable (cen is negated) */
-	if (pdata->cen)
+	if (gpio_is_valid(pdata->cen))
 		gpio_set_value(pdata->cen, usb_in ? 0 :
 				(data->ta_in ? 0 : 1));
 
@@ -190,7 +190,7 @@ static int max8903_setup_gpios(struct platform_device *pdev)
 	int usb_in = 0;
 
 	if (pdata->dc_valid) {
-		if (pdata->dok && gpio_is_valid(pdata->dok)) {
+		if (gpio_is_valid(pdata->dok)) {
 			ret = devm_gpio_request(dev, pdata->dok,
 						data->psy_desc.name);
 			if (ret) {
@@ -208,27 +208,21 @@ static int max8903_setup_gpios(struct platform_device *pdev)
 		}
 	}
 
-	if (pdata->dcm) {
-		if (gpio_is_valid(pdata->dcm)) {
-			ret = devm_gpio_request(dev, pdata->dcm,
-						data->psy_desc.name);
-			if (ret) {
-				dev_err(dev,
-					"Failed GPIO request for dcm: %d err %d\n",
-					pdata->dcm, ret);
-				return ret;
-			}
-
-			gpio = pdata->dcm; /* Output */
-			gpio_set_value(gpio, ta_in);
-		} else {
-			dev_err(dev, "Invalid pin: dcm.\n");
-			return -EINVAL;
+	if (gpio_is_valid(pdata->dcm)) {
+		ret = devm_gpio_request(dev, pdata->dcm, data->psy_desc.name);
+		if (ret) {
+			dev_err(dev,
+				"Failed GPIO request for dcm: %d err %d\n",
+				pdata->dcm, ret);
+			return ret;
 		}
+
+		gpio = pdata->dcm; /* Output */
+		gpio_set_value(gpio, ta_in);
 	}
 
 	if (pdata->usb_valid) {
-		if (pdata->uok && gpio_is_valid(pdata->uok)) {
+		if (gpio_is_valid(pdata->uok)) {
 			ret = devm_gpio_request(dev, pdata->uok,
 						data->psy_desc.name);
 			if (ret) {
@@ -247,60 +241,45 @@ static int max8903_setup_gpios(struct platform_device *pdev)
 		}
 	}
 
-	if (pdata->cen) {
-		if (gpio_is_valid(pdata->cen)) {
-			ret = devm_gpio_request(dev, pdata->cen,
-						data->psy_desc.name);
-			if (ret) {
-				dev_err(dev,
-					"Failed GPIO request for cen: %d err %d\n",
-					pdata->cen, ret);
-				return ret;
-			}
-
-			gpio_set_value(pdata->cen, (ta_in || usb_in) ? 0 : 1);
-		} else {
-			dev_err(dev, "Invalid pin: cen.\n");
-			return -EINVAL;
+	if (gpio_is_valid(pdata->cen)) {
+		ret = devm_gpio_request(dev, pdata->cen, data->psy_desc.name);
+		if (ret) {
+			dev_err(dev,
+				"Failed GPIO request for cen: %d err %d\n",
+				pdata->cen, ret);
+			return ret;
 		}
+
+		gpio_set_value(pdata->cen, (ta_in || usb_in) ? 0 : 1);
 	}
 
-	if (pdata->chg) {
-		if (gpio_is_valid(pdata->chg)) {
-			ret = devm_gpio_request(dev, pdata->chg,
-						data->psy_desc.name);
-			if (ret) {
-				dev_err(dev,
-					"Failed GPIO request for chg: %d err %d\n",
-					pdata->chg, ret);
-				return ret;
-			}
+	if (gpio_is_valid(pdata->chg)) {
+		ret = devm_gpio_request(dev, pdata->chg, data->psy_desc.name);
+		if (ret) {
+			dev_err(dev,
+				"Failed GPIO request for chg: %d err %d\n",
+				pdata->chg, ret);
+			return ret;
 		}
 	}
 
-	if (pdata->flt) {
-		if (gpio_is_valid(pdata->flt)) {
-			ret = devm_gpio_request(dev, pdata->flt,
-						data->psy_desc.name);
-			if (ret) {
-				dev_err(dev,
-					"Failed GPIO request for flt: %d err %d\n",
-					pdata->flt, ret);
-				return ret;
-			}
+	if (gpio_is_valid(pdata->flt)) {
+		ret = devm_gpio_request(dev, pdata->flt, data->psy_desc.name);
+		if (ret) {
+			dev_err(dev,
+				"Failed GPIO request for flt: %d err %d\n",
+				pdata->flt, ret);
+			return ret;
 		}
 	}
 
-	if (pdata->usus) {
-		if (gpio_is_valid(pdata->usus)) {
-			ret = devm_gpio_request(dev, pdata->usus,
-						data->psy_desc.name);
-			if (ret) {
-				dev_err(dev,
-					"Failed GPIO request for usus: %d err %d\n",
-					pdata->usus, ret);
-				return ret;
-			}
+	if (gpio_is_valid(pdata->usus)) {
+		ret = devm_gpio_request(dev, pdata->usus, data->psy_desc.name);
+		if (ret) {
+			dev_err(dev,
+				"Failed GPIO request for usus: %d err %d\n",
+				pdata->usus, ret);
+			return ret;
 		}
 	}
 
@@ -385,7 +364,7 @@ static int max8903_probe(struct platform_device *pdev)
 		}
 	}
 
-	if (pdata->flt) {
+	if (gpio_is_valid(pdata->flt)) {
 		ret = devm_request_threaded_irq(dev, gpio_to_irq(pdata->flt),
 					NULL, max8903_fault,
 					IRQF_TRIGGER_FALLING |
-- 
1.9.1

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

#1430307 — [PATCH v5 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..fdc73d6 100644
--- a/drivers/power/max8903_charger.c
+++ b/drivers/power/max8903_charger.c
@@ -23,6 +23,9 @@
 #include <linux/gpio.h>
 #include <linux/interrupt.h>
 #include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
 #include <linux/slab.h>
 #include <linux/power_supply.h>
 #include <linux/platform_device.h>
@@ -75,6 +78,7 @@ static int max8903_get_property(struct power_supply *psy,
 	default:
 		return -EINVAL;
 	}
+
 	return 0;
 }
 
@@ -179,6 +183,56 @@ static irqreturn_t max8903_fault(int irq, void *_data)
 	return IRQ_HANDLED;
 }
 
+static struct max8903_pdata *max8903_parse_dt_data(struct device *dev)
+{
+	struct device_node *np = dev->of_node;
+	struct max8903_pdata *pdata = NULL;
+
+	if (!np)
+		return NULL;
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return NULL;
+
+	pdata->dc_valid = false;
+	pdata->usb_valid = false;
+
+	pdata->cen = of_get_named_gpio(np, "cen-gpios", 0);
+	if (!gpio_is_valid(pdata->cen))
+		pdata->cen = -EINVAL;
+
+	pdata->chg = of_get_named_gpio(np, "chg-gpios", 0);
+	if (!gpio_is_valid(pdata->chg))
+		pdata->chg = -EINVAL;
+
+	pdata->flt = of_get_named_gpio(np, "flt-gpios", 0);
+	if (!gpio_is_valid(pdata->flt))
+		pdata->flt = -EINVAL;
+
+	pdata->usus = of_get_named_gpio(np, "usus-gpios", 0);
+	if (!gpio_is_valid(pdata->usus))
+		pdata->usus = -EINVAL;
+
+	pdata->dcm = of_get_named_gpio(np, "dcm-gpios", 0);
+	if (!gpio_is_valid(pdata->dcm))
+		pdata->dcm = -EINVAL;
+
+	pdata->dok = of_get_named_gpio(np, "dok-gpios", 0);
+	if (!gpio_is_valid(pdata->dok))
+		pdata->dok = -EINVAL;
+	else
+		pdata->dc_valid = true;
+
+	pdata->uok = of_get_named_gpio(np, "uok-gpios", 0);
+	if (!gpio_is_valid(pdata->uok))
+		pdata->uok = -EINVAL;
+	else
+		pdata->usb_valid = true;
+
+	return pdata;
+}
+
 static int max8903_setup_gpios(struct platform_device *pdev)
 {
 	struct max8903_data *data = platform_get_drvdata(pdev);
@@ -298,16 +352,20 @@ static int max8903_probe(struct platform_device *pdev)
 	struct power_supply_config psy_cfg = {};
 	int ret = 0;
 
-	if (pdata == NULL) {
-		dev_err(dev, "No platform data.\n");
-		return -EINVAL;
-	}
-
 	data = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
 
-	data->pdata = pdev->dev.platform_data;
+	if (IS_ENABLED(CONFIG_OF) && !pdata && dev->of_node)
+		pdata = max8903_parse_dt_data(dev);
+
+	if (!pdata) {
+		dev_err(dev, "No platform data.\n");
+		return -EINVAL;
+	}
+
+	pdev->dev.platform_data = pdata;
+	data->pdata = pdata;
 	data->dev = dev;
 	platform_set_drvdata(pdev, data);
 
@@ -328,6 +386,7 @@ static int max8903_probe(struct platform_device *pdev)
 	data->psy_desc.properties = max8903_charger_props;
 	data->psy_desc.num_properties = ARRAY_SIZE(max8903_charger_props);
 
+	psy_cfg.of_node = dev->of_node;
 	psy_cfg.drv_data = data;
 
 	data->psy = devm_power_supply_register(dev, &data->psy_desc, &psy_cfg);
@@ -378,10 +437,17 @@ static int max8903_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id max8903_match_ids[] = {
+	{ .compatible = "maxim,max8903", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, max8903_match_ids);
+
 static struct platform_driver max8903_driver = {
 	.probe	= max8903_probe,
 	.driver = {
 		.name	= "max8903-charger",
+		.of_match_table = max8903_match_ids
 	},
 };
 
-- 
1.9.1

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

#1430308 — [PATCH v5 1/7] max8903: adds documentation for device tree bindings.

From: Chris Lapa <chris@lapa.com.au>

Signed-off-by: Chris Lapa <chris@lapa.com.au>
---
 .../devicetree/bindings/power/max8903-charger.txt  | 25 +++++++++++++++
 arch/arm/boot/dts/dairytest-servo.dtsi             | 36 ++++++++++++++++++++++
 2 files changed, 61 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/max8903-charger.txt
 create mode 100644 arch/arm/boot/dts/dairytest-servo.dtsi

diff --git a/Documentation/devicetree/bindings/power/max8903-charger.txt b/Documentation/devicetree/bindings/power/max8903-charger.txt
new file mode 100644
index 0000000..f0f4e12
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/max8903-charger.txt
@@ -0,0 +1,25 @@
+Maxim Semiconductor MAX8903 Battery Charger bindings
+
+Required properties:
+- compatible: "maxim,max8903" for MAX8903 Battery Charger
+- dok-gpios: Valid DC power has been detected (active low, input), optional if uok-gpios is provided
+- uok-gpios: Valid USB power has been detected (active low, input), optional if dok-gpios is provided
+
+Optional properties:
+- cen-gpios: Charge enable pin (active low, output)
+- chg-gpios: Charger status pin (active low, input)
+- flt-gpios: Fault pin (active low, output)
+- dcm-gpios: Current limit mode setting (DC=1 or USB=0, output)
+- usus-gpios: USB suspend pin (active high, output)
+
+
+Example:
+
+	max8903-charger {
+		compatible = "maxim,max8903";
+		dok-gpios = <&gpio2 3 GPIO_ACTIVE_LOW>;
+		flt-gpios = <&gpio2 2 GPIO_ACTIVE_LOW>;
+		chg-gpios = <&gpio3 15 GPIO_ACTIVE_LOW>;
+		cen-gpios = <&gpio2 5 GPIO_ACTIVE_LOW>;
+		status = "okay";
+	};
diff --git a/arch/arm/boot/dts/dairytest-servo.dtsi b/arch/arm/boot/dts/dairytest-servo.dtsi
new file mode 100644
index 0000000..3fbe81d
--- /dev/null
+++ b/arch/arm/boot/dts/dairytest-servo.dtsi
@@ -0,0 +1,36 @@
+#include <dt-bindings/board/am335x-bbw-bbb-base.h>
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/pinctrl/am33xx.h>
+
+&am33xx_pinmux {
+    servo_power_pin: pinmux_servo_power_pin{
+        pinctrl-single,pins = <
+            BONE_P9_27 (PIN_OUTPUT| MUX_MODE7)  /* ZCZ(C13) - gpio3_19, OUTPUT | MODE7 */
+        >;
+    };
+
+    ehrpwm2_pin: pinmux_ehrpwm2_pin{
+        pinctrl-single,pins = <
+            BONE_P8_19 (PIN_OUTPUT | MUX_MODE4) /* ZCZ(R1) - ehrpwm2a, OUTPUT | MODE3 */
+        >;
+    };
+};
+
+/ {
+    servo_power {
+        status = "okay";
+        compatible = "pinctrl-single";
+        pinctrl-names = "default";
+        pinctrl-0 = <&servo_power_pin>;
+    };
+};
+
+&epwmss2 {
+    status = "okay";
+};
+
+&ehrpwm2 {
+    pinctrl-names = "default";
+    pinctrl-0 = <&ehrpwm2_pin>;
+    status = "okay";
+};
-- 
1.9.1

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

#1433299 — Re: [PATCH v5 1/7] max8903: adds documentation for device tree bindings.

On Fri, Jun 24, 2016 at 12:26:06PM +1000, Chris Lapa wrote:
> From: Chris Lapa <chris@lapa.com.au>
> 
> Signed-off-by: Chris Lapa <chris@lapa.com.au>
> ---
>  .../devicetree/bindings/power/max8903-charger.txt  | 25 +++++++++++++++
>  arch/arm/boot/dts/dairytest-servo.dtsi             | 36 ++++++++++++++++++++++

Binding looks fine, but this dtsi is unrelated changed.

>  2 files changed, 61 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/max8903-charger.txt
>  create mode 100644 arch/arm/boot/dts/dairytest-servo.dtsi

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

#1433204

[Multipart message — attachments visible in raw view] — view raw

Hi Chris,

On Fri, Jun 24, 2016 at 12:26:05PM +1000, Chris Lapa wrote:
> This patch set adds device tree support for the MAX8903 battery charger.
> It also cleans up logic with dc_valid, dok and dcm pins as well as
> fixing up validity checking of gpios.
> 
> I verified these patches work on a board I have here, which uses the
> DC power side (not the USB portition) of the MAX8903.

I queued this into my for-next branch, except for the dtsi file,
which I dropped from your first patch. Please submit it separately
to the correct maintainers.

[...]

>  arch/arm/boot/dts/dairytest-servo.dtsi             |  36 ++++

[...]

-- Sebastian

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

#1433367

Hi Sebastian,

Sorry about the extra dtsi file, I accidentally included it in the v5 
patch set (wasn't in <= v4).

Thanks,
Chris

On 29/06/2016 4:16 AM, Sebastian Reichel wrote:
> Hi Chris,
>
> On Fri, Jun 24, 2016 at 12:26:05PM +1000, Chris Lapa wrote:
>> This patch set adds device tree support for the MAX8903 battery charger.
>> It also cleans up logic with dc_valid, dok and dcm pins as well as
>> fixing up validity checking of gpios.
>>
>> I verified these patches work on a board I have here, which uses the
>> DC power side (not the USB portition) of the MAX8903.
>
> I queued this into my for-next branch, except for the dtsi file,
> which I dropped from your first patch. Please submit it separately
> to the correct maintainers.
>
> [...]
>
>>  arch/arm/boot/dts/dairytest-servo.dtsi             |  36 ++++
>
> [...]
>
> -- Sebastian
>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web