linux-omap.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH v2 1/2] power: supply: cpcap-battery: Add battery type auto detection for mapphone devices
@ 2021-04-24  7:00 carl
  2021-04-24  9:17 ` Carl Philipp Klemm
  0 siblings, 1 reply; 6+ messages in thread
From: carl @ 2021-04-24  7:00 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Pavel Machek, Arthur Demchenkov, Merlijn Wajer,
	Carl Philipp Klemm, linux-omap, Sebastian Reichel

On Apr 24, 2021 08:47, Tony Lindgren <tony@atomide.com> wrote:
>
> Hi, 
>
> * Carl Philipp Klemm <philipp@uvos.xyz> [210423 12:55]: 
> > +static void cpcap_battery_detect_battery_type(struct cpcap_battery_ddata *ddata) 
> > +{ 
> > + struct nvmem_device *nvmem; 
> > + u8 battery_id = 0; 
> > + 
> > + ddata->check_nvmem = false; 
> > + 
> > + nvmem = nvmem_device_find(NULL, &cpcap_battery_match_nvmem); 
> > + if (IS_ERR_OR_NULL(nvmem)) { 
> > + ddata->check_nvmem = true; 
> > + if (!ddata->no_nvmem_warned) { 
> > + dev_info(ddata->dev, "Can not find battery nvmem device. Assuming generic lipo battery\n"); 
> > + ddata->no_nvmem_warned = true; 
> > + } 
>
> Folks are also using the device with no battery at all to have it directly 
> connected to the power supply. This is handy for remotely power cycling 
> the device, and also for measuring power consumption with a bench power 
> supply. So by default I think we should continue assuming no battery is 
> inserted rather than assume a generic battery is inserted. 
>
> How about require configuring the undetected battery parameters via 
> /sys/class/power_supply to indicate a non-standard battery is inserted? 
>
> At least battery type, capacity, and voltage can depend on the generic 
> battery inserted. 
>
> Other than that, the NVRAM changes look nice to me. 
>
> Regards, 
>
> Tony 

The battery inserted property is still based on the presence of a thermistor, so I don't see how this patch changes the bevior with regards to this use case at all except for that info print. Previously the battery information struct was simply set to the values expected from eb41 no matter what.

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

* Re: [PATCH v2 1/2] power: supply: cpcap-battery: Add battery type auto detection for mapphone devices
  2021-04-24  7:00 [PATCH v2 1/2] power: supply: cpcap-battery: Add battery type auto detection for mapphone devices carl
@ 2021-04-24  9:17 ` Carl Philipp Klemm
  2021-04-24 12:59   ` Tony Lindgren
  0 siblings, 1 reply; 6+ messages in thread
From: Carl Philipp Klemm @ 2021-04-24  9:17 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Pavel Machek, Arthur Demchenkov, Merlijn Wajer, linux-omap,
	Sebastian Reichel

> On Apr 24, 2021 08:47, Tony Lindgren <tony@atomide.com> wrote:
> >
> > Hi, 
> >
> > * Carl Philipp Klemm <philipp@uvos.xyz> [210423 12:55]: 
> > > +static void cpcap_battery_detect_battery_type(struct cpcap_battery_ddata *ddata) 
> > > +{ 
> > > + struct nvmem_device *nvmem; 
> > > + u8 battery_id = 0; 
> > > + 
> > > + ddata->check_nvmem = false; 
> > > + 
> > > + nvmem = nvmem_device_find(NULL, &cpcap_battery_match_nvmem); 
> > > + if (IS_ERR_OR_NULL(nvmem)) { 
> > > + ddata->check_nvmem = true; 
> > > + if (!ddata->no_nvmem_warned) { 
> > > + dev_info(ddata->dev, "Can not find battery nvmem device. Assuming generic lipo battery\n"); 
> > > + ddata->no_nvmem_warned = true; 
> > > + } 
> >
> > Folks are also using the device with no battery at all to have it directly 
> > connected to the power supply. This is handy for remotely power cycling 
> > the device, and also for measuring power consumption with a bench power 
> > supply. So by default I think we should continue assuming no battery is 
> > inserted rather than assume a generic battery is inserted. 
> >
> > How about require configuring the undetected battery parameters via 
> > /sys/class/power_supply to indicate a non-standard battery is inserted? 
> >
> > At least battery type, capacity, and voltage can depend on the generic 
> > battery inserted. 
> >
> > Other than that, the NVRAM changes look nice to me. 
> >
> > Regards, 
> >
> > Tony 
> 
> The battery inserted property is still based on the presence of a thermistor, so I don't see how this patch changes the bevior with regards to this use case at all except for that info print. Previously the battery information struct was simply set to the values expected from eb41 no matter what.

Vefore writing this patch i did use the below on my xt875, which dose what you ask. maybe this is something you would like to see included in addition to the above? if so i can submitt it aswell:

[PATCH] power: supply: cpcap-battery: make charge_full_design writeable, so that different/custom batteries can be used.

Especially usfull on XTT875 where both HW4X and BW8X exsist

---
 drivers/power/supply/cpcap-battery.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c
index be8d8b746f24..6465cb1b084c 100644
--- a/drivers/power/supply/cpcap-battery.c
+++ b/drivers/power/supply/cpcap-battery.c
@@ -769,6 +769,13 @@ static int cpcap_battery_set_property(struct power_supply *psy,
 
 		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;
@@ -783,6 +790,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;
-- 
2.31.0

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

* Re: [PATCH v2 1/2] power: supply: cpcap-battery: Add battery type auto detection for mapphone devices
  2021-04-24  9:17 ` Carl Philipp Klemm
@ 2021-04-24 12:59   ` Tony Lindgren
  0 siblings, 0 replies; 6+ messages in thread
From: Tony Lindgren @ 2021-04-24 12:59 UTC (permalink / raw)
  To: Carl Philipp Klemm
  Cc: Pavel Machek, Arthur Demchenkov, Merlijn Wajer, linux-omap,
	Sebastian Reichel

* Carl Philipp Klemm <philipp@uvos.xyz> [210424 09:17]:
> > The battery inserted property is still based on the presence of a thermistor, so I don't see how this patch changes the bevior with regards to this use case at all except for that info print. Previously the battery information struct was simply set to the values expected from eb41 no matter what.

OK sounds like I should test these patches with a power supply.

> Vefore writing this patch i did use the below on my xt875, which dose what you ask. maybe this is something you would like to see included in addition to the above? if so i can submitt it aswell:
> 
> [PATCH] power: supply: cpcap-battery: make charge_full_design writeable, so that different/custom batteries can be used.
> 
> Especially usfull on XTT875 where both HW4X and BW8X exsist

Yeah I think for generic batteries we should require that the battery
properties get initialized. Also the type can be li-ion vs li-po, I think
the charge voltage we already have configurable.

Regards,

Tony

> ---
>  drivers/power/supply/cpcap-battery.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c
> index be8d8b746f24..6465cb1b084c 100644
> --- a/drivers/power/supply/cpcap-battery.c
> +++ b/drivers/power/supply/cpcap-battery.c
> @@ -769,6 +769,13 @@ static int cpcap_battery_set_property(struct power_supply *psy,
>  
>  		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;
> @@ -783,6 +790,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;
> -- 
> 2.31.0

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

* Re: [PATCH v2 1/2] power: supply: cpcap-battery: Add battery type auto detection for mapphone devices
  2021-04-23 12:55 Carl Philipp Klemm
  2021-04-24  6:47 ` Tony Lindgren
@ 2021-06-04 12:54 ` Sebastian Reichel
  1 sibling, 0 replies; 6+ messages in thread
From: Sebastian Reichel @ 2021-06-04 12:54 UTC (permalink / raw)
  To: Carl Philipp Klemm
  Cc: linux-omap, Arthur Demchenkov, Tony Lindgren, Merlijn Wajer,
	Pavel Machek

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

Hi,

On Fri, Apr 23, 2021 at 02:55:43PM +0200, Carl Philipp Klemm wrote:
> This allows cpcap-battery to detect whitch battery is inserted, HW4X or BW8X for
> xt875 and EB41 for xt894 by examining the battery nvmem. If no known battery is
> detected sane defaults are used.
> 
> Signed-off-by: Carl Philipp Klemm <philipp@uvos.xyz>
> ---
>  drivers/power/supply/cpcap-battery.c | 131 ++++++++++++++++++++-------
>  1 file changed, 98 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c
> index 6d5bcdb9f45d..386d269e699f 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/nvmem-consumer.h>

You need to squash PATCH 2/2 into this patch. The kernel tree should
be bisectable and this patch adds the requirement for NVMEM.

>  #include <linux/moduleparam.h>
>  
>  #include <linux/iio/consumer.h>
> @@ -73,6 +74,9 @@
>  
>  #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,
> @@ -97,6 +101,7 @@ struct cpcap_interrupt_desc {
>  
>  struct cpcap_battery_config {
>  	int cd_factor;
> +	char id;

id is unused?

>  	struct power_supply_info info;
>  	struct power_supply_battery_info bat;
>  };
> @@ -138,6 +143,8 @@ struct cpcap_battery_ddata {
>  	int charge_full;
>  	int status;
>  	u16 vendor;
> +	bool check_nvmem;
> +	bool no_nvmem_warned;
>  	unsigned int is_full:1;
>  };
>  
> @@ -354,6 +361,91 @@ cpcap_battery_read_accumulated(struct cpcap_battery_ddata *ddata,
>  				       ccd->offset);
>  }
>  
> +
> +/*
> + * Based on the values from Motorola mapphone Linux kernel for the
> + * stock Droid 4 battery eb41. In the Motorola mapphone Linux
> + * kernel tree the value for pm_cd_factor is passed to the kernel
> + * via device tree. If it turns out to be something device specific
> + * we can consider that too later. These values are also fine for
> + * Bionic's hw4x.
> + *
> + * And looking at the battery full and shutdown values for the stock
> + * 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 = {
> +	.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,
> +};
> +
> +/*
> + * Safe values for any lipo battery likely to fit into a mapphone
> + * battery bay.
> + */
> +static const struct cpcap_battery_config cpcap_battery_unkown_data = {
> +	.cd_factor = 0x3cc,
> +	.info.technology = POWER_SUPPLY_TECHNOLOGY_LION,
> +	.info.voltage_max_design = 4200000,
> +	.info.voltage_min_design = 3200000,
> +	.info.charge_full_design = 3000000,
> +	.bat.constant_charge_voltage_max_uv = 4200000,
> +};
> +
> +static int cpcap_battery_match_nvmem(struct device *dev, const void *data)
> +{
> +	if (strcmp(dev_name(dev), "89-500029ba0f73") == 0)
> +		return 1;
> +	else
> +		return 0;
> +}
> +
> +static void cpcap_battery_detect_battery_type(struct cpcap_battery_ddata *ddata)
> +{
> +	struct nvmem_device *nvmem;
> +	u8 battery_id = 0;
> +
> +	ddata->check_nvmem = false;
> +
> +	nvmem = nvmem_device_find(NULL, &cpcap_battery_match_nvmem);
> +	if (IS_ERR_OR_NULL(nvmem)) {
> +		ddata->check_nvmem = true;
> +		if (!ddata->no_nvmem_warned) {
> +			dev_info(ddata->dev, "Can not find battery nvmem device. Assuming generic lipo battery\n");
> +			ddata->no_nvmem_warned = true;
> +		}

Please use dev_info_once() and drop no_nvmem_warned.

> +	} 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");
> +	}
> +
> +	switch (battery_id) {
> +	case CPCAP_BATTERY_EB41_HW4X_ID:
> +		memcpy(&ddata->config, &cpcap_battery_eb41_data, sizeof(ddata->config));
> +		break;
> +	case CPCAP_BATTERY_BW8X_ID:
> +		memcpy(&ddata->config, &cpcap_battery_bw8x_data, sizeof(ddata->config));
> +		break;
> +	default:
> +		memcpy(&ddata->config, &cpcap_battery_unkown_data, sizeof(ddata->config));
> +	}

No need to use memcpy, you can just assign the struct and get free
type checking, e.g.:

ddata->config = cpcap_battery_eb41_data;

> +}
> +
>  /**
>   * cpcap_battery_cc_get_avg_current - read cpcap coulumb counter
>   * @ddata: cpcap battery driver device data
> @@ -571,6 +663,9 @@ static int cpcap_battery_get_property(struct power_supply *psy,
>  	latest = cpcap_battery_latest(ddata);
>  	previous = cpcap_battery_previous(ddata);
>  
> +	if (ddata->check_nvmem)
> +		cpcap_battery_detect_battery_type(ddata);
> +
>  	switch (psp) {
>  	case POWER_SUPPLY_PROP_PRESENT:
>  		if (latest->temperature > CPCAP_NO_BATTERY || ignore_temperature_probe)
> @@ -969,30 +1064,10 @@ static int cpcap_battery_calibrate(struct cpcap_battery_ddata *ddata)
>  	return error;
>  }
>  
> -/*
> - * Based on the values from Motorola mapphone Linux kernel. In the
> - * the Motorola mapphone Linux kernel tree the value for pm_cd_factor
> - * is passed to the kernel via device tree. If it turns out to be
> - * something device specific we can consider that too later.
> - *
> - * And looking at the battery full and shutdown values for the stock
> - * 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_default_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,
> -};
> -
>  #ifdef CONFIG_OF
>  static const struct of_device_id cpcap_battery_id_table[] = {
>  	{
>  		.compatible = "motorola,cpcap-battery",
> -		.data = &cpcap_battery_default_data,
>  	},
>  	{},
>  };
> @@ -1010,31 +1085,21 @@ static const struct power_supply_desc cpcap_charger_battery_desc = {
>  	.external_power_changed = cpcap_battery_external_power_changed,
>  };
>  
> +

spurious newline.

>  static int cpcap_battery_probe(struct platform_device *pdev)
>  {
>  	struct cpcap_battery_ddata *ddata;
> -	const struct of_device_id *match;
>  	struct power_supply_config psy_cfg = {};
>  	int error;
>  
> -	match = of_match_device(of_match_ptr(cpcap_battery_id_table),
> -				&pdev->dev);
> -	if (!match)
> -		return -EINVAL;
> -
> -	if (!match->data) {
> -		dev_err(&pdev->dev, "no configuration data found\n");
> -
> -		return -ENODEV;
> -	}
> -
>  	ddata = devm_kzalloc(&pdev->dev, sizeof(*ddata), GFP_KERNEL);
>  	if (!ddata)
>  		return -ENOMEM;
>  
> +	cpcap_battery_detect_battery_type(ddata);
> +
>  	INIT_LIST_HEAD(&ddata->irq_list);
>  	ddata->dev = &pdev->dev;
> -	memcpy(&ddata->config, match->data, sizeof(ddata->config));
>  
>  	ddata->reg = dev_get_regmap(ddata->dev->parent, NULL);
>  	if (!ddata->reg)

Otherwise LGTM.

-- Sebastian

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

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

* Re: [PATCH v2 1/2] power: supply: cpcap-battery: Add battery type auto detection for mapphone devices
  2021-04-23 12:55 Carl Philipp Klemm
@ 2021-04-24  6:47 ` Tony Lindgren
  2021-06-04 12:54 ` Sebastian Reichel
  1 sibling, 0 replies; 6+ messages in thread
From: Tony Lindgren @ 2021-04-24  6:47 UTC (permalink / raw)
  To: Carl Philipp Klemm
  Cc: Sebastian Reichel, linux-omap, Arthur Demchenkov, Merlijn Wajer,
	Pavel Machek

Hi,

* Carl Philipp Klemm <philipp@uvos.xyz> [210423 12:55]:
> +static void cpcap_battery_detect_battery_type(struct cpcap_battery_ddata *ddata)
> +{
> +	struct nvmem_device *nvmem;
> +	u8 battery_id = 0;
> +
> +	ddata->check_nvmem = false;
> +
> +	nvmem = nvmem_device_find(NULL, &cpcap_battery_match_nvmem);
> +	if (IS_ERR_OR_NULL(nvmem)) {
> +		ddata->check_nvmem = true;
> +		if (!ddata->no_nvmem_warned) {
> +			dev_info(ddata->dev, "Can not find battery nvmem device. Assuming generic lipo battery\n");
> +			ddata->no_nvmem_warned = true;
> +		}

Folks are also using the device with no battery at all to have it directly
connected to the power supply. This is handy for remotely power cycling
the device, and also for measuring power consumption with a bench power
supply. So by default I think we should continue assuming no battery is
inserted rather than assume a generic battery is inserted.

How about require configuring the undetected battery parameters via
/sys/class/power_supply to indicate a non-standard battery is inserted?

At least battery type, capacity, and voltage can depend on the generic
battery inserted.

Other than that, the NVRAM changes look nice to me.

Regards,

Tony

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

* [PATCH v2 1/2] power: supply: cpcap-battery: Add battery type auto detection for mapphone devices
@ 2021-04-23 12:55 Carl Philipp Klemm
  2021-04-24  6:47 ` Tony Lindgren
  2021-06-04 12:54 ` Sebastian Reichel
  0 siblings, 2 replies; 6+ messages in thread
From: Carl Philipp Klemm @ 2021-04-23 12:55 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: linux-omap, Arthur Demchenkov, Tony Lindgren, Merlijn Wajer,
	Pavel Machek

This allows cpcap-battery to detect whitch battery is inserted, HW4X or BW8X for
xt875 and EB41 for xt894 by examining the battery nvmem. If no known battery is
detected sane defaults are used.

Signed-off-by: Carl Philipp Klemm <philipp@uvos.xyz>
---
 drivers/power/supply/cpcap-battery.c | 131 ++++++++++++++++++++-------
 1 file changed, 98 insertions(+), 33 deletions(-)

diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c
index 6d5bcdb9f45d..386d269e699f 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/nvmem-consumer.h>
 #include <linux/moduleparam.h>
 
 #include <linux/iio/consumer.h>
@@ -73,6 +74,9 @@
 
 #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,
@@ -97,6 +101,7 @@ struct cpcap_interrupt_desc {
 
 struct cpcap_battery_config {
 	int cd_factor;
+	char id;
 	struct power_supply_info info;
 	struct power_supply_battery_info bat;
 };
@@ -138,6 +143,8 @@ struct cpcap_battery_ddata {
 	int charge_full;
 	int status;
 	u16 vendor;
+	bool check_nvmem;
+	bool no_nvmem_warned;
 	unsigned int is_full:1;
 };
 
@@ -354,6 +361,91 @@ cpcap_battery_read_accumulated(struct cpcap_battery_ddata *ddata,
 				       ccd->offset);
 }
 
+
+/*
+ * Based on the values from Motorola mapphone Linux kernel for the
+ * stock Droid 4 battery eb41. In the Motorola mapphone Linux
+ * kernel tree the value for pm_cd_factor is passed to the kernel
+ * via device tree. If it turns out to be something device specific
+ * we can consider that too later. These values are also fine for
+ * Bionic's hw4x.
+ *
+ * And looking at the battery full and shutdown values for the stock
+ * 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 = {
+	.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,
+};
+
+/*
+ * Safe values for any lipo battery likely to fit into a mapphone
+ * battery bay.
+ */
+static const struct cpcap_battery_config cpcap_battery_unkown_data = {
+	.cd_factor = 0x3cc,
+	.info.technology = POWER_SUPPLY_TECHNOLOGY_LION,
+	.info.voltage_max_design = 4200000,
+	.info.voltage_min_design = 3200000,
+	.info.charge_full_design = 3000000,
+	.bat.constant_charge_voltage_max_uv = 4200000,
+};
+
+static int cpcap_battery_match_nvmem(struct device *dev, const void *data)
+{
+	if (strcmp(dev_name(dev), "89-500029ba0f73") == 0)
+		return 1;
+	else
+		return 0;
+}
+
+static void cpcap_battery_detect_battery_type(struct cpcap_battery_ddata *ddata)
+{
+	struct nvmem_device *nvmem;
+	u8 battery_id = 0;
+
+	ddata->check_nvmem = false;
+
+	nvmem = nvmem_device_find(NULL, &cpcap_battery_match_nvmem);
+	if (IS_ERR_OR_NULL(nvmem)) {
+		ddata->check_nvmem = true;
+		if (!ddata->no_nvmem_warned) {
+			dev_info(ddata->dev, "Can not find battery nvmem device. Assuming generic lipo battery\n");
+			ddata->no_nvmem_warned = true;
+		}
+	} 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");
+	}
+
+	switch (battery_id) {
+	case CPCAP_BATTERY_EB41_HW4X_ID:
+		memcpy(&ddata->config, &cpcap_battery_eb41_data, sizeof(ddata->config));
+		break;
+	case CPCAP_BATTERY_BW8X_ID:
+		memcpy(&ddata->config, &cpcap_battery_bw8x_data, sizeof(ddata->config));
+		break;
+	default:
+		memcpy(&ddata->config, &cpcap_battery_unkown_data, sizeof(ddata->config));
+	}
+}
+
 /**
  * cpcap_battery_cc_get_avg_current - read cpcap coulumb counter
  * @ddata: cpcap battery driver device data
@@ -571,6 +663,9 @@ static int cpcap_battery_get_property(struct power_supply *psy,
 	latest = cpcap_battery_latest(ddata);
 	previous = cpcap_battery_previous(ddata);
 
+	if (ddata->check_nvmem)
+		cpcap_battery_detect_battery_type(ddata);
+
 	switch (psp) {
 	case POWER_SUPPLY_PROP_PRESENT:
 		if (latest->temperature > CPCAP_NO_BATTERY || ignore_temperature_probe)
@@ -969,30 +1064,10 @@ static int cpcap_battery_calibrate(struct cpcap_battery_ddata *ddata)
 	return error;
 }
 
-/*
- * Based on the values from Motorola mapphone Linux kernel. In the
- * the Motorola mapphone Linux kernel tree the value for pm_cd_factor
- * is passed to the kernel via device tree. If it turns out to be
- * something device specific we can consider that too later.
- *
- * And looking at the battery full and shutdown values for the stock
- * 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_default_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,
-};
-
 #ifdef CONFIG_OF
 static const struct of_device_id cpcap_battery_id_table[] = {
 	{
 		.compatible = "motorola,cpcap-battery",
-		.data = &cpcap_battery_default_data,
 	},
 	{},
 };
@@ -1010,31 +1085,21 @@ static const struct power_supply_desc cpcap_charger_battery_desc = {
 	.external_power_changed = cpcap_battery_external_power_changed,
 };
 
+
 static int cpcap_battery_probe(struct platform_device *pdev)
 {
 	struct cpcap_battery_ddata *ddata;
-	const struct of_device_id *match;
 	struct power_supply_config psy_cfg = {};
 	int error;
 
-	match = of_match_device(of_match_ptr(cpcap_battery_id_table),
-				&pdev->dev);
-	if (!match)
-		return -EINVAL;
-
-	if (!match->data) {
-		dev_err(&pdev->dev, "no configuration data found\n");
-
-		return -ENODEV;
-	}
-
 	ddata = devm_kzalloc(&pdev->dev, sizeof(*ddata), GFP_KERNEL);
 	if (!ddata)
 		return -ENOMEM;
 
+	cpcap_battery_detect_battery_type(ddata);
+
 	INIT_LIST_HEAD(&ddata->irq_list);
 	ddata->dev = &pdev->dev;
-	memcpy(&ddata->config, match->data, sizeof(ddata->config));
 
 	ddata->reg = dev_get_regmap(ddata->dev->parent, NULL);
 	if (!ddata->reg)
-- 
2.31.0


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

end of thread, other threads:[~2021-06-04 12:54 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-24  7:00 [PATCH v2 1/2] power: supply: cpcap-battery: Add battery type auto detection for mapphone devices carl
2021-04-24  9:17 ` Carl Philipp Klemm
2021-04-24 12:59   ` Tony Lindgren
  -- strict thread matches above, loose matches on Subject: below --
2021-04-23 12:55 Carl Philipp Klemm
2021-04-24  6:47 ` Tony Lindgren
2021-06-04 12:54 ` Sebastian Reichel

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