diff mbox series

[1/2] usb: dwc3: dwc3-qcom: Find USB connector and register role switch

Message ID 20210629144449.2550737-2-bryan.odonoghue@linaro.org
State New
Headers show
Series dwc3-qcom: Prepare the ground for pm8150b tcpm | expand

Commit Message

Bryan O'Donoghue June 29, 2021, 2:44 p.m. UTC
From: Wesley Cheng <wcheng@codeaurora.org>


If registering a USB typeC connector, the connector node may not be a child
of the DWC3 QCOM device.  Utilize devcon graph search to lookup if any
remote endpoints contain the connector.  If a connector is present, the
DWC3 QCOM will register a USB role switch to receive role change events, as
well as attain a reference to the DWC3 core role switch to pass the event
down.

Signed-off-by: Wesley Cheng <wcheng@codeaurora.org>

Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

---
 drivers/usb/dwc3/dwc3-qcom.c | 118 ++++++++++++++++++++++++++++++++++-
 1 file changed, 116 insertions(+), 2 deletions(-)

-- 
2.30.1

Comments

Bjorn Andersson June 29, 2021, 3:48 p.m. UTC | #1
On Tue 29 Jun 09:44 CDT 2021, Bryan O'Donoghue wrote:

> From: Wesley Cheng <wcheng@codeaurora.org>

> 

> If registering a USB typeC connector, the connector node may not be a child

> of the DWC3 QCOM device.  Utilize devcon graph search to lookup if any

> remote endpoints contain the connector.  If a connector is present, the

> DWC3 QCOM will register a USB role switch to receive role change events, as

> well as attain a reference to the DWC3 core role switch to pass the event

> down.

> 


What's wrong with the switch that dwc3_setup_role_switch() sets up?

That's what I've been using in my UCSI hacking on Snapdragon 888 and it
seems to work...

Regards,
Bjorn

> Signed-off-by: Wesley Cheng <wcheng@codeaurora.org>

> Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

> ---

>  drivers/usb/dwc3/dwc3-qcom.c | 118 ++++++++++++++++++++++++++++++++++-

>  1 file changed, 116 insertions(+), 2 deletions(-)

> 

> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c

> index 49e6ca94486d..4491704503ab 100644

> --- a/drivers/usb/dwc3/dwc3-qcom.c

> +++ b/drivers/usb/dwc3/dwc3-qcom.c

> @@ -20,6 +20,8 @@

>  #include <linux/usb/of.h>

>  #include <linux/reset.h>

>  #include <linux/iopoll.h>

> +#include <linux/fwnode.h>

> +#include <linux/usb/role.h>

>  

>  #include "core.h"

>  

> @@ -82,6 +84,9 @@ struct dwc3_qcom {

>  	struct notifier_block	vbus_nb;

>  	struct notifier_block	host_nb;

>  

> +	struct usb_role_switch *role_sw;

> +	struct usb_role_switch *dwc3_drd_sw;

> +

>  	const struct dwc3_acpi_pdata *acpi_pdata;

>  

>  	enum usb_dr_mode	mode;

> @@ -296,6 +301,73 @@ static void dwc3_qcom_interconnect_exit(struct dwc3_qcom *qcom)

>  	icc_put(qcom->icc_path_apps);

>  }

>  

> +static int dwc3_qcom_usb_role_switch_set(struct usb_role_switch *sw,

> +					 enum usb_role role)

> +{

> +	struct dwc3_qcom *qcom = usb_role_switch_get_drvdata(sw);

> +	struct fwnode_handle *child;

> +	bool enable = false;

> +

> +	if (!qcom->dwc3_drd_sw) {

> +		child = device_get_next_child_node(qcom->dev, NULL);

> +		if (child) {

> +			qcom->dwc3_drd_sw = usb_role_switch_find_by_fwnode(child);

> +			fwnode_handle_put(child);

> +			if (IS_ERR(qcom->dwc3_drd_sw)) {

> +				qcom->dwc3_drd_sw = NULL;

> +				return 0;

> +			}

> +		}

> +	}

> +

> +	usb_role_switch_set_role(qcom->dwc3_drd_sw, role);

> +

> +	if (role == USB_ROLE_DEVICE)

> +		enable = true;

> +	else

> +		enable = false;

> +

> +	qcom->mode = (role == USB_ROLE_HOST) ? USB_DR_MODE_HOST :

> +					       USB_DR_MODE_PERIPHERAL;

> +	dwc3_qcom_vbus_overrride_enable(qcom, enable);

> +	return 0;

> +}

> +

> +static enum usb_role dwc3_qcom_usb_role_switch_get(struct usb_role_switch *sw)

> +{

> +	struct dwc3_qcom *qcom = usb_role_switch_get_drvdata(sw);

> +	enum usb_role role;

> +

> +	switch (qcom->mode) {

> +	case USB_DR_MODE_HOST:

> +		role = USB_ROLE_HOST;

> +		break;

> +	case USB_DR_MODE_PERIPHERAL:

> +		role = USB_ROLE_DEVICE;

> +		break;

> +	default:

> +		role = USB_ROLE_DEVICE;

> +		break;

> +	}

> +

> +	return role;

> +}

> +

> +static int dwc3_qcom_setup_role_switch(struct dwc3_qcom *qcom)

> +{

> +	struct usb_role_switch_desc dwc3_role_switch = {NULL};

> +

> +	dwc3_role_switch.fwnode = dev_fwnode(qcom->dev);

> +	dwc3_role_switch.set = dwc3_qcom_usb_role_switch_set;

> +	dwc3_role_switch.get = dwc3_qcom_usb_role_switch_get;

> +	dwc3_role_switch.driver_data = qcom;

> +	qcom->role_sw = usb_role_switch_register(qcom->dev, &dwc3_role_switch);

> +	if (IS_ERR(qcom->role_sw))

> +		return PTR_ERR(qcom->role_sw);

> +

> +	return 0;

> +}

> +

>  static void dwc3_qcom_disable_interrupts(struct dwc3_qcom *qcom)

>  {

>  	if (qcom->hs_phy_irq) {

> @@ -698,6 +770,40 @@ dwc3_qcom_create_urs_usb_platdev(struct device *dev)

>  	return acpi_create_platform_device(adev, NULL);

>  }

>  

> +static int dwc3_qcom_connector_check(struct fwnode_handle *fwnode)

> +{

> +	if (fwnode && (!fwnode_property_match_string(fwnode, "compatible",

> +						     "gpio-usb-b-connector") ||

> +	    !fwnode_property_match_string(fwnode, "compatible",

> +					  "usb-c-connector")))

> +		return 1;

> +

> +	return 0;

> +}

> +

> +static void *dwc3_qcom_find_usb_connector_match(struct fwnode_handle *fwnode,

> +						const char *id, void *data)

> +{

> +	/* Check if the "connector" node is the parent of the remote endpoint */

> +	if (dwc3_qcom_connector_check(fwnode))

> +		return fwnode;

> +

> +	/* else, check if it is a child node */

> +	fwnode = fwnode_get_named_child_node(fwnode, "connector");

> +	if (dwc3_qcom_connector_check(fwnode))

> +		return fwnode;

> +

> +	return 0;

> +}

> +

> +static bool dwc3_qcom_find_usb_connector(struct platform_device *pdev)

> +{

> +	struct fwnode_handle *fwnode = pdev->dev.fwnode;

> +

> +	return fwnode_connection_find_match(fwnode, "connector", NULL,

> +					    dwc3_qcom_find_usb_connector_match);

> +}

> +

>  static int dwc3_qcom_probe(struct platform_device *pdev)

>  {

>  	struct device_node	*np = pdev->dev.of_node;

> @@ -813,8 +919,13 @@ static int dwc3_qcom_probe(struct platform_device *pdev)

>  	if (qcom->mode == USB_DR_MODE_PERIPHERAL)

>  		dwc3_qcom_vbus_overrride_enable(qcom, true);

>  

> -	/* register extcon to override sw_vbus on Vbus change later */

> -	ret = dwc3_qcom_register_extcon(qcom);

> +	if (dwc3_qcom_find_usb_connector(pdev)) {

> +		ret = dwc3_qcom_setup_role_switch(qcom);

> +	} else {

> +		/* register extcon to override sw_vbus on Vbus change later */

> +		ret = dwc3_qcom_register_extcon(qcom);

> +	}

> +

>  	if (ret)

>  		goto interconnect_exit;

>  

> @@ -850,6 +961,9 @@ static int dwc3_qcom_remove(struct platform_device *pdev)

>  	struct device *dev = &pdev->dev;

>  	int i;

>  

> +	usb_role_switch_unregister(qcom->role_sw);

> +	usb_role_switch_put(qcom->dwc3_drd_sw);

> +

>  	device_remove_software_node(&qcom->dwc3->dev);

>  	of_platform_depopulate(dev);

>  

> -- 

> 2.30.1

>
Bryan O'Donoghue June 29, 2021, 7:23 p.m. UTC | #2
On 29/06/2021 16:48, Bjorn Andersson wrote:
> What's wrong with the switch that dwc3_setup_role_switch() sets up?
> 
> That's what I've been using in my UCSI hacking on Snapdragon 888 and it
> seems to work...
> 
> Regards,
> Bjorn

A good question, which as soon as you asked it made me completely doubt 
if I'd tested the tree without the patch applied.

I reverted both on my working tree and indeed it breaks role-switch 
detection.

In TCPM the connector is a child node of TCPM

https://git.linaro.org/people/bryan.odonoghue/kernel.git/tree/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts?h=usb-next-5.13.rcx-rb5-tcpm 
line 1396

We use a remote endpoint inside of TCPM to hook up into &usb_1 line 1303

Not entirely sure what the graph tree looks like in your USCI case but I 
guess the connector is a child node of the controller ?

But I think your question is why bother with the role-switch in dwc3-qcom

dwc3_qcom_vbus_override_enable(){} is switching bits that probably ought 
to be in the PHY but for whatever reason ended up being buried in the 
qcom-dwc3 wrapper.

Certainly we want that for qcs405 if you remember - I'm assuming for the 
sm8x50 too.

Even if we shouldn't twiddle these bits on sm8x50 I believe its wanted 
on qcs405.

I'm open to correction on that one though

---
bod
Jack Pham June 29, 2021, 8:02 p.m. UTC | #3
On Tue, Jun 29, 2021 at 08:23:46PM +0100, Bryan O'Donoghue wrote:
> On 29/06/2021 16:48, Bjorn Andersson wrote:

> > What's wrong with the switch that dwc3_setup_role_switch() sets up?

> > 

> > That's what I've been using in my UCSI hacking on Snapdragon 888 and it

> > seems to work...


Bjorn are you exercising dual-role or just host mode?

> A good question, which as soon as you asked it made me completely doubt if

> I'd tested the tree without the patch applied.

> 

> I reverted both on my working tree and indeed it breaks role-switch

> detection.

> 

> In TCPM the connector is a child node of TCPM

> 

> https://git.linaro.org/people/bryan.odonoghue/kernel.git/tree/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts?h=usb-next-5.13.rcx-rb5-tcpm

> line 1396

> 

> We use a remote endpoint inside of TCPM to hook up into &usb_1 line 1303

> 

> Not entirely sure what the graph tree looks like in your USCI case but I

> guess the connector is a child node of the controller ?

> 

> But I think your question is why bother with the role-switch in dwc3-qcom

> 

> dwc3_qcom_vbus_override_enable(){} is switching bits that probably ought to

> be in the PHY but for whatever reason ended up being buried in the qcom-dwc3

> wrapper.


This. When switching dwc3 into peripheral mode we also need
dwc3_qcom_vbus_override_enable() to write those registers to manually
drive the UTMI VBUS valid signal high to the controller since our SoCs
are never physically wired to the connector's VBUS. These registers
(QSCRATCH_{HS,SS}_PHY_CTRL) however don't belong to the PHYs but are
part of our dwc3 wrapper's IO map and hence dwc3-qcom is the only
appropriate place to handle them since dwc3-qcom has over the years
paired with several different PHYs depending on SoC.

Wesley's approach in $SUBJECT was to designate dwc3-qcom itself as a
usb-role-switch-able device so that in the DT the connector graph
endpoints would wire to the dwc3-qcom device itself instead of its dwc3
core child. The idea would be for dwc3-qcom would intercept the
role_switch_set call from TCPM/UCSI, call the vbus_override_enable() and
then pass on the notification to the child to let dwc3/drd.c handle the
rest.

IIRC there had been an alternate proposal to keep the role switch
connection only at the dwc3 core but in order to handle the vbus
override business an additional notification would have needed to be
done either from the usb_role_switch class driver or as an "upcall" from
dwc3 core -> glue. (found it, it was by you Bryan [1])

[1] https://lore.kernel.org/linux-usb/20200311191501.8165-7-bryan.odonoghue@linaro.org/

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
Bryan O'Donoghue June 29, 2021, 8:16 p.m. UTC | #4
On 29/06/2021 21:02, Jack Pham wrote:
> IIRC there had been an alternate proposal to keep the role switch

> connection only at the dwc3 core but in order to handle the vbus

> override business an additional notification would have needed to be

> done either from the usb_role_switch class driver or as an "upcall" from

> dwc3 core -> glue. (found it, it was by you Bryan [1])

> 

> [1]https://lore.kernel.org/linux-usb/20200311191501.8165-7-bryan.odonoghue@linaro.org/


Which had a bug :( Bjorn found

My excuse for not following up is I had to hand back my hardware and got 
sucked into doing something else.

I think Wesley's approach here is a good one so, that's also why I'm 
re-posting.

I don't have a functional qcs405 setup but, I have validated it on sm8250.

---
bod
Bjorn Andersson June 29, 2021, 8:18 p.m. UTC | #5
On Tue 29 Jun 14:23 CDT 2021, Bryan O'Donoghue wrote:

> On 29/06/2021 16:48, Bjorn Andersson wrote:
> > What's wrong with the switch that dwc3_setup_role_switch() sets up?
> > 
> > That's what I've been using in my UCSI hacking on Snapdragon 888 and it
> > seems to work...
> > 
> > Regards,
> > Bjorn
> 
> A good question, which as soon as you asked it made me completely doubt if
> I'd tested the tree without the patch applied.
> 
> I reverted both on my working tree and indeed it breaks role-switch
> detection.
> 
> In TCPM the connector is a child node of TCPM
> 
> https://git.linaro.org/people/bryan.odonoghue/kernel.git/tree/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts?h=usb-next-5.13.rcx-rb5-tcpm
> line 1396
> 
> We use a remote endpoint inside of TCPM to hook up into &usb_1 line 1303
> 

That's expected, because the dwc3 role switching logic registers on
&usb_1_dwc3 instead of &usb_1.

> Not entirely sure what the graph tree looks like in your USCI case but I
> guess the connector is a child node of the controller ?
> 

No, the connector currently a child of a completely different rpmsg
device, i.e. in system terms it's the same as your design. One thing
detecting the change in parameters and then invoking the change on the
dwc3 controller (not the qcom variant thereof).

> But I think your question is why bother with the role-switch in dwc3-qcom
> 
> dwc3_qcom_vbus_override_enable(){} is switching bits that probably ought to
> be in the PHY but for whatever reason ended up being buried in the qcom-dwc3
> wrapper.
> 

I missed this part when I wired up my role switcher and things works
fine, but I presume there's a good reason for the vbus override and it's
not unlikely that I missed something in my testing.

I do however think that it would be appropriate to use the generic dwc3
role switcher and then make that somehow poke the vbus_override in the
qcom instance.

> Certainly we want that for qcs405 if you remember - I'm assuming for the
> sm8x50 too.
> 
> Even if we shouldn't twiddle these bits on sm8x50 I believe its wanted on
> qcs405.
> 

Can you remind me about the purpose of these bits?

Regards,
Bjorn

> I'm open to correction on that one though
> 
> ---
> bod
Bjorn Andersson June 29, 2021, 8:30 p.m. UTC | #6
On Tue 29 Jun 15:02 CDT 2021, Jack Pham wrote:

> On Tue, Jun 29, 2021 at 08:23:46PM +0100, Bryan O'Donoghue wrote:
> > On 29/06/2021 16:48, Bjorn Andersson wrote:
> > > What's wrong with the switch that dwc3_setup_role_switch() sets up?
> > > 
> > > That's what I've been using in my UCSI hacking on Snapdragon 888 and it
> > > seems to work...
> 
> Bjorn are you exercising dual-role or just host mode?
> 

I remember testing switching between host and peripheral, but given
your explanation below I doubt that I actually validated that peripheral
mode was functional.

> > A good question, which as soon as you asked it made me completely doubt if
> > I'd tested the tree without the patch applied.
> > 
> > I reverted both on my working tree and indeed it breaks role-switch
> > detection.
> > 
> > In TCPM the connector is a child node of TCPM
> > 
> > https://git.linaro.org/people/bryan.odonoghue/kernel.git/tree/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts?h=usb-next-5.13.rcx-rb5-tcpm
> > line 1396
> > 
> > We use a remote endpoint inside of TCPM to hook up into &usb_1 line 1303
> > 
> > Not entirely sure what the graph tree looks like in your USCI case but I
> > guess the connector is a child node of the controller ?
> > 
> > But I think your question is why bother with the role-switch in dwc3-qcom
> > 
> > dwc3_qcom_vbus_override_enable(){} is switching bits that probably ought to
> > be in the PHY but for whatever reason ended up being buried in the qcom-dwc3
> > wrapper.
> 
> This. When switching dwc3 into peripheral mode we also need
> dwc3_qcom_vbus_override_enable() to write those registers to manually
> drive the UTMI VBUS valid signal high to the controller since our SoCs
> are never physically wired to the connector's VBUS. These registers
> (QSCRATCH_{HS,SS}_PHY_CTRL) however don't belong to the PHYs but are
> part of our dwc3 wrapper's IO map and hence dwc3-qcom is the only
> appropriate place to handle them since dwc3-qcom has over the years
> paired with several different PHYs depending on SoC.
> 
> Wesley's approach in $SUBJECT was to designate dwc3-qcom itself as a
> usb-role-switch-able device so that in the DT the connector graph
> endpoints would wire to the dwc3-qcom device itself instead of its dwc3
> core child. The idea would be for dwc3-qcom would intercept the
> role_switch_set call from TCPM/UCSI, call the vbus_override_enable() and
> then pass on the notification to the child to let dwc3/drd.c handle the
> rest.
> 

I really do object against us adding another role switch implementation
just to, once again, circumvent the annoying split between the platform
specific and core part of the dwc3 driver.

> IIRC there had been an alternate proposal to keep the role switch
> connection only at the dwc3 core but in order to handle the vbus
> override business an additional notification would have needed to be
> done either from the usb_role_switch class driver or as an "upcall" from
> dwc3 core -> glue. (found it, it was by you Bryan [1])
> 
> [1] https://lore.kernel.org/linux-usb/20200311191501.8165-7-bryan.odonoghue@linaro.org/

I liked this, and it worked when I tested it, but iirc it suffered from
the problem that the core's probe may or may not have finished
successfully at the time that of_platform_populate() returns.

But fixing this problem would save us quite a bit of headache.

Regards,
Bjorn
Bryan O'Donoghue June 29, 2021, 9:57 p.m. UTC | #7
On 29/06/2021 21:30, Bjorn Andersson wrote:
> I liked this, and it worked when I tested it, but iirc it suffered from

> the problem that the core's probe may or may not have finished

> successfully at the time that of_platform_populate() returns.

> 

> But fixing this problem would save us quite a bit of headache.


OK.

I will take a look at resurrecting the old patches either fixing the 
probe order - or perhaps using something like Wesley's role-switch to 
have dwc3 core optionally trigger dwc3-qcom

Binding tcpm into &usb_1_dwc3 instead of &usb_1

---
bod
Bryan O'Donoghue July 1, 2021, 2:08 a.m. UTC | #8
On 01/07/2021 02:12, Jack Pham wrote:
> I'm afraid I'm not too familiar with weak symbols. Would that still work
> if dwc3 & dwc3-qcom are built as modules? Also is it supported with
> Clang/LLVM?

__weak would work fine, until you tried to have two strong implementations.

Your linker should choke if dwc3-meson-g12a, dwc3-qcom and dwc3-drd 
linked together with both wrappers implementing normal linkage.

However, I do think its possible to use role switching to have dwc3-drd 
trigger dwc3-qcom.

The role switch code is resilient to deferral so we don't have to solve 
the problem we had with get_drvdata() in the notifier solution and it 
gets us out of the business of having dwc3-qcom relay the role-switch 
onto the core - or indeed care about what sort of connector is attached.

dwc3-qcom shouldn't have to know or care what sort of connector is 
attached to it, ecros, gpio-typec, tcpm or a raw type-c driver and like 
Bjorn said, it shouldn't be the case that the wrapper relays onto the core.

Anyway I'm playing with that prototype now
diff mbox series

Patch

diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index 49e6ca94486d..4491704503ab 100644
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -20,6 +20,8 @@ 
 #include <linux/usb/of.h>
 #include <linux/reset.h>
 #include <linux/iopoll.h>
+#include <linux/fwnode.h>
+#include <linux/usb/role.h>
 
 #include "core.h"
 
@@ -82,6 +84,9 @@  struct dwc3_qcom {
 	struct notifier_block	vbus_nb;
 	struct notifier_block	host_nb;
 
+	struct usb_role_switch *role_sw;
+	struct usb_role_switch *dwc3_drd_sw;
+
 	const struct dwc3_acpi_pdata *acpi_pdata;
 
 	enum usb_dr_mode	mode;
@@ -296,6 +301,73 @@  static void dwc3_qcom_interconnect_exit(struct dwc3_qcom *qcom)
 	icc_put(qcom->icc_path_apps);
 }
 
+static int dwc3_qcom_usb_role_switch_set(struct usb_role_switch *sw,
+					 enum usb_role role)
+{
+	struct dwc3_qcom *qcom = usb_role_switch_get_drvdata(sw);
+	struct fwnode_handle *child;
+	bool enable = false;
+
+	if (!qcom->dwc3_drd_sw) {
+		child = device_get_next_child_node(qcom->dev, NULL);
+		if (child) {
+			qcom->dwc3_drd_sw = usb_role_switch_find_by_fwnode(child);
+			fwnode_handle_put(child);
+			if (IS_ERR(qcom->dwc3_drd_sw)) {
+				qcom->dwc3_drd_sw = NULL;
+				return 0;
+			}
+		}
+	}
+
+	usb_role_switch_set_role(qcom->dwc3_drd_sw, role);
+
+	if (role == USB_ROLE_DEVICE)
+		enable = true;
+	else
+		enable = false;
+
+	qcom->mode = (role == USB_ROLE_HOST) ? USB_DR_MODE_HOST :
+					       USB_DR_MODE_PERIPHERAL;
+	dwc3_qcom_vbus_overrride_enable(qcom, enable);
+	return 0;
+}
+
+static enum usb_role dwc3_qcom_usb_role_switch_get(struct usb_role_switch *sw)
+{
+	struct dwc3_qcom *qcom = usb_role_switch_get_drvdata(sw);
+	enum usb_role role;
+
+	switch (qcom->mode) {
+	case USB_DR_MODE_HOST:
+		role = USB_ROLE_HOST;
+		break;
+	case USB_DR_MODE_PERIPHERAL:
+		role = USB_ROLE_DEVICE;
+		break;
+	default:
+		role = USB_ROLE_DEVICE;
+		break;
+	}
+
+	return role;
+}
+
+static int dwc3_qcom_setup_role_switch(struct dwc3_qcom *qcom)
+{
+	struct usb_role_switch_desc dwc3_role_switch = {NULL};
+
+	dwc3_role_switch.fwnode = dev_fwnode(qcom->dev);
+	dwc3_role_switch.set = dwc3_qcom_usb_role_switch_set;
+	dwc3_role_switch.get = dwc3_qcom_usb_role_switch_get;
+	dwc3_role_switch.driver_data = qcom;
+	qcom->role_sw = usb_role_switch_register(qcom->dev, &dwc3_role_switch);
+	if (IS_ERR(qcom->role_sw))
+		return PTR_ERR(qcom->role_sw);
+
+	return 0;
+}
+
 static void dwc3_qcom_disable_interrupts(struct dwc3_qcom *qcom)
 {
 	if (qcom->hs_phy_irq) {
@@ -698,6 +770,40 @@  dwc3_qcom_create_urs_usb_platdev(struct device *dev)
 	return acpi_create_platform_device(adev, NULL);
 }
 
+static int dwc3_qcom_connector_check(struct fwnode_handle *fwnode)
+{
+	if (fwnode && (!fwnode_property_match_string(fwnode, "compatible",
+						     "gpio-usb-b-connector") ||
+	    !fwnode_property_match_string(fwnode, "compatible",
+					  "usb-c-connector")))
+		return 1;
+
+	return 0;
+}
+
+static void *dwc3_qcom_find_usb_connector_match(struct fwnode_handle *fwnode,
+						const char *id, void *data)
+{
+	/* Check if the "connector" node is the parent of the remote endpoint */
+	if (dwc3_qcom_connector_check(fwnode))
+		return fwnode;
+
+	/* else, check if it is a child node */
+	fwnode = fwnode_get_named_child_node(fwnode, "connector");
+	if (dwc3_qcom_connector_check(fwnode))
+		return fwnode;
+
+	return 0;
+}
+
+static bool dwc3_qcom_find_usb_connector(struct platform_device *pdev)
+{
+	struct fwnode_handle *fwnode = pdev->dev.fwnode;
+
+	return fwnode_connection_find_match(fwnode, "connector", NULL,
+					    dwc3_qcom_find_usb_connector_match);
+}
+
 static int dwc3_qcom_probe(struct platform_device *pdev)
 {
 	struct device_node	*np = pdev->dev.of_node;
@@ -813,8 +919,13 @@  static int dwc3_qcom_probe(struct platform_device *pdev)
 	if (qcom->mode == USB_DR_MODE_PERIPHERAL)
 		dwc3_qcom_vbus_overrride_enable(qcom, true);
 
-	/* register extcon to override sw_vbus on Vbus change later */
-	ret = dwc3_qcom_register_extcon(qcom);
+	if (dwc3_qcom_find_usb_connector(pdev)) {
+		ret = dwc3_qcom_setup_role_switch(qcom);
+	} else {
+		/* register extcon to override sw_vbus on Vbus change later */
+		ret = dwc3_qcom_register_extcon(qcom);
+	}
+
 	if (ret)
 		goto interconnect_exit;
 
@@ -850,6 +961,9 @@  static int dwc3_qcom_remove(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	int i;
 
+	usb_role_switch_unregister(qcom->role_sw);
+	usb_role_switch_put(qcom->dwc3_drd_sw);
+
 	device_remove_software_node(&qcom->dwc3->dev);
 	of_platform_depopulate(dev);