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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 29/10/2024 10:44, Andreas Kemnade wrote:
Am Tue, 29 Oct 2024 09:53:33 +0200
schrieb Matti Vaittinen <mazziesaccount@xxxxxxxxx>:

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@xxxxxxxxxxxx>

...

+
+	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>;

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...

Oh. So I have upstreamed it at some point... I suppose there is no users though.

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 would like to suggest adding:
rohm,charger-sense-resistor-milli-ohms = <30>;

and a binding with minimum 1 and maximum 50. Those should be sane enough limits. At the same time the 'rohm,charger-sense-resistor-ohms' could probably be deprecated.

This, however, is an extra mile for you. So, if you don't feel like doing it, then dropping the entry is Ok as well.


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

Excellent question.

The reason why this driver is not upstream is that (as far as I know) the PMIC variants supported by the driver have always been tailored for a specific customer's needs. I don't think these PMICs have been sold for anyone else besides the specific customers. Hence, there has been little benefit for creating an upstream driver.

For the charger there has been additional complexity because the bd71827-charger driver implements also an in-kernel battery fuel gauge which computes the SOC values and sends them to the user-space. Like you know, this is very battery specific and requires correct battery parameters to be given.

I think your work changes things a bit. Seems we will be having an upstream board (kobo) using the PMIC drivers - and it might also use some information from the charger block. I know you also implemented some control logic to get the charging working with limited power-supplies.

(I think the BD71815 is also used in some projects which wanted to have upstream drivers - but I haven't seen upstream dts using it).

Furthermore, we still seem to be having new variants - I am right now writing drivers for another customer specific PMIC using somewhat similar charging logic. Having upstream driver would help in work like that. (By the way, I took your rebased kobo patches and continued my work on top of those - so thanks for the rebasing!)

So - in my opinion, upstreaming would be beneficial and I will push for this direction - but it will be slow process.

Also, I think that maybe the downstream driver shouldn't be upstreamed as such. I am not 100% convinced the SOC should be computed in-kernel. It'd might be easier to implement the SOC computation in a user-space library which just gets the details from the kernel. On the other hand, I think the driver should support a few currently unsupported features like setting the currents for different charge phases based on device-tree properties.

Giving I am the only guy at ROHM working on these Linux drivers (and not only these drivers), and knowing I don't have a system which is equipped with a real battery - I wouldn't hold my breath waiting :(

Yours,
	-- Matti

Regards,
Andreas





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux