diff mbox series

usb: gadget: udc: renesas_usb3: add suspend event support

Message ID 1567589047-29816-1-git-send-email-external.veeraiyan.c@de.adit-jv.com (mailing list archive)
State Superseded
Delegated to: Geert Uytterhoeven
Headers show
Series usb: gadget: udc: renesas_usb3: add suspend event support | expand

Commit Message

veeraiyan chidambaram Sept. 4, 2019, 9:24 a.m. UTC
From: Veeraiyan Chidambaram <veeraiyan.chidambaram@in.bosch.com>

In RCAR3 USB 3.0 Function, if host is detached an interrupt
will be generated and Suspended state bit is set in interrupt status
register. Interrupt handler will call driver->suspend(composite_suspend)
if suspended state bit is set. composite_suspend will call
ffs_func_suspend which will post FUNCTIONFS_SUSPEND and will be consumed
by user space application via /dev/ep0.

To be able to detect the host detach, USB_INT_1_B2_SPND to cover the
Suspended bit of the B2_SPND_OUT[9] from the USB Status Register
(USB_STA) register and perform appropriate action in the
usb3_irq_epc_int_1 function.

Without this commit, disconnection of the phone from R-Car-H3 ES2.0
Salvator-X CN11 port is not recognized and reverse role switch does
not happen. If phone is connected again it does not enumerate.

With this commit, disconnection will be recognized and reverse role
switch will happen. If phone is connected again it will enumerate
properly and will become visible in the output of 'lsusb'.

Signed-off-by: Veeraiyan Chidambaram <veeraiyan.chidambaram@in.bosch.com>
---
 drivers/usb/gadget/udc/renesas_usb3.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

Comments

Yoshihiro Shimoda Sept. 4, 2019, 12:08 p.m. UTC | #1
Hi Veeraiyan,

Thank you very much for the patch! I didn't realize that using USB_INT_1_B2_SPND
can resolve such a behavior. I'd like to apply this patch into upstream, but
I have some comments below.

> From: Veeraiyan Chidambaram, Sent: Wednesday, September 4, 2019 6:24 PM
> 
> In RCAR3 USB 3.0 Function, if host is detached an interrupt

I'd like to replace "RCAR3" with "R-Car Gen3".

> will be generated and Suspended state bit is set in interrupt status
> register. Interrupt handler will call driver->suspend(composite_suspend)
> if suspended state bit is set. composite_suspend will call
> ffs_func_suspend which will post FUNCTIONFS_SUSPEND and will be consumed
> by user space application via /dev/ep0.
> 
> To be able to detect the host detach, USB_INT_1_B2_SPND to cover the
> Suspended bit of the B2_SPND_OUT[9] from the USB Status Register
> (USB_STA) register and perform appropriate action in the
> usb3_irq_epc_int_1 function.
> 
> Without this commit, disconnection of the phone from R-Car-H3 ES2.0

s/R-Car-H3/R-Car H3/

> Salvator-X CN11 port is not recognized and reverse role switch does
> not happen. If phone is connected again it does not enumerate.
> 
> With this commit, disconnection will be recognized and reverse role
> switch will happen. If phone is connected again it will enumerate

IIUC, reverse role switch will happen by a user space application.
Is it correct?

> properly and will become visible in the output of 'lsusb'.
> 
> Signed-off-by: Veeraiyan Chidambaram <veeraiyan.chidambaram@in.bosch.com>
> ---
>  drivers/usb/gadget/udc/renesas_usb3.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/usb/gadget/udc/renesas_usb3.c b/drivers/usb/gadget/udc/renesas_usb3.c
> index eaa3339b30a2..4ec703e302f5 100644
> --- a/drivers/usb/gadget/udc/renesas_usb3.c
> +++ b/drivers/usb/gadget/udc/renesas_usb3.c
> @@ -767,6 +767,19 @@ static void usb3_irq_epc_int_1_resume(struct renesas_usb3 *usb3)
>  	usb3_transition_to_default_state(usb3, false);
>  }
> 
> +static void usb3_irq_epc_int_1_suspend(struct renesas_usb3 *usb3)
> +{
> +	usb3_disable_irq_1(usb3, USB_INT_1_B2_SPND);
> +
> +	if (usb3->driver &&
> +	    usb3->driver->suspend &&
> +	    usb3->gadget.speed != USB_SPEED_UNKNOWN &&
> +	    usb3->gadget.state != USB_STATE_NOTATTACHED) {
> +		usb3->driver->suspend(&usb3->gadget);
> +		usb_gadget_set_state(&usb3->gadget, USB_STATE_SUSPENDED);

I'd like to change the conditions like below if it still works on your environment.
This is because I'd like to set the gadget state if other gadget driver except f_fs
is used anyway. After that, a user also can recognize the state by using
/sys/devices/platform/soc/ee020000.usb/udc/ee020000.usb/state even if the use
doesn't use f_fs driver.

	if (usb3->gadget.speed != USB_SPEED_UNKNOWN &&
	    usb3->gadget.state != USB_STATE_NOTATTACHED) {
		if (usb3->driver && usb3->driver->suspend)
			usb3->driver->suspend(&usb3->gadget);
		usb_gadget_set_state(&usb3->gadget, USB_STATE_SUSPENDED);
	}

Best regards,
Yoshihiro Shimoda
veeraiyan chidambaram Sept. 4, 2019, 2:31 p.m. UTC | #2
Hi Shimoda-san,

Thanks a lot  for your feedback.

On Wed, Sep 04, 2019 at 12:08:39PM +0000, Yoshihiro Shimoda wrote:
> Hi Veeraiyan,
> 
> Thank you very much for the patch! I didn't realize that using USB_INT_1_B2_SPND
> can resolve such a behavior. I'd like to apply this patch into upstream, but
> I have some comments below.
> 
> > From: Veeraiyan Chidambaram, Sent: Wednesday, September 4, 2019 6:24 PM
> > 
> > In RCAR3 USB 3.0 Function, if host is detached an interrupt
> 
> I'd like to replace "RCAR3" with "R-Car Gen3".
  Yes . I agree  
> > Without this commit, disconnection of the phone from R-Car-H3 ES2.0
> 
> s/R-Car-H3/R-Car H3/
  Yes . I agree 
> IIUC, reverse role switch will happen by a user space application.
> Is it correct?
  Yes, understanding is correct and made the change in commit message. 

> I'd like to change the conditions like below if it still works on your environment.
> This is because I'd like to set the gadget state if other gadget driver except f_fs
> is used anyway. After that, a user also can recognize the state by using
> /sys/devices/platform/soc/ee020000.usb/udc/ee020000.usb/state even if the use
> doesn't use f_fs driver.
> 
> 	if (usb3->gadget.speed != USB_SPEED_UNKNOWN &&
> 	    usb3->gadget.state != USB_STATE_NOTATTACHED) {
> 		if (usb3->driver && usb3->driver->suspend)
> 			usb3->driver->suspend(&usb3->gadget);
> 		usb_gadget_set_state(&usb3->gadget, USB_STATE_SUSPENDED);
> 	}

Above change worked in our environment. I prepare patch for this
> Best regards,
> Yoshihiro Shimoda
>
diff mbox series

Patch

diff --git a/drivers/usb/gadget/udc/renesas_usb3.c b/drivers/usb/gadget/udc/renesas_usb3.c
index eaa3339b30a2..4ec703e302f5 100644
--- a/drivers/usb/gadget/udc/renesas_usb3.c
+++ b/drivers/usb/gadget/udc/renesas_usb3.c
@@ -767,6 +767,19 @@  static void usb3_irq_epc_int_1_resume(struct renesas_usb3 *usb3)
 	usb3_transition_to_default_state(usb3, false);
 }
 
+static void usb3_irq_epc_int_1_suspend(struct renesas_usb3 *usb3)
+{
+	usb3_disable_irq_1(usb3, USB_INT_1_B2_SPND);
+
+	if (usb3->driver &&
+	    usb3->driver->suspend &&
+	    usb3->gadget.speed != USB_SPEED_UNKNOWN &&
+	    usb3->gadget.state != USB_STATE_NOTATTACHED) {
+		usb3->driver->suspend(&usb3->gadget);
+		usb_gadget_set_state(&usb3->gadget, USB_STATE_SUSPENDED);
+	}
+}
+
 static void usb3_irq_epc_int_1_disable(struct renesas_usb3 *usb3)
 {
 	usb3_stop_usb3_connection(usb3);
@@ -852,6 +865,9 @@  static void usb3_irq_epc_int_1(struct renesas_usb3 *usb3, u32 int_sta_1)
 	if (int_sta_1 & USB_INT_1_B2_RSUM)
 		usb3_irq_epc_int_1_resume(usb3);
 
+	if (int_sta_1 & USB_INT_1_B2_SPND)
+		usb3_irq_epc_int_1_suspend(usb3);
+
 	if (int_sta_1 & USB_INT_1_SPEED)
 		usb3_irq_epc_int_1_speed(usb3);