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

[Wolfgang Wiedmeyer]

Krzysztof Kozlowski writes:

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

When I was in the middle of implementing this, I noticed that the
charger manager does everything that is needed. But it took me quite
some time to configure the DTS correctly until I realized that the
charger manager used a deprecated function
(extcon_register_interest()) and thus couldn't work. And as I didn't see
the charger-manager in any other device's DTS, I thought that this might
not be right way.
But Chanwoo Choi has a fix: https://patchwork.kernel.org/patch/8898541/
So I will try to get it working with this patch.

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

Ok, then I will try to reduce the patch to the CURRENT_NOW property
support.

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

Is there a reason that this patch is not in the kernel? It would have
been very helpful for me :)

Thanks,
Wolfgang

> 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@125B0000 {
>                 status = "okay";
>         };
>
> Signed-off-by: Krzysztof Kozlowski <krzk@xxxxxxxxxx>
>
> Best regards,
> Krzysztof


-- 
Website: https://fossencdi.org
OpenPGP: 0F30 D1A0 2F73 F70A 6FEE  048E 5816 A24C 1075 7FC4
Key download: https://wiedmeyer.de/keys/ww.asc

Attachment: signature.asc
Description: PGP signature