linux-kernel - Re: [PATCH 3/3] power_supply: max77693: Listen for cable events and enable charging

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]

Date:   Tue, 27 Sep 2016 10:13:44 +0200
From:   Krzysztof Kozlowski <krzk@...nel.org>
To:     Wolfgang Wiedmeyer <wolfgit@...dmeyer.de>
Cc:     krzk@...nel.org, sre@...nel.org, dbaryshkov@...il.com,
        dwmw2@...radead.org, cw00.choi@...sung.com,
        b.zolnierkie@...sung.com, broonie@...nel.org, lgirdwood@...il.com,
        lee.jones@...aro.org, linux-pm@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] power_supply: max77693: Listen for cable events and
 enable charging

On Tue, Sep 27, 2016 at 01:31:10AM +0200, Wolfgang Wiedmeyer wrote:
> This patch adds a listener for extcon cable events and enables
> charging if an USB cable is connected. It recognizes SDP and DCP cable
> types and treats them the same (same input current and fast charge
> current). The maximum input current is set before the charger is
> enabled and before the charger gets disabled, the maximum input
> current is set to zero. The listener is inspired by the listener
> implementation that was used for the AXP288 Charger driver.
> 
> The patch also adds support for the CURRENT_NOW property. It reads the
> fast charge current that gets set before the charger is enabled or
> disabled.
> 
> Signed-off-by: Wolfgang Wiedmeyer <wolfgit@...dmeyer.de>

No. This power supply driver should not manage regulators. It is not a
regulator consumer. For that specific need, there is a charger-manager driver.

I agree that you might configure here the charger. You might even expose
some writeable properties through power supply class. However the
purpose of this driver is to expose the battery charger to user-space,
not to replace the user-space with its work.

So... NACK.

If you would like to play with charger-manager, here is my old DTS for
Trats2 (might need updates):

index 595ad4ba6977..b4361b4a9de7 100644
--- a/arch/arm/boot/dts/exynos4412-trats2.dts
+++ b/arch/arm/boot/dts/exynos4412-trats2.dts
@@ -856,6 +856,44 @@
                };
        };
 
+       charger-manager@0 {
+               compatible = "charger-manager";
+               status = "okay";
+               chg-reg-supply = <&charger_reg>;
+
+               cm-name = "battery";
+               /* Polling only for external power source */
+               cm-poll-mode = <2>;
+               cm-poll-interval = <30000>;
+
+               cm-fullbatt-vchkdrop-ms = <30000>;
+               cm-fullbatt-vchkdrop-volt = <150000>;
+               cm-fullbatt-soc = <100>;
+
+               cm-battery-stat = <0>;
+               cm-fuel-gauge = "max170xx_battery";
+
+               /* Allow charging for 5hr */
+               cm-charging-max = <18000000>;
+               /* Allow discharging for 2hr */
+               cm-discharging-max = <7200000>;
+
+               cm-num-chargers = <1>;
+               cm-chargers = "max77693-charger";
+
+               charger@0 {
+                       cm-regulator-name = "chg-reg";
+                       cable@0 {
+                               cm-cable-name = "USB";
+                               cm-cable-extcon = "max77693-muic";
+                       };
+                       cable@1 {
+                               cm-cable-name = "TA";
+                               cm-cable-extcon = "max77693-muic";
+                       };
+               };
+       };
+
        exynos-usbphy@...B0000 {
                status = "okay";
        };

Signed-off-by: Krzysztof Kozlowski <krzk@...nel.org>

Best regards,
Krzysztof