* [PATCH] bq25890_charger.c : add the BQ25896 part
@ 2018-07-23 13:51 Angus Ainslie
2018-07-25 9:58 ` Krzysztof Kozlowski
0 siblings, 1 reply; 32+ messages in thread
From: Angus Ainslie @ 2018-07-23 13:51 UTC (permalink / raw)
To: Sebastian Reichel, linux-pm, linux-kernel; +Cc: Angus Ainslie, Angus Ainslie
Add some debugging to be able to check the proper initialization
of the BQ25896 part.
Enable the BQ25896 part.
Add 2 new parameters "voltage_now" and "model_name".
Signed-off-by: Angus Ainslie <angus.ainslie@puri.sm>
---
drivers/power/supply/bq25890_charger.c | 68 ++++++++++++++++++++++----
1 file changed, 58 insertions(+), 10 deletions(-)
diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index 8e2c41ded171..32cdae15ce40 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -32,6 +32,7 @@
#define BQ25890_IRQ_PIN "bq25890_irq"
#define BQ25890_ID 3
+#define BQ25896_ID 0
enum bq25890_fields {
F_EN_HIZ, F_EN_ILIM, F_IILIM, /* Reg00 */
@@ -153,8 +154,8 @@ static const struct reg_field bq25890_reg_fields[] = {
[F_CONV_RATE] = REG_FIELD(0x02, 6, 6),
[F_BOOSTF] = REG_FIELD(0x02, 5, 5),
[F_ICO_EN] = REG_FIELD(0x02, 4, 4),
- [F_HVDCP_EN] = REG_FIELD(0x02, 3, 3),
- [F_MAXC_EN] = REG_FIELD(0x02, 2, 2),
+ [F_HVDCP_EN] = REG_FIELD(0x02, 3, 3), // reserved on BQ25896
+ [F_MAXC_EN] = REG_FIELD(0x02, 2, 2), // reserved on BQ25896
[F_FORCE_DPM] = REG_FIELD(0x02, 1, 1),
[F_AUTO_DPDM_EN] = REG_FIELD(0x02, 0, 0),
/* REG03 */
@@ -163,6 +164,7 @@ static const struct reg_field bq25890_reg_fields[] = {
[F_OTG_CFG] = REG_FIELD(0x03, 5, 5),
[F_CHG_CFG] = REG_FIELD(0x03, 4, 4),
[F_SYSVMIN] = REG_FIELD(0x03, 1, 3),
+ /* MIN_VBAT_SEL on BQ25896 */
/* REG04 */
[F_PUMPX_EN] = REG_FIELD(0x04, 7, 7),
[F_ICHG] = REG_FIELD(0x04, 0, 6),
@@ -181,7 +183,7 @@ static const struct reg_field bq25890_reg_fields[] = {
[F_CHG_TMR] = REG_FIELD(0x07, 1, 2),
[F_JEITA_ISET] = REG_FIELD(0x07, 0, 0),
/* REG08 */
- [F_BATCMP] = REG_FIELD(0x08, 6, 7),
+ [F_BATCMP] = REG_FIELD(0x08, 6, 7), // 5-7 on BQ25896
[F_VCLAMP] = REG_FIELD(0x08, 2, 4),
[F_TREG] = REG_FIELD(0x08, 0, 1),
/* REG09 */
@@ -195,12 +197,13 @@ static const struct reg_field bq25890_reg_fields[] = {
[F_PUMPX_DN] = REG_FIELD(0x09, 0, 0),
/* REG0A */
[F_BOOSTV] = REG_FIELD(0x0A, 4, 7),
+ /* PFM_OTG_DIS 3 on BQ25896 */
[F_BOOSTI] = REG_FIELD(0x0A, 0, 2),
/* REG0B */
[F_VBUS_STAT] = REG_FIELD(0x0B, 5, 7),
[F_CHG_STAT] = REG_FIELD(0x0B, 3, 4),
[F_PG_STAT] = REG_FIELD(0x0B, 2, 2),
- [F_SDP_STAT] = REG_FIELD(0x0B, 1, 1),
+ [F_SDP_STAT] = REG_FIELD(0x0B, 1, 1), // reserved on BQ25896
[F_VSYS_STAT] = REG_FIELD(0x0B, 0, 0),
/* REG0C */
[F_WD_FAULT] = REG_FIELD(0x0C, 7, 7),
@@ -401,6 +404,18 @@ static int bq25890_power_supply_get_property(struct power_supply *psy,
val->strval = BQ25890_MANUFACTURER;
break;
+ case POWER_SUPPLY_PROP_MODEL_NAME:
+ bq->chip_id = bq25890_field_read(bq, F_PN);
+
+ if (bq->chip_id == BQ25890_ID)
+ val->strval = "BQ25890";
+ else if (bq->chip_id == BQ25896_ID)
+ val->strval = "BQ25896";
+ else
+ val->strval = "UNKNOWN";
+
+ break;
+
case POWER_SUPPLY_PROP_ONLINE:
val->intval = state.online;
break;
@@ -453,6 +468,20 @@ 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_VOLTAGE_NOW:
+ if (!state.online) {
+ val->intval = 0;
+ break;
+ }
+
+ ret = bq25890_field_read(bq, F_SYSV); /* read measured value */
+ if (ret < 0)
+ return ret;
+
+ /* converted_val = 2.304V + ADC_val * 20mV (table 10.3.15) */
+ val->intval = 2304000 + ret * 20000;
+ break;
+
default:
return -EINVAL;
}
@@ -608,30 +637,43 @@ static int bq25890_hw_init(struct bq25890_device *bq)
};
ret = bq25890_chip_reset(bq);
- if (ret < 0)
+ if (ret < 0) {
+ dev_dbg(bq->dev, "BQ259x reset failed %d\n", ret);
return ret;
+ }
/* disable watchdog */
ret = bq25890_field_write(bq, F_WD, 0);
- if (ret < 0)
+ if (ret < 0) {
+ dev_dbg(bq->dev, "disabling watchdog failed %d\n", ret );
return ret;
+ }
/* initialize currents/voltages and other parameters */
for (i = 0; i < ARRAY_SIZE(init_data); i++) {
ret = bq25890_field_write(bq, init_data[i].id,
init_data[i].value);
- if (ret < 0)
+ if (ret < 0) {
+ dev_dbg(bq->dev, "writing init data failed %d\n",
+ ret);
return ret;
+ }
}
/* Configure ADC for continuous conversions. This does not enable it. */
ret = bq25890_field_write(bq, F_CONV_RATE, 1);
- if (ret < 0)
+ if (ret < 0) {
+ dev_dbg(bq->dev, "Config ADC failed %d\n",
+ ret);
return ret;
+ }
ret = bq25890_get_chip_state(bq, &state);
- if (ret < 0)
+ if (ret < 0) {
+ dev_dbg(bq->dev, "BQ2589x get state failed %d\n",
+ ret);
return ret;
+ }
mutex_lock(&bq->lock);
bq->state = state;
@@ -644,12 +686,14 @@ static enum power_supply_property bq25890_power_supply_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_ONLINE,
+ POWER_SUPPLY_PROP_MODEL_NAME,
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,
};
static char *bq25890_charger_supplied_to[] = {
@@ -767,6 +811,9 @@ static int bq25890_fw_read_u32_props(struct bq25890_device *bq)
if (props[i].optional)
continue;
+ printk( KERN_ERR "unable to read property %d %s\n", ret
+ , props[i].name );
+
return ret;
}
@@ -840,7 +887,7 @@ static int bq25890_probe(struct i2c_client *client,
return bq->chip_id;
}
- if (bq->chip_id != BQ25890_ID) {
+ if ((bq->chip_id != BQ25890_ID) && (bq->chip_id != BQ25896_ID)) {
dev_err(dev, "Chip with ID=%d, not supported!\n", bq->chip_id);
return -ENODEV;
}
@@ -966,6 +1013,7 @@ MODULE_DEVICE_TABLE(i2c, bq25890_i2c_ids);
static const struct of_device_id bq25890_of_match[] = {
{ .compatible = "ti,bq25890", },
+ { .compatible = "ti,bq25896", },
{ },
};
MODULE_DEVICE_TABLE(of, bq25890_of_match);
--
2.17.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH] bq25890_charger.c : add the BQ25896 part
2018-07-23 13:51 [PATCH] bq25890_charger.c : add the BQ25896 part Angus Ainslie
@ 2018-07-25 9:58 ` Krzysztof Kozlowski
2018-07-25 12:17 ` Angus Ainslie
` (3 more replies)
0 siblings, 4 replies; 32+ messages in thread
From: Krzysztof Kozlowski @ 2018-07-25 9:58 UTC (permalink / raw)
To: Angus Ainslie; +Cc: Sebastian Reichel, linux-pm, linux-kernel, Angus Ainslie
On 23 July 2018 at 15:51, Angus Ainslie <angus@akkea.ca> wrote:
> Add some debugging to be able to check the proper initialization
> of the BQ25896 part.
Hi,
This should be split into separate patchset. Do not mix two features
in one commit.
> Enable the BQ25896 part.
>
> Add 2 new parameters "voltage_now" and "model_name".
>
> Signed-off-by: Angus Ainslie <angus.ainslie@puri.sm>
Your signed-off-by does not match From address.
> ---
> drivers/power/supply/bq25890_charger.c | 68 ++++++++++++++++++++++----
> 1 file changed, 58 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
> index 8e2c41ded171..32cdae15ce40 100644
> --- a/drivers/power/supply/bq25890_charger.c
> +++ b/drivers/power/supply/bq25890_charger.c
> @@ -32,6 +32,7 @@
> #define BQ25890_IRQ_PIN "bq25890_irq"
>
> #define BQ25890_ID 3
> +#define BQ25896_ID 0
>
> enum bq25890_fields {
> F_EN_HIZ, F_EN_ILIM, F_IILIM, /* Reg00 */
> @@ -153,8 +154,8 @@ static const struct reg_field bq25890_reg_fields[] = {
> [F_CONV_RATE] = REG_FIELD(0x02, 6, 6),
> [F_BOOSTF] = REG_FIELD(0x02, 5, 5),
> [F_ICO_EN] = REG_FIELD(0x02, 4, 4),
> - [F_HVDCP_EN] = REG_FIELD(0x02, 3, 3),
> - [F_MAXC_EN] = REG_FIELD(0x02, 2, 2),
> + [F_HVDCP_EN] = REG_FIELD(0x02, 3, 3), // reserved on BQ25896
> + [F_MAXC_EN] = REG_FIELD(0x02, 2, 2), // reserved on BQ25896
> [F_FORCE_DPM] = REG_FIELD(0x02, 1, 1),
> [F_AUTO_DPDM_EN] = REG_FIELD(0x02, 0, 0),
> /* REG03 */
> @@ -163,6 +164,7 @@ static const struct reg_field bq25890_reg_fields[] = {
> [F_OTG_CFG] = REG_FIELD(0x03, 5, 5),
> [F_CHG_CFG] = REG_FIELD(0x03, 4, 4),
> [F_SYSVMIN] = REG_FIELD(0x03, 1, 3),
> + /* MIN_VBAT_SEL on BQ25896 */
> /* REG04 */
> [F_PUMPX_EN] = REG_FIELD(0x04, 7, 7),
> [F_ICHG] = REG_FIELD(0x04, 0, 6),
> @@ -181,7 +183,7 @@ static const struct reg_field bq25890_reg_fields[] = {
> [F_CHG_TMR] = REG_FIELD(0x07, 1, 2),
> [F_JEITA_ISET] = REG_FIELD(0x07, 0, 0),
> /* REG08 */
> - [F_BATCMP] = REG_FIELD(0x08, 6, 7),
> + [F_BATCMP] = REG_FIELD(0x08, 6, 7), // 5-7 on BQ25896
So this field is different on BQ25896...
> [F_VCLAMP] = REG_FIELD(0x08, 2, 4),
> [F_TREG] = REG_FIELD(0x08, 0, 1),
> /* REG09 */
> @@ -195,12 +197,13 @@ static const struct reg_field bq25890_reg_fields[] = {
> [F_PUMPX_DN] = REG_FIELD(0x09, 0, 0),
> /* REG0A */
> [F_BOOSTV] = REG_FIELD(0x0A, 4, 7),
> + /* PFM_OTG_DIS 3 on BQ25896 */
> [F_BOOSTI] = REG_FIELD(0x0A, 0, 2),
> /* REG0B */
> [F_VBUS_STAT] = REG_FIELD(0x0B, 5, 7),
> [F_CHG_STAT] = REG_FIELD(0x0B, 3, 4),
> [F_PG_STAT] = REG_FIELD(0x0B, 2, 2),
> - [F_SDP_STAT] = REG_FIELD(0x0B, 1, 1),
> + [F_SDP_STAT] = REG_FIELD(0x0B, 1, 1), // reserved on BQ25896
> [F_VSYS_STAT] = REG_FIELD(0x0B, 0, 0),
> /* REG0C */
> [F_WD_FAULT] = REG_FIELD(0x0C, 7, 7),
> @@ -401,6 +404,18 @@ static int bq25890_power_supply_get_property(struct power_supply *psy,
> val->strval = BQ25890_MANUFACTURER;
> break;
>
> + case POWER_SUPPLY_PROP_MODEL_NAME:
> + bq->chip_id = bq25890_field_read(bq, F_PN);
Why do you need to read it again?
> +
> + if (bq->chip_id == BQ25890_ID)
> + val->strval = "BQ25890";
> + else if (bq->chip_id == BQ25896_ID)
> + val->strval = "BQ25896";
> + else
> + val->strval = "UNKNOWN";
> +
> + break;
> +
> case POWER_SUPPLY_PROP_ONLINE:
> val->intval = state.online;
> break;
> @@ -453,6 +468,20 @@ 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_VOLTAGE_NOW:
> + if (!state.online) {
> + val->intval = 0;
> + break;
> + }
This looks like unrelated change. Please split it.
> +
> + ret = bq25890_field_read(bq, F_SYSV); /* read measured value */
> + if (ret < 0)
> + return ret;
> +
> + /* converted_val = 2.304V + ADC_val * 20mV (table 10.3.15) */
> + val->intval = 2304000 + ret * 20000;
> + break;
> +
> default:
> return -EINVAL;
> }
> @@ -608,30 +637,43 @@ static int bq25890_hw_init(struct bq25890_device *bq)
> };
>
> ret = bq25890_chip_reset(bq);
> - if (ret < 0)
> + if (ret < 0) {
> + dev_dbg(bq->dev, "BQ259x reset failed %d\n", ret);
"BQ259x" prefix is not needed.
> return ret;
> + }
>
> /* disable watchdog */
> ret = bq25890_field_write(bq, F_WD, 0);
> - if (ret < 0)
> + if (ret < 0) {
> + dev_dbg(bq->dev, "disabling watchdog failed %d\n", ret );
1. Run checkpatch to point coding style issues and fix them.
2. Start string with capital letter.
> return ret;
> + }
>
> /* initialize currents/voltages and other parameters */
> for (i = 0; i < ARRAY_SIZE(init_data); i++) {
> ret = bq25890_field_write(bq, init_data[i].id,
> init_data[i].value);
> - if (ret < 0)
> + if (ret < 0) {
> + dev_dbg(bq->dev, "writing init data failed %d\n",
> + ret);
Start string with capital letter.
> return ret;
> + }
> }
>
> /* Configure ADC for continuous conversions. This does not enable it. */
> ret = bq25890_field_write(bq, F_CONV_RATE, 1);
> - if (ret < 0)
> + if (ret < 0) {
> + dev_dbg(bq->dev, "Config ADC failed %d\n",
> + ret);
Line break looks unneeded. Run checkpatch.
> return ret;
> + }
>
> ret = bq25890_get_chip_state(bq, &state);
> - if (ret < 0)
> + if (ret < 0) {
> + dev_dbg(bq->dev, "BQ2589x get state failed %d\n",
> + ret);
Prefix is not needed.
> return ret;
> + }
>
> mutex_lock(&bq->lock);
> bq->state = state;
> @@ -644,12 +686,14 @@ static enum power_supply_property bq25890_power_supply_props[] = {
> POWER_SUPPLY_PROP_MANUFACTURER,
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_ONLINE,
> + POWER_SUPPLY_PROP_MODEL_NAME,
> 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,
> };
>
> static char *bq25890_charger_supplied_to[] = {
> @@ -767,6 +811,9 @@ static int bq25890_fw_read_u32_props(struct bq25890_device *bq)
> if (props[i].optional)
> continue;
>
> + printk( KERN_ERR "unable to read property %d %s\n", ret
> + , props[i].name );
Remove it. And again - run checkpatch.
> +
> return ret;
> }
>
> @@ -840,7 +887,7 @@ static int bq25890_probe(struct i2c_client *client,
> return bq->chip_id;
> }
>
> - if (bq->chip_id != BQ25890_ID) {
> + if ((bq->chip_id != BQ25890_ID) && (bq->chip_id != BQ25896_ID)) {
> dev_err(dev, "Chip with ID=%d, not supported!\n", bq->chip_id);
> return -ENODEV;
> }
> @@ -966,6 +1013,7 @@ MODULE_DEVICE_TABLE(i2c, bq25890_i2c_ids);
>
> static const struct of_device_id bq25890_of_match[] = {
> { .compatible = "ti,bq25890", },
> + { .compatible = "ti,bq25896", },
Document the compatible. If you run checkpatch.... it would point it.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] bq25890_charger.c : add the BQ25896 part
2018-07-25 9:58 ` Krzysztof Kozlowski
@ 2018-07-25 12:17 ` Angus Ainslie
2018-07-26 6:37 ` Krzysztof Kozlowski
2018-07-25 19:46 ` [PATCH v2 0/4] Add the BQ25896 charger IC Angus Ainslie (Purism)
` (2 subsequent siblings)
3 siblings, 1 reply; 32+ messages in thread
From: Angus Ainslie @ 2018-07-25 12:17 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Sebastian Reichel, linux-pm, linux-kernel, Angus Ainslie
Hi Krzysztof,
On 2018-07-25 03:58, Krzysztof Kozlowski wrote:
> On 23 July 2018 at 15:51, Angus Ainslie <angus@akkea.ca> wrote:
>> Add some debugging to be able to check the proper initialization
>> of the BQ25896 part.
>
> Hi,
>
> This should be split into separate patchset. Do not mix two features
> in one commit.
>
Ok, I'll take it apart
>> Enable the BQ25896 part.
>>
>> Add 2 new parameters "voltage_now" and "model_name".
>>
>> Signed-off-by: Angus Ainslie <angus.ainslie@puri.sm>
>
> Your signed-off-by does not match From address.
>
That was intentional as I wanted Purism to get credit for it. I'm
guessing that's not the correct way of doing it.
>> ---
>> drivers/power/supply/bq25890_charger.c | 68
>> ++++++++++++++++++++++----
>> 1 file changed, 58 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/power/supply/bq25890_charger.c
>> b/drivers/power/supply/bq25890_charger.c
>> index 8e2c41ded171..32cdae15ce40 100644
>> --- a/drivers/power/supply/bq25890_charger.c
>> +++ b/drivers/power/supply/bq25890_charger.c
>> @@ -32,6 +32,7 @@
>> #define BQ25890_IRQ_PIN "bq25890_irq"
>>
>> #define BQ25890_ID 3
>> +#define BQ25896_ID 0
>>
>> enum bq25890_fields {
>> F_EN_HIZ, F_EN_ILIM, F_IILIM,
>> /* Reg00 */
>> @@ -153,8 +154,8 @@ static const struct reg_field bq25890_reg_fields[]
>> = {
>> [F_CONV_RATE] = REG_FIELD(0x02, 6, 6),
>> [F_BOOSTF] = REG_FIELD(0x02, 5, 5),
>> [F_ICO_EN] = REG_FIELD(0x02, 4, 4),
>> - [F_HVDCP_EN] = REG_FIELD(0x02, 3, 3),
>> - [F_MAXC_EN] = REG_FIELD(0x02, 2, 2),
>> + [F_HVDCP_EN] = REG_FIELD(0x02, 3, 3), // reserved
>> on BQ25896
>> + [F_MAXC_EN] = REG_FIELD(0x02, 2, 2), // reserved
>> on BQ25896
>> [F_FORCE_DPM] = REG_FIELD(0x02, 1, 1),
>> [F_AUTO_DPDM_EN] = REG_FIELD(0x02, 0, 0),
>> /* REG03 */
>> @@ -163,6 +164,7 @@ static const struct reg_field bq25890_reg_fields[]
>> = {
>> [F_OTG_CFG] = REG_FIELD(0x03, 5, 5),
>> [F_CHG_CFG] = REG_FIELD(0x03, 4, 4),
>> [F_SYSVMIN] = REG_FIELD(0x03, 1, 3),
>> + /* MIN_VBAT_SEL on BQ25896 */
>> /* REG04 */
>> [F_PUMPX_EN] = REG_FIELD(0x04, 7, 7),
>> [F_ICHG] = REG_FIELD(0x04, 0, 6),
>> @@ -181,7 +183,7 @@ static const struct reg_field bq25890_reg_fields[]
>> = {
>> [F_CHG_TMR] = REG_FIELD(0x07, 1, 2),
>> [F_JEITA_ISET] = REG_FIELD(0x07, 0, 0),
>> /* REG08 */
>> - [F_BATCMP] = REG_FIELD(0x08, 6, 7),
>> + [F_BATCMP] = REG_FIELD(0x08, 6, 7), // 5-7 on
>> BQ25896
>
> So this field is different on BQ25896...
>
The BATCMP field is not currently used by the driver but it is reference
by "bq25890_tables". Should I remove that reference or make a special
case for something that isn't used ?
>> [F_VCLAMP] = REG_FIELD(0x08, 2, 4),
>> [F_TREG] = REG_FIELD(0x08, 0, 1),
>> /* REG09 */
>> @@ -195,12 +197,13 @@ static const struct reg_field
>> bq25890_reg_fields[] = {
>> [F_PUMPX_DN] = REG_FIELD(0x09, 0, 0),
>> /* REG0A */
>> [F_BOOSTV] = REG_FIELD(0x0A, 4, 7),
>> + /* PFM_OTG_DIS 3 on BQ25896 */
>> [F_BOOSTI] = REG_FIELD(0x0A, 0, 2),
>> /* REG0B */
>> [F_VBUS_STAT] = REG_FIELD(0x0B, 5, 7),
>> [F_CHG_STAT] = REG_FIELD(0x0B, 3, 4),
>> [F_PG_STAT] = REG_FIELD(0x0B, 2, 2),
>> - [F_SDP_STAT] = REG_FIELD(0x0B, 1, 1),
>> + [F_SDP_STAT] = REG_FIELD(0x0B, 1, 1), // reserved
>> on BQ25896
>> [F_VSYS_STAT] = REG_FIELD(0x0B, 0, 0),
>> /* REG0C */
>> [F_WD_FAULT] = REG_FIELD(0x0C, 7, 7),
>> @@ -401,6 +404,18 @@ static int
>> bq25890_power_supply_get_property(struct power_supply *psy,
>> val->strval = BQ25890_MANUFACTURER;
>> break;
>>
>> + case POWER_SUPPLY_PROP_MODEL_NAME:
>> + bq->chip_id = bq25890_field_read(bq, F_PN);
>
> Why do you need to read it again?
>
Right, it should be set in probe.
>> +
>> + if (bq->chip_id == BQ25890_ID)
>> + val->strval = "BQ25890";
>> + else if (bq->chip_id == BQ25896_ID)
>> + val->strval = "BQ25896";
>> + else
>> + val->strval = "UNKNOWN";
>> +
>> + break;
>> +
>> case POWER_SUPPLY_PROP_ONLINE:
>> val->intval = state.online;
>> break;
>> @@ -453,6 +468,20 @@ 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_VOLTAGE_NOW:
>> + if (!state.online) {
>> + val->intval = 0;
>> + break;
>> + }
>
> This looks like unrelated change. Please split it.
>
Ok
>> +
>> + ret = bq25890_field_read(bq, F_SYSV); /* read measured
>> value */
>> + if (ret < 0)
>> + return ret;
>> +
>> + /* converted_val = 2.304V + ADC_val * 20mV (table
>> 10.3.15) */
>> + val->intval = 2304000 + ret * 20000;
>> + break;
>> +
>> default:
>> return -EINVAL;
>> }
>> @@ -608,30 +637,43 @@ static int bq25890_hw_init(struct bq25890_device
>> *bq)
>> };
>>
>> ret = bq25890_chip_reset(bq);
>> - if (ret < 0)
>> + if (ret < 0) {
>> + dev_dbg(bq->dev, "BQ259x reset failed %d\n", ret);
>
> "BQ259x" prefix is not needed.
>
Ok
[snip]
>
> Document the compatible. If you run checkpatch.... it would point it.
Sorry, I'll fix all the checkpatch errors.
>
> Best regards,
> Krzysztof
Cheers
Angus
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 0/4] Add the BQ25896 charger IC
2018-07-25 9:58 ` Krzysztof Kozlowski
2018-07-25 12:17 ` Angus Ainslie
@ 2018-07-25 19:46 ` Angus Ainslie (Purism)
2018-07-25 19:46 ` [PATCH v2 1/4] power: bq25890_charger.c: Add debugging output of failed initialization Angus Ainslie (Purism)
` (3 more replies)
2018-07-31 16:43 ` [PATCH v3 0/4] Add the BQ25896 charger IC Angus Ainslie (Purism)
2018-07-31 17:49 ` [PATCH v4 0/4] Add the BQ25896 charger IC Angus Ainslie (Purism)
3 siblings, 4 replies; 32+ messages in thread
From: Angus Ainslie (Purism) @ 2018-07-25 19:46 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: angus.ainslie, Sebastian Reichel, linux-pm, linux-kernel,
Angus Ainslie (Purism)
Revised patch set based on review comments
Angus Ainslie (Purism) (4):
power: bq25890_charger.c: Add debugging output of failed
initialization
power: bq25890_charger.c: Remove unused table
power: bq25890_charger.c: Add the BQ25896 part
power: bq25890_charger.c: Read back the current battery voltage
.../bindings/power/supply/bq25890.txt | 1 +
drivers/power/supply/bq25890_charger.c | 65 +++++++++++++++----
2 files changed, 54 insertions(+), 12 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 1/4] power: bq25890_charger.c: Add debugging output of failed initialization
2018-07-25 19:46 ` [PATCH v2 0/4] Add the BQ25896 charger IC Angus Ainslie (Purism)
@ 2018-07-25 19:46 ` Angus Ainslie (Purism)
2018-07-26 6:44 ` Krzysztof Kozlowski
2018-07-25 19:46 ` [PATCH v2 2/4] power: bq25890_charger.c: Remove unused table Angus Ainslie (Purism)
` (2 subsequent siblings)
3 siblings, 1 reply; 32+ messages in thread
From: Angus Ainslie (Purism) @ 2018-07-25 19:46 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: angus.ainslie, Sebastian Reichel, linux-pm, linux-kernel,
Angus Ainslie (Purism)
To ease adding a new part variant some debugging is handy
Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
drivers/power/supply/bq25890_charger.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index 8e2c41ded171..7f0b3a46c50c 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -608,30 +608,40 @@ static int bq25890_hw_init(struct bq25890_device *bq)
};
ret = bq25890_chip_reset(bq);
- if (ret < 0)
+ if (ret < 0) {
+ dev_dbg(bq->dev, "Reset failed %d\n", ret);
return ret;
+ };
/* disable watchdog */
ret = bq25890_field_write(bq, F_WD, 0);
- if (ret < 0)
+ if (ret < 0) {
+ dev_dbg(bq->dev, "Disabling watchdog failed %d\n", ret);
return ret;
+ };
/* initialize currents/voltages and other parameters */
for (i = 0; i < ARRAY_SIZE(init_data); i++) {
ret = bq25890_field_write(bq, init_data[i].id,
init_data[i].value);
- if (ret < 0)
+ if (ret < 0) {
+ dev_dbg(bq->dev, "Writing init data failed %d\n", ret);
return ret;
+ };
}
/* Configure ADC for continuous conversions. This does not enable it. */
ret = bq25890_field_write(bq, F_CONV_RATE, 1);
- if (ret < 0)
+ if (ret < 0) {
+ dev_dbg(bq->dev, "Config ADC failed %d\n", ret);
return ret;
+ };
ret = bq25890_get_chip_state(bq, &state);
- if (ret < 0)
+ if (ret < 0) {
+ dev_dbg(bq->dev, "Get state failed %d\n", ret);
return ret;
+ };
mutex_lock(&bq->lock);
bq->state = state;
@@ -767,6 +777,9 @@ static int bq25890_fw_read_u32_props(struct bq25890_device *bq)
if (props[i].optional)
continue;
+ dev_err(bq->dev, "Unable to read property %d %s\n", ret,
+ props[i].name);
+
return ret;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v2 2/4] power: bq25890_charger.c: Remove unused table
2018-07-25 19:46 ` [PATCH v2 0/4] Add the BQ25896 charger IC Angus Ainslie (Purism)
2018-07-25 19:46 ` [PATCH v2 1/4] power: bq25890_charger.c: Add debugging output of failed initialization Angus Ainslie (Purism)
@ 2018-07-25 19:46 ` Angus Ainslie (Purism)
2018-07-26 6:51 ` Krzysztof Kozlowski
2018-07-25 19:46 ` [PATCH v2 3/4] power: bq25890_charger.c: Add the BQ25896 part Angus Ainslie (Purism)
2018-07-25 19:46 ` [PATCH v2 4/4] power: bq25890_charger.c: Read back the current battery voltage Angus Ainslie (Purism)
3 siblings, 1 reply; 32+ messages in thread
From: Angus Ainslie (Purism) @ 2018-07-25 19:46 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: angus.ainslie, Sebastian Reichel, linux-pm, linux-kernel,
Angus Ainslie (Purism)
The BATCMP table isn't used so drop it
Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
drivers/power/supply/bq25890_charger.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index 7f0b3a46c50c..641f7d779e2f 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -246,7 +246,6 @@ enum bq25890_table_ids {
TBL_ITERM,
TBL_IPRECHG,
TBL_VREG,
- TBL_BATCMP,
TBL_VCLAMP,
TBL_BOOSTV,
TBL_SYSVMIN,
@@ -287,7 +286,6 @@ static const union {
[TBL_ICHG] = { .rt = {0, 5056000, 64000} }, /* uA */
[TBL_ITERM] = { .rt = {64000, 1024000, 64000} }, /* uA */
[TBL_VREG] = { .rt = {3840000, 4608000, 16000} }, /* uV */
- [TBL_BATCMP] = { .rt = {0, 140, 20} }, /* mOhm */
[TBL_VCLAMP] = { .rt = {0, 224000, 32000} }, /* uV */
[TBL_BOOSTV] = { .rt = {4550000, 5510000, 64000} }, /* uV */
[TBL_SYSVMIN] = { .rt = {3000000, 3700000, 100000} }, /* uV */
--
2.17.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v2 3/4] power: bq25890_charger.c: Add the BQ25896 part
2018-07-25 19:46 ` [PATCH v2 0/4] Add the BQ25896 charger IC Angus Ainslie (Purism)
2018-07-25 19:46 ` [PATCH v2 1/4] power: bq25890_charger.c: Add debugging output of failed initialization Angus Ainslie (Purism)
2018-07-25 19:46 ` [PATCH v2 2/4] power: bq25890_charger.c: Remove unused table Angus Ainslie (Purism)
@ 2018-07-25 19:46 ` Angus Ainslie (Purism)
2018-07-26 6:56 ` Krzysztof Kozlowski
2018-07-25 19:46 ` [PATCH v2 4/4] power: bq25890_charger.c: Read back the current battery voltage Angus Ainslie (Purism)
3 siblings, 1 reply; 32+ messages in thread
From: Angus Ainslie (Purism) @ 2018-07-25 19:46 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: angus.ainslie, Sebastian Reichel, linux-pm, linux-kernel,
Angus Ainslie (Purism)
The BQ25896 is almost identical the the BQ25890
Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
.../bindings/power/supply/bq25890.txt | 1 +
drivers/power/supply/bq25890_charger.c | 25 +++++++++++++++----
2 files changed, 21 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/power/supply/bq25890.txt b/Documentation/devicetree/bindings/power/supply/bq25890.txt
index c9dd17d142ad..e98312fe6e26 100644
--- a/Documentation/devicetree/bindings/power/supply/bq25890.txt
+++ b/Documentation/devicetree/bindings/power/supply/bq25890.txt
@@ -3,6 +3,7 @@ Binding for TI bq25890 Li-Ion Charger
Required properties:
- compatible: Should contain one of the following:
* "ti,bq25890"
+ * "ti,bq25896"
- reg: integer, i2c address of the device.
- ti,battery-regulation-voltage: integer, maximum charging voltage (in uV);
- ti,charge-current: integer, maximum charging current (in uA);
diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index 641f7d779e2f..f4cf2987996b 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -32,6 +32,7 @@
#define BQ25890_IRQ_PIN "bq25890_irq"
#define BQ25890_ID 3
+#define BQ25896_ID 0
enum bq25890_fields {
F_EN_HIZ, F_EN_ILIM, F_IILIM, /* Reg00 */
@@ -153,8 +154,8 @@ static const struct reg_field bq25890_reg_fields[] = {
[F_CONV_RATE] = REG_FIELD(0x02, 6, 6),
[F_BOOSTF] = REG_FIELD(0x02, 5, 5),
[F_ICO_EN] = REG_FIELD(0x02, 4, 4),
- [F_HVDCP_EN] = REG_FIELD(0x02, 3, 3),
- [F_MAXC_EN] = REG_FIELD(0x02, 2, 2),
+ [F_HVDCP_EN] = REG_FIELD(0x02, 3, 3), // reserved on BQ25896
+ [F_MAXC_EN] = REG_FIELD(0x02, 2, 2), // reserved on BQ25896
[F_FORCE_DPM] = REG_FIELD(0x02, 1, 1),
[F_AUTO_DPDM_EN] = REG_FIELD(0x02, 0, 0),
/* REG03 */
@@ -163,6 +164,7 @@ static const struct reg_field bq25890_reg_fields[] = {
[F_OTG_CFG] = REG_FIELD(0x03, 5, 5),
[F_CHG_CFG] = REG_FIELD(0x03, 4, 4),
[F_SYSVMIN] = REG_FIELD(0x03, 1, 3),
+ /* MIN_VBAT_SEL on BQ25896 */
/* REG04 */
[F_PUMPX_EN] = REG_FIELD(0x04, 7, 7),
[F_ICHG] = REG_FIELD(0x04, 0, 6),
@@ -181,7 +183,7 @@ static const struct reg_field bq25890_reg_fields[] = {
[F_CHG_TMR] = REG_FIELD(0x07, 1, 2),
[F_JEITA_ISET] = REG_FIELD(0x07, 0, 0),
/* REG08 */
- [F_BATCMP] = REG_FIELD(0x08, 6, 7),
+ [F_BATCMP] = REG_FIELD(0x08, 6, 7), // 5-7 on BQ25896
[F_VCLAMP] = REG_FIELD(0x08, 2, 4),
[F_TREG] = REG_FIELD(0x08, 0, 1),
/* REG09 */
@@ -195,12 +197,13 @@ static const struct reg_field bq25890_reg_fields[] = {
[F_PUMPX_DN] = REG_FIELD(0x09, 0, 0),
/* REG0A */
[F_BOOSTV] = REG_FIELD(0x0A, 4, 7),
+ /* PFM_OTG_DIS 3 on BQ25896 */
[F_BOOSTI] = REG_FIELD(0x0A, 0, 2),
/* REG0B */
[F_VBUS_STAT] = REG_FIELD(0x0B, 5, 7),
[F_CHG_STAT] = REG_FIELD(0x0B, 3, 4),
[F_PG_STAT] = REG_FIELD(0x0B, 2, 2),
- [F_SDP_STAT] = REG_FIELD(0x0B, 1, 1),
+ [F_SDP_STAT] = REG_FIELD(0x0B, 1, 1), // reserved on BQ25896
[F_VSYS_STAT] = REG_FIELD(0x0B, 0, 0),
/* REG0C */
[F_WD_FAULT] = REG_FIELD(0x0C, 7, 7),
@@ -399,6 +402,16 @@ static int bq25890_power_supply_get_property(struct power_supply *psy,
val->strval = BQ25890_MANUFACTURER;
break;
+ case POWER_SUPPLY_PROP_MODEL_NAME:
+ if (bq->chip_id == BQ25890_ID)
+ val->strval = "BQ25890";
+ else if (bq->chip_id == BQ25896_ID)
+ val->strval = "BQ25896";
+ else
+ val->strval = "UNKNOWN";
+
+ break;
+
case POWER_SUPPLY_PROP_ONLINE:
val->intval = state.online;
break;
@@ -650,6 +663,7 @@ static int bq25890_hw_init(struct bq25890_device *bq)
static enum power_supply_property bq25890_power_supply_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
+ POWER_SUPPLY_PROP_MODEL_NAME,
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_ONLINE,
POWER_SUPPLY_PROP_HEALTH,
@@ -851,7 +865,7 @@ static int bq25890_probe(struct i2c_client *client,
return bq->chip_id;
}
- if (bq->chip_id != BQ25890_ID) {
+ if ((bq->chip_id != BQ25890_ID) && (bq->chip_id != BQ25896_ID)) {
dev_err(dev, "Chip with ID=%d, not supported!\n", bq->chip_id);
return -ENODEV;
}
@@ -977,6 +991,7 @@ MODULE_DEVICE_TABLE(i2c, bq25890_i2c_ids);
static const struct of_device_id bq25890_of_match[] = {
{ .compatible = "ti,bq25890", },
+ { .compatible = "ti,bq25896", },
{ },
};
MODULE_DEVICE_TABLE(of, bq25890_of_match);
--
2.17.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v2 4/4] power: bq25890_charger.c: Read back the current battery voltage
2018-07-25 19:46 ` [PATCH v2 0/4] Add the BQ25896 charger IC Angus Ainslie (Purism)
` (2 preceding siblings ...)
2018-07-25 19:46 ` [PATCH v2 3/4] power: bq25890_charger.c: Add the BQ25896 part Angus Ainslie (Purism)
@ 2018-07-25 19:46 ` Angus Ainslie (Purism)
2018-07-26 7:03 ` Krzysztof Kozlowski
3 siblings, 1 reply; 32+ messages in thread
From: Angus Ainslie (Purism) @ 2018-07-25 19:46 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: angus.ainslie, Sebastian Reichel, linux-pm, linux-kernel,
Angus Ainslie (Purism)
The part has the capability of reading the current battery voltage
Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
drivers/power/supply/bq25890_charger.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index f4cf2987996b..60ccd6c2b7ad 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -464,6 +464,20 @@ 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_VOLTAGE_NOW:
+ if (!state.online) {
+ val->intval = 0;
+ break;
+ }
+
+ ret = bq25890_field_read(bq, F_SYSV); /* read measured value */
+ if (ret < 0)
+ return ret;
+
+ /* converted_val = 2.304V + ADC_val * 20mV (table 10.3.15) */
+ val->intval = 2304000 + ret * 20000;
+ break;
+
default:
return -EINVAL;
}
@@ -672,6 +686,7 @@ static enum power_supply_property bq25890_power_supply_props[] = {
POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX,
POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT,
+ POWER_SUPPLY_PROP_VOLTAGE_NOW,
};
static char *bq25890_charger_supplied_to[] = {
--
2.17.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH] bq25890_charger.c : add the BQ25896 part
2018-07-25 12:17 ` Angus Ainslie
@ 2018-07-26 6:37 ` Krzysztof Kozlowski
0 siblings, 0 replies; 32+ messages in thread
From: Krzysztof Kozlowski @ 2018-07-26 6:37 UTC (permalink / raw)
To: Angus Ainslie; +Cc: Sebastian Reichel, linux-pm, linux-kernel, Angus Ainslie
On 25 July 2018 at 14:17, Angus Ainslie <angus@akkea.ca> wrote:
> Hi Krzysztof,
>
> On 2018-07-25 03:58, Krzysztof Kozlowski wrote:
>>
>> On 23 July 2018 at 15:51, Angus Ainslie <angus@akkea.ca> wrote:
>>>
>>> Add some debugging to be able to check the proper initialization
>>> of the BQ25896 part.
>>
>>
>> Hi,
>>
>> This should be split into separate patchset. Do not mix two features
>> in one commit.
>>
>
> Ok, I'll take it apart
>
>>> Enable the BQ25896 part.
>>>
>>> Add 2 new parameters "voltage_now" and "model_name".
>>>
>>> Signed-off-by: Angus Ainslie <angus.ainslie@puri.sm>
>>
>>
>> Your signed-off-by does not match From address.
>>
>
> That was intentional as I wanted Purism to get credit for it. I'm guessing
> that's not the correct way of doing it.
The author of patch (appearing as "From") should match Signed-off-by.
You might however easily create commit and signed it with your Purism
email. If you send such email, you will notice that "From" field is
visible in the mail message and it differs from Sender.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 1/4] power: bq25890_charger.c: Add debugging output of failed initialization
2018-07-25 19:46 ` [PATCH v2 1/4] power: bq25890_charger.c: Add debugging output of failed initialization Angus Ainslie (Purism)
@ 2018-07-26 6:44 ` Krzysztof Kozlowski
0 siblings, 0 replies; 32+ messages in thread
From: Krzysztof Kozlowski @ 2018-07-26 6:44 UTC (permalink / raw)
To: Angus Ainslie (Purism)
Cc: angus.ainslie, Sebastian Reichel, linux-pm, linux-kernel
On 25 July 2018 at 21:46, Angus Ainslie (Purism) <angus@akkea.ca> wrote:
> To ease adding a new part variant some debugging is handy
>
> Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
> ---
> drivers/power/supply/bq25890_charger.c | 23 ++++++++++++++++++-----
> 1 file changed, 18 insertions(+), 5 deletions(-)
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 2/4] power: bq25890_charger.c: Remove unused table
2018-07-25 19:46 ` [PATCH v2 2/4] power: bq25890_charger.c: Remove unused table Angus Ainslie (Purism)
@ 2018-07-26 6:51 ` Krzysztof Kozlowski
2018-07-27 15:26 ` Angus Ainslie
0 siblings, 1 reply; 32+ messages in thread
From: Krzysztof Kozlowski @ 2018-07-26 6:51 UTC (permalink / raw)
To: Angus Ainslie (Purism)
Cc: angus.ainslie, Sebastian Reichel, linux-pm, linux-kernel
On 25 July 2018 at 21:46, Angus Ainslie (Purism) <angus@akkea.ca> wrote:
> The BATCMP table isn't used so drop it
TBL_VCLAMP also looks unused. TBL_IPRECHG does not have table entry.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 3/4] power: bq25890_charger.c: Add the BQ25896 part
2018-07-25 19:46 ` [PATCH v2 3/4] power: bq25890_charger.c: Add the BQ25896 part Angus Ainslie (Purism)
@ 2018-07-26 6:56 ` Krzysztof Kozlowski
0 siblings, 0 replies; 32+ messages in thread
From: Krzysztof Kozlowski @ 2018-07-26 6:56 UTC (permalink / raw)
To: Angus Ainslie (Purism)
Cc: angus.ainslie, Sebastian Reichel, linux-pm, linux-kernel
On 25 July 2018 at 21:46, Angus Ainslie (Purism) <angus@akkea.ca> wrote:
> The BQ25896 is almost identical the the BQ25890
Please use full stop on sentences.
This patch (strip from other addons) nicely shows that driver does not
need to do anything more to support BQ25896. These two devices are
just compatible. You do not change behavior based on matching to
compatible,
Therefore you should not add new compatible but use old/existing one.
If you wish to notify users that they should use this driver, mention
in bindings that "ti,bq25896" compatible is for BQ2589x (BQ25890.
BQ25896) devices.
Best regards,
Krzysztof
>
> Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
> ---
> .../bindings/power/supply/bq25890.txt | 1 +
> drivers/power/supply/bq25890_charger.c | 25 +++++++++++++++----
> 2 files changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/power/supply/bq25890.txt b/Documentation/devicetree/bindings/power/supply/bq25890.txt
> index c9dd17d142ad..e98312fe6e26 100644
> --- a/Documentation/devicetree/bindings/power/supply/bq25890.txt
> +++ b/Documentation/devicetree/bindings/power/supply/bq25890.txt
> @@ -3,6 +3,7 @@ Binding for TI bq25890 Li-Ion Charger
> Required properties:
> - compatible: Should contain one of the following:
> * "ti,bq25890"
> + * "ti,bq25896"
> - reg: integer, i2c address of the device.
> - ti,battery-regulation-voltage: integer, maximum charging voltage (in uV);
> - ti,charge-current: integer, maximum charging current (in uA);
> diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
> index 641f7d779e2f..f4cf2987996b 100644
> --- a/drivers/power/supply/bq25890_charger.c
> +++ b/drivers/power/supply/bq25890_charger.c
> @@ -32,6 +32,7 @@
> #define BQ25890_IRQ_PIN "bq25890_irq"
>
> #define BQ25890_ID 3
> +#define BQ25896_ID 0
>
> enum bq25890_fields {
> F_EN_HIZ, F_EN_ILIM, F_IILIM, /* Reg00 */
> @@ -153,8 +154,8 @@ static const struct reg_field bq25890_reg_fields[] = {
> [F_CONV_RATE] = REG_FIELD(0x02, 6, 6),
> [F_BOOSTF] = REG_FIELD(0x02, 5, 5),
> [F_ICO_EN] = REG_FIELD(0x02, 4, 4),
> - [F_HVDCP_EN] = REG_FIELD(0x02, 3, 3),
> - [F_MAXC_EN] = REG_FIELD(0x02, 2, 2),
> + [F_HVDCP_EN] = REG_FIELD(0x02, 3, 3), // reserved on BQ25896
> + [F_MAXC_EN] = REG_FIELD(0x02, 2, 2), // reserved on BQ25896
> [F_FORCE_DPM] = REG_FIELD(0x02, 1, 1),
> [F_AUTO_DPDM_EN] = REG_FIELD(0x02, 0, 0),
> /* REG03 */
> @@ -163,6 +164,7 @@ static const struct reg_field bq25890_reg_fields[] = {
> [F_OTG_CFG] = REG_FIELD(0x03, 5, 5),
> [F_CHG_CFG] = REG_FIELD(0x03, 4, 4),
> [F_SYSVMIN] = REG_FIELD(0x03, 1, 3),
> + /* MIN_VBAT_SEL on BQ25896 */
> /* REG04 */
> [F_PUMPX_EN] = REG_FIELD(0x04, 7, 7),
> [F_ICHG] = REG_FIELD(0x04, 0, 6),
> @@ -181,7 +183,7 @@ static const struct reg_field bq25890_reg_fields[] = {
> [F_CHG_TMR] = REG_FIELD(0x07, 1, 2),
> [F_JEITA_ISET] = REG_FIELD(0x07, 0, 0),
> /* REG08 */
> - [F_BATCMP] = REG_FIELD(0x08, 6, 7),
> + [F_BATCMP] = REG_FIELD(0x08, 6, 7), // 5-7 on BQ25896
> [F_VCLAMP] = REG_FIELD(0x08, 2, 4),
> [F_TREG] = REG_FIELD(0x08, 0, 1),
> /* REG09 */
> @@ -195,12 +197,13 @@ static const struct reg_field bq25890_reg_fields[] = {
> [F_PUMPX_DN] = REG_FIELD(0x09, 0, 0),
> /* REG0A */
> [F_BOOSTV] = REG_FIELD(0x0A, 4, 7),
> + /* PFM_OTG_DIS 3 on BQ25896 */
> [F_BOOSTI] = REG_FIELD(0x0A, 0, 2),
> /* REG0B */
> [F_VBUS_STAT] = REG_FIELD(0x0B, 5, 7),
> [F_CHG_STAT] = REG_FIELD(0x0B, 3, 4),
> [F_PG_STAT] = REG_FIELD(0x0B, 2, 2),
> - [F_SDP_STAT] = REG_FIELD(0x0B, 1, 1),
> + [F_SDP_STAT] = REG_FIELD(0x0B, 1, 1), // reserved on BQ25896
> [F_VSYS_STAT] = REG_FIELD(0x0B, 0, 0),
> /* REG0C */
> [F_WD_FAULT] = REG_FIELD(0x0C, 7, 7),
> @@ -399,6 +402,16 @@ static int bq25890_power_supply_get_property(struct power_supply *psy,
> val->strval = BQ25890_MANUFACTURER;
> break;
>
> + case POWER_SUPPLY_PROP_MODEL_NAME:
> + if (bq->chip_id == BQ25890_ID)
> + val->strval = "BQ25890";
> + else if (bq->chip_id == BQ25896_ID)
> + val->strval = "BQ25896";
> + else
> + val->strval = "UNKNOWN";
> +
> + break;
> +
> case POWER_SUPPLY_PROP_ONLINE:
> val->intval = state.online;
> break;
> @@ -650,6 +663,7 @@ static int bq25890_hw_init(struct bq25890_device *bq)
>
> static enum power_supply_property bq25890_power_supply_props[] = {
> POWER_SUPPLY_PROP_MANUFACTURER,
> + POWER_SUPPLY_PROP_MODEL_NAME,
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_ONLINE,
> POWER_SUPPLY_PROP_HEALTH,
> @@ -851,7 +865,7 @@ static int bq25890_probe(struct i2c_client *client,
> return bq->chip_id;
> }
>
> - if (bq->chip_id != BQ25890_ID) {
> + if ((bq->chip_id != BQ25890_ID) && (bq->chip_id != BQ25896_ID)) {
> dev_err(dev, "Chip with ID=%d, not supported!\n", bq->chip_id);
> return -ENODEV;
> }
> @@ -977,6 +991,7 @@ MODULE_DEVICE_TABLE(i2c, bq25890_i2c_ids);
>
> static const struct of_device_id bq25890_of_match[] = {
> { .compatible = "ti,bq25890", },
> + { .compatible = "ti,bq25896", },
> { },
> };
> MODULE_DEVICE_TABLE(of, bq25890_of_match);
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 4/4] power: bq25890_charger.c: Read back the current battery voltage
2018-07-25 19:46 ` [PATCH v2 4/4] power: bq25890_charger.c: Read back the current battery voltage Angus Ainslie (Purism)
@ 2018-07-26 7:03 ` Krzysztof Kozlowski
2018-07-27 15:30 ` Angus Ainslie
0 siblings, 1 reply; 32+ messages in thread
From: Krzysztof Kozlowski @ 2018-07-26 7:03 UTC (permalink / raw)
To: Angus Ainslie (Purism)
Cc: angus.ainslie, Sebastian Reichel, linux-pm, linux-kernel
On 25 July 2018 at 21:46, Angus Ainslie (Purism) <angus@akkea.ca> wrote:
> The part has the capability of reading the current battery voltage
>
> Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
> ---
> drivers/power/supply/bq25890_charger.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
> index f4cf2987996b..60ccd6c2b7ad 100644
> --- a/drivers/power/supply/bq25890_charger.c
> +++ b/drivers/power/supply/bq25890_charger.c
> @@ -464,6 +464,20 @@ 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_VOLTAGE_NOW:
> + if (!state.online) {
> + val->intval = 0;
> + break;
> + }
You use state from cached value (from last interrupt) but the voltage
is read directly from registers. The driver follows this convention in
few other places so maybe it is okay. It depends how accurately the
interrupts are generated - on every change? IOW, is always guaranteed
that state will be consistent with values read below?
Best regards,
Krzysztof
> +
> + ret = bq25890_field_read(bq, F_SYSV); /* read measured value */
> + if (ret < 0)
> + return ret;
> +
> + /* converted_val = 2.304V + ADC_val * 20mV (table 10.3.15) */
> + val->intval = 2304000 + ret * 20000;
> + break;
> +
> default:
> return -EINVAL;
> }
> @@ -672,6 +686,7 @@ static enum power_supply_property bq25890_power_supply_props[] = {
> POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
> POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX,
> POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT,
> + POWER_SUPPLY_PROP_VOLTAGE_NOW,
> };
>
> static char *bq25890_charger_supplied_to[] = {
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 2/4] power: bq25890_charger.c: Remove unused table
2018-07-26 6:51 ` Krzysztof Kozlowski
@ 2018-07-27 15:26 ` Angus Ainslie
2018-07-31 6:40 ` Krzysztof Kozlowski
0 siblings, 1 reply; 32+ messages in thread
From: Angus Ainslie @ 2018-07-27 15:26 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: angus.ainslie, Sebastian Reichel, linux-pm, linux-kernel
Hi Krzysztof,
On 2018-07-26 00:51, Krzysztof Kozlowski wrote:
> On 25 July 2018 at 21:46, Angus Ainslie (Purism) <angus@akkea.ca>
> wrote:
>> The BATCMP table isn't used so drop it
>
> TBL_VCLAMP also looks unused. TBL_IPRECHG does not have table entry.
>
I as looking to drop the tables that would be problematic to the new
part. Would you like me to resolve these 2 as well ?
Regards,
Angus
> Best regards,
> Krzysztof
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 4/4] power: bq25890_charger.c: Read back the current battery voltage
2018-07-26 7:03 ` Krzysztof Kozlowski
@ 2018-07-27 15:30 ` Angus Ainslie
0 siblings, 0 replies; 32+ messages in thread
From: Angus Ainslie @ 2018-07-27 15:30 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: angus.ainslie, Sebastian Reichel, linux-pm, linux-kernel
On 2018-07-26 01:03, Krzysztof Kozlowski wrote:
> On 25 July 2018 at 21:46, Angus Ainslie (Purism) <angus@akkea.ca>
> wrote:
>> The part has the capability of reading the current battery voltage
>>
>> Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
>> ---
>> drivers/power/supply/bq25890_charger.c | 15 +++++++++++++++
>> 1 file changed, 15 insertions(+)
>>
>> diff --git a/drivers/power/supply/bq25890_charger.c
>> b/drivers/power/supply/bq25890_charger.c
>> index f4cf2987996b..60ccd6c2b7ad 100644
>> --- a/drivers/power/supply/bq25890_charger.c
>> +++ b/drivers/power/supply/bq25890_charger.c
>> @@ -464,6 +464,20 @@ 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_VOLTAGE_NOW:
>> + if (!state.online) {
>> + val->intval = 0;
>> + break;
>> + }
>
> You use state from cached value (from last interrupt) but the voltage
> is read directly from registers. The driver follows this convention in
> few other places so maybe it is okay. It depends how accurately the
> interrupts are generated - on every change? IOW, is always guaranteed
> that state will be consistent with values read below?
>
Well I shouldn't even be checking state here as the VOLTAGE_NOW should
be valid without it. I'll drop that check.
( Krzysztof sorry for the duplicate email )
> Best regards,
> Krzysztof
>
>> +
>> + ret = bq25890_field_read(bq, F_SYSV); /* read measured
>> value */
>> + if (ret < 0)
>> + return ret;
>> +
>> + /* converted_val = 2.304V + ADC_val * 20mV (table
>> 10.3.15) */
>> + val->intval = 2304000 + ret * 20000;
>> + break;
>> +
>> default:
>> return -EINVAL;
>> }
>> @@ -672,6 +686,7 @@ static enum power_supply_property
>> bq25890_power_supply_props[] = {
>> POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
>> POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX,
>> POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT,
>> + POWER_SUPPLY_PROP_VOLTAGE_NOW,
>> };
>>
>> static char *bq25890_charger_supplied_to[] = {
>> --
>> 2.17.1
>>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 2/4] power: bq25890_charger.c: Remove unused table
2018-07-27 15:26 ` Angus Ainslie
@ 2018-07-31 6:40 ` Krzysztof Kozlowski
0 siblings, 0 replies; 32+ messages in thread
From: Krzysztof Kozlowski @ 2018-07-31 6:40 UTC (permalink / raw)
To: Angus Ainslie; +Cc: angus.ainslie, Sebastian Reichel, linux-pm, linux-kernel
On 27 July 2018 at 17:26, Angus Ainslie <angus@akkea.ca> wrote:
> Hi Krzysztof,
>
> On 2018-07-26 00:51, Krzysztof Kozlowski wrote:
>>
>> On 25 July 2018 at 21:46, Angus Ainslie (Purism) <angus@akkea.ca> wrote:
>>>
>>> The BATCMP table isn't used so drop it
>>
>>
>> TBL_VCLAMP also looks unused. TBL_IPRECHG does not have table entry.
>>
>
> I as looking to drop the tables that would be problematic to the new part.
> Would you like me to resolve these 2 as well ?
Yes. Since you are removing some entry then let's remove all unneeded entries.
Krzysztof
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v3 0/4] Add the BQ25896 charger IC
2018-07-25 9:58 ` Krzysztof Kozlowski
2018-07-25 12:17 ` Angus Ainslie
2018-07-25 19:46 ` [PATCH v2 0/4] Add the BQ25896 charger IC Angus Ainslie (Purism)
@ 2018-07-31 16:43 ` Angus Ainslie (Purism)
2018-07-31 16:43 ` [PATCH v3 1/4] power: bq25890_charger.c: Add debugging output of failed initialization Angus Ainslie (Purism)
` (3 more replies)
2018-07-31 17:49 ` [PATCH v4 0/4] Add the BQ25896 charger IC Angus Ainslie (Purism)
3 siblings, 4 replies; 32+ messages in thread
From: Angus Ainslie (Purism) @ 2018-07-31 16:43 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: angus.ainslie, Sebastian Reichel, linux-pm, linux-kernel,
Angus Ainslie (Purism)
Changes since v2:
Remove additional un-used table entries.
Use the exsisting compitble string for the new part.
Voltage now doesn't depend on state "online".
Angus Ainslie (Purism) (4):
power: bq25890_charger.c: Add debugging output of failed
initialization
power: bq25890_charger.c: Remove unused table entries
power: bq25890_charger.c: Add the BQ25896 part
power: bq25890_charger.c: Read back the current battery voltage
.../bindings/power/supply/bq25890.txt | 3 +
drivers/power/supply/bq25890_charger.c | 62 ++++++++++++++-----
2 files changed, 50 insertions(+), 15 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v3 1/4] power: bq25890_charger.c: Add debugging output of failed initialization
2018-07-31 16:43 ` [PATCH v3 0/4] Add the BQ25896 charger IC Angus Ainslie (Purism)
@ 2018-07-31 16:43 ` Angus Ainslie (Purism)
2018-07-31 16:57 ` Krzysztof Kozlowski
2018-07-31 16:43 ` [PATCH v3 2/4] power: bq25890_charger.c: Remove unused table entries Angus Ainslie (Purism)
` (2 subsequent siblings)
3 siblings, 1 reply; 32+ messages in thread
From: Angus Ainslie (Purism) @ 2018-07-31 16:43 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: angus.ainslie, Sebastian Reichel, linux-pm, linux-kernel,
Angus Ainslie (Purism)
To ease adding a new part variant some debugging is handy.
Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
drivers/power/supply/bq25890_charger.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index 8e2c41ded171..7f0b3a46c50c 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -608,30 +608,40 @@ static int bq25890_hw_init(struct bq25890_device *bq)
};
ret = bq25890_chip_reset(bq);
- if (ret < 0)
+ if (ret < 0) {
+ dev_dbg(bq->dev, "Reset failed %d\n", ret);
return ret;
+ };
/* disable watchdog */
ret = bq25890_field_write(bq, F_WD, 0);
- if (ret < 0)
+ if (ret < 0) {
+ dev_dbg(bq->dev, "Disabling watchdog failed %d\n", ret);
return ret;
+ };
/* initialize currents/voltages and other parameters */
for (i = 0; i < ARRAY_SIZE(init_data); i++) {
ret = bq25890_field_write(bq, init_data[i].id,
init_data[i].value);
- if (ret < 0)
+ if (ret < 0) {
+ dev_dbg(bq->dev, "Writing init data failed %d\n", ret);
return ret;
+ };
}
/* Configure ADC for continuous conversions. This does not enable it. */
ret = bq25890_field_write(bq, F_CONV_RATE, 1);
- if (ret < 0)
+ if (ret < 0) {
+ dev_dbg(bq->dev, "Config ADC failed %d\n", ret);
return ret;
+ };
ret = bq25890_get_chip_state(bq, &state);
- if (ret < 0)
+ if (ret < 0) {
+ dev_dbg(bq->dev, "Get state failed %d\n", ret);
return ret;
+ };
mutex_lock(&bq->lock);
bq->state = state;
@@ -767,6 +777,9 @@ static int bq25890_fw_read_u32_props(struct bq25890_device *bq)
if (props[i].optional)
continue;
+ dev_err(bq->dev, "Unable to read property %d %s\n", ret,
+ props[i].name);
+
return ret;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v3 2/4] power: bq25890_charger.c: Remove unused table entries
2018-07-31 16:43 ` [PATCH v3 0/4] Add the BQ25896 charger IC Angus Ainslie (Purism)
2018-07-31 16:43 ` [PATCH v3 1/4] power: bq25890_charger.c: Add debugging output of failed initialization Angus Ainslie (Purism)
@ 2018-07-31 16:43 ` Angus Ainslie (Purism)
2018-07-31 16:58 ` Krzysztof Kozlowski
2018-07-31 16:43 ` [PATCH v3 3/4] power: bq25890_charger.c: Add the BQ25896 part Angus Ainslie (Purism)
2018-07-31 16:43 ` [PATCH v3 4/4] power: bq25890_charger.c: Read back the current battery voltage Angus Ainslie (Purism)
3 siblings, 1 reply; 32+ messages in thread
From: Angus Ainslie (Purism) @ 2018-07-31 16:43 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: angus.ainslie, Sebastian Reichel, linux-pm, linux-kernel,
Angus Ainslie (Purism)
There are a few table entries that aren't used. Drop them.
Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
drivers/power/supply/bq25890_charger.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index 7f0b3a46c50c..ca61278e932e 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -244,10 +244,7 @@ enum bq25890_table_ids {
/* range tables */
TBL_ICHG,
TBL_ITERM,
- TBL_IPRECHG,
TBL_VREG,
- TBL_BATCMP,
- TBL_VCLAMP,
TBL_BOOSTV,
TBL_SYSVMIN,
@@ -287,8 +284,6 @@ static const union {
[TBL_ICHG] = { .rt = {0, 5056000, 64000} }, /* uA */
[TBL_ITERM] = { .rt = {64000, 1024000, 64000} }, /* uA */
[TBL_VREG] = { .rt = {3840000, 4608000, 16000} }, /* uV */
- [TBL_BATCMP] = { .rt = {0, 140, 20} }, /* mOhm */
- [TBL_VCLAMP] = { .rt = {0, 224000, 32000} }, /* uV */
[TBL_BOOSTV] = { .rt = {4550000, 5510000, 64000} }, /* uV */
[TBL_SYSVMIN] = { .rt = {3000000, 3700000, 100000} }, /* uV */
--
2.17.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v3 3/4] power: bq25890_charger.c: Add the BQ25896 part
2018-07-31 16:43 ` [PATCH v3 0/4] Add the BQ25896 charger IC Angus Ainslie (Purism)
2018-07-31 16:43 ` [PATCH v3 1/4] power: bq25890_charger.c: Add debugging output of failed initialization Angus Ainslie (Purism)
2018-07-31 16:43 ` [PATCH v3 2/4] power: bq25890_charger.c: Remove unused table entries Angus Ainslie (Purism)
@ 2018-07-31 16:43 ` Angus Ainslie (Purism)
2018-07-31 17:00 ` Krzysztof Kozlowski
2018-07-31 16:43 ` [PATCH v3 4/4] power: bq25890_charger.c: Read back the current battery voltage Angus Ainslie (Purism)
3 siblings, 1 reply; 32+ messages in thread
From: Angus Ainslie (Purism) @ 2018-07-31 16:43 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: angus.ainslie, Sebastian Reichel, linux-pm, linux-kernel,
Angus Ainslie (Purism)
The BQ25896 is almost identical the the BQ25890.
Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
.../bindings/power/supply/bq25890.txt | 3 +++
drivers/power/supply/bq25890_charger.c | 24 +++++++++++++++----
2 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/power/supply/bq25890.txt b/Documentation/devicetree/bindings/power/supply/bq25890.txt
index c9dd17d142ad..ccb3a123ab57 100644
--- a/Documentation/devicetree/bindings/power/supply/bq25890.txt
+++ b/Documentation/devicetree/bindings/power/supply/bq25890.txt
@@ -1,5 +1,8 @@
Binding for TI bq25890 Li-Ion Charger
+This driver will support the bq25896 and the bq25890. There are other parts
+in the same family but those have not been tested.
+
Required properties:
- compatible: Should contain one of the following:
* "ti,bq25890"
diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index ca61278e932e..23b39da07e56 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -32,6 +32,7 @@
#define BQ25890_IRQ_PIN "bq25890_irq"
#define BQ25890_ID 3
+#define BQ25896_ID 0
enum bq25890_fields {
F_EN_HIZ, F_EN_ILIM, F_IILIM, /* Reg00 */
@@ -153,8 +154,8 @@ static const struct reg_field bq25890_reg_fields[] = {
[F_CONV_RATE] = REG_FIELD(0x02, 6, 6),
[F_BOOSTF] = REG_FIELD(0x02, 5, 5),
[F_ICO_EN] = REG_FIELD(0x02, 4, 4),
- [F_HVDCP_EN] = REG_FIELD(0x02, 3, 3),
- [F_MAXC_EN] = REG_FIELD(0x02, 2, 2),
+ [F_HVDCP_EN] = REG_FIELD(0x02, 3, 3), // reserved on BQ25896
+ [F_MAXC_EN] = REG_FIELD(0x02, 2, 2), // reserved on BQ25896
[F_FORCE_DPM] = REG_FIELD(0x02, 1, 1),
[F_AUTO_DPDM_EN] = REG_FIELD(0x02, 0, 0),
/* REG03 */
@@ -163,6 +164,7 @@ static const struct reg_field bq25890_reg_fields[] = {
[F_OTG_CFG] = REG_FIELD(0x03, 5, 5),
[F_CHG_CFG] = REG_FIELD(0x03, 4, 4),
[F_SYSVMIN] = REG_FIELD(0x03, 1, 3),
+ /* MIN_VBAT_SEL on BQ25896 */
/* REG04 */
[F_PUMPX_EN] = REG_FIELD(0x04, 7, 7),
[F_ICHG] = REG_FIELD(0x04, 0, 6),
@@ -181,7 +183,7 @@ static const struct reg_field bq25890_reg_fields[] = {
[F_CHG_TMR] = REG_FIELD(0x07, 1, 2),
[F_JEITA_ISET] = REG_FIELD(0x07, 0, 0),
/* REG08 */
- [F_BATCMP] = REG_FIELD(0x08, 6, 7),
+ [F_BATCMP] = REG_FIELD(0x08, 6, 7), // 5-7 on BQ25896
[F_VCLAMP] = REG_FIELD(0x08, 2, 4),
[F_TREG] = REG_FIELD(0x08, 0, 1),
/* REG09 */
@@ -195,12 +197,13 @@ static const struct reg_field bq25890_reg_fields[] = {
[F_PUMPX_DN] = REG_FIELD(0x09, 0, 0),
/* REG0A */
[F_BOOSTV] = REG_FIELD(0x0A, 4, 7),
+ /* PFM_OTG_DIS 3 on BQ25896 */
[F_BOOSTI] = REG_FIELD(0x0A, 0, 2),
/* REG0B */
[F_VBUS_STAT] = REG_FIELD(0x0B, 5, 7),
[F_CHG_STAT] = REG_FIELD(0x0B, 3, 4),
[F_PG_STAT] = REG_FIELD(0x0B, 2, 2),
- [F_SDP_STAT] = REG_FIELD(0x0B, 1, 1),
+ [F_SDP_STAT] = REG_FIELD(0x0B, 1, 1), // reserved on BQ25896
[F_VSYS_STAT] = REG_FIELD(0x0B, 0, 0),
/* REG0C */
[F_WD_FAULT] = REG_FIELD(0x0C, 7, 7),
@@ -396,6 +399,16 @@ static int bq25890_power_supply_get_property(struct power_supply *psy,
val->strval = BQ25890_MANUFACTURER;
break;
+ case POWER_SUPPLY_PROP_MODEL_NAME:
+ if (bq->chip_id == BQ25890_ID)
+ val->strval = "BQ25890";
+ else if (bq->chip_id == BQ25896_ID)
+ val->strval = "BQ25896";
+ else
+ val->strval = "UNKNOWN";
+
+ break;
+
case POWER_SUPPLY_PROP_ONLINE:
val->intval = state.online;
break;
@@ -647,6 +660,7 @@ static int bq25890_hw_init(struct bq25890_device *bq)
static enum power_supply_property bq25890_power_supply_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
+ POWER_SUPPLY_PROP_MODEL_NAME,
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_ONLINE,
POWER_SUPPLY_PROP_HEALTH,
@@ -848,7 +862,7 @@ static int bq25890_probe(struct i2c_client *client,
return bq->chip_id;
}
- if (bq->chip_id != BQ25890_ID) {
+ if ((bq->chip_id != BQ25890_ID) && (bq->chip_id != BQ25896_ID)) {
dev_err(dev, "Chip with ID=%d, not supported!\n", bq->chip_id);
return -ENODEV;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v3 4/4] power: bq25890_charger.c: Read back the current battery voltage
2018-07-31 16:43 ` [PATCH v3 0/4] Add the BQ25896 charger IC Angus Ainslie (Purism)
` (2 preceding siblings ...)
2018-07-31 16:43 ` [PATCH v3 3/4] power: bq25890_charger.c: Add the BQ25896 part Angus Ainslie (Purism)
@ 2018-07-31 16:43 ` Angus Ainslie (Purism)
2018-07-31 17:02 ` Krzysztof Kozlowski
3 siblings, 1 reply; 32+ messages in thread
From: Angus Ainslie (Purism) @ 2018-07-31 16:43 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: angus.ainslie, Sebastian Reichel, linux-pm, linux-kernel,
Angus Ainslie (Purism)
The part has the capability of reading the current battery voltage.
Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
drivers/power/supply/bq25890_charger.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index 23b39da07e56..1aa7872ddeb0 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -461,6 +461,15 @@ 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_VOLTAGE_NOW:
+ ret = bq25890_field_read(bq, F_SYSV); /* read measured value */
+ if (ret < 0)
+ return ret;
+
+ /* converted_val = 2.304V + ADC_val * 20mV (table 10.3.15) */
+ val->intval = 2304000 + ret * 20000;
+ break;
+
default:
return -EINVAL;
}
@@ -669,6 +678,7 @@ static enum power_supply_property bq25890_power_supply_props[] = {
POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX,
POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT,
+ POWER_SUPPLY_PROP_VOLTAGE_NOW,
};
static char *bq25890_charger_supplied_to[] = {
--
2.17.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH v3 1/4] power: bq25890_charger.c: Add debugging output of failed initialization
2018-07-31 16:43 ` [PATCH v3 1/4] power: bq25890_charger.c: Add debugging output of failed initialization Angus Ainslie (Purism)
@ 2018-07-31 16:57 ` Krzysztof Kozlowski
2018-07-31 18:16 ` Angus Ainslie
0 siblings, 1 reply; 32+ messages in thread
From: Krzysztof Kozlowski @ 2018-07-31 16:57 UTC (permalink / raw)
To: Angus Ainslie (Purism)
Cc: angus.ainslie, Sebastian Reichel, linux-pm, linux-kernel
On 31 July 2018 at 18:43, Angus Ainslie (Purism) <angus@akkea.ca> wrote:
> To ease adding a new part variant some debugging is handy.
>
> Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
You missed my tag - I already reviewed it. Unless you change
something, the tag should be added to commit msg:
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
Krzysztof
> ---
> drivers/power/supply/bq25890_charger.c | 23 ++++++++++++++++++-----
> 1 file changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
> index 8e2c41ded171..7f0b3a46c50c 100644
> --- a/drivers/power/supply/bq25890_charger.c
> +++ b/drivers/power/supply/bq25890_charger.c
> @@ -608,30 +608,40 @@ static int bq25890_hw_init(struct bq25890_device *bq)
> };
>
> ret = bq25890_chip_reset(bq);
> - if (ret < 0)
> + if (ret < 0) {
> + dev_dbg(bq->dev, "Reset failed %d\n", ret);
> return ret;
> + };
>
> /* disable watchdog */
> ret = bq25890_field_write(bq, F_WD, 0);
> - if (ret < 0)
> + if (ret < 0) {
> + dev_dbg(bq->dev, "Disabling watchdog failed %d\n", ret);
> return ret;
> + };
>
> /* initialize currents/voltages and other parameters */
> for (i = 0; i < ARRAY_SIZE(init_data); i++) {
> ret = bq25890_field_write(bq, init_data[i].id,
> init_data[i].value);
> - if (ret < 0)
> + if (ret < 0) {
> + dev_dbg(bq->dev, "Writing init data failed %d\n", ret);
> return ret;
> + };
> }
>
> /* Configure ADC for continuous conversions. This does not enable it. */
> ret = bq25890_field_write(bq, F_CONV_RATE, 1);
> - if (ret < 0)
> + if (ret < 0) {
> + dev_dbg(bq->dev, "Config ADC failed %d\n", ret);
> return ret;
> + };
>
> ret = bq25890_get_chip_state(bq, &state);
> - if (ret < 0)
> + if (ret < 0) {
> + dev_dbg(bq->dev, "Get state failed %d\n", ret);
> return ret;
> + };
>
> mutex_lock(&bq->lock);
> bq->state = state;
> @@ -767,6 +777,9 @@ static int bq25890_fw_read_u32_props(struct bq25890_device *bq)
> if (props[i].optional)
> continue;
>
> + dev_err(bq->dev, "Unable to read property %d %s\n", ret,
> + props[i].name);
> +
> return ret;
> }
>
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 2/4] power: bq25890_charger.c: Remove unused table entries
2018-07-31 16:43 ` [PATCH v3 2/4] power: bq25890_charger.c: Remove unused table entries Angus Ainslie (Purism)
@ 2018-07-31 16:58 ` Krzysztof Kozlowski
0 siblings, 0 replies; 32+ messages in thread
From: Krzysztof Kozlowski @ 2018-07-31 16:58 UTC (permalink / raw)
To: Angus Ainslie (Purism)
Cc: angus.ainslie, Sebastian Reichel, linux-pm, linux-kernel
On 31 July 2018 at 18:43, Angus Ainslie (Purism) <angus@akkea.ca> wrote:
> There are a few table entries that aren't used. Drop them.
>
> Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
> ---
> drivers/power/supply/bq25890_charger.c | 5 -----
> 1 file changed, 5 deletions(-)
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 3/4] power: bq25890_charger.c: Add the BQ25896 part
2018-07-31 16:43 ` [PATCH v3 3/4] power: bq25890_charger.c: Add the BQ25896 part Angus Ainslie (Purism)
@ 2018-07-31 17:00 ` Krzysztof Kozlowski
0 siblings, 0 replies; 32+ messages in thread
From: Krzysztof Kozlowski @ 2018-07-31 17:00 UTC (permalink / raw)
To: Angus Ainslie (Purism)
Cc: angus.ainslie, Sebastian Reichel, linux-pm, linux-kernel
On 31 July 2018 at 18:43, Angus Ainslie (Purism) <angus@akkea.ca> wrote:
> The BQ25896 is almost identical the the BQ25890.
s/the the/to the/
>
> Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
> ---
> .../bindings/power/supply/bq25890.txt | 3 +++
> drivers/power/supply/bq25890_charger.c | 24 +++++++++++++++----
> 2 files changed, 22 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/power/supply/bq25890.txt b/Documentation/devicetree/bindings/power/supply/bq25890.txt
> index c9dd17d142ad..ccb3a123ab57 100644
> --- a/Documentation/devicetree/bindings/power/supply/bq25890.txt
> +++ b/Documentation/devicetree/bindings/power/supply/bq25890.txt
> @@ -1,5 +1,8 @@
> Binding for TI bq25890 Li-Ion Charger
>
> +This driver will support the bq25896 and the bq25890. There are other parts
To me "other parts" in that context sounds unusual. Probably you
meant "other flavors" or "other devices from the same family".
With both changes:
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
Best regards,
Krzysztof
> +in the same family but those have not been tested.
> +
> Required properties:
> - compatible: Should contain one of the following:
> * "ti,bq25890"
> diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
> index ca61278e932e..23b39da07e56 100644
> --- a/drivers/power/supply/bq25890_charger.c
> +++ b/drivers/power/supply/bq25890_charger.c
> @@ -32,6 +32,7 @@
> #define BQ25890_IRQ_PIN "bq25890_irq"
>
> #define BQ25890_ID 3
> +#define BQ25896_ID 0
>
> enum bq25890_fields {
> F_EN_HIZ, F_EN_ILIM, F_IILIM, /* Reg00 */
> @@ -153,8 +154,8 @@ static const struct reg_field bq25890_reg_fields[] = {
> [F_CONV_RATE] = REG_FIELD(0x02, 6, 6),
> [F_BOOSTF] = REG_FIELD(0x02, 5, 5),
> [F_ICO_EN] = REG_FIELD(0x02, 4, 4),
> - [F_HVDCP_EN] = REG_FIELD(0x02, 3, 3),
> - [F_MAXC_EN] = REG_FIELD(0x02, 2, 2),
> + [F_HVDCP_EN] = REG_FIELD(0x02, 3, 3), // reserved on BQ25896
> + [F_MAXC_EN] = REG_FIELD(0x02, 2, 2), // reserved on BQ25896
> [F_FORCE_DPM] = REG_FIELD(0x02, 1, 1),
> [F_AUTO_DPDM_EN] = REG_FIELD(0x02, 0, 0),
> /* REG03 */
> @@ -163,6 +164,7 @@ static const struct reg_field bq25890_reg_fields[] = {
> [F_OTG_CFG] = REG_FIELD(0x03, 5, 5),
> [F_CHG_CFG] = REG_FIELD(0x03, 4, 4),
> [F_SYSVMIN] = REG_FIELD(0x03, 1, 3),
> + /* MIN_VBAT_SEL on BQ25896 */
> /* REG04 */
> [F_PUMPX_EN] = REG_FIELD(0x04, 7, 7),
> [F_ICHG] = REG_FIELD(0x04, 0, 6),
> @@ -181,7 +183,7 @@ static const struct reg_field bq25890_reg_fields[] = {
> [F_CHG_TMR] = REG_FIELD(0x07, 1, 2),
> [F_JEITA_ISET] = REG_FIELD(0x07, 0, 0),
> /* REG08 */
> - [F_BATCMP] = REG_FIELD(0x08, 6, 7),
> + [F_BATCMP] = REG_FIELD(0x08, 6, 7), // 5-7 on BQ25896
> [F_VCLAMP] = REG_FIELD(0x08, 2, 4),
> [F_TREG] = REG_FIELD(0x08, 0, 1),
> /* REG09 */
> @@ -195,12 +197,13 @@ static const struct reg_field bq25890_reg_fields[] = {
> [F_PUMPX_DN] = REG_FIELD(0x09, 0, 0),
> /* REG0A */
> [F_BOOSTV] = REG_FIELD(0x0A, 4, 7),
> + /* PFM_OTG_DIS 3 on BQ25896 */
> [F_BOOSTI] = REG_FIELD(0x0A, 0, 2),
> /* REG0B */
> [F_VBUS_STAT] = REG_FIELD(0x0B, 5, 7),
> [F_CHG_STAT] = REG_FIELD(0x0B, 3, 4),
> [F_PG_STAT] = REG_FIELD(0x0B, 2, 2),
> - [F_SDP_STAT] = REG_FIELD(0x0B, 1, 1),
> + [F_SDP_STAT] = REG_FIELD(0x0B, 1, 1), // reserved on BQ25896
> [F_VSYS_STAT] = REG_FIELD(0x0B, 0, 0),
> /* REG0C */
> [F_WD_FAULT] = REG_FIELD(0x0C, 7, 7),
> @@ -396,6 +399,16 @@ static int bq25890_power_supply_get_property(struct power_supply *psy,
> val->strval = BQ25890_MANUFACTURER;
> break;
>
> + case POWER_SUPPLY_PROP_MODEL_NAME:
> + if (bq->chip_id == BQ25890_ID)
> + val->strval = "BQ25890";
> + else if (bq->chip_id == BQ25896_ID)
> + val->strval = "BQ25896";
> + else
> + val->strval = "UNKNOWN";
> +
> + break;
> +
> case POWER_SUPPLY_PROP_ONLINE:
> val->intval = state.online;
> break;
> @@ -647,6 +660,7 @@ static int bq25890_hw_init(struct bq25890_device *bq)
>
> static enum power_supply_property bq25890_power_supply_props[] = {
> POWER_SUPPLY_PROP_MANUFACTURER,
> + POWER_SUPPLY_PROP_MODEL_NAME,
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_ONLINE,
> POWER_SUPPLY_PROP_HEALTH,
> @@ -848,7 +862,7 @@ static int bq25890_probe(struct i2c_client *client,
> return bq->chip_id;
> }
>
> - if (bq->chip_id != BQ25890_ID) {
> + if ((bq->chip_id != BQ25890_ID) && (bq->chip_id != BQ25896_ID)) {
> dev_err(dev, "Chip with ID=%d, not supported!\n", bq->chip_id);
> return -ENODEV;
> }
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 4/4] power: bq25890_charger.c: Read back the current battery voltage
2018-07-31 16:43 ` [PATCH v3 4/4] power: bq25890_charger.c: Read back the current battery voltage Angus Ainslie (Purism)
@ 2018-07-31 17:02 ` Krzysztof Kozlowski
0 siblings, 0 replies; 32+ messages in thread
From: Krzysztof Kozlowski @ 2018-07-31 17:02 UTC (permalink / raw)
To: Angus Ainslie (Purism)
Cc: angus.ainslie, Sebastian Reichel, linux-pm, linux-kernel
On 31 July 2018 at 18:43, Angus Ainslie (Purism) <angus@akkea.ca> wrote:
> The part has the capability of reading the current battery voltage.
Instead of "part": BQ25890-family of chargers? Chip?
Anyway,
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
Best regards,
Krzysztof
>
> Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
> ---
> drivers/power/supply/bq25890_charger.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
> index 23b39da07e56..1aa7872ddeb0 100644
> --- a/drivers/power/supply/bq25890_charger.c
> +++ b/drivers/power/supply/bq25890_charger.c
> @@ -461,6 +461,15 @@ 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_VOLTAGE_NOW:
> + ret = bq25890_field_read(bq, F_SYSV); /* read measured value */
> + if (ret < 0)
> + return ret;
> +
> + /* converted_val = 2.304V + ADC_val * 20mV (table 10.3.15) */
> + val->intval = 2304000 + ret * 20000;
> + break;
> +
> default:
> return -EINVAL;
> }
> @@ -669,6 +678,7 @@ static enum power_supply_property bq25890_power_supply_props[] = {
> POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
> POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX,
> POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT,
> + POWER_SUPPLY_PROP_VOLTAGE_NOW,
> };
>
> static char *bq25890_charger_supplied_to[] = {
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v4 0/4] Add the BQ25896 charger IC
2018-07-25 9:58 ` Krzysztof Kozlowski
` (2 preceding siblings ...)
2018-07-31 16:43 ` [PATCH v3 0/4] Add the BQ25896 charger IC Angus Ainslie (Purism)
@ 2018-07-31 17:49 ` Angus Ainslie (Purism)
2018-07-31 17:49 ` [PATCH v4 1/4] power: bq25890_charger.c: Add debugging output of failed initialization Angus Ainslie (Purism)
` (4 more replies)
3 siblings, 5 replies; 32+ messages in thread
From: Angus Ainslie (Purism) @ 2018-07-31 17:49 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: angus.ainslie, Sebastian Reichel, linux-pm, linux-kernel,
Angus Ainslie (Purism)
Changes since v3:
Corrected reviewed-by tags.
Fix typos.
Clarify desscriptions of patches.
Changes since v2:
Remove additional un-used table entries.
Use the exsisting compitble string for the new part.
Voltage now doesn't depend on state "online".
Angus Ainslie (Purism) (4):
power: bq25890_charger.c: Add debugging output of failed
initialization
power: bq25890_charger.c: Remove unused table entries
power: bq25890_charger.c: Add the BQ25896 part
power: bq25890_charger.c: Read back the current battery voltage
.../bindings/power/supply/bq25890.txt | 3 +
drivers/power/supply/bq25890_charger.c | 62 ++++++++++++++-----
2 files changed, 50 insertions(+), 15 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v4 1/4] power: bq25890_charger.c: Add debugging output of failed initialization
2018-07-31 17:49 ` [PATCH v4 0/4] Add the BQ25896 charger IC Angus Ainslie (Purism)
@ 2018-07-31 17:49 ` Angus Ainslie (Purism)
2018-07-31 17:49 ` [PATCH v4 2/4] power: bq25890_charger.c: Remove unused table entries Angus Ainslie (Purism)
` (3 subsequent siblings)
4 siblings, 0 replies; 32+ messages in thread
From: Angus Ainslie (Purism) @ 2018-07-31 17:49 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: angus.ainslie, Sebastian Reichel, linux-pm, linux-kernel,
Angus Ainslie (Purism)
To ease adding a new part variant some debugging is handy.
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
drivers/power/supply/bq25890_charger.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index 8e2c41ded171..7f0b3a46c50c 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -608,30 +608,40 @@ static int bq25890_hw_init(struct bq25890_device *bq)
};
ret = bq25890_chip_reset(bq);
- if (ret < 0)
+ if (ret < 0) {
+ dev_dbg(bq->dev, "Reset failed %d\n", ret);
return ret;
+ };
/* disable watchdog */
ret = bq25890_field_write(bq, F_WD, 0);
- if (ret < 0)
+ if (ret < 0) {
+ dev_dbg(bq->dev, "Disabling watchdog failed %d\n", ret);
return ret;
+ };
/* initialize currents/voltages and other parameters */
for (i = 0; i < ARRAY_SIZE(init_data); i++) {
ret = bq25890_field_write(bq, init_data[i].id,
init_data[i].value);
- if (ret < 0)
+ if (ret < 0) {
+ dev_dbg(bq->dev, "Writing init data failed %d\n", ret);
return ret;
+ };
}
/* Configure ADC for continuous conversions. This does not enable it. */
ret = bq25890_field_write(bq, F_CONV_RATE, 1);
- if (ret < 0)
+ if (ret < 0) {
+ dev_dbg(bq->dev, "Config ADC failed %d\n", ret);
return ret;
+ };
ret = bq25890_get_chip_state(bq, &state);
- if (ret < 0)
+ if (ret < 0) {
+ dev_dbg(bq->dev, "Get state failed %d\n", ret);
return ret;
+ };
mutex_lock(&bq->lock);
bq->state = state;
@@ -767,6 +777,9 @@ static int bq25890_fw_read_u32_props(struct bq25890_device *bq)
if (props[i].optional)
continue;
+ dev_err(bq->dev, "Unable to read property %d %s\n", ret,
+ props[i].name);
+
return ret;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v4 2/4] power: bq25890_charger.c: Remove unused table entries
2018-07-31 17:49 ` [PATCH v4 0/4] Add the BQ25896 charger IC Angus Ainslie (Purism)
2018-07-31 17:49 ` [PATCH v4 1/4] power: bq25890_charger.c: Add debugging output of failed initialization Angus Ainslie (Purism)
@ 2018-07-31 17:49 ` Angus Ainslie (Purism)
2018-07-31 17:49 ` [PATCH v4 3/4] power: bq25890_charger.c: Add the BQ25896 part Angus Ainslie (Purism)
` (2 subsequent siblings)
4 siblings, 0 replies; 32+ messages in thread
From: Angus Ainslie (Purism) @ 2018-07-31 17:49 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: angus.ainslie, Sebastian Reichel, linux-pm, linux-kernel,
Angus Ainslie (Purism)
There are a few table entries that aren't used. Drop them.
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
drivers/power/supply/bq25890_charger.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index 7f0b3a46c50c..ca61278e932e 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -244,10 +244,7 @@ enum bq25890_table_ids {
/* range tables */
TBL_ICHG,
TBL_ITERM,
- TBL_IPRECHG,
TBL_VREG,
- TBL_BATCMP,
- TBL_VCLAMP,
TBL_BOOSTV,
TBL_SYSVMIN,
@@ -287,8 +284,6 @@ static const union {
[TBL_ICHG] = { .rt = {0, 5056000, 64000} }, /* uA */
[TBL_ITERM] = { .rt = {64000, 1024000, 64000} }, /* uA */
[TBL_VREG] = { .rt = {3840000, 4608000, 16000} }, /* uV */
- [TBL_BATCMP] = { .rt = {0, 140, 20} }, /* mOhm */
- [TBL_VCLAMP] = { .rt = {0, 224000, 32000} }, /* uV */
[TBL_BOOSTV] = { .rt = {4550000, 5510000, 64000} }, /* uV */
[TBL_SYSVMIN] = { .rt = {3000000, 3700000, 100000} }, /* uV */
--
2.17.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v4 3/4] power: bq25890_charger.c: Add the BQ25896 part
2018-07-31 17:49 ` [PATCH v4 0/4] Add the BQ25896 charger IC Angus Ainslie (Purism)
2018-07-31 17:49 ` [PATCH v4 1/4] power: bq25890_charger.c: Add debugging output of failed initialization Angus Ainslie (Purism)
2018-07-31 17:49 ` [PATCH v4 2/4] power: bq25890_charger.c: Remove unused table entries Angus Ainslie (Purism)
@ 2018-07-31 17:49 ` Angus Ainslie (Purism)
2018-07-31 17:49 ` [PATCH v4 4/4] power: bq25890_charger.c: Read back the current battery voltage Angus Ainslie (Purism)
2018-08-30 21:46 ` [PATCH v4 0/4] Add the BQ25896 charger IC Sebastian Reichel
4 siblings, 0 replies; 32+ messages in thread
From: Angus Ainslie (Purism) @ 2018-07-31 17:49 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: angus.ainslie, Sebastian Reichel, linux-pm, linux-kernel,
Angus Ainslie (Purism)
The BQ25896 is almost identical to the BQ25890.
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
.../bindings/power/supply/bq25890.txt | 3 +++
drivers/power/supply/bq25890_charger.c | 24 +++++++++++++++----
2 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/power/supply/bq25890.txt b/Documentation/devicetree/bindings/power/supply/bq25890.txt
index c9dd17d142ad..dc0568933359 100644
--- a/Documentation/devicetree/bindings/power/supply/bq25890.txt
+++ b/Documentation/devicetree/bindings/power/supply/bq25890.txt
@@ -1,5 +1,8 @@
Binding for TI bq25890 Li-Ion Charger
+This driver will support the bq25896 and the bq25890. There are other ICs
+in the same family but those have not been tested.
+
Required properties:
- compatible: Should contain one of the following:
* "ti,bq25890"
diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index ca61278e932e..23b39da07e56 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -32,6 +32,7 @@
#define BQ25890_IRQ_PIN "bq25890_irq"
#define BQ25890_ID 3
+#define BQ25896_ID 0
enum bq25890_fields {
F_EN_HIZ, F_EN_ILIM, F_IILIM, /* Reg00 */
@@ -153,8 +154,8 @@ static const struct reg_field bq25890_reg_fields[] = {
[F_CONV_RATE] = REG_FIELD(0x02, 6, 6),
[F_BOOSTF] = REG_FIELD(0x02, 5, 5),
[F_ICO_EN] = REG_FIELD(0x02, 4, 4),
- [F_HVDCP_EN] = REG_FIELD(0x02, 3, 3),
- [F_MAXC_EN] = REG_FIELD(0x02, 2, 2),
+ [F_HVDCP_EN] = REG_FIELD(0x02, 3, 3), // reserved on BQ25896
+ [F_MAXC_EN] = REG_FIELD(0x02, 2, 2), // reserved on BQ25896
[F_FORCE_DPM] = REG_FIELD(0x02, 1, 1),
[F_AUTO_DPDM_EN] = REG_FIELD(0x02, 0, 0),
/* REG03 */
@@ -163,6 +164,7 @@ static const struct reg_field bq25890_reg_fields[] = {
[F_OTG_CFG] = REG_FIELD(0x03, 5, 5),
[F_CHG_CFG] = REG_FIELD(0x03, 4, 4),
[F_SYSVMIN] = REG_FIELD(0x03, 1, 3),
+ /* MIN_VBAT_SEL on BQ25896 */
/* REG04 */
[F_PUMPX_EN] = REG_FIELD(0x04, 7, 7),
[F_ICHG] = REG_FIELD(0x04, 0, 6),
@@ -181,7 +183,7 @@ static const struct reg_field bq25890_reg_fields[] = {
[F_CHG_TMR] = REG_FIELD(0x07, 1, 2),
[F_JEITA_ISET] = REG_FIELD(0x07, 0, 0),
/* REG08 */
- [F_BATCMP] = REG_FIELD(0x08, 6, 7),
+ [F_BATCMP] = REG_FIELD(0x08, 6, 7), // 5-7 on BQ25896
[F_VCLAMP] = REG_FIELD(0x08, 2, 4),
[F_TREG] = REG_FIELD(0x08, 0, 1),
/* REG09 */
@@ -195,12 +197,13 @@ static const struct reg_field bq25890_reg_fields[] = {
[F_PUMPX_DN] = REG_FIELD(0x09, 0, 0),
/* REG0A */
[F_BOOSTV] = REG_FIELD(0x0A, 4, 7),
+ /* PFM_OTG_DIS 3 on BQ25896 */
[F_BOOSTI] = REG_FIELD(0x0A, 0, 2),
/* REG0B */
[F_VBUS_STAT] = REG_FIELD(0x0B, 5, 7),
[F_CHG_STAT] = REG_FIELD(0x0B, 3, 4),
[F_PG_STAT] = REG_FIELD(0x0B, 2, 2),
- [F_SDP_STAT] = REG_FIELD(0x0B, 1, 1),
+ [F_SDP_STAT] = REG_FIELD(0x0B, 1, 1), // reserved on BQ25896
[F_VSYS_STAT] = REG_FIELD(0x0B, 0, 0),
/* REG0C */
[F_WD_FAULT] = REG_FIELD(0x0C, 7, 7),
@@ -396,6 +399,16 @@ static int bq25890_power_supply_get_property(struct power_supply *psy,
val->strval = BQ25890_MANUFACTURER;
break;
+ case POWER_SUPPLY_PROP_MODEL_NAME:
+ if (bq->chip_id == BQ25890_ID)
+ val->strval = "BQ25890";
+ else if (bq->chip_id == BQ25896_ID)
+ val->strval = "BQ25896";
+ else
+ val->strval = "UNKNOWN";
+
+ break;
+
case POWER_SUPPLY_PROP_ONLINE:
val->intval = state.online;
break;
@@ -647,6 +660,7 @@ static int bq25890_hw_init(struct bq25890_device *bq)
static enum power_supply_property bq25890_power_supply_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
+ POWER_SUPPLY_PROP_MODEL_NAME,
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_ONLINE,
POWER_SUPPLY_PROP_HEALTH,
@@ -848,7 +862,7 @@ static int bq25890_probe(struct i2c_client *client,
return bq->chip_id;
}
- if (bq->chip_id != BQ25890_ID) {
+ if ((bq->chip_id != BQ25890_ID) && (bq->chip_id != BQ25896_ID)) {
dev_err(dev, "Chip with ID=%d, not supported!\n", bq->chip_id);
return -ENODEV;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v4 4/4] power: bq25890_charger.c: Read back the current battery voltage
2018-07-31 17:49 ` [PATCH v4 0/4] Add the BQ25896 charger IC Angus Ainslie (Purism)
` (2 preceding siblings ...)
2018-07-31 17:49 ` [PATCH v4 3/4] power: bq25890_charger.c: Add the BQ25896 part Angus Ainslie (Purism)
@ 2018-07-31 17:49 ` Angus Ainslie (Purism)
2018-08-30 21:46 ` [PATCH v4 0/4] Add the BQ25896 charger IC Sebastian Reichel
4 siblings, 0 replies; 32+ messages in thread
From: Angus Ainslie (Purism) @ 2018-07-31 17:49 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: angus.ainslie, Sebastian Reichel, linux-pm, linux-kernel,
Angus Ainslie (Purism)
The BQ2589x family has the capability of reading the current battery voltage.
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
drivers/power/supply/bq25890_charger.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index 23b39da07e56..1aa7872ddeb0 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -461,6 +461,15 @@ 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_VOLTAGE_NOW:
+ ret = bq25890_field_read(bq, F_SYSV); /* read measured value */
+ if (ret < 0)
+ return ret;
+
+ /* converted_val = 2.304V + ADC_val * 20mV (table 10.3.15) */
+ val->intval = 2304000 + ret * 20000;
+ break;
+
default:
return -EINVAL;
}
@@ -669,6 +678,7 @@ static enum power_supply_property bq25890_power_supply_props[] = {
POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX,
POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT,
+ POWER_SUPPLY_PROP_VOLTAGE_NOW,
};
static char *bq25890_charger_supplied_to[] = {
--
2.17.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH v3 1/4] power: bq25890_charger.c: Add debugging output of failed initialization
2018-07-31 16:57 ` Krzysztof Kozlowski
@ 2018-07-31 18:16 ` Angus Ainslie
0 siblings, 0 replies; 32+ messages in thread
From: Angus Ainslie @ 2018-07-31 18:16 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: angus.ainslie, Sebastian Reichel, linux-pm, linux-kernel
On 2018-07-31 10:57, Krzysztof Kozlowski wrote:
> On 31 July 2018 at 18:43, Angus Ainslie (Purism) <angus@akkea.ca>
> wrote:
>> To ease adding a new part variant some debugging is handy.
>>
>> Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
>
> You missed my tag - I already reviewed it. Unless you change
> something, the tag should be added to commit msg:
> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
>
> Krzysztof
>
Hi Krzysztof,
Ooops, sorry about that.
Thanks for the review.
Angus
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v4 0/4] Add the BQ25896 charger IC
2018-07-31 17:49 ` [PATCH v4 0/4] Add the BQ25896 charger IC Angus Ainslie (Purism)
` (3 preceding siblings ...)
2018-07-31 17:49 ` [PATCH v4 4/4] power: bq25890_charger.c: Read back the current battery voltage Angus Ainslie (Purism)
@ 2018-08-30 21:46 ` Sebastian Reichel
4 siblings, 0 replies; 32+ messages in thread
From: Sebastian Reichel @ 2018-08-30 21:46 UTC (permalink / raw)
To: Angus Ainslie (Purism)
Cc: Krzysztof Kozlowski, angus.ainslie, linux-pm, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 913 bytes --]
Hi,
On Tue, Jul 31, 2018 at 11:49:05AM -0600, Angus Ainslie (Purism) wrote:
> Changes since v3:
>
> Corrected reviewed-by tags.
> Fix typos.
> Clarify desscriptions of patches.
>
> Changes since v2:
>
> Remove additional un-used table entries.
> Use the exsisting compitble string for the new part.
> Voltage now doesn't depend on state "online".
Thanks, queued.
-- Sebastian
> Angus Ainslie (Purism) (4):
> power: bq25890_charger.c: Add debugging output of failed
> initialization
> power: bq25890_charger.c: Remove unused table entries
> power: bq25890_charger.c: Add the BQ25896 part
> power: bq25890_charger.c: Read back the current battery voltage
>
> .../bindings/power/supply/bq25890.txt | 3 +
> drivers/power/supply/bq25890_charger.c | 62 ++++++++++++++-----
> 2 files changed, 50 insertions(+), 15 deletions(-)
>
> --
> 2.17.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2018-08-30 21:46 UTC | newest]
Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-23 13:51 [PATCH] bq25890_charger.c : add the BQ25896 part Angus Ainslie
2018-07-25 9:58 ` Krzysztof Kozlowski
2018-07-25 12:17 ` Angus Ainslie
2018-07-26 6:37 ` Krzysztof Kozlowski
2018-07-25 19:46 ` [PATCH v2 0/4] Add the BQ25896 charger IC Angus Ainslie (Purism)
2018-07-25 19:46 ` [PATCH v2 1/4] power: bq25890_charger.c: Add debugging output of failed initialization Angus Ainslie (Purism)
2018-07-26 6:44 ` Krzysztof Kozlowski
2018-07-25 19:46 ` [PATCH v2 2/4] power: bq25890_charger.c: Remove unused table Angus Ainslie (Purism)
2018-07-26 6:51 ` Krzysztof Kozlowski
2018-07-27 15:26 ` Angus Ainslie
2018-07-31 6:40 ` Krzysztof Kozlowski
2018-07-25 19:46 ` [PATCH v2 3/4] power: bq25890_charger.c: Add the BQ25896 part Angus Ainslie (Purism)
2018-07-26 6:56 ` Krzysztof Kozlowski
2018-07-25 19:46 ` [PATCH v2 4/4] power: bq25890_charger.c: Read back the current battery voltage Angus Ainslie (Purism)
2018-07-26 7:03 ` Krzysztof Kozlowski
2018-07-27 15:30 ` Angus Ainslie
2018-07-31 16:43 ` [PATCH v3 0/4] Add the BQ25896 charger IC Angus Ainslie (Purism)
2018-07-31 16:43 ` [PATCH v3 1/4] power: bq25890_charger.c: Add debugging output of failed initialization Angus Ainslie (Purism)
2018-07-31 16:57 ` Krzysztof Kozlowski
2018-07-31 18:16 ` Angus Ainslie
2018-07-31 16:43 ` [PATCH v3 2/4] power: bq25890_charger.c: Remove unused table entries Angus Ainslie (Purism)
2018-07-31 16:58 ` Krzysztof Kozlowski
2018-07-31 16:43 ` [PATCH v3 3/4] power: bq25890_charger.c: Add the BQ25896 part Angus Ainslie (Purism)
2018-07-31 17:00 ` Krzysztof Kozlowski
2018-07-31 16:43 ` [PATCH v3 4/4] power: bq25890_charger.c: Read back the current battery voltage Angus Ainslie (Purism)
2018-07-31 17:02 ` Krzysztof Kozlowski
2018-07-31 17:49 ` [PATCH v4 0/4] Add the BQ25896 charger IC Angus Ainslie (Purism)
2018-07-31 17:49 ` [PATCH v4 1/4] power: bq25890_charger.c: Add debugging output of failed initialization Angus Ainslie (Purism)
2018-07-31 17:49 ` [PATCH v4 2/4] power: bq25890_charger.c: Remove unused table entries Angus Ainslie (Purism)
2018-07-31 17:49 ` [PATCH v4 3/4] power: bq25890_charger.c: Add the BQ25896 part Angus Ainslie (Purism)
2018-07-31 17:49 ` [PATCH v4 4/4] power: bq25890_charger.c: Read back the current battery voltage Angus Ainslie (Purism)
2018-08-30 21:46 ` [PATCH v4 0/4] Add the BQ25896 charger IC 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).