diff mbox series

[13/13] extcon: intel-cht-wc: Add support for devices with an USB-micro-B connector

Message ID 20211030182813.116672-14-hdegoede@redhat.com
State Not Applicable
Headers show
Series power-suppy/i2c/extcon: Add support for cht-wc PMIC without USB-PD support | expand

Commit Message

Hans de Goede Oct. 30, 2021, 6:28 p.m. UTC 
So far the extcon-intel-cht-wc code has only been tested on devices with
a Type-C connector with USB-PD, USB3 (superspeed) and DP-altmode support
through a FUSB302 Type-C controller.

Some devices with the intel-cht-wc PMIC however come with an USB-micro-B
connector, or an USB-2 only Type-C connector without USB-PD.

These device are identified by "intel,cht-wc-setup" = "bq25890,bq27520",
since there is no Type-C controller on these devices the extcon code must
control the Vbus 5V boost converter and the USB role switch depending on
the detected cable-type.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/extcon/extcon-intel-cht-wc.c | 81 ++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

Comments

Andy Shevchenko Oct. 31, 2021, 12:52 p.m. UTC | #1 
On Sat, Oct 30, 2021 at 9:29 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> So far the extcon-intel-cht-wc code has only been tested on devices with
> a Type-C connector with USB-PD, USB3 (superspeed) and DP-altmode support
> through a FUSB302 Type-C controller.
>
> Some devices with the intel-cht-wc PMIC however come with an USB-micro-B
> connector, or an USB-2 only Type-C connector without USB-PD.
>
> These device are identified by "intel,cht-wc-setup" = "bq25890,bq27520",
> since there is no Type-C controller on these devices the extcon code must
> control the Vbus 5V boost converter and the USB role switch depending on
> the detected cable-type.

...

> +       if (ext->vbus_boost && ext->vbus_boost_enabled != enable) {
> +               if (enable)
> +                       ret = regulator_enable(ext->vbus_boost);
> +               else
> +                       ret = regulator_disable(ext->vbus_boost);

> +               if (ret == 0)
> +                       ext->vbus_boost_enabled = enable;
> +               else
> +                       dev_err(ext->dev, "Error updating Vbus boost regulator: %d\n", ret);

if (ret)
 dev_err()
else
 ...

?

> +       }

...


> +               /*
> +                * Classic micro USB-B setup, this requires controling

controlling

> +                * the role-sw and vbus based on the id-pin.
> +                */
Hans de Goede Nov. 8, 2021, 3:44 p.m. UTC | #2 
Hi,

On 10/31/21 13:52, Andy Shevchenko wrote:
> On Sat, Oct 30, 2021 at 9:29 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> So far the extcon-intel-cht-wc code has only been tested on devices with
>> a Type-C connector with USB-PD, USB3 (superspeed) and DP-altmode support
>> through a FUSB302 Type-C controller.
>>
>> Some devices with the intel-cht-wc PMIC however come with an USB-micro-B
>> connector, or an USB-2 only Type-C connector without USB-PD.
>>
>> These device are identified by "intel,cht-wc-setup" = "bq25890,bq27520",
>> since there is no Type-C controller on these devices the extcon code must
>> control the Vbus 5V boost converter and the USB role switch depending on
>> the detected cable-type.
> 
> ...
> 
>> +       if (ext->vbus_boost && ext->vbus_boost_enabled != enable) {
>> +               if (enable)
>> +                       ret = regulator_enable(ext->vbus_boost);
>> +               else
>> +                       ret = regulator_disable(ext->vbus_boost);
> 
>> +               if (ret == 0)
>> +                       ext->vbus_boost_enabled = enable;
>> +               else
>> +                       dev_err(ext->dev, "Error updating Vbus boost regulator: %d\n", ret);
> 
> if (ret)
>  dev_err()
> else
>  ...
> 
> ?

When doing if-else branches around an error code I always put the success
handling in the if branch and have the else branch deal with the error
to me that feels as the most natural way to do it the error is the exception
and thus the "else"

> 
>> +       }
> 
> ...
> 
> 
>> +               /*
>> +                * Classic micro USB-B setup, this requires controling
> 
> controlling

Fixed for the next version.

Thanks & Regards,

Hans


> 
>> +                * the role-sw and vbus based on the id-pin.
>> +                */
>
diff mbox series

Patch

diff --git a/drivers/extcon/extcon-intel-cht-wc.c b/drivers/extcon/extcon-intel-cht-wc.c
index a7a6b43d699b..d8d0a69f4ddd 100644
--- a/drivers/extcon/extcon-intel-cht-wc.c
+++ b/drivers/extcon/extcon-intel-cht-wc.c
@@ -16,7 +16,9 @@ 
 #include <linux/platform_device.h>
 #include <linux/property.h>
 #include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
 #include <linux/slab.h>
+#include <linux/usb/role.h>
 
 #include "extcon-intel.h"
 
@@ -102,8 +104,11 @@  struct cht_wc_extcon_data {
 	struct device *dev;
 	struct regmap *regmap;
 	struct extcon_dev *edev;
+	struct usb_role_switch *role_sw;
+	struct regulator *vbus_boost;
 	unsigned int previous_cable;
 	bool usb_host;
+	bool vbus_boost_enabled;
 };
 
 static int cht_wc_extcon_get_id(struct cht_wc_extcon_data *ext, int pwrsrc_sts)
@@ -217,6 +222,18 @@  static void cht_wc_extcon_set_otgmode(struct cht_wc_extcon_data *ext,
 				 CHT_WC_CHGRCTRL1_OTGMODE, val);
 	if (ret)
 		dev_err(ext->dev, "Error updating CHGRCTRL1 reg: %d\n", ret);
+
+	if (ext->vbus_boost && ext->vbus_boost_enabled != enable) {
+		if (enable)
+			ret = regulator_enable(ext->vbus_boost);
+		else
+			ret = regulator_disable(ext->vbus_boost);
+
+		if (ret == 0)
+			ext->vbus_boost_enabled = enable;
+		else
+			dev_err(ext->dev, "Error updating Vbus boost regulator: %d\n", ret);
+	}
 }
 
 static void cht_wc_extcon_enable_charging(struct cht_wc_extcon_data *ext,
@@ -246,6 +263,7 @@  static void cht_wc_extcon_pwrsrc_event(struct cht_wc_extcon_data *ext)
 	unsigned int cable = EXTCON_NONE;
 	/* Ignore errors in host mode, as the 5v boost converter is on then */
 	bool ignore_get_charger_errors = ext->usb_host;
+	enum usb_role role;
 
 	ret = regmap_read(ext->regmap, CHT_WC_PWRSRC_STS, &pwrsrc_sts);
 	if (ret) {
@@ -289,6 +307,18 @@  static void cht_wc_extcon_pwrsrc_event(struct cht_wc_extcon_data *ext)
 
 	ext->usb_host = ((id == INTEL_USB_ID_GND) || (id == INTEL_USB_RID_A));
 	extcon_set_state_sync(ext->edev, EXTCON_USB_HOST, ext->usb_host);
+
+	if (ext->usb_host)
+		role = USB_ROLE_HOST;
+	else if (pwrsrc_sts & CHT_WC_PWRSRC_VBUS)
+		role = USB_ROLE_DEVICE;
+	else
+		role = USB_ROLE_NONE;
+
+	/* Note: this is a no-op when ext->role_sw is NULL */
+	ret = usb_role_switch_set_role(ext->role_sw, role);
+	if (ret)
+		dev_err(ext->dev, "Error setting USB-role: %d\n", ret);
 }
 
 static irqreturn_t cht_wc_extcon_isr(int irq, void *data)
@@ -334,6 +364,29 @@  static int cht_wc_extcon_sw_control(struct cht_wc_extcon_data *ext, bool enable)
 	return ret;
 }
 
+static int cht_wc_find_role_sw(struct cht_wc_extcon_data *ext)
+{
+	const struct software_node *swnode;
+	struct fwnode_handle *fwnode;
+
+	swnode = software_node_find_by_name(NULL, "intel-xhci-usb-sw");
+	if (!swnode)
+		return -EPROBE_DEFER;
+
+	fwnode = software_node_fwnode(swnode);
+	ext->role_sw = usb_role_switch_find_by_fwnode(fwnode);
+	fwnode_handle_put(fwnode);
+
+	return ext->role_sw ? 0 : -EPROBE_DEFER;
+}
+
+static void cht_wc_put_role_sw(void *data)
+{
+	struct cht_wc_extcon_data *ext = data;
+
+	usb_role_switch_put(ext->role_sw);
+}
+
 static int cht_wc_extcon_probe(struct platform_device *pdev)
 {
 	struct intel_soc_pmic *pmic = dev_get_drvdata(pdev->dev.parent);
@@ -377,6 +430,34 @@  static int cht_wc_extcon_probe(struct platform_device *pdev)
 		 * external 5v boost converter off and leave it off entirely.
 		 */
 		cht_wc_extcon_set_5v_boost(ext, false);
+	} else if (!strcmp(setup, "bq25890,bq27520")) {
+		/*
+		 * Classic micro USB-B setup, this requires controling
+		 * the role-sw and vbus based on the id-pin.
+		 */
+		ret = cht_wc_find_role_sw(ext);
+		if (ret)
+			return ret;
+
+		ret = devm_add_action_or_reset(ext->dev, cht_wc_put_role_sw, ext);
+		if (ret)
+			return ret;
+
+		/*
+		 * On x86/ACPI platforms the regulator <-> consumer link is provided
+		 * by platform_data passed to the regulator driver. This means that
+		 * this info is not available before the regulator driver has bound.
+		 * Use devm_regulator_get_optional() to avoid getting a dummy
+		 * regulator and wait for the regulator to show up if necessary.
+		 */
+		ext->vbus_boost = devm_regulator_get_optional(ext->dev, "vbus");
+		if (IS_ERR(ext->vbus_boost)) {
+			ret = PTR_ERR(ext->vbus_boost);
+			if (ret == -ENODEV)
+				ret = -EPROBE_DEFER;
+
+			return dev_err_probe(ext->dev, ret, "getting vbus regulator");
+		}
 	}
 
 	/* Enable sw control */