[PATCH 0/3] power: supply: cpcap-battery improvements

[PATCH 0/3] power: supply: cpcap-battery improvements

[Ivaylo Dimitrov]

This patch series implements few improvememts to cpcap-battery driver

[PATCH 1/3] power: cpcap-battery: Do not issue low signal too frequently

[Ivaylo Dimitrov]

It seems that low battery irq may be generated tens of times per second,
leading to userspace being flooded with unnecessary events.

Fix that by preventing such events being generated more than once every 30
seconds.

Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
---
 drivers/power/supply/cpcap-battery.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c
index 4676560..2659df7 100644
--- a/drivers/power/supply/cpcap-battery.c
+++ b/drivers/power/supply/cpcap-battery.c
@@ -137,6 +137,7 @@ struct cpcap_battery_ddata {
 	struct power_supply *psy;
 	struct cpcap_battery_config config;
 	struct cpcap_battery_state_data state[CPCAP_BATTERY_STATE_NR];
+	struct timer_list low_timer;
 	u32 cc_lsb;		/* μAms per LSB */
 	atomic_t active;
 	int charge_full;
@@ -914,9 +915,14 @@ static irqreturn_t cpcap_battery_irq_thread(int irq, void *data)
 		dev_info(ddata->dev, "Coulomb counter calibration done\n");
 		break;
 	case CPCAP_BATTERY_IRQ_ACTION_BATTERY_LOW:
-		if (latest->current_ua >= 0)
+		if (latest->current_ua >= 0 &&
+		    !timer_pending(&ddata->low_timer)) {
 			dev_warn(ddata->dev, "Battery low at %imV!\n",
 				latest->voltage / 1000);
+			mod_timer(&ddata->low_timer,
+				  jiffies + msecs_to_jiffies(30000));
+			disable_irq_nosync(d->irq);
+		}
 		break;
 	case CPCAP_BATTERY_IRQ_ACTION_POWEROFF:
 		if (latest->current_ua >= 0 && latest->voltage <= 3200000) {
@@ -1087,6 +1093,19 @@ static int cpcap_battery_calibrate(struct cpcap_battery_ddata *ddata)
 	return error;
 }
 
+static void cpcap_battery_lowbph_enable(struct timer_list *t)
+{
+	struct cpcap_battery_ddata *ddata = from_timer(ddata, t, low_timer);
+	struct cpcap_interrupt_desc *d;
+
+	list_for_each_entry(d, &ddata->irq_list, node) {
+		if (d->action == CPCAP_BATTERY_IRQ_ACTION_BATTERY_LOW)
+			break;
+	}
+
+	enable_irq(d->irq);
+}
+
 #ifdef CONFIG_OF
 static const struct of_device_id cpcap_battery_id_table[] = {
 	{
@@ -1118,6 +1137,8 @@ static int cpcap_battery_probe(struct platform_device *pdev)
 	if (!ddata)
 		return -ENOMEM;
 
+	timer_setup(&ddata->low_timer, cpcap_battery_lowbph_enable, 0);
+
 	cpcap_battery_detect_battery_type(ddata);
 
 	INIT_LIST_HEAD(&ddata->irq_list);
@@ -1185,6 +1206,8 @@ static int cpcap_battery_remove(struct platform_device *pdev)
 	if (error)
 		dev_err(&pdev->dev, "could not disable: %i\n", error);
 
+	del_timer_sync(&ddata->low_timer);
+
 	return 0;
 }
 
-- 
1.9.1

[PATCH 2/3] power: supply: cpcap-battery: Fix battery identification

[Ivaylo Dimitrov]

Use the same logic to identify genuine batteries as Android does.

Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
---
 drivers/power/supply/cpcap-battery.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c
index 2659df7..4cfc325 100644
--- a/drivers/power/supply/cpcap-battery.c
+++ b/drivers/power/supply/cpcap-battery.c
@@ -422,7 +422,7 @@ static int cpcap_battery_cc_to_ua(struct cpcap_battery_ddata *ddata,
 
 static int cpcap_battery_match_nvmem(struct device *dev, const void *data)
 {
-	if (strcmp(dev_name(dev), "89-500029ba0f73") == 0)
+	if (strncmp(dev_name(dev), "89-500", 6) == 0)
 		return 1;
 	else
 		return 0;
@@ -439,10 +439,19 @@ static void cpcap_battery_detect_battery_type(struct cpcap_battery_ddata *ddata)
 	if (IS_ERR_OR_NULL(nvmem)) {
 		ddata->check_nvmem = true;
 		dev_info_once(ddata->dev, "Can not find battery nvmem device. Assuming generic lipo battery\n");
-	} else if (nvmem_device_read(nvmem, 2, 1, &battery_id) < 0) {
-		battery_id = 0;
-		ddata->check_nvmem = true;
-		dev_warn(ddata->dev, "Can not read battery nvmem device. Assuming generic lipo battery\n");
+	} else {
+		char buf[24];
+
+		if (nvmem_device_read(nvmem, 96, 4, buf) < 0 ||
+		    strncmp(buf, "COPR", 4) != 0 ||
+		    nvmem_device_read(nvmem, 104, 24, buf) < 0 ||
+		    strncmp(buf, "MOTOROLA E.P CHARGE ONLY", 24) != 0 ||
+		    nvmem_device_read(nvmem, 2, 1, &battery_id) < 0) {
+			battery_id = 0;
+			ddata->check_nvmem = true;
+			dev_warn(ddata->dev, "Can not read battery nvmem device. Assuming generic lipo battery\n");
+		}
+
 	}
 
 	switch (battery_id) {
-- 
1.9.1

[PATCH 3/3] power: supply: cpcap_battery: Read battery parameters from nvram

[Ivaylo Dimitrov]

Formulas were taken from android blob

Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
---
 drivers/power/supply/cpcap-battery.c | 88 ++++++++++++++++++++----------------
 1 file changed, 48 insertions(+), 40 deletions(-)

diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c
index 4cfc325..5c51685 100644
--- a/drivers/power/supply/cpcap-battery.c
+++ b/drivers/power/supply/cpcap-battery.c
@@ -74,9 +74,6 @@
 
 #define CPCAP_BATTERY_CC_SAMPLE_PERIOD_MS	250
 
-#define CPCAP_BATTERY_EB41_HW4X_ID 0x9E
-#define CPCAP_BATTERY_BW8X_ID 0x98
-
 enum {
 	CPCAP_BATTERY_IIO_BATTDET,
 	CPCAP_BATTERY_IIO_VOLTAGE,
@@ -388,22 +385,9 @@ static int cpcap_battery_cc_to_ua(struct cpcap_battery_ddata *ddata,
  * kernel on droid 4, full is 4351000 and software initiates shutdown
  * at 3078000. The device will die around 2743000.
  */
-static const struct cpcap_battery_config cpcap_battery_eb41_data = {
-	.cd_factor = 0x3cc,
-	.info.technology = POWER_SUPPLY_TECHNOLOGY_LION,
-	.info.voltage_max_design = 4351000,
-	.info.voltage_min_design = 3100000,
-	.info.charge_full_design = 1740000,
-	.bat.constant_charge_voltage_max_uv = 4200000,
-};
-
-/* Values for the extended Droid Bionic battery bw8x. */
-static const struct cpcap_battery_config cpcap_battery_bw8x_data = {
+static struct cpcap_battery_config cpcap_battery_mot_data = {
 	.cd_factor = 0x3cc,
 	.info.technology = POWER_SUPPLY_TECHNOLOGY_LION,
-	.info.voltage_max_design = 4200000,
-	.info.voltage_min_design = 3200000,
-	.info.charge_full_design = 2760000,
 	.bat.constant_charge_voltage_max_uv = 4200000,
 };
 
@@ -431,39 +415,63 @@ static int cpcap_battery_match_nvmem(struct device *dev, const void *data)
 static void cpcap_battery_detect_battery_type(struct cpcap_battery_ddata *ddata)
 {
 	struct nvmem_device *nvmem;
-	u8 battery_id = 0;
+	char buf[24];
+	u8 capacity;
+	u8 mul_idx;
+	u8 charge_voltage;
+	u32 v;
+	static const u32 multipliers[] = {20, 10, 10, 10, 10, 40, 10, 20, 40};
 
 	ddata->check_nvmem = false;
 
 	nvmem = nvmem_device_find(NULL, &cpcap_battery_match_nvmem);
 	if (IS_ERR_OR_NULL(nvmem)) {
-		ddata->check_nvmem = true;
 		dev_info_once(ddata->dev, "Can not find battery nvmem device. Assuming generic lipo battery\n");
-	} else {
-		char buf[24];
-
-		if (nvmem_device_read(nvmem, 96, 4, buf) < 0 ||
-		    strncmp(buf, "COPR", 4) != 0 ||
-		    nvmem_device_read(nvmem, 104, 24, buf) < 0 ||
-		    strncmp(buf, "MOTOROLA E.P CHARGE ONLY", 24) != 0 ||
-		    nvmem_device_read(nvmem, 2, 1, &battery_id) < 0) {
-			battery_id = 0;
-			ddata->check_nvmem = true;
-			dev_warn(ddata->dev, "Can not read battery nvmem device. Assuming generic lipo battery\n");
-		}
+		goto unknown;
+	}
 
+	if (nvmem_device_read(nvmem, 96, 4, buf) < 0 ||
+	    strncmp(buf, "COPR", 4) != 0 ||
+	    nvmem_device_read(nvmem, 104, 24, buf) < 0 ||
+	    strncmp(buf, "MOTOROLA E.P CHARGE ONLY", 24) != 0) {
+		dev_warn(ddata->dev, "Unknown battery nvmem device. Assuming generic lipo battery\n");
+		goto unknown;
 	}
 
-	switch (battery_id) {
-	case CPCAP_BATTERY_EB41_HW4X_ID:
-		ddata->config = cpcap_battery_eb41_data;
-		break;
-	case CPCAP_BATTERY_BW8X_ID:
-		ddata->config = cpcap_battery_bw8x_data;
-		break;
-	default:
-		ddata->config = cpcap_battery_unkown_data;
+	if (nvmem_device_read(nvmem, 49, 1, &mul_idx) < 0 ||
+	    nvmem_device_read(nvmem, 34, 1, &capacity) < 0 ||
+	    nvmem_device_read(nvmem, 65, 1, &charge_voltage) < 0) {
+		dev_warn(ddata->dev, "Can not read battery nvmem device. Assuming generic lipo battery\n");
+		goto unknown;
 	}
+
+	/* design capacity */
+	mul_idx -= 2;
+
+	if (mul_idx < ARRAY_SIZE(multipliers))
+		v = multipliers[mul_idx];
+	else
+		v = 10;
+
+	cpcap_battery_mot_data.info.charge_full_design = 1000 * v * capacity;
+
+	/* design max voltage */
+	v = 1000 * ((16702 * charge_voltage) / 1000 + 1260);
+	cpcap_battery_mot_data.info.voltage_max_design = v;
+
+	/* design min voltage */
+	if (v > 4200000)
+		cpcap_battery_mot_data.info.voltage_min_design = 3100000;
+	else
+		cpcap_battery_mot_data.info.voltage_min_design = 3200000;
+
+	ddata->config = cpcap_battery_mot_data;
+
+	return;
+
+unknown:
+	ddata->check_nvmem = true;
+	ddata->config = cpcap_battery_unkown_data;
 }
 
 /**
-- 
1.9.1

Re: [PATCH 1/3] power: cpcap-battery: Do not issue low signal too frequently

[Ivaylo Dimitrov]

This patch has issues (calling enable_irq() from timer function), will 
send v2 that uses delayed_work.

On 1.11.22 г. 21:53 ч., Ivaylo Dimitrov wrote:
> It seems that low battery irq may be generated tens of times per second,
> leading to userspace being flooded with unnecessary events.
> 
> Fix that by preventing such events being generated more than once every 30
> seconds.
> 
> Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
> ---
>   drivers/power/supply/cpcap-battery.c | 25 ++++++++++++++++++++++++-
>   1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c
> index 4676560..2659df7 100644
> --- a/drivers/power/supply/cpcap-battery.c
> +++ b/drivers/power/supply/cpcap-battery.c
> @@ -137,6 +137,7 @@ struct cpcap_battery_ddata {
>   	struct power_supply *psy;
>   	struct cpcap_battery_config config;
>   	struct cpcap_battery_state_data state[CPCAP_BATTERY_STATE_NR];
> +	struct timer_list low_timer;
>   	u32 cc_lsb;		/* μAms per LSB */
>   	atomic_t active;
>   	int charge_full;
> @@ -914,9 +915,14 @@ static irqreturn_t cpcap_battery_irq_thread(int irq, void *data)
>   		dev_info(ddata->dev, "Coulomb counter calibration done\n");
>   		break;
>   	case CPCAP_BATTERY_IRQ_ACTION_BATTERY_LOW:
> -		if (latest->current_ua >= 0)
> +		if (latest->current_ua >= 0 &&
> +		    !timer_pending(&ddata->low_timer)) {
>   			dev_warn(ddata->dev, "Battery low at %imV!\n",
>   				latest->voltage / 1000);
> +			mod_timer(&ddata->low_timer,
> +				  jiffies + msecs_to_jiffies(30000));
> +			disable_irq_nosync(d->irq);
> +		}
>   		break;
>   	case CPCAP_BATTERY_IRQ_ACTION_POWEROFF:
>   		if (latest->current_ua >= 0 && latest->voltage <= 3200000) {
> @@ -1087,6 +1093,19 @@ static int cpcap_battery_calibrate(struct cpcap_battery_ddata *ddata)
>   	return error;
>   }
>   
> +static void cpcap_battery_lowbph_enable(struct timer_list *t)
> +{
> +	struct cpcap_battery_ddata *ddata = from_timer(ddata, t, low_timer);
> +	struct cpcap_interrupt_desc *d;
> +
> +	list_for_each_entry(d, &ddata->irq_list, node) {
> +		if (d->action == CPCAP_BATTERY_IRQ_ACTION_BATTERY_LOW)
> +			break;
> +	}
> +
> +	enable_irq(d->irq);
> +}
> +
>   #ifdef CONFIG_OF
>   static const struct of_device_id cpcap_battery_id_table[] = {
>   	{
> @@ -1118,6 +1137,8 @@ static int cpcap_battery_probe(struct platform_device *pdev)
>   	if (!ddata)
>   		return -ENOMEM;
>   
> +	timer_setup(&ddata->low_timer, cpcap_battery_lowbph_enable, 0);
> +
>   	cpcap_battery_detect_battery_type(ddata);
>   
>   	INIT_LIST_HEAD(&ddata->irq_list);
> @@ -1185,6 +1206,8 @@ static int cpcap_battery_remove(struct platform_device *pdev)
>   	if (error)
>   		dev_err(&pdev->dev, "could not disable: %i\n", error);
>   
> +	del_timer_sync(&ddata->low_timer);
> +
>   	return 0;
>   }
>   
> 

[PATCH 2/3] power: supply: cpcap-battery: Fix battery identification

[Ivaylo Dimitrov]

Use the same logic to identify genuine batteries as Android does.

Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
---
 drivers/power/supply/cpcap-battery.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c
index 8869067..ca6ee2b 100644
--- a/drivers/power/supply/cpcap-battery.c
+++ b/drivers/power/supply/cpcap-battery.c
@@ -422,7 +422,7 @@ static int cpcap_battery_cc_to_ua(struct cpcap_battery_ddata *ddata,
 
 static int cpcap_battery_match_nvmem(struct device *dev, const void *data)
 {
-	if (strcmp(dev_name(dev), "89-500029ba0f73") == 0)
+	if (strncmp(dev_name(dev), "89-500", 6) == 0)
 		return 1;
 	else
 		return 0;
@@ -439,10 +439,19 @@ static void cpcap_battery_detect_battery_type(struct cpcap_battery_ddata *ddata)
 	if (IS_ERR_OR_NULL(nvmem)) {
 		ddata->check_nvmem = true;
 		dev_info_once(ddata->dev, "Can not find battery nvmem device. Assuming generic lipo battery\n");
-	} else if (nvmem_device_read(nvmem, 2, 1, &battery_id) < 0) {
-		battery_id = 0;
-		ddata->check_nvmem = true;
-		dev_warn(ddata->dev, "Can not read battery nvmem device. Assuming generic lipo battery\n");
+	} else {
+		char buf[24];
+
+		if (nvmem_device_read(nvmem, 96, 4, buf) < 0 ||
+		    strncmp(buf, "COPR", 4) != 0 ||
+		    nvmem_device_read(nvmem, 104, 24, buf) < 0 ||
+		    strncmp(buf, "MOTOROLA E.P CHARGE ONLY", 24) != 0 ||
+		    nvmem_device_read(nvmem, 2, 1, &battery_id) < 0) {
+			battery_id = 0;
+			ddata->check_nvmem = true;
+			dev_warn(ddata->dev, "Can not read battery nvmem device. Assuming generic lipo battery\n");
+		}
+
 	}
 
 	switch (battery_id) {
-- 
1.9.1

Re: [PATCH 2/3] power: supply: cpcap-battery: Fix battery identification

[Sebastian Reichel]

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

Hi,

On Sat, Nov 05, 2022 at 01:25:43PM +0200, Ivaylo Dimitrov wrote:
> Use the same logic to identify genuine batteries as Android does.
> 
> Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
> ---

Why do we care?

-- Sebastian

>  drivers/power/supply/cpcap-battery.c | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c
> index 8869067..ca6ee2b 100644
> --- a/drivers/power/supply/cpcap-battery.c
> +++ b/drivers/power/supply/cpcap-battery.c
> @@ -422,7 +422,7 @@ static int cpcap_battery_cc_to_ua(struct cpcap_battery_ddata *ddata,
>  
>  static int cpcap_battery_match_nvmem(struct device *dev, const void *data)
>  {
> -	if (strcmp(dev_name(dev), "89-500029ba0f73") == 0)
> +	if (strncmp(dev_name(dev), "89-500", 6) == 0)
>  		return 1;
>  	else
>  		return 0;
> @@ -439,10 +439,19 @@ static void cpcap_battery_detect_battery_type(struct cpcap_battery_ddata *ddata)
>  	if (IS_ERR_OR_NULL(nvmem)) {
>  		ddata->check_nvmem = true;
>  		dev_info_once(ddata->dev, "Can not find battery nvmem device. Assuming generic lipo battery\n");
> -	} else if (nvmem_device_read(nvmem, 2, 1, &battery_id) < 0) {
> -		battery_id = 0;
> -		ddata->check_nvmem = true;
> -		dev_warn(ddata->dev, "Can not read battery nvmem device. Assuming generic lipo battery\n");
> +	} else {
> +		char buf[24];
> +
> +		if (nvmem_device_read(nvmem, 96, 4, buf) < 0 ||
> +		    strncmp(buf, "COPR", 4) != 0 ||
> +		    nvmem_device_read(nvmem, 104, 24, buf) < 0 ||
> +		    strncmp(buf, "MOTOROLA E.P CHARGE ONLY", 24) != 0 ||
> +		    nvmem_device_read(nvmem, 2, 1, &battery_id) < 0) {
> +			battery_id = 0;
> +			ddata->check_nvmem = true;
> +			dev_warn(ddata->dev, "Can not read battery nvmem device. Assuming generic lipo battery\n");
> +		}
> +
>  	}
>  
>  	switch (battery_id) {
> -- 
> 1.9.1
> 

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

Re: [PATCH 2/3] power: supply: cpcap-battery: Fix battery identification

[Ivaylo Dimitrov]

Hi,

On 10.11.22 г. 18:05 ч., Sebastian Reichel wrote:
> Hi,
> 
> On Sat, Nov 05, 2022 at 01:25:43PM +0200, Ivaylo Dimitrov wrote:
>> Use the same logic to identify genuine batteries as Android does.
>>
>> Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
>> ---
> 
> Why do we care?
> 

Because if we know the battery is genuine (or at least pretends to be :) 
), then we can read battery parameters from nvram, see patch 3/3. This 
will allow us to charge HV LiPo batteries to 4.35V, using the full capacity.

Not to say the code currently identifies a specific battery with id of 
"89-500029ba0f73" as genuine one, ignoring all others. So this patch is 
more or less a bugfix too.

Regards,
Ivo


> -- Sebastian
> 
>>   drivers/power/supply/cpcap-battery.c | 19 ++++++++++++++-----
>>   1 file changed, 14 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c
>> index 8869067..ca6ee2b 100644
>> --- a/drivers/power/supply/cpcap-battery.c
>> +++ b/drivers/power/supply/cpcap-battery.c
>> @@ -422,7 +422,7 @@ static int cpcap_battery_cc_to_ua(struct cpcap_battery_ddata *ddata,
>>   
>>   static int cpcap_battery_match_nvmem(struct device *dev, const void *data)
>>   {
>> -	if (strcmp(dev_name(dev), "89-500029ba0f73") == 0)
>> +	if (strncmp(dev_name(dev), "89-500", 6) == 0)
>>   		return 1;
>>   	else
>>   		return 0;
>> @@ -439,10 +439,19 @@ static void cpcap_battery_detect_battery_type(struct cpcap_battery_ddata *ddata)
>>   	if (IS_ERR_OR_NULL(nvmem)) {
>>   		ddata->check_nvmem = true;
>>   		dev_info_once(ddata->dev, "Can not find battery nvmem device. Assuming generic lipo battery\n");
>> -	} else if (nvmem_device_read(nvmem, 2, 1, &battery_id) < 0) {
>> -		battery_id = 0;
>> -		ddata->check_nvmem = true;
>> -		dev_warn(ddata->dev, "Can not read battery nvmem device. Assuming generic lipo battery\n");
>> +	} else {
>> +		char buf[24];
>> +
>> +		if (nvmem_device_read(nvmem, 96, 4, buf) < 0 ||
>> +		    strncmp(buf, "COPR", 4) != 0 ||
>> +		    nvmem_device_read(nvmem, 104, 24, buf) < 0 ||
>> +		    strncmp(buf, "MOTOROLA E.P CHARGE ONLY", 24) != 0 ||
>> +		    nvmem_device_read(nvmem, 2, 1, &battery_id) < 0) {
>> +			battery_id = 0;
>> +			ddata->check_nvmem = true;
>> +			dev_warn(ddata->dev, "Can not read battery nvmem device. Assuming generic lipo battery\n");
>> +		}
>> +
>>   	}
>>   
>>   	switch (battery_id) {
>> -- 
>> 1.9.1
>>

Re: [PATCH 2/3] power: supply: cpcap-battery: Fix battery identification

[Tony Lindgren]

Hi,

* Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> [221110 16:40]:
> On 10.11.22 г. 18:05 ч., Sebastian Reichel wrote:
> > Why do we care?
> >
> Because if we know the battery is genuine (or at least pretends to be :) ),
> then we can read battery parameters from nvram, see patch 3/3. This will
> allow us to charge HV LiPo batteries to 4.35V, using the full capacity.

Let's not enable charge voltages above 4.2V automatically at all unless
the user chooses to set a higher charge voltage via sysfs manually.

We have had reports of bloated batteries if left connected to the charger
at higher voltage than 4.2V. This seems to happen after connected for some
weeks or months. AFAIK this happens both with Android and mainline kernel
at higher voltages.

For more information, please see commit d4ee021c410f ("power: supply:
cpcap-charger: Limit voltage to 4.2V for battery").

No objections for using the NVRAM to detect the battery max voltages
though. That is as long as the default charge voltage does not go above
4.2V.

Regards,

Tony

Re: [PATCH 2/3] power: supply: cpcap-battery: Fix battery identification

[Ivaylo Dimitrov]

Hi,

On 15.11.22 г. 15:49 ч., Tony Lindgren wrote:
> Hi,
> 
> * Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> [221110 16:40]:
>> On 10.11.22 г. 18:05 ч., Sebastian Reichel wrote:
>>> Why do we care?
>>>
>> Because if we know the battery is genuine (or at least pretends to be :) ),
>> then we can read battery parameters from nvram, see patch 3/3. This will
>> allow us to charge HV LiPo batteries to 4.35V, using the full capacity.
> 
> Let's not enable charge voltages above 4.2V automatically at all unless
> the user chooses to set a higher charge voltage via sysfs manually.
> 
> We have had reports of bloated batteries if left connected to the charger
> at higher voltage than 4.2V. This seems to happen after connected for some
> weeks or months. AFAIK this happens both with Android and mainline kernel
> at higher voltages.
> 

Not that I sent such patch yet, but still, thinking about it, we should 
be able to easily prevent such damage by not restarting the charging 
after battery is full and voltage has dropped by 50mV or so. There can 
be a threshold (lets say 4.25 or 4.2) above which charging shall not be 
re-enabled unless the user reconnects the charger. Even if default stays 
4.2 and it is the user that has enabled 4.35. Just an idea.

> For more information, please see commit d4ee021c410f ("power: supply:
> cpcap-charger: Limit voltage to 4.2V for battery").
> 
> No objections for using the NVRAM to detect the battery max voltages
> though. That is as long as the default charge voltage does not go above
> 4.2V.

Patch 3/3 does just that - reads battery design parameters from NVRAM.

Regards,
Ivo

> 
> Regards,
> 
> Tony
> 

Re: [PATCH 2/3] power: supply: cpcap-battery: Fix battery identification

[Tony Lindgren]

* Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> [221115 15:31]:
> Hi,
> 
> On 15.11.22 г. 15:49 ч., Tony Lindgren wrote:
> > Hi,
> > 
> > * Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> [221110 16:40]:
> > > On 10.11.22 г. 18:05 ч., Sebastian Reichel wrote:
> > > > Why do we care?
> > > > 
> > > Because if we know the battery is genuine (or at least pretends to be :) ),
> > > then we can read battery parameters from nvram, see patch 3/3. This will
> > > allow us to charge HV LiPo batteries to 4.35V, using the full capacity.
> > 
> > Let's not enable charge voltages above 4.2V automatically at all unless
> > the user chooses to set a higher charge voltage via sysfs manually.
> > 
> > We have had reports of bloated batteries if left connected to the charger
> > at higher voltage than 4.2V. This seems to happen after connected for some
> > weeks or months. AFAIK this happens both with Android and mainline kernel
> > at higher voltages.
> > 
> 
> Not that I sent such patch yet, but still, thinking about it, we should be
> able to easily prevent such damage by not restarting the charging after
> battery is full and voltage has dropped by 50mV or so. There can be a
> threshold (lets say 4.25 or 4.2) above which charging shall not be
> re-enabled unless the user reconnects the charger. Even if default stays 4.2
> and it is the user that has enabled 4.35. Just an idea.

Sure the logic to handle max charge voltage and maintenance charge voltage
could be there. With commit d4ee021c410f we now just wait for the charge
to come down to 4.2V if charged at 4.35V with Android.

We still should not enable higher charge voltages by default though. It
still needs to be enabled by the user via sysfs. It's possible that also
shorter peaks of higher charge voltage accelerate the battery degration.
It just may happen slower than what we've seen earlier. To test this,
multiple devices would need to be left connected to a charger for several
months :)

Regards,

Tony

Re: [PATCH 2/3] power: supply: cpcap-battery: Fix battery identification

[Carl Klemm]

Hi Tony,

We also have a pretty good case having the battery on 4.35V for regular
amounts of time at a time is not damageing:

1. For one thing this corroborated by literature about hvlipos.
2. Personally i used the d4 for manny years with andorid without issue,
giveing the battery manny cycles
3. I think so far we have found very few, if any, devices whos batteris
where replaced by thair previous owners, judgeing by the condition of
the stickers and the battery production dates. Even though maemo leste
has access to manny manny devices.

It is true the hvlipos have a lower cycle lifetime than regular 4.35V
lipos when charged to 4.35 than regular lipos when charged to 4.2V,
however it this effect is not as large as you might think.
It is also true that leaving a Lithium cell of any chemistry on Vmax
for long periods of time siginifcantly accelerates degradion, if this
is sufficant cause to drop the "full" soc a couple of percent is a
debateable and reasonable trade off and would be something we should
then apply to all batteries if chosen, not just hvlipos as it affects
regular lipos just the same.

Regards,

Philipp

On Thu, 2022-11-17 at 06:53 +0200, Tony Lindgren wrote:
> * Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> [221115 15:31]:
> > Hi,
> > 
> > On 15.11.22 г. 15:49 ч., Tony Lindgren wrote:
> > > Hi,
> > > 
> > > * Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> [221110 16:40]:
> > > > On 10.11.22 г. 18:05 ч., Sebastian Reichel wrote:
> > > > > Why do we care?
> > > > > 
> > > > Because if we know the battery is genuine (or at least pretends
> > > > to be :) ),
> > > > then we can read battery parameters from nvram, see patch 3/3.
> > > > This will
> > > > allow us to charge HV LiPo batteries to 4.35V, using the full
> > > > capacity.
> > > 
> > > Let's not enable charge voltages above 4.2V automatically at all
> > > unless
> > > the user chooses to set a higher charge voltage via sysfs
> > > manually.
> > > 
> > > We have had reports of bloated batteries if left connected to the
> > > charger
> > > at higher voltage than 4.2V. This seems to happen after connected
> > > for some
> > > weeks or months. AFAIK this happens both with Android and
> > > mainline kernel
> > > at higher voltages.
> > > 
> > 
> > Not that I sent such patch yet, but still, thinking about it, we
> > should be
> > able to easily prevent such damage by not restarting the charging
> > after
> > battery is full and voltage has dropped by 50mV or so. There can be
> > a
> > threshold (lets say 4.25 or 4.2) above which charging shall not be
> > re-enabled unless the user reconnects the charger. Even if default
> > stays 4.2
> > and it is the user that has enabled 4.35. Just an idea.
> 
> Sure the logic to handle max charge voltage and maintenance charge
> voltage
> could be there. With commit d4ee021c410f we now just wait for the
> charge
> to come down to 4.2V if charged at 4.35V with Android.
> 
> We still should not enable higher charge voltages by default though.
> It
> still needs to be enabled by the user via sysfs. It's possible that
> also
> shorter peaks of higher charge voltage accelerate the battery
> degration.
> It just may happen slower than what we've seen earlier. To test this,
> multiple devices would need to be left connected to a charger for
> several
> months :)
> 
> Regards,
> 
> Tony

Re: [PATCH 2/3] power: supply: cpcap-battery: Fix battery identification

[Tony Lindgren]

* Carl Klemm <carl@uvos.xyz> [221117 08:06]:
> 2. Personally i used the d4 for manny years with andorid without issue,
> giveing the battery manny cycles

Many cycles does not seem to be the issue, the issue seems to be leaving
the device connected to the charger for longer periods of time at higher
charge voltages.

No objection to having the capability there. But enabling it by default
is a different story. We need several folks test connected to a charger
for months with proper Tested-by if we want to enable it by default.

Regards,

Tony