[Gta04-owner] bq27x00 driver for Replicant

Lukas Maerdian luk at slyon.de
Tue Apr 1 11:01:44 CEST 2014


Hi all!

2014-04-01 8:39 GMT+02:00 Dr. H. Nikolaus Schaller <hns at goldelico.com>:
>>> Yes, there *is* something wrong.
>>>
>>> While looking for bugs in the 3.14 kernel I reviewed some diffs of our kernel vs. linus/v3.14
>>>
>>> And found that there is a slightly (or significantly) different rule when to report power_supply_changed()
>>>
>>> We have:
>>>
>>>      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);
>>>
>>> Linus/master has:
>>>
>>>      if (memcmp(&di->cache, &cache, sizeof(cache)) != 0) {
>>>              di->cache = cache;
>>>              power_supply_changed(&di->bat);
>>>      }
>>
>> That was one of my changes.  With the mainline code the bq27000 is every
>> noisy with uevents.
>>
>>>
>>> So our driver reports only changes in *important* flags. And nothing else. And *important*
>>> is mainly the charging status as seen by the bq27x00 chip.
>>>
>>> Contrarily, the mainline driver reports *any* change in the cached data. Being charging status
>>> or level of charge or whatever is stored in di->cache (which is a struct with the values you
>>> can read from the /sys node).
>>>
>>> Neil apparently has added a mask to reduce the reports to charging status.
>>>
>>> Reasons are not clear to me. I don#t know if the commit message in the 3.7 kernel tells something.
>>>
>>> IMHO it is no problem reporting almost any change, since it is polled already very rarely as defined by the poll interval.
>>> So it should neither influence system load nor power demand.
>>>
>>> So I think we should revert neil's extension here (but keep two others).
>>>
>>> If you don't disagree, I can prepare it for the 3.14 kernel and Lukas can backport it to the
>>> 3.12-replicant kernel.
>>>
>>> BR,
>>> Nikolaus
>>
>> Please do feel free to discard any of my changes that seem to be a problem.
>> They seemed like a good idea at the time, but maybe they aren't.
>>
>> NeilBrown
>
> thanks for these explanations. I didn't think about the driver potentially flooding udevd with requests.
> And thanks to question what "real time" means in this case.
>
> IMHO that flooding not pose a problem if the battery is kernel polled only once every 360 seconds. And although this introduces a neglectable delay. If we consider that it takes 3 hours to drain a battery this gives around 30 events. This should make the charging level go down in steps of 3%. If this is not fine grained enough, the poll_interval can be modified from user space.
>
> So the mainline caching mechanism appears to be right.
>
> Replicant should be able to live with poll_interval=360.

Thanks to everybody involved with solving this issue. I've reverted
commit ca8f1c8e2cdbb8a475a397e37c066972dfaeae7c to go back to the
mainline caching mechanism in 3.12-replicant4.

Nikolaus, you can integrate it in 3.14 with:
git cherry-pick 76c42635c60fd6a2380fec2fba448cffe636ce66

BR,
  Lukas

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: OpenPGP digital signature
URL: <http://lists.goldelico.com/pipermail/gta04-owner/attachments/20140401/7f9db49b/attachment.bin>


More information about the Gta04-owner mailing list