linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v7 0/7] power: act8945a_charger: Improvements
@ 2016-08-22  6:49 Wenyou Yang
       [not found] ` <1471848557-27278-1-git-send-email-wenyou.yang-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Wenyou Yang @ 2016-08-22  6:49 UTC (permalink / raw)
  To: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
	Rob Herring, Pawel Moll, Mark Brown, Ian Campbell, Kumar Gala
  Cc: linux-kernel, devicetree, linux-arm-kernel, Nicolas Ferre,
	linux-pm, Wenyou Yang

This patch series is to improve the implementation of act8945a-charger
driver, such as 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 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 (7):
  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: act8945a-charger: Update properties

 .../bindings/power/supply/act8945a-charger.txt     |  16 +-
 drivers/power/supply/Kconfig                       |   2 +-
 drivers/power/supply/act8945a_charger.c            | 418 ++++++++++++++++++---
 3 files changed, 386 insertions(+), 50 deletions(-)

-- 
2.7.4


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v7 1/7] power: act8945a_charger: Remove "battery_temperature"
       [not found] ` <1471848557-27278-1-git-send-email-wenyou.yang-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
@ 2016-08-22  6:49   ` Wenyou Yang
  2016-08-22  6:49   ` [PATCH v7 3/7] power: act8945a_charger: Add status change update support Wenyou Yang
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: Wenyou Yang @ 2016-08-22  6:49 UTC (permalink / raw)
  To: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
	Rob Herring, Pawel Moll, Mark Brown, Ian Campbell, Kumar Gala
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Nicolas Ferre,
	linux-pm-u79uwXL29TY76Z2rM5mHXA, 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-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
---

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 b5c00e4..9894577 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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v7 2/7] power: act8945a_charger: Improve
  2016-08-22  6:49 [PATCH v7 0/7] power: act8945a_charger: Improvements Wenyou Yang
       [not found] ` <1471848557-27278-1-git-send-email-wenyou.yang-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
@ 2016-08-22  6:49 ` Wenyou Yang
  2016-08-22  6:49 ` [PATCH v7 6/7] power: act8945a_charger: Add max current property Wenyou Yang
  2016-08-22  6:49 ` [PATCH v7 7/7] doc: bindings: act8945a-charger: Update properties Wenyou Yang
  3 siblings, 0 replies; 11+ messages in thread
From: Wenyou Yang @ 2016-08-22  6:49 UTC (permalink / raw)
  To: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
	Rob Herring, Pawel Moll, Mark Brown, Ian Campbell, Kumar Gala
  Cc: linux-kernel, devicetree, linux-arm-kernel, Nicolas Ferre,
	linux-pm, Wenyou Yang, Fengguang Wu

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 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 9894577..cee8f75 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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v7 3/7] power: act8945a_charger: Add status change update support
       [not found] ` <1471848557-27278-1-git-send-email-wenyou.yang-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
  2016-08-22  6:49   ` [PATCH v7 1/7] power: act8945a_charger: Remove "battery_temperature" Wenyou Yang
@ 2016-08-22  6:49   ` Wenyou Yang
  2016-08-22  6:49   ` [PATCH v7 4/7] power: act8945a_charger: Fix the power supply type Wenyou Yang
  2016-08-22  6:49   ` [PATCH v7 5/7] power: act8945a_charger: Add capacity level property Wenyou Yang
  3 siblings, 0 replies; 11+ messages in thread
From: Wenyou Yang @ 2016-08-22  6:49 UTC (permalink / raw)
  To: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
	Rob Herring, Pawel Moll, Mark Brown, Ian Campbell, Kumar Gala
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Nicolas Ferre,
	linux-pm-u79uwXL29TY76Z2rM5mHXA, 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-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
---

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 cee8f75..fd99739 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.parent->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.parent->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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v7 4/7] power: act8945a_charger: Fix the power supply type
       [not found] ` <1471848557-27278-1-git-send-email-wenyou.yang-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
  2016-08-22  6:49   ` [PATCH v7 1/7] power: act8945a_charger: Remove "battery_temperature" Wenyou Yang
  2016-08-22  6:49   ` [PATCH v7 3/7] power: act8945a_charger: Add status change update support Wenyou Yang
@ 2016-08-22  6:49   ` Wenyou Yang
  2016-08-22  6:49   ` [PATCH v7 5/7] power: act8945a_charger: Add capacity level property Wenyou Yang
  3 siblings, 0 replies; 11+ messages in thread
From: Wenyou Yang @ 2016-08-22  6:49 UTC (permalink / raw)
  To: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
	Rob Herring, Pawel Moll, Mark Brown, Ian Campbell, Kumar Gala
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Nicolas Ferre,
	linux-pm-u79uwXL29TY76Z2rM5mHXA, 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-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
---

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 fd99739..3536b67 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.parent->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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v7 5/7] power: act8945a_charger: Add capacity level property
       [not found] ` <1471848557-27278-1-git-send-email-wenyou.yang-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
                     ` (2 preceding siblings ...)
  2016-08-22  6:49   ` [PATCH v7 4/7] power: act8945a_charger: Fix the power supply type Wenyou Yang
@ 2016-08-22  6:49   ` Wenyou Yang
  2016-08-23  2:40     ` Sebastian Reichel
  3 siblings, 1 reply; 11+ messages in thread
From: Wenyou Yang @ 2016-08-22  6:49 UTC (permalink / raw)
  To: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
	Rob Herring, Pawel Moll, Mark Brown, Ian Campbell, Kumar Gala
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Nicolas Ferre,
	linux-pm-u79uwXL29TY76Z2rM5mHXA, 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-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
---

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 | 110 +++++++++++++++++++++++++++++---
 2 files changed, 102 insertions(+), 10 deletions(-)

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 3536b67..cc79abe 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;
@@ -327,7 +389,7 @@ static irqreturn_t act8945a_status_changed(int irq, void *dev_id)
 static int act8945a_charger_config(struct device *dev,
 				   struct act8945a_charger *charger)
 {
-	struct device_node *np = dev->of_node;
+	struct device_node *np = dev->parent->of_node;
 	enum of_gpio_flags flags;
 	struct regmap *regmap = charger->regmap;
 
@@ -354,6 +416,21 @@ static int act8945a_charger_config(struct device *dev,
 		dev_info(dev, "have been suspended\n");
 	}
 
+	charger->lbo_gpio = gpiod_get(dev->parent, "active-semi,lbo", GPIOD_IN);
+	if (PTR_ERR(charger->lbo_gpio) == -EPROBE_DEFER) {
+		dev_info(dev, "probe retry requested for gpio \"lbo\"\n");
+	} 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);
 
@@ -443,14 +520,15 @@ 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;
 
 	irq = of_irq_get(pdev->dev.parent->of_node, 0);
 	if (irq == -EPROBE_DEFER) {
 		dev_err(&pdev->dev, "failed to find IRQ number\n");
-		return -EPROBE_DEFER;
+		ret = -EPROBE_DEFER;
+		goto fail;
 	}
 
 	ret = devm_request_irq(&pdev->dev, irq, act8945a_status_changed,
@@ -458,7 +536,7 @@ static int act8945a_charger_probe(struct platform_device *pdev)
 			       charger);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to request nIRQ pin IRQ\n");
-		return ret;
+		goto fail;
 	}
 
 	charger->desc.name = "act8945a-charger";
@@ -467,8 +545,10 @@ static int act8945a_charger_probe(struct platform_device *pdev)
 	charger->desc.num_properties = ARRAY_SIZE(act8945a_charger_props);
 
 	ret = act8945a_set_supply_type(charger, &charger->desc.type);
-	if (ret)
-		return -EINVAL;
+	if (ret) {
+		ret = -EINVAL;
+		goto fail;
+	}
 
 	psy_cfg.of_node	= pdev->dev.parent->of_node;
 	psy_cfg.drv_data = charger;
@@ -478,7 +558,8 @@ static int act8945a_charger_probe(struct platform_device *pdev)
 						  &psy_cfg);
 	if (IS_ERR(charger->psy)) {
 		dev_err(&pdev->dev, "failed to register power supply\n");
-		return PTR_ERR(charger->psy);
+		ret = PTR_ERR(charger->psy);
+		goto fail;
 	}
 
 	platform_set_drvdata(pdev, charger);
@@ -486,12 +567,20 @@ static int act8945a_charger_probe(struct platform_device *pdev)
 	INIT_WORK(&charger->work, act8945a_work);
 
 	ret = act8945a_enable_interrupt(charger);
-	if (ret)
-		return -EIO;
+	if (ret) {
+		ret = -EIO;
+		goto fail;
+	}
 
 	charger->init_done = true;
 
 	return 0;
+
+fail:
+	if (charger->lbo_gpio)
+		gpiod_put(charger->lbo_gpio);
+
+	return ret;
 }
 
 static int act8945a_charger_remove(struct platform_device *pdev)
@@ -501,6 +590,9 @@ static int act8945a_charger_remove(struct platform_device *pdev)
 	charger->init_done = false;
 	cancel_work_sync(&charger->work);
 
+	if (charger->lbo_gpio)
+		gpiod_put(charger->lbo_gpio);
+
 	return 0;
 }
 
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v7 6/7] power: act8945a_charger: Add max current property
  2016-08-22  6:49 [PATCH v7 0/7] power: act8945a_charger: Improvements Wenyou Yang
       [not found] ` <1471848557-27278-1-git-send-email-wenyou.yang-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
  2016-08-22  6:49 ` [PATCH v7 2/7] power: act8945a_charger: Improve Wenyou Yang
@ 2016-08-22  6:49 ` Wenyou Yang
       [not found]   ` <1471848557-27278-7-git-send-email-wenyou.yang-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
  2016-08-22  6:49 ` [PATCH v7 7/7] doc: bindings: act8945a-charger: Update properties Wenyou Yang
  3 siblings, 1 reply; 11+ messages in thread
From: Wenyou Yang @ 2016-08-22  6:49 UTC (permalink / raw)
  To: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
	Rob Herring, Pawel Moll, Mark Brown, Ian Campbell, Kumar Gala
  Cc: linux-kernel, devicetree, linux-arm-kernel, Nicolas Ferre,
	linux-pm, 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 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 | 93 +++++++++++++++++++++++++++++----
 1 file changed, 84 insertions(+), 9 deletions(-)

diff --git a/drivers/power/supply/act8945a_charger.c b/drivers/power/supply/act8945a_charger.c
index cc79abe..a40526c 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->parent->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;
@@ -431,12 +501,13 @@ 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 = gpiod_get(dev->parent,
+					 "active-semi,chglev", GPIOD_IN);
+	if (PTR_ERR(charger->chglev_gpio) == -EPROBE_DEFER) {
+		dev_info(dev, "probe retry requested for gpio \"chglev\"\n");
+	} 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,
@@ -579,6 +650,8 @@ static int act8945a_charger_probe(struct platform_device *pdev)
 fail:
 	if (charger->lbo_gpio)
 		gpiod_put(charger->lbo_gpio);
+	if (charger->chglev_gpio)
+		gpiod_put(charger->chglev_gpio);
 
 	return ret;
 }
@@ -592,6 +665,8 @@ static int act8945a_charger_remove(struct platform_device *pdev)
 
 	if (charger->lbo_gpio)
 		gpiod_put(charger->lbo_gpio);
+	if (charger->chglev_gpio)
+		gpiod_put(charger->chglev_gpio);
 
 	return 0;
 }
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v7 7/7] doc: bindings: act8945a-charger: Update properties
  2016-08-22  6:49 [PATCH v7 0/7] power: act8945a_charger: Improvements Wenyou Yang
                   ` (2 preceding siblings ...)
  2016-08-22  6:49 ` [PATCH v7 6/7] power: act8945a_charger: Add max current property Wenyou Yang
@ 2016-08-22  6:49 ` Wenyou Yang
  2016-08-23  3:03   ` Sebastian Reichel
  3 siblings, 1 reply; 11+ messages in thread
From: Wenyou Yang @ 2016-08-22  6:49 UTC (permalink / raw)
  To: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
	Rob Herring, Pawel Moll, Mark Brown, Ian Campbell, Kumar Gala
  Cc: linux-kernel, devicetree, linux-arm-kernel, Nicolas Ferre,
	linux-pm, 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".

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

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           | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/power/supply/act8945a-charger.txt b/Documentation/devicetree/bindings/power/supply/act8945a-charger.txt
index bea254c..f9866c4 100644
--- a/Documentation/devicetree/bindings/power/supply/act8945a-charger.txt
+++ b/Documentation/devicetree/bindings/power/supply/act8945a-charger.txt
@@ -4,10 +4,15 @@ Required properties:
  - compatible: "active-semi,act8945a", please refer to ../mfd/act8945a.txt.
  - 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
@@ -27,9 +32,14 @@ Example:
 		status = "okay";
 
 		pinctrl-names = "default";
-		pinctrl-0 = <&pinctrl_charger_chglev>;
+		pinctrl-0 = <&pinctrl_charger_chglev &pinctrl_charger_lbo &pinctrl_charger_irq>;
+
 		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>;
+		interrupt-parent = <&pioA>;
+		interrupts = <45 GPIO_ACTIVE_LOW>;
+
 	};
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v7 5/7] power: act8945a_charger: Add capacity level property
  2016-08-22  6:49   ` [PATCH v7 5/7] power: act8945a_charger: Add capacity level property Wenyou Yang
@ 2016-08-23  2:40     ` Sebastian Reichel
  0 siblings, 0 replies; 11+ messages in thread
From: Sebastian Reichel @ 2016-08-23  2:40 UTC (permalink / raw)
  To: Wenyou Yang
  Cc: Dmitry Eremin-Solenikov, David Woodhouse, Rob Herring,
	Pawel Moll, Mark Brown, Ian Campbell, Kumar Gala, linux-kernel,
	devicetree, linux-arm-kernel, Nicolas Ferre, linux-pm

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

Hi,

On Mon, Aug 22, 2016 at 02:49:15PM +0800, Wenyou Yang wrote:
> [...]
>
> +	charger->lbo_gpio = gpiod_get(dev->parent, "active-semi,lbo", GPIOD_IN);
> +	if (PTR_ERR(charger->lbo_gpio) == -EPROBE_DEFER) {
> +		dev_info(dev, "probe retry requested for gpio \"lbo\"\n");
> +	} 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");

that does not work, since you need to release the irq before
releasing the gpio. So if the gpio is not requested via devm_
than you can't request the irq via devm_ (actually you can by
using devm_free_irq in remove(), but then you can also drop
the devm_ alltogether).

> [...]

-- Sebastian

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v7 6/7] power: act8945a_charger: Add max current property
       [not found]   ` <1471848557-27278-7-git-send-email-wenyou.yang-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
@ 2016-08-23  2:43     ` Sebastian Reichel
  0 siblings, 0 replies; 11+ messages in thread
From: Sebastian Reichel @ 2016-08-23  2:43 UTC (permalink / raw)
  To: Wenyou Yang
  Cc: Dmitry Eremin-Solenikov, David Woodhouse, Rob Herring,
	Pawel Moll, Mark Brown, Ian Campbell, Kumar Gala,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Nicolas Ferre,
	linux-pm-u79uwXL29TY76Z2rM5mHXA

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

Hi,

On Mon, Aug 22, 2016 at 02:49:16PM +0800, Wenyou Yang wrote:
> [...]
>
> -	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 = gpiod_get(dev->parent,
> +					 "active-semi,chglev", GPIOD_IN);
> +	if (PTR_ERR(charger->chglev_gpio) == -EPROBE_DEFER) {
> +		dev_info(dev, "probe retry requested for gpio \"chglev\"\n");

missing return? You need to return EPROBE_DEFER in probe().

> +	} else if (IS_ERR(charger->chglev_gpio)) {
> +		dev_err(dev, "unable to claim gpio \"chglev\"\n");
> +		charger->chglev_gpio = NULL;
>  	}
>  
> [...]

-- Sebastian

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v7 7/7] doc: bindings: act8945a-charger: Update properties
  2016-08-22  6:49 ` [PATCH v7 7/7] doc: bindings: act8945a-charger: Update properties Wenyou Yang
@ 2016-08-23  3:03   ` Sebastian Reichel
  0 siblings, 0 replies; 11+ messages in thread
From: Sebastian Reichel @ 2016-08-23  3:03 UTC (permalink / raw)
  To: Wenyou Yang, Rob Herring
  Cc: Pawel Moll, Mark Brown, Ian Campbell, Kumar Gala, linux-kernel,
	devicetree, linux-arm-kernel, Nicolas Ferre, linux-pm

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

Hi,

On Mon, Aug 22, 2016 at 02:49:17PM +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".
> 
> Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
> Acked-by: Rob Herring <robh@kernel.org>

I wonder if it's too late to slightly break the DT API by
introducing a proper sub-device for the charger and having all those
charger specific details described there. Not being able to use the
devm_ functions because all properties are assigned to the parent
device is not very nice.

We could easily support reading timeouts and threshold information
from the both (parent and sub-device) and just mark the parent
binding deprecated. The only real API break would be
"active-semi,chglev-gpios", which is currently broken anyways in the
driver as far as I can see (I should have reviewed this more
carefully previously).

In other words I suggest the following:

pmic@5b {
    compatible = "active-semi,act8945a";
    reg = <0x5b>;
    status = "okay";

    /* the following 3 are deprecated, but still supported */
    //active-semi,input-voltage-threshold-microvolt = <6600>;
    //active-semi,precondition-timeout = <40>;
    //active-semi,total-timeout = <3>;

    /* no longer supported, never worked properly */
    //active-semi,chglev-gpios = <&pioA 12 GPIO_ACTIVE_HIGH>;

    /* this is new, mfd should register charger sub-dev independently
     * of the subnode for backwards compatibility. Then optional
     * gpios/irqs are not available. But those either never worked
     * or did not exist before this update. */
    charger {
        active-semi,chglev-gpios = <&pioA 12 GPIO_ACTIVE_HIGH>;
        active-semi,lbo-gpios = <&pioA 72 GPIO_ACTIVE_LOW>;
		interrupt-parent = <&pioA>;
        interrupts = <45 GPIO_ACTIVE_LOW>

        active-semi,input-voltage-threshold-microvolt = <6600>;
        active-semi,precondition-timeout = <40>;
        active-semi,total-timeout = <3>;
    };
};

-- Sebastian

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2016-08-23  3:03 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-22  6:49 [PATCH v7 0/7] power: act8945a_charger: Improvements Wenyou Yang
     [not found] ` <1471848557-27278-1-git-send-email-wenyou.yang-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
2016-08-22  6:49   ` [PATCH v7 1/7] power: act8945a_charger: Remove "battery_temperature" Wenyou Yang
2016-08-22  6:49   ` [PATCH v7 3/7] power: act8945a_charger: Add status change update support Wenyou Yang
2016-08-22  6:49   ` [PATCH v7 4/7] power: act8945a_charger: Fix the power supply type Wenyou Yang
2016-08-22  6:49   ` [PATCH v7 5/7] power: act8945a_charger: Add capacity level property Wenyou Yang
2016-08-23  2:40     ` Sebastian Reichel
2016-08-22  6:49 ` [PATCH v7 2/7] power: act8945a_charger: Improve Wenyou Yang
2016-08-22  6:49 ` [PATCH v7 6/7] power: act8945a_charger: Add max current property Wenyou Yang
     [not found]   ` <1471848557-27278-7-git-send-email-wenyou.yang-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
2016-08-23  2:43     ` Sebastian Reichel
2016-08-22  6:49 ` [PATCH v7 7/7] doc: bindings: act8945a-charger: Update properties Wenyou Yang
2016-08-23  3:03   ` Sebastian Reichel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).