2020-05-04 19:49:34

by Michał Mirosław

[permalink] [raw]
Subject: [PATCH v3 0/8] power: supply: bq25890: fix and extend

This series consists of a set of fixes and enchancements to bq25890
driver. This is tested on a board using bq25896 as battery controller.

Patches 1-3 fix property value reading, 4-6 add more information to
be read from the chip, 7-8 add IBAT compensation support.

---
v2 removes VBUS and VSYS additions (they need more intrusive changes
to properly fit into power supply class ABI) and adds binding
description to IBAT compensation devicetree properties.

v3 drops cleanup patches already applied and reintroduces a patch
to fix IBAT reading property ID (patch 1)

Michał Mirosław (8):
power: bq25890: use proper CURRENT_NOW property for I_BAT
power: bq25890: fix ADC mode configuration
power: bq25890: update state on property read
power: bq25890: implement CHARGE_TYPE property
power: bq25890: implement PRECHARGE_CURRENT property
power: bq25890: implement INPUT_CURRENT_LIMIT property
power: bq25890: support IBAT compensation
power: bq25890: document IBAT compensation DT properties

.../bindings/power/supply/bq25890.txt | 4 +
drivers/power/supply/bq25890_charger.c | 99 +++++++++++++++----
2 files changed, 86 insertions(+), 17 deletions(-)

--
2.20.1


2020-05-04 19:49:42

by Michał Mirosław

[permalink] [raw]
Subject: [PATCH v3 3/8] power: bq25890: update state on property read

Edge interrupts from the charger may be lost or stuck in fault mode
since probe(). Check if something changed everytime userspace wants
some data.

Signed-off-by: Michał Mirosław <[email protected]>
---
drivers/power/supply/bq25890_charger.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index 325fdd1b1d23..322d48d28fe5 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -387,6 +387,8 @@ static bool bq25890_is_adc_property(enum power_supply_property psp)
}
}

+static irqreturn_t __bq25890_handle_irq(struct bq25890_device *bq);
+
static int bq25890_power_supply_get_property(struct power_supply *psy,
enum power_supply_property psp,
union power_supply_propval *val)
@@ -397,6 +399,8 @@ static int bq25890_power_supply_get_property(struct power_supply *psy,
int ret;

mutex_lock(&bq->lock);
+ /* update state in case we lost an interrupt */
+ __bq25890_handle_irq(bq);
state = bq->state;
do_adc_conv = !state.online && bq25890_is_adc_property(psp);
if (do_adc_conv)
--
2.20.1

2020-05-04 19:49:45

by Michał Mirosław

[permalink] [raw]
Subject: [PATCH v3 8/8] power: bq25890: document IBAT compensation DT properties

Document newly introduced IBAT compensation settings.

Signed-off-by: Michał Mirosław <[email protected]>
---
v2: initial version
---
Documentation/devicetree/bindings/power/supply/bq25890.txt | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/supply/bq25890.txt b/Documentation/devicetree/bindings/power/supply/bq25890.txt
index dc9c8f76e06c..bd214945d9dc 100644
--- a/Documentation/devicetree/bindings/power/supply/bq25890.txt
+++ b/Documentation/devicetree/bindings/power/supply/bq25890.txt
@@ -32,6 +32,10 @@ Optional properties:
- ti,thermal-regulation-threshold: integer, temperature above which the charge
current is lowered, to avoid overheating (in degrees Celsius). If omitted,
the default setting will be used (120 degrees);
+- ti,ibatcomp-resistance: integer, value of a resistor in series with
+ the battery (in uOhm);
+- ti,ibatcomp-clamp-voltage: integer, maximum charging voltage adjustment due
+ to expected voltage drop on in-series resistor (in uV);

Example:

--
2.20.1

2020-05-04 19:49:51

by Michał Mirosław

[permalink] [raw]
Subject: [PATCH v3 7/8] power: bq25890: support IBAT compensation

Add configuration for compensation of IBAT measuring resistor in series
with the battery.

Signed-off-by: Michał Mirosław <[email protected]>
---
drivers/power/supply/bq25890_charger.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index cb57125ea988..80ff767b2422 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -83,6 +83,8 @@ struct bq25890_init_data {
u8 boostf; /* boost frequency */
u8 ilim_en; /* enable ILIM pin */
u8 treg; /* thermal regulation threshold */
+ u8 rbatcomp; /* IBAT sense resistor value */
+ u8 vclamp; /* IBAT compensation voltage limit */
};

struct bq25890_state {
@@ -258,6 +260,8 @@ enum bq25890_table_ids {
TBL_VREG,
TBL_BOOSTV,
TBL_SYSVMIN,
+ TBL_VBATCOMP,
+ TBL_RBATCOMP,

/* lookup tables */
TBL_TREG,
@@ -299,6 +303,8 @@ static const union {
[TBL_VREG] = { .rt = {3840000, 4608000, 16000} }, /* uV */
[TBL_BOOSTV] = { .rt = {4550000, 5510000, 64000} }, /* uV */
[TBL_SYSVMIN] = { .rt = {3000000, 3700000, 100000} }, /* uV */
+ [TBL_VBATCOMP] ={ .rt = {0, 224000, 32000} }, /* uV */
+ [TBL_RBATCOMP] ={ .rt = {0, 140000, 20000} }, /* uOhm */

/* lookup tables */
[TBL_TREG] = { .lt = {bq25890_treg_tbl, BQ25890_TREG_TBL_SIZE} },
@@ -648,7 +654,9 @@ static int bq25890_hw_init(struct bq25890_device *bq)
{F_BOOSTI, bq->init_data.boosti},
{F_BOOSTF, bq->init_data.boostf},
{F_EN_ILIM, bq->init_data.ilim_en},
- {F_TREG, bq->init_data.treg}
+ {F_TREG, bq->init_data.treg},
+ {F_BATCMP, bq->init_data.rbatcomp},
+ {F_VCLAMP, bq->init_data.vclamp},
};

ret = bq25890_chip_reset(bq);
@@ -859,11 +867,14 @@ static int bq25890_fw_read_u32_props(struct bq25890_device *bq)
{"ti,boost-max-current", false, TBL_BOOSTI, &init->boosti},

/* optional properties */
- {"ti,thermal-regulation-threshold", true, TBL_TREG, &init->treg}
+ {"ti,thermal-regulation-threshold", true, TBL_TREG, &init->treg},
+ {"ti,ibatcomp-resistance", true, TBL_RBATCOMP, &init->rbatcomp},
+ {"ti,ibatcomp-clamp-voltage", true, TBL_VBATCOMP, &init->vclamp},
};

/* initialize data for optional properties */
init->treg = 3; /* 120 degrees Celsius */
+ init->rbatcomp = init->vclamp = 0; /* IBAT compensation disabled */

for (i = 0; i < ARRAY_SIZE(props); i++) {
ret = device_property_read_u32(bq->dev, props[i].name,
--
2.20.1

2020-05-04 19:50:43

by Michał Mirosław

[permalink] [raw]
Subject: [PATCH v3 4/8] power: bq25890: implement CHARGE_TYPE property

Report charging type based on recently read state.

Signed-off-by: Michał Mirosław <[email protected]>
---
drivers/power/supply/bq25890_charger.c | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index 322d48d28fe5..02e62ac76e15 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -427,6 +427,18 @@ static int bq25890_power_supply_get_property(struct power_supply *psy,

break;

+ case POWER_SUPPLY_PROP_CHARGE_TYPE:
+ if (!state.online || state.chrg_status == STATUS_NOT_CHARGING ||
+ state.chrg_status == STATUS_TERMINATION_DONE)
+ val->intval = POWER_SUPPLY_CHARGE_TYPE_NONE;
+ else if (state.chrg_status == STATUS_PRE_CHARGING)
+ val->intval = POWER_SUPPLY_CHARGE_TYPE_STANDARD;
+ else if (state.chrg_status == STATUS_FAST_CHARGING)
+ val->intval = POWER_SUPPLY_CHARGE_TYPE_FAST;
+ else /* unreachable */
+ val->intval = POWER_SUPPLY_CHARGE_TYPE_UNKNOWN;
+ break;
+
case POWER_SUPPLY_PROP_MANUFACTURER:
val->strval = BQ25890_MANUFACTURER;
break;
@@ -668,6 +680,7 @@ static const enum power_supply_property bq25890_power_supply_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
POWER_SUPPLY_PROP_MODEL_NAME,
POWER_SUPPLY_PROP_STATUS,
+ POWER_SUPPLY_PROP_CHARGE_TYPE,
POWER_SUPPLY_PROP_ONLINE,
POWER_SUPPLY_PROP_HEALTH,
POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX,
--
2.20.1

2020-05-04 19:51:20

by Michał Mirosław

[permalink] [raw]
Subject: [PATCH v3 5/8] power: bq25890: implement PRECHARGE_CURRENT property

Report configured precharge current.

Signed-off-by: Michał Mirosław <[email protected]>
---
drivers/power/supply/bq25890_charger.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index 02e62ac76e15..dfd7bf9a3a55 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -486,6 +486,10 @@ static int bq25890_power_supply_get_property(struct power_supply *psy,
val->intval = bq25890_find_val(bq->init_data.vreg, TBL_VREG);
break;

+ case POWER_SUPPLY_PROP_PRECHARGE_CURRENT:
+ val->intval = bq25890_find_val(bq->init_data.iprechg, TBL_ITERM);
+ break;
+
case POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT:
val->intval = bq25890_find_val(bq->init_data.iterm, TBL_ITERM);
break;
@@ -686,6 +690,7 @@ static const enum power_supply_property bq25890_power_supply_props[] = {
POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX,
POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX,
+ POWER_SUPPLY_PROP_PRECHARGE_CURRENT,
POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT,
POWER_SUPPLY_PROP_VOLTAGE_NOW,
POWER_SUPPLY_PROP_CURRENT_NOW,
--
2.20.1

2020-05-04 19:52:02

by Michał Mirosław

[permalink] [raw]
Subject: [PATCH v3 2/8] power: bq25890: fix ADC mode configuration

Datasheet describes two modes for reading ADC measurements:
1. continuous, 1 Hz - enabled and started by CONV_RATE bit
2. one-shot - triggered by CONV_START bit

In continuous mode, CONV_START is read-only and signifies an ongoing
conversion.

Change the code to follow the datasheet and really disable continuous
mode for power saving.

Signed-off-by: Michał Mirosław <[email protected]>
---
v3: drop dependency on new input/output properties
---
drivers/power/supply/bq25890_charger.c | 31 +++++++++++++++++++++-----
1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index 70ecc38fe772..325fdd1b1d23 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -126,6 +126,7 @@ static const struct regmap_access_table bq25890_writeable_regs = {

static const struct regmap_range bq25890_volatile_reg_ranges[] = {
regmap_reg_range(0x00, 0x00),
+ regmap_reg_range(0x02, 0x02),
regmap_reg_range(0x09, 0x09),
regmap_reg_range(0x0b, 0x14),
};
@@ -374,18 +375,38 @@ enum bq25890_chrg_fault {
CHRG_FAULT_TIMER_EXPIRED,
};

+static bool bq25890_is_adc_property(enum power_supply_property psp)
+{
+ switch (psp) {
+ case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+ case POWER_SUPPLY_PROP_CURRENT_NOW:
+ return true;
+
+ default:
+ return false;
+ }
+}
+
static int bq25890_power_supply_get_property(struct power_supply *psy,
enum power_supply_property psp,
union power_supply_propval *val)
{
- int ret;
struct bq25890_device *bq = power_supply_get_drvdata(psy);
struct bq25890_state state;
+ bool do_adc_conv;
+ int ret;

mutex_lock(&bq->lock);
state = bq->state;
+ do_adc_conv = !state.online && bq25890_is_adc_property(psp);
+ if (do_adc_conv)
+ bq25890_field_write(bq, F_CONV_START, 1);
mutex_unlock(&bq->lock);

+ if (do_adc_conv)
+ regmap_field_read_poll_timeout(bq->rmap_fields[F_CONV_START],
+ ret, !ret, 25000, 1000000);
+
switch (psp) {
case POWER_SUPPLY_PROP_STATUS:
if (!state.online)
@@ -623,8 +644,8 @@ static int bq25890_hw_init(struct bq25890_device *bq)
}
}

- /* Configure ADC for continuous conversions. This does not enable it. */
- ret = bq25890_field_write(bq, F_CONV_RATE, 1);
+ /* Configure ADC for continuous conversions when charging */
+ ret = bq25890_field_write(bq, F_CONV_RATE, !!bq->state.online);
if (ret < 0) {
dev_dbg(bq->dev, "Config ADC failed %d\n", ret);
return ret;
@@ -966,7 +987,7 @@ static int bq25890_suspend(struct device *dev)
* If charger is removed, while in suspend, make sure ADC is diabled
* since it consumes slightly more power.
*/
- return bq25890_field_write(bq, F_CONV_START, 0);
+ return bq25890_field_write(bq, F_CONV_RATE, 0);
}

static int bq25890_resume(struct device *dev)
@@ -982,7 +1003,7 @@ static int bq25890_resume(struct device *dev)

/* Re-enable ADC only if charger is plugged in. */
if (bq->state.online) {
- ret = bq25890_field_write(bq, F_CONV_START, 1);
+ ret = bq25890_field_write(bq, F_CONV_RATE, 1);
if (ret < 0)
return ret;
}
--
2.20.1

2020-05-04 19:52:21

by Michał Mirosław

[permalink] [raw]
Subject: [PATCH v3 6/8] power: bq25890: implement INPUT_CURRENT_LIMIT property

Report REG00.IINLIM value as INPUT_CURRENT_LIMIT property.

Signed-off-by: Michał Mirosław <[email protected]>
---
drivers/power/supply/bq25890_charger.c | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index dfd7bf9a3a55..cb57125ea988 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -254,6 +254,7 @@ enum bq25890_table_ids {
/* range tables */
TBL_ICHG,
TBL_ITERM,
+ TBL_IILIM,
TBL_VREG,
TBL_BOOSTV,
TBL_SYSVMIN,
@@ -294,6 +295,7 @@ static const union {
/* TODO: BQ25896 has max ICHG 3008 mA */
[TBL_ICHG] = { .rt = {0, 5056000, 64000} }, /* uA */
[TBL_ITERM] = { .rt = {64000, 1024000, 64000} }, /* uA */
+ [TBL_IILIM] = { .rt = {50000, 3200000, 50000} }, /* uA */
[TBL_VREG] = { .rt = {3840000, 4608000, 16000} }, /* uV */
[TBL_BOOSTV] = { .rt = {4550000, 5510000, 64000} }, /* uV */
[TBL_SYSVMIN] = { .rt = {3000000, 3700000, 100000} }, /* uV */
@@ -494,6 +496,14 @@ static int bq25890_power_supply_get_property(struct power_supply *psy,
val->intval = bq25890_find_val(bq->init_data.iterm, TBL_ITERM);
break;

+ case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+ ret = bq25890_field_read(bq, F_IILIM);
+ if (ret < 0)
+ return ret;
+
+ val->intval = bq25890_find_val(ret, TBL_IILIM);
+ break;
+
case POWER_SUPPLY_PROP_VOLTAGE_NOW:
ret = bq25890_field_read(bq, F_SYSV); /* read measured value */
if (ret < 0)
@@ -692,6 +702,7 @@ static const enum power_supply_property bq25890_power_supply_props[] = {
POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX,
POWER_SUPPLY_PROP_PRECHARGE_CURRENT,
POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT,
+ POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
POWER_SUPPLY_PROP_VOLTAGE_NOW,
POWER_SUPPLY_PROP_CURRENT_NOW,
};
--
2.20.1

2020-05-04 19:52:55

by Michał Mirosław

[permalink] [raw]
Subject: [PATCH v3 1/8] power: bq25890: use proper CURRENT_NOW property for I_BAT

Charge Current is more apropriately reflected by CURRENT_NOW property
(measured current) than CONSTANT_CURRENT_VOLTAGE (configured CC-phase
current limit). Fix the reference and make the sign reflect direction
of the current.

Signed-off-by: Michał Mirosław <[email protected]>
---
drivers/power/supply/bq25890_charger.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index 9339e216651f..70ecc38fe772 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -427,15 +427,6 @@ static int bq25890_power_supply_get_property(struct power_supply *psy,
val->intval = POWER_SUPPLY_HEALTH_UNSPEC_FAILURE;
break;

- case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT:
- ret = bq25890_field_read(bq, F_ICHGR); /* read measured value */
- if (ret < 0)
- return ret;
-
- /* converted_val = ADC_val * 50mA (table 10.3.19) */
- val->intval = ret * 50000;
- break;
-
case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX:
val->intval = bq25890_find_val(bq->init_data.ichg, TBL_ICHG);
break;
@@ -471,6 +462,15 @@ static int bq25890_power_supply_get_property(struct power_supply *psy,
val->intval = 2304000 + ret * 20000;
break;

+ case POWER_SUPPLY_PROP_CURRENT_NOW:
+ ret = bq25890_field_read(bq, F_ICHGR); /* read measured value */
+ if (ret < 0)
+ return ret;
+
+ /* converted_val = ADC_val * 50mA (table 10.3.19) */
+ val->intval = ret * -50000;
+ break;
+
default:
return -EINVAL;
}
@@ -645,12 +645,12 @@ static const enum power_supply_property bq25890_power_supply_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_ONLINE,
POWER_SUPPLY_PROP_HEALTH,
- POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT,
POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX,
POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX,
POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT,
POWER_SUPPLY_PROP_VOLTAGE_NOW,
+ POWER_SUPPLY_PROP_CURRENT_NOW,
};

static char *bq25890_charger_supplied_to[] = {
--
2.20.1

2020-05-12 23:02:03

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v3 8/8] power: bq25890: document IBAT compensation DT properties

On Mon, May 04, 2020 at 09:47:48PM +0200, Michał Mirosław wrote:
> Document newly introduced IBAT compensation settings.
>
> Signed-off-by: Michał Mirosław <[email protected]>
> ---
> v2: initial version
> ---
> Documentation/devicetree/bindings/power/supply/bq25890.txt | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/power/supply/bq25890.txt b/Documentation/devicetree/bindings/power/supply/bq25890.txt
> index dc9c8f76e06c..bd214945d9dc 100644
> --- a/Documentation/devicetree/bindings/power/supply/bq25890.txt
> +++ b/Documentation/devicetree/bindings/power/supply/bq25890.txt
> @@ -32,6 +32,10 @@ Optional properties:
> - ti,thermal-regulation-threshold: integer, temperature above which the charge
> current is lowered, to avoid overheating (in degrees Celsius). If omitted,
> the default setting will be used (120 degrees);
> +- ti,ibatcomp-resistance: integer, value of a resistor in series with
> + the battery (in uOhm);
> +- ti,ibatcomp-clamp-voltage: integer, maximum charging voltage adjustment due
> + to expected voltage drop on in-series resistor (in uV);

Need a unit suffix as defined in property-units.txt

Rob