linux-omap.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 01/15] power: supply: cpcap-battery: Fix battery full status reporting
@ 2020-03-15 15:11 Arthur Demchenkov
  2020-03-15 15:11 ` [PATCH 02/15] power: supply: cpcap-battery: Improve battery full status detection Arthur Demchenkov
                   ` (14 more replies)
  0 siblings, 15 replies; 27+ messages in thread
From: Arthur Demchenkov @ 2020-03-15 15:11 UTC (permalink / raw)
  To: Tony Lindgren, Merlijn Wajer, Pavel Machek, sre, linux-pm, linux-omap
  Cc: Arthur Demchenkov

Don't report that the battery is fully charged if the charging current
exceeds 100 mA.

Signed-off-by: Arthur Demchenkov <spinal.by@gmail.com>
---
 drivers/power/supply/cpcap-battery.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c
index d7b3234ec264..34a9dbcd1a23 100644
--- a/drivers/power/supply/cpcap-battery.c
+++ b/drivers/power/supply/cpcap-battery.c
@@ -408,7 +408,8 @@ static bool cpcap_battery_full(struct cpcap_battery_ddata *ddata)
 	struct cpcap_battery_state_data *state = cpcap_battery_latest(ddata);
 
 	if (state->voltage >=
-	    (ddata->config.bat.constant_charge_voltage_max_uv - 18000))
+	    (ddata->config.bat.constant_charge_voltage_max_uv - 18000) &&
+		state->current_ua > -100000)
 		return true;
 
 	return false;
-- 
2.11.0


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

* [PATCH 02/15] power: supply: cpcap-battery: Improve battery full status detection
  2020-03-15 15:11 [PATCH 01/15] power: supply: cpcap-battery: Fix battery full status reporting Arthur Demchenkov
@ 2020-03-15 15:11 ` Arthur Demchenkov
  2020-03-15 16:46   ` Tony Lindgren
  2020-03-15 15:11 ` [PATCH 03/15] power: supply: cpcap-battery: Fix battery low status reporting Arthur Demchenkov
                   ` (13 subsequent siblings)
  14 siblings, 1 reply; 27+ messages in thread
From: Arthur Demchenkov @ 2020-03-15 15:11 UTC (permalink / raw)
  To: Tony Lindgren, Merlijn Wajer, Pavel Machek, sre, linux-pm, linux-omap
  Cc: Arthur Demchenkov

If the battery status is detected as full for the charging current that
doesn't exceed 100 mA, it will then be reported as full for charging
currents in the range of 100-150 mA. This is needed because
charge_current value has a spread.

We don't use avg_current here because it can trigger wrong battery full
status on charger connected event.

Signed-off-by: Arthur Demchenkov <spinal.by@gmail.com>
---
 drivers/power/supply/cpcap-battery.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c
index 34a9dbcd1a23..52f03a2662a5 100644
--- a/drivers/power/supply/cpcap-battery.c
+++ b/drivers/power/supply/cpcap-battery.c
@@ -406,13 +406,16 @@ static int cpcap_battery_cc_get_avg_current(struct cpcap_battery_ddata *ddata)
 static bool cpcap_battery_full(struct cpcap_battery_ddata *ddata)
 {
 	struct cpcap_battery_state_data *state = cpcap_battery_latest(ddata);
+	static bool is_full;
 
 	if (state->voltage >=
 	    (ddata->config.bat.constant_charge_voltage_max_uv - 18000) &&
-		state->current_ua > -100000)
-		return true;
+	    state->current_ua > (is_full ? -150000 : -100000))
+		is_full = true;
+	else
+		is_full = false;
 
-	return false;
+	return is_full;
 }
 
 static bool cpcap_battery_low(struct cpcap_battery_ddata *ddata)
-- 
2.11.0


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

* [PATCH 03/15] power: supply: cpcap-battery: Fix battery low status reporting
  2020-03-15 15:11 [PATCH 01/15] power: supply: cpcap-battery: Fix battery full status reporting Arthur Demchenkov
  2020-03-15 15:11 ` [PATCH 02/15] power: supply: cpcap-battery: Improve battery full status detection Arthur Demchenkov
@ 2020-03-15 15:11 ` Arthur Demchenkov
  2020-03-21 14:47   ` Tony Lindgren
  2020-03-15 15:11 ` [PATCH 04/15] power: supply: cpcap-battery: Add charge_full property Arthur Demchenkov
                   ` (12 subsequent siblings)
  14 siblings, 1 reply; 27+ messages in thread
From: Arthur Demchenkov @ 2020-03-15 15:11 UTC (permalink / raw)
  To: Tony Lindgren, Merlijn Wajer, Pavel Machek, sre, linux-pm, linux-omap
  Cc: Arthur Demchenkov

If we hit battery low once, we should stick on reporting it until the
charger is connected. This way low->counter_uah will be updated
properly, and that will allow us to get more accurate charge_full value.

Signed-off-by: Arthur Demchenkov <spinal.by@gmail.com>
---
 drivers/power/supply/cpcap-battery.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c
index 52f03a2662a5..8a58ad943960 100644
--- a/drivers/power/supply/cpcap-battery.c
+++ b/drivers/power/supply/cpcap-battery.c
@@ -421,11 +421,14 @@ static bool cpcap_battery_full(struct cpcap_battery_ddata *ddata)
 static bool cpcap_battery_low(struct cpcap_battery_ddata *ddata)
 {
 	struct cpcap_battery_state_data *state = cpcap_battery_latest(ddata);
+	static bool is_low;
 
-	if (state->current_ua && state->voltage <= 3300000)
-		return true;
+	if (state->current_ua > 0 && (state->voltage <= 3300000 || is_low))
+		is_low = true;
+	else
+		is_low = false;
 
-	return false;
+	return is_low;
 }
 
 static int cpcap_battery_update_status(struct cpcap_battery_ddata *ddata)
-- 
2.11.0


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

* [PATCH 04/15] power: supply: cpcap-battery: Add charge_full property
  2020-03-15 15:11 [PATCH 01/15] power: supply: cpcap-battery: Fix battery full status reporting Arthur Demchenkov
  2020-03-15 15:11 ` [PATCH 02/15] power: supply: cpcap-battery: Improve battery full status detection Arthur Demchenkov
  2020-03-15 15:11 ` [PATCH 03/15] power: supply: cpcap-battery: Fix battery low status reporting Arthur Demchenkov
@ 2020-03-15 15:11 ` Arthur Demchenkov
  2020-03-15 15:11 ` [PATCH 05/15] power: supply: cpcap-battery: Add charge_now property Arthur Demchenkov
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 27+ messages in thread
From: Arthur Demchenkov @ 2020-03-15 15:11 UTC (permalink / raw)
  To: Tony Lindgren, Merlijn Wajer, Pavel Machek, sre, linux-pm, linux-omap
  Cc: Arthur Demchenkov

Add charge_full property.

Signed-off-by: Arthur Demchenkov <spinal.by@gmail.com>
---
 drivers/power/supply/cpcap-battery.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c
index 8a58ad943960..43e39485a617 100644
--- a/drivers/power/supply/cpcap-battery.c
+++ b/drivers/power/supply/cpcap-battery.c
@@ -134,6 +134,7 @@ struct cpcap_battery_ddata {
 	struct cpcap_battery_state_data state[CPCAP_BATTERY_STATE_NR];
 	u32 cc_lsb;		/* μAms per LSB */
 	atomic_t active;
+	int charge_full;
 	int status;
 	u16 vendor;
 };
@@ -530,6 +531,7 @@ static enum power_supply_property cpcap_battery_props[] = {
 	POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
 	POWER_SUPPLY_PROP_CURRENT_AVG,
 	POWER_SUPPLY_PROP_CURRENT_NOW,
+	POWER_SUPPLY_PROP_CHARGE_FULL,
 	POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
 	POWER_SUPPLY_PROP_CHARGE_COUNTER,
 	POWER_SUPPLY_PROP_POWER_NOW,
@@ -649,6 +651,11 @@ static int cpcap_battery_get_property(struct power_supply *psy,
 	case POWER_SUPPLY_PROP_CAPACITY_LEVEL:
 		val->intval = cpcap_battery_get_rough_capacity(ddata);
 		break;
+	case POWER_SUPPLY_PROP_CHARGE_FULL:
+		if (!ddata->charge_full)
+			return -ENODATA;
+		val->intval = ddata->charge_full;
+		break;
 	case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN:
 		val->intval = ddata->config.info.charge_full_design;
 		break;
@@ -710,6 +717,15 @@ static int cpcap_battery_set_property(struct power_supply *psy,
 		ddata->config.bat.constant_charge_voltage_max_uv = val->intval;
 
 		return cpcap_battery_update_charger(ddata, val->intval);
+	case POWER_SUPPLY_PROP_CHARGE_FULL:
+		if (val->intval < 0)
+			return -EINVAL;
+		if (val->intval > ddata->config.info.charge_full_design)
+			return -EINVAL;
+
+		ddata->charge_full = val->intval;
+
+		return 0;
 	default:
 		return -EINVAL;
 	}
@@ -722,6 +738,7 @@ static int cpcap_battery_property_is_writeable(struct power_supply *psy,
 {
 	switch (psp) {
 	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
+	case POWER_SUPPLY_PROP_CHARGE_FULL:
 		return 1;
 	default:
 		return 0;
-- 
2.11.0


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

* [PATCH 05/15] power: supply: cpcap-battery: Add charge_now property
  2020-03-15 15:11 [PATCH 01/15] power: supply: cpcap-battery: Fix battery full status reporting Arthur Demchenkov
                   ` (2 preceding siblings ...)
  2020-03-15 15:11 ` [PATCH 04/15] power: supply: cpcap-battery: Add charge_full property Arthur Demchenkov
@ 2020-03-15 15:11 ` Arthur Demchenkov
  2020-03-15 15:11 ` [PATCH 06/15] power: supply: cpcap-battery: Initialize with user provided data Arthur Demchenkov
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 27+ messages in thread
From: Arthur Demchenkov @ 2020-03-15 15:11 UTC (permalink / raw)
  To: Tony Lindgren, Merlijn Wajer, Pavel Machek, sre, linux-pm, linux-omap
  Cc: Arthur Demchenkov

Add charge_now property.

Signed-off-by: Arthur Demchenkov <spinal.by@gmail.com>
---
 drivers/power/supply/cpcap-battery.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c
index 43e39485a617..db1b519e87c6 100644
--- a/drivers/power/supply/cpcap-battery.c
+++ b/drivers/power/supply/cpcap-battery.c
@@ -532,6 +532,7 @@ static enum power_supply_property cpcap_battery_props[] = {
 	POWER_SUPPLY_PROP_CURRENT_AVG,
 	POWER_SUPPLY_PROP_CURRENT_NOW,
 	POWER_SUPPLY_PROP_CHARGE_FULL,
+	POWER_SUPPLY_PROP_CHARGE_NOW,
 	POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
 	POWER_SUPPLY_PROP_CHARGE_COUNTER,
 	POWER_SUPPLY_PROP_POWER_NOW,
@@ -651,6 +652,12 @@ static int cpcap_battery_get_property(struct power_supply *psy,
 	case POWER_SUPPLY_PROP_CAPACITY_LEVEL:
 		val->intval = cpcap_battery_get_rough_capacity(ddata);
 		break;
+	case POWER_SUPPLY_PROP_CHARGE_NOW:
+		low = cpcap_battery_get_lowest(ddata);
+		if (!low->voltage)
+			return -ENODATA;
+		val->intval = low->counter_uah - latest->counter_uah;
+		break;
 	case POWER_SUPPLY_PROP_CHARGE_FULL:
 		if (!ddata->charge_full)
 			return -ENODATA;
-- 
2.11.0


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

* [PATCH 06/15] power: supply: cpcap-battery: Initialize with user provided data
  2020-03-15 15:11 [PATCH 01/15] power: supply: cpcap-battery: Fix battery full status reporting Arthur Demchenkov
                   ` (3 preceding siblings ...)
  2020-03-15 15:11 ` [PATCH 05/15] power: supply: cpcap-battery: Add charge_now property Arthur Demchenkov
@ 2020-03-15 15:11 ` Arthur Demchenkov
  2020-03-21 14:54   ` Tony Lindgren
  2020-03-15 15:11 ` [PATCH 07/15] power: supply: cpcap-battery: Rewrite capacity reporting Arthur Demchenkov
                   ` (9 subsequent siblings)
  14 siblings, 1 reply; 27+ messages in thread
From: Arthur Demchenkov @ 2020-03-15 15:11 UTC (permalink / raw)
  To: Tony Lindgren, Merlijn Wajer, Pavel Machek, sre, linux-pm, linux-omap
  Cc: Arthur Demchenkov

If the user provides us with charge_full value (which it could save in a
permanent storage between reboots), initialize low and high counter_uah
with calculated values.

Signed-off-by: Arthur Demchenkov <spinal.by@gmail.com>
---
 drivers/power/supply/cpcap-battery.c | 39 +++++++++++++++++++++++++++++-------
 1 file changed, 32 insertions(+), 7 deletions(-)

diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c
index db1b519e87c6..669ed1513201 100644
--- a/drivers/power/supply/cpcap-battery.c
+++ b/drivers/power/supply/cpcap-battery.c
@@ -434,7 +434,7 @@ static bool cpcap_battery_low(struct cpcap_battery_ddata *ddata)
 
 static int cpcap_battery_update_status(struct cpcap_battery_ddata *ddata)
 {
-	struct cpcap_battery_state_data state, *latest, *previous, *tmp;
+	struct cpcap_battery_state_data state, *latest, *previous, *low, *high;
 	ktime_t now;
 	int error;
 
@@ -464,15 +464,40 @@ static int cpcap_battery_update_status(struct cpcap_battery_ddata *ddata)
 	memcpy(latest, &state, sizeof(*latest));
 
 	if (cpcap_battery_full(ddata)) {
-		tmp = cpcap_battery_get_highest(ddata);
+		high = cpcap_battery_get_highest(ddata);
 		/* Update highest charge seen? */
-		if (latest->counter_uah <= tmp->counter_uah)
-			memcpy(tmp, latest, sizeof(*tmp));
+		if (latest->counter_uah <= high->counter_uah ||
+		    !high->voltage) {
+			memcpy(high, latest, sizeof(*high));
+
+			low = cpcap_battery_get_lowest(ddata);
+			if (low->voltage && low->voltage != -1)
+				ddata->charge_full =
+					low->counter_uah - high->counter_uah;
+			else if (ddata->charge_full) {
+				/* Initialize with user provided data */
+				low->counter_uah =
+					high->counter_uah + ddata->charge_full;
+				/* Mark it as initialized */
+				low->voltage = -1;
+			}
+		}
 	} else if (cpcap_battery_low(ddata)) {
-		tmp = cpcap_battery_get_lowest(ddata);
+		low = cpcap_battery_get_lowest(ddata);
 		/* Update lowest charge seen? */
-		if (latest->counter_uah >= tmp->counter_uah)
-			memcpy(tmp, latest, sizeof(*tmp));
+		if (latest->counter_uah >= low->counter_uah ||
+		    !low->voltage) {
+			memcpy(low, latest, sizeof(*low));
+
+			high = cpcap_battery_get_highest(ddata);
+			if (high->voltage)
+				ddata->charge_full =
+					low->counter_uah - high->counter_uah;
+			else if (ddata->charge_full)
+				/* Initialize with user provided data */
+				high->counter_uah =
+					low->counter_uah - ddata->charge_full;
+		}
 	}
 
 	return 0;
-- 
2.11.0


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

* [PATCH 07/15] power: supply: cpcap-battery: Rewrite capacity reporting
  2020-03-15 15:11 [PATCH 01/15] power: supply: cpcap-battery: Fix battery full status reporting Arthur Demchenkov
                   ` (4 preceding siblings ...)
  2020-03-15 15:11 ` [PATCH 06/15] power: supply: cpcap-battery: Initialize with user provided data Arthur Demchenkov
@ 2020-03-15 15:11 ` Arthur Demchenkov
  2020-03-15 15:11 ` [PATCH 08/15] power: supply: cpcap-battery: Get rid of rough capacity percentage Arthur Demchenkov
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 27+ messages in thread
From: Arthur Demchenkov @ 2020-03-15 15:11 UTC (permalink / raw)
  To: Tony Lindgren, Merlijn Wajer, Pavel Machek, sre, linux-pm, linux-omap
  Cc: Arthur Demchenkov

* Return -ENODATA instead of confusing rough values
* Calculate percentage using charge_full value provided

Signed-off-by: Arthur Demchenkov <spinal.by@gmail.com>
---
 drivers/power/supply/cpcap-battery.c | 27 +++++++--------------------
 1 file changed, 7 insertions(+), 20 deletions(-)

diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c
index 669ed1513201..f712a3bda315 100644
--- a/drivers/power/supply/cpcap-battery.c
+++ b/drivers/power/supply/cpcap-battery.c
@@ -573,10 +573,10 @@ static int cpcap_battery_get_property(struct power_supply *psy,
 				      union power_supply_propval *val)
 {
 	struct cpcap_battery_ddata *ddata = power_supply_get_drvdata(psy);
-	struct cpcap_battery_state_data *latest, *previous, *low, *high;
+	struct cpcap_battery_state_data *latest, *previous, *low;
 	u32 sample;
 	s32 accumulator;
-	int cached, delta, est;
+	int cached;
 	s64 tmp;
 
 	cached = cpcap_battery_update_status(ddata);
@@ -654,25 +654,12 @@ static int cpcap_battery_get_property(struct power_supply *psy,
 		val->intval = div64_s64(tmp, 100);
 		break;
 	case POWER_SUPPLY_PROP_CAPACITY:
-		est = cpcap_battery_get_rough_percentage(ddata);
-		high = cpcap_battery_get_highest(ddata);
-		if (high->voltage) {
-			delta = latest->counter_uah - high->counter_uah;
-			val->intval = (ddata->config.info.charge_full_design -
-				       delta) * 100;
-			val->intval /= ddata->config.info.charge_full_design;
-			delta = abs(val->intval - est);
-			break;
-		}
 		low = cpcap_battery_get_lowest(ddata);
-		if (low->voltage) {
-			delta = low->counter_uah - latest->counter_uah;
-			val->intval = (delta * 100) /
-				ddata->config.info.charge_full_design;
-			delta = abs(val->intval - est);
-			break;
-		}
-		val->intval = est;
+		if (!low->voltage || !ddata->charge_full)
+			return -ENODATA;
+		val->intval = (low->counter_uah -
+			       latest->counter_uah) * 100;
+		val->intval /= ddata->charge_full;
 		break;
 	case POWER_SUPPLY_PROP_CAPACITY_LEVEL:
 		val->intval = cpcap_battery_get_rough_capacity(ddata);
-- 
2.11.0


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

* [PATCH 08/15] power: supply: cpcap-battery: Get rid of rough capacity percentage
  2020-03-15 15:11 [PATCH 01/15] power: supply: cpcap-battery: Fix battery full status reporting Arthur Demchenkov
                   ` (5 preceding siblings ...)
  2020-03-15 15:11 ` [PATCH 07/15] power: supply: cpcap-battery: Rewrite capacity reporting Arthur Demchenkov
@ 2020-03-15 15:11 ` Arthur Demchenkov
  2020-03-15 15:12 ` [PATCH 09/15] power: supply: cpcap-battery: Increse low voltage bound Arthur Demchenkov
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 27+ messages in thread
From: Arthur Demchenkov @ 2020-03-15 15:11 UTC (permalink / raw)
  To: Tony Lindgren, Merlijn Wajer, Pavel Machek, sre, linux-pm, linux-omap
  Cc: Arthur Demchenkov

Get rid of rough capacity percentage.

Signed-off-by: Arthur Demchenkov <spinal.by@gmail.com>
---
 drivers/power/supply/cpcap-battery.c | 46 +++++++++---------------------------
 1 file changed, 11 insertions(+), 35 deletions(-)

diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c
index f712a3bda315..2f4c6669c37d 100644
--- a/drivers/power/supply/cpcap-battery.c
+++ b/drivers/power/supply/cpcap-battery.c
@@ -142,24 +142,22 @@ struct cpcap_battery_ddata {
 struct cpcap_battery_capacity {
 	int capacity;
 	int voltage;
-	int percentage;
 };
 
-#define CPCAP_CAP(l, v, p)			\
+#define CPCAP_CAP(l, v)				\
 {						\
 	.capacity = (l),			\
 	.voltage = (v),				\
-	.percentage = (p),			\
 },
 
 /* Pessimistic battery capacity mapping before high or low value is seen */
 static const struct cpcap_battery_capacity cpcap_battery_cap[] = {
-	CPCAP_CAP(POWER_SUPPLY_CAPACITY_LEVEL_UNKNOWN,        0,   0)
-	CPCAP_CAP(POWER_SUPPLY_CAPACITY_LEVEL_CRITICAL, 3100000,   0)
-	CPCAP_CAP(POWER_SUPPLY_CAPACITY_LEVEL_LOW,      3300000,   2)
-	CPCAP_CAP(POWER_SUPPLY_CAPACITY_LEVEL_NORMAL,   3700000,  50)
-	CPCAP_CAP(POWER_SUPPLY_CAPACITY_LEVEL_HIGH,     4000000,  75)
-	CPCAP_CAP(POWER_SUPPLY_CAPACITY_LEVEL_FULL,     4200000 - 18000, 100)
+	CPCAP_CAP(POWER_SUPPLY_CAPACITY_LEVEL_UNKNOWN,        0)
+	CPCAP_CAP(POWER_SUPPLY_CAPACITY_LEVEL_CRITICAL, 3100000)
+	CPCAP_CAP(POWER_SUPPLY_CAPACITY_LEVEL_LOW,      3300000)
+	CPCAP_CAP(POWER_SUPPLY_CAPACITY_LEVEL_NORMAL,   3700000)
+	CPCAP_CAP(POWER_SUPPLY_CAPACITY_LEVEL_HIGH,     4000000)
+	CPCAP_CAP(POWER_SUPPLY_CAPACITY_LEVEL_FULL,     4200000 - 18000)
 };
 
 #define CPCAP_NO_BATTERY	-400
@@ -503,8 +501,7 @@ static int cpcap_battery_update_status(struct cpcap_battery_ddata *ddata)
 	return 0;
 }
 
-static void cpcap_battery_get_rough(struct cpcap_battery_ddata *ddata,
-				    int *level, int *percentage)
+static int cpcap_battery_get_capacity_level(struct cpcap_battery_ddata *ddata)
 {
 	struct cpcap_battery_state_data *latest;
 	const struct cpcap_battery_capacity *cap = NULL;
@@ -520,30 +517,9 @@ static void cpcap_battery_get_rough(struct cpcap_battery_ddata *ddata,
 	}
 
 	if (!cap)
-		return;
-
-	if (level)
-		*level = cap->capacity;
-	if (percentage)
-		*percentage = cap->percentage;
-}
-
-static int cpcap_battery_get_rough_capacity(struct cpcap_battery_ddata *ddata)
-{
-	int capacity = 0;
-
-	cpcap_battery_get_rough(ddata, &capacity, NULL);
-
-	return capacity;
-}
-
-static int cpcap_battery_get_rough_percentage(struct cpcap_battery_ddata *ddata)
-{
-	int percentage = 0;
-
-	cpcap_battery_get_rough(ddata, NULL, &percentage);
+		return 0;
 
-	return percentage;
+	return cap->capacity;
 }
 
 static enum power_supply_property cpcap_battery_props[] = {
@@ -662,7 +638,7 @@ static int cpcap_battery_get_property(struct power_supply *psy,
 		val->intval /= ddata->charge_full;
 		break;
 	case POWER_SUPPLY_PROP_CAPACITY_LEVEL:
-		val->intval = cpcap_battery_get_rough_capacity(ddata);
+		val->intval = cpcap_battery_get_capacity_level(ddata);
 		break;
 	case POWER_SUPPLY_PROP_CHARGE_NOW:
 		low = cpcap_battery_get_lowest(ddata);
-- 
2.11.0


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

* [PATCH 09/15] power: supply: cpcap-battery: Increse low voltage bound
  2020-03-15 15:11 [PATCH 01/15] power: supply: cpcap-battery: Fix battery full status reporting Arthur Demchenkov
                   ` (6 preceding siblings ...)
  2020-03-15 15:11 ` [PATCH 08/15] power: supply: cpcap-battery: Get rid of rough capacity percentage Arthur Demchenkov
@ 2020-03-15 15:12 ` Arthur Demchenkov
  2020-03-15 15:12 ` [PATCH 10/15] power: supply: cpcap-battery: Improve full status reporting Arthur Demchenkov
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 27+ messages in thread
From: Arthur Demchenkov @ 2020-03-15 15:12 UTC (permalink / raw)
  To: Tony Lindgren, Merlijn Wajer, Pavel Machek, sre, linux-pm, linux-omap
  Cc: Arthur Demchenkov

Thus, the user will have more time to start charging when calibrating
the charge_full value.

Signed-off-by: Arthur Demchenkov <spinal.by@gmail.com>
---
 drivers/power/supply/cpcap-battery.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c
index 2f4c6669c37d..da6138df2117 100644
--- a/drivers/power/supply/cpcap-battery.c
+++ b/drivers/power/supply/cpcap-battery.c
@@ -422,7 +422,7 @@ static bool cpcap_battery_low(struct cpcap_battery_ddata *ddata)
 	struct cpcap_battery_state_data *state = cpcap_battery_latest(ddata);
 	static bool is_low;
 
-	if (state->current_ua > 0 && (state->voltage <= 3300000 || is_low))
+	if (state->current_ua > 0 && (state->voltage <= 3350000 || is_low))
 		is_low = true;
 	else
 		is_low = false;
-- 
2.11.0


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

* [PATCH 10/15] power: supply: cpcap-battery: Improve full status reporting
  2020-03-15 15:11 [PATCH 01/15] power: supply: cpcap-battery: Fix battery full status reporting Arthur Demchenkov
                   ` (7 preceding siblings ...)
  2020-03-15 15:12 ` [PATCH 09/15] power: supply: cpcap-battery: Increse low voltage bound Arthur Demchenkov
@ 2020-03-15 15:12 ` Arthur Demchenkov
  2020-03-15 15:12 ` [PATCH 11/15] power: supply: cpcap-battery: Rename properties, variables and functions Arthur Demchenkov
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 27+ messages in thread
From: Arthur Demchenkov @ 2020-03-15 15:12 UTC (permalink / raw)
  To: Tony Lindgren, Merlijn Wajer, Pavel Machek, sre, linux-pm, linux-omap
  Cc: Arthur Demchenkov

Use average current when detecting the fully charged state

Signed-off-by: Arthur Demchenkov <spinal.by@gmail.com>
---
 drivers/power/supply/cpcap-battery.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c
index da6138df2117..6205a5e43a32 100644
--- a/drivers/power/supply/cpcap-battery.c
+++ b/drivers/power/supply/cpcap-battery.c
@@ -407,12 +407,16 @@ static bool cpcap_battery_full(struct cpcap_battery_ddata *ddata)
 	struct cpcap_battery_state_data *state = cpcap_battery_latest(ddata);
 	static bool is_full;
 
-	if (state->voltage >=
-	    (ddata->config.bat.constant_charge_voltage_max_uv - 18000) &&
-	    state->current_ua > (is_full ? -150000 : -100000))
-		is_full = true;
-	else
+	if (state->voltage <
+	    (ddata->config.bat.constant_charge_voltage_max_uv - 18000)) {
 		is_full = false;
+	} else if (is_full) {
+		if (state->current_ua < -150000)
+			is_full = false;
+	} else if (state->current_ua >= -150000 &&
+		   cpcap_battery_cc_get_avg_current(ddata) >= -100000) {
+		is_full = true;
+	}
 
 	return is_full;
 }
-- 
2.11.0


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

* [PATCH 11/15] power: supply: cpcap-battery: Rename properties, variables and functions
  2020-03-15 15:11 [PATCH 01/15] power: supply: cpcap-battery: Fix battery full status reporting Arthur Demchenkov
                   ` (8 preceding siblings ...)
  2020-03-15 15:12 ` [PATCH 10/15] power: supply: cpcap-battery: Improve full status reporting Arthur Demchenkov
@ 2020-03-15 15:12 ` Arthur Demchenkov
  2020-03-15 15:12 ` [PATCH 12/15] power: supply: cpcap-battery: stabilize charge_full value Arthur Demchenkov
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 27+ messages in thread
From: Arthur Demchenkov @ 2020-03-15 15:12 UTC (permalink / raw)
  To: Tony Lindgren, Merlijn Wajer, Pavel Machek, sre, linux-pm, linux-omap
  Cc: Arthur Demchenkov

This is a preparation for the following change which will stabilize
battery full charge value when calibrating.

Signed-off-by: Arthur Demchenkov <spinal.by@gmail.com>
---
 drivers/power/supply/cpcap-battery.c | 75 ++++++++++++++++++------------------
 1 file changed, 38 insertions(+), 37 deletions(-)

diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c
index 6205a5e43a32..66ea1a718e02 100644
--- a/drivers/power/supply/cpcap-battery.c
+++ b/drivers/power/supply/cpcap-battery.c
@@ -110,8 +110,8 @@ struct cpcap_coulomb_counter_data {
 enum cpcap_battery_state {
 	CPCAP_BATTERY_STATE_PREVIOUS,
 	CPCAP_BATTERY_STATE_LATEST,
-	CPCAP_BATTERY_STATE_LOW,
-	CPCAP_BATTERY_STATE_HIGH,
+	CPCAP_BATTERY_STATE_EMPTY,
+	CPCAP_BATTERY_STATE_FULL,
 	CPCAP_BATTERY_STATE_NR,
 };
 
@@ -185,15 +185,15 @@ cpcap_battery_previous(struct cpcap_battery_ddata *ddata)
 }
 
 static struct cpcap_battery_state_data *
-cpcap_battery_get_lowest(struct cpcap_battery_ddata *ddata)
+cpcap_battery_get_empty(struct cpcap_battery_ddata *ddata)
 {
-	return cpcap_battery_get_state(ddata, CPCAP_BATTERY_STATE_LOW);
+	return cpcap_battery_get_state(ddata, CPCAP_BATTERY_STATE_EMPTY);
 }
 
 static struct cpcap_battery_state_data *
-cpcap_battery_get_highest(struct cpcap_battery_ddata *ddata)
+cpcap_battery_get_full(struct cpcap_battery_ddata *ddata)
 {
-	return cpcap_battery_get_state(ddata, CPCAP_BATTERY_STATE_HIGH);
+	return cpcap_battery_get_state(ddata, CPCAP_BATTERY_STATE_FULL);
 }
 
 static int cpcap_charger_battery_temperature(struct cpcap_battery_ddata *ddata,
@@ -436,7 +436,8 @@ static bool cpcap_battery_low(struct cpcap_battery_ddata *ddata)
 
 static int cpcap_battery_update_status(struct cpcap_battery_ddata *ddata)
 {
-	struct cpcap_battery_state_data state, *latest, *previous, *low, *high;
+	struct cpcap_battery_state_data state, *latest, *previous,
+					*empty, *full;
 	ktime_t now;
 	int error;
 
@@ -466,39 +467,39 @@ static int cpcap_battery_update_status(struct cpcap_battery_ddata *ddata)
 	memcpy(latest, &state, sizeof(*latest));
 
 	if (cpcap_battery_full(ddata)) {
-		high = cpcap_battery_get_highest(ddata);
-		/* Update highest charge seen? */
-		if (latest->counter_uah <= high->counter_uah ||
-		    !high->voltage) {
-			memcpy(high, latest, sizeof(*high));
-
-			low = cpcap_battery_get_lowest(ddata);
-			if (low->voltage && low->voltage != -1)
+		full = cpcap_battery_get_full(ddata);
+		/* Update full state value? */
+		if (latest->counter_uah <= full->counter_uah ||
+		    !full->voltage) {
+			memcpy(full, latest, sizeof(*full));
+
+			empty = cpcap_battery_get_empty(ddata);
+			if (empty->voltage && empty->voltage != -1)
 				ddata->charge_full =
-					low->counter_uah - high->counter_uah;
+					empty->counter_uah - full->counter_uah;
 			else if (ddata->charge_full) {
 				/* Initialize with user provided data */
-				low->counter_uah =
-					high->counter_uah + ddata->charge_full;
+				empty->counter_uah =
+					full->counter_uah + ddata->charge_full;
 				/* Mark it as initialized */
-				low->voltage = -1;
+				empty->voltage = -1;
 			}
 		}
 	} else if (cpcap_battery_low(ddata)) {
-		low = cpcap_battery_get_lowest(ddata);
-		/* Update lowest charge seen? */
-		if (latest->counter_uah >= low->counter_uah ||
-		    !low->voltage) {
-			memcpy(low, latest, sizeof(*low));
-
-			high = cpcap_battery_get_highest(ddata);
-			if (high->voltage)
+		empty = cpcap_battery_get_empty(ddata);
+		/* Update empty state value? */
+		if (latest->counter_uah >= empty->counter_uah ||
+		    !empty->voltage) {
+			memcpy(empty, latest, sizeof(*empty));
+
+			full = cpcap_battery_get_full(ddata);
+			if (full->voltage)
 				ddata->charge_full =
-					low->counter_uah - high->counter_uah;
+					empty->counter_uah - full->counter_uah;
 			else if (ddata->charge_full)
 				/* Initialize with user provided data */
-				high->counter_uah =
-					low->counter_uah - ddata->charge_full;
+				full->counter_uah =
+					empty->counter_uah - ddata->charge_full;
 		}
 	}
 
@@ -553,7 +554,7 @@ static int cpcap_battery_get_property(struct power_supply *psy,
 				      union power_supply_propval *val)
 {
 	struct cpcap_battery_ddata *ddata = power_supply_get_drvdata(psy);
-	struct cpcap_battery_state_data *latest, *previous, *low;
+	struct cpcap_battery_state_data *latest, *previous, *empty;
 	u32 sample;
 	s32 accumulator;
 	int cached;
@@ -634,10 +635,10 @@ static int cpcap_battery_get_property(struct power_supply *psy,
 		val->intval = div64_s64(tmp, 100);
 		break;
 	case POWER_SUPPLY_PROP_CAPACITY:
-		low = cpcap_battery_get_lowest(ddata);
-		if (!low->voltage || !ddata->charge_full)
+		empty = cpcap_battery_get_empty(ddata);
+		if (!empty->voltage || !ddata->charge_full)
 			return -ENODATA;
-		val->intval = (low->counter_uah -
+		val->intval = (empty->counter_uah -
 			       latest->counter_uah) * 100;
 		val->intval /= ddata->charge_full;
 		break;
@@ -645,10 +646,10 @@ static int cpcap_battery_get_property(struct power_supply *psy,
 		val->intval = cpcap_battery_get_capacity_level(ddata);
 		break;
 	case POWER_SUPPLY_PROP_CHARGE_NOW:
-		low = cpcap_battery_get_lowest(ddata);
-		if (!low->voltage)
+		empty = cpcap_battery_get_empty(ddata);
+		if (!empty->voltage)
 			return -ENODATA;
-		val->intval = low->counter_uah - latest->counter_uah;
+		val->intval = empty->counter_uah - latest->counter_uah;
 		break;
 	case POWER_SUPPLY_PROP_CHARGE_FULL:
 		if (!ddata->charge_full)
-- 
2.11.0


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

* [PATCH 12/15] power: supply: cpcap-battery: stabilize charge_full value
  2020-03-15 15:11 [PATCH 01/15] power: supply: cpcap-battery: Fix battery full status reporting Arthur Demchenkov
                   ` (9 preceding siblings ...)
  2020-03-15 15:12 ` [PATCH 11/15] power: supply: cpcap-battery: Rename properties, variables and functions Arthur Demchenkov
@ 2020-03-15 15:12 ` Arthur Demchenkov
  2020-03-15 15:12 ` [PATCH 13/15] power: supply: cpcap-battery: Fine tune end of charge current Arthur Demchenkov
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 27+ messages in thread
From: Arthur Demchenkov @ 2020-03-15 15:12 UTC (permalink / raw)
  To: Tony Lindgren, Merlijn Wajer, Pavel Machek, sre, linux-pm, linux-omap
  Cc: Arthur Demchenkov

It will not grow after the battery met "fully charged" state. This is
needed because of how constant voltage charging works. If we want "fully
charged" state to be reported in a user expected manner we are forced to
do it when the charging current is low enough, but not a zero value.

So, we report "battery full" when the charging current is as low as 100
mA. But the actual charging continues until the user disconnects the
charger. It means that a few mAh will be added to the reported
charge_full value, which the user shouldn't see. With that purpose we
clamp the charge_now value to not exceed charge_full.

Signed-off-by: Arthur Demchenkov <spinal.by@gmail.com>
---
 drivers/power/supply/cpcap-battery.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c
index 66ea1a718e02..517acdfa6009 100644
--- a/drivers/power/supply/cpcap-battery.c
+++ b/drivers/power/supply/cpcap-battery.c
@@ -469,15 +469,15 @@ static int cpcap_battery_update_status(struct cpcap_battery_ddata *ddata)
 	if (cpcap_battery_full(ddata)) {
 		full = cpcap_battery_get_full(ddata);
 		/* Update full state value? */
-		if (latest->counter_uah <= full->counter_uah ||
-		    !full->voltage) {
+		if (!full->voltage) {
 			memcpy(full, latest, sizeof(*full));
 
 			empty = cpcap_battery_get_empty(ddata);
-			if (empty->voltage && empty->voltage != -1)
+			if (empty->voltage) {
 				ddata->charge_full =
 					empty->counter_uah - full->counter_uah;
-			else if (ddata->charge_full) {
+				empty->voltage = -1;
+			} else if (ddata->charge_full) {
 				/* Initialize with user provided data */
 				empty->counter_uah =
 					full->counter_uah + ddata->charge_full;
@@ -488,18 +488,15 @@ static int cpcap_battery_update_status(struct cpcap_battery_ddata *ddata)
 	} else if (cpcap_battery_low(ddata)) {
 		empty = cpcap_battery_get_empty(ddata);
 		/* Update empty state value? */
-		if (latest->counter_uah >= empty->counter_uah ||
-		    !empty->voltage) {
+		if (!empty->voltage || empty->voltage == -1) {
 			memcpy(empty, latest, sizeof(*empty));
 
 			full = cpcap_battery_get_full(ddata);
-			if (full->voltage)
+			if (full->voltage) {
 				ddata->charge_full =
 					empty->counter_uah - full->counter_uah;
-			else if (ddata->charge_full)
-				/* Initialize with user provided data */
-				full->counter_uah =
-					empty->counter_uah - ddata->charge_full;
+				full->voltage = 0;
+			}
 		}
 	}
 
@@ -638,9 +635,9 @@ static int cpcap_battery_get_property(struct power_supply *psy,
 		empty = cpcap_battery_get_empty(ddata);
 		if (!empty->voltage || !ddata->charge_full)
 			return -ENODATA;
-		val->intval = (empty->counter_uah -
-			       latest->counter_uah) * 100;
-		val->intval /= ddata->charge_full;
+		val->intval = empty->counter_uah - latest->counter_uah;
+		val->intval = clamp(val->intval, 0, ddata->charge_full);
+		val->intval = val->intval * 100 / ddata->charge_full;
 		break;
 	case POWER_SUPPLY_PROP_CAPACITY_LEVEL:
 		val->intval = cpcap_battery_get_capacity_level(ddata);
@@ -650,6 +647,10 @@ static int cpcap_battery_get_property(struct power_supply *psy,
 		if (!empty->voltage)
 			return -ENODATA;
 		val->intval = empty->counter_uah - latest->counter_uah;
+		if (val->intval < 0)
+			val->intval = 0;
+		else if (ddata->charge_full && ddata->charge_full < val->intval)
+			val->intval = ddata->charge_full;
 		break;
 	case POWER_SUPPLY_PROP_CHARGE_FULL:
 		if (!ddata->charge_full)
-- 
2.11.0


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

* [PATCH 13/15] power: supply: cpcap-battery: Fine tune end of charge current
  2020-03-15 15:11 [PATCH 01/15] power: supply: cpcap-battery: Fix battery full status reporting Arthur Demchenkov
                   ` (10 preceding siblings ...)
  2020-03-15 15:12 ` [PATCH 12/15] power: supply: cpcap-battery: stabilize charge_full value Arthur Demchenkov
@ 2020-03-15 15:12 ` Arthur Demchenkov
  2020-03-15 15:12 ` [PATCH 14/15] power: supply: cpcap-battery: Make it behave like bq27200 Arthur Demchenkov
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 27+ messages in thread
From: Arthur Demchenkov @ 2020-03-15 15:12 UTC (permalink / raw)
  To: Tony Lindgren, Merlijn Wajer, Pavel Machek, sre, linux-pm, linux-omap
  Cc: Arthur Demchenkov

For some reason the charger device gets disconnected prior we have 100 mA
current. Set the fully charged state detection current to 112 mA.

Signed-off-by: Arthur Demchenkov <spinal.by@gmail.com>
---
 drivers/power/supply/cpcap-battery.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c
index 517acdfa6009..938117638983 100644
--- a/drivers/power/supply/cpcap-battery.c
+++ b/drivers/power/supply/cpcap-battery.c
@@ -411,10 +411,10 @@ static bool cpcap_battery_full(struct cpcap_battery_ddata *ddata)
 	    (ddata->config.bat.constant_charge_voltage_max_uv - 18000)) {
 		is_full = false;
 	} else if (is_full) {
-		if (state->current_ua < -150000)
+		if (state->current_ua < -170000)
 			is_full = false;
-	} else if (state->current_ua >= -150000 &&
-		   cpcap_battery_cc_get_avg_current(ddata) >= -100000) {
+	} else if (state->current_ua >= -170000 &&
+		   cpcap_battery_cc_get_avg_current(ddata) >= -112000) {
 		is_full = true;
 	}
 
-- 
2.11.0


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

* [PATCH 14/15] power: supply: cpcap-battery: Make it behave like bq27200
  2020-03-15 15:11 [PATCH 01/15] power: supply: cpcap-battery: Fix battery full status reporting Arthur Demchenkov
                   ` (11 preceding siblings ...)
  2020-03-15 15:12 ` [PATCH 13/15] power: supply: cpcap-battery: Fine tune end of charge current Arthur Demchenkov
@ 2020-03-15 15:12 ` Arthur Demchenkov
  2020-03-15 15:12 ` [PATCH 15/15] power: supply: cpcap-battery: Add rounding to capacity reporting Arthur Demchenkov
  2020-03-15 18:58 ` [PATCH 01/15] power: supply: cpcap-battery: Fix battery full status reporting Pavel Machek
  14 siblings, 0 replies; 27+ messages in thread
From: Arthur Demchenkov @ 2020-03-15 15:12 UTC (permalink / raw)
  To: Tony Lindgren, Merlijn Wajer, Pavel Machek, sre, linux-pm, linux-omap
  Cc: Arthur Demchenkov

Now we begin to update the charge_now value immediately after connecting
or disconnecting the charger. No more hidden mAh buffer when the battery
is fully charged or discharged.

Signed-off-by: Arthur Demchenkov <spinal.by@gmail.com>
---
 drivers/power/supply/cpcap-battery.c | 44 +++++++++++++++---------------------
 1 file changed, 18 insertions(+), 26 deletions(-)

diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c
index 938117638983..4e872bd36ccf 100644
--- a/drivers/power/supply/cpcap-battery.c
+++ b/drivers/power/supply/cpcap-battery.c
@@ -468,35 +468,27 @@ static int cpcap_battery_update_status(struct cpcap_battery_ddata *ddata)
 
 	if (cpcap_battery_full(ddata)) {
 		full = cpcap_battery_get_full(ddata);
-		/* Update full state value? */
-		if (!full->voltage) {
-			memcpy(full, latest, sizeof(*full));
-
-			empty = cpcap_battery_get_empty(ddata);
-			if (empty->voltage) {
-				ddata->charge_full =
-					empty->counter_uah - full->counter_uah;
-				empty->voltage = -1;
-			} else if (ddata->charge_full) {
-				/* Initialize with user provided data */
-				empty->counter_uah =
-					full->counter_uah + ddata->charge_full;
-				/* Mark it as initialized */
-				empty->voltage = -1;
-			}
+		memcpy(full, latest, sizeof(*full));
+
+		empty = cpcap_battery_get_empty(ddata);
+		if (empty->voltage && empty->voltage != -1) {
+			empty->voltage = -1;
+			ddata->charge_full =
+				empty->counter_uah - full->counter_uah;
+		} else if (ddata->charge_full) {
+			empty->voltage = -1;
+			empty->counter_uah =
+				full->counter_uah + ddata->charge_full;
 		}
 	} else if (cpcap_battery_low(ddata)) {
 		empty = cpcap_battery_get_empty(ddata);
-		/* Update empty state value? */
-		if (!empty->voltage || empty->voltage == -1) {
-			memcpy(empty, latest, sizeof(*empty));
-
-			full = cpcap_battery_get_full(ddata);
-			if (full->voltage) {
-				ddata->charge_full =
-					empty->counter_uah - full->counter_uah;
-				full->voltage = 0;
-			}
+		memcpy(empty, latest, sizeof(*empty));
+
+		full = cpcap_battery_get_full(ddata);
+		if (full->voltage) {
+			full->voltage = 0;
+			ddata->charge_full =
+				empty->counter_uah - full->counter_uah;
 		}
 	}
 
-- 
2.11.0


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

* [PATCH 15/15] power: supply: cpcap-battery: Add rounding to capacity reporting
  2020-03-15 15:11 [PATCH 01/15] power: supply: cpcap-battery: Fix battery full status reporting Arthur Demchenkov
                   ` (12 preceding siblings ...)
  2020-03-15 15:12 ` [PATCH 14/15] power: supply: cpcap-battery: Make it behave like bq27200 Arthur Demchenkov
@ 2020-03-15 15:12 ` Arthur Demchenkov
  2020-03-15 18:58 ` [PATCH 01/15] power: supply: cpcap-battery: Fix battery full status reporting Pavel Machek
  14 siblings, 0 replies; 27+ messages in thread
From: Arthur Demchenkov @ 2020-03-15 15:12 UTC (permalink / raw)
  To: Tony Lindgren, Merlijn Wajer, Pavel Machek, sre, linux-pm, linux-omap
  Cc: Arthur Demchenkov

Add rounding to capacity reporting.

Signed-off-by: Arthur Demchenkov <spinal.by@gmail.com>
---
 drivers/power/supply/cpcap-battery.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c
index 4e872bd36ccf..b16848cfb58c 100644
--- a/drivers/power/supply/cpcap-battery.c
+++ b/drivers/power/supply/cpcap-battery.c
@@ -627,7 +627,9 @@ static int cpcap_battery_get_property(struct power_supply *psy,
 		empty = cpcap_battery_get_empty(ddata);
 		if (!empty->voltage || !ddata->charge_full)
 			return -ENODATA;
-		val->intval = empty->counter_uah - latest->counter_uah;
+		/* (ddata->charge_full / 200) is needed for rounding */
+		val->intval = empty->counter_uah - latest->counter_uah +
+			ddata->charge_full / 200;
 		val->intval = clamp(val->intval, 0, ddata->charge_full);
 		val->intval = val->intval * 100 / ddata->charge_full;
 		break;
-- 
2.11.0


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

* Re: [PATCH 02/15] power: supply: cpcap-battery: Improve battery full status detection
  2020-03-15 15:11 ` [PATCH 02/15] power: supply: cpcap-battery: Improve battery full status detection Arthur Demchenkov
@ 2020-03-15 16:46   ` Tony Lindgren
  0 siblings, 0 replies; 27+ messages in thread
From: Tony Lindgren @ 2020-03-15 16:46 UTC (permalink / raw)
  To: Arthur Demchenkov; +Cc: Merlijn Wajer, Pavel Machek, sre, linux-pm, linux-omap

* Arthur Demchenkov <spinal.by@gmail.com> [200315 15:15]:
> If the battery status is detected as full for the charging current that
> doesn't exceed 100 mA, it will then be reported as full for charging
> currents in the range of 100-150 mA. This is needed because
> charge_current value has a spread.
> 
> We don't use avg_current here because it can trigger wrong battery full
> status on charger connected event.

Hmm oh so this is against my earlier RFC patches. Care to respin
the series against v5.6-rc?

Feel free tweak my patches to leave out the unnecessary stuff as we
decided to do things in a better way :) Just add a note like:

[spinal: dropped out unusable foo and bar]

So we know what got changed compared to the RFC patches.

And then we might have a nice working set for merging in subject
to others approving this approach of course :)

Regards,

Tony

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

* Re: [PATCH 01/15] power: supply: cpcap-battery: Fix battery full status reporting
  2020-03-15 15:11 [PATCH 01/15] power: supply: cpcap-battery: Fix battery full status reporting Arthur Demchenkov
                   ` (13 preceding siblings ...)
  2020-03-15 15:12 ` [PATCH 15/15] power: supply: cpcap-battery: Add rounding to capacity reporting Arthur Demchenkov
@ 2020-03-15 18:58 ` Pavel Machek
  2020-03-15 19:19   ` Tony Lindgren
  2020-03-15 20:51   ` Arthur D.
  14 siblings, 2 replies; 27+ messages in thread
From: Pavel Machek @ 2020-03-15 18:58 UTC (permalink / raw)
  To: Arthur Demchenkov; +Cc: Tony Lindgren, Merlijn Wajer, sre, linux-pm, linux-omap

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

Hi!

> Don't report that the battery is fully charged if the charging current
> exceeds 100 mA.

Could you merge patches together for possibility of reasonable review?

> @@ -408,7 +408,8 @@ static bool cpcap_battery_full(struct cpcap_battery_ddata *ddata)
>  	struct cpcap_battery_state_data *state = cpcap_battery_latest(ddata);
>  
>  	if (state->voltage >=
> -	    (ddata->config.bat.constant_charge_voltage_max_uv - 18000))
> +	    (ddata->config.bat.constant_charge_voltage_max_uv - 18000) &&
> +		state->current_ua > -100000)

It seems that this 100mA threshold is changed about 3 times in the
series :-(.

Plus, it might be better to place booleans into struct, rather than
using "static bool" inside a function.

Could we get some kind of explanations for the whole series? 100mA is
rather high current for end of charge. If you stop updating
full capacity when "your" end of charge is met, you'll have battery
charged to more than 100%. I... don't think that makes sense.

Best regards,
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

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

* Re: [PATCH 01/15] power: supply: cpcap-battery: Fix battery full status reporting
  2020-03-15 18:58 ` [PATCH 01/15] power: supply: cpcap-battery: Fix battery full status reporting Pavel Machek
@ 2020-03-15 19:19   ` Tony Lindgren
  2020-03-15 20:51   ` Arthur D.
  1 sibling, 0 replies; 27+ messages in thread
From: Tony Lindgren @ 2020-03-15 19:19 UTC (permalink / raw)
  To: Pavel Machek; +Cc: Arthur Demchenkov, Merlijn Wajer, sre, linux-pm, linux-omap

* Pavel Machek <pavel@ucw.cz> [200315 18:59]:
> Hi!
> 
> > Don't report that the battery is fully charged if the charging current
> > exceeds 100 mA.
> 
> Could you merge patches together for possibility of reasonable review?
> 
> > @@ -408,7 +408,8 @@ static bool cpcap_battery_full(struct cpcap_battery_ddata *ddata)
> >  	struct cpcap_battery_state_data *state = cpcap_battery_latest(ddata);
> >  
> >  	if (state->voltage >=
> > -	    (ddata->config.bat.constant_charge_voltage_max_uv - 18000))
> > +	    (ddata->config.bat.constant_charge_voltage_max_uv - 18000) &&
> > +		state->current_ua > -100000)
> 
> It seems that this 100mA threshold is changed about 3 times in the
> series :-(.
> 
> Plus, it might be better to place booleans into struct, rather than
> using "static bool" inside a function.
> 
> Could we get some kind of explanations for the whole series? 100mA is
> rather high current for end of charge. If you stop updating
> full capacity when "your" end of charge is met, you'll have battery
> charged to more than 100%. I... don't think that makes sense.

Yeh please compress out the unncessary parts of my RFC patches :)

For reference only, below is a diff of all the changes for
cpcap-battery.c against v5.6-rc5.

I applied this series on top of a droid4-pending-v5.6 based branch
that has the earlier RFC changes applied.

Regards,

Tony

8< -----------------
diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c
--- a/drivers/power/supply/cpcap-battery.c
+++ b/drivers/power/supply/cpcap-battery.c
@@ -110,6 +110,8 @@ struct cpcap_coulomb_counter_data {
 enum cpcap_battery_state {
 	CPCAP_BATTERY_STATE_PREVIOUS,
 	CPCAP_BATTERY_STATE_LATEST,
+	CPCAP_BATTERY_STATE_EMPTY,
+	CPCAP_BATTERY_STATE_FULL,
 	CPCAP_BATTERY_STATE_NR,
 };
 
@@ -132,10 +134,32 @@ struct cpcap_battery_ddata {
 	struct cpcap_battery_state_data state[CPCAP_BATTERY_STATE_NR];
 	u32 cc_lsb;		/* μAms per LSB */
 	atomic_t active;
+	int charge_full;
 	int status;
 	u16 vendor;
 };
 
+struct cpcap_battery_capacity {
+	int capacity;
+	int voltage;
+};
+
+#define CPCAP_CAP(l, v)				\
+{						\
+	.capacity = (l),			\
+	.voltage = (v),				\
+},
+
+/* Pessimistic battery capacity mapping before high or low value is seen */
+static const struct cpcap_battery_capacity cpcap_battery_cap[] = {
+	CPCAP_CAP(POWER_SUPPLY_CAPACITY_LEVEL_UNKNOWN,        0)
+	CPCAP_CAP(POWER_SUPPLY_CAPACITY_LEVEL_CRITICAL, 3100000)
+	CPCAP_CAP(POWER_SUPPLY_CAPACITY_LEVEL_LOW,      3300000)
+	CPCAP_CAP(POWER_SUPPLY_CAPACITY_LEVEL_NORMAL,   3700000)
+	CPCAP_CAP(POWER_SUPPLY_CAPACITY_LEVEL_HIGH,     4000000)
+	CPCAP_CAP(POWER_SUPPLY_CAPACITY_LEVEL_FULL,     4200000 - 18000)
+};
+
 #define CPCAP_NO_BATTERY	-400
 
 static struct cpcap_battery_state_data *
@@ -160,6 +184,18 @@ cpcap_battery_previous(struct cpcap_battery_ddata *ddata)
 	return cpcap_battery_get_state(ddata, CPCAP_BATTERY_STATE_PREVIOUS);
 }
 
+static struct cpcap_battery_state_data *
+cpcap_battery_get_empty(struct cpcap_battery_ddata *ddata)
+{
+	return cpcap_battery_get_state(ddata, CPCAP_BATTERY_STATE_EMPTY);
+}
+
+static struct cpcap_battery_state_data *
+cpcap_battery_get_full(struct cpcap_battery_ddata *ddata)
+{
+	return cpcap_battery_get_state(ddata, CPCAP_BATTERY_STATE_FULL);
+}
+
 static int cpcap_charger_battery_temperature(struct cpcap_battery_ddata *ddata,
 					     int *value)
 {
@@ -369,17 +405,39 @@ static int cpcap_battery_cc_get_avg_current(struct cpcap_battery_ddata *ddata)
 static bool cpcap_battery_full(struct cpcap_battery_ddata *ddata)
 {
 	struct cpcap_battery_state_data *state = cpcap_battery_latest(ddata);
+	static bool is_full;
+
+	if (state->voltage <
+	    (ddata->config.bat.constant_charge_voltage_max_uv - 18000)) {
+		is_full = false;
+	} else if (is_full) {
+		if (state->current_ua < -170000)
+			is_full = false;
+	} else if (state->current_ua >= -170000 &&
+		   cpcap_battery_cc_get_avg_current(ddata) >= -112000) {
+		is_full = true;
+	}
+
+	return is_full;
+}
 
-	if (state->voltage >=
-	    (ddata->config.bat.constant_charge_voltage_max_uv - 18000))
-		return true;
+static bool cpcap_battery_low(struct cpcap_battery_ddata *ddata)
+{
+	struct cpcap_battery_state_data *state = cpcap_battery_latest(ddata);
+	static bool is_low;
 
-	return false;
+	if (state->current_ua > 0 && (state->voltage <= 3350000 || is_low))
+		is_low = true;
+	else
+		is_low = false;
+
+	return is_low;
 }
 
 static int cpcap_battery_update_status(struct cpcap_battery_ddata *ddata)
 {
-	struct cpcap_battery_state_data state, *latest, *previous;
+	struct cpcap_battery_state_data state, *latest, *previous,
+					*empty, *full;
 	ktime_t now;
 	int error;
 
@@ -408,9 +466,56 @@ static int cpcap_battery_update_status(struct cpcap_battery_ddata *ddata)
 	memcpy(previous, latest, sizeof(*previous));
 	memcpy(latest, &state, sizeof(*latest));
 
+	if (cpcap_battery_full(ddata)) {
+		full = cpcap_battery_get_full(ddata);
+		memcpy(full, latest, sizeof(*full));
+
+		empty = cpcap_battery_get_empty(ddata);
+		if (empty->voltage && empty->voltage != -1) {
+			empty->voltage = -1;
+			ddata->charge_full =
+				empty->counter_uah - full->counter_uah;
+		} else if (ddata->charge_full) {
+			empty->voltage = -1;
+			empty->counter_uah =
+				full->counter_uah + ddata->charge_full;
+		}
+	} else if (cpcap_battery_low(ddata)) {
+		empty = cpcap_battery_get_empty(ddata);
+		memcpy(empty, latest, sizeof(*empty));
+
+		full = cpcap_battery_get_full(ddata);
+		if (full->voltage) {
+			full->voltage = 0;
+			ddata->charge_full =
+				empty->counter_uah - full->counter_uah;
+		}
+	}
+
 	return 0;
 }
 
+static int cpcap_battery_get_capacity_level(struct cpcap_battery_ddata *ddata)
+{
+	struct cpcap_battery_state_data *latest;
+	const struct cpcap_battery_capacity *cap = NULL;
+	int voltage, i;
+
+	latest = cpcap_battery_latest(ddata);
+	voltage = latest->voltage;
+
+	for (i = ARRAY_SIZE(cpcap_battery_cap) - 1; i >=0; i--) {
+		cap = &cpcap_battery_cap[i];
+		if (voltage >= cap->voltage)
+			break;
+	}
+
+	if (!cap)
+		return 0;
+
+	return cap->capacity;
+}
+
 static enum power_supply_property cpcap_battery_props[] = {
 	POWER_SUPPLY_PROP_STATUS,
 	POWER_SUPPLY_PROP_PRESENT,
@@ -421,10 +526,13 @@ static enum power_supply_property cpcap_battery_props[] = {
 	POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
 	POWER_SUPPLY_PROP_CURRENT_AVG,
 	POWER_SUPPLY_PROP_CURRENT_NOW,
+	POWER_SUPPLY_PROP_CHARGE_FULL,
+	POWER_SUPPLY_PROP_CHARGE_NOW,
 	POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
 	POWER_SUPPLY_PROP_CHARGE_COUNTER,
 	POWER_SUPPLY_PROP_POWER_NOW,
 	POWER_SUPPLY_PROP_POWER_AVG,
+	POWER_SUPPLY_PROP_CAPACITY,
 	POWER_SUPPLY_PROP_CAPACITY_LEVEL,
 	POWER_SUPPLY_PROP_SCOPE,
 	POWER_SUPPLY_PROP_TEMP,
@@ -435,7 +543,7 @@ static int cpcap_battery_get_property(struct power_supply *psy,
 				      union power_supply_propval *val)
 {
 	struct cpcap_battery_ddata *ddata = power_supply_get_drvdata(psy);
-	struct cpcap_battery_state_data *latest, *previous;
+	struct cpcap_battery_state_data *latest, *previous, *empty;
 	u32 sample;
 	s32 accumulator;
 	int cached;
@@ -515,19 +623,33 @@ static int cpcap_battery_get_property(struct power_supply *psy,
 		tmp *= ((latest->voltage + previous->voltage) / 20000);
 		val->intval = div64_s64(tmp, 100);
 		break;
+	case POWER_SUPPLY_PROP_CAPACITY:
+		empty = cpcap_battery_get_empty(ddata);
+		if (!empty->voltage || !ddata->charge_full)
+			return -ENODATA;
+		/* (ddata->charge_full / 200) is needed for rounding */
+		val->intval = empty->counter_uah - latest->counter_uah +
+			ddata->charge_full / 200;
+		val->intval = clamp(val->intval, 0, ddata->charge_full);
+		val->intval = val->intval * 100 / ddata->charge_full;
+		break;
 	case POWER_SUPPLY_PROP_CAPACITY_LEVEL:
-		if (cpcap_battery_full(ddata))
-			val->intval = POWER_SUPPLY_CAPACITY_LEVEL_FULL;
-		else if (latest->voltage >= 3750000)
-			val->intval = POWER_SUPPLY_CAPACITY_LEVEL_HIGH;
-		else if (latest->voltage >= 3300000)
-			val->intval = POWER_SUPPLY_CAPACITY_LEVEL_NORMAL;
-		else if (latest->voltage > 3100000)
-			val->intval = POWER_SUPPLY_CAPACITY_LEVEL_LOW;
-		else if (latest->voltage <= 3100000)
-			val->intval = POWER_SUPPLY_CAPACITY_LEVEL_CRITICAL;
-		else
-			val->intval = POWER_SUPPLY_CAPACITY_LEVEL_UNKNOWN;
+		val->intval = cpcap_battery_get_capacity_level(ddata);
+		break;
+	case POWER_SUPPLY_PROP_CHARGE_NOW:
+		empty = cpcap_battery_get_empty(ddata);
+		if (!empty->voltage)
+			return -ENODATA;
+		val->intval = empty->counter_uah - latest->counter_uah;
+		if (val->intval < 0)
+			val->intval = 0;
+		else if (ddata->charge_full && ddata->charge_full < val->intval)
+			val->intval = ddata->charge_full;
+		break;
+	case POWER_SUPPLY_PROP_CHARGE_FULL:
+		if (!ddata->charge_full)
+			return -ENODATA;
+		val->intval = ddata->charge_full;
 		break;
 	case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN:
 		val->intval = ddata->config.info.charge_full_design;
@@ -590,6 +712,15 @@ static int cpcap_battery_set_property(struct power_supply *psy,
 		ddata->config.bat.constant_charge_voltage_max_uv = val->intval;
 
 		return cpcap_battery_update_charger(ddata, val->intval);
+	case POWER_SUPPLY_PROP_CHARGE_FULL:
+		if (val->intval < 0)
+			return -EINVAL;
+		if (val->intval > ddata->config.info.charge_full_design)
+			return -EINVAL;
+
+		ddata->charge_full = val->intval;
+
+		return 0;
 	default:
 		return -EINVAL;
 	}
@@ -602,6 +733,7 @@ static int cpcap_battery_property_is_writeable(struct power_supply *psy,
 {
 	switch (psp) {
 	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
+	case POWER_SUPPLY_PROP_CHARGE_FULL:
 		return 1;
 	default:
 		return 0;

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

* Re: [PATCH 01/15] power: supply: cpcap-battery: Fix battery full status reporting
  2020-03-15 18:58 ` [PATCH 01/15] power: supply: cpcap-battery: Fix battery full status reporting Pavel Machek
  2020-03-15 19:19   ` Tony Lindgren
@ 2020-03-15 20:51   ` Arthur D.
  2020-03-15 21:59     ` Tony Lindgren
  1 sibling, 1 reply; 27+ messages in thread
From: Arthur D. @ 2020-03-15 20:51 UTC (permalink / raw)
  To: Pavel Machek; +Cc: Tony Lindgren, Merlijn Wajer, sre, linux-pm, linux-omap

> Could we get some kind of explanations for the whole series? 100mA is
> rather high current for end of charge. If you stop updating
> full capacity when "your" end of charge is met, you'll have battery
> charged to more than 100%. I... don't think that makes sense.

The aim was to allow userspace to see accurate values for charge_now,
charge_full and capacity (percentage) of the battery. It will allow
the user to estimate how long the device can work on the battery
with current power consumption.

For this the user will need to do a battery calibration. I.e.
he will need to fully charge and then discharge the battery.
Or vice versa: discharge -> charge. Once the user completes the
calibration cycle, he will be able to see pretty accurate values
for charge_now, charge_full and capacity. This is similar to how
bq27200 IC from Nokia N900 works.

Also this patchset allows the userspace to restore the calibration
value after reboot. By the calibration value I mean charge_full.

We can't rely on restoring charge_now value, because the user may
have multiple operating systems installed and if he works in another
OS for a while the charge_now value will become invalid.

So, after a reboot the user may want to restore the charge_full
value, so the kernel will be able to estimate the percentage
and capacity values without forcing the user to do a full calib-
ration cycle again. The only thing the user will have to do
is to fully charge OR fully discharge the battery at least once.
And he will get all values set.

Now about the chosen limits. For some reason the charging is
interrupted (and restarted after a while) when the following
conditions are met:
1) the charging current is < 112 mA
2) the display backlight is off

This behaviour was observed in Maemo Leste with hildon-desktop
running. I tested these patches for several days, so I picked up
the parameters for optimal (from my point of view) work in practice
taking into account the current "features" of Droid 4 drivers.

If we could somehow fix this behaviour (charging interruption),
I'd reconsider the end of charge current value to be 50 mA.

Making it lower than 50 mA doesn't seem to make much sence
because of the extended charging time visibility without giving
significant improvement in charge_full accuracy.

> If you stop updating full capacity when "your" end of charge is met,
> you'll have battery charged to more than 100%.

No worries. With the implemented algorithm, the user will not notice
that the battery is more than 100% charged (which is just a convention
here). And this situation gives an advantage in that it has a slightly
pessimistic charge_full value, which in practice turns out to be good:
the user will be warned about low battery a little ahead of time.

--
Best regards, Spinal

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

* Re: [PATCH 01/15] power: supply: cpcap-battery: Fix battery full status reporting
  2020-03-15 20:51   ` Arthur D.
@ 2020-03-15 21:59     ` Tony Lindgren
  2020-03-16  1:30       ` Arthur D.
  0 siblings, 1 reply; 27+ messages in thread
From: Tony Lindgren @ 2020-03-15 21:59 UTC (permalink / raw)
  To: Arthur D.; +Cc: Pavel Machek, Merlijn Wajer, sre, linux-pm, linux-omap

* Arthur D. <spinal.by@gmail.com> [200315 20:51]:
> Now about the chosen limits. For some reason the charging is
> interrupted (and restarted after a while) when the following
> conditions are met:
> 1) the charging current is < 112 mA
> 2) the display backlight is off
> 
> This behaviour was observed in Maemo Leste with hildon-desktop
> running. I tested these patches for several days, so I picked up
> the parameters for optimal (from my point of view) work in practice
> taking into account the current "features" of Droid 4 drivers.
> 
> If we could somehow fix this behaviour (charging interruption),
> I'd reconsider the end of charge current value to be 50 mA.

Hmm well we do have two chargers, the usb charger and the
unknown inductive charger for the pins on the back.

It would be best to keep cpcap-battery.c independent of the
chargers to avoid depndencies as the chargers do usually start
charging briefly always when connected.

Maybe just adding something like below would be enough of a check:

static int
cpcap_battery_get_counter_rate(struct cpcap_battery_ddata *ddata,
			       int poll_time_ms);

And then based on the value being negative or positive you
would know if it's charging or not. I guess we could then
use this also for POWER_SUPPLY_PROP_CHARGE_NOW with poll_time_ms
value of 0. I think the charge counter is configure to poll
at 250 ms right now.

Regards,

Tony

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

* Re: [PATCH 01/15] power: supply: cpcap-battery: Fix battery full status reporting
  2020-03-15 21:59     ` Tony Lindgren
@ 2020-03-16  1:30       ` Arthur D.
  2020-03-16 16:02         ` Tony Lindgren
  0 siblings, 1 reply; 27+ messages in thread
From: Arthur D. @ 2020-03-16  1:30 UTC (permalink / raw)
  To: Tony Lindgren; +Cc: Pavel Machek, Merlijn Wajer, sre, linux-pm, linux-omap

Hi, Tony.

It seems like a misunderstanding here. There's no problem in detecting
if the charging is in progress. The green led is switched off and
the battery current sign is changed from "-" to "+" (which means
that the battery is being discharged). So there's no need in additional
checks. For cpcap-battery this situation seems like a battery stopped
charging. And it doesn't matter if that was a user who disconnected
the charger or it was done somewhere in a driver/firmware/hardware.

The problem is that the charging current cant get to the point <100 mA,
not talking about <50 mA. And that's why I set the value of 112 mA for
the end of charge current: to help the kernel to detect this plateau and
to stop the calibration cycle, so the userspace can get all the battery
parameters I mentioned in the previous mail.

Please note, that the behaviour I mentioned was observed only when the
conditions written in my last mail were met. The important one was:
> 2) the display backlight is off

Because when I unlocked the display the charging current was able
to go below 112 mA. Of course I couldn't rely on something like this:
the user should stay with backlight on to have the battery calibrated.
Think about it: waiting for the charging current to drop from 100 mA
to 50 mA can take dozens of minutes (it depends on the age of battery -
the older the battery the longer it will take), and the user should
force somehow the device to not switch off the display hightlight
until the battery is calibrated.

Of course it's unacceptable, so I decided to set the end of charge
current limit to 112 mA. Which allows the user to just put the device
on a table and to wait until it's fully charged without a need
to interfere the charging process with some action from the user.

--
Best regards, Spinal


>> Now about the chosen limits. For some reason the charging is
>> interrupted (and restarted after a while) when the following
>> conditions are met:
>> 1) the charging current is < 112 mA
>> 2) the display backlight is off
>>
>> This behaviour was observed in Maemo Leste with hildon-desktop
>> running. I tested these patches for several days, so I picked up
>> the parameters for optimal (from my point of view) work in practice
>> taking into account the current "features" of Droid 4 drivers.
>>
>> If we could somehow fix this behaviour (charging interruption),
>> I'd reconsider the end of charge current value to be 50 mA.
>
> Hmm well we do have two chargers, the usb charger and the
> unknown inductive charger for the pins on the back.
>
> It would be best to keep cpcap-battery.c independent of the
> chargers to avoid depndencies as the chargers do usually start
> charging briefly always when connected.
>
> Maybe just adding something like below would be enough of a check:
>
> static int
> cpcap_battery_get_counter_rate(struct cpcap_battery_ddata *ddata,
> 			       int poll_time_ms);
>
> And then based on the value being negative or positive you
> would know if it's charging or not. I guess we could then
> use this also for POWER_SUPPLY_PROP_CHARGE_NOW with poll_time_ms
> value of 0. I think the charge counter is configure to poll
> at 250 ms right now.

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

* Re: [PATCH 01/15] power: supply: cpcap-battery: Fix battery full status reporting
  2020-03-16  1:30       ` Arthur D.
@ 2020-03-16 16:02         ` Tony Lindgren
  0 siblings, 0 replies; 27+ messages in thread
From: Tony Lindgren @ 2020-03-16 16:02 UTC (permalink / raw)
  To: Arthur D.; +Cc: Pavel Machek, Merlijn Wajer, sre, linux-pm, linux-omap

* Arthur D. <spinal.by@gmail.com> [200316 01:31]:
> Hi, Tony.
> 
> It seems like a misunderstanding here. There's no problem in detecting
> if the charging is in progress. The green led is switched off and
> the battery current sign is changed from "-" to "+" (which means
> that the battery is being discharged). So there's no need in additional
> checks. For cpcap-battery this situation seems like a battery stopped
> charging. And it doesn't matter if that was a user who disconnected
> the charger or it was done somewhere in a driver/firmware/hardware.
> 
> The problem is that the charging current cant get to the point <100 mA,
> not talking about <50 mA. And that's why I set the value of 112 mA for
> the end of charge current: to help the kernel to detect this plateau and
> to stop the calibration cycle, so the userspace can get all the battery
> parameters I mentioned in the previous mail.

OK I guess that's easy to change if we figure out something better :)
Maybe add some define for it like CPCAP_BATTERY_FULL_CHARGE_CURRENT or
similar?

> Please note, that the behaviour I mentioned was observed only when the
> conditions written in my last mail were met. The important one was:
> > 2) the display backlight is off
> 
> Because when I unlocked the display the charging current was able
> to go below 112 mA. Of course I couldn't rely on something like this:
> the user should stay with backlight on to have the battery calibrated.
> Think about it: waiting for the charging current to drop from 100 mA
> to 50 mA can take dozens of minutes (it depends on the age of battery -
> the older the battery the longer it will take), and the user should
> force somehow the device to not switch off the display hightlight
> until the battery is calibrated.
> 
> Of course it's unacceptable, so I decided to set the end of charge
> current limit to 112 mA. Which allows the user to just put the device
> on a table and to wait until it's fully charged without a need
> to interfere the charging process with some action from the user.

Yeah OK thanks.

Tony

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

* Re: [PATCH 03/15] power: supply: cpcap-battery: Fix battery low status reporting
  2020-03-15 15:11 ` [PATCH 03/15] power: supply: cpcap-battery: Fix battery low status reporting Arthur Demchenkov
@ 2020-03-21 14:47   ` Tony Lindgren
  2020-03-21 21:40     ` Arthur D.
  0 siblings, 1 reply; 27+ messages in thread
From: Tony Lindgren @ 2020-03-21 14:47 UTC (permalink / raw)
  To: Arthur Demchenkov; +Cc: Merlijn Wajer, Pavel Machek, sre, linux-pm, linux-omap

* Arthur Demchenkov <spinal.by@gmail.com> [200315 15:15]:
> If we hit battery low once, we should stick on reporting it until the
> charger is connected. This way low->counter_uah will be updated
> properly, and that will allow us to get more accurate charge_full value.
> 
> Signed-off-by: Arthur Demchenkov <spinal.by@gmail.com>
> ---
>  drivers/power/supply/cpcap-battery.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c
> index 52f03a2662a5..8a58ad943960 100644
> --- a/drivers/power/supply/cpcap-battery.c
> +++ b/drivers/power/supply/cpcap-battery.c
> @@ -421,11 +421,14 @@ static bool cpcap_battery_full(struct cpcap_battery_ddata *ddata)
>  static bool cpcap_battery_low(struct cpcap_battery_ddata *ddata)
>  {
>  	struct cpcap_battery_state_data *state = cpcap_battery_latest(ddata);
> +	static bool is_low;
>  
> -	if (state->current_ua && state->voltage <= 3300000)
> -		return true;
> +	if (state->current_ua > 0 && (state->voltage <= 3300000 || is_low))
> +		is_low = true;
> +	else
> +		is_low = false;
>  
> -	return false;
> +	return is_low;
>  }

Please set up bitmask in ddata for unsigned int battery_low:1; instead of
using a static variable here. If we ever had two instances of cpcap on the
same device the static variable here would not work :)

Regards,

Tony


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

* Re: [PATCH 06/15] power: supply: cpcap-battery: Initialize with user provided data
  2020-03-15 15:11 ` [PATCH 06/15] power: supply: cpcap-battery: Initialize with user provided data Arthur Demchenkov
@ 2020-03-21 14:54   ` Tony Lindgren
  2020-03-21 22:08     ` Arthur D.
  0 siblings, 1 reply; 27+ messages in thread
From: Tony Lindgren @ 2020-03-21 14:54 UTC (permalink / raw)
  To: Arthur Demchenkov; +Cc: Merlijn Wajer, Pavel Machek, sre, linux-pm, linux-omap

* Arthur Demchenkov <spinal.by@gmail.com> [200315 15:15]:
> If the user provides us with charge_full value (which it could save in a
> permanent storage between reboots), initialize low and high counter_uah
> with calculated values.

Hmm I'm getting -EINVAL when I do echo 1600000 > charge_now but yet the
value does get updated?

And I'm still not seeing capacity show up after that though even with full
battery.. I think we should be able to calculate it if either a high or
low value has been seen?

Also, we should have the driver default to using the charge_full_design
value if nothing is written from userspace and we see a high or low value.

Maybe some pessimistic estimate could be used instead of just using
charge_full_design if no value is initialized from the userspace?
Something like (charge_full_design / 4) * 3 maybe?

Yes it will be off, but the driver should work also without user
interaction :)

Regards,

Tony

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

* Re: [PATCH 03/15] power: supply: cpcap-battery: Fix battery low status reporting
  2020-03-21 14:47   ` Tony Lindgren
@ 2020-03-21 21:40     ` Arthur D.
  0 siblings, 0 replies; 27+ messages in thread
From: Arthur D. @ 2020-03-21 21:40 UTC (permalink / raw)
  To: Tony Lindgren; +Cc: Merlijn Wajer, Pavel Machek, sre, linux-pm, linux-omap

Will do. Thanks!

> Please set up bitmask in ddata for unsigned int battery_low:1; instead of
> using a static variable here. If we ever had two instances of cpcap on  
> the same device the static variable here would not work :)

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

* Re: [PATCH 06/15] power: supply: cpcap-battery: Initialize with user provided data
  2020-03-21 14:54   ` Tony Lindgren
@ 2020-03-21 22:08     ` Arthur D.
  2020-03-21 22:21       ` Tony Lindgren
  0 siblings, 1 reply; 27+ messages in thread
From: Arthur D. @ 2020-03-21 22:08 UTC (permalink / raw)
  To: Tony Lindgren; +Cc: Merlijn Wajer, Pavel Machek, sre, linux-pm, linux-omap

Hi.

> Hmm I'm getting -EINVAL when I do echo 1600000 > charge_now but yet the
> value does get updated?

Hm. I'm getting "Permission denied" when trying to do this.
Anyway, charge_now value is not supposed to be initialized by the user.
Only charge_full should be writable.

> And I'm still not seeing capacity show up after that though even with  
> full battery.. I think we should be able to calculate it if either a  
> high or
> low value has been seen?

There are two steps needed to calibrate the battery: to hit the "fully  
charged"
state and to hit "battery empty" state. When the both states were hit the  
driver
will initialize charge_full value. And it will start reporting correct  
charge_now
and capacity (battery charge percentage) values.

Once the charge_full value is calculated it's recommended to be saved by  
the user
to a permanent storage between reboots.

Saving/restoring the value can be done in init scripts.

For saving the calibration value just use the command like:
cat /sys/class/power_supply/battery/charge_full > /battery_cf

To restore (after device reboot or power on):
cat /battery_cf > /sys/class/power_supply/battery/charge_full

This will allow the kernel to do fast calibration. I.e. you will
only need to fully charge _OR_ fully discharge the battery to start
seeing correct charge_now and capacity values.

> Also, we should have the driver default to using the charge_full_design
> value if nothing is written from userspace and we see a high or low  
> value.

I'd prefer to have charge_full value undefined until the battery is  
calibrated.
This way the userspace can estimate current battery capacity using voltage  
as a
fallback for uncalibrated battery. The voltage estimation will be  way more
accurate than having charge_full = charge_full_design on pretty old Droid 4
batteries. For example, my battery after calibration has about 1000 mAh,
while charge_full_design is 1740000.

> Maybe some pessimistic estimate could be used instead of just using
> charge_full_design if no value is initialized from the userspace?
> Something like (charge_full_design / 4) * 3 maybe?

It's better to rely on voltage estimated percentage.
In Maemo Leste we patched upower to use the following formula, which gives  
pretty
good results:

percentage = (voltage - voltage_empty) / (voltage_full - voltage_empty) *  
100

Actually, the formula we use is a bit more complicated.

If you're curious, here's how it's actually done:
https://github.com/maemo-leste/upower/blob/master/src/linux/up-device-supply.c#L756

--
Best regards, Spinal

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

* Re: [PATCH 06/15] power: supply: cpcap-battery: Initialize with user provided data
  2020-03-21 22:08     ` Arthur D.
@ 2020-03-21 22:21       ` Tony Lindgren
  0 siblings, 0 replies; 27+ messages in thread
From: Tony Lindgren @ 2020-03-21 22:21 UTC (permalink / raw)
  To: Arthur D.; +Cc: Merlijn Wajer, Pavel Machek, sre, linux-pm, linux-omap

* Arthur D. <spinal.by@gmail.com> [200321 22:09]:
> Hi.
> 
> > Hmm I'm getting -EINVAL when I do echo 1600000 > charge_now but yet the
> > value does get updated?
> 
> Hm. I'm getting "Permission denied" when trying to do this.
> Anyway, charge_now value is not supposed to be initialized by the user.
> Only charge_full should be writable.

Sorry I meant writing to charge_full :) Anyways, looks like echo -n works
with no errors:

# echo 1305000 > charge_full
bash: echo: write error: Invalid argument
# echo -n 1305000 > charge_full

> > And I'm still not seeing capacity show up after that though even with
> > full battery.. I think we should be able to calculate it if either a
> > high or
> > low value has been seen?
> 
> There are two steps needed to calibrate the battery: to hit the "fully
> charged"
> state and to hit "battery empty" state. When the both states were hit the
> driver
> will initialize charge_full value. And it will start reporting correct
> charge_now
> and capacity (battery charge percentage) values.

Hmm OK. So far no luck triggering that though.

> Once the charge_full value is calculated it's recommended to be saved by the
> user
> to a permanent storage between reboots.
> 
> Saving/restoring the value can be done in init scripts.
> 
> For saving the calibration value just use the command like:
> cat /sys/class/power_supply/battery/charge_full > /battery_cf
> 
> To restore (after device reboot or power on):
> cat /battery_cf > /sys/class/power_supply/battery/charge_full

OK

> This will allow the kernel to do fast calibration. I.e. you will
> only need to fully charge _OR_ fully discharge the battery to start
> seeing correct charge_now and capacity values.

OK

> > Also, we should have the driver default to using the charge_full_design
> > value if nothing is written from userspace and we see a high or low
> > value.
> 
> I'd prefer to have charge_full value undefined until the battery is
> calibrated.
> This way the userspace can estimate current battery capacity using voltage
> as a
> fallback for uncalibrated battery. The voltage estimation will be  way more
> accurate than having charge_full = charge_full_design on pretty old Droid 4
> batteries. For example, my battery after calibration has about 1000 mAh,
> while charge_full_design is 1740000.

OK

> > Maybe some pessimistic estimate could be used instead of just using
> > charge_full_design if no value is initialized from the userspace?
> > Something like (charge_full_design / 4) * 3 maybe?
> 
> It's better to rely on voltage estimated percentage.
> In Maemo Leste we patched upower to use the following formula, which gives
> pretty
> good results:
> 
> percentage = (voltage - voltage_empty) / (voltage_full - voltage_empty) *
> 100
> 
> Actually, the formula we use is a bit more complicated.

OK

> If you're curious, here's how it's actually done:
> https://github.com/maemo-leste/upower/blob/master/src/linux/up-device-supply.c#L756

Thanks, I'll give the calibration another try. Maybe I did not wait low
enough, I think I went down to 3.3V.

Regards,

Tony

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

end of thread, other threads:[~2020-03-21 22:21 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-15 15:11 [PATCH 01/15] power: supply: cpcap-battery: Fix battery full status reporting Arthur Demchenkov
2020-03-15 15:11 ` [PATCH 02/15] power: supply: cpcap-battery: Improve battery full status detection Arthur Demchenkov
2020-03-15 16:46   ` Tony Lindgren
2020-03-15 15:11 ` [PATCH 03/15] power: supply: cpcap-battery: Fix battery low status reporting Arthur Demchenkov
2020-03-21 14:47   ` Tony Lindgren
2020-03-21 21:40     ` Arthur D.
2020-03-15 15:11 ` [PATCH 04/15] power: supply: cpcap-battery: Add charge_full property Arthur Demchenkov
2020-03-15 15:11 ` [PATCH 05/15] power: supply: cpcap-battery: Add charge_now property Arthur Demchenkov
2020-03-15 15:11 ` [PATCH 06/15] power: supply: cpcap-battery: Initialize with user provided data Arthur Demchenkov
2020-03-21 14:54   ` Tony Lindgren
2020-03-21 22:08     ` Arthur D.
2020-03-21 22:21       ` Tony Lindgren
2020-03-15 15:11 ` [PATCH 07/15] power: supply: cpcap-battery: Rewrite capacity reporting Arthur Demchenkov
2020-03-15 15:11 ` [PATCH 08/15] power: supply: cpcap-battery: Get rid of rough capacity percentage Arthur Demchenkov
2020-03-15 15:12 ` [PATCH 09/15] power: supply: cpcap-battery: Increse low voltage bound Arthur Demchenkov
2020-03-15 15:12 ` [PATCH 10/15] power: supply: cpcap-battery: Improve full status reporting Arthur Demchenkov
2020-03-15 15:12 ` [PATCH 11/15] power: supply: cpcap-battery: Rename properties, variables and functions Arthur Demchenkov
2020-03-15 15:12 ` [PATCH 12/15] power: supply: cpcap-battery: stabilize charge_full value Arthur Demchenkov
2020-03-15 15:12 ` [PATCH 13/15] power: supply: cpcap-battery: Fine tune end of charge current Arthur Demchenkov
2020-03-15 15:12 ` [PATCH 14/15] power: supply: cpcap-battery: Make it behave like bq27200 Arthur Demchenkov
2020-03-15 15:12 ` [PATCH 15/15] power: supply: cpcap-battery: Add rounding to capacity reporting Arthur Demchenkov
2020-03-15 18:58 ` [PATCH 01/15] power: supply: cpcap-battery: Fix battery full status reporting Pavel Machek
2020-03-15 19:19   ` Tony Lindgren
2020-03-15 20:51   ` Arthur D.
2020-03-15 21:59     ` Tony Lindgren
2020-03-16  1:30       ` Arthur D.
2020-03-16 16:02         ` Tony Lindgren

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).