[PATCH] bq25890_charger.c : add the BQ25896 part

[PATCH] bq25890_charger.c : add the BQ25896 part

[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

Re: [PATCH] bq25890_charger.c : add the BQ25896 part

[Krzysztof Kozlowski]

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

Re: [PATCH] bq25890_charger.c : add the BQ25896 part

[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

Re: [PATCH] bq25890_charger.c : add the BQ25896 part

[Krzysztof Kozlowski]

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

[PATCH v2 0/4] Add the BQ25896 charger IC

[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

[PATCH v2 1/4] power: bq25890_charger.c: Add debugging output of failed initialization

[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

Re: [PATCH v2 1/4] power: bq25890_charger.c: Add debugging output of failed initialization

[Krzysztof Kozlowski]

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

[PATCH v2 2/4] power: bq25890_charger.c: Remove unused table

[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

Re: [PATCH v2 2/4] power: bq25890_charger.c: Remove unused table

[Krzysztof Kozlowski]

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

Re: [PATCH v2 2/4] power: bq25890_charger.c: Remove unused table

[Angus Ainslie]

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

Re: [PATCH v2 2/4] power: bq25890_charger.c: Remove unused table

[Krzysztof Kozlowski]

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

[PATCH v2 3/4] power: bq25890_charger.c: Add the BQ25896 part

[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

Re: [PATCH v2 3/4] power: bq25890_charger.c: Add the BQ25896 part

[Krzysztof Kozlowski]

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
>

[PATCH v2 4/4] power: bq25890_charger.c: Read back the current battery voltage

[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

Re: [PATCH v2 4/4] power: bq25890_charger.c: Read back the current battery voltage

[Krzysztof Kozlowski]

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
>

Re: [PATCH v2 4/4] power: bq25890_charger.c: Read back the current battery voltage

[Angus Ainslie]

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
>> 

[PATCH v3 0/4] Add the BQ25896 charger IC

[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

[PATCH v3 1/4] power: bq25890_charger.c: Add debugging output of failed initialization

[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

Re: [PATCH v3 1/4] power: bq25890_charger.c: Add debugging output of failed initialization

[Krzysztof Kozlowski]

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
>

Re: [PATCH v3 1/4] power: bq25890_charger.c: Add debugging output of failed initialization

[Angus Ainslie]

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

[PATCH v3 2/4] power: bq25890_charger.c: Remove unused table entries

[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

Re: [PATCH v3 2/4] power: bq25890_charger.c: Remove unused table entries

[Krzysztof Kozlowski]

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

[PATCH v3 3/4] power: bq25890_charger.c: Add the BQ25896 part

[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

Re: [PATCH v3 3/4] power: bq25890_charger.c: Add the BQ25896 part

[Krzysztof Kozlowski]

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
>

[PATCH v3 4/4] power: bq25890_charger.c: Read back the current battery voltage

[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

Re: [PATCH v3 4/4] power: bq25890_charger.c: Read back the current battery voltage

[Krzysztof Kozlowski]

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
>

[PATCH v4 0/4] Add the BQ25896 charger IC

[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

[PATCH v4 1/4] power: bq25890_charger.c: Add debugging output of failed initialization

[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

[PATCH v4 2/4] power: bq25890_charger.c: Remove unused table entries

[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

[PATCH v4 3/4] power: bq25890_charger.c: Add the BQ25896 part

[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

[PATCH v4 4/4] power: bq25890_charger.c: Read back the current battery voltage

[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

Re: [PATCH v4 0/4] Add the BQ25896 charger IC

[Sebastian Reichel]

[-- 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 --]