Hi Eugeniu,
Thank you very much for the patches!
> From: Eugeniu Rosca, Sent: Thursday, August 30, 2018 7:54 PM
>
> From: Veeraiyan Chidambaram <veeraiyan.chidambaram@xxxxxxxxxxxx>
>
> When RCAR3 is in Gadget mode, 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 host detach, extend the DVSQ_MASK to cover the
> Suspended bit of the DVSQ[2:0] bitfield from the Interrupt Status
> Register 0 (INTSTS0) register and perform appropriate action in the
> DVST interrupt handler (usbhsg_irq_dev_state).
>
> Without this commit, disconnection of the phone from R-Car-H3 ES2.0
> Salvator-X CN9 port is not recognized and reverse role switch does
> not not happen. If phone is connected again it does not enumerate.
I think s/not not/not/.
> 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@xxxxxxxxxxxx>
> Signed-off-by: Eugeniu Rosca <erosca@xxxxxxxxxxxxxx>
> ---
> drivers/usb/renesas_usbhs/common.h | 3 ++-
> drivers/usb/renesas_usbhs/mod_gadget.c | 13 ++++++++++---
> 2 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/renesas_usbhs/common.h b/drivers/usb/renesas_usbhs/common.h
> index 6e02aa3e287f..9eb69d2a4640 100644
> --- a/drivers/usb/renesas_usbhs/common.h
> +++ b/drivers/usb/renesas_usbhs/common.h
> @@ -163,11 +163,12 @@ struct usbhs_priv;
> #define VBSTS (1 << 7) /* VBUS_0 and VBUSIN_0 Input Status */
> #define VALID (1 << 3) /* USB Request Receive */
>
> -#define DVSQ_MASK (0x3 << 4) /* Device State */
> +#define DVSQ_MASK (0x7 << 4) /* Device State */
> #define POWER_STATE (0 << 4)
> #define DEFAULT_STATE (1 << 4)
> #define ADDRESS_STATE (2 << 4)
> #define CONFIGURATION_STATE (3 << 4)
> +#define SUSPENDED_STATE (4 << 4)
>
> #define CTSQ_MASK (0x7) /* Control Transfer Stage */
> #define IDLE_SETUP_STAGE 0 /* Idle stage or setup stage */
> diff --git a/drivers/usb/renesas_usbhs/mod_gadget.c b/drivers/usb/renesas_usbhs/mod_gadget.c
> index c068b673420b..1af8034d17c5 100644
> --- a/drivers/usb/renesas_usbhs/mod_gadget.c
> +++ b/drivers/usb/renesas_usbhs/mod_gadget.c
> @@ -465,12 +465,19 @@ static int usbhsg_irq_dev_state(struct usbhs_priv *priv,
> {
> struct usbhsg_gpriv *gpriv = usbhsg_priv_to_gpriv(priv);
> struct device *dev = usbhsg_gpriv_to_dev(gpriv);
> + int state = usbhs_status_get_device_state(irq_state);
>
> gpriv->gadget.speed = usbhs_bus_get_speed(priv);
>
> - dev_dbg(dev, "state = %x : speed : %d\n",
> - usbhs_status_get_device_state(irq_state),
> - gpriv->gadget.speed);
> + dev_dbg(dev, "state = %x : speed : %d\n", state, gpriv->gadget.speed);
> +
> + if (gpriv->driver &&
> + gpriv->driver->suspend &&
> + gpriv->gadget.speed != USB_SPEED_UNKNOWN &&
> + (state & SUSPENDED_STATE)) {
> + gpriv->driver->suspend(&gpriv->gadget);
> + usb_gadget_set_state(&gpriv->gadget, USB_STATE_SUSPENDED);
> + }
I think we also call gpriv->driver->resume() somehow. Otherwise,
/sys/devices/platform/soc/e6590000.usb/gadget/suspended value on R-Car H3
keeps 1 forever after the driver detects suspend state.
If we call the ->resume(), I think we also call usb_gadget_set_state() with
other USB_STATE_* value. So, I'm thinking if usbhs_status_get_device_state() returns
enum usb_device_state value, it's useful.
What do you think?
Best regards,
Yoshihiro Shimoda
> return 0;
> }
> --
> 2.18.0
