* [PATCH v2 03/11] power: bq25890: make property table const
2020-05-03 15:21 [PATCH v2 00/11] power: supply: bq25890: fix and extend Michał Mirosław
@ 2020-05-03 15:21 ` Michał Mirosław
2020-05-03 19:55 ` Sebastian Reichel
2020-05-03 15:21 ` [PATCH v2 01/11] power: bq25890: remove redundant I2C bus check Michał Mirosław
` (9 subsequent siblings)
10 siblings, 1 reply; 21+ messages in thread
From: Michał Mirosław @ 2020-05-03 15:21 UTC (permalink / raw)
To: Rob Herring, Sebastian Reichel; +Cc: devicetree, linux-kernel, linux-pm
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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v2 03/11] power: bq25890: make property table const
2020-05-03 15:21 ` [PATCH v2 03/11] power: bq25890: make property table const Michał Mirosław
@ 2020-05-03 19:55 ` Sebastian Reichel
0 siblings, 0 replies; 21+ messages in thread
From: Sebastian Reichel @ 2020-05-03 19:55 UTC (permalink / raw)
To: Michał Mirosław; +Cc: Rob Herring, devicetree, linux-kernel, linux-pm
[-- 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 --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 01/11] power: bq25890: remove redundant I2C bus check
2020-05-03 15:21 [PATCH v2 00/11] power: supply: bq25890: fix and extend Michał Mirosław
2020-05-03 15:21 ` [PATCH v2 03/11] power: bq25890: make property table const Michał Mirosław
@ 2020-05-03 15:21 ` Michał Mirosław
2020-05-03 19:56 ` Sebastian Reichel
2020-05-03 15:21 ` [PATCH v2 02/11] power: bq25890: simplify chip name property getter Michał Mirosław
` (8 subsequent siblings)
10 siblings, 1 reply; 21+ messages in thread
From: Michał Mirosław @ 2020-05-03 15:21 UTC (permalink / raw)
To: Sebastian Reichel, Rob Herring; +Cc: linux-pm, linux-kernel, devicetree
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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v2 01/11] power: bq25890: remove redundant I2C bus check
2020-05-03 15:21 ` [PATCH v2 01/11] power: bq25890: remove redundant I2C bus check Michał Mirosław
@ 2020-05-03 19:56 ` Sebastian Reichel
0 siblings, 0 replies; 21+ messages in thread
From: Sebastian Reichel @ 2020-05-03 19:56 UTC (permalink / raw)
To: Michał Mirosław; +Cc: Rob Herring, linux-pm, linux-kernel, devicetree
[-- 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 --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 02/11] power: bq25890: simplify chip name property getter
2020-05-03 15:21 [PATCH v2 00/11] power: supply: bq25890: fix and extend Michał Mirosław
2020-05-03 15:21 ` [PATCH v2 03/11] power: bq25890: make property table const Michał Mirosław
2020-05-03 15:21 ` [PATCH v2 01/11] power: bq25890: remove redundant I2C bus check Michał Mirosław
@ 2020-05-03 15:21 ` Michał Mirosław
2020-05-03 19:55 ` Sebastian Reichel
2020-05-03 15:21 ` [PATCH v2 04/11] power: bq25890: protect view of the chip's state Michał Mirosław
` (7 subsequent siblings)
10 siblings, 1 reply; 21+ messages in thread
From: Michał Mirosław @ 2020-05-03 15:21 UTC (permalink / raw)
To: Rob Herring, Sebastian Reichel; +Cc: devicetree, linux-kernel, linux-pm
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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v2 02/11] power: bq25890: simplify chip name property getter
2020-05-03 15:21 ` [PATCH v2 02/11] power: bq25890: simplify chip name property getter Michał Mirosław
@ 2020-05-03 19:55 ` Sebastian Reichel
0 siblings, 0 replies; 21+ messages in thread
From: Sebastian Reichel @ 2020-05-03 19:55 UTC (permalink / raw)
To: Michał Mirosław; +Cc: Rob Herring, devicetree, linux-kernel, linux-pm
[-- 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 --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 04/11] power: bq25890: protect view of the chip's state
2020-05-03 15:21 [PATCH v2 00/11] power: supply: bq25890: fix and extend Michał Mirosław
` (2 preceding siblings ...)
2020-05-03 15:21 ` [PATCH v2 02/11] power: bq25890: simplify chip name property getter Michał Mirosław
@ 2020-05-03 15:21 ` Michał Mirosław
2020-05-03 21:10 ` Sebastian Reichel
2020-05-03 15:21 ` [PATCH v2 05/11] power: bq25890: fix ADC mode configuration Michał Mirosław
` (6 subsequent siblings)
10 siblings, 1 reply; 21+ messages in thread
From: Michał Mirosław @ 2020-05-03 15:21 UTC (permalink / raw)
To: Rob Herring, Sebastian Reichel; +Cc: devicetree, linux-kernel, linux-pm
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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v2 04/11] power: bq25890: protect view of the chip's state
2020-05-03 15:21 ` [PATCH v2 04/11] power: bq25890: protect view of the chip's state Michał Mirosław
@ 2020-05-03 21:10 ` Sebastian Reichel
0 siblings, 0 replies; 21+ messages in thread
From: Sebastian Reichel @ 2020-05-03 21:10 UTC (permalink / raw)
To: Michał Mirosław; +Cc: Rob Herring, devicetree, linux-kernel, linux-pm
[-- 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 --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 05/11] power: bq25890: fix ADC mode configuration
2020-05-03 15:21 [PATCH v2 00/11] power: supply: bq25890: fix and extend Michał Mirosław
` (3 preceding siblings ...)
2020-05-03 15:21 ` [PATCH v2 04/11] power: bq25890: protect view of the chip's state Michał Mirosław
@ 2020-05-03 15:21 ` Michał Mirosław
2020-05-03 20:01 ` Sebastian Reichel
2020-05-03 15:21 ` [PATCH v2 07/11] power: bq25890: implement CHARGE_TYPE property Michał Mirosław
` (5 subsequent siblings)
10 siblings, 1 reply; 21+ messages in thread
From: Michał Mirosław @ 2020-05-03 15:21 UTC (permalink / raw)
To: Rob Herring, Sebastian Reichel; +Cc: devicetree, linux-kernel, linux-pm
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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v2 05/11] power: bq25890: fix ADC mode configuration
2020-05-03 15:21 ` [PATCH v2 05/11] power: bq25890: fix ADC mode configuration Michał Mirosław
@ 2020-05-03 20:01 ` Sebastian Reichel
0 siblings, 0 replies; 21+ messages in thread
From: Sebastian Reichel @ 2020-05-03 20:01 UTC (permalink / raw)
To: Michał Mirosław; +Cc: Rob Herring, devicetree, linux-kernel, linux-pm
[-- 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 --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 07/11] power: bq25890: implement CHARGE_TYPE property
2020-05-03 15:21 [PATCH v2 00/11] power: supply: bq25890: fix and extend Michał Mirosław
` (4 preceding siblings ...)
2020-05-03 15:21 ` [PATCH v2 05/11] power: bq25890: fix ADC mode configuration Michał Mirosław
@ 2020-05-03 15:21 ` Michał Mirosław
2020-05-03 21:14 ` Sebastian Reichel
2020-05-03 15:21 ` [PATCH v2 06/11] power: bq25890: update state on property read Michał Mirosław
` (4 subsequent siblings)
10 siblings, 1 reply; 21+ messages in thread
From: Michał Mirosław @ 2020-05-03 15:21 UTC (permalink / raw)
To: Rob Herring, Sebastian Reichel; +Cc: devicetree, linux-kernel, linux-pm
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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v2 07/11] power: bq25890: implement CHARGE_TYPE property
2020-05-03 15:21 ` [PATCH v2 07/11] power: bq25890: implement CHARGE_TYPE property Michał Mirosław
@ 2020-05-03 21:14 ` Sebastian Reichel
0 siblings, 0 replies; 21+ messages in thread
From: Sebastian Reichel @ 2020-05-03 21:14 UTC (permalink / raw)
To: Michał Mirosław; +Cc: Rob Herring, devicetree, linux-kernel, linux-pm
[-- 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 --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 06/11] power: bq25890: update state on property read
2020-05-03 15:21 [PATCH v2 00/11] power: supply: bq25890: fix and extend Michał Mirosław
` (5 preceding siblings ...)
2020-05-03 15:21 ` [PATCH v2 07/11] power: bq25890: implement CHARGE_TYPE property Michał Mirosław
@ 2020-05-03 15:21 ` Michał Mirosław
2020-05-03 21:14 ` Sebastian Reichel
2020-05-03 15:21 ` [PATCH v2 09/11] power: bq25890: implement INPUT_CURRENT_LIMIT property Michał Mirosław
` (3 subsequent siblings)
10 siblings, 1 reply; 21+ messages in thread
From: Michał Mirosław @ 2020-05-03 15:21 UTC (permalink / raw)
To: Rob Herring, Sebastian Reichel; +Cc: devicetree, linux-kernel, linux-pm
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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v2 06/11] power: bq25890: update state on property read
2020-05-03 15:21 ` [PATCH v2 06/11] power: bq25890: update state on property read Michał Mirosław
@ 2020-05-03 21:14 ` Sebastian Reichel
0 siblings, 0 replies; 21+ messages in thread
From: Sebastian Reichel @ 2020-05-03 21:14 UTC (permalink / raw)
To: Michał Mirosław; +Cc: Rob Herring, devicetree, linux-kernel, linux-pm
[-- 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 --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 09/11] power: bq25890: implement INPUT_CURRENT_LIMIT property
2020-05-03 15:21 [PATCH v2 00/11] power: supply: bq25890: fix and extend Michał Mirosław
` (6 preceding siblings ...)
2020-05-03 15:21 ` [PATCH v2 06/11] power: bq25890: update state on property read Michał Mirosław
@ 2020-05-03 15:21 ` Michał Mirosław
2020-05-03 21:16 ` Sebastian Reichel
2020-05-03 15:21 ` [PATCH v2 08/11] power: bq25890: implement PRECHARGE_CURRENT property Michał Mirosław
` (2 subsequent siblings)
10 siblings, 1 reply; 21+ messages in thread
From: Michał Mirosław @ 2020-05-03 15:21 UTC (permalink / raw)
To: Rob Herring, Sebastian Reichel; +Cc: devicetree, linux-kernel, linux-pm
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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v2 09/11] power: bq25890: implement INPUT_CURRENT_LIMIT property
2020-05-03 15:21 ` [PATCH v2 09/11] power: bq25890: implement INPUT_CURRENT_LIMIT property Michał Mirosław
@ 2020-05-03 21:16 ` Sebastian Reichel
0 siblings, 0 replies; 21+ messages in thread
From: Sebastian Reichel @ 2020-05-03 21:16 UTC (permalink / raw)
To: Michał Mirosław; +Cc: Rob Herring, devicetree, linux-kernel, linux-pm
[-- 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 --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 08/11] power: bq25890: implement PRECHARGE_CURRENT property
2020-05-03 15:21 [PATCH v2 00/11] power: supply: bq25890: fix and extend Michał Mirosław
` (7 preceding siblings ...)
2020-05-03 15:21 ` [PATCH v2 09/11] power: bq25890: implement INPUT_CURRENT_LIMIT property Michał Mirosław
@ 2020-05-03 15:21 ` Michał Mirosław
2020-05-03 21:15 ` Sebastian Reichel
2020-05-03 15:21 ` [PATCH v2 10/11] power: bq25890: support IBAT compensation Michał Mirosław
2020-05-03 15:21 ` [PATCH v2 11/11] power: bq25890: document IBAT compensation DT properties Michał Mirosław
10 siblings, 1 reply; 21+ messages in thread
From: Michał Mirosław @ 2020-05-03 15:21 UTC (permalink / raw)
To: Rob Herring, Sebastian Reichel; +Cc: devicetree, linux-kernel, linux-pm
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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v2 08/11] power: bq25890: implement PRECHARGE_CURRENT property
2020-05-03 15:21 ` [PATCH v2 08/11] power: bq25890: implement PRECHARGE_CURRENT property Michał Mirosław
@ 2020-05-03 21:15 ` Sebastian Reichel
0 siblings, 0 replies; 21+ messages in thread
From: Sebastian Reichel @ 2020-05-03 21:15 UTC (permalink / raw)
To: Michał Mirosław; +Cc: Rob Herring, devicetree, linux-kernel, linux-pm
[-- 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 --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 10/11] power: bq25890: support IBAT compensation
2020-05-03 15:21 [PATCH v2 00/11] power: supply: bq25890: fix and extend Michał Mirosław
` (8 preceding siblings ...)
2020-05-03 15:21 ` [PATCH v2 08/11] power: bq25890: implement PRECHARGE_CURRENT property Michał Mirosław
@ 2020-05-03 15:21 ` Michał Mirosław
2020-05-03 15:21 ` [PATCH v2 11/11] power: bq25890: document IBAT compensation DT properties Michał Mirosław
10 siblings, 0 replies; 21+ messages in thread
From: Michał Mirosław @ 2020-05-03 15:21 UTC (permalink / raw)
To: Rob Herring, Sebastian Reichel; +Cc: devicetree, linux-kernel, linux-pm
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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2 11/11] power: bq25890: document IBAT compensation DT properties
2020-05-03 15:21 [PATCH v2 00/11] power: supply: bq25890: fix and extend Michał Mirosław
` (9 preceding siblings ...)
2020-05-03 15:21 ` [PATCH v2 10/11] power: bq25890: support IBAT compensation Michał Mirosław
@ 2020-05-03 15:21 ` Michał Mirosław
10 siblings, 0 replies; 21+ messages in thread
From: Michał Mirosław @ 2020-05-03 15:21 UTC (permalink / raw)
To: Sebastian Reichel, Rob Herring; +Cc: linux-pm, devicetree, linux-kernel
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
^ permalink raw reply related [flat|nested] 21+ messages in thread