linux-omap.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/1] power: supply: cpcap-battery: improve handling of 3rd party and XT875 batteries.
@ 2020-10-20 20:53 Dev Null
  2020-10-20 21:41 ` Sebastian Reichel
  0 siblings, 1 reply; 3+ messages in thread
From: Dev Null @ 2020-10-20 20:53 UTC (permalink / raw)
  To: sre; +Cc: linux-omap

Adds a module option to ignore a missing temerature sensor.
Invalidates empty->counter_uah and charge_full when charge_now indicates that they are grossly wrong
Makes charge_full_design writeable, so that different/custom batteries can be used.
This is especially usfull on XTT875 where both HW4X and BW8X exsist.

diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c
index 3be76cd7584a..ffa70c31c1cb 100644
--- a/drivers/power/supply/cpcap-battery.c
+++ b/drivers/power/supply/cpcap-battery.c
@@ -28,6 +28,7 @@
 #include <linux/power_supply.h>
 #include <linux/reboot.h>
 #include <linux/regmap.h>
+#include <linux/moduleparam.h>
 
 #include <linux/iio/consumer.h>
 #include <linux/iio/types.h>
@@ -162,6 +163,9 @@ static const struct cpcap_battery_capacity cpcap_battery_cap[] = {
 
 #define CPCAP_NO_BATTERY	-400
 
+static bool ignore_temperature_probe;
+module_param(ignore_temperature_probe, bool, 0660);
+
 static struct cpcap_battery_state_data *
 cpcap_battery_get_state(struct cpcap_battery_ddata *ddata,
 			enum cpcap_battery_state state)
@@ -205,7 +209,8 @@ static int cpcap_charger_battery_temperature(struct cpcap_battery_ddata *ddata,
 	channel = ddata->channels[CPCAP_BATTERY_IIO_BATTDET];
 	error = iio_read_channel_processed(channel, value);
 	if (error < 0) {
-		dev_warn(ddata->dev, "%s failed: %i\n", __func__, error);
+		if (!ignore_temperature_probe)
+			dev_warn(ddata->dev, "%s failed: %i\n", __func__, error);
 		*value = CPCAP_NO_BATTERY;
 
 		return error;
@@ -558,7 +563,7 @@ static int cpcap_battery_get_property(struct power_supply *psy,
 
 	switch (psp) {
 	case POWER_SUPPLY_PROP_PRESENT:
-		if (latest->temperature > CPCAP_NO_BATTERY)
+		if (latest->temperature > CPCAP_NO_BATTERY || ignore_temperature_probe)
 			val->intval = 1;
 		else
 			val->intval = 0;
@@ -641,10 +646,22 @@ 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)
+		if (val->intval < 0) {
+			if (ddata->charge_full && abs(val->intval) > ddata->charge_full/5) {
+				empty->voltage = 0;
+				ddata->charge_full = 0;
+				return -ENODATA;
+			}
 			val->intval = 0;
-		else if (ddata->charge_full && ddata->charge_full < val->intval)
+		}
+		else if (ddata->charge_full && ddata->charge_full < val->intval) {
+			if (val->intval > (6*ddata->charge_full)/5) {
+				empty->voltage = 0;
+				ddata->charge_full = 0;
+				return -ENODATA;
+			}
 			val->intval = ddata->charge_full;
+		}
 		break;
 	case POWER_SUPPLY_PROP_CHARGE_FULL:
 		if (!ddata->charge_full)
@@ -658,6 +675,8 @@ static int cpcap_battery_get_property(struct power_supply *psy,
 		val->intval = POWER_SUPPLY_SCOPE_SYSTEM;
 		break;
 	case POWER_SUPPLY_PROP_TEMP:
+		if (ignore_temperature_probe)
+			return -ENODATA;
 		val->intval = latest->temperature;
 		break;
 	default:
@@ -715,11 +734,18 @@ static int cpcap_battery_set_property(struct power_supply *psy,
 	case POWER_SUPPLY_PROP_CHARGE_FULL:
 		if (val->intval < 0)
 			return -EINVAL;
-		if (val->intval > ddata->config.info.charge_full_design)
+		if (val->intval > (6*ddata->config.info.charge_full_design)/5)
 			return -EINVAL;
 
 		ddata->charge_full = val->intval;
 
+		return 0;
+	case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN:
+		if (val->intval < 0)
+			return -EINVAL;
+
+		ddata->config.info.charge_full_design = val->intval;
+
 		return 0;
 	default:
 		return -EINVAL;
@@ -734,6 +760,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:
+	case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN:
 		return 1;
 	default:
 		return 0;

-- 
Dev Null <devnull@uvos.xyz>

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

* Re: [PATCH 1/1] power: supply: cpcap-battery: improve handling of 3rd party and XT875 batteries.
  2020-10-20 20:53 [PATCH 1/1] power: supply: cpcap-battery: improve handling of 3rd party and XT875 batteries Dev Null
@ 2020-10-20 21:41 ` Sebastian Reichel
  2020-10-21  4:51   ` Tony Lindgren
  0 siblings, 1 reply; 3+ messages in thread
From: Sebastian Reichel @ 2020-10-20 21:41 UTC (permalink / raw)
  To: Dev Null; +Cc: linux-omap, linux-pm, Tony Lindgren

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

Hi,

Please do one change per patch:

On Tue, Oct 20, 2020 at 10:53:12PM +0200, Dev Null wrote:
> Adds a module option to ignore a missing temerature sensor.

first patch

> Invalidates empty->counter_uah and charge_full when charge_now indicates that they are grossly wrong

second patch

> Makes charge_full_design writeable, so that different/custom batteries can be used.

third patch

> This is especially usfull on XTT875 where both HW4X and BW8X exsist.

missing Signed-off-by.

> diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c
> index 3be76cd7584a..ffa70c31c1cb 100644
> --- a/drivers/power/supply/cpcap-battery.c
> +++ b/drivers/power/supply/cpcap-battery.c
> @@ -28,6 +28,7 @@
>  #include <linux/power_supply.h>
>  #include <linux/reboot.h>
>  #include <linux/regmap.h>
> +#include <linux/moduleparam.h>
>  
>  #include <linux/iio/consumer.h>
>  #include <linux/iio/types.h>
> @@ -162,6 +163,9 @@ static const struct cpcap_battery_capacity cpcap_battery_cap[] = {
>  
>  #define CPCAP_NO_BATTERY	-400
>  
> +static bool ignore_temperature_probe;
> +module_param(ignore_temperature_probe, bool, 0660);

Can this be deferred from DT (i.e. always disable temperature probe
on XT875)? Having to specify a module parameter to get a working
system is not very user friendly.

>  static struct cpcap_battery_state_data *
>  cpcap_battery_get_state(struct cpcap_battery_ddata *ddata,
>  			enum cpcap_battery_state state)
> @@ -205,7 +209,8 @@ static int cpcap_charger_battery_temperature(struct cpcap_battery_ddata *ddata,
>  	channel = ddata->channels[CPCAP_BATTERY_IIO_BATTDET];
>  	error = iio_read_channel_processed(channel, value);
>  	if (error < 0) {
> -		dev_warn(ddata->dev, "%s failed: %i\n", __func__, error);
> +		if (!ignore_temperature_probe)
> +			dev_warn(ddata->dev, "%s failed: %i\n", __func__, error);
>  		*value = CPCAP_NO_BATTERY;
>  
>  		return error;
> @@ -558,7 +563,7 @@ static int cpcap_battery_get_property(struct power_supply *psy,
>  
>  	switch (psp) {
>  	case POWER_SUPPLY_PROP_PRESENT:
> -		if (latest->temperature > CPCAP_NO_BATTERY)
> +		if (latest->temperature > CPCAP_NO_BATTERY || ignore_temperature_probe)
>  			val->intval = 1;
>  		else
>  			val->intval = 0;
> @@ -641,10 +646,22 @@ 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)
> +		if (val->intval < 0) {
> +			if (ddata->charge_full && abs(val->intval) > ddata->charge_full/5) {
> +				empty->voltage = 0;
> +				ddata->charge_full = 0;
> +				return -ENODATA;
> +			}
>  			val->intval = 0;
> -		else if (ddata->charge_full && ddata->charge_full < val->intval)
> +		}
> +		else if (ddata->charge_full && ddata->charge_full < val->intval) {
> +			if (val->intval > (6*ddata->charge_full)/5) {
> +				empty->voltage = 0;
> +				ddata->charge_full = 0;
> +				return -ENODATA;
> +			}
>  			val->intval = ddata->charge_full;
> +		}

The context of this patch is not available in cpcap-battery driver
from mainline. So this has dependencies on other patches, which
need to be submitted first. I currently do not have any pending
cpcap patches, but IIRC there was a big patchset which had to be
resubmitted.

>  		break;
>  	case POWER_SUPPLY_PROP_CHARGE_FULL:
>  		if (!ddata->charge_full)
> @@ -658,6 +675,8 @@ static int cpcap_battery_get_property(struct power_supply *psy,
>  		val->intval = POWER_SUPPLY_SCOPE_SYSTEM;
>  		break;
>  	case POWER_SUPPLY_PROP_TEMP:
> +		if (ignore_temperature_probe)
> +			return -ENODATA;
>  		val->intval = latest->temperature;
>  		break;
>  	default:
> @@ -715,11 +734,18 @@ static int cpcap_battery_set_property(struct power_supply *psy,
>  	case POWER_SUPPLY_PROP_CHARGE_FULL:
>  		if (val->intval < 0)
>  			return -EINVAL;
> -		if (val->intval > ddata->config.info.charge_full_design)
> +		if (val->intval > (6*ddata->config.info.charge_full_design)/5)

This deserves a comment. Why do we allow to set charge full to be
above full design capacity?

>  			return -EINVAL;
>  
>  		ddata->charge_full = val->intval;
>  
> +		return 0;
> +	case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN:
> +		if (val->intval < 0)
> +			return -EINVAL;
> +
> +		ddata->config.info.charge_full_design = val->intval;
> +
>  		return 0;
>  	default:
>  		return -EINVAL;
> @@ -734,6 +760,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:
> +	case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN:
>  		return 1;
>  	default:
>  		return 0;
> 
> -- 
> Dev Null <devnull@uvos.xyz>

-- Sebastian

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

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

* Re: [PATCH 1/1] power: supply: cpcap-battery: improve handling of 3rd party and XT875 batteries.
  2020-10-20 21:41 ` Sebastian Reichel
@ 2020-10-21  4:51   ` Tony Lindgren
  0 siblings, 0 replies; 3+ messages in thread
From: Tony Lindgren @ 2020-10-21  4:51 UTC (permalink / raw)
  To: Sebastian Reichel; +Cc: Dev Null, linux-omap, linux-pm

Hi,

* Sebastian Reichel <sre@kernel.org> [201020 21:41]:
> The context of this patch is not available in cpcap-battery driver
> from mainline. So this has dependencies on other patches, which
> need to be submitted first. I currently do not have any pending
> cpcap patches, but IIRC there was a big patchset which had to be
> resubmitted.

Yes this $subject patch as a split up series should be based on
the mainline kernel, and not on the WIP battery capacity patches.
I doubt there will be any severe rebase issues for the capacity
patches.

FYI, I'm slowly working on the battery capacity series but still
need to decouple the battrery full status from charge current as
that won't work for cases where we have CPU load :) We just need
to set the status based on charger full interrupt.

Regards,

Tony

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

end of thread, other threads:[~2020-10-21  4:51 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-20 20:53 [PATCH 1/1] power: supply: cpcap-battery: improve handling of 3rd party and XT875 batteries Dev Null
2020-10-20 21:41 ` Sebastian Reichel
2020-10-21  4:51   ` 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).