[PATCH v8 00/11] power: act8945a_charger: Improvements

[PATCH v8 00/11] power: act8945a_charger: Improvements

[Wenyou Yang]

This patch series is to improve the implementation of act8945a-charger
driver, such as regard the act8945-charger as a sub-device, improve
the way to check the status, fix the power supply type property,
add the status change update, and add more properties: capacity level
property and max current property.

Changes in v8:
 - Achieve "interrupts" property from its own node directly.
 - As the act8945a_charger is regarded as a sub-device, all
   properties can be achieved from its own device node,
   use devm_gpiod_get() properly as well to get "lbo-gpios".
 - Add missing return -EPROBE_DEFER  for "lbo-gpios".
 - As the act8945a_charger is regarded as a sub-device, all
   properties can be achieved from its own device node,
   use devm_gpiod_get() properly as well to get "chglev-gpios".
 - Add missing return -EPROBE_DEFER  for "chglev-gpios".
 - Since the act8945a-charger is regarded as a sub-device, update
   the compatible decription and examples.
 - Add 4 new patches: 1/11, 2/11, 9/11, and 11/11.

Changes in v7:
 - Fix the missing negation sign.
 - Add a prompt information if the charger suspended.
 - Fix the wrong using devm_request_irq() with with parent
   device as *dev.
 - For "lbo-gpios", use gpiod_get() to fix devm_gpiod_get() wrong use
   with parent device as *dev argument.
 - Add the handle -EPROBE_DEFER returned from gpiod_get "lbo-gpios".
 - Use dev_info() to print log if the lbo irq request failed.
 - Remove unneeded semicolon.
 - Add depends on GPIOLIB for use gpiod.
 - For "chglev-gpios", use gpiod_get() to fix devm_gpiod_get() wrong use
   with parent device as *dev argument.
 - Add the handle -EPROBE_DEFER returned from gpiod_get "chglev-gpio".
 - Remove unneeded semicolon.

Changes in v6:
 - Add .remove callback function.
 - Fix the 'dev' argument of devm_request_irq() to pdev->dev.parent.
 - For "lbo-gpios", use gpiod API instead of old gpio API to handle.
 - For "chglev-gpios", use gpiod API instead of old gpio API to handle.

Changes in v5:
 - Add Acked-by tag.

Changes in v4:
 - Use "interrupts" property, instead of "active-semi,irq-gpios"
   for irq.
 - Change devname of devm_request_irq() from "lbo-detect" to
   "act8945a, lbo-detect".
 - Fix wrong gpio assignment for chglev_pin.
 - Update the doc/binding for using "interrupts" property.

Changes in v3:
 - Remove unneeded semicolon to fix semicolon.cocci warning.

Changes in v2:
 - Add missing ret declaration.

Wenyou Yang (11):
  mfd: act8945a: Add .of_compatible for act8945a-charger
  power: act8945a_charger: Achieve properties from its node
  power: act8945a_charger: Remove "battery_temperature"
  power: act8945a_charger: Improve
  power: act8945a_charger: Add status change update support
  power: act8945a_charger: Fix the power supply type
  power: act8945a_charger: Add capacity level property
  power: act8945a_charger: Add max current property
  doc: bindings: mfd: act8945a: Update the example
  doc: bindings: power: act8945a-charger: Update properties.
  ARM: at91/dt: sama5d2_xplained: Add act8945a-charger node.

 Documentation/devicetree/bindings/mfd/act8945a.txt |  22 +-
 .../bindings/power/supply/act8945a-charger.txt     |  31 +-
 arch/arm/boot/dts/at91-sama5d2_xplained.dts        |  23 +-
 drivers/mfd/act8945a.c                             |   1 +
 drivers/power/supply/Kconfig                       |   2 +-
 drivers/power/supply/act8945a_charger.c            | 401 ++++++++++++++++++---
 6 files changed, 409 insertions(+), 71 deletions(-)

-- 
2.7.4

[PATCH v8 01/11] mfd: act8945a: Add .of_compatible for act8945a-charger

[Wenyou Yang]

To regard the act8945a-charger as a sub-device, add .of_compatible for
act8945a-charger cell.

Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
---

Changes in v8: None
Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

 drivers/mfd/act8945a.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mfd/act8945a.c b/drivers/mfd/act8945a.c
index 10c6d2d..a4024d9 100644
--- a/drivers/mfd/act8945a.c
+++ b/drivers/mfd/act8945a.c
@@ -23,6 +23,7 @@ static const struct mfd_cell act8945a_devs[] = {
 	},
 	{
 		.name = "act8945a-charger",
+		.of_compatible = "active-semi,act8945a-charger",
 	},
 };
 
-- 
2.7.4

Re: [PATCH v8 01/11] mfd: act8945a: Add .of_compatible for act8945a-charger

[Lee Jones]

On Thu, 25 Aug 2016, Wenyou Yang wrote:

> To regard the act8945a-charger as a sub-device, add .of_compatible for
> act8945a-charger cell.
> 
> Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
> ---
> 
> Changes in v8: None
> Changes in v7: None
> Changes in v6: None
> Changes in v5: None
> Changes in v4: None
> Changes in v3: None
> Changes in v2: None
> 
>  drivers/mfd/act8945a.c | 1 +
>  1 file changed, 1 insertion(+)

Applied, thanks.

> diff --git a/drivers/mfd/act8945a.c b/drivers/mfd/act8945a.c
> index 10c6d2d..a4024d9 100644
> --- a/drivers/mfd/act8945a.c
> +++ b/drivers/mfd/act8945a.c
> @@ -23,6 +23,7 @@ static const struct mfd_cell act8945a_devs[] = {
>  	},
>  	{
>  		.name = "act8945a-charger",
> +		.of_compatible = "active-semi,act8945a-charger",
>  	},
>  };
>  

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH v8 02/11] power: act8945a_charger: Achieve properties from its node

[Wenyou Yang]

Since the act8945a_charger is regarded as a sub-device, all properties will
be assigned to its own device node. All properties can be achieved from its
own node, instead of from its parent device.

Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
---

Changes in v8: None
Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

 drivers/power/supply/act8945a_charger.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/power/supply/act8945a_charger.c b/drivers/power/supply/act8945a_charger.c
index b5c00e4..3f486c9 100644
--- a/drivers/power/supply/act8945a_charger.c
+++ b/drivers/power/supply/act8945a_charger.c
@@ -328,11 +328,11 @@ static int act8945a_charger_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	ret = act8945a_charger_config(pdev->dev.parent, charger);
+	ret = act8945a_charger_config(&pdev->dev, charger);
 	if (ret)
 		return ret;
 
-	psy_cfg.of_node	= pdev->dev.parent->of_node;
+	psy_cfg.of_node	= pdev->dev.of_node;
 	psy_cfg.drv_data = charger;
 
 	psy = devm_power_supply_register(&pdev->dev,
-- 
2.7.4

Re: [PATCH v8 02/11] power: act8945a_charger: Achieve properties from its node

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 300 bytes --]

Hi,

On Thu, Aug 25, 2016 at 03:19:50PM +0800, Wenyou Yang wrote:
> Since the act8945a_charger is regarded as a sub-device, all properties will
> be assigned to its own device node. All properties can be achieved from its
> own node, instead of from its parent device.

Thanks, queued.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v8 03/11] power: act8945a_charger: Remove "battery_temperature"

[Wenyou Yang]

Remove "battery_temperature" member, it is redundant, it is the
hardware's responsibility to handle TH pin properly.
It is unnecessary to use the dt property to check if there is
a battery temperature monitor or not.

Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
---

Changes in v8: None
Changes in v7:
 - Fix the missing negation sign.

Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

 drivers/power/supply/act8945a_charger.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/power/supply/act8945a_charger.c b/drivers/power/supply/act8945a_charger.c
index 3f486c9..af00668 100644
--- a/drivers/power/supply/act8945a_charger.c
+++ b/drivers/power/supply/act8945a_charger.c
@@ -76,7 +76,6 @@ static const char *act8945a_charger_manufacturer = "Active-semi";
 
 struct act8945a_charger {
 	struct regmap *regmap;
-	bool battery_temperature;
 };
 
 static int act8945a_get_charger_state(struct regmap *regmap, int *val)
@@ -138,8 +137,7 @@ static int act8945a_get_charge_type(struct regmap *regmap, int *val)
 	return 0;
 }
 
-static int act8945a_get_battery_health(struct act8945a_charger *charger,
-				       struct regmap *regmap, int *val)
+static int act8945a_get_battery_health(struct regmap *regmap, int *val)
 {
 	int ret;
 	unsigned int status;
@@ -148,7 +146,7 @@ static int act8945a_get_battery_health(struct act8945a_charger *charger,
 	if (ret < 0)
 		return ret;
 
-	if (charger->battery_temperature && !(status & APCH_STATUS_TEMPDAT))
+	if (!(status & APCH_STATUS_TEMPDAT))
 		*val = POWER_SUPPLY_HEALTH_OVERHEAT;
 	else if (!(status & APCH_STATUS_INDAT))
 		*val = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
@@ -188,8 +186,7 @@ static int act8945a_charger_get_property(struct power_supply *psy,
 		val->intval = POWER_SUPPLY_TECHNOLOGY_LION;
 		break;
 	case POWER_SUPPLY_PROP_HEALTH:
-		ret = act8945a_get_battery_health(charger,
-						  regmap, &val->intval);
+		ret = act8945a_get_battery_health(regmap, &val->intval);
 		break;
 	case POWER_SUPPLY_PROP_MODEL_NAME:
 		val->strval = act8945a_charger_model;
@@ -235,9 +232,6 @@ static int act8945a_charger_config(struct device *dev,
 		return -EINVAL;
 	}
 
-	charger->battery_temperature = of_property_read_bool(np,
-				"active-semi,check-battery-temperature");
-
 	chglev_pin = of_get_named_gpio_flags(np,
 				"active-semi,chglev-gpios", 0, &flags);
 
-- 
2.7.4

Re: [PATCH v8 03/11] power: act8945a_charger: Remove "battery_temperature"

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 391 bytes --]

Hi,

On Thu, Aug 25, 2016 at 03:19:51PM +0800, Wenyou Yang wrote:
> Remove "battery_temperature" member, it is redundant, it is the
> hardware's responsibility to handle TH pin properly.
> It is unnecessary to use the dt property to check if there is
> a battery temperature monitor or not.
> 
> Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>

Thanks, queued.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v8 04/11] power: act8945a_charger: Improve

[Wenyou Yang]

When get the property, first check the charger state machine,
then check the status bit to decide what value is assigned to
the corresponding property.

Retain the SUSCHG bit of REG 0x71 when configure the timers to
avoid losting the charger suspending info after boot.

Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---

Changes in v8: None
Changes in v7:
 - Add a prompt information if the charger suspended.

Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3:
 - Remove unneeded semicolon to fix semicolon.cocci warning.

Changes in v2:
 - Add missing ret declaration.

 drivers/power/supply/act8945a_charger.c | 86 +++++++++++++++++++++++++++------
 1 file changed, 70 insertions(+), 16 deletions(-)

diff --git a/drivers/power/supply/act8945a_charger.c b/drivers/power/supply/act8945a_charger.c
index af00668..a6bbaff 100644
--- a/drivers/power/supply/act8945a_charger.c
+++ b/drivers/power/supply/act8945a_charger.c
@@ -94,16 +94,24 @@ static int act8945a_get_charger_state(struct regmap *regmap, int *val)
 	state &= APCH_STATE_CSTATE;
 	state >>= APCH_STATE_CSTATE_SHIFT;
 
-	if (state == APCH_STATE_CSTATE_EOC) {
+	switch (state) {
+	case APCH_STATE_CSTATE_PRE:
+	case APCH_STATE_CSTATE_FAST:
+		*val = POWER_SUPPLY_STATUS_CHARGING;
+		break;
+	case APCH_STATE_CSTATE_EOC:
 		if (status & APCH_STATUS_CHGDAT)
 			*val = POWER_SUPPLY_STATUS_FULL;
 		else
+			*val = POWER_SUPPLY_STATUS_CHARGING;
+		break;
+	case APCH_STATE_CSTATE_DISABLED:
+	default:
+		if (!(status & APCH_STATUS_INDAT))
+			*val = POWER_SUPPLY_STATUS_DISCHARGING;
+		else
 			*val = POWER_SUPPLY_STATUS_NOT_CHARGING;
-	} else if ((state == APCH_STATE_CSTATE_FAST) ||
-		   (state == APCH_STATE_CSTATE_PRE)) {
-		*val = POWER_SUPPLY_STATUS_CHARGING;
-	} else {
-		*val = POWER_SUPPLY_STATUS_NOT_CHARGING;
+		break;
 	}
 
 	return 0;
@@ -112,7 +120,11 @@ static int act8945a_get_charger_state(struct regmap *regmap, int *val)
 static int act8945a_get_charge_type(struct regmap *regmap, int *val)
 {
 	int ret;
-	unsigned int state;
+	unsigned int status, state;
+
+	ret = regmap_read(regmap, ACT8945A_APCH_STATUS, &status);
+	if (ret < 0)
+		return ret;
 
 	ret = regmap_read(regmap, ACT8945A_APCH_STATE, &state);
 	if (ret < 0)
@@ -129,9 +141,15 @@ static int act8945a_get_charge_type(struct regmap *regmap, int *val)
 		*val = POWER_SUPPLY_CHARGE_TYPE_FAST;
 		break;
 	case APCH_STATE_CSTATE_EOC:
+		*val = POWER_SUPPLY_CHARGE_TYPE_NONE;
+		break;
 	case APCH_STATE_CSTATE_DISABLED:
 	default:
-		*val = POWER_SUPPLY_CHARGE_TYPE_NONE;
+		if (!(status & APCH_STATUS_INDAT))
+			*val = POWER_SUPPLY_CHARGE_TYPE_NONE;
+		else
+			*val = POWER_SUPPLY_CHARGE_TYPE_UNKNOWN;
+		break;
 	}
 
 	return 0;
@@ -140,20 +158,45 @@ static int act8945a_get_charge_type(struct regmap *regmap, int *val)
 static int act8945a_get_battery_health(struct regmap *regmap, int *val)
 {
 	int ret;
-	unsigned int status;
+	unsigned int status, state, config;
 
 	ret = regmap_read(regmap, ACT8945A_APCH_STATUS, &status);
 	if (ret < 0)
 		return ret;
 
-	if (!(status & APCH_STATUS_TEMPDAT))
-		*val = POWER_SUPPLY_HEALTH_OVERHEAT;
-	else if (!(status & APCH_STATUS_INDAT))
-		*val = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
-	else if (status & APCH_STATUS_TIMRDAT)
-		*val = POWER_SUPPLY_HEALTH_SAFETY_TIMER_EXPIRE;
-	else
+	ret = regmap_read(regmap, ACT8945A_APCH_CFG, &config);
+	if (ret < 0)
+		return ret;
+
+	ret = regmap_read(regmap, ACT8945A_APCH_STATE, &state);
+	if (ret < 0)
+		return ret;
+
+	state &= APCH_STATE_CSTATE;
+	state >>= APCH_STATE_CSTATE_SHIFT;
+
+	switch (state) {
+	case APCH_STATE_CSTATE_DISABLED:
+		if (config & APCH_CFG_SUSCHG) {
+			*val = POWER_SUPPLY_HEALTH_UNKNOWN;
+		} else if (status & APCH_STATUS_INDAT) {
+			if (!(status & APCH_STATUS_TEMPDAT))
+				*val = POWER_SUPPLY_HEALTH_OVERHEAT;
+			else if (status & APCH_STATUS_TIMRDAT)
+				*val = POWER_SUPPLY_HEALTH_SAFETY_TIMER_EXPIRE;
+			else
+				*val = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
+		} else {
+			*val = POWER_SUPPLY_HEALTH_GOOD;
+		}
+		break;
+	case APCH_STATE_CSTATE_PRE:
+	case APCH_STATE_CSTATE_FAST:
+	case APCH_STATE_CSTATE_EOC:
+	default:
 		*val = POWER_SUPPLY_HEALTH_GOOD;
+		break;
+	}
 
 	return 0;
 }
@@ -224,7 +267,9 @@ static int act8945a_charger_config(struct device *dev,
 	u32 pre_time_out;
 	u32 input_voltage_threshold;
 	int chglev_pin;
+	int ret;
 
+	unsigned int tmp;
 	unsigned int value = 0;
 
 	if (!np) {
@@ -232,6 +277,15 @@ static int act8945a_charger_config(struct device *dev,
 		return -EINVAL;
 	}
 
+	ret = regmap_read(regmap, ACT8945A_APCH_CFG, &tmp);
+	if (ret)
+		return ret;
+
+	if (tmp & APCH_CFG_SUSCHG) {
+		value |= APCH_CFG_SUSCHG;
+		dev_info(dev, "have been suspended\n");
+	}
+
 	chglev_pin = of_get_named_gpio_flags(np,
 				"active-semi,chglev-gpios", 0, &flags);
 
-- 
2.7.4

Re: [PATCH v8 04/11] power: act8945a_charger: Improve

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 506 bytes --]

Hi,

On Thu, Aug 25, 2016 at 03:19:52PM +0800, Wenyou Yang wrote:
> When get the property, first check the charger state machine,
> then check the status bit to decide what value is assigned to
> the corresponding property.
> 
> Retain the SUSCHG bit of REG 0x71 when configure the timers to
> avoid losting the charger suspending info after boot.
> 
> Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>

Thanks, queued.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v8 05/11] power: act8945a_charger: Add status change update support

[Wenyou Yang]

Add the charger status change interrupt support, it will report
the power supply changed event.

This interrupt is generated by one of the conditions as below:
 - the state machine jumps out of or into the EOC state
 - the CHGIN input voltage goes out of or into the valid range.
 - the battery temperature goes out of or into the valid range.
 - the PRECHARGE time-out occurs.
 - the total charge time-out occurs.

Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
---

Changes in v8:
 - Achieve "interrupts" property from its own node directly.

Changes in v7:
 - Fix the wrong using devm_request_irq() with with parent
   device as *dev.

Changes in v6:
 - Add .remove callback function.
 - Fix the 'dev' argument of devm_request_irq() to pdev->dev.parent.

Changes in v5: None
Changes in v4:
 - Use "interrupts" property, instead of "active-semi,irq-gpios"
   for irq.

Changes in v3: None
Changes in v2: None

 drivers/power/supply/act8945a_charger.c | 95 ++++++++++++++++++++++++++++++---
 1 file changed, 88 insertions(+), 7 deletions(-)

diff --git a/drivers/power/supply/act8945a_charger.c b/drivers/power/supply/act8945a_charger.c
index a6bbaff..e129255 100644
--- a/drivers/power/supply/act8945a_charger.c
+++ b/drivers/power/supply/act8945a_charger.c
@@ -10,9 +10,11 @@
  * published by the Free Software Foundation.
  *
  */
+#include <linux/interrupt.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_gpio.h>
+#include <linux/of_irq.h>
 #include <linux/platform_device.h>
 #include <linux/power_supply.h>
 #include <linux/regmap.h>
@@ -75,7 +77,11 @@ static const char *act8945a_charger_manufacturer = "Active-semi";
 #define APCH_STATE_CSTATE_PRE		0x03
 
 struct act8945a_charger {
+	struct power_supply *psy;
 	struct regmap *regmap;
+	struct work_struct work;
+
+	bool init_done;
 };
 
 static int act8945a_get_charger_state(struct regmap *regmap, int *val)
@@ -252,6 +258,47 @@ static const struct power_supply_desc act8945a_charger_desc = {
 	.num_properties	= ARRAY_SIZE(act8945a_charger_props),
 };
 
+static int act8945a_enable_interrupt(struct act8945a_charger *charger)
+{
+	struct regmap *regmap = charger->regmap;
+	unsigned char ctrl;
+	int ret;
+
+	ctrl = APCH_CTRL_CHGEOCOUT | APCH_CTRL_CHGEOCIN |
+	       APCH_CTRL_INDIS | APCH_CTRL_INCON |
+	       APCH_CTRL_TEMPOUT | APCH_CTRL_TEMPIN |
+	       APCH_CTRL_TIMRPRE | APCH_CTRL_TIMRTOT;
+	ret = regmap_write(regmap, ACT8945A_APCH_CTRL, ctrl);
+	if (ret)
+		return ret;
+
+	ctrl = APCH_STATUS_CHGSTAT | APCH_STATUS_INSTAT |
+	       APCH_STATUS_TEMPSTAT | APCH_STATUS_TIMRSTAT;
+	ret = regmap_write(regmap, ACT8945A_APCH_STATUS, ctrl);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static void act8945a_work(struct work_struct *work)
+{
+	struct act8945a_charger *charger =
+			container_of(work, struct act8945a_charger, work);
+
+	power_supply_changed(charger->psy);
+}
+
+static irqreturn_t act8945a_status_changed(int irq, void *dev_id)
+{
+	struct act8945a_charger *charger = dev_id;
+
+	if (charger->init_done)
+		schedule_work(&charger->work);
+
+	return IRQ_HANDLED;
+}
+
 #define DEFAULT_TOTAL_TIME_OUT		3
 #define DEFAULT_PRE_TIME_OUT		40
 #define DEFAULT_INPUT_OVP_THRESHOLD	6600
@@ -362,9 +409,8 @@ static int act8945a_charger_config(struct device *dev,
 static int act8945a_charger_probe(struct platform_device *pdev)
 {
 	struct act8945a_charger *charger;
-	struct power_supply *psy;
 	struct power_supply_config psy_cfg = {};
-	int ret;
+	int irq, ret;
 
 	charger = devm_kzalloc(&pdev->dev, sizeof(*charger), GFP_KERNEL);
 	if (!charger)
@@ -380,17 +426,51 @@ static int act8945a_charger_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	irq = of_irq_get(pdev->dev.of_node, 0);
+	if (irq == -EPROBE_DEFER) {
+		dev_err(&pdev->dev, "failed to find IRQ number\n");
+		return -EPROBE_DEFER;
+	}
+
+	ret = devm_request_irq(&pdev->dev, irq, act8945a_status_changed,
+			       IRQF_TRIGGER_FALLING, "act8945a_interrupt",
+			       charger);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to request nIRQ pin IRQ\n");
+		return ret;
+	}
+
 	psy_cfg.of_node	= pdev->dev.of_node;
 	psy_cfg.drv_data = charger;
 
-	psy = devm_power_supply_register(&pdev->dev,
-					 &act8945a_charger_desc,
-					 &psy_cfg);
-	if (IS_ERR(psy)) {
+	charger->psy = devm_power_supply_register(&pdev->dev,
+						  &act8945a_charger_desc,
+						  &psy_cfg);
+	if (IS_ERR(charger->psy)) {
 		dev_err(&pdev->dev, "failed to register power supply\n");
-		return PTR_ERR(psy);
+		return PTR_ERR(charger->psy);
 	}
 
+	platform_set_drvdata(pdev, charger);
+
+	INIT_WORK(&charger->work, act8945a_work);
+
+	ret = act8945a_enable_interrupt(charger);
+	if (ret)
+		return -EIO;
+
+	charger->init_done = true;
+
+	return 0;
+}
+
+static int act8945a_charger_remove(struct platform_device *pdev)
+{
+	struct act8945a_charger *charger = platform_get_drvdata(pdev);
+
+	charger->init_done = false;
+	cancel_work_sync(&charger->work);
+
 	return 0;
 }
 
@@ -399,6 +479,7 @@ static struct platform_driver act8945a_charger_driver = {
 		.name = "act8945a-charger",
 	},
 	.probe	= act8945a_charger_probe,
+	.remove = act8945a_charger_remove,
 };
 module_platform_driver(act8945a_charger_driver);
 
-- 
2.7.4

Re: [PATCH v8 05/11] power: act8945a_charger: Add status change update support

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 604 bytes --]

Hi,

On Thu, Aug 25, 2016 at 03:19:53PM +0800, Wenyou Yang wrote:
> Add the charger status change interrupt support, it will report
> the power supply changed event.
> 
> This interrupt is generated by one of the conditions as below:
>  - the state machine jumps out of or into the EOC state
>  - the CHGIN input voltage goes out of or into the valid range.
>  - the battery temperature goes out of or into the valid range.
>  - the PRECHARGE time-out occurs.
>  - the total charge time-out occurs.
> 
> Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>

Thanks, queued.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v8 06/11] power: act8945a_charger: Fix the power supply type

[Wenyou Yang]

The power supply type property is varying as the external power
supply changes. It is not a constant.

Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
---

Changes in v8: None
Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

 drivers/power/supply/act8945a_charger.c | 48 ++++++++++++++++++++++++++-------
 1 file changed, 39 insertions(+), 9 deletions(-)

diff --git a/drivers/power/supply/act8945a_charger.c b/drivers/power/supply/act8945a_charger.c
index e129255..48775f8 100644
--- a/drivers/power/supply/act8945a_charger.c
+++ b/drivers/power/supply/act8945a_charger.c
@@ -78,6 +78,7 @@ static const char *act8945a_charger_manufacturer = "Active-semi";
 
 struct act8945a_charger {
 	struct power_supply *psy;
+	struct power_supply_desc desc;
 	struct regmap *regmap;
 	struct work_struct work;
 
@@ -250,14 +251,6 @@ static int act8945a_charger_get_property(struct power_supply *psy,
 	return ret;
 }
 
-static const struct power_supply_desc act8945a_charger_desc = {
-	.name		= "act8945a-charger",
-	.type		= POWER_SUPPLY_TYPE_BATTERY,
-	.get_property	= act8945a_charger_get_property,
-	.properties	= act8945a_charger_props,
-	.num_properties	= ARRAY_SIZE(act8945a_charger_props),
-};
-
 static int act8945a_enable_interrupt(struct act8945a_charger *charger)
 {
 	struct regmap *regmap = charger->regmap;
@@ -281,11 +274,39 @@ static int act8945a_enable_interrupt(struct act8945a_charger *charger)
 	return 0;
 }
 
+static unsigned int act8945a_set_supply_type(struct act8945a_charger *charger,
+					     unsigned int *type)
+{
+	unsigned int status, state;
+	int ret;
+
+	ret = regmap_read(charger->regmap, ACT8945A_APCH_STATUS, &status);
+	if (ret < 0)
+		return ret;
+
+	ret = regmap_read(charger->regmap, ACT8945A_APCH_STATE, &state);
+	if (ret < 0)
+		return ret;
+
+	if (status & APCH_STATUS_INDAT) {
+		if (state & APCH_STATE_ACINSTAT)
+			*type = POWER_SUPPLY_TYPE_MAINS;
+		else
+			*type = POWER_SUPPLY_TYPE_USB;
+	} else {
+		*type = POWER_SUPPLY_TYPE_BATTERY;
+	}
+
+	return 0;
+}
+
 static void act8945a_work(struct work_struct *work)
 {
 	struct act8945a_charger *charger =
 			container_of(work, struct act8945a_charger, work);
 
+	act8945a_set_supply_type(charger, &charger->desc.type);
+
 	power_supply_changed(charger->psy);
 }
 
@@ -440,11 +461,20 @@ static int act8945a_charger_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	charger->desc.name = "act8945a-charger";
+	charger->desc.get_property = act8945a_charger_get_property;
+	charger->desc.properties = act8945a_charger_props;
+	charger->desc.num_properties = ARRAY_SIZE(act8945a_charger_props);
+
+	ret = act8945a_set_supply_type(charger, &charger->desc.type);
+	if (ret)
+		return -EINVAL;
+
 	psy_cfg.of_node	= pdev->dev.of_node;
 	psy_cfg.drv_data = charger;
 
 	charger->psy = devm_power_supply_register(&pdev->dev,
-						  &act8945a_charger_desc,
+						  &charger->desc,
 						  &psy_cfg);
 	if (IS_ERR(charger->psy)) {
 		dev_err(&pdev->dev, "failed to register power supply\n");
-- 
2.7.4

Re: [PATCH v8 06/11] power: act8945a_charger: Fix the power supply type

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 270 bytes --]

Hi,

On Thu, Aug 25, 2016 at 03:19:54PM +0800, Wenyou Yang wrote:
> The power supply type property is varying as the external power
> supply changes. It is not a constant.
> 
> Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>

Thanks, queued.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v8 07/11] power: act8945a_charger: Add capacity level property

[Wenyou Yang]

Add the power supply capacity level property, it corresponds to
POWER_SUPPLY_CAPACITY_LEVEL_*.

It also utilizes the precision voltage detector function module
to catch the low battery voltage.

Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
---

Changes in v8:
 - As the act8945a_charger is regarded as a sub-device, all
   properties can be achieved from its own device node,
   use devm_gpiod_get() properly as well to get "lbo-gpios".
 - Add missing return -EPROBE_DEFER  for "lbo-gpios".

Changes in v7:
 - For "lbo-gpios", use gpiod_get() to fix devm_gpiod_get() wrong use
   with parent device as *dev argument.
 - Add the handle -EPROBE_DEFER returned from gpiod_get "lbo-gpios".
 - Use dev_info() to print log if the lbo irq request failed.
 - Remove unneeded semicolon.
 - Add depends on GPIOLIB for use gpiod.

Changes in v6:
 - For "lbo-gpios", use gpiod API instead of old gpio API to handle.

Changes in v5: None
Changes in v4:
 - Change devname of devm_request_irq() from "lbo-detect" to
   "act8945a, lbo-detect".

Changes in v3: None
Changes in v2: None

 drivers/power/supply/Kconfig            |  2 +-
 drivers/power/supply/act8945a_charger.c | 78 +++++++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+), 1 deletion(-)

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index 76806a0..a983de2 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -77,7 +77,7 @@ config BATTERY_88PM860X
 
 config BATTERY_ACT8945A
 	tristate "Active-semi ACT8945A charger driver"
-	depends on MFD_ACT8945A || COMPILE_TEST
+	depends on MFD_ACT8945A || GPIOLIB || COMPILE_TEST
 	help
 	  Say Y here to enable support for power supply provided by
 	  Active-semi ActivePath ACT8945A charger.
diff --git a/drivers/power/supply/act8945a_charger.c b/drivers/power/supply/act8945a_charger.c
index 48775f8..b66b023 100644
--- a/drivers/power/supply/act8945a_charger.c
+++ b/drivers/power/supply/act8945a_charger.c
@@ -18,6 +18,7 @@
 #include <linux/platform_device.h>
 #include <linux/power_supply.h>
 #include <linux/regmap.h>
+#include <linux/gpio/consumer.h>
 
 static const char *act8945a_charger_model = "ACT8945A";
 static const char *act8945a_charger_manufacturer = "Active-semi";
@@ -83,6 +84,7 @@ struct act8945a_charger {
 	struct work_struct work;
 
 	bool init_done;
+	struct gpio_desc *lbo_gpio;
 };
 
 static int act8945a_get_charger_state(struct regmap *regmap, int *val)
@@ -208,11 +210,67 @@ static int act8945a_get_battery_health(struct regmap *regmap, int *val)
 	return 0;
 }
 
+static int act8945a_get_capacity_level(struct act8945a_charger *charger,
+				       struct regmap *regmap, int *val)
+{
+	int ret;
+	unsigned int status, state, config;
+	int lbo_level = gpiod_get_value(charger->lbo_gpio);
+
+	ret = regmap_read(regmap, ACT8945A_APCH_STATUS, &status);
+	if (ret < 0)
+		return ret;
+
+	ret = regmap_read(regmap, ACT8945A_APCH_CFG, &config);
+	if (ret < 0)
+		return ret;
+
+	ret = regmap_read(regmap, ACT8945A_APCH_STATE, &state);
+	if (ret < 0)
+		return ret;
+
+	state &= APCH_STATE_CSTATE;
+	state >>= APCH_STATE_CSTATE_SHIFT;
+
+	switch (state) {
+	case APCH_STATE_CSTATE_PRE:
+		*val = POWER_SUPPLY_CAPACITY_LEVEL_LOW;
+		break;
+	case APCH_STATE_CSTATE_FAST:
+		if (lbo_level)
+			*val = POWER_SUPPLY_CAPACITY_LEVEL_HIGH;
+		else
+			*val = POWER_SUPPLY_CAPACITY_LEVEL_LOW;
+		break;
+	case APCH_STATE_CSTATE_EOC:
+		if (status & APCH_STATUS_CHGDAT)
+			*val = POWER_SUPPLY_CAPACITY_LEVEL_FULL;
+		else
+			*val = POWER_SUPPLY_CAPACITY_LEVEL_NORMAL;
+		break;
+	case APCH_STATE_CSTATE_DISABLED:
+	default:
+		if (config & APCH_CFG_SUSCHG) {
+			*val = POWER_SUPPLY_CAPACITY_LEVEL_UNKNOWN;
+		} else {
+			*val = POWER_SUPPLY_CAPACITY_LEVEL_NORMAL;
+			if (!(status & APCH_STATUS_INDAT)) {
+				if (!lbo_level)
+					*val = POWER_SUPPLY_CAPACITY_LEVEL_CRITICAL;
+			}
+		}
+		break;
+	}
+
+	return 0;
+}
+
 static enum power_supply_property act8945a_charger_props[] = {
 	POWER_SUPPLY_PROP_STATUS,
 	POWER_SUPPLY_PROP_CHARGE_TYPE,
 	POWER_SUPPLY_PROP_TECHNOLOGY,
 	POWER_SUPPLY_PROP_HEALTH,
+	POWER_SUPPLY_PROP_CAPACITY_LEVEL,
 	POWER_SUPPLY_PROP_MODEL_NAME,
 	POWER_SUPPLY_PROP_MANUFACTURER
 };
@@ -238,6 +296,10 @@ static int act8945a_charger_get_property(struct power_supply *psy,
 	case POWER_SUPPLY_PROP_HEALTH:
 		ret = act8945a_get_battery_health(regmap, &val->intval);
 		break;
+	case POWER_SUPPLY_PROP_CAPACITY_LEVEL:
+		ret = act8945a_get_capacity_level(charger,
+						  regmap, &val->intval);
+		break;
 	case POWER_SUPPLY_PROP_MODEL_NAME:
 		val->strval = act8945a_charger_model;
 		break;
@@ -354,6 +416,22 @@ static int act8945a_charger_config(struct device *dev,
 		dev_info(dev, "have been suspended\n");
 	}
 
+	charger->lbo_gpio = devm_gpiod_get(dev, "active-semi,lbo", GPIOD_IN);
+	if (PTR_ERR(charger->lbo_gpio) == -EPROBE_DEFER) {
+		dev_info(dev, "probe retry requested for gpio \"lbo\"\n");
+		return -EPROBE_DEFER;
+	} else if (IS_ERR(charger->lbo_gpio)) {
+		dev_err(dev, "unable to claim gpio \"lbo\"\n");
+		charger->lbo_gpio = NULL;
+	}
+
+	ret = devm_request_irq(dev, gpiod_to_irq(charger->lbo_gpio),
+			       act8945a_status_changed,
+			       (IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING),
+			       "act8945a_lbo_detect", charger);
+	if (ret)
+		dev_info(dev, "failed to request gpio \"lbo\" IRQ\n");
+
 	chglev_pin = of_get_named_gpio_flags(np,
 				"active-semi,chglev-gpios", 0, &flags);
 
-- 
2.7.4

Re: [PATCH v8 07/11] power: act8945a_charger: Add capacity level property

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1665 bytes --]

Hi,

On Thu, Aug 25, 2016 at 03:19:55PM +0800, Wenyou Yang wrote:
> Add the power supply capacity level property, it corresponds to
> POWER_SUPPLY_CAPACITY_LEVEL_*.
> 
> It also utilizes the precision voltage detector function module
> to catch the low battery voltage.

[...]

> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index 76806a0..a983de2 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -77,7 +77,7 @@ config BATTERY_88PM860X
>  
>  config BATTERY_ACT8945A
>  	tristate "Active-semi ACT8945A charger driver"
> -	depends on MFD_ACT8945A || COMPILE_TEST
> +	depends on MFD_ACT8945A || GPIOLIB || COMPILE_TEST

This looks wrong. You probably wanted (MFD_ACT8945A && GPIOLIB).
But since the gpio consumer headers defines stubs for the !GPIOLIB
case just drop the dependency.

>  	help
>  	  Say Y here to enable support for power supply provided by
>  	  Active-semi ActivePath ACT8945A charger.

[...]

> +	charger->lbo_gpio = devm_gpiod_get(dev, "active-semi,lbo", GPIOD_IN);
> +	if (PTR_ERR(charger->lbo_gpio) == -EPROBE_DEFER) {
> +		dev_info(dev, "probe retry requested for gpio \"lbo\"\n");
> +		return -EPROBE_DEFER;
> +	} else if (IS_ERR(charger->lbo_gpio)) {
> +		dev_err(dev, "unable to claim gpio \"lbo\"\n");
> +		charger->lbo_gpio = NULL;
> +	}

Please use devm_gpiod_get_optional():

charger->lbo_gpio = devm_gpiod_get_optional(dev, "active-semi,lbo", GPIOD_IN);
if (IS_ERR(charger->lbo_gpio)) {
    err = PTR_ERR(charger->lbo_gpio)
    dev_err(dev, "unable to claim gpio \"lbo\": %ld\n", err);
    return err;
}

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v8 08/11] power: act8945a_charger: Add max current property

[Wenyou Yang]

Add the power supply's current max property,
POWER_SUPPLY_PROP_CURRENT_MAX.

Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
---

Changes in v8:
 - As the act8945a_charger is regarded as a sub-device, all
   properties can be achieved from its own device node,
   use devm_gpiod_get() properly as well to get "chglev-gpios".
 - Add missing return -EPROBE_DEFER  for "chglev-gpios".

Changes in v7:
 - For "chglev-gpios", use gpiod_get() to fix devm_gpiod_get() wrong use
   with parent device as *dev argument.
 - Add the handle -EPROBE_DEFER returned from gpiod_get "chglev-gpio".
 - Remove unneeded semicolon.

Changes in v6:
 - For "chglev-gpios", use gpiod API instead of old gpio API to handle.

Changes in v5: None
Changes in v4:
 - Fix wrong gpio assignment for chglev_pin.

Changes in v3: None
Changes in v2: None

 drivers/power/supply/act8945a_charger.c | 90 +++++++++++++++++++++++++++++----
 1 file changed, 81 insertions(+), 9 deletions(-)

diff --git a/drivers/power/supply/act8945a_charger.c b/drivers/power/supply/act8945a_charger.c
index b66b023..d6263e9 100644
--- a/drivers/power/supply/act8945a_charger.c
+++ b/drivers/power/supply/act8945a_charger.c
@@ -13,7 +13,6 @@
 #include <linux/interrupt.h>
 #include <linux/module.h>
 #include <linux/of.h>
-#include <linux/of_gpio.h>
 #include <linux/of_irq.h>
 #include <linux/platform_device.h>
 #include <linux/power_supply.h>
@@ -85,6 +84,7 @@ struct act8945a_charger {
 
 	bool init_done;
 	struct gpio_desc *lbo_gpio;
+	struct gpio_desc *chglev_gpio;
 };
 
 static int act8945a_get_charger_state(struct regmap *regmap, int *val)
@@ -265,12 +265,80 @@ static int act8945a_get_capacity_level(struct act8945a_charger *charger,
 	return 0;
 }
 
+#define MAX_CURRENT_USB_HIGH	450000
+#define MAX_CURRENT_USB_LOW	90000
+#define MAX_CURRENT_USB_PRE	45000
+/*
+ * Riset(K) = 2336 * (1V/Ichg(mA)) - 0.205
+ * Riset = 2.43K
+ */
+#define MAX_CURRENT_AC_HIGH		886527
+#define MAX_CURRENT_AC_LOW		117305
+#define MAX_CURRENT_AC_HIGH_PRE		88653
+#define MAX_CURRENT_AC_LOW_PRE		11731
+
+static int act8945a_get_current_max(struct act8945a_charger *charger,
+				    struct regmap *regmap, int *val)
+{
+	int ret;
+	unsigned int status, state;
+	unsigned int acin_state;
+	int chgin_level = gpiod_get_value(charger->chglev_gpio);
+
+	ret = regmap_read(regmap, ACT8945A_APCH_STATUS, &status);
+	if (ret < 0)
+		return ret;
+
+	ret = regmap_read(regmap, ACT8945A_APCH_STATE, &state);
+	if (ret < 0)
+		return ret;
+
+	acin_state = (state & APCH_STATE_ACINSTAT) >> 1;
+
+	state &= APCH_STATE_CSTATE;
+	state >>= APCH_STATE_CSTATE_SHIFT;
+
+	switch (state) {
+	case APCH_STATE_CSTATE_PRE:
+		if (acin_state) {
+			if (chgin_level)
+				*val = MAX_CURRENT_AC_HIGH_PRE;
+			else
+				*val = MAX_CURRENT_AC_LOW_PRE;
+		} else {
+			*val = MAX_CURRENT_USB_PRE;
+		}
+		break;
+	case APCH_STATE_CSTATE_FAST:
+		if (acin_state) {
+			if (chgin_level)
+				*val = MAX_CURRENT_AC_HIGH;
+			else
+				*val = MAX_CURRENT_AC_LOW;
+		} else {
+			if (chgin_level)
+				*val = MAX_CURRENT_USB_HIGH;
+			else
+				*val = MAX_CURRENT_USB_LOW;
+		}
+		break;
+	case APCH_STATE_CSTATE_EOC:
+	case APCH_STATE_CSTATE_DISABLED:
+	default:
+		*val = 0;
+		break;
+	}
+
+	return 0;
+}
+
 static enum power_supply_property act8945a_charger_props[] = {
 	POWER_SUPPLY_PROP_STATUS,
 	POWER_SUPPLY_PROP_CHARGE_TYPE,
 	POWER_SUPPLY_PROP_TECHNOLOGY,
 	POWER_SUPPLY_PROP_HEALTH,
 	POWER_SUPPLY_PROP_CAPACITY_LEVEL,
+	POWER_SUPPLY_PROP_CURRENT_MAX,
 	POWER_SUPPLY_PROP_MODEL_NAME,
 	POWER_SUPPLY_PROP_MANUFACTURER
 };
@@ -300,6 +368,10 @@ static int act8945a_charger_get_property(struct power_supply *psy,
 		ret = act8945a_get_capacity_level(charger,
 						  regmap, &val->intval);
 		break;
+	case POWER_SUPPLY_PROP_CURRENT_MAX:
+		ret = act8945a_get_current_max(charger,
+					       regmap, &val->intval);
+		break;
 	case POWER_SUPPLY_PROP_MODEL_NAME:
 		val->strval = act8945a_charger_model;
 		break;
@@ -390,13 +462,11 @@ static int act8945a_charger_config(struct device *dev,
 				   struct act8945a_charger *charger)
 {
 	struct device_node *np = dev->of_node;
-	enum of_gpio_flags flags;
 	struct regmap *regmap = charger->regmap;
 
 	u32 total_time_out;
 	u32 pre_time_out;
 	u32 input_voltage_threshold;
-	int chglev_pin;
 	int ret;
 
 	unsigned int tmp;
@@ -432,12 +502,14 @@ static int act8945a_charger_config(struct device *dev,
 	if (ret)
 		dev_info(dev, "failed to request gpio \"lbo\" IRQ\n");
 
-	chglev_pin = of_get_named_gpio_flags(np,
-				"active-semi,chglev-gpios", 0, &flags);
-
-	if (gpio_is_valid(chglev_pin)) {
-		gpio_set_value(chglev_pin,
-			       ((flags == OF_GPIO_ACTIVE_LOW) ? 0 : 1));
+	charger->chglev_gpio = devm_gpiod_get(dev,
+					      "active-semi,chglev", GPIOD_IN);
+	if (PTR_ERR(charger->chglev_gpio) == -EPROBE_DEFER) {
+		dev_info(dev, "probe retry requested for gpio \"chglev\"\n");
+		return -EPROBE_DEFER;
+	} else if (IS_ERR(charger->chglev_gpio)) {
+		dev_err(dev, "unable to claim gpio \"chglev\"\n");
+		charger->chglev_gpio = NULL;
 	}
 
 	if (of_property_read_u32(np,
-- 
2.7.4

Re: [PATCH v8 08/11] power: act8945a_charger: Add max current property

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 869 bytes --]

Hi,

On Thu, Aug 25, 2016 at 03:19:56PM +0800, Wenyou Yang wrote:
> Add the power supply's current max property,
> POWER_SUPPLY_PROP_CURRENT_MAX.

[...]
  
> -	chglev_pin = of_get_named_gpio_flags(np,
> -				"active-semi,chglev-gpios", 0, &flags);
> -
> -	if (gpio_is_valid(chglev_pin)) {
> -		gpio_set_value(chglev_pin,
> -			       ((flags == OF_GPIO_ACTIVE_LOW) ? 0 : 1));
> +	charger->chglev_gpio = devm_gpiod_get(dev,
> +					      "active-semi,chglev", GPIOD_IN);
> +	if (PTR_ERR(charger->chglev_gpio) == -EPROBE_DEFER) {
> +		dev_info(dev, "probe retry requested for gpio \"chglev\"\n");
> +		return -EPROBE_DEFER;
> +	} else if (IS_ERR(charger->chglev_gpio)) {
> +		dev_err(dev, "unable to claim gpio \"chglev\"\n");
> +		charger->chglev_gpio = NULL;

Please also use devm_gpiod_get_optional() here (see previous patch).

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v8 09/11] doc: bindings: mfd: act8945a: Update the example

[Wenyou Yang]

Since the act8945a-charger is regarded as a sub-device and it using
"interrupts" property, update the examples section.

Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
---

Changes in v8: None
Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

 Documentation/devicetree/bindings/mfd/act8945a.txt | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/act8945a.txt b/Documentation/devicetree/bindings/mfd/act8945a.txt
index f712830..462819a 100644
--- a/Documentation/devicetree/bindings/mfd/act8945a.txt
+++ b/Documentation/devicetree/bindings/mfd/act8945a.txt
@@ -14,13 +14,6 @@ Example:
 		reg = <0x5b>;
 		status = "okay";
 
-		pinctrl-names = "default";
-		pinctrl-0 = <&pinctrl_charger_chglev>;
-		active-semi,chglev-gpio = <&pioA 12 GPIO_ACTIVE_HIGH>;
-		active-semi,input-voltage-threshold-microvolt = <6600>;
-		active-semi,precondition-timeout = <40>;
-		active-semi,total-timeout = <3>;
-
 		active-semi,vsel-high;
 
 		regulators {
@@ -73,4 +66,19 @@ Example:
 				regulator-always-on;
 			};
 		};
+
+		charger {
+			compatible = "active-semi,act8945a-charger";
+			pinctrl-names = "default";
+			pinctrl-0 = <&pinctrl_charger_chglev &pinctrl_charger_lbo &pinctrl_charger_irq>;
+			interrupt-parent = <&pioA>;
+			interrupts = <45 GPIO_ACTIVE_LOW>;
+
+			active-semi,chglev-gpios = <&pioA 12 GPIO_ACTIVE_HIGH>;
+			active-semi,lbo-gpios = <&pioA 72 GPIO_ACTIVE_LOW>;
+			active-semi,input-voltage-threshold-microvolt = <6600>;
+			active-semi,precondition-timeout = <40>;
+			active-semi,total-timeout = <3>;
+			status = "okay";
+		};
 	};
-- 
2.7.4

Re: [PATCH v8 09/11] doc: bindings: mfd: act8945a: Update the example

[Rob Herring]

On Thu, Aug 25, 2016 at 03:38:35PM +0800, Wenyou Yang wrote:
> Since the act8945a-charger is regarded as a sub-device and it using
> "interrupts" property, update the examples section.
> 
> Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
> ---
> 
> Changes in v8: None
> Changes in v7: None
> Changes in v6: None
> Changes in v5: None
> Changes in v4: None
> Changes in v3: None
> Changes in v2: None

Humm, appears to be a new patch to me.

> 
>  Documentation/devicetree/bindings/mfd/act8945a.txt | 22 +++++++++++++++-------
>  1 file changed, 15 insertions(+), 7 deletions(-)

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH v8 09/11] doc: bindings: mfd: act8945a: Update the example

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 256 bytes --]

Hi Lee,

On Thu, Aug 25, 2016 at 03:38:35PM +0800, Wenyou Yang wrote:
> Since the act8945a-charger is regarded as a sub-device and it using
> "interrupts" property, update the examples section.

I assume, that this will go through your tree.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v8 10/11] doc: bindings: power: act8945a-charger: Update properties.

[Wenyou Yang]

Due the driver improvements, update the properties,
 - Remove "active-semi,check-battery-temperature" property.
 - Add the properties, "active-semi,irq_gpio"
   and "active-semi,lbo-gpios".
 - As act8945a-charger is regarded as a sub-device, update
   the compatible and examples.

Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
Acked-by: Rob Herring <robh@kernel.org>
---

Changes in v8:
 - Since the act8945a-charger is regarded as a sub-device, update
   the compatible decription and examples.

Changes in v7: None
Changes in v6: None
Changes in v5:
 - Add Acked-by tag.

Changes in v4:
 - Update the doc/binding for using "interrupts" property.

Changes in v3: None
Changes in v2: None

 .../bindings/power/supply/act8945a-charger.txt     | 31 +++++++++++++++-------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/power/supply/act8945a-charger.txt b/Documentation/devicetree/bindings/power/supply/act8945a-charger.txt
index bea254c..de78d76 100644
--- a/Documentation/devicetree/bindings/power/supply/act8945a-charger.txt
+++ b/Documentation/devicetree/bindings/power/supply/act8945a-charger.txt
@@ -1,13 +1,18 @@
 Device-Tree bindings for charger of Active-semi ACT8945A Multi-Function Device
 
 Required properties:
- - compatible: "active-semi,act8945a", please refer to ../mfd/act8945a.txt.
+ - compatible: "active-semi,act8945a-charger".
  - active-semi,chglev-gpios: charge current level phandle with args
    as described in ../gpio/gpio.txt.
+ - active-semi,lbo-gpios: specify the low battery voltage detect phandle
+   with args as as described in ../gpio/gpio.txt.
+ - interrupts: <a b> where a is the interrupt number and b is a
+   field that represents an encoding of the sense and level
+   information for the interrupt.
+ - interrupt-parent: the phandle for the interrupt controller that
+   services interrupts for this device.
 
 Optional properties:
- - active-semi,check-battery-temperature: boolean to check the battery
-   temperature or not.
  - active-semi,input-voltage-threshold-microvolt: unit: mV;
    Specifies the charger's input over-voltage threshold value;
    The value can be: 6600, 7000, 7500, 8000; default: 6600
@@ -26,10 +31,18 @@ Example:
 		reg = <0x5b>;
 		status = "okay";
 
-		pinctrl-names = "default";
-		pinctrl-0 = <&pinctrl_charger_chglev>;
-		active-semi,chglev-gpios = <&pioA 12 GPIO_ACTIVE_HIGH>;
-		active-semi,input-voltage-threshold-microvolt = <6600>;
-		active-semi,precondition-timeout = <40>;
-		active-semi,total-timeout = <3>;
+		charger {
+			compatible = "active-semi,act8945a-charger";
+			pinctrl-names = "default";
+			pinctrl-0 = <&pinctrl_charger_chglev &pinctrl_charger_lbo &pinctrl_charger_irq>;
+			interrupt-parent = <&pioA>;
+			interrupts = <45 GPIO_ACTIVE_LOW>;
+
+			active-semi,chglev-gpios = <&pioA 12 GPIO_ACTIVE_HIGH>;
+			active-semi,lbo-gpios = <&pioA 72 GPIO_ACTIVE_LOW>;
+			active-semi,input-voltage-threshold-microvolt = <6600>;
+			active-semi,precondition-timeout = <40>;
+			active-semi,total-timeout = <3>;
+			status = "okay";
+		};
 	};
-- 
2.7.4

Re: [PATCH v8 10/11] doc: bindings: power: act8945a-charger: Update properties.

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 504 bytes --]

Hi,

On Thu, Aug 25, 2016 at 03:40:26PM +0800, Wenyou Yang wrote:
> Due the driver improvements, update the properties,
>  - Remove "active-semi,check-battery-temperature" property.
>  - Add the properties, "active-semi,irq_gpio"
>    and "active-semi,lbo-gpios".
>  - As act8945a-charger is regarded as a sub-device, update
>    the compatible and examples.
> 
> Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
> Acked-by: Rob Herring <robh@kernel.org>

Thanks, queued.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v8 11/11] ARM: at91/dt: sama5d2_xplained: Add act8945a-charger node.

[Wenyou Yang]

Add act8945a-charger as a sub-device node.
Use the "interrupts" property, instead of the "active-semi,irq_gpios"
to denote the act8945a chager's irq.

Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
---

Changes in v8:
 - Add 4 new patches: 1/11, 2/11, 9/11, and 11/11.

Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

 arch/arm/boot/dts/at91-sama5d2_xplained.dts | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/arch/arm/boot/dts/at91-sama5d2_xplained.dts b/arch/arm/boot/dts/at91-sama5d2_xplained.dts
index 0b9a59d..87eff10 100644
--- a/arch/arm/boot/dts/at91-sama5d2_xplained.dts
+++ b/arch/arm/boot/dts/at91-sama5d2_xplained.dts
@@ -162,14 +162,6 @@
 					compatible = "active-semi,act8945a";
 					reg = <0x5b>;
 					active-semi,vsel-high;
-					active-semi,chglev-gpios = <&pioA 12 GPIO_ACTIVE_HIGH>;
-					active-semi,lbo-gpios = <&pioA 72 GPIO_ACTIVE_LOW>;
-					active-semi,irq_gpios = <&pioA 45 GPIO_ACTIVE_LOW>;
-					active-semi,input-voltage-threshold-microvolt = <6600>;
-					active-semi,precondition-timeout = <40>;
-					active-semi,total-timeout = <3>;
-					pinctrl-names = "default";
-					pinctrl-0 = <&pinctrl_charger_chglev &pinctrl_charger_lbo &pinctrl_charger_irq>;
 					status = "okay";
 
 					regulators {
@@ -222,6 +214,21 @@
 							regulator-always-on;
 						};
 					};
+
+					charger {
+						compatible = "active-semi,act8945a-charger";
+						pinctrl-names = "default";
+						pinctrl-0 = <&pinctrl_charger_chglev &pinctrl_charger_lbo &pinctrl_charger_irq>;
+						interrupt-parent = <&pioA>;
+						interrupts = <45 GPIO_ACTIVE_LOW>;
+
+						active-semi,chglev-gpios = <&pioA 12 GPIO_ACTIVE_HIGH>;
+						active-semi,lbo-gpios = <&pioA 72 GPIO_ACTIVE_LOW>;
+						active-semi,input-voltage-threshold-microvolt = <6600>;
+						active-semi,precondition-timeout = <40>;
+						active-semi,total-timeout = <3>;
+						status = "okay";
+					};
 				};
 			};
 
-- 
2.7.4

Re: [PATCH v8 00/11] power: act8945a_charger: Improvements

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 628 bytes --]

On Thu, Aug 25, 2016 at 03:19:48PM +0800, Wenyou Yang wrote:
> This patch series is to improve the implementation of act8945a-charger
> driver, such as regard the act8945-charger as a sub-device, improve
> the way to check the status, fix the power supply type property,
> add the status change update, and add more properties: capacity level
> property and max current property.

I don't know why you're copying me on this series but can you please
either stop or let me know what you're looking for me to do with it?
It's quite large, is getting resent regularly with frequent replies and
I'm never quite sure why it's there.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]