diff mbox series

[v2,6/9] power: supply: max77693: Add USB extcon detection for enabling charging

Message ID 20240715-max77693-charger-extcon-v2-6-0838ffbb18c3@gmail.com (mailing list archive)
State New, archived
Headers show
Series power: supply: max77693: Toggle charging/OTG based on extcon status | expand

Commit Message

Artur Weber July 15, 2024, 12:55 p.m. UTC
Add a device tree property, "maxim,usb-connector", that can be used to
specify a USB connector to use to detect whether a charging cable has
been plugged in/out, and enable/disable charging accordingly.

To accommodate this, also add an internal pointer to the CHARGER regulator,
which is used to enable/disable charging and set the input current limit
(which will be done in subsequent commits).

The extcon listener/worker implementation is inspired by the rt5033_charger
driver.

Signed-off-by: Artur Weber <aweber.kernel@gmail.com>
---
Changes in v2:
- Changed to adapt to both current limits being managed by one function
---
 drivers/power/supply/Kconfig            |   1 +
 drivers/power/supply/max77693_charger.c | 125 ++++++++++++++++++++++++++++++++
 2 files changed, 126 insertions(+)

Comments

Artur Weber July 20, 2024, 8:59 p.m. UTC | #1
On 15.07.2024 14:55, Artur Weber wrote:
> Add a device tree property, "maxim,usb-connector", that can be used to
> specify a USB connector to use to detect whether a charging cable has
> been plugged in/out, and enable/disable charging accordingly.
> 
> To accommodate this, also add an internal pointer to the CHARGER regulator,
> which is used to enable/disable charging and set the input current limit
> (which will be done in subsequent commits).
> 
> The extcon listener/worker implementation is inspired by the rt5033_charger
> driver.
> 
> Signed-off-by: Artur Weber <aweber.kernel@gmail.com>
> ...> diff --git a/drivers/power/supply/max77693_charger.c 
b/drivers/power/supply/max77693_charger.c
> index 0ddaa03669a2..2dc273dd96ee 100644
> --- a/drivers/power/supply/max77693_charger.c
> +++ b/drivers/power/supply/max77693_charger.c
> ...
> +static int max77693_set_charging(struct max77693_charger *chg, bool enable)
> +{
> +	int is_enabled;
> +	int ret = 0;
> +
> +	is_enabled = regulator_is_enabled(chg->regu);
> +	if (is_enabled < 0)
> +		return is_enabled;
> +
> +	if (enable && !is_enabled)
> +		ret = regulator_enable(chg->regu);
> +	else if (!enable && is_enabled)
> +		ret = regulator_disable(chg->regu);
> +
> +	return ret;
> +}

Nevermind, the regulator-based approach simply doesn't work here, 
because we pretty frequently end up in a situation like this:

[   20.162612] ------------[ cut here ]------------
[   20.162618] WARNING: CPU: 0 PID: 29 at drivers/regulator/core.c:3015 
_regulator_disable+0x140/0x1a0
[   20.162645] unbalanced disables for CHARGER
[   20.162649] Modules linked in: fuse brcmfmac_wcc hci_uart btintel 
btbcm bluetooth snd_soc_midas_wm1811 st_accel_i2c st_sensors_i2c 
st_accel_spi st_accel brcmfmac ecdh_generic st_sensors st_sensors_spi 
ecc libaes brcmutil cfg80211 rfkill exynos_adc yamaha_yas530 
industrialio_triggered_buffer kfifo_buf exynos_rng s5p_sss cm3323 
industrialio
[   20.162737] CPU: 0 PID: 29 Comm: kworker/0:2 Tainted: G        W 
     6.10.0-postmarketos-exynos4 #82
[   20.162747] Hardware name: Samsung Exynos (Flattened Device Tree)
[   20.162754] Workqueue: events max77693_charger_extcon_work
[   20.162770] Call trace:
[   20.162778]  unwind_backtrace from show_stack+0x10/0x14
[   20.162804]  show_stack from dump_stack_lvl+0x50/0x64
[   20.162824]  dump_stack_lvl from __warn+0x94/0xc0
[   20.162838]  __warn from warn_slowpath_fmt+0x120/0x1b4
[   20.162855]  warn_slowpath_fmt from _regulator_disable+0x140/0x1a0
[   20.162873]  _regulator_disable from regulator_disable+0x34/0x6c
[   20.162890]  regulator_disable from 
max77693_charger_extcon_work+0x1e4/0x268
[   20.162907]  max77693_charger_extcon_work from 
process_one_work+0x154/0x2dc
[   20.162925]  process_one_work from worker_thread+0x250/0x438
[   20.162941]  worker_thread from kthread+0x110/0x12c
[   20.162958]  kthread from ret_from_fork+0x14/0x28
[   20.162970] Exception stack(0xc18edfb0 to 0xc18edff8)
[   20.162979] dfa0:                                     00000000 
00000000 00000000 00000000
[   20.162987] dfc0: 00000000 00000000 00000000 00000000 00000000 
00000000 00000000 00000000
[   20.162994] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000
[   20.162999] ---[ end trace 0000000000000000 ]---
[   20.163007] max77693-charger max77693-charger: failed to set charging 
(-5)

This can be reproduced by booting the device in with no cable plugged 
in, then plugging in an OTG cable. It prints the warning on connect and 
disconnect. More importantly, this prevents a switch to/from OTG mode in 
the scenarios that print a warning. (I've also encountered some issues 
with similar warnings being printed on unsuspend, but wasn't able to 
reproduce them; I'm willing to assume they were related to plugging in 
an OTG cable as the wakeup trigger.)

As far as I understand it, this is because regulator_is_enabled checks 
for the hardware state, not for the in-software regulator enable count, 
and the charging bits are flipped on by default (presumably by the 
bootloader). I thought regulator-boot-on would handle this, but clearly 
it doesn't...

Best regards
Artur
diff mbox series

Patch

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index 3e31375491d5..e4aeff9e7ad1 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -549,6 +549,7 @@  config CHARGER_MAX77650
 config CHARGER_MAX77693
 	tristate "Maxim MAX77693 battery charger driver"
 	depends on MFD_MAX77693
+	depends on EXTCON || !EXTCON
 	help
 	  Say Y to enable support for the Maxim MAX77693 battery charger.
 
diff --git a/drivers/power/supply/max77693_charger.c b/drivers/power/supply/max77693_charger.c
index 0ddaa03669a2..2dc273dd96ee 100644
--- a/drivers/power/supply/max77693_charger.c
+++ b/drivers/power/supply/max77693_charger.c
@@ -5,6 +5,8 @@ 
 // Copyright (C) 2014 Samsung Electronics
 // Krzysztof Kozlowski <krzk@kernel.org>
 
+#include <linux/devm-helpers.h>
+#include <linux/extcon.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/power_supply.h>
@@ -30,6 +32,13 @@  struct max77693_charger {
 	u32 batttery_overcurrent;
 	u32 fast_charge_current;
 	u32 charge_input_threshold_volt;
+
+	/* USB cable notifications */
+	struct {
+		struct extcon_dev *edev;
+		struct notifier_block nb;
+		struct work_struct work;
+	} cable;
 };
 
 static int max77693_get_charger_state(struct regmap *regmap, int *val)
@@ -664,17 +673,110 @@  static int max77693_reg_init(struct max77693_charger *chg)
 			chg->charge_input_threshold_volt);
 }
 
+static int max77693_set_charging(struct max77693_charger *chg, bool enable)
+{
+	int is_enabled;
+	int ret = 0;
+
+	is_enabled = regulator_is_enabled(chg->regu);
+	if (is_enabled < 0)
+		return is_enabled;
+
+	if (enable && !is_enabled)
+		ret = regulator_enable(chg->regu);
+	else if (!enable && is_enabled)
+		ret = regulator_disable(chg->regu);
+
+	return ret;
+}
+
+static void max77693_charger_extcon_work(struct work_struct *work)
+{
+	struct max77693_charger *chg = container_of(work, struct max77693_charger,
+						  cable.work);
+	struct extcon_dev *edev = chg->cable.edev;
+	int connector, state;
+	int ret;
+
+	for (connector = EXTCON_USB_HOST; connector <= EXTCON_CHG_USB_PD;
+	     connector++) {
+		state = extcon_get_state(edev, connector);
+		if (state == 1)
+			break;
+	}
+
+	switch (connector) {
+	case EXTCON_CHG_USB_SDP:
+	case EXTCON_CHG_USB_DCP:
+	case EXTCON_CHG_USB_CDP:
+	case EXTCON_CHG_USB_ACA:
+	case EXTCON_CHG_USB_FAST:
+	case EXTCON_CHG_USB_SLOW:
+	case EXTCON_CHG_USB_PD:
+		ret = max77693_set_charging(chg, true);
+		if (ret) {
+			dev_err(chg->dev, "failed to enable charging\n");
+			break;
+		}
+		dev_info(chg->dev, "charging. connector type: %d\n",
+			 connector);
+		break;
+	default:
+		ret = max77693_set_charging(chg, false);
+		if (ret) {
+			dev_err(chg->dev, "failed to disable charging\n");
+			break;
+		}
+		dev_info(chg->dev, "charging. connector type: %d\n",
+			 connector);
+		break;
+	}
+
+	power_supply_changed(chg->charger);
+}
+
+static int max77693_charger_extcon_notifier(struct notifier_block *nb,
+					  unsigned long event, void *param)
+{
+	struct max77693_charger *chg = container_of(nb, struct max77693_charger,
+						    cable.nb);
+
+	schedule_work(&chg->cable.work);
+
+	return NOTIFY_OK;
+}
+
 #ifdef CONFIG_OF
 static int max77693_dt_init(struct device *dev, struct max77693_charger *chg)
 {
 	struct power_supply_battery_info *battery_info;
 	struct device_node *np = dev->of_node;
+	struct device_node *np_conn, *np_edev;
 
 	if (!np) {
 		dev_err(dev, "no charger OF node\n");
 		return -EINVAL;
 	}
 
+	np_conn = of_parse_phandle(np, "maxim,usb-connector", 0);
+	np_edev = of_get_parent(np_conn);
+
+	chg->cable.edev = extcon_find_edev_by_node(np_edev);
+	if (IS_ERR(chg->cable.edev)) {
+		/*
+		 * In case of deferred extcon probe, defer our probe as well
+		 * until it appears.
+		 */
+		if (PTR_ERR(chg->cable.edev) == -EPROBE_DEFER)
+			return PTR_ERR(chg->cable.edev);
+		/*
+		 * Otherwise, ignore errors (the charger can run without a
+		 * connector provided).
+		 */
+		dev_warn(dev, "no extcon device found in device-tree (%ld)\n",
+			 PTR_ERR(chg->cable.edev));
+	}
+
 	if (of_property_read_u32(np, "maxim,constant-microvolt",
 			&chg->constant_volt))
 		chg->constant_volt = DEFAULT_CONSTANT_VOLT;
@@ -752,6 +854,26 @@  static int max77693_charger_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	/* Set up extcon if the USB connector node was found */
+	if (!IS_ERR(chg->cable.edev)) {
+		ret = devm_work_autocancel(&pdev->dev, &chg->cable.work,
+					   max77693_charger_extcon_work);
+		if (ret) {
+			dev_err(&pdev->dev, "failed: initialize extcon work\n");
+			return ret;
+		}
+
+		chg->cable.nb.notifier_call = max77693_charger_extcon_notifier;
+
+		ret = devm_extcon_register_notifier_all(&pdev->dev,
+							chg->cable.edev,
+							&chg->cable.nb);
+		if (ret) {
+			dev_err(&pdev->dev, "failed: register extcon notifier\n");
+			return ret;
+		}
+	}
+
 	ret = device_create_file(&pdev->dev, &dev_attr_fast_charge_timer);
 	if (ret) {
 		dev_err(&pdev->dev, "failed: create fast charge timer sysfs entry\n");
@@ -778,6 +900,9 @@  static int max77693_charger_probe(struct platform_device *pdev)
 	device_remove_file(&pdev->dev, &dev_attr_top_off_threshold_current);
 	device_remove_file(&pdev->dev, &dev_attr_fast_charge_timer);
 
+	devm_extcon_unregister_notifier_all(&pdev->dev, chg->cable.edev,
+					    &chg->cable.nb);
+
 	return ret;
 }