Message-Id: <20240530-max77693-charger-extcon-v1-0-dc2a9e5bdf30@gmail.com>
Date: Thu, 30 May 2024 10:55:50 +0200
From: Artur Weber <aweber.kernel@...il.com>
To: Krzysztof Kozlowski <krzk@...nel.org>, 
 Chanwoo Choi <cw00.choi@...sung.com>
Cc: Sebastian Reichel <sre@...nel.org>, Rob Herring <robh@...nel.org>, 
 Conor Dooley <conor+dt@...nel.org>, Lee Jones <lee@...nel.org>, 
 Krzysztof Kozlowski <krzk+dt@...nel.org>, 
 Alim Akhtar <alim.akhtar@...sung.com>, linux-pm@...r.kernel.org, 
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
 linux-arm-kernel@...ts.infradead.org, linux-samsung-soc@...r.kernel.org, 
 ~postmarketos/upstreaming@...ts.sr.ht, Henrik Grimler <henrik@...mler.se>, 
 Wolfgang Wiedmeyer <wolfgit@...dmeyer.de>, 
 Denis 'GNUtoo' Carikli <GNUtoo@...erdimension.org>, 
 Artur Weber <aweber.kernel@...il.com>
Subject: [PATCH RFC 00/11] power: supply: max77693: Toggle charging/OTG
 based on extcon status

This patchset does the following:

- Add CURRENT_MAX and INPUT_CURRENT_MAX power supply properties to
  expose the "fast charge current" (maximum current from charger to
  battery) and "CHGIN input current limit" (maximum current from
  external supply to charger).

- Add functions for toggling charging and OTG modes.

- Add an extcon-based handler that enables charging or OTG depending
  on the cable type plugged in. The extcon device to use for cable
  detection can be specified in the device tree, and is entirely
  optional.

The extcon listener implementation is inspired by the rt5033 charger
driver (commit 8242336dc8a8 ("power: supply: rt5033_charger: Add cable
detection and USB OTG supply")).

Signed-off-by: Artur Weber <aweber.kernel@...il.com>
---
This started off as a cleanup of "[PATCH 0/3] max77693: USB event listener
for charger"[1] (and its later iterations[2]), but I ended up replacing 99%
of the code and adding a few more additions on top.

This patchset was made for two reasons:

- I noticed that charging on my Galaxy Tab 3 8.0 seems to "work", but is
  so slow that it starts to discharge during normal usage. After some
  investigation I found that under the downstream kernel, the charging
  limit registers (two of them! see patch descriptions) are changed to a
  higher value when plugged into a charger, and adjusted based on the cable
  type.

- I was investigating what was needed to get OTG support running, and
  found that some OTG mode bits had to be set in the charger when an OTG
  cable is detected, and that required me to modify the charging driver.

That's why the patchset includes *both* charging detection and OTG support.

== Kernel-side charging management ==

The original patchset this was meant to supersede was denied because,
among other reasons, upstream did not agree to having the charging be
controlled this way in the charger driver, suggesting that either
user-space or the charger-manager driver should do it[3][4].

charger-manager is not used in any devices, plus the already mentioned
patchset comments suggest that it's not the correct way to do things
("DT should be used for concrete hardware, not glue drivers")[4].

There are also some drivers that already do this sort of kernel-side charger
toggling/management: other than the aforementioned RT5033 driver which this
new patchset is based off[5], there's also the MAX8997 charger[6] (the patch
for which was merged just a year after the MAX77693 discussion[7]!) and the
AXP288 driver[8]. Still, that's just 3 out of many more drivers...

For this reason, I'm sending this first version of the patchset as an RFC
for further discussion. I've done some thinking about what a potential
implementation allowing for controlling charging from userspace might look
like, and here's what I came up with:

- Allow for setting the charging input current by setting the INPUT_CURRENT_
  MAX property of the power supply. (Leave current from charger to battery
  set by the device tree in the kernel, since it's arguably more dangerous.)

- Allow for setting the CHARGER regulator on/off through some sysfs property.
  How this should be done is tough to specify; there seems to be a few
  ways to do it (e.g. setting the STATUS property[9]), we could use a custom
  sysfs property.

- Add a custom sysfs property to disable kernel-side charging.

About that last one - I'm convinced at least *some* level of kernel-side
charging control should exist, and that's for a practical reason:

A reccuring complaint[10] is devices that are low on battery and plugged
into a charger shutting down before they can fully boot. If we set up
charging during boot, the charger driver can handle charging until a
userspace daemon appears and takes over.

Of course, this is all up for discussion - comments appreciated ;)

== CHARGER regulator ==

I think it's worth also analysing the role that the CHARGER regulator plays
here. At the moment, the CHARGER regulator (managed in drivers/regulator/
max77693-regulator.c) handles:

* the CHGIN input limit as the regulator current limit;
* charger enable/disable as the regulator enable/disable.

A comment in the regulator driver suggests that this should in fact manage
the *fast charge* current, not the CHGIN input current, and that "this
should be fixed"[11]. But now we're managing the fast charge current in
the charger driver... and, if the regulator driver ever was to be "fixed"
this way, it would break the charger driver's ability to set input current.

We could potentially drop the CHARGER regulator and manage these registers
within the charger driver, but since it's not causing us any real problems
at the moment (and the "fix" scenario mentioned earlier is very unlikely),
I decided to keep it and use it to handle the two functions mentioned above.
(Although one of the reasons for the superseded patchset being denied was
that "[this] power supply driver should not manage regulators"[3]...)

---

Special thanks to Wolfgang Wiedmeyer whose original patchset[1] served as
the base for this one, and to Henrik Grimler for helping me out with
figuring out how the input current/fast charge current registers are
used.

[1] https://lore.kernel.org/all/1474932670-11953-1-git-send-email-wolfgit@wiedmeyer.de/
[2] https://lore.kernel.org/all/20190910200233.3195-1-GNUtoo@cyberdimension.org/
[3] https://lore.kernel.org/all/20160927081344.GC4394@kozik-lap/
[4] https://lore.kernel.org/all/298d81d5-fe41-e2d1-32a7-d3dc35b0fe25@kernel.org/
[5] https://github.com/torvalds/linux/blob/v6.9/drivers/power/supply/rt5033_charger.c
[6] https://github.com/torvalds/linux/blob/v6.9/drivers/power/supply/max8997_charger.c#L98-L141
[7] https://github.com/torvalds/linux/commit/f384989e88d4484fc9a9e31b0cf0a36e6f172136
[8] https://github.com/torvalds/linux/blob/v6.9/drivers/power/supply/axp288_charger.c#L617-L673
[9] https://github.com/torvalds/linux/blob/v6.9/drivers/power/supply/rt9471.c#L370-L371
[10] https://gitlab.com/postmarketOS/pmaports/-/issues/2594
[11] https://github.com/torvalds/linux/blob/v6.9/drivers/regulator/max77693-regulator.c#L45-L54

---
Artur Weber (10):
      dt-bindings: power: supply: max77693: Add fast charge current property
      dt-bindings: power: supply: max77693: Add maxim,usb-connector property
      mfd: max77693: Add defines for OTG control
      power: supply: max77693: Expose INPUT_CURRENT_LIMIT and CURRENT_MAX
      power: supply: max77693: Set charge current limits during init
      power: supply: max77693: Add USB extcon detection for enabling charging
      power: supply: max77693: Add support for detecting and enabling OTG
      power: supply: max77693: Set up charge/input current according to cable type
      ARM: dts: samsung: exynos4212-tab3: Set fast charge current for MAX77693
      ARM: dts: samsung: exynos4212-tab3: Add USB connector node

Wolfgang Wiedmeyer (1):
      mfd: max77693: Add defines for charger current control

 .../bindings/power/supply/maxim,max77693.yaml      |  13 +
 arch/arm/boot/dts/samsung/exynos4212-tab3.dtsi     |  14 +
 drivers/power/supply/Kconfig                       |   1 +
 drivers/power/supply/max77693_charger.c            | 291 +++++++++++++++++++++
 include/linux/mfd/max77693-private.h               |   9 +
 5 files changed, 328 insertions(+)
---
base-commit: a38297e3fb012ddfa7ce0321a7e5a8daeb1872b6
change-id: 20240525-max77693-charger-extcon-9ebb7bad83ce

Best regards,
-- 
Artur Weber <aweber.kernel@...il.com>

Powered by blists - more mailing lists