[PATCH v2 00/11] power: supply: bq25890: fix and extend

[PATCH v2 00/11] power: supply: bq25890: fix and extend

[Michał Mirosław]

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

Patches 1-3 are cleans up the code a bit, 4-6 fix property value
reading, 7-9 add more information to be read from the chip, 10-11 add
IBAT compensation support.

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

Michał Mirosław (11):
  power: bq25890: remove redundant I2C bus check
  power: bq25890: simplify chip name property getter
  power: bq25890: make property table const
  power: bq25890: protect view of the chip's state
  power: bq25890: fix ADC mode configuration
  power: bq25890: update state on property read
  power: bq25890: implement CHARGE_TYPE property
  power: bq25890: implement PRECHARGE_CURRENT property
  power: bq25890: implement INPUT_CURRENT_LIMIT property
  power: bq25890: support IBAT compensation
  power: bq25890: document IBAT compensation DT properties

 .../bindings/power/supply/bq25890.txt         |   4 +
 drivers/power/supply/bq25890_charger.c        | 190 ++++++++++--------
 2 files changed, 113 insertions(+), 81 deletions(-)

-- 
2.20.1

[PATCH v2 03/11] power: bq25890: make property table const

[Michał Mirosław]

Property list should not change, so mark it const.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/power/supply/bq25890_charger.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index f9f29edadddc..c4a69fd69f34 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -668,7 +668,7 @@ static int bq25890_hw_init(struct bq25890_device *bq)
 	return 0;
 }
 
-static enum power_supply_property bq25890_power_supply_props[] = {
+static const enum power_supply_property bq25890_power_supply_props[] = {
 	POWER_SUPPLY_PROP_MANUFACTURER,
 	POWER_SUPPLY_PROP_MODEL_NAME,
 	POWER_SUPPLY_PROP_STATUS,
-- 
2.20.1

Re: [PATCH v2 03/11] power: bq25890: make property table const

[Sebastian Reichel]

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

Hi,

On Sun, May 03, 2020 at 05:21:10PM +0200, Michał Mirosław wrote:
> Property list should not change, so mark it const.
> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---

Thanks, queued.

-- Sebastian

>  drivers/power/supply/bq25890_charger.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
> index f9f29edadddc..c4a69fd69f34 100644
> --- a/drivers/power/supply/bq25890_charger.c
> +++ b/drivers/power/supply/bq25890_charger.c
> @@ -668,7 +668,7 @@ static int bq25890_hw_init(struct bq25890_device *bq)
>  	return 0;
>  }
>  
> -static enum power_supply_property bq25890_power_supply_props[] = {
> +static const enum power_supply_property bq25890_power_supply_props[] = {
>  	POWER_SUPPLY_PROP_MANUFACTURER,
>  	POWER_SUPPLY_PROP_MODEL_NAME,
>  	POWER_SUPPLY_PROP_STATUS,
> -- 
> 2.20.1
> 

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

[PATCH v2 01/11] power: bq25890: remove redundant I2C bus check

[Michał Mirosław]

regmap initialization will check I2C adapter functionality.
Remove redundant check in the driver.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/power/supply/bq25890_charger.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index aebd1253dbc9..c642519ef7b2 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -881,17 +881,11 @@ static int bq25890_fw_probe(struct bq25890_device *bq)
 static int bq25890_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
-	struct i2c_adapter *adapter = client->adapter;
 	struct device *dev = &client->dev;
 	struct bq25890_device *bq;
 	int ret;
 	int i;
 
-	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) {
-		dev_err(dev, "No support for SMBUS_BYTE_DATA\n");
-		return -ENODEV;
-	}
-
 	bq = devm_kzalloc(dev, sizeof(*bq), GFP_KERNEL);
 	if (!bq)
 		return -ENOMEM;
-- 
2.20.1

Re: [PATCH v2 01/11] power: bq25890: remove redundant I2C bus check

[Sebastian Reichel]

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

Hi,

On Sun, May 03, 2020 at 05:21:10PM +0200, Michał Mirosław wrote:
> regmap initialization will check I2C adapter functionality.
> Remove redundant check in the driver.
> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---

Thanks, queued.

-- Sebastian

>  drivers/power/supply/bq25890_charger.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
> index aebd1253dbc9..c642519ef7b2 100644
> --- a/drivers/power/supply/bq25890_charger.c
> +++ b/drivers/power/supply/bq25890_charger.c
> @@ -881,17 +881,11 @@ static int bq25890_fw_probe(struct bq25890_device *bq)
>  static int bq25890_probe(struct i2c_client *client,
>  			 const struct i2c_device_id *id)
>  {
> -	struct i2c_adapter *adapter = client->adapter;
>  	struct device *dev = &client->dev;
>  	struct bq25890_device *bq;
>  	int ret;
>  	int i;
>  
> -	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) {
> -		dev_err(dev, "No support for SMBUS_BYTE_DATA\n");
> -		return -ENODEV;
> -	}
> -
>  	bq = devm_kzalloc(dev, sizeof(*bq), GFP_KERNEL);
>  	if (!bq)
>  		return -ENOMEM;
> -- 
> 2.20.1
> 

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

[PATCH v2 02/11] power: bq25890: simplify chip name property getter

[Michał Mirosław]

Driver rejects unknown chips early in the probe(), so when
bq25890_power_supply_get_property() is made reachable, bq->chip_version
will already be set to correct value - there is no need to check
it again.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/power/supply/bq25890_charger.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index c642519ef7b2..f9f29edadddc 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -32,6 +32,13 @@ enum bq25890_chip_version {
 	BQ25896,
 };
 
+static const char *const bq25890_chip_name[] = {
+	"BQ25890",
+	"BQ25892",
+	"BQ25895",
+	"BQ25896",
+};
+
 enum bq25890_fields {
 	F_EN_HIZ, F_EN_ILIM, F_IILIM,				     /* Reg00 */
 	F_BHOT, F_BCOLD, F_VINDPM_OFS,				     /* Reg01 */
@@ -400,17 +407,7 @@ static int bq25890_power_supply_get_property(struct power_supply *psy,
 		break;
 
 	case POWER_SUPPLY_PROP_MODEL_NAME:
-		if (bq->chip_version == BQ25890)
-			val->strval = "BQ25890";
-		else if (bq->chip_version == BQ25892)
-			val->strval = "BQ25892";
-		else if (bq->chip_version == BQ25895)
-			val->strval = "BQ25895";
-		else if (bq->chip_version == BQ25896)
-			val->strval = "BQ25896";
-		else
-			val->strval = "UNKNOWN";
-
+		val->strval = bq25890_chip_name[bq->chip_version];
 		break;
 
 	case POWER_SUPPLY_PROP_ONLINE:
-- 
2.20.1

Re: [PATCH v2 02/11] power: bq25890: simplify chip name property getter

[Sebastian Reichel]

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

Hi,

On Sun, May 03, 2020 at 05:21:10PM +0200, Michał Mirosław wrote:
> Driver rejects unknown chips early in the probe(), so when
> bq25890_power_supply_get_property() is made reachable, bq->chip_version
> will already be set to correct value - there is no need to check
> it again.
> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---

Thanks, queued.

-- Sebastian

>  drivers/power/supply/bq25890_charger.c | 19 ++++++++-----------
>  1 file changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
> index c642519ef7b2..f9f29edadddc 100644
> --- a/drivers/power/supply/bq25890_charger.c
> +++ b/drivers/power/supply/bq25890_charger.c
> @@ -32,6 +32,13 @@ enum bq25890_chip_version {
>  	BQ25896,
>  };
>  
> +static const char *const bq25890_chip_name[] = {
> +	"BQ25890",
> +	"BQ25892",
> +	"BQ25895",
> +	"BQ25896",
> +};
> +
>  enum bq25890_fields {
>  	F_EN_HIZ, F_EN_ILIM, F_IILIM,				     /* Reg00 */
>  	F_BHOT, F_BCOLD, F_VINDPM_OFS,				     /* Reg01 */
> @@ -400,17 +407,7 @@ static int bq25890_power_supply_get_property(struct power_supply *psy,
>  		break;
>  
>  	case POWER_SUPPLY_PROP_MODEL_NAME:
> -		if (bq->chip_version == BQ25890)
> -			val->strval = "BQ25890";
> -		else if (bq->chip_version == BQ25892)
> -			val->strval = "BQ25892";
> -		else if (bq->chip_version == BQ25895)
> -			val->strval = "BQ25895";
> -		else if (bq->chip_version == BQ25896)
> -			val->strval = "BQ25896";
> -		else
> -			val->strval = "UNKNOWN";
> -
> +		val->strval = bq25890_chip_name[bq->chip_version];
>  		break;
>  
>  	case POWER_SUPPLY_PROP_ONLINE:
> -- 
> 2.20.1
> 

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

[PATCH v2 04/11] power: bq25890: protect view of the chip's state

[Michał Mirosław]

Extend bq->lock over whole updating of the chip's state. Might get
useful later for switching ADC modes correctly.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/power/supply/bq25890_charger.c | 82 ++++++++------------------
 1 file changed, 26 insertions(+), 56 deletions(-)

diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index c4a69fd69f34..9339e216651f 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -510,74 +510,50 @@ static int bq25890_get_chip_state(struct bq25890_device *bq,
 	return 0;
 }
 
-static bool bq25890_state_changed(struct bq25890_device *bq,
-				  struct bq25890_state *new_state)
-{
-	struct bq25890_state old_state;
-
-	mutex_lock(&bq->lock);
-	old_state = bq->state;
-	mutex_unlock(&bq->lock);
-
-	return (old_state.chrg_status != new_state->chrg_status ||
-		old_state.chrg_fault != new_state->chrg_fault	||
-		old_state.online != new_state->online		||
-		old_state.bat_fault != new_state->bat_fault	||
-		old_state.boost_fault != new_state->boost_fault ||
-		old_state.vsys_status != new_state->vsys_status);
-}
-
-static void bq25890_handle_state_change(struct bq25890_device *bq,
-					struct bq25890_state *new_state)
+static irqreturn_t __bq25890_handle_irq(struct bq25890_device *bq)
 {
+	struct bq25890_state new_state;
 	int ret;
-	struct bq25890_state old_state;
 
-	mutex_lock(&bq->lock);
-	old_state = bq->state;
-	mutex_unlock(&bq->lock);
+	ret = bq25890_get_chip_state(bq, &new_state);
+	if (ret < 0)
+		return IRQ_NONE;
 
-	if (!new_state->online) {			     /* power removed */
+	if (!memcmp(&bq->state, &new_state, sizeof(new_state)))
+		return IRQ_NONE;
+
+	if (!new_state.online && bq->state.online) {	    /* power removed */
 		/* disable ADC */
 		ret = bq25890_field_write(bq, F_CONV_START, 0);
 		if (ret < 0)
 			goto error;
-	} else if (!old_state.online) {			    /* power inserted */
+	} else if (new_state.online && !bq->state.online) { /* power inserted */
 		/* enable ADC, to have control of charge current/voltage */
 		ret = bq25890_field_write(bq, F_CONV_START, 1);
 		if (ret < 0)
 			goto error;
 	}
 
-	return;
+	bq->state = new_state;
+	power_supply_changed(bq->charger);
 
+	return IRQ_HANDLED;
 error:
-	dev_err(bq->dev, "Error communicating with the chip.\n");
+	dev_err(bq->dev, "Error communicating with the chip: %pe\n",
+		ERR_PTR(ret));
+	return IRQ_HANDLED;
 }
 
 static irqreturn_t bq25890_irq_handler_thread(int irq, void *private)
 {
 	struct bq25890_device *bq = private;
-	int ret;
-	struct bq25890_state state;
-
-	ret = bq25890_get_chip_state(bq, &state);
-	if (ret < 0)
-		goto handled;
-
-	if (!bq25890_state_changed(bq, &state))
-		goto handled;
-
-	bq25890_handle_state_change(bq, &state);
+	irqreturn_t ret;
 
 	mutex_lock(&bq->lock);
-	bq->state = state;
+	ret = __bq25890_handle_irq(bq);
 	mutex_unlock(&bq->lock);
 
-	power_supply_changed(bq->charger);
-
-handled:
-	return IRQ_HANDLED;
+	return ret;
 }
 
 static int bq25890_chip_reset(struct bq25890_device *bq)
@@ -607,7 +583,6 @@ static int bq25890_hw_init(struct bq25890_device *bq)
 {
 	int ret;
 	int i;
-	struct bq25890_state state;
 
 	const struct {
 		enum bq25890_fields id;
@@ -655,16 +630,12 @@ static int bq25890_hw_init(struct bq25890_device *bq)
 		return ret;
 	}
 
-	ret = bq25890_get_chip_state(bq, &state);
+	ret = bq25890_get_chip_state(bq, &bq->state);
 	if (ret < 0) {
 		dev_dbg(bq->dev, "Get state failed %d\n", ret);
 		return ret;
 	}
 
-	mutex_lock(&bq->lock);
-	bq->state = state;
-	mutex_unlock(&bq->lock);
-
 	return 0;
 }
 
@@ -1001,19 +972,16 @@ static int bq25890_suspend(struct device *dev)
 static int bq25890_resume(struct device *dev)
 {
 	int ret;
-	struct bq25890_state state;
 	struct bq25890_device *bq = dev_get_drvdata(dev);
 
-	ret = bq25890_get_chip_state(bq, &state);
+	mutex_lock(&bq->lock);
+
+	ret = bq25890_get_chip_state(bq, &bq->state);
 	if (ret < 0)
 		return ret;
 
-	mutex_lock(&bq->lock);
-	bq->state = state;
-	mutex_unlock(&bq->lock);
-
 	/* Re-enable ADC only if charger is plugged in. */
-	if (state.online) {
+	if (bq->state.online) {
 		ret = bq25890_field_write(bq, F_CONV_START, 1);
 		if (ret < 0)
 			return ret;
@@ -1022,6 +990,8 @@ static int bq25890_resume(struct device *dev)
 	/* signal userspace, maybe state changed while suspended */
 	power_supply_changed(bq->charger);
 
+	mutex_unlock(&bq->lock);
+
 	return 0;
 }
 #endif
-- 
2.20.1

Re: [PATCH v2 04/11] power: bq25890: protect view of the chip's state

[Sebastian Reichel]

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

Hi,

On Sun, May 03, 2020 at 05:21:11PM +0200, Michał Mirosław wrote:
> Extend bq->lock over whole updating of the chip's state. Might get
> useful later for switching ADC modes correctly.
> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---

Thanks, queued.

-- Sebastian

>  drivers/power/supply/bq25890_charger.c | 82 ++++++++------------------
>  1 file changed, 26 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
> index c4a69fd69f34..9339e216651f 100644
> --- a/drivers/power/supply/bq25890_charger.c
> +++ b/drivers/power/supply/bq25890_charger.c
> @@ -510,74 +510,50 @@ static int bq25890_get_chip_state(struct bq25890_device *bq,
>  	return 0;
>  }
>  
> -static bool bq25890_state_changed(struct bq25890_device *bq,
> -				  struct bq25890_state *new_state)
> -{
> -	struct bq25890_state old_state;
> -
> -	mutex_lock(&bq->lock);
> -	old_state = bq->state;
> -	mutex_unlock(&bq->lock);
> -
> -	return (old_state.chrg_status != new_state->chrg_status ||
> -		old_state.chrg_fault != new_state->chrg_fault	||
> -		old_state.online != new_state->online		||
> -		old_state.bat_fault != new_state->bat_fault	||
> -		old_state.boost_fault != new_state->boost_fault ||
> -		old_state.vsys_status != new_state->vsys_status);
> -}
> -
> -static void bq25890_handle_state_change(struct bq25890_device *bq,
> -					struct bq25890_state *new_state)
> +static irqreturn_t __bq25890_handle_irq(struct bq25890_device *bq)
>  {
> +	struct bq25890_state new_state;
>  	int ret;
> -	struct bq25890_state old_state;
>  
> -	mutex_lock(&bq->lock);
> -	old_state = bq->state;
> -	mutex_unlock(&bq->lock);
> +	ret = bq25890_get_chip_state(bq, &new_state);
> +	if (ret < 0)
> +		return IRQ_NONE;
>  
> -	if (!new_state->online) {			     /* power removed */
> +	if (!memcmp(&bq->state, &new_state, sizeof(new_state)))
> +		return IRQ_NONE;
> +
> +	if (!new_state.online && bq->state.online) {	    /* power removed */
>  		/* disable ADC */
>  		ret = bq25890_field_write(bq, F_CONV_START, 0);
>  		if (ret < 0)
>  			goto error;
> -	} else if (!old_state.online) {			    /* power inserted */
> +	} else if (new_state.online && !bq->state.online) { /* power inserted */
>  		/* enable ADC, to have control of charge current/voltage */
>  		ret = bq25890_field_write(bq, F_CONV_START, 1);
>  		if (ret < 0)
>  			goto error;
>  	}
>  
> -	return;
> +	bq->state = new_state;
> +	power_supply_changed(bq->charger);
>  
> +	return IRQ_HANDLED;
>  error:
> -	dev_err(bq->dev, "Error communicating with the chip.\n");
> +	dev_err(bq->dev, "Error communicating with the chip: %pe\n",
> +		ERR_PTR(ret));
> +	return IRQ_HANDLED;
>  }
>  
>  static irqreturn_t bq25890_irq_handler_thread(int irq, void *private)
>  {
>  	struct bq25890_device *bq = private;
> -	int ret;
> -	struct bq25890_state state;
> -
> -	ret = bq25890_get_chip_state(bq, &state);
> -	if (ret < 0)
> -		goto handled;
> -
> -	if (!bq25890_state_changed(bq, &state))
> -		goto handled;
> -
> -	bq25890_handle_state_change(bq, &state);
> +	irqreturn_t ret;
>  
>  	mutex_lock(&bq->lock);
> -	bq->state = state;
> +	ret = __bq25890_handle_irq(bq);
>  	mutex_unlock(&bq->lock);
>  
> -	power_supply_changed(bq->charger);
> -
> -handled:
> -	return IRQ_HANDLED;
> +	return ret;
>  }
>  
>  static int bq25890_chip_reset(struct bq25890_device *bq)
> @@ -607,7 +583,6 @@ static int bq25890_hw_init(struct bq25890_device *bq)
>  {
>  	int ret;
>  	int i;
> -	struct bq25890_state state;
>  
>  	const struct {
>  		enum bq25890_fields id;
> @@ -655,16 +630,12 @@ static int bq25890_hw_init(struct bq25890_device *bq)
>  		return ret;
>  	}
>  
> -	ret = bq25890_get_chip_state(bq, &state);
> +	ret = bq25890_get_chip_state(bq, &bq->state);
>  	if (ret < 0) {
>  		dev_dbg(bq->dev, "Get state failed %d\n", ret);
>  		return ret;
>  	}
>  
> -	mutex_lock(&bq->lock);
> -	bq->state = state;
> -	mutex_unlock(&bq->lock);
> -
>  	return 0;
>  }
>  
> @@ -1001,19 +972,16 @@ static int bq25890_suspend(struct device *dev)
>  static int bq25890_resume(struct device *dev)
>  {
>  	int ret;
> -	struct bq25890_state state;
>  	struct bq25890_device *bq = dev_get_drvdata(dev);
>  
> -	ret = bq25890_get_chip_state(bq, &state);
> +	mutex_lock(&bq->lock);
> +
> +	ret = bq25890_get_chip_state(bq, &bq->state);
>  	if (ret < 0)
>  		return ret;
>  
> -	mutex_lock(&bq->lock);
> -	bq->state = state;
> -	mutex_unlock(&bq->lock);
> -
>  	/* Re-enable ADC only if charger is plugged in. */
> -	if (state.online) {
> +	if (bq->state.online) {
>  		ret = bq25890_field_write(bq, F_CONV_START, 1);
>  		if (ret < 0)
>  			return ret;
> @@ -1022,6 +990,8 @@ static int bq25890_resume(struct device *dev)
>  	/* signal userspace, maybe state changed while suspended */
>  	power_supply_changed(bq->charger);
>  
> +	mutex_unlock(&bq->lock);
> +
>  	return 0;
>  }
>  #endif
> -- 
> 2.20.1
> 

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

[PATCH v2 05/11] power: bq25890: fix ADC mode configuration

[Michał Mirosław]

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

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

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

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/power/supply/bq25890_charger.c | 33 ++++++++++++++++++++++----
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index 9339e216651f..3b02fa80aedd 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -126,6 +126,7 @@ static const struct regmap_access_table bq25890_writeable_regs = {
 
 static const struct regmap_range bq25890_volatile_reg_ranges[] = {
 	regmap_reg_range(0x00, 0x00),
+	regmap_reg_range(0x02, 0x02),
 	regmap_reg_range(0x09, 0x09),
 	regmap_reg_range(0x0b, 0x14),
 };
@@ -374,18 +375,40 @@ enum bq25890_chrg_fault {
 	CHRG_FAULT_TIMER_EXPIRED,
 };
 
+static bool bq25890_is_adc_property(enum power_supply_property psp)
+{
+	switch (psp) {
+	case POWER_SUPPLY_PROP_INPUT_VOLTAGE_NOW:
+	case POWER_SUPPLY_PROP_OUTPUT_VOLTAGE_NOW:
+	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+	case POWER_SUPPLY_PROP_CURRENT_NOW:
+		return true;
+
+	default:
+		return false;
+	}
+}
+
 static int bq25890_power_supply_get_property(struct power_supply *psy,
 					     enum power_supply_property psp,
 					     union power_supply_propval *val)
 {
-	int ret;
 	struct bq25890_device *bq = power_supply_get_drvdata(psy);
 	struct bq25890_state state;
+	bool do_adc_conv;
+	int ret;
 
 	mutex_lock(&bq->lock);
 	state = bq->state;
+	do_adc_conv = !state.online && bq25890_is_adc_property(psp);
+	if (do_adc_conv)
+		bq25890_field_write(bq, F_CONV_START, 1);
 	mutex_unlock(&bq->lock);
 
+	if (do_adc_conv)
+		regmap_field_read_poll_timeout(bq->rmap_fields[F_CONV_START],
+			ret, !ret, 25000, 1000000);
+
 	switch (psp) {
 	case POWER_SUPPLY_PROP_STATUS:
 		if (!state.online)
@@ -623,8 +646,8 @@ static int bq25890_hw_init(struct bq25890_device *bq)
 		}
 	}
 
-	/* Configure ADC for continuous conversions. This does not enable it. */
-	ret = bq25890_field_write(bq, F_CONV_RATE, 1);
+	/* Configure ADC for continuous conversions when charging */
+	ret = bq25890_field_write(bq, F_CONV_RATE, !!bq->state.online);
 	if (ret < 0) {
 		dev_dbg(bq->dev, "Config ADC failed %d\n", ret);
 		return ret;
@@ -966,7 +989,7 @@ static int bq25890_suspend(struct device *dev)
 	 * If charger is removed, while in suspend, make sure ADC is diabled
 	 * since it consumes slightly more power.
 	 */
-	return bq25890_field_write(bq, F_CONV_START, 0);
+	return bq25890_field_write(bq, F_CONV_RATE, 0);
 }
 
 static int bq25890_resume(struct device *dev)
@@ -982,7 +1005,7 @@ static int bq25890_resume(struct device *dev)
 
 	/* Re-enable ADC only if charger is plugged in. */
 	if (bq->state.online) {
-		ret = bq25890_field_write(bq, F_CONV_START, 1);
+		ret = bq25890_field_write(bq, F_CONV_RATE, 1);
 		if (ret < 0)
 			return ret;
 	}
-- 
2.20.1

Re: [PATCH v2 05/11] power: bq25890: fix ADC mode configuration

[Sebastian Reichel]

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

Hi,

On Sun, May 03, 2020 at 05:21:11PM +0200, Michał Mirosław wrote:
> Datasheet describes two modes for reading ADC measurements:
> 1. continuous, 1 Hz - enabled and started by CONV_RATE bit
> 2. one-shot - triggered by CONV_START bit
> 
> In continuous mode, CONV_START is read-only and signifies an ongoing
> conversion.
> 
> Change the code to follow the datasheet and really disable continuous
> mode for power saving.
> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---
>  drivers/power/supply/bq25890_charger.c | 33 ++++++++++++++++++++++----
>  1 file changed, 28 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
> index 9339e216651f..3b02fa80aedd 100644
> --- a/drivers/power/supply/bq25890_charger.c
> +++ b/drivers/power/supply/bq25890_charger.c
> @@ -126,6 +126,7 @@ static const struct regmap_access_table bq25890_writeable_regs = {
>  
>  static const struct regmap_range bq25890_volatile_reg_ranges[] = {
>  	regmap_reg_range(0x00, 0x00),
> +	regmap_reg_range(0x02, 0x02),
>  	regmap_reg_range(0x09, 0x09),
>  	regmap_reg_range(0x0b, 0x14),
>  };
> @@ -374,18 +375,40 @@ enum bq25890_chrg_fault {
>  	CHRG_FAULT_TIMER_EXPIRED,
>  };
>  
> +static bool bq25890_is_adc_property(enum power_supply_property psp)
> +{
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_INPUT_VOLTAGE_NOW:
> +	case POWER_SUPPLY_PROP_OUTPUT_VOLTAGE_NOW:

^^^ does not compile without your other patchset, so not applied.

When you send a new version, please Cc some of the recent
contributors to bq25890_charger.c, so that they have a chance
to test the changes with their setup:

Angus Ainslie (Purism) <angus@akkea.ca>
Yauhen Kharuzhy <jekhor@gmail.com>

-- Sebastian

> +	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> +	case POWER_SUPPLY_PROP_CURRENT_NOW:
> +		return true;
> +
> +	default:
> +		return false;
> +	}
> +}
> +
>  static int bq25890_power_supply_get_property(struct power_supply *psy,
>  					     enum power_supply_property psp,
>  					     union power_supply_propval *val)
>  {
> -	int ret;
>  	struct bq25890_device *bq = power_supply_get_drvdata(psy);
>  	struct bq25890_state state;
> +	bool do_adc_conv;
> +	int ret;
>  
>  	mutex_lock(&bq->lock);
>  	state = bq->state;
> +	do_adc_conv = !state.online && bq25890_is_adc_property(psp);
> +	if (do_adc_conv)
> +		bq25890_field_write(bq, F_CONV_START, 1);
>  	mutex_unlock(&bq->lock);
>  
> +	if (do_adc_conv)
> +		regmap_field_read_poll_timeout(bq->rmap_fields[F_CONV_START],
> +			ret, !ret, 25000, 1000000);
> +
>  	switch (psp) {
>  	case POWER_SUPPLY_PROP_STATUS:
>  		if (!state.online)
> @@ -623,8 +646,8 @@ static int bq25890_hw_init(struct bq25890_device *bq)
>  		}
>  	}
>  
> -	/* Configure ADC for continuous conversions. This does not enable it. */
> -	ret = bq25890_field_write(bq, F_CONV_RATE, 1);
> +	/* Configure ADC for continuous conversions when charging */
> +	ret = bq25890_field_write(bq, F_CONV_RATE, !!bq->state.online);
>  	if (ret < 0) {
>  		dev_dbg(bq->dev, "Config ADC failed %d\n", ret);
>  		return ret;
> @@ -966,7 +989,7 @@ static int bq25890_suspend(struct device *dev)
>  	 * If charger is removed, while in suspend, make sure ADC is diabled
>  	 * since it consumes slightly more power.
>  	 */
> -	return bq25890_field_write(bq, F_CONV_START, 0);
> +	return bq25890_field_write(bq, F_CONV_RATE, 0);
>  }
>  
>  static int bq25890_resume(struct device *dev)
> @@ -982,7 +1005,7 @@ static int bq25890_resume(struct device *dev)
>  
>  	/* Re-enable ADC only if charger is plugged in. */
>  	if (bq->state.online) {
> -		ret = bq25890_field_write(bq, F_CONV_START, 1);
> +		ret = bq25890_field_write(bq, F_CONV_RATE, 1);
>  		if (ret < 0)
>  			return ret;
>  	}
> -- 
> 2.20.1
> 

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

[PATCH v2 07/11] power: bq25890: implement CHARGE_TYPE property

[Michał Mirosław]

Report charging type based on recently read state.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/power/supply/bq25890_charger.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index e4368d01396a..ad0901fdceb6 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -429,6 +429,18 @@ static int bq25890_power_supply_get_property(struct power_supply *psy,
 
 		break;
 
+	case POWER_SUPPLY_PROP_CHARGE_TYPE:
+		if (!state.online || state.chrg_status == STATUS_NOT_CHARGING ||
+		    state.chrg_status == STATUS_TERMINATION_DONE)
+			val->intval = POWER_SUPPLY_CHARGE_TYPE_NONE;
+		else if (state.chrg_status == STATUS_PRE_CHARGING)
+			val->intval = POWER_SUPPLY_CHARGE_TYPE_STANDARD;
+		else if (state.chrg_status == STATUS_FAST_CHARGING)
+			val->intval = POWER_SUPPLY_CHARGE_TYPE_FAST;
+		else /* unreachable */
+			val->intval = POWER_SUPPLY_CHARGE_TYPE_UNKNOWN;
+		break;
+
 	case POWER_SUPPLY_PROP_MANUFACTURER:
 		val->strval = BQ25890_MANUFACTURER;
 		break;
@@ -670,6 +682,7 @@ static const enum power_supply_property bq25890_power_supply_props[] = {
 	POWER_SUPPLY_PROP_MANUFACTURER,
 	POWER_SUPPLY_PROP_MODEL_NAME,
 	POWER_SUPPLY_PROP_STATUS,
+	POWER_SUPPLY_PROP_CHARGE_TYPE,
 	POWER_SUPPLY_PROP_ONLINE,
 	POWER_SUPPLY_PROP_HEALTH,
 	POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT,
-- 
2.20.1

Re: [PATCH v2 07/11] power: bq25890: implement CHARGE_TYPE property

[Sebastian Reichel]

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

Hi,

On Sun, May 03, 2020 at 05:21:12PM +0200, Michał Mirosław wrote:
> Report charging type based on recently read state.
> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>

-- Sebastian

>  drivers/power/supply/bq25890_charger.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
> index e4368d01396a..ad0901fdceb6 100644
> --- a/drivers/power/supply/bq25890_charger.c
> +++ b/drivers/power/supply/bq25890_charger.c
> @@ -429,6 +429,18 @@ static int bq25890_power_supply_get_property(struct power_supply *psy,
>  
>  		break;
>  
> +	case POWER_SUPPLY_PROP_CHARGE_TYPE:
> +		if (!state.online || state.chrg_status == STATUS_NOT_CHARGING ||
> +		    state.chrg_status == STATUS_TERMINATION_DONE)
> +			val->intval = POWER_SUPPLY_CHARGE_TYPE_NONE;
> +		else if (state.chrg_status == STATUS_PRE_CHARGING)
> +			val->intval = POWER_SUPPLY_CHARGE_TYPE_STANDARD;
> +		else if (state.chrg_status == STATUS_FAST_CHARGING)
> +			val->intval = POWER_SUPPLY_CHARGE_TYPE_FAST;
> +		else /* unreachable */
> +			val->intval = POWER_SUPPLY_CHARGE_TYPE_UNKNOWN;
> +		break;
> +
>  	case POWER_SUPPLY_PROP_MANUFACTURER:
>  		val->strval = BQ25890_MANUFACTURER;
>  		break;
> @@ -670,6 +682,7 @@ static const enum power_supply_property bq25890_power_supply_props[] = {
>  	POWER_SUPPLY_PROP_MANUFACTURER,
>  	POWER_SUPPLY_PROP_MODEL_NAME,
>  	POWER_SUPPLY_PROP_STATUS,
> +	POWER_SUPPLY_PROP_CHARGE_TYPE,
>  	POWER_SUPPLY_PROP_ONLINE,
>  	POWER_SUPPLY_PROP_HEALTH,
>  	POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT,
> -- 
> 2.20.1
> 

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

[PATCH v2 06/11] power: bq25890: update state on property read

[Michał Mirosław]

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

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/power/supply/bq25890_charger.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index 3b02fa80aedd..e4368d01396a 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -389,6 +389,8 @@ static bool bq25890_is_adc_property(enum power_supply_property psp)
 	}
 }
 
+static irqreturn_t __bq25890_handle_irq(struct bq25890_device *bq);
+
 static int bq25890_power_supply_get_property(struct power_supply *psy,
 					     enum power_supply_property psp,
 					     union power_supply_propval *val)
@@ -399,6 +401,8 @@ static int bq25890_power_supply_get_property(struct power_supply *psy,
 	int ret;
 
 	mutex_lock(&bq->lock);
+	/* update state in case we lost an interrupt */
+	__bq25890_handle_irq(bq);
 	state = bq->state;
 	do_adc_conv = !state.online && bq25890_is_adc_property(psp);
 	if (do_adc_conv)
-- 
2.20.1

Re: [PATCH v2 06/11] power: bq25890: update state on property read

[Sebastian Reichel]

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

Hi,

On Sun, May 03, 2020 at 05:21:12PM +0200, Michał Mirosław wrote:
> Edge interrupts from the charger may be lost or stuck in fault mode
> since probe(). Check if something changed everytime userspace wants
> some data.
> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>

-- Sebastian

>  drivers/power/supply/bq25890_charger.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
> index 3b02fa80aedd..e4368d01396a 100644
> --- a/drivers/power/supply/bq25890_charger.c
> +++ b/drivers/power/supply/bq25890_charger.c
> @@ -389,6 +389,8 @@ static bool bq25890_is_adc_property(enum power_supply_property psp)
>  	}
>  }
>  
> +static irqreturn_t __bq25890_handle_irq(struct bq25890_device *bq);
> +
>  static int bq25890_power_supply_get_property(struct power_supply *psy,
>  					     enum power_supply_property psp,
>  					     union power_supply_propval *val)
> @@ -399,6 +401,8 @@ static int bq25890_power_supply_get_property(struct power_supply *psy,
>  	int ret;
>  
>  	mutex_lock(&bq->lock);
> +	/* update state in case we lost an interrupt */
> +	__bq25890_handle_irq(bq);
>  	state = bq->state;
>  	do_adc_conv = !state.online && bq25890_is_adc_property(psp);
>  	if (do_adc_conv)
> -- 
> 2.20.1
> 

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

[PATCH v2 09/11] power: bq25890: implement INPUT_CURRENT_LIMIT property

[Michał Mirosław]

Report REG00.IINLIM value as INPUT_CURRENT_LIMIT property.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/power/supply/bq25890_charger.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index b48685009048..87c5832e23d3 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -254,6 +254,7 @@ enum bq25890_table_ids {
 	/* range tables */
 	TBL_ICHG,
 	TBL_ITERM,
+	TBL_IILIM,
 	TBL_VREG,
 	TBL_BOOSTV,
 	TBL_SYSVMIN,
@@ -294,6 +295,7 @@ static const union {
 	/* TODO: BQ25896 has max ICHG 3008 mA */
 	[TBL_ICHG] =	{ .rt = {0,	  5056000, 64000} },	 /* uA */
 	[TBL_ITERM] =	{ .rt = {64000,   1024000, 64000} },	 /* uA */
+	[TBL_IILIM] =   { .rt = {50000,   3200000, 50000} },	 /* uA */
 	[TBL_VREG] =	{ .rt = {3840000, 4608000, 16000} },	 /* uV */
 	[TBL_BOOSTV] =	{ .rt = {4550000, 5510000, 64000} },	 /* uV */
 	[TBL_SYSVMIN] = { .rt = {3000000, 3700000, 100000} },	 /* uV */
@@ -505,6 +507,14 @@ static int bq25890_power_supply_get_property(struct power_supply *psy,
 		val->intval = bq25890_find_val(bq->init_data.iterm, TBL_ITERM);
 		break;
 
+	case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+		ret = bq25890_field_read(bq, F_IILIM);
+		if (ret < 0)
+			return ret;
+
+		val->intval = bq25890_find_val(ret, TBL_IILIM);
+		break;
+
 	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
 		ret = bq25890_field_read(bq, F_SYSV); /* read measured value */
 		if (ret < 0)
@@ -695,6 +705,7 @@ static const enum power_supply_property bq25890_power_supply_props[] = {
 	POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX,
 	POWER_SUPPLY_PROP_PRECHARGE_CURRENT,
 	POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT,
+	POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
 	POWER_SUPPLY_PROP_VOLTAGE_NOW,
 };
 
-- 
2.20.1

Re: [PATCH v2 09/11] power: bq25890: implement INPUT_CURRENT_LIMIT property

[Sebastian Reichel]

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

Hi,

On Sun, May 03, 2020 at 05:21:13PM +0200, Michał Mirosław wrote:
> Report REG00.IINLIM value as INPUT_CURRENT_LIMIT property.
> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>

-- Sebastian

>  drivers/power/supply/bq25890_charger.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
> index b48685009048..87c5832e23d3 100644
> --- a/drivers/power/supply/bq25890_charger.c
> +++ b/drivers/power/supply/bq25890_charger.c
> @@ -254,6 +254,7 @@ enum bq25890_table_ids {
>  	/* range tables */
>  	TBL_ICHG,
>  	TBL_ITERM,
> +	TBL_IILIM,
>  	TBL_VREG,
>  	TBL_BOOSTV,
>  	TBL_SYSVMIN,
> @@ -294,6 +295,7 @@ static const union {
>  	/* TODO: BQ25896 has max ICHG 3008 mA */
>  	[TBL_ICHG] =	{ .rt = {0,	  5056000, 64000} },	 /* uA */
>  	[TBL_ITERM] =	{ .rt = {64000,   1024000, 64000} },	 /* uA */
> +	[TBL_IILIM] =   { .rt = {50000,   3200000, 50000} },	 /* uA */
>  	[TBL_VREG] =	{ .rt = {3840000, 4608000, 16000} },	 /* uV */
>  	[TBL_BOOSTV] =	{ .rt = {4550000, 5510000, 64000} },	 /* uV */
>  	[TBL_SYSVMIN] = { .rt = {3000000, 3700000, 100000} },	 /* uV */
> @@ -505,6 +507,14 @@ static int bq25890_power_supply_get_property(struct power_supply *psy,
>  		val->intval = bq25890_find_val(bq->init_data.iterm, TBL_ITERM);
>  		break;
>  
> +	case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
> +		ret = bq25890_field_read(bq, F_IILIM);
> +		if (ret < 0)
> +			return ret;
> +
> +		val->intval = bq25890_find_val(ret, TBL_IILIM);
> +		break;
> +
>  	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
>  		ret = bq25890_field_read(bq, F_SYSV); /* read measured value */
>  		if (ret < 0)
> @@ -695,6 +705,7 @@ static const enum power_supply_property bq25890_power_supply_props[] = {
>  	POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX,
>  	POWER_SUPPLY_PROP_PRECHARGE_CURRENT,
>  	POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT,
> +	POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
>  	POWER_SUPPLY_PROP_VOLTAGE_NOW,
>  };
>  
> -- 
> 2.20.1
> 

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

[PATCH v2 08/11] power: bq25890: implement PRECHARGE_CURRENT property

[Michał Mirosław]

Report configured precharge current.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/power/supply/bq25890_charger.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index ad0901fdceb6..b48685009048 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -497,6 +497,10 @@ static int bq25890_power_supply_get_property(struct power_supply *psy,
 		val->intval = bq25890_find_val(bq->init_data.vreg, TBL_VREG);
 		break;
 
+	case POWER_SUPPLY_PROP_PRECHARGE_CURRENT:
+		val->intval = bq25890_find_val(bq->init_data.iprechg, TBL_ITERM);
+		break;
+
 	case POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT:
 		val->intval = bq25890_find_val(bq->init_data.iterm, TBL_ITERM);
 		break;
@@ -689,6 +693,7 @@ static const enum power_supply_property bq25890_power_supply_props[] = {
 	POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX,
 	POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
 	POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX,
+	POWER_SUPPLY_PROP_PRECHARGE_CURRENT,
 	POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT,
 	POWER_SUPPLY_PROP_VOLTAGE_NOW,
 };
-- 
2.20.1

Re: [PATCH v2 08/11] power: bq25890: implement PRECHARGE_CURRENT property

[Sebastian Reichel]

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

Hi,

On Sun, May 03, 2020 at 05:21:13PM +0200, Michał Mirosław wrote:
> Report configured precharge current.
> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>

-- Sebastian

>  drivers/power/supply/bq25890_charger.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
> index ad0901fdceb6..b48685009048 100644
> --- a/drivers/power/supply/bq25890_charger.c
> +++ b/drivers/power/supply/bq25890_charger.c
> @@ -497,6 +497,10 @@ static int bq25890_power_supply_get_property(struct power_supply *psy,
>  		val->intval = bq25890_find_val(bq->init_data.vreg, TBL_VREG);
>  		break;
>  
> +	case POWER_SUPPLY_PROP_PRECHARGE_CURRENT:
> +		val->intval = bq25890_find_val(bq->init_data.iprechg, TBL_ITERM);
> +		break;
> +
>  	case POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT:
>  		val->intval = bq25890_find_val(bq->init_data.iterm, TBL_ITERM);
>  		break;
> @@ -689,6 +693,7 @@ static const enum power_supply_property bq25890_power_supply_props[] = {
>  	POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX,
>  	POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
>  	POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX,
> +	POWER_SUPPLY_PROP_PRECHARGE_CURRENT,
>  	POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT,
>  	POWER_SUPPLY_PROP_VOLTAGE_NOW,
>  };
> -- 
> 2.20.1
> 

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

[PATCH v2 10/11] power: bq25890: support IBAT compensation

[Michał Mirosław]

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

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/power/supply/bq25890_charger.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index 87c5832e23d3..9d1ba3bd0bea 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -83,6 +83,8 @@ struct bq25890_init_data {
 	u8 boostf;	/* boost frequency		*/
 	u8 ilim_en;	/* enable ILIM pin		*/
 	u8 treg;	/* thermal regulation threshold */
+	u8 rbatcomp;	/* IBAT sense resistor value    */
+	u8 vclamp;	/* IBAT compensation voltage limit */
 };
 
 struct bq25890_state {
@@ -258,6 +260,8 @@ enum bq25890_table_ids {
 	TBL_VREG,
 	TBL_BOOSTV,
 	TBL_SYSVMIN,
+	TBL_VBATCOMP,
+	TBL_RBATCOMP,
 
 	/* lookup tables */
 	TBL_TREG,
@@ -299,6 +303,8 @@ static const union {
 	[TBL_VREG] =	{ .rt = {3840000, 4608000, 16000} },	 /* uV */
 	[TBL_BOOSTV] =	{ .rt = {4550000, 5510000, 64000} },	 /* uV */
 	[TBL_SYSVMIN] = { .rt = {3000000, 3700000, 100000} },	 /* uV */
+	[TBL_VBATCOMP] ={ .rt = {0,        224000, 32000} },	 /* uV */
+	[TBL_RBATCOMP] ={ .rt = {0,        140000, 20000} },	 /* uOhm */
 
 	/* lookup tables */
 	[TBL_TREG] =	{ .lt = {bq25890_treg_tbl, BQ25890_TREG_TBL_SIZE} },
@@ -650,7 +656,9 @@ static int bq25890_hw_init(struct bq25890_device *bq)
 		{F_BOOSTI,	 bq->init_data.boosti},
 		{F_BOOSTF,	 bq->init_data.boostf},
 		{F_EN_ILIM,	 bq->init_data.ilim_en},
-		{F_TREG,	 bq->init_data.treg}
+		{F_TREG,	 bq->init_data.treg},
+		{F_BATCMP,	 bq->init_data.rbatcomp},
+		{F_VCLAMP,	 bq->init_data.vclamp},
 	};
 
 	ret = bq25890_chip_reset(bq);
@@ -861,11 +869,14 @@ static int bq25890_fw_read_u32_props(struct bq25890_device *bq)
 		{"ti,boost-max-current", false, TBL_BOOSTI, &init->boosti},
 
 		/* optional properties */
-		{"ti,thermal-regulation-threshold", true, TBL_TREG, &init->treg}
+		{"ti,thermal-regulation-threshold", true, TBL_TREG, &init->treg},
+		{"ti,ibatcomp-resistance", true, TBL_RBATCOMP, &init->rbatcomp},
+		{"ti,ibatcomp-clamp-voltage", true, TBL_VBATCOMP, &init->vclamp},
 	};
 
 	/* initialize data for optional properties */
 	init->treg = 3; /* 120 degrees Celsius */
+	init->rbatcomp = init->vclamp = 0; /* IBAT compensation disabled */
 
 	for (i = 0; i < ARRAY_SIZE(props); i++) {
 		ret = device_property_read_u32(bq->dev, props[i].name,
-- 
2.20.1

[PATCH v2 11/11] power: bq25890: document IBAT compensation DT properties

[Michał Mirosław]

Document newly introduced IBAT compensation settings.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
v2: initial version
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 Documentation/devicetree/bindings/power/supply/bq25890.txt | 4 ++++
 1 file changed, 4 insertions(+)

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