From: Lars-Peter Clausen <lars@metafoo.de>
To: NeilBrown <neilb@suse.de>
Cc: Grazvydas Ignotas <notasas@gmail.com>,
linux-omap@vger.kernel.org, linux-kernel@vger.kernel.org,
Anton Vorontsov <cbouatmailru@gmail.com>
Subject: Re: [PATCH 5/5] bq27x00 - don't report power-supply change so often.
Date: Sat, 31 Dec 2011 12:27:43 +0100 [thread overview]
Message-ID: <4EFEF1AF.9060001@metafoo.de> (raw)
In-Reply-To: <CANOLnOM7QbF5aMb1iW0W2t6B1q1J+cROMU8SpD185DyC33w4kA@mail.gmail.com>
On 12/30/2011 12:13 PM, Grazvydas Ignotas wrote:
> CCing Lars who added this. I vaguely recall something about generating
> events to make some battery monitors update but I forget the details
> now, maybe it was about something else. Also CCing Anton (the
> maintainer).
>
> On Fri, Dec 30, 2011 at 2:58 AM, NeilBrown <neilb@suse.de> wrote:
>> A power_supply_changed should only be reported on significant changes
>> such as transition between charging and not. Incremental changes
>> such as charge increasing should not be reported - that can easily
>> be polled for.
Well, you can also poll for those "significant" changes, too. The point of
adding this was to have a centralized point where polling takes place instead
of letting each battery monitor do this on its own. Though if, as you wrote in
the cover letter, the some properties change every time the values are read it
might makes sense to exclude these from the comparison. On the other hand one
event every 6 minutes doesn't really sound harmful and I would suspect that
battery monitors will use a similar interval when manually polling the device.
- Lars
>>
>> Signed-off-by: NeilBrown <neilb@suse.de>
>> ---
>>
>> drivers/power/bq27x00_battery.c | 15 ++++++++++++---
>> 1 files changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
>> index bb16f5b..7993a17 100644
>> --- a/drivers/power/bq27x00_battery.c
>> +++ b/drivers/power/bq27x00_battery.c
>> @@ -57,11 +57,15 @@
>> #define BQ27000_FLAG_CHGS BIT(7)
>> #define BQ27000_FLAG_FC BIT(5)
>>
>> +#define BQ27000_FLAGS_IMPORTANT (BQ27000_FLAG_FC|BQ27000_FLAG_CHGS|BIT(31))
>> +
>> #define BQ27500_REG_SOC 0x2C
>> #define BQ27500_REG_DCAP 0x3C /* Design capacity */
>> #define BQ27500_FLAG_DSC BIT(0)
>> #define BQ27500_FLAG_FC BIT(9)
>>
>> +#define BQ27500_FLAGS_IMPORTANT (BQ27500_FLAG_FC|BQ27500_FLAG_DSC|BIT(31))
>> +
>> #define BQ27000_RS 20 /* Resistor sense */
>>
>> struct bq27x00_device_info;
>> @@ -259,6 +263,7 @@ static void bq27x00_update(struct bq27x00_device_info *di)
>> {
>> struct bq27x00_reg_cache cache = {0, };
>> bool is_bq27500 = di->chip == BQ27500;
>> + int flags_changed;
>>
>> cache.flags = bq27x00_read(di, BQ27x00_REG_FLAGS, is_bq27500);
>> if (cache.flags >= 0) {
>> @@ -280,10 +285,14 @@ static void bq27x00_update(struct bq27x00_device_info *di)
>>
>> /* Ignore current_now which is a snapshot of the current battery state
>> * and is likely to be different even between two consecutive reads */
>> - if (memcmp(&di->cache, &cache, sizeof(cache) - sizeof(int)) != 0) {
>> - di->cache = cache;
>> + flags_changed = di->cache.flags ^ cache.flags;
>> + di->cache = cache;
>> + if (is_bq27500)
>> + flags_changed &= BQ27500_FLAGS_IMPORTANT;
>> + else
>> + flags_changed &= BQ27000_FLAGS_IMPORTANT;
>> + if (flags_changed)
>> power_supply_changed(&di->bat);
>> - }
>>
>> di->last_update = jiffies;
>> }
next prev parent reply other threads:[~2011-12-31 11:28 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-12-30 0:58 [PATCH 0/5] Improvements to bq27000 management on OMAP NeilBrown
2011-12-30 0:58 ` [PATCH 3/5] omap_hdq: use wait_event_timeout to wait for read to complete NeilBrown
2011-12-30 0:58 ` [PATCH 1/5] Fix w1_bq27000 NeilBrown
2012-02-15 15:36 ` Thomas Weber
2012-02-16 2:18 ` NeilBrown
2011-12-30 0:58 ` [PATCH 5/5] bq27x00 - don't report power-supply change so often NeilBrown
2011-12-30 11:13 ` Grazvydas Ignotas
2011-12-31 11:27 ` Lars-Peter Clausen [this message]
2012-01-03 1:02 ` NeilBrown
2011-12-30 0:58 ` [PATCH 2/5] omap_hdq: Fix some error/debug handling NeilBrown
2011-12-30 0:58 ` [PATCH 4/5] omap_hdq: handle case where isr sees a 0 status byte NeilBrown
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4EFEF1AF.9060001@metafoo.de \
--to=lars@metafoo.de \
--cc=cbouatmailru@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-omap@vger.kernel.org \
--cc=neilb@suse.de \
--cc=notasas@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).