Re: [PATCH v2 2/3] ARM: dts: imx: Add devicetree for Kobo Clara 2E

[Andreas Kemnade <andreas@kemnade.info>]

Am Tue, 29 Oct 2024 09:53:33 +0200
schrieb Matti Vaittinen <mazziesaccount@gmail.com>:

> On 24/10/2024 17:22, Andreas Kemnade wrote:
> > Adds a devicetree for the Kobo Clara 2E Ebook reader. It is based
> > on boards marked with "37NB-E60K2M+4A2" or "37NB-E60K2M+4B0". It is
> > equipped with an i.MX6SLL SoC.
> > 
> > Expected to work:
> >    - Buttons
> >    - Wifi
> >    - Bluetooth
> >      (if Wifi is initialized first, driver does not handle
> > regulators yet)
> >    - LED
> >    - uSD
> >    - USB
> >    - RTC
> >    - Touchscreen
> > 
> > Add human-readable comments for devices without mainlined driver and
> > binding. Such comments can e.g. be help to find testers if someone
> > starts to work on the missing drivers.
> > 
> > Signed-off-by: Andreas Kemnade <andreas@kemnade.info>  
> 
> ...
> 
> > +
> > +	pmic@4b {
> > +		compatible = "rohm,bd71879", "rohm,bd71828";
> > +		reg = <0x4b>;
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&pinctrl_bd71828>;
> > +
> > +		interrupt-parent = <&gpio4>;
> > +		interrupts = <19 IRQ_TYPE_LEVEL_LOW>;
> > +		system-power-controller;
> > +
> > +		clocks = <&clks 0>;
> > +		#clock-cells = <0>;
> > +		clock-output-names = "bd71828-32k-out";
> > +
> > +		gpio-controller;
> > +		#gpio-cells = <2>;
> > +		gpio-reserved-ranges = <0 1>, <2 1>;
> > +
> > +		rohm,charger-sense-resistor-ohms = <30000000>;  
> 
> I am afraid that this one is _my_ very much terrible brainfart. Yeah, 
> pile up the stones and start casting ;)
>
... at everyone who had looked at this and did not question it ;-)
 
> I am fairly sure the sense resistor is 30 mOhm (0,030 Ohm), not 30
> MOhm (30 000 000 Ohm). (And I am the one who misinterpreted the M in
> some email/data-sheet in the past - and never questioned the sanity).
> 
Well, I did question it, but then thought, ok there might be some
current mirror to scale things down so that the massive rsense might
make sense. Well, no schematics here.

> In short, AFAICS the sense resistor is added "in series" to the
> system load. Eg:
> 
>            --------
>        ---| Rsense |-----
>       |    --------      |
>   ---------           -------
> | VSupply |         | Rload |
>   ---------           -------
>       |                  |
>        ------------------
> 
> Hence, by measuring the voltage drop on the Rsense gives us the
> current flowing through the system ( good old U = RI ).
> 
Yes, that is the way I did know how these things are usually done.
So I am still on track.

> I believe having 30 Mohm (30 000 000 Ohm) resistor there would not
> make much of sense... With a Fermi estimate that the system works
> with voltage magnitude of 1V and current magnitude of 1A and then
> applying good old P = UI and U = RI would give us wonderful results
> :) Quite a battery on poor Kobo, right? You'd better to not touch the
> battery termninals ;) Oh, and looking the driver code I've written
> for handling this property... Sometimes I really don't like mirrors :)
> 

> Well, now that I got this out - I suppose this could be
> rohm,charger-sense-resistor-milli-ohms = <30>;
> or
> rohm,charger-sense-resistor-micro-ohms = <30000>;
> 
> I further guess there is no upstreamn binding doc for this property.

The binding doc is upstream. So an impressive amount of maintainers
had a look at it...

Well, everyone seem to entrust Rohm Semiconductors to do magic...
wonderful reputation.

So how to proceed? As this property is not required, I can simply
remove it and add a comment.

> I think there is also no upstream charger driver for the
> BD71828/BD71879 - only an early RFC and some downstream mess - but
> stil it'd be nice to have the property in place as the size of the
> sense resistor is needed when converting coulomb counter register
> values to current.
> 
What are you upstreaming plans here? For all:
I rebased the charger stuff to v6.11 on
https://github.com/akemnade/linux branch kobo/power-6.11

Regards,
Andreas