* [PATCH 1/3] power: supply: bq24735: move down bq24735_{en,dis}able_charging
2016-12-20 11:32 [PATCH 0/3] power: supply: bq24735: handle AC adapter absence Peter Rosin
@ 2016-12-20 11:33 ` Peter Rosin
2016-12-20 11:33 ` [PATCH 2/3] power: supply: bq24735: configure the charger as part of enabling it Peter Rosin
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Peter Rosin @ 2016-12-20 11:33 UTC (permalink / raw)
To: linux-kernel; +Cc: Peter Rosin, Sebastian Reichel, linux-pm
bq24735_enable_charging() needs to call bq24735_config_charging(),
which is something to change later, this is just a preparatory patch.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/power/supply/bq24735-charger.c | 38 +++++++++++++++++-----------------
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/drivers/power/supply/bq24735-charger.c b/drivers/power/supply/bq24735-charger.c
index 8a0242c13b7e..2787c19d6696 100644
--- a/drivers/power/supply/bq24735-charger.c
+++ b/drivers/power/supply/bq24735-charger.c
@@ -107,25 +107,6 @@ static int bq24735_update_word(struct i2c_client *client, u8 reg,
return bq24735_write_word(client, reg, tmp);
}
-static inline int bq24735_enable_charging(struct bq24735 *charger)
-{
- if (charger->pdata->ext_control)
- return 0;
-
- return bq24735_update_word(charger->client, BQ24735_CHG_OPT,
- BQ24735_CHG_OPT_CHARGE_DISABLE, 0);
-}
-
-static inline int bq24735_disable_charging(struct bq24735 *charger)
-{
- if (charger->pdata->ext_control)
- return 0;
-
- return bq24735_update_word(charger->client, BQ24735_CHG_OPT,
- BQ24735_CHG_OPT_CHARGE_DISABLE,
- BQ24735_CHG_OPT_CHARGE_DISABLE);
-}
-
static int bq24735_config_charger(struct bq24735 *charger)
{
struct bq24735_platform *pdata = charger->pdata;
@@ -177,6 +158,25 @@ static int bq24735_config_charger(struct bq24735 *charger)
return 0;
}
+static inline int bq24735_enable_charging(struct bq24735 *charger)
+{
+ if (charger->pdata->ext_control)
+ return 0;
+
+ return bq24735_update_word(charger->client, BQ24735_CHG_OPT,
+ BQ24735_CHG_OPT_CHARGE_DISABLE, 0);
+}
+
+static inline int bq24735_disable_charging(struct bq24735 *charger)
+{
+ if (charger->pdata->ext_control)
+ return 0;
+
+ return bq24735_update_word(charger->client, BQ24735_CHG_OPT,
+ BQ24735_CHG_OPT_CHARGE_DISABLE,
+ BQ24735_CHG_OPT_CHARGE_DISABLE);
+}
+
static bool bq24735_charger_is_present(struct bq24735 *charger)
{
if (charger->status_gpio) {
--
2.1.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/3] power: supply: bq24735: configure the charger as part of enabling it
2016-12-20 11:32 [PATCH 0/3] power: supply: bq24735: handle AC adapter absence Peter Rosin
2016-12-20 11:33 ` [PATCH 1/3] power: supply: bq24735: move down bq24735_{en,dis}able_charging Peter Rosin
@ 2016-12-20 11:33 ` Peter Rosin
2016-12-20 11:33 ` [PATCH 3/3] power: supply: bq24735: always check for AC adapter presence in probe Peter Rosin
2016-12-22 15:47 ` [PATCH 0/3] power: supply: bq24735: handle AC adapter absence Sebastian Reichel
3 siblings, 0 replies; 5+ messages in thread
From: Peter Rosin @ 2016-12-20 11:33 UTC (permalink / raw)
To: linux-kernel; +Cc: Peter Rosin, Sebastian Reichel, linux-pm
During probe, it makes no sense to take care to first not issue any
i2c commands to verify if the connected part really is a bq24735, to
later simply fail the probe in the next step when trying to configure
the charger. So, delay configuration of the charging parameters until
the charger is accessible (i.e. when the AC adapter is present) as
part of enabling the charging.
This also fixes the rather serious issue that the charging parameters
are lost when the AC adapter is disconnected.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/power/supply/bq24735-charger.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/power/supply/bq24735-charger.c b/drivers/power/supply/bq24735-charger.c
index 2787c19d6696..71f977d055d7 100644
--- a/drivers/power/supply/bq24735-charger.c
+++ b/drivers/power/supply/bq24735-charger.c
@@ -160,9 +160,15 @@ static int bq24735_config_charger(struct bq24735 *charger)
static inline int bq24735_enable_charging(struct bq24735 *charger)
{
+ int ret;
+
if (charger->pdata->ext_control)
return 0;
+ ret = bq24735_config_charger(charger);
+ if (ret)
+ return ret;
+
return bq24735_update_word(charger->client, BQ24735_CHG_OPT,
BQ24735_CHG_OPT_CHARGE_DISABLE, 0);
}
@@ -292,7 +298,6 @@ static int bq24735_charger_set_property(struct power_supply *psy,
mutex_unlock(&charger->lock);
if (ret)
return ret;
- bq24735_config_charger(charger);
break;
case POWER_SUPPLY_STATUS_DISCHARGING:
case POWER_SUPPLY_STATUS_NOT_CHARGING:
@@ -434,12 +439,6 @@ static int bq24735_charger_probe(struct i2c_client *client,
}
}
- ret = bq24735_config_charger(charger);
- if (ret < 0) {
- dev_err(&client->dev, "failed in configuring charger");
- return ret;
- }
-
/* check for AC adapter presence */
if (bq24735_charger_is_present(charger)) {
ret = bq24735_enable_charging(charger);
--
2.1.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 3/3] power: supply: bq24735: always check for AC adapter presence in probe
2016-12-20 11:32 [PATCH 0/3] power: supply: bq24735: handle AC adapter absence Peter Rosin
2016-12-20 11:33 ` [PATCH 1/3] power: supply: bq24735: move down bq24735_{en,dis}able_charging Peter Rosin
2016-12-20 11:33 ` [PATCH 2/3] power: supply: bq24735: configure the charger as part of enabling it Peter Rosin
@ 2016-12-20 11:33 ` Peter Rosin
2016-12-22 15:47 ` [PATCH 0/3] power: supply: bq24735: handle AC adapter absence Sebastian Reichel
3 siblings, 0 replies; 5+ messages in thread
From: Peter Rosin @ 2016-12-20 11:33 UTC (permalink / raw)
To: linux-kernel; +Cc: Peter Rosin, Sebastian Reichel, linux-pm
So what if there is a status_gpio specified? bq24735_charger_is_present()
do have a working fallback for the case of no status_gpio.
Simplify this by not special casing setups w/o status_gpio, folding
two consecutive if-blocks in the process.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/power/supply/bq24735-charger.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/power/supply/bq24735-charger.c b/drivers/power/supply/bq24735-charger.c
index 71f977d055d7..4f6275e5cf1c 100644
--- a/drivers/power/supply/bq24735-charger.c
+++ b/drivers/power/supply/bq24735-charger.c
@@ -416,7 +416,7 @@ static int bq24735_charger_probe(struct i2c_client *client,
return ret;
}
- if (!charger->status_gpio || bq24735_charger_is_present(charger)) {
+ if (bq24735_charger_is_present(charger)) {
ret = bq24735_read_word(client, BQ24735_MANUFACTURER_ID);
if (ret < 0) {
dev_err(&client->dev, "Failed to read manufacturer id : %d\n",
@@ -437,10 +437,7 @@ static int bq24735_charger_probe(struct i2c_client *client,
"device id mismatch. 0x000b != 0x%04x\n", ret);
return -ENODEV;
}
- }
- /* check for AC adapter presence */
- if (bq24735_charger_is_present(charger)) {
ret = bq24735_enable_charging(charger);
if (ret < 0) {
dev_err(&client->dev, "Failed to enable charging\n");
--
2.1.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 0/3] power: supply: bq24735: handle AC adapter absence
2016-12-20 11:32 [PATCH 0/3] power: supply: bq24735: handle AC adapter absence Peter Rosin
` (2 preceding siblings ...)
2016-12-20 11:33 ` [PATCH 3/3] power: supply: bq24735: always check for AC adapter presence in probe Peter Rosin
@ 2016-12-22 15:47 ` Sebastian Reichel
3 siblings, 0 replies; 5+ messages in thread
From: Sebastian Reichel @ 2016-12-22 15:47 UTC (permalink / raw)
To: Peter Rosin; +Cc: linux-kernel, linux-pm
[-- Attachment #1: Type: text/plain, Size: 1538 bytes --]
Hi,
On Tue, Dec 20, 2016 at 12:32:59PM +0100, Peter Rosin wrote:
> There is provision in the driver for handling the case where the
> chargers are not responding to i2c requests when the AC adapter
> is absent. I think this has been used by some Tegra boards that
> leave the charger handling to some other component and that only
> wants the kernel to be aware of the charging status for reporting
> (or whatever).
>
> However, in my case, the kernel should handle configuration and
> enable/disable the charging, but this is not working very well
> when the charger disappears (and is reset) on AC adapter absence.
>
> This series fixes the issues I have found in this area so that I
> can boot without AC adapter and have the correct charging config
> every time I do attach the AC adapter.
>
> Cheers,
> Peter
>
> Peter Rosin (3):
> power: supply: bq24735: move down bq24735_{en,dis}able_charging
> power: supply: bq24735: configure the charger as part of enabling it
> power: supply: bq24735: always check for AC adapter presence in probe
>
> drivers/power/supply/bq24735-charger.c | 56 ++++++++++++++++------------------
> 1 file changed, 26 insertions(+), 30 deletions(-)
Thanks for your patchset. We are currently in the merge
window and your patchset will appear in linux-next once
4.10-rc1 has been tagged by Linus Torvalds.
Until then I queued it into this branch:
https://git.kernel.org/cgit/linux/kernel/git/sre/linux-power-supply.git/log/?h=for-next-next
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread