Re: [RFC PATCH v2 09/10] power: supply: Initial support for ROHM BD70528 PMIC charger block

On Thu, Feb 6, 2020 at 8:54 AM Vaittinen, Matti
[off-list ref] wrote:

I started working for a driver for ROHM BD99954 charger chip and ended
up writing the linear range code once again. And that reminded me of
this year old discussion :)

OK!

quoted

They all seem to have trickle and fast charging, USB phy
interaction and AC plug interaction of some kind for detecting
those and in some cases temperature detection directly
or using an ADC.

Yes. That's pretty much what these ROHM chips have. But SW mostly needs
to set the limit values - it does not need to participate in state
changing and most of these things are also something users do not care
(I guess). Fuel gauging is other topic but I'm not planning to
implement it for in-tree drivers at least for now :/

The thing that comes to (my) mind is just a common DT properties for
input current/voltage limits, battery voltages for switching to next
charging state, current values for different charging states - and
parsing of these properties.

I've seen some comments about DT not being a good place for battery
specific properties as batteries may be replaced with another having
different type - but I don't think this is valid problem for many of
the devices today. Of course some devices may have batteries that can
be changed - but many don't - and for those it is perfectly Ok to have
battery data defined in DT, right?

If something is a property of the charging circuit, then it can be derived
from the compatible value, so anything that is just peculiar for this
chip version doesn't go into device tree, just use a custom struct and
the match data for that.

Then we have the battery. I suppose that could be defined as a DT
node for the battery itself, and not as part of the charging IC, and we
should think about how to create a battery DT binding. What
characteristics are chargers really interested in? This could be some
work.

If different devices have different batteries then the boot loader
could ideally patch the DT with the right battery. This was the solution
advocated for devices with different display panels, to a question
from Hans de Goede at one point IIRC.

But ... do we need to provide a way for user-space to configure these
settings instead of the DT? Or any other possible sources for this
information?

What we usually have said is that "policy should be in userspace"
and "kernel manages hardware". This creates a not-so-small
grey area in between and it is unfortunately up to interpretation.

Today there is also a THIRD alternative, and that is to let
userspace insert a policy using a BPF program. This is the
approach taken by the network and tracing stacks and we look at
more applications. If the kernel needs to be able to handle charging
and emergencies even if userspace is not up or available, this or other
userspace policies are not viable.

I have a strong feeling that it should be a battery node in the device
tree.

Hmm. If changing means state changes - then these ROHM chips probably
play no role here as there the HW does most of the state changes
autonomously. (AFAIR, it's been a while since I looked at the other but
BD99954).

Good it's a simpler hardware. We just need to think about making
the code reusable.

So ... If I extract the linear_range code from BD70528 driver (it
should be usable for the BD99954 as such) - should I place it to some
common header/lib? (This is plain integer math so I don't like the
idea of placing it in fixp-arith.h).

Start with moving it to some separate file, like
power_supply_interpolation.c that we just always compile in
and we can take it from there.

Just some thoughts...

Yours,
Linus Walleij