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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241029094141.04540d91@akair>
Date: Tue, 29 Oct 2024 09:41:41 +0100
From: Andreas Kemnade <andreas@...nade.info>
To: Matti Vaittinen <mazziesaccount@...il.com>
Cc: Conor Dooley <conor+dt@...nel.org>, Shawn Guo <shawnguo@...nel.org>,
 linux-kernel@...r.kernel.org, Fabio Estevam <festevam@...il.com>,
 devicetree@...r.kernel.org, Pengutronix Kernel Team
 <kernel@...gutronix.de>, Rob Herring <robh@...nel.org>, Krzysztof Kozlowski
 <krzk+dt@...nel.org>, linux-arm-kernel@...ts.infradead.org, Sascha Hauer
 <s.hauer@...gutronix.de>, Alexander Stein
 <alexander.stein@...tq-group.com>, imx@...ts.linux.dev, sre@...nel.org
Subject: Re: [PATCH v2 2/3] ARM: dts: imx: Add devicetree for Kobo Clara 2E

Am Tue, 29 Oct 2024 09:53:33 +0200
schrieb Matti Vaittinen <mazziesaccount@...il.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@...nade.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