* [PATCH] bq27x00: use cached flags
@ 2019-02-18 6:59 Arthur Demchenkov
2019-02-19 16:55 ` Andrew F. Davis
0 siblings, 1 reply; 3+ messages in thread
From: Arthur Demchenkov @ 2019-02-18 6:59 UTC (permalink / raw)
Cc: Arthur Demchenkov, Pali Rohár, Andrew F. Davis,
Sebastian Reichel, linux-pm, linux-kernel
The flags were just read by bq27xxx_battery_update(),
no need to read them again.
Signed-off-by: Arthur Demchenkov <spinal.by@gmail.com>
---
drivers/power/supply/bq27xxx_battery.c | 20 ++++----------------
1 file changed, 4 insertions(+), 16 deletions(-)
diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
index 6dbbe95844a3..29b3a4056865 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -1555,27 +1555,14 @@ static bool bq27xxx_battery_dead(struct bq27xxx_device_info *di, u16 flags)
return flags & (BQ27XXX_FLAG_SOC1 | BQ27XXX_FLAG_SOCF);
}
-/*
- * Read flag register.
- * Return < 0 if something fails.
- */
static int bq27xxx_battery_read_health(struct bq27xxx_device_info *di)
{
- int flags;
- bool has_singe_flag = di->opts & BQ27XXX_O_ZERO;
-
- flags = bq27xxx_read(di, BQ27XXX_REG_FLAGS, has_singe_flag);
- if (flags < 0) {
- dev_err(di->dev, "error reading flag register:%d\n", flags);
- return flags;
- }
-
/* Unlikely but important to return first */
- if (unlikely(bq27xxx_battery_overtemp(di, flags)))
+ if (unlikely(bq27xxx_battery_overtemp(di, di->cache.flags)))
return POWER_SUPPLY_HEALTH_OVERHEAT;
- if (unlikely(bq27xxx_battery_undertemp(di, flags)))
+ if (unlikely(bq27xxx_battery_undertemp(di, di->cache.flags)))
return POWER_SUPPLY_HEALTH_COLD;
- if (unlikely(bq27xxx_battery_dead(di, flags)))
+ if (unlikely(bq27xxx_battery_dead(di, di->cache.flags)))
return POWER_SUPPLY_HEALTH_DEAD;
return POWER_SUPPLY_HEALTH_GOOD;
@@ -1612,6 +1599,7 @@ void bq27xxx_battery_update(struct bq27xxx_device_info *di)
cache.capacity = bq27xxx_battery_read_soc(di);
if (di->regs[BQ27XXX_REG_AE] != INVALID_REG_ADDR)
cache.energy = bq27xxx_battery_read_energy(di);
+ di->cache.flags = cache.flags;
cache.health = bq27xxx_battery_read_health(di);
}
if (di->regs[BQ27XXX_REG_CYCT] != INVALID_REG_ADDR)
--
2.11.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] bq27x00: use cached flags
2019-02-18 6:59 [PATCH] bq27x00: use cached flags Arthur Demchenkov
@ 2019-02-19 16:55 ` Andrew F. Davis
2019-02-19 23:30 ` Sebastian Reichel
0 siblings, 1 reply; 3+ messages in thread
From: Andrew F. Davis @ 2019-02-19 16:55 UTC (permalink / raw)
To: Arthur Demchenkov
Cc: Pali Rohár, Sebastian Reichel, linux-pm, linux-kernel
On 2/18/19 12:59 AM, Arthur Demchenkov wrote:
> The flags were just read by bq27xxx_battery_update(),
> no need to read them again.
>
> Signed-off-by: Arthur Demchenkov <spinal.by@gmail.com>
> ---
Nothing obviously wrong with this patch so:
Reviewed-by: Andrew F. Davis <afd@ti.com>
At this point we have W1 regmap and so we now have everything we should
need to convert this driver over to regmap. Then the caching comes for
free and a lot of checks and such can be dropped.
Thanks,
Andrew
> drivers/power/supply/bq27xxx_battery.c | 20 ++++----------------
> 1 file changed, 4 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
> index 6dbbe95844a3..29b3a4056865 100644
> --- a/drivers/power/supply/bq27xxx_battery.c
> +++ b/drivers/power/supply/bq27xxx_battery.c
> @@ -1555,27 +1555,14 @@ static bool bq27xxx_battery_dead(struct bq27xxx_device_info *di, u16 flags)
> return flags & (BQ27XXX_FLAG_SOC1 | BQ27XXX_FLAG_SOCF);
> }
>
> -/*
> - * Read flag register.
> - * Return < 0 if something fails.
> - */
> static int bq27xxx_battery_read_health(struct bq27xxx_device_info *di)
> {
> - int flags;
> - bool has_singe_flag = di->opts & BQ27XXX_O_ZERO;
> -
> - flags = bq27xxx_read(di, BQ27XXX_REG_FLAGS, has_singe_flag);
> - if (flags < 0) {
> - dev_err(di->dev, "error reading flag register:%d\n", flags);
> - return flags;
> - }
> -
> /* Unlikely but important to return first */
> - if (unlikely(bq27xxx_battery_overtemp(di, flags)))
> + if (unlikely(bq27xxx_battery_overtemp(di, di->cache.flags)))
> return POWER_SUPPLY_HEALTH_OVERHEAT;
> - if (unlikely(bq27xxx_battery_undertemp(di, flags)))
> + if (unlikely(bq27xxx_battery_undertemp(di, di->cache.flags)))
> return POWER_SUPPLY_HEALTH_COLD;
> - if (unlikely(bq27xxx_battery_dead(di, flags)))
> + if (unlikely(bq27xxx_battery_dead(di, di->cache.flags)))
> return POWER_SUPPLY_HEALTH_DEAD;
>
> return POWER_SUPPLY_HEALTH_GOOD;
> @@ -1612,6 +1599,7 @@ void bq27xxx_battery_update(struct bq27xxx_device_info *di)
> cache.capacity = bq27xxx_battery_read_soc(di);
> if (di->regs[BQ27XXX_REG_AE] != INVALID_REG_ADDR)
> cache.energy = bq27xxx_battery_read_energy(di);
> + di->cache.flags = cache.flags;
> cache.health = bq27xxx_battery_read_health(di);
> }
> if (di->regs[BQ27XXX_REG_CYCT] != INVALID_REG_ADDR)
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] bq27x00: use cached flags
2019-02-19 16:55 ` Andrew F. Davis
@ 2019-02-19 23:30 ` Sebastian Reichel
0 siblings, 0 replies; 3+ messages in thread
From: Sebastian Reichel @ 2019-02-19 23:30 UTC (permalink / raw)
To: Andrew F. Davis
Cc: Arthur Demchenkov, Pali Rohár, linux-pm, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 3018 bytes --]
Hi,
On Tue, Feb 19, 2019 at 10:55:37AM -0600, Andrew F. Davis wrote:
> On 2/18/19 12:59 AM, Arthur Demchenkov wrote:
> > The flags were just read by bq27xxx_battery_update(),
> > no need to read them again.
> >
> > Signed-off-by: Arthur Demchenkov <spinal.by@gmail.com>
> > ---
>
> Nothing obviously wrong with this patch so:
>
> Reviewed-by: Andrew F. Davis <afd@ti.com>
>
> At this point we have W1 regmap and so we now have everything we should
> need to convert this driver over to regmap. Then the caching comes for
> free and a lot of checks and such can be dropped.
Yes, bq27xxx could be simplified a lot by converting to regmap and a
patch would be appreciated. This register cannot be cached by regmap,
though. Regmap caching is only for registers, that are not modified
by the hardware (except during reset).
Thanks for the patch and the review, I just merged this to power-supply-next.
-- Sebastian
>
> Thanks,
> Andrew
>
> > drivers/power/supply/bq27xxx_battery.c | 20 ++++----------------
> > 1 file changed, 4 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
> > index 6dbbe95844a3..29b3a4056865 100644
> > --- a/drivers/power/supply/bq27xxx_battery.c
> > +++ b/drivers/power/supply/bq27xxx_battery.c
> > @@ -1555,27 +1555,14 @@ static bool bq27xxx_battery_dead(struct bq27xxx_device_info *di, u16 flags)
> > return flags & (BQ27XXX_FLAG_SOC1 | BQ27XXX_FLAG_SOCF);
> > }
> >
> > -/*
> > - * Read flag register.
> > - * Return < 0 if something fails.
> > - */
> > static int bq27xxx_battery_read_health(struct bq27xxx_device_info *di)
> > {
> > - int flags;
> > - bool has_singe_flag = di->opts & BQ27XXX_O_ZERO;
> > -
> > - flags = bq27xxx_read(di, BQ27XXX_REG_FLAGS, has_singe_flag);
> > - if (flags < 0) {
> > - dev_err(di->dev, "error reading flag register:%d\n", flags);
> > - return flags;
> > - }
> > -
> > /* Unlikely but important to return first */
> > - if (unlikely(bq27xxx_battery_overtemp(di, flags)))
> > + if (unlikely(bq27xxx_battery_overtemp(di, di->cache.flags)))
> > return POWER_SUPPLY_HEALTH_OVERHEAT;
> > - if (unlikely(bq27xxx_battery_undertemp(di, flags)))
> > + if (unlikely(bq27xxx_battery_undertemp(di, di->cache.flags)))
> > return POWER_SUPPLY_HEALTH_COLD;
> > - if (unlikely(bq27xxx_battery_dead(di, flags)))
> > + if (unlikely(bq27xxx_battery_dead(di, di->cache.flags)))
> > return POWER_SUPPLY_HEALTH_DEAD;
> >
> > return POWER_SUPPLY_HEALTH_GOOD;
> > @@ -1612,6 +1599,7 @@ void bq27xxx_battery_update(struct bq27xxx_device_info *di)
> > cache.capacity = bq27xxx_battery_read_soc(di);
> > if (di->regs[BQ27XXX_REG_AE] != INVALID_REG_ADDR)
> > cache.energy = bq27xxx_battery_read_energy(di);
> > + di->cache.flags = cache.flags;
> > cache.health = bq27xxx_battery_read_health(di);
> > }
> > if (di->regs[BQ27XXX_REG_CYCT] != INVALID_REG_ADDR)
> >
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-02-19 23:30 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-18 6:59 [PATCH] bq27x00: use cached flags Arthur Demchenkov
2019-02-19 16:55 ` Andrew F. Davis
2019-02-19 23:30 ` Sebastian Reichel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).