linux-kernel - Re: [PATCH 5/5] bq27x00 - don't report power-supply change so often.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4EFEF1AF.9060001@metafoo.de>
Date:	Sat, 31 Dec 2011 12:27:43 +0100
From:	Lars-Peter Clausen <lars@...afoo.de>
To:	NeilBrown <neilb@...e.de>
CC:	Grazvydas Ignotas <notasas@...il.com>, linux-omap@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Anton Vorontsov <cbouatmailru@...il.com>
Subject: Re: [PATCH 5/5] bq27x00 - don't report power-supply change so often.

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@...e.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@...e.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;
>>  }

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/