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 are cleans up the code a bit, 4-6 fix property value
reading, 7-9 add more information to be read from the chip, 10-11 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.
Michał Mirosław (11):
power: bq25890: remove redundant I2C bus check
power: bq25890: simplify chip name property getter
power: bq25890: make property table const
power: bq25890: protect view of the chip's state
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 | 190 ++++++++++--------
2 files changed, 113 insertions(+), 81 deletions(-)
--
2.20.1
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 87c5832e23d3..9d1ba3bd0bea 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} },
@@ -650,7 +656,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);
@@ -861,11 +869,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
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 e4368d01396a..ad0901fdceb6 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -429,6 +429,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;
@@ -670,6 +682,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,
--
2.20.1
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 ad0901fdceb6..b48685009048 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -497,6 +497,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;
@@ -689,6 +693,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,
};
--
2.20.1
Property list should not change, so mark it const.
Signed-off-by: Michał Mirosław <[email protected]>
---
drivers/power/supply/bq25890_charger.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index f9f29edadddc..c4a69fd69f34 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -668,7 +668,7 @@ static int bq25890_hw_init(struct bq25890_device *bq)
return 0;
}
-static enum power_supply_property bq25890_power_supply_props[] = {
+static const enum power_supply_property bq25890_power_supply_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
POWER_SUPPLY_PROP_MODEL_NAME,
POWER_SUPPLY_PROP_STATUS,
--
2.20.1
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]>
---
drivers/power/supply/bq25890_charger.c | 33 ++++++++++++++++++++++----
1 file changed, 28 insertions(+), 5 deletions(-)
diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index 9339e216651f..3b02fa80aedd 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,40 @@ 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_INPUT_VOLTAGE_NOW:
+ case POWER_SUPPLY_PROP_OUTPUT_VOLTAGE_NOW:
+ 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 +646,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 +989,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 +1005,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
Driver rejects unknown chips early in the probe(), so when
bq25890_power_supply_get_property() is made reachable, bq->chip_version
will already be set to correct value - there is no need to check
it again.
Signed-off-by: Michał Mirosław <[email protected]>
---
drivers/power/supply/bq25890_charger.c | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index c642519ef7b2..f9f29edadddc 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -32,6 +32,13 @@ enum bq25890_chip_version {
BQ25896,
};
+static const char *const bq25890_chip_name[] = {
+ "BQ25890",
+ "BQ25892",
+ "BQ25895",
+ "BQ25896",
+};
+
enum bq25890_fields {
F_EN_HIZ, F_EN_ILIM, F_IILIM, /* Reg00 */
F_BHOT, F_BCOLD, F_VINDPM_OFS, /* Reg01 */
@@ -400,17 +407,7 @@ static int bq25890_power_supply_get_property(struct power_supply *psy,
break;
case POWER_SUPPLY_PROP_MODEL_NAME:
- if (bq->chip_version == BQ25890)
- val->strval = "BQ25890";
- else if (bq->chip_version == BQ25892)
- val->strval = "BQ25892";
- else if (bq->chip_version == BQ25895)
- val->strval = "BQ25895";
- else if (bq->chip_version == BQ25896)
- val->strval = "BQ25896";
- else
- val->strval = "UNKNOWN";
-
+ val->strval = bq25890_chip_name[bq->chip_version];
break;
case POWER_SUPPLY_PROP_ONLINE:
--
2.20.1
Extend bq->lock over whole updating of the chip's state. Might get
useful later for switching ADC modes correctly.
Signed-off-by: Michał Mirosław <[email protected]>
---
drivers/power/supply/bq25890_charger.c | 82 ++++++++------------------
1 file changed, 26 insertions(+), 56 deletions(-)
diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index c4a69fd69f34..9339e216651f 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -510,74 +510,50 @@ static int bq25890_get_chip_state(struct bq25890_device *bq,
return 0;
}
-static bool bq25890_state_changed(struct bq25890_device *bq,
- struct bq25890_state *new_state)
-{
- struct bq25890_state old_state;
-
- mutex_lock(&bq->lock);
- old_state = bq->state;
- mutex_unlock(&bq->lock);
-
- return (old_state.chrg_status != new_state->chrg_status ||
- old_state.chrg_fault != new_state->chrg_fault ||
- old_state.online != new_state->online ||
- old_state.bat_fault != new_state->bat_fault ||
- old_state.boost_fault != new_state->boost_fault ||
- old_state.vsys_status != new_state->vsys_status);
-}
-
-static void bq25890_handle_state_change(struct bq25890_device *bq,
- struct bq25890_state *new_state)
+static irqreturn_t __bq25890_handle_irq(struct bq25890_device *bq)
{
+ struct bq25890_state new_state;
int ret;
- struct bq25890_state old_state;
- mutex_lock(&bq->lock);
- old_state = bq->state;
- mutex_unlock(&bq->lock);
+ ret = bq25890_get_chip_state(bq, &new_state);
+ if (ret < 0)
+ return IRQ_NONE;
- if (!new_state->online) { /* power removed */
+ if (!memcmp(&bq->state, &new_state, sizeof(new_state)))
+ return IRQ_NONE;
+
+ if (!new_state.online && bq->state.online) { /* power removed */
/* disable ADC */
ret = bq25890_field_write(bq, F_CONV_START, 0);
if (ret < 0)
goto error;
- } else if (!old_state.online) { /* power inserted */
+ } else if (new_state.online && !bq->state.online) { /* power inserted */
/* enable ADC, to have control of charge current/voltage */
ret = bq25890_field_write(bq, F_CONV_START, 1);
if (ret < 0)
goto error;
}
- return;
+ bq->state = new_state;
+ power_supply_changed(bq->charger);
+ return IRQ_HANDLED;
error:
- dev_err(bq->dev, "Error communicating with the chip.\n");
+ dev_err(bq->dev, "Error communicating with the chip: %pe\n",
+ ERR_PTR(ret));
+ return IRQ_HANDLED;
}
static irqreturn_t bq25890_irq_handler_thread(int irq, void *private)
{
struct bq25890_device *bq = private;
- int ret;
- struct bq25890_state state;
-
- ret = bq25890_get_chip_state(bq, &state);
- if (ret < 0)
- goto handled;
-
- if (!bq25890_state_changed(bq, &state))
- goto handled;
-
- bq25890_handle_state_change(bq, &state);
+ irqreturn_t ret;
mutex_lock(&bq->lock);
- bq->state = state;
+ ret = __bq25890_handle_irq(bq);
mutex_unlock(&bq->lock);
- power_supply_changed(bq->charger);
-
-handled:
- return IRQ_HANDLED;
+ return ret;
}
static int bq25890_chip_reset(struct bq25890_device *bq)
@@ -607,7 +583,6 @@ static int bq25890_hw_init(struct bq25890_device *bq)
{
int ret;
int i;
- struct bq25890_state state;
const struct {
enum bq25890_fields id;
@@ -655,16 +630,12 @@ static int bq25890_hw_init(struct bq25890_device *bq)
return ret;
}
- ret = bq25890_get_chip_state(bq, &state);
+ ret = bq25890_get_chip_state(bq, &bq->state);
if (ret < 0) {
dev_dbg(bq->dev, "Get state failed %d\n", ret);
return ret;
}
- mutex_lock(&bq->lock);
- bq->state = state;
- mutex_unlock(&bq->lock);
-
return 0;
}
@@ -1001,19 +972,16 @@ static int bq25890_suspend(struct device *dev)
static int bq25890_resume(struct device *dev)
{
int ret;
- struct bq25890_state state;
struct bq25890_device *bq = dev_get_drvdata(dev);
- ret = bq25890_get_chip_state(bq, &state);
+ mutex_lock(&bq->lock);
+
+ ret = bq25890_get_chip_state(bq, &bq->state);
if (ret < 0)
return ret;
- mutex_lock(&bq->lock);
- bq->state = state;
- mutex_unlock(&bq->lock);
-
/* Re-enable ADC only if charger is plugged in. */
- if (state.online) {
+ if (bq->state.online) {
ret = bq25890_field_write(bq, F_CONV_START, 1);
if (ret < 0)
return ret;
@@ -1022,6 +990,8 @@ static int bq25890_resume(struct device *dev)
/* signal userspace, maybe state changed while suspended */
power_supply_changed(bq->charger);
+ mutex_unlock(&bq->lock);
+
return 0;
}
#endif
--
2.20.1
Hi,
On Sun, May 03, 2020 at 05:21:10PM +0200, Michał Mirosław wrote:
> Driver rejects unknown chips early in the probe(), so when
> bq25890_power_supply_get_property() is made reachable, bq->chip_version
> will already be set to correct value - there is no need to check
> it again.
>
> Signed-off-by: Michał Mirosław <[email protected]>
> ---
Thanks, queued.
-- Sebastian
> drivers/power/supply/bq25890_charger.c | 19 ++++++++-----------
> 1 file changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
> index c642519ef7b2..f9f29edadddc 100644
> --- a/drivers/power/supply/bq25890_charger.c
> +++ b/drivers/power/supply/bq25890_charger.c
> @@ -32,6 +32,13 @@ enum bq25890_chip_version {
> BQ25896,
> };
>
> +static const char *const bq25890_chip_name[] = {
> + "BQ25890",
> + "BQ25892",
> + "BQ25895",
> + "BQ25896",
> +};
> +
> enum bq25890_fields {
> F_EN_HIZ, F_EN_ILIM, F_IILIM, /* Reg00 */
> F_BHOT, F_BCOLD, F_VINDPM_OFS, /* Reg01 */
> @@ -400,17 +407,7 @@ static int bq25890_power_supply_get_property(struct power_supply *psy,
> break;
>
> case POWER_SUPPLY_PROP_MODEL_NAME:
> - if (bq->chip_version == BQ25890)
> - val->strval = "BQ25890";
> - else if (bq->chip_version == BQ25892)
> - val->strval = "BQ25892";
> - else if (bq->chip_version == BQ25895)
> - val->strval = "BQ25895";
> - else if (bq->chip_version == BQ25896)
> - val->strval = "BQ25896";
> - else
> - val->strval = "UNKNOWN";
> -
> + val->strval = bq25890_chip_name[bq->chip_version];
> break;
>
> case POWER_SUPPLY_PROP_ONLINE:
> --
> 2.20.1
>
Hi,
On Sun, May 03, 2020 at 05:21:10PM +0200, Michał Mirosław wrote:
> Property list should not change, so mark it const.
>
> Signed-off-by: Michał Mirosław <[email protected]>
> ---
Thanks, queued.
-- Sebastian
> drivers/power/supply/bq25890_charger.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
> index f9f29edadddc..c4a69fd69f34 100644
> --- a/drivers/power/supply/bq25890_charger.c
> +++ b/drivers/power/supply/bq25890_charger.c
> @@ -668,7 +668,7 @@ static int bq25890_hw_init(struct bq25890_device *bq)
> return 0;
> }
>
> -static enum power_supply_property bq25890_power_supply_props[] = {
> +static const enum power_supply_property bq25890_power_supply_props[] = {
> POWER_SUPPLY_PROP_MANUFACTURER,
> POWER_SUPPLY_PROP_MODEL_NAME,
> POWER_SUPPLY_PROP_STATUS,
> --
> 2.20.1
>
Hi,
On Sun, May 03, 2020 at 05:21:11PM +0200, Michał Mirosław wrote:
> 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]>
> ---
> drivers/power/supply/bq25890_charger.c | 33 ++++++++++++++++++++++----
> 1 file changed, 28 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
> index 9339e216651f..3b02fa80aedd 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,40 @@ 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_INPUT_VOLTAGE_NOW:
> + case POWER_SUPPLY_PROP_OUTPUT_VOLTAGE_NOW:
^^^ does not compile without your other patchset, so not applied.
When you send a new version, please Cc some of the recent
contributors to bq25890_charger.c, so that they have a chance
to test the changes with their setup:
Angus Ainslie (Purism) <[email protected]>
Yauhen Kharuzhy <[email protected]>
-- Sebastian
> + 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 +646,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 +989,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 +1005,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
>
Hi,
On Sun, May 03, 2020 at 05:21:11PM +0200, Michał Mirosław wrote:
> Extend bq->lock over whole updating of the chip's state. Might get
> useful later for switching ADC modes correctly.
>
> Signed-off-by: Michał Mirosław <[email protected]>
> ---
Thanks, queued.
-- Sebastian
> drivers/power/supply/bq25890_charger.c | 82 ++++++++------------------
> 1 file changed, 26 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
> index c4a69fd69f34..9339e216651f 100644
> --- a/drivers/power/supply/bq25890_charger.c
> +++ b/drivers/power/supply/bq25890_charger.c
> @@ -510,74 +510,50 @@ static int bq25890_get_chip_state(struct bq25890_device *bq,
> return 0;
> }
>
> -static bool bq25890_state_changed(struct bq25890_device *bq,
> - struct bq25890_state *new_state)
> -{
> - struct bq25890_state old_state;
> -
> - mutex_lock(&bq->lock);
> - old_state = bq->state;
> - mutex_unlock(&bq->lock);
> -
> - return (old_state.chrg_status != new_state->chrg_status ||
> - old_state.chrg_fault != new_state->chrg_fault ||
> - old_state.online != new_state->online ||
> - old_state.bat_fault != new_state->bat_fault ||
> - old_state.boost_fault != new_state->boost_fault ||
> - old_state.vsys_status != new_state->vsys_status);
> -}
> -
> -static void bq25890_handle_state_change(struct bq25890_device *bq,
> - struct bq25890_state *new_state)
> +static irqreturn_t __bq25890_handle_irq(struct bq25890_device *bq)
> {
> + struct bq25890_state new_state;
> int ret;
> - struct bq25890_state old_state;
>
> - mutex_lock(&bq->lock);
> - old_state = bq->state;
> - mutex_unlock(&bq->lock);
> + ret = bq25890_get_chip_state(bq, &new_state);
> + if (ret < 0)
> + return IRQ_NONE;
>
> - if (!new_state->online) { /* power removed */
> + if (!memcmp(&bq->state, &new_state, sizeof(new_state)))
> + return IRQ_NONE;
> +
> + if (!new_state.online && bq->state.online) { /* power removed */
> /* disable ADC */
> ret = bq25890_field_write(bq, F_CONV_START, 0);
> if (ret < 0)
> goto error;
> - } else if (!old_state.online) { /* power inserted */
> + } else if (new_state.online && !bq->state.online) { /* power inserted */
> /* enable ADC, to have control of charge current/voltage */
> ret = bq25890_field_write(bq, F_CONV_START, 1);
> if (ret < 0)
> goto error;
> }
>
> - return;
> + bq->state = new_state;
> + power_supply_changed(bq->charger);
>
> + return IRQ_HANDLED;
> error:
> - dev_err(bq->dev, "Error communicating with the chip.\n");
> + dev_err(bq->dev, "Error communicating with the chip: %pe\n",
> + ERR_PTR(ret));
> + return IRQ_HANDLED;
> }
>
> static irqreturn_t bq25890_irq_handler_thread(int irq, void *private)
> {
> struct bq25890_device *bq = private;
> - int ret;
> - struct bq25890_state state;
> -
> - ret = bq25890_get_chip_state(bq, &state);
> - if (ret < 0)
> - goto handled;
> -
> - if (!bq25890_state_changed(bq, &state))
> - goto handled;
> -
> - bq25890_handle_state_change(bq, &state);
> + irqreturn_t ret;
>
> mutex_lock(&bq->lock);
> - bq->state = state;
> + ret = __bq25890_handle_irq(bq);
> mutex_unlock(&bq->lock);
>
> - power_supply_changed(bq->charger);
> -
> -handled:
> - return IRQ_HANDLED;
> + return ret;
> }
>
> static int bq25890_chip_reset(struct bq25890_device *bq)
> @@ -607,7 +583,6 @@ static int bq25890_hw_init(struct bq25890_device *bq)
> {
> int ret;
> int i;
> - struct bq25890_state state;
>
> const struct {
> enum bq25890_fields id;
> @@ -655,16 +630,12 @@ static int bq25890_hw_init(struct bq25890_device *bq)
> return ret;
> }
>
> - ret = bq25890_get_chip_state(bq, &state);
> + ret = bq25890_get_chip_state(bq, &bq->state);
> if (ret < 0) {
> dev_dbg(bq->dev, "Get state failed %d\n", ret);
> return ret;
> }
>
> - mutex_lock(&bq->lock);
> - bq->state = state;
> - mutex_unlock(&bq->lock);
> -
> return 0;
> }
>
> @@ -1001,19 +972,16 @@ static int bq25890_suspend(struct device *dev)
> static int bq25890_resume(struct device *dev)
> {
> int ret;
> - struct bq25890_state state;
> struct bq25890_device *bq = dev_get_drvdata(dev);
>
> - ret = bq25890_get_chip_state(bq, &state);
> + mutex_lock(&bq->lock);
> +
> + ret = bq25890_get_chip_state(bq, &bq->state);
> if (ret < 0)
> return ret;
>
> - mutex_lock(&bq->lock);
> - bq->state = state;
> - mutex_unlock(&bq->lock);
> -
> /* Re-enable ADC only if charger is plugged in. */
> - if (state.online) {
> + if (bq->state.online) {
> ret = bq25890_field_write(bq, F_CONV_START, 1);
> if (ret < 0)
> return ret;
> @@ -1022,6 +990,8 @@ static int bq25890_resume(struct device *dev)
> /* signal userspace, maybe state changed while suspended */
> power_supply_changed(bq->charger);
>
> + mutex_unlock(&bq->lock);
> +
> return 0;
> }
> #endif
> --
> 2.20.1
>
Hi,
On Sun, May 03, 2020 at 05:21:12PM +0200, Michał Mirosław wrote:
> Report charging type based on recently read state.
>
> Signed-off-by: Michał Mirosław <[email protected]>
> ---
Reviewed-by: Sebastian Reichel <[email protected]>
-- Sebastian
> 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 e4368d01396a..ad0901fdceb6 100644
> --- a/drivers/power/supply/bq25890_charger.c
> +++ b/drivers/power/supply/bq25890_charger.c
> @@ -429,6 +429,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;
> @@ -670,6 +682,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,
> --
> 2.20.1
>
Hi,
On Sun, May 03, 2020 at 05:21:13PM +0200, Michał Mirosław wrote:
> Report configured precharge current.
>
> Signed-off-by: Michał Mirosław <[email protected]>
> ---
Reviewed-by: Sebastian Reichel <[email protected]>
-- Sebastian
> 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 ad0901fdceb6..b48685009048 100644
> --- a/drivers/power/supply/bq25890_charger.c
> +++ b/drivers/power/supply/bq25890_charger.c
> @@ -497,6 +497,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;
> @@ -689,6 +693,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,
> };
> --
> 2.20.1
>