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

On Mon, Jan 28, 2019 at 2:54 PM Matti Vaittinen
[off-list ref] wrote:

quoted

These charging drivers are growing wild. This is starting to get out
of hand, we need some more framework for properly handling charging
state machines the kernel. Not specifically your problem, but
when working on the driver try to keep generic support in mind.

I for sure can try - but as the power subsystem is quite new to me - any
specific items you would like me to really pay attention?

So I am not saying you have to do this or anything, but I think
what we need is a generic state machine and policy engine
for charging, where different charging drivers just plug in.

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.

We have a bunch of drivers with software-controlled charging:
ab8500*
axp288*
etc.

If we could get just two drivers to share some changing
infrastructure we would have a start.

I have no idea how hard it could be, but I think it would
be a pretty interesting adventure if someone gathered
some different hardwares and started to generalize parts
of this.

quoted

So what I am seeing is that these states are starting to turn up in more
and more drivers, so we really need to think about a central management
component for charging state machines. I do not think they are all
that different after all.

Any suggestions how I should take this into account with bd70528?

Not more than above, it is unfair to ask random contributors to
invent entire new worlds of kernel infrastructure. But I guess I
just need to say it so we are aware: this charging state machine
engine is needed. Doing it is another thing.

quoted

I am certain you must have a graphical picture of this state
machine somewhere, it seems to be how charging hardware people
do their thinking.

I don't have any document I could link to yet. I can ask around if we
can have some public doc for this :/ And as a last resort I can do some
ASCII art in commenets - if this is seen helpfull.

That'd be nice.

quoted

If I'm not mistaken this is yet another instance of linear interpolation
from a table?

"linear interpolation from a table" is really not part of my
vocabulary :] But I guess you know the REGULATOR_LINEAR_VOLTAGE - macro?
I borrowed the idea from there...

I'm referring to this essentially:
https://en.wikipedia.org/wiki/Linear_interpolation

What many charging drivers tends to do is:
- Look up where I am in a table, say between row 4 and 5
- For x-axis n..n+1, interpolate y-axis between
  the values found for x=1 and x=n+1 using
  common linear interpolation.

quoted

We really need to think about abstracting this. Last time this
duplication appeared I suggested adding linear interpolation
primitives to:
include/linux/fixp-arith.h

... I really think a generic helper for this would be usefull.

Indeed.

It will take some time untill I can send a proper (non RFC patch) for
the charger block as I currently lack of HW I could use for testing the
charger properly. Do you think it is better to drop the charger part
from the series untill then and submit it only later?

Not really, we need to face the hardware out there and I usually
use the stance "rough consensus and running code". If noone
invents a unified charging framework, your hardware needs to
run so a custom driver isn't that bad either, and it's still better
to have it in-tree than to maintain it out-of-tree as long as it
is clearly isolated.

As I mentioned in
cover-letter, the charger part is currently submitted more to give an
overview of the chip than to be applied as 'finalized' version of
driver.

That's OK.

Yours,
Linus Walleij