diff mbox series

[v2,4/7] usb: gadget: udc: renesas_usb3: Use usb_role_switch framework

Message ID 1552552775-51667-5-git-send-email-biju.das@bp.renesas.com (mailing list archive)
State Superseded
Delegated to: Geert Uytterhoeven
Headers show
Series Add USB3.0 and TI HD3SS3220 driver support | expand

Commit Message

Biju Das March 14, 2019, 8:39 a.m. UTC
RZ/G2E cat874 board is capable of detecting cable connect and disconnect
events. Add support for renesas_usb3 to receive connect and disconnect
events and support dual-role switch using usb-role-switch framework.

Signed-off-by: Biju Das <biju.das@bp.renesas.com>
---
 V1-->V2
   * Driver uses usb role clas for handling dual role switch and handling
     connect/disconnect events instead of extcon.
---
 drivers/usb/gadget/udc/renesas_usb3.c | 125 ++++++++++++++++++++++++++++------
 1 file changed, 103 insertions(+), 22 deletions(-)

Comments

Yoshihiro Shimoda March 14, 2019, 9:21 a.m. UTC | #1
Hi Biju-san,

> From: Biju Das, Sent: Thursday, March 14, 2019 5:40 PM
> 
> RZ/G2E cat874 board is capable of detecting cable connect and disconnect
> events. Add support for renesas_usb3 to receive connect and disconnect
> events and support dual-role switch using usb-role-switch framework.
> 
> Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> ---
>  V1-->V2
>    * Driver uses usb role clas for handling dual role switch and handling
>      connect/disconnect events instead of extcon.
> ---

Thank you for the patch!

>  drivers/usb/gadget/udc/renesas_usb3.c | 125 ++++++++++++++++++++++++++++------
>  1 file changed, 103 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/usb/gadget/udc/renesas_usb3.c b/drivers/usb/gadget/udc/renesas_usb3.c
> index 7dc2485..359a92b 100644
> --- a/drivers/usb/gadget/udc/renesas_usb3.c
> +++ b/drivers/usb/gadget/udc/renesas_usb3.c
> @@ -351,6 +351,10 @@ struct renesas_usb3 {
>  	int disabled_count;
> 
>  	struct usb_request *ep0_req;
> +
> +	struct usb_role_switch *typec_role_sw;

This typec_role_sw is not used.

> +	enum usb_role connection_state;
> +
>  	u16 test_mode;
>  	u8 ep0_buf[USB3_EP0_BUF_SIZE];
>  	bool softconnect;
> @@ -359,6 +363,7 @@ struct renesas_usb3 {
>  	bool extcon_usb;		/* check vbus and set EXTCON_USB */
>  	bool forced_b_device;
>  	bool start_to_connect;
> +	bool usb_role_switch_property;
>  };
> 
>  #define gadget_to_renesas_usb3(_gadget)	\
> @@ -644,22 +649,6 @@ static void usb3_disconnect(struct renesas_usb3 *usb3)
>  		usb3->driver->disconnect(&usb3->gadget);
>  }
> 
> -static void usb3_check_vbus(struct renesas_usb3 *usb3)
> -{
> -	if (usb3->workaround_for_vbus) {
> -		usb3_connect(usb3);
> -	} else {
> -		usb3->extcon_usb = !!(usb3_read(usb3, USB3_USB_STA) &
> -							USB_STA_VBUS_STA);
> -		if (usb3->extcon_usb)
> -			usb3_connect(usb3);
> -		else
> -			usb3_disconnect(usb3);
> -
> -		schedule_work(&usb3->extcon_work);
> -	}
> -}
> -
>  static void renesas_usb3_role_work(struct work_struct *work)
>  {
>  	struct renesas_usb3 *usb3 =
> @@ -699,8 +688,11 @@ static void usb3_mode_config(struct renesas_usb3 *usb3, bool host, bool a_dev)
>  	unsigned long flags;
> 
>  	spin_lock_irqsave(&usb3->lock, flags);
> -	usb3_set_mode_by_role_sw(usb3, host);
> -	usb3_vbus_out(usb3, a_dev);
> +	if ((!usb3->usb_role_switch_property) ||
> +				(usb3->connection_state != USB_ROLE_NONE)) {

We can modify the code as following:
	if (!usb3->usb_role_switch_property ||
	    usb3->connection_state != USB_ROLE_NONE) {

<snip>
> @@ -2650,7 +2726,7 @@ static const unsigned int renesas_usb3_cable[] = {
>  	EXTCON_NONE,
>  };
> 
> -static const struct usb_role_switch_desc renesas_usb3_role_switch_desc = {
> +static struct usb_role_switch_desc renesas_usb3_role_switch_desc = {
>  	.set = renesas_usb3_role_switch_set,
>  	.get = renesas_usb3_role_switch_get,
>  	.allow_userspace_control = true,
> @@ -2741,6 +2817,11 @@ static int renesas_usb3_probe(struct platform_device *pdev)
>  	if (ret < 0)
>  		goto err_dev_create;
> 
> +	if (device_property_read_bool(&pdev->dev, "usb-role-switch")) {
> +		usb3->usb_role_switch_property = true;
> +		renesas_usb3_role_switch_desc.fwnode = dev_fwnode(&pdev->dev);

This causes build error because struct usb_role_switch_desc doesn't have fwnode.

Best regards,
Yoshihiro Shimoda
Biju Das March 14, 2019, 9:28 a.m. UTC | #2
Hi Shimoda-San,

Thanks for the feedback.

> Subject: RE: [PATCH v2 4/7] usb: gadget: udc: renesas_usb3: Use
> usb_role_switch framework
> 
> Hi Biju-san,
> 
> > From: Biju Das, Sent: Thursday, March 14, 2019 5:40 PM
> >
> > RZ/G2E cat874 board is capable of detecting cable connect and
> > disconnect events. Add support for renesas_usb3 to receive connect and
> > disconnect events and support dual-role switch using usb-role-switch
> framework.
> >
> > Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> > ---
> >  V1-->V2
> >    * Driver uses usb role clas for handling dual role switch and handling
> >      connect/disconnect events instead of extcon.
> > ---
> 
> Thank you for the patch!
> 
> >  drivers/usb/gadget/udc/renesas_usb3.c | 125
> > ++++++++++++++++++++++++++++------
> >  1 file changed, 103 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/usb/gadget/udc/renesas_usb3.c
> > b/drivers/usb/gadget/udc/renesas_usb3.c
> > index 7dc2485..359a92b 100644
> > --- a/drivers/usb/gadget/udc/renesas_usb3.c
> > +++ b/drivers/usb/gadget/udc/renesas_usb3.c
> > @@ -351,6 +351,10 @@ struct renesas_usb3 {
> >  	int disabled_count;
> >
> >  	struct usb_request *ep0_req;
> > +
> > +	struct usb_role_switch *typec_role_sw;
> 
> This typec_role_sw is not used.

 OK. Will remove this.
> 
> > +	enum usb_role connection_state;
> > +
> >  	u16 test_mode;
> >  	u8 ep0_buf[USB3_EP0_BUF_SIZE];
> >  	bool softconnect;
> > @@ -359,6 +363,7 @@ struct renesas_usb3 {
> >  	bool extcon_usb;		/* check vbus and set EXTCON_USB
> */
> >  	bool forced_b_device;
> >  	bool start_to_connect;
> > +	bool usb_role_switch_property;
> >  };
> >
> >  #define gadget_to_renesas_usb3(_gadget)	\
> > @@ -644,22 +649,6 @@ static void usb3_disconnect(struct renesas_usb3
> *usb3)
> >  		usb3->driver->disconnect(&usb3->gadget);
> >  }
> >
> > -static void usb3_check_vbus(struct renesas_usb3 *usb3) -{
> > -	if (usb3->workaround_for_vbus) {
> > -		usb3_connect(usb3);
> > -	} else {
> > -		usb3->extcon_usb = !!(usb3_read(usb3, USB3_USB_STA) &
> > -
> 	USB_STA_VBUS_STA);
> > -		if (usb3->extcon_usb)
> > -			usb3_connect(usb3);
> > -		else
> > -			usb3_disconnect(usb3);
> > -
> > -		schedule_work(&usb3->extcon_work);
> > -	}
> > -}
> > -
> >  static void renesas_usb3_role_work(struct work_struct *work)  {
> >  	struct renesas_usb3 *usb3 =
> > @@ -699,8 +688,11 @@ static void usb3_mode_config(struct renesas_usb3
> *usb3, bool host, bool a_dev)
> >  	unsigned long flags;
> >
> >  	spin_lock_irqsave(&usb3->lock, flags);
> > -	usb3_set_mode_by_role_sw(usb3, host);
> > -	usb3_vbus_out(usb3, a_dev);
> > +	if ((!usb3->usb_role_switch_property) ||
> > +				(usb3->connection_state !=
> USB_ROLE_NONE)) {
> 
> We can modify the code as following:
> 	if (!usb3->usb_role_switch_property ||
> 	    usb3->connection_state != USB_ROLE_NONE) {

OK. Will change this.

> <snip>
> > @@ -2650,7 +2726,7 @@ static const unsigned int renesas_usb3_cable[] = {
> >  	EXTCON_NONE,
> >  };
> >
> > -static const struct usb_role_switch_desc
> > renesas_usb3_role_switch_desc = {
> > +static struct usb_role_switch_desc renesas_usb3_role_switch_desc = {
> >  	.set = renesas_usb3_role_switch_set,
> >  	.get = renesas_usb3_role_switch_get,
> >  	.allow_userspace_control = true,
> > @@ -2741,6 +2817,11 @@ static int renesas_usb3_probe(struct
> platform_device *pdev)
> >  	if (ret < 0)
> >  		goto err_dev_create;
> >
> > +	if (device_property_read_bool(&pdev->dev, "usb-role-switch")) {
> > +		usb3->usb_role_switch_property = true;
> > +		renesas_usb3_role_switch_desc.fwnode =
> dev_fwnode(&pdev->dev);
> 
> This causes build error because struct usb_role_switch_desc doesn't have
> fwnode.

I have compiled this code with linux-next and the commit 
09aa11cfda9d8186046b ("device connection: Add fwnode member to struct device_connection")
defines the property fwnode. The HD3SS3220 port controller driver uses usb-role-switch property to
get role-switch handle using graph api's.

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20190306&id=09aa11cfda9d8186046bcd1adcd6498b688114f4

regards,
Biju
Biju Das March 14, 2019, 9:32 a.m. UTC | #3
Hi Shimoda-San,

Sorry. I pasted a wrong link.

> > This causes build error because struct usb_role_switch_desc doesn't
> > have fwnode.
> 
> I have compiled this code with linux-next and the commit
> 09aa11cfda9d8186046b ("device connection: Add fwnode member to struct
> device_connection") defines the property fwnode. The HD3SS3220 port
> controller driver uses usb-role-switch property to get role-switch handle
> using graph api's.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-
> next.git/commit/?h=next-
> 20190306&id=09aa11cfda9d8186046bcd1adcd6498b688114f4

Commit id ec69e9533c4879c81eb7122771792864eb49af35 ("usb: roles: Find the muxes by also matching against the device node")
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20190306&id=ec69e9533c4879c81eb7122771792864eb49af35

regards,
Biju
diff mbox series

Patch

diff --git a/drivers/usb/gadget/udc/renesas_usb3.c b/drivers/usb/gadget/udc/renesas_usb3.c
index 7dc2485..359a92b 100644
--- a/drivers/usb/gadget/udc/renesas_usb3.c
+++ b/drivers/usb/gadget/udc/renesas_usb3.c
@@ -351,6 +351,10 @@  struct renesas_usb3 {
 	int disabled_count;
 
 	struct usb_request *ep0_req;
+
+	struct usb_role_switch *typec_role_sw;
+	enum usb_role connection_state;
+
 	u16 test_mode;
 	u8 ep0_buf[USB3_EP0_BUF_SIZE];
 	bool softconnect;
@@ -359,6 +363,7 @@  struct renesas_usb3 {
 	bool extcon_usb;		/* check vbus and set EXTCON_USB */
 	bool forced_b_device;
 	bool start_to_connect;
+	bool usb_role_switch_property;
 };
 
 #define gadget_to_renesas_usb3(_gadget)	\
@@ -644,22 +649,6 @@  static void usb3_disconnect(struct renesas_usb3 *usb3)
 		usb3->driver->disconnect(&usb3->gadget);
 }
 
-static void usb3_check_vbus(struct renesas_usb3 *usb3)
-{
-	if (usb3->workaround_for_vbus) {
-		usb3_connect(usb3);
-	} else {
-		usb3->extcon_usb = !!(usb3_read(usb3, USB3_USB_STA) &
-							USB_STA_VBUS_STA);
-		if (usb3->extcon_usb)
-			usb3_connect(usb3);
-		else
-			usb3_disconnect(usb3);
-
-		schedule_work(&usb3->extcon_work);
-	}
-}
-
 static void renesas_usb3_role_work(struct work_struct *work)
 {
 	struct renesas_usb3 *usb3 =
@@ -699,8 +688,11 @@  static void usb3_mode_config(struct renesas_usb3 *usb3, bool host, bool a_dev)
 	unsigned long flags;
 
 	spin_lock_irqsave(&usb3->lock, flags);
-	usb3_set_mode_by_role_sw(usb3, host);
-	usb3_vbus_out(usb3, a_dev);
+	if ((!usb3->usb_role_switch_property) ||
+				(usb3->connection_state != USB_ROLE_NONE)) {
+		usb3_set_mode_by_role_sw(usb3, host);
+		usb3_vbus_out(usb3, a_dev);
+	}
 	/* for A-Peripheral or forced B-device mode */
 	if ((!host && a_dev) || usb3->start_to_connect)
 		usb3_connect(usb3);
@@ -724,6 +716,28 @@  static void usb3_check_id(struct renesas_usb3 *usb3)
 	schedule_work(&usb3->extcon_work);
 }
 
+static void usb3_check_vbus(struct renesas_usb3 *usb3)
+{
+	if (usb3->workaround_for_vbus) {
+		if (usb3->usb_role_switch_property) {
+			if (usb3->connection_state == USB_ROLE_DEVICE) {
+				usb3_mode_config(usb3, false, false);
+				usb3_connect(usb3);
+			}
+		} else
+			usb3_connect(usb3);
+	} else {
+		usb3->extcon_usb = !!(usb3_read(usb3, USB3_USB_STA) &
+							USB_STA_VBUS_STA);
+		if (usb3->extcon_usb)
+			usb3_connect(usb3);
+		else
+			usb3_disconnect(usb3);
+
+		schedule_work(&usb3->extcon_work);
+	}
+}
+
 static void renesas_usb3_init_controller(struct renesas_usb3 *usb3)
 {
 	usb3_init_axi_bridge(usb3);
@@ -2343,14 +2357,62 @@  static enum usb_role renesas_usb3_role_switch_get(struct device *dev)
 	return cur_role;
 }
 
-static int renesas_usb3_role_switch_set(struct device *dev,
-					enum usb_role role)
+static void handle_ext_role_switch_states(struct device *dev,
+					    enum usb_role role)
+{
+	struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
+	struct device *host = usb3->host_dev;
+	enum usb_role cur_role = renesas_usb3_role_switch_get(dev);
+
+	switch (role) {
+	case USB_ROLE_NONE:
+		usb3->connection_state = USB_ROLE_NONE;
+		if (usb3->driver)
+			usb3_disconnect(usb3);
+		usb3_vbus_out(usb3, false);
+		break;
+	case USB_ROLE_DEVICE:
+		if (usb3->connection_state == USB_ROLE_NONE) {
+			usb3->connection_state = USB_ROLE_DEVICE;
+			usb3_set_mode(usb3, false);
+			if (usb3->driver)
+				usb3_connect(usb3);
+		} else if (cur_role == USB_ROLE_HOST)  {
+			device_release_driver(host);
+			usb3_set_mode(usb3, false);
+			if (usb3->driver)
+				usb3_connect(usb3);
+		}
+		usb3_vbus_out(usb3, false);
+		break;
+	case USB_ROLE_HOST:
+		if (usb3->connection_state == USB_ROLE_NONE) {
+			if (usb3->driver)
+				usb3_disconnect(usb3);
+			usb3_set_mode(usb3, true);
+			usb3_vbus_out(usb3, true);
+			usb3->connection_state = USB_ROLE_HOST;
+		} else if (cur_role == USB_ROLE_DEVICE) {
+			usb3_disconnect(usb3);
+			/* Must set the mode before device_attach of the host */
+			usb3_set_mode(usb3, true);
+			/* This device_attach() might sleep */
+			if (device_attach(host) < 0)
+				dev_err(dev, "device_attach(host) failed\n");
+		}
+		break;
+	default:
+		break;
+	}
+}
+
+static void handle_role_switch_states(struct device *dev,
+					    enum usb_role role)
 {
 	struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
 	struct device *host = usb3->host_dev;
 	enum usb_role cur_role = renesas_usb3_role_switch_get(dev);
 
-	pm_runtime_get_sync(dev);
 	if (cur_role == USB_ROLE_HOST && role == USB_ROLE_DEVICE) {
 		device_release_driver(host);
 		usb3_set_mode(usb3, false);
@@ -2361,6 +2423,20 @@  static int renesas_usb3_role_switch_set(struct device *dev,
 		if (device_attach(host) < 0)
 			dev_err(dev, "device_attach(host) failed\n");
 	}
+}
+
+static int renesas_usb3_role_switch_set(struct device *dev,
+					enum usb_role role)
+{
+	struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
+
+	pm_runtime_get_sync(dev);
+
+	if (usb3->usb_role_switch_property)
+		handle_ext_role_switch_states(dev, role);
+	else
+		handle_role_switch_states(dev, role);
+
 	pm_runtime_put(dev);
 
 	return 0;
@@ -2650,7 +2726,7 @@  static const unsigned int renesas_usb3_cable[] = {
 	EXTCON_NONE,
 };
 
-static const struct usb_role_switch_desc renesas_usb3_role_switch_desc = {
+static struct usb_role_switch_desc renesas_usb3_role_switch_desc = {
 	.set = renesas_usb3_role_switch_set,
 	.get = renesas_usb3_role_switch_get,
 	.allow_userspace_control = true,
@@ -2741,6 +2817,11 @@  static int renesas_usb3_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto err_dev_create;
 
+	if (device_property_read_bool(&pdev->dev, "usb-role-switch")) {
+		usb3->usb_role_switch_property = true;
+		renesas_usb3_role_switch_desc.fwnode = dev_fwnode(&pdev->dev);
+	}
+
 	INIT_WORK(&usb3->role_work, renesas_usb3_role_work);
 	usb3->role_sw = usb_role_switch_register(&pdev->dev,
 					&renesas_usb3_role_switch_desc);