linux-kernel - Re: [PATCH v2] power: supply: bq27xxx: Divide the reg cache to each register

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] [day] [month] [year] [list]

Date: Wed, 10 Apr 2024 10:08:48 +0200
From: Sebastian Reichel <sebastian.reichel@...labora.com>
To: Hermes Zhang <chenhuiz@...s.com>
Cc: Andrew Davis <afd@...com>, Hermes Zhang <Hermes.Zhang@...s.com>, 
	kernel@...s.com, Pali Rohár <pali@...nel.org>, 
	linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] power: supply: bq27xxx: Divide the reg cache to each
 register

Hi,

On Tue, Apr 02, 2024 at 10:04:22AM +0800, Hermes Zhang wrote:
> On 2024/4/1 21:57, Andrew Davis wrote:
> > On 4/1/24 8:15 AM, Sebastian Reichel wrote:
> > > [+cc Andrew Davis]
> > > 
> > > Hello Hermes,
> > > 
> > > Sorry for the delay. This arrived too close to the 6.9 merge window.
> > > I had a look now and while the patch looks fine to me on a conceptual
> > > level, it did not apply. It looks like you used a pre-2024 kernel tree
> > > to generate the patch against. Please always use something recent base
> > > tree (and ideally use git's --base option to document the used
> > > parent commit).
> > > 
> > > Other than that I just applied a series from Andrew, which cleans up
> > > the register caching in bq27xxx and removed most registers from the
> > > cache. That's something I did not consider earlier, since I thought
> > > the cache was introduced to fix a different issue. But that was
> > > apparently sbs-battery and not bq27xxx.
> > 
> > The original BQ27000 device did not have an interrupt pin to signal
> > when important updates to the battery occurred, so early devices
> > using it would have userspace poll those values. As the kernel driver
> > added its own polling for updates, it seems the early driver authors
> > simply decided to cache the values between kernel reads and return
> > those to userspace when it reads.
> > 
> > This is a problem though for two reasons.
> > 1. If no one is interested in these values the kernel will still poll
> >    them anyway, wasting I2C bus time and power.
> > 2. If userspace is actually interested in some value and so checks it
> >    often, it will only get real updated values when the kernel next
> >    polls, which might not be often enough for some use-cases.
> > 
> Agree! Also good to know the history.

well, there was another driver, which had to introduce caching to
reduce I2C bus load when userspace polls data too quickly. Since
bq27xxx already had caching, its removal might have regressed some
platform. If nobody complains I'm of course happy with the much
simpler code :)

-- Sebastian

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)