diff mbox

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

Message ID 1464849897-21527-3-git-send-email-chris@lapa.com.au (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

chris@lapa.com.au June 2, 2016, 6:44 a.m. UTC 
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(-)

Comments

chris@lapa.com.au June 10, 2016, 12:32 p.m. UTC | #1 
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
chris@lapa.com.au June 17, 2016, 5 a.m. UTC | #2 
From: Chris Lapa <chris@lapa.com.au>

This patch set adds device tree support for the MAX8903 battery charger.
It also cleans up logic with dc_valid, dok and dcm pins as well as
fixing up validity checking of gpios.

I verified these patches work on a board I have here, which uses the
DC power side (not the USB portition) of the MAX8903.

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

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

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

 .../devicetree/bindings/power/max8903-charger.txt  |  25 ++
 drivers/power/max8903_charger.c                    | 278 +++++++++++++++------
 include/linux/power/max8903_charger.h              |   6 +-
 3 files changed, 227 insertions(+), 82 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power/max8903-charger.txt
chris@lapa.com.au June 19, 2016, 10:27 p.m. UTC | #3 
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
chris@lapa.com.au June 24, 2016, 2:26 a.m. UTC | #4 
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
Sebastian Reichel June 28, 2016, 6:16 p.m. UTC | #5 
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
chris@lapa.com.au June 28, 2016, 11:05 p.m. UTC | #6 
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
>

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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.
 	 */