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

[Sebastian Reichel]

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

Attachment: signature.asc
Description: PGP signature