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

Andreas Kemnade
2024-10-29T08:41:47Z

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