[PATCH v2 0/6] add USB Type-B GPIO based role switch driver

[PATCH v2 0/6] add USB Type-B GPIO based role switch driver

[Chunfeng Yun]

Because the USB Connector is introduced and the requirement of
usb-connector.txt binding, the old way using extcon to support
USB Dual-Role switch is now deprecated, meanwhile there is no
available common driver when use Type-B connector.
This patch series introduce a Type-B connector driver and try
to replace the function provided by extcon-usb-gpio driver.
The main purpose of the patches is also to solve the Type-B
connector problem encountered in [1].

[1]: https://patchwork.kernel.org/patch/10819377/

v2 changes:
 1. make binding clear, and add a extra compatible suggested by Hans

Chunfeng Yun (6):
  dt-bindings: connector: add optional properties for Type-B
  dt-bindings: usb: add binding for Type-B dual role switch by GPIO
  dt-bindings: usb: mtu3: add properties about USB Role Switch
  usb: roles: add API to get usb_role_switch by node
  usb: roles: add driver of USB Type-B role switch by GPIO
  usb: mtu3: register a USB Role Switch for dual role mode

 .../bindings/connector/usb-connector.txt      |   9 +
 .../devicetree/bindings/usb/mediatek,mtu3.txt |  10 +-
 .../bindings/usb/typeb-switch-gpio.txt        |  26 ++
 drivers/usb/mtu3/mtu3.h                       |   5 +
 drivers/usb/mtu3/mtu3_dr.c                    |  50 ++-
 drivers/usb/mtu3/mtu3_plat.c                  |   3 +-
 drivers/usb/roles/Kconfig                     |  14 +
 drivers/usb/roles/Makefile                    |   1 +
 drivers/usb/roles/class.c                     |  30 ++
 drivers/usb/roles/typeb-switch-gpio.c         | 285 ++++++++++++++++++
 include/linux/usb/role.h                      |   1 +
 11 files changed, 428 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/usb/typeb-switch-gpio.txt
 create mode 100644 drivers/usb/roles/typeb-switch-gpio.c

-- 
2.20.1

[PATCH v2 1/6] dt-bindings: connector: add optional properties for Type-B

[Chunfeng Yun]

Add id-gpios, vbus-gpios, vbus-supply and pinctrl properties for
usb-b-connector

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
v2 changes:
  1. describe more clear for vbus-gpios and vbus-supply suggested by Hans
---
 .../devicetree/bindings/connector/usb-connector.txt      | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/connector/usb-connector.txt b/Documentation/devicetree/bindings/connector/usb-connector.txt
index a9a2f2fc44f2..1c4ed6c9afa8 100644
--- a/Documentation/devicetree/bindings/connector/usb-connector.txt
+++ b/Documentation/devicetree/bindings/connector/usb-connector.txt
@@ -17,6 +17,15 @@ Optional properties:
 - self-powered: Set this property if the usb device that has its own power
   source.
 
+Optional properties for usb-b-connector:
+- id-gpios: gpio for USB ID pin.
+- vbus-gpios: gpio for USB VBUS pin, used to detect presence of VBUS 5V.
+  see gpio/gpio.txt.
+- vbus-supply: a phandle to the regulator for USB VBUS if needed.
+- pinctrl-names : a pinctrl state named "default" is optional
+- pinctrl-0 : pin control group
+  see pinctrl/pinctrl-bindings.txt
+
 Optional properties for usb-c-connector:
 - power-role: should be one of "source", "sink" or "dual"(DRP) if typec
   connector has power support.
-- 
2.20.1

Re: [PATCH v2 1/6] dt-bindings: connector: add optional properties for Type-B

[Rob Herring]

On Fri, Mar 15, 2019 at 03:38:28PM +0800, Chunfeng Yun wrote:
> Add id-gpios, vbus-gpios, vbus-supply and pinctrl properties for
> usb-b-connector
> 
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
> v2 changes:
>   1. describe more clear for vbus-gpios and vbus-supply suggested by Hans
> ---
>  .../devicetree/bindings/connector/usb-connector.txt      | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.txt b/Documentation/devicetree/bindings/connector/usb-connector.txt
> index a9a2f2fc44f2..1c4ed6c9afa8 100644
> --- a/Documentation/devicetree/bindings/connector/usb-connector.txt
> +++ b/Documentation/devicetree/bindings/connector/usb-connector.txt
> @@ -17,6 +17,15 @@ Optional properties:
>  - self-powered: Set this property if the usb device that has its own power
>    source.
>  
> +Optional properties for usb-b-connector:
> +- id-gpios: gpio for USB ID pin.
> +- vbus-gpios: gpio for USB VBUS pin, used to detect presence of VBUS 5V.
> +  see gpio/gpio.txt.

Please make it explicit that this is an input gpio. An output GPIO 
controlling a Vbus regulator should be modelled as a regulator.

> +- vbus-supply: a phandle to the regulator for USB VBUS if needed.
> +- pinctrl-names : a pinctrl state named "default" is optional
> +- pinctrl-0 : pin control group
> +  see pinctrl/pinctrl-bindings.txt
> +
>  Optional properties for usb-c-connector:
>  - power-role: should be one of "source", "sink" or "dual"(DRP) if typec
>    connector has power support.
> -- 
> 2.20.1
> 

Re: [PATCH v2 1/6] dt-bindings: connector: add optional properties for Type-B

[Chunfeng Yun]

Hi Rob,
On Sun, 2019-03-31 at 01:40 -0500, Rob Herring wrote:
> On Fri, Mar 15, 2019 at 03:38:28PM +0800, Chunfeng Yun wrote:
> > Add id-gpios, vbus-gpios, vbus-supply and pinctrl properties for
> > usb-b-connector
> > 
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > ---
> > v2 changes:
> >   1. describe more clear for vbus-gpios and vbus-supply suggested by Hans
> > ---
> >  .../devicetree/bindings/connector/usb-connector.txt      | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/connector/usb-connector.txt b/Documentation/devicetree/bindings/connector/usb-connector.txt
> > index a9a2f2fc44f2..1c4ed6c9afa8 100644
> > --- a/Documentation/devicetree/bindings/connector/usb-connector.txt
> > +++ b/Documentation/devicetree/bindings/connector/usb-connector.txt
> > @@ -17,6 +17,15 @@ Optional properties:
> >  - self-powered: Set this property if the usb device that has its own power
> >    source.
> >  
> > +Optional properties for usb-b-connector:
> > +- id-gpios: gpio for USB ID pin.
> > +- vbus-gpios: gpio for USB VBUS pin, used to detect presence of VBUS 5V.
> > +  see gpio/gpio.txt.
> 
> Please make it explicit that this is an input gpio. An output GPIO 
> controlling a Vbus regulator should be modelled as a regulator.
Ok, thanks

> 
> > +- vbus-supply: a phandle to the regulator for USB VBUS if needed.
> > +- pinctrl-names : a pinctrl state named "default" is optional
> > +- pinctrl-0 : pin control group
> > +  see pinctrl/pinctrl-bindings.txt
> > +
> >  Optional properties for usb-c-connector:
> >  - power-role: should be one of "source", "sink" or "dual"(DRP) if typec
> >    connector has power support.
> > -- 
> > 2.20.1
> > 
> 

[PATCH v2 2/6] dt-bindings: usb: add binding for Type-B dual role switch by GPIO

[Chunfeng Yun]

It's used to support dual role switch via GPIO when use Type-B
receptacle

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
v2 changes:
  1. new patch to make binding clear suggested by Hans
---
 .../bindings/usb/typeb-switch-gpio.txt        | 26 +++++++++++++++++++
 1 file changed, 26 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/typeb-switch-gpio.txt

diff --git a/Documentation/devicetree/bindings/usb/typeb-switch-gpio.txt b/Documentation/devicetree/bindings/usb/typeb-switch-gpio.txt
new file mode 100644
index 000000000000..d46d83bc6500
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/typeb-switch-gpio.txt
@@ -0,0 +1,26 @@
+USB Type-B dual role switch via a GPIO
+
+This is a virtual device used to switch dual role mode from the USB ID pin
+connected to a GPIO pin.
+
+Required properties:
+- compatible: Should be "linux,typeb-switch-gpio"
+
+Either one of id-gpios or vbus-gpios must be present, and both can be present
+as well.
+vbus-supply could be present if needed when support dual role mode.
+	see connector/usb-connector.txt
+
+Example:
+
+&mtu3 {
+	status = "okay";
+
+	connector@0 {
+		compatible = "usb-b-connector", "linux,typeb-switch-gpio";
+		label = "micro-USB";
+		type = "micro";
+		id-gpio = <&pio 12 GPIO_ACTIVE_HIGH>;
+		vbus-supply = <&usb_p0_vbus>;
+	};
+};
-- 
2.20.1

Re: [PATCH v2 2/6] dt-bindings: usb: add binding for Type-B dual role switch by GPIO

[Chunfeng Yun]

Hi Rob,

There are two ways that I think out to support type-B GPIO based dual
role switch.

The first one, as the series used:

&mtu3 {
    status = "okay";

    connector@0 {
        compatible = "usb-b-connector", "linux,typeb-switch-gpio";
        label = "micro-USB";
        type = "micro";
        id-gpio = <&pio 12 GPIO_ACTIVE_HIGH>;
        vbus-supply = <&usb_p0_vbus>;
    };
};

but there are some shortcomings:
1. It's not transparent, USB controller driver should be modified, and
call of_platform_populate() to create platform device for connector,
this is not friendly when the controller driver hasn't child node
before, and also not compatible with other cases, such as Type-C

2. It doesn't keep format consistent with the case using Type-C, or
using PMIC to detect ID pin.

3. vbus-supply will be got by USB controller driver if it aready
supports it


another way as following:

role_sw_iddig {
    compatible = "linux,typeb-switch-gpio";
    status = "okay";

    connector {
        compatible = "usb-b-connector";
        label = "micro-USB";
        type = "micro";
        id-gpios = <&pio 12 GPIO_ACTIVE_HIGH>;
        vbus-supply = <&usb_p0_vbus>;
    };

    port {
        bsw_ep: endpoint@0 {
            remote-endpoint = <&usb_role_sw>;
        };
    };
};

&mtu3 {
    status = "okay";

    port {
        usb_role_sw: endpoint@0 {
            remote-endpoint = <&bsw_ep>;
        };
    };
};

strength:
avoid the shortcomings of the first way
shortcoming:
it's a virtual device and seems not following graph.txt binding,

Do you have any suggestion?

Thanks


On Fri, 2019-03-15 at 15:38 +0800, Chunfeng Yun wrote:
> It's used to support dual role switch via GPIO when use Type-B
> receptacle
> 
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
> v2 changes:
>   1. new patch to make binding clear suggested by Hans
> ---
>  .../bindings/usb/typeb-switch-gpio.txt        | 26 +++++++++++++++++++
>  1 file changed, 26 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/typeb-switch-gpio.txt
> 
> diff --git a/Documentation/devicetree/bindings/usb/typeb-switch-gpio.txt b/Documentation/devicetree/bindings/usb/typeb-switch-gpio.txt
> new file mode 100644
> index 000000000000..d46d83bc6500
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/typeb-switch-gpio.txt
> @@ -0,0 +1,26 @@
> +USB Type-B dual role switch via a GPIO
> +
> +This is a virtual device used to switch dual role mode from the USB ID pin
> +connected to a GPIO pin.
> +
> +Required properties:
> +- compatible: Should be "linux,typeb-switch-gpio"
> +
> +Either one of id-gpios or vbus-gpios must be present, and both can be present
> +as well.
> +vbus-supply could be present if needed when support dual role mode.
> +	see connector/usb-connector.txt
> +
> +Example:
> +
> +&mtu3 {
> +	status = "okay";
> +
> +	connector@0 {
> +		compatible = "usb-b-connector", "linux,typeb-switch-gpio";
> +		label = "micro-USB";
> +		type = "micro";
> +		id-gpio = <&pio 12 GPIO_ACTIVE_HIGH>;
> +		vbus-supply = <&usb_p0_vbus>;
> +	};
> +};

[PATCH v2 3/6] dt-bindings: usb: mtu3: add properties about USB Role Switch

[Chunfeng Yun]

Now the USB Role Switch is supported, so add properties about it

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
v2 changes:
  1. fix typo
  2. refer new binding about connector property
---
 .../devicetree/bindings/usb/mediatek,mtu3.txt          | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtu3.txt b/Documentation/devicetree/bindings/usb/mediatek,mtu3.txt
index 3382b5cb471d..359e8d565b54 100644
--- a/Documentation/devicetree/bindings/usb/mediatek,mtu3.txt
+++ b/Documentation/devicetree/bindings/usb/mediatek,mtu3.txt
@@ -27,7 +27,9 @@ Optional properties:
  - ranges : allows valid 1:1 translation between child's address space and
 	parent's address space
  - extcon : external connector for vbus and idpin changes detection, needed
-	when supports dual-role mode.
+	when supports dual-role mode; it's consiedered valid for compatibility
+	reasons, and not allowed for new bindings, use the property
+	usb-role-switch instead.
  - vbus-supply : reference to the VBUS regulator, needed when supports
 	dual-role mode.
  - pinctrl-names : a pinctrl state named "default" is optional, and need be
@@ -36,7 +38,8 @@ Optional properties:
 	is not set.
  - pinctrl-0 : pin control group
 	See: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
-
+ - usb-role-switch : use USB Role Switch to support dual-role switch, but
+	not extcon
  - maximum-speed : valid arguments are "super-speed", "high-speed" and
 	"full-speed"; refer to usb/generic.txt
  - enable-manual-drd : supports manual dual-role switch via debugfs; usually
@@ -61,6 +64,9 @@ The xhci should be added as subnode to mtu3 as shown in the following example
 if host mode is enabled. The DT binding details of xhci can be found in:
 Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
 
+The connector would be added as subnode if receptacle is Micro.
+	see usb/typeb-switch-gpio.txt
+
 Example:
 ssusb: usb@11271000 {
 	compatible = "mediatek,mt8173-mtu3";
-- 
2.20.1

[PATCH v2 4/6] usb: roles: add API to get usb_role_switch by node

[Chunfeng Yun]

Add usb_role_switch_get_by_node() to make easier to get
usb_role_switch by node which register it.
It's useful when there is not device_connection registered
between two drivers and only knows the node which register
usb_role_switch.

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/roles/class.c | 30 ++++++++++++++++++++++++++++++
 include/linux/usb/role.h  |  1 +
 2 files changed, 31 insertions(+)

diff --git a/drivers/usb/roles/class.c b/drivers/usb/roles/class.c
index 99116af07f1d..284b19856dc4 100644
--- a/drivers/usb/roles/class.c
+++ b/drivers/usb/roles/class.c
@@ -11,6 +11,7 @@
 #include <linux/device.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
+#include <linux/of.h>
 #include <linux/slab.h>
 
 static struct class *role_class;
@@ -121,6 +122,35 @@ struct usb_role_switch *usb_role_switch_get(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(usb_role_switch_get);
 
+static int __switch_match_node(struct device *dev, const void *node)
+{
+	return dev->parent->of_node == (const struct device_node *)node;
+}
+
+/**
+ * usb_role_switch_get_by_node - Find USB role switch by it's parent node
+ * @node: The node that register USB role switch
+ *
+ * Finds and returns role switch registered by @node. The reference count
+ * for the found switch is incremented.
+ */
+struct usb_role_switch *usb_role_switch_get_by_node(struct device_node *node)
+{
+	struct usb_role_switch *sw;
+	struct device *dev;
+
+	dev = class_find_device(role_class, NULL, node,
+				__switch_match_node);
+	if (!dev)
+		return ERR_PTR(-EPROBE_DEFER);
+
+	sw = to_role_switch(dev);
+	WARN_ON(!try_module_get(sw->dev.parent->driver->owner));
+
+	return sw;
+}
+EXPORT_SYMBOL_GPL(usb_role_switch_get_by_node);
+
 /**
  * usb_role_switch_put - Release handle to a switch
  * @sw: USB Role Switch
diff --git a/include/linux/usb/role.h b/include/linux/usb/role.h
index edc51be4a77c..056498b83dee 100644
--- a/include/linux/usb/role.h
+++ b/include/linux/usb/role.h
@@ -42,6 +42,7 @@ struct usb_role_switch_desc {
 
 int usb_role_switch_set_role(struct usb_role_switch *sw, enum usb_role role);
 enum usb_role usb_role_switch_get_role(struct usb_role_switch *sw);
+struct usb_role_switch *usb_role_switch_get_by_node(struct device_node *node);
 struct usb_role_switch *usb_role_switch_get(struct device *dev);
 void usb_role_switch_put(struct usb_role_switch *sw);
 
-- 
2.20.1

Re: [PATCH v2 4/6] usb: roles: add API to get usb_role_switch by node

[Heikki Krogerus]

On Fri, Mar 15, 2019 at 03:38:31PM +0800, Chunfeng Yun wrote:
> Add usb_role_switch_get_by_node() to make easier to get
> usb_role_switch by node which register it.
> It's useful when there is not device_connection registered
> between two drivers and only knows the node which register
> usb_role_switch.
> 
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
>  drivers/usb/roles/class.c | 30 ++++++++++++++++++++++++++++++
>  include/linux/usb/role.h  |  1 +
>  2 files changed, 31 insertions(+)
> 
> diff --git a/drivers/usb/roles/class.c b/drivers/usb/roles/class.c
> index 99116af07f1d..284b19856dc4 100644
> --- a/drivers/usb/roles/class.c
> +++ b/drivers/usb/roles/class.c
> @@ -11,6 +11,7 @@
>  #include <linux/device.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
> +#include <linux/of.h>
>  #include <linux/slab.h>
>  
>  static struct class *role_class;
> @@ -121,6 +122,35 @@ struct usb_role_switch *usb_role_switch_get(struct device *dev)
>  }
>  EXPORT_SYMBOL_GPL(usb_role_switch_get);
>  
> +static int __switch_match_node(struct device *dev, const void *node)
> +{
> +	return dev->parent->of_node == (const struct device_node *)node;
> +}

Andy already pointed out that you need to use fwnodes.

Rule of thumb: You always use fwnodes. Only if there is something that
can't be done with fwnodes you use DT or ACPI nodes directly.

In this case there is absolutely nothing that would prevent you from
using fwnodes.


thanks,

-- 
heikki

Re: [PATCH v2 4/6] usb: roles: add API to get usb_role_switch by node

[Heikki Krogerus]

On Fri, Mar 15, 2019 at 10:18:35AM +0200, Heikki Krogerus wrote:
> Andy already pointed out that you need to use fwnodes.
> 
> Rule of thumb: You always use fwnodes. Only if there is something that
> can't be done with fwnodes you use DT or ACPI nodes directly.
> 
> In this case there is absolutely nothing that would prevent you from
> using fwnodes.

...

> +/**
> + * usb_role_switch_get_by_node - Find USB role switch by it's parent node
> + * @node: The node that register USB role switch
> + *
> + * Finds and returns role switch registered by @node. The reference count
> + * for the found switch is incremented.
> + */
> +struct usb_role_switch *usb_role_switch_get_by_node(struct device_node *node)

This is my proposal for function prototype:

struct usb_role_switch *
fwnode_usb_role_switch_get(struct fwnode_handle *fwnode);


thanks,

-- 
heikki

Re: [PATCH v2 4/6] usb: roles: add API to get usb_role_switch by node

[Chunfeng Yun]

Hi,
On Fri, 2019-03-15 at 11:11 +0200, Heikki Krogerus wrote:
> On Fri, Mar 15, 2019 at 10:18:35AM +0200, Heikki Krogerus wrote:
> > Andy already pointed out that you need to use fwnodes.
> > 
> > Rule of thumb: You always use fwnodes. Only if there is something that
> > can't be done with fwnodes you use DT or ACPI nodes directly.
> > 
> > In this case there is absolutely nothing that would prevent you from
> > using fwnodes.
> 
> ...
> 
> > +/**
> > + * usb_role_switch_get_by_node - Find USB role switch by it's parent node
> > + * @node: The node that register USB role switch
> > + *
> > + * Finds and returns role switch registered by @node. The reference count
> > + * for the found switch is incremented.
> > + */
> > +struct usb_role_switch *usb_role_switch_get_by_node(struct device_node *node)
> 
> This is my proposal for function prototype:
> 
> struct usb_role_switch *
> fwnode_usb_role_switch_get(struct fwnode_handle *fwnode);
Ok, thanks again
> 
> 
> thanks,
> 

Re: [PATCH v2 4/6] usb: roles: add API to get usb_role_switch by node

[Chunfeng Yun]

Hi,
On Fri, 2019-03-15 at 10:18 +0200, Heikki Krogerus wrote:
> On Fri, Mar 15, 2019 at 03:38:31PM +0800, Chunfeng Yun wrote:
> > Add usb_role_switch_get_by_node() to make easier to get
> > usb_role_switch by node which register it.
> > It's useful when there is not device_connection registered
> > between two drivers and only knows the node which register
> > usb_role_switch.
> > 
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > ---
> >  drivers/usb/roles/class.c | 30 ++++++++++++++++++++++++++++++
> >  include/linux/usb/role.h  |  1 +
> >  2 files changed, 31 insertions(+)
> > 
> > diff --git a/drivers/usb/roles/class.c b/drivers/usb/roles/class.c
> > index 99116af07f1d..284b19856dc4 100644
> > --- a/drivers/usb/roles/class.c
> > +++ b/drivers/usb/roles/class.c
> > @@ -11,6 +11,7 @@
> >  #include <linux/device.h>
> >  #include <linux/module.h>
> >  #include <linux/mutex.h>
> > +#include <linux/of.h>
> >  #include <linux/slab.h>
> >  
> >  static struct class *role_class;
> > @@ -121,6 +122,35 @@ struct usb_role_switch *usb_role_switch_get(struct device *dev)
> >  }
> >  EXPORT_SYMBOL_GPL(usb_role_switch_get);
> >  
> > +static int __switch_match_node(struct device *dev, const void *node)
> > +{
> > +	return dev->parent->of_node == (const struct device_node *)node;
> > +}
> 
> Andy already pointed out that you need to use fwnodes.
> 
> Rule of thumb: You always use fwnodes. Only if there is something that
> can't be done with fwnodes you use DT or ACPI nodes directly.
> 
> In this case there is absolutely nothing that would prevent you from
> using fwnodes.
> 
Got it, will fix it in next version.

BTW:

I encounter a build error when CONFIG_USB_ROLE_SWITCH is not enabled,

drivers/usb/mtu3/mtu3_dr.o: In function `ssusb_role_sw_register':
./drivers/usb/mtu3/mtu3_dr.c:460: undefined reference to
`usb_role_switch_register'
drivers/usb/mtu3/mtu3_dr.o: In function `ssusb_otg_switch_exit':
./drivers/usb/mtu3/mtu3_dr.c:491: undefined reference to
`usb_role_switch_unregister'

the following patch has fixed the issue, but seems not get into kernel,
[v3,08/12] usb: roles: Add usb role switch notifier.
https://patchwork.kernel.org/patch/10836525/

What should I do if I add a new API? Thanks

> 
> thanks,
> 

Re: [PATCH v2 4/6] usb: roles: add API to get usb_role_switch by node

[Heikki Krogerus]

Hi Chunfeng,

On Fri, Mar 15, 2019 at 05:13:24PM +0800, Chunfeng Yun wrote:
> I encounter a build error when CONFIG_USB_ROLE_SWITCH is not enabled,
> 
> drivers/usb/mtu3/mtu3_dr.o: In function `ssusb_role_sw_register':
> ./drivers/usb/mtu3/mtu3_dr.c:460: undefined reference to
> `usb_role_switch_register'
> drivers/usb/mtu3/mtu3_dr.o: In function `ssusb_otg_switch_exit':
> ./drivers/usb/mtu3/mtu3_dr.c:491: undefined reference to
> `usb_role_switch_unregister'

So you need to add dependency on USB_ROLE_SWITCH, right?

--- a/drivers/usb/mtu3/Kconfig
+++ b/drivers/usb/mtu3/Kconfig
@@ -43,6 +43,7 @@ config USB_MTU3_DUAL_ROLE
        bool "Dual Role mode"
        depends on ((USB=y || USB=USB_MTU3) && (USB_GADGET=y || USB_GADGET=USB_MTU3))
        depends on (EXTCON=y || EXTCON=USB_MTU3)
+       depends on USB_ROLE_SWITCH
        help
          This is the default mode of working of MTU3 controller where
          both host and gadget features are enabled.

> the following patch has fixed the issue, but seems not get into kernel,
> [v3,08/12] usb: roles: Add usb role switch notifier.
> https://patchwork.kernel.org/patch/10836525/

I don't understand how that fixes the problem? That patch will in any
case be targeting v5.2. We are in the middle of merge window, so
nothing is happening until v5.1-rc1 is tagged.


thanks,

-- 
heikki

Re: [PATCH v2 4/6] usb: roles: add API to get usb_role_switch by node

[Chunfeng Yun]

Hi,
On Fri, 2019-03-15 at 11:26 +0200, Heikki Krogerus wrote:
> Hi Chunfeng,
> 
> On Fri, Mar 15, 2019 at 05:13:24PM +0800, Chunfeng Yun wrote:
> > I encounter a build error when CONFIG_USB_ROLE_SWITCH is not enabled,
> > 
> > drivers/usb/mtu3/mtu3_dr.o: In function `ssusb_role_sw_register':
> > ./drivers/usb/mtu3/mtu3_dr.c:460: undefined reference to
> > `usb_role_switch_register'
> > drivers/usb/mtu3/mtu3_dr.o: In function `ssusb_otg_switch_exit':
> > ./drivers/usb/mtu3/mtu3_dr.c:491: undefined reference to
> > `usb_role_switch_unregister'
> 
> So you need to add dependency on USB_ROLE_SWITCH, right?
Yes

> 
> --- a/drivers/usb/mtu3/Kconfig
> +++ b/drivers/usb/mtu3/Kconfig
> @@ -43,6 +43,7 @@ config USB_MTU3_DUAL_ROLE
>         bool "Dual Role mode"
>         depends on ((USB=y || USB=USB_MTU3) && (USB_GADGET=y || USB_GADGET=USB_MTU3))
>         depends on (EXTCON=y || EXTCON=USB_MTU3)
> +       depends on USB_ROLE_SWITCH
>         help
>           This is the default mode of working of MTU3 controller where
>           both host and gadget features are enabled.
> 
> > the following patch has fixed the issue, but seems not get into kernel,
> > [v3,08/12] usb: roles: Add usb role switch notifier.
> > https://patchwork.kernel.org/patch/10836525/
> 
> I don't understand how that fixes the problem? That patch will in any
> case be targeting v5.2. We are in the middle of merge window, so
> nothing is happening until v5.1-rc1 is tagged.
It provides some dummy inline functions when USB_ROLE_SWITCH is not
enabled, this will avoid build error

> 
> 
> thanks,
> 

Re: [PATCH v2 4/6] usb: roles: add API to get usb_role_switch by node

[Heikki Krogerus]

On Fri, Mar 15, 2019 at 05:32:59PM +0800, Chunfeng Yun wrote:
> Hi,
> On Fri, 2019-03-15 at 11:26 +0200, Heikki Krogerus wrote:
> > Hi Chunfeng,
> > 
> > On Fri, Mar 15, 2019 at 05:13:24PM +0800, Chunfeng Yun wrote:
> > > I encounter a build error when CONFIG_USB_ROLE_SWITCH is not enabled,
> > > 
> > > drivers/usb/mtu3/mtu3_dr.o: In function `ssusb_role_sw_register':
> > > ./drivers/usb/mtu3/mtu3_dr.c:460: undefined reference to
> > > `usb_role_switch_register'
> > > drivers/usb/mtu3/mtu3_dr.o: In function `ssusb_otg_switch_exit':
> > > ./drivers/usb/mtu3/mtu3_dr.c:491: undefined reference to
> > > `usb_role_switch_unregister'
> > 
> > So you need to add dependency on USB_ROLE_SWITCH, right?
> Yes
> 
> > 
> > --- a/drivers/usb/mtu3/Kconfig
> > +++ b/drivers/usb/mtu3/Kconfig
> > @@ -43,6 +43,7 @@ config USB_MTU3_DUAL_ROLE
> >         bool "Dual Role mode"
> >         depends on ((USB=y || USB=USB_MTU3) && (USB_GADGET=y || USB_GADGET=USB_MTU3))
> >         depends on (EXTCON=y || EXTCON=USB_MTU3)
> > +       depends on USB_ROLE_SWITCH
> >         help
> >           This is the default mode of working of MTU3 controller where
> >           both host and gadget features are enabled.
> > 
> > > the following patch has fixed the issue, but seems not get into kernel,
> > > [v3,08/12] usb: roles: Add usb role switch notifier.
> > > https://patchwork.kernel.org/patch/10836525/
> > 
> > I don't understand how that fixes the problem? That patch will in any
> > case be targeting v5.2. We are in the middle of merge window, so
> > nothing is happening until v5.1-rc1 is tagged.
> It provides some dummy inline functions when USB_ROLE_SWITCH is not
> enabled, this will avoid build error

Ah, true. Those should brobable be introduced in their own patch.


thanks,

-- 
heikki

Re: [PATCH v2 4/6] usb: roles: add API to get usb_role_switch by node

[Heikki Krogerus]

On Fri, Mar 15, 2019 at 05:13:24PM +0800, Chunfeng Yun wrote:
> I encounter a build error when CONFIG_USB_ROLE_SWITCH is not enabled,
> 
> drivers/usb/mtu3/mtu3_dr.o: In function `ssusb_role_sw_register':
> ./drivers/usb/mtu3/mtu3_dr.c:460: undefined reference to
> `usb_role_switch_register'
> drivers/usb/mtu3/mtu3_dr.o: In function `ssusb_otg_switch_exit':
> ./drivers/usb/mtu3/mtu3_dr.c:491: undefined reference to
> `usb_role_switch_unregister'
> 
> the following patch has fixed the issue, but seems not get into kernel,
> [v3,08/12] usb: roles: Add usb role switch notifier.
> https://patchwork.kernel.org/patch/10836525/
> 
> What should I do if I add a new API? Thanks

So if you are asking should you supply dummy functions for the new API,
then I would just say that if you do so, you need to prepare these
patches on top of that series from Yu Chen. In general I'm not sure we
need dummy functions with this API.

Hans, comments?


thanks,

-- 
heikki

[PATCH v2 5/6] usb: roles: add driver of USB Type-B role switch by GPIO

[Chunfeng Yun]

Due to the requirement of usb-connector.txt binding, the old way
using extcon to support USB Dual-Role switch is now deprecated
when use Type-B connector.
This patch introduce a Type-B connector driver and try to replace
the function provided by extcon-usb-gpio driver

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
v2 changes:
  1. file name is changed
  2. use new compatible
---
 drivers/usb/roles/Kconfig             |  14 ++
 drivers/usb/roles/Makefile            |   1 +
 drivers/usb/roles/typeb-switch-gpio.c | 285 ++++++++++++++++++++++++++
 3 files changed, 300 insertions(+)
 create mode 100644 drivers/usb/roles/typeb-switch-gpio.c

diff --git a/drivers/usb/roles/Kconfig b/drivers/usb/roles/Kconfig
index e4194ac94510..3c4320a73d17 100644
--- a/drivers/usb/roles/Kconfig
+++ b/drivers/usb/roles/Kconfig
@@ -24,4 +24,18 @@ config USB_ROLES_INTEL_XHCI
 	  To compile the driver as a module, choose M here: the module will
 	  be called intel-xhci-usb-role-switch.
 
+config TYPEB_SWITCH_GPIO
+	tristate "USB Type-B Role Switch Via GPIO"
+	depends on GPIOLIB
+	help
+	  The driver supports USB role switch between host and device via GPIO
+	  based USB cable detection, used typically if GPIO is used for USB ID
+	  pin detection. And it's used to replace the function provided by
+	  drivers/extcon/extcon-usb-gpio.c driver due to the requirement by
+	  connector/usb-connector.txt
+
+	  Say Y here if your USB Type-B Connector supports Dual-Role mode.
+	  To compile the driver as a module, choose M here: the module will
+	  be called usb-b-connector.ko
+
 endif # USB_ROLE_SWITCH
diff --git a/drivers/usb/roles/Makefile b/drivers/usb/roles/Makefile
index c02873206fc1..00f697bd419f 100644
--- a/drivers/usb/roles/Makefile
+++ b/drivers/usb/roles/Makefile
@@ -1,3 +1,4 @@
 obj-$(CONFIG_USB_ROLE_SWITCH)		+= roles.o
 roles-y					:= class.o
+obj-$(CONFIG_TYPEB_SWITCH_GPIO)		+= typeb-switch-gpio.o
 obj-$(CONFIG_USB_ROLES_INTEL_XHCI)	+= intel-xhci-usb-role-switch.o
diff --git a/drivers/usb/roles/typeb-switch-gpio.c b/drivers/usb/roles/typeb-switch-gpio.c
new file mode 100644
index 000000000000..d8a15269bf8f
--- /dev/null
+++ b/drivers/usb/roles/typeb-switch-gpio.c
@@ -0,0 +1,285 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * USB Type-B Connector Driver
+ *
+ * Copyright (C) 2019 MediaTek Inc.
+ *
+ * Author: Chunfeng Yun <chunfeng.yun@mediatek.com>
+ *
+ * Some code borrowed from drivers/extcon/extcon-usb-gpio.c
+ */
+
+#include <linux/device.h>
+#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+#include <linux/usb/role.h>
+
+#define USB_GPIO_DEB_MS		20	/* ms */
+#define USB_GPIO_DEB_US		((USB_GPIO_DEB_MS) * 1000)	/* us */
+
+#define USB_CONN_IRQF	\
+	(IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT)
+
+struct usb_conn_info {
+	struct device *dev;
+	struct usb_role_switch *role_sw;
+	enum usb_role last_role;
+	struct regulator *vbus;
+	struct delayed_work dw_det;
+	unsigned long debounce_jiffies;
+
+	struct gpio_desc *id_gpiod;
+	struct gpio_desc *vbus_gpiod;
+	int id_irq;
+	int vbus_irq;
+};
+
+/**
+ * "DEVICE" = VBUS and "HOST" = !ID, so we have:
+ * Both "DEVICE" and "HOST" can't be set as active at the same time
+ * so if "HOST" is active (i.e. ID is 0)  we keep "DEVICE" inactive
+ * even if VBUS is on.
+ *
+ *  Role          |   ID  |  VBUS
+ * ------------------------------------
+ *  [1] DEVICE    |   H   |   H
+ *  [2] NONE      |   H   |   L
+ *  [3] HOST      |   L   |   H
+ *  [4] HOST      |   L   |   L
+ *
+ * In case we have only one of these signals:
+ * - VBUS only - we want to distinguish between [1] and [2], so ID is always 1
+ * - ID only - we want to distinguish between [1] and [4], so VBUS = ID
+ */
+static void usb_conn_detect_cable(struct work_struct *work)
+{
+	struct usb_conn_info *info;
+	enum usb_role role;
+	int id, vbus, ret;
+
+	info = container_of(to_delayed_work(work),
+			    struct usb_conn_info, dw_det);
+
+	/* check ID and VBUS */
+	id = info->id_gpiod ?
+		gpiod_get_value_cansleep(info->id_gpiod) : 1;
+	vbus = info->vbus_gpiod ?
+		gpiod_get_value_cansleep(info->vbus_gpiod) : id;
+
+	if (!id)
+		role = USB_ROLE_HOST;
+	else if (vbus)
+		role = USB_ROLE_DEVICE;
+	else
+		role = USB_ROLE_NONE;
+
+	dev_dbg(info->dev, "role %d/%d, gpios: id %d, vbus %d\n",
+		info->last_role, role, id, vbus);
+
+	if (info->last_role == role) {
+		dev_warn(info->dev, "repeated role: %d\n", role);
+		return;
+	}
+
+	if (info->last_role == USB_ROLE_HOST)
+		regulator_disable(info->vbus);
+
+	ret = usb_role_switch_set_role(info->role_sw, role);
+	if (ret)
+		dev_err(info->dev, "failed to set role: %d\n", ret);
+
+	if (role == USB_ROLE_HOST) {
+		ret = regulator_enable(info->vbus);
+		if (ret)
+			dev_err(info->dev, "enable vbus regulator failed\n");
+	}
+
+	info->last_role = role;
+
+	dev_dbg(info->dev, "vbus regulator is %s\n",
+		regulator_is_enabled(info->vbus) ? "enabled" : "disabled");
+}
+
+static void usb_conn_queue_dwork(struct usb_conn_info *info,
+				 unsigned long delay)
+{
+	queue_delayed_work(system_power_efficient_wq, &info->dw_det, delay);
+}
+
+static irqreturn_t usb_conn_isr(int irq, void *dev_id)
+{
+	struct usb_conn_info *info = dev_id;
+
+	usb_conn_queue_dwork(info, info->debounce_jiffies);
+
+	return IRQ_HANDLED;
+}
+
+static int usb_conn_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct device_node *parent;
+	struct usb_conn_info *info;
+	int ret = 0;
+
+	if (!np)
+		return -EINVAL;
+
+	info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+
+	info->dev = dev;
+	info->id_gpiod = devm_gpiod_get_optional(dev, "id", GPIOD_IN);
+	if (IS_ERR(info->id_gpiod))
+		return PTR_ERR(info->id_gpiod);
+
+	info->vbus_gpiod = devm_gpiod_get_optional(dev, "vbus", GPIOD_IN);
+	if (IS_ERR(info->vbus_gpiod))
+		return PTR_ERR(info->vbus_gpiod);
+
+	if (!info->id_gpiod && !info->vbus_gpiod) {
+		dev_err(dev, "failed to get gpios\n");
+		return -ENODEV;
+	}
+
+	if (info->id_gpiod)
+		ret = gpiod_set_debounce(info->id_gpiod, USB_GPIO_DEB_US);
+	if (!ret && info->vbus_gpiod)
+		ret = gpiod_set_debounce(info->vbus_gpiod, USB_GPIO_DEB_US);
+	if (ret < 0)
+		info->debounce_jiffies = msecs_to_jiffies(USB_GPIO_DEB_MS);
+
+	INIT_DELAYED_WORK(&info->dw_det, usb_conn_detect_cable);
+
+	info->vbus = devm_regulator_get(dev, "vbus");
+	if (IS_ERR(info->vbus)) {
+		dev_err(dev, "failed to get vbus\n");
+		return PTR_ERR(info->vbus);
+	}
+
+	parent = of_get_parent(np);
+	info->role_sw = usb_role_switch_get_by_node(parent);
+	if (IS_ERR_OR_NULL(info->role_sw)) {
+		dev_err(dev, "failed to get role switch\n");
+		return PTR_ERR(info->role_sw);
+	}
+
+	of_node_put(parent);
+
+	if (info->id_gpiod) {
+		info->id_irq = gpiod_to_irq(info->id_gpiod);
+		if (info->id_irq < 0) {
+			dev_err(dev, "failed to get ID IRQ\n");
+			return info->id_irq;
+		}
+
+		ret = devm_request_threaded_irq(dev, info->id_irq, NULL,
+						usb_conn_isr, USB_CONN_IRQF,
+						pdev->name, info);
+		if (ret < 0) {
+			dev_err(dev, "failed to request ID IRQ\n");
+			return ret;
+		}
+	}
+
+	if (info->vbus_gpiod) {
+		info->vbus_irq = gpiod_to_irq(info->vbus_gpiod);
+		if (info->vbus_irq < 0) {
+			dev_err(dev, "failed to get VBUS IRQ\n");
+			return info->vbus_irq;
+		}
+
+		ret = devm_request_threaded_irq(dev, info->vbus_irq, NULL,
+						usb_conn_isr, USB_CONN_IRQF,
+						pdev->name, info);
+		if (ret < 0) {
+			dev_err(dev, "failed to request VBUS IRQ\n");
+			return ret;
+		}
+	}
+
+	platform_set_drvdata(pdev, info);
+
+	/* Perform initial detection */
+	usb_conn_queue_dwork(info, 0);
+
+	return 0;
+}
+
+static int usb_conn_remove(struct platform_device *pdev)
+{
+	struct usb_conn_info *info = platform_get_drvdata(pdev);
+
+	cancel_delayed_work_sync(&info->dw_det);
+
+	if (info->last_role == USB_ROLE_HOST)
+		regulator_disable(info->vbus);
+
+	usb_role_switch_put(info->role_sw);
+
+	return 0;
+}
+
+static int __maybe_unused usb_conn_suspend(struct device *dev)
+{
+	struct usb_conn_info *info = dev_get_drvdata(dev);
+
+	if (info->id_gpiod)
+		disable_irq(info->id_irq);
+	if (info->vbus_gpiod)
+		disable_irq(info->vbus_irq);
+
+	pinctrl_pm_select_sleep_state(dev);
+
+	return 0;
+}
+
+static int __maybe_unused usb_conn_resume(struct device *dev)
+{
+	struct usb_conn_info *info = dev_get_drvdata(dev);
+
+	pinctrl_pm_select_default_state(dev);
+
+	if (info->id_gpiod)
+		enable_irq(info->id_irq);
+	if (info->vbus_gpiod)
+		enable_irq(info->vbus_irq);
+
+	usb_conn_queue_dwork(info, 0);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(usb_conn_pm_ops,
+			 usb_conn_suspend, usb_conn_resume);
+
+#define DEV_PMS_OPS (IS_ENABLED(CONFIG_PM_SLEEP) ? &usb_conn_pm_ops : NULL)
+
+static const struct of_device_id usb_conn_dt_match[] = {
+	{ .compatible = "linux,typeb-switch-gpio", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, usb_conn_dt_match);
+
+static struct platform_driver usb_conn_driver = {
+	.probe		= usb_conn_probe,
+	.remove		= usb_conn_remove,
+	.driver		= {
+		.name	= "usb_b_conn",
+		.pm	= DEV_PMS_OPS,
+		.of_match_table = usb_conn_dt_match,
+	},
+};
+
+module_platform_driver(usb_conn_driver);
+
+MODULE_AUTHOR("Chunfeng Yun <chunfeng.yun@mediatek.com>");
+MODULE_DESCRIPTION("USB GPIO conn driver");
+MODULE_LICENSE("GPL v2");
-- 
2.20.1

[PATCH v2 6/6] usb: mtu3: register a USB Role Switch for dual role mode

[Chunfeng Yun]

Due to extcon is not allowed for new bindings, and the
dual role switch is supported by USB Role Switch,
especially for Type-C drivers, so register a USB Role
Switch to support the new way

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/mtu3/mtu3.h      |  5 ++++
 drivers/usb/mtu3/mtu3_dr.c   | 50 +++++++++++++++++++++++++++++++++---
 drivers/usb/mtu3/mtu3_plat.c |  3 ++-
 3 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/mtu3/mtu3.h b/drivers/usb/mtu3/mtu3.h
index 87823ac0d120..8f9da03255b6 100644
--- a/drivers/usb/mtu3/mtu3.h
+++ b/drivers/usb/mtu3/mtu3.h
@@ -202,6 +202,9 @@ struct mtu3_gpd_ring {
 * @id_nb : notifier for iddig(idpin) detection
 * @id_work : work of iddig detection notifier
 * @id_event : event of iddig detecion notifier
+* @role_sw : use USB Role Switch to support dual-role switch, can't use
+*		extcon at the same time, and extcon is deprecated.
+* @role_sw_used : true when the USB Role Switch is used.
 * @is_u3_drd: whether port0 supports usb3.0 dual-role device or not
 * @manual_drd_enabled: it's true when supports dual-role device by debugfs
 *		to switch host/device modes depending on user input.
@@ -215,6 +218,8 @@ struct otg_switch_mtk {
 	struct notifier_block id_nb;
 	struct work_struct id_work;
 	unsigned long id_event;
+	struct usb_role_switch *role_sw;
+	bool role_sw_used;
 	bool is_u3_drd;
 	bool manual_drd_enabled;
 };
diff --git a/drivers/usb/mtu3/mtu3_dr.c b/drivers/usb/mtu3/mtu3_dr.c
index ac60e9c8564e..b9272295fe2b 100644
--- a/drivers/usb/mtu3/mtu3_dr.c
+++ b/drivers/usb/mtu3/mtu3_dr.c
@@ -14,6 +14,7 @@
 #include <linux/pinctrl/consumer.h>
 #include <linux/seq_file.h>
 #include <linux/uaccess.h>
+#include <linux/usb/role.h>
 
 #include "mtu3.h"
 #include "mtu3_dr.h"
@@ -266,7 +267,7 @@ static int ssusb_extcon_register(struct otg_switch_mtk *otg_sx)
  * This is useful in special cases, such as uses TYPE-A receptacle but also
  * wants to support dual-role mode.
  */
-static void ssusb_mode_manual_switch(struct ssusb_mtk *ssusb, int to_host)
+static void ssusb_mode_switch(struct ssusb_mtk *ssusb, int to_host)
 {
 	struct otg_switch_mtk *otg_sx = &ssusb->otg_switch;
 
@@ -308,9 +309,9 @@ static ssize_t ssusb_mode_write(struct file *file,
 		return -EFAULT;
 
 	if (!strncmp(buf, "host", 4) && !ssusb->is_host) {
-		ssusb_mode_manual_switch(ssusb, 1);
+		ssusb_mode_switch(ssusb, 1);
 	} else if (!strncmp(buf, "device", 6) && ssusb->is_host) {
-		ssusb_mode_manual_switch(ssusb, 0);
+		ssusb_mode_switch(ssusb, 0);
 	} else {
 		dev_err(ssusb->dev, "wrong or duplicated setting\n");
 		return -EINVAL;
@@ -412,6 +413,46 @@ void ssusb_set_force_mode(struct ssusb_mtk *ssusb,
 	mtu3_writel(ssusb->ippc_base, SSUSB_U2_CTRL(0), value);
 }
 
+static int ssusb_role_sw_set(struct device *dev, enum usb_role role)
+{
+	struct ssusb_mtk *ssusb = dev_get_drvdata(dev);
+	bool to_host = false;
+
+	if (role == USB_ROLE_HOST)
+		to_host = true;
+
+	if (to_host ^ ssusb->is_host)
+		ssusb_mode_switch(ssusb, to_host);
+
+	return 0;
+}
+
+static enum usb_role ssusb_role_sw_get(struct device *dev)
+{
+	struct ssusb_mtk *ssusb = dev_get_drvdata(dev);
+	enum usb_role role;
+
+	role = ssusb->is_host ? USB_ROLE_HOST : USB_ROLE_DEVICE;
+
+	return role;
+}
+
+static int ssusb_role_sw_register(struct otg_switch_mtk *otg_sx)
+{
+	struct usb_role_switch_desc role_sx_desc = { 0 };
+	struct ssusb_mtk *ssusb =
+		container_of(otg_sx, struct ssusb_mtk, otg_switch);
+
+	if (!otg_sx->role_sw_used)
+		return 0;
+
+	role_sx_desc.set = ssusb_role_sw_set;
+	role_sx_desc.get = ssusb_role_sw_get;
+	otg_sx->role_sw = usb_role_switch_register(ssusb->dev, &role_sx_desc);
+
+	return PTR_ERR_OR_ZERO(otg_sx->role_sw);
+}
+
 int ssusb_otg_switch_init(struct ssusb_mtk *ssusb)
 {
 	struct otg_switch_mtk *otg_sx = &ssusb->otg_switch;
@@ -421,6 +462,8 @@ int ssusb_otg_switch_init(struct ssusb_mtk *ssusb)
 
 	if (otg_sx->manual_drd_enabled)
 		ssusb_debugfs_init(ssusb);
+	else if (otg_sx->role_sw_used)
+		ssusb_role_sw_register(otg_sx);
 	else
 		ssusb_extcon_register(otg_sx);
 
@@ -436,4 +479,5 @@ void ssusb_otg_switch_exit(struct ssusb_mtk *ssusb)
 
 	cancel_work_sync(&otg_sx->id_work);
 	cancel_work_sync(&otg_sx->vbus_work);
+	usb_role_switch_unregister(otg_sx->role_sw);
 }
diff --git a/drivers/usb/mtu3/mtu3_plat.c b/drivers/usb/mtu3/mtu3_plat.c
index e086630e41a9..02c288a85989 100644
--- a/drivers/usb/mtu3/mtu3_plat.c
+++ b/drivers/usb/mtu3/mtu3_plat.c
@@ -313,8 +313,9 @@ static int get_ssusb_rscs(struct platform_device *pdev, struct ssusb_mtk *ssusb)
 	otg_sx->is_u3_drd = of_property_read_bool(node, "mediatek,usb3-drd");
 	otg_sx->manual_drd_enabled =
 		of_property_read_bool(node, "enable-manual-drd");
+	otg_sx->role_sw_used = of_property_read_bool(node, "usb-role-switch");
 
-	if (of_property_read_bool(node, "extcon")) {
+	if (!otg_sx->role_sw_used && of_property_read_bool(node, "extcon")) {
 		otg_sx->edev = extcon_get_edev_by_phandle(ssusb->dev, 0);
 		if (IS_ERR(otg_sx->edev)) {
 			dev_err(ssusb->dev, "couldn't get extcon device\n");
-- 
2.20.1