linux-omap.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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;
>>  }


  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).