Re: [PATCH] usb: chipidea: udc: disconnect/reconnect from host when do suspend/resume

["Peter Chen (CIX)" <peter.chen@kernel.org>]

On 25-06-14 20:49:14, Xu Yang wrote:
> Shawn and John reported a hang issue during system suspend as below:
> 
>  - USB gadget is enabled as Ethernet
>  - There is data transfer over USB Ethernet (scp a big file between host
>                                              and device)
>  - Device is going in/out suspend (echo mem > /sys/power/state)
> 
> The root cause is the USB device controller is suspended but the USB bus
> is still active which caused the USB host continues to transfer data with
> device and the device continues to queue USB requests (in this case, a
> delayed TCP ACK packet trigger the issue) after controller is suspended,
> however the USB controller clock is already gated off. Then if udc driver
> access registers after that point, the system will hang.
> 
> The correct way to avoid such issue is to disconnect device from host when
> the USB bus is not at suspend state. Then the host will receive disconnect
> event and stop data transfer in time. To continue make USB gadget device
> work after system resume, this will reconnect device automatically.
> 
> To make usb wakeup work if USB bus is already at suspend state, this will
> keep connection for it only when USB device controller has enabled wakeup
> capability.
> 
> Reported-by: Shawn Guo <shawnguo@kernel.org>
> Reported-by: John Ernberg <john.ernberg@actia.se>
> Closes: https://lore.kernel.org/linux-usb/aEZxmlHmjeWcXiF3@dragon/
> Tested-by: John Ernberg <john.ernberg@actia.se> # iMX8QXP
> Fixes: 235ffc17d014 ("usb: chipidea: udc: add suspend/resume support for device controller")
> Cc: stable@vger.kernel.org
> Reviewed-by: Jun Li <jun.li@nxp.com>
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> ---
>  drivers/usb/chipidea/udc.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> index 8a9b31fd5c89..1a48e6440e6c 100644
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -2374,6 +2374,10 @@ static void udc_suspend(struct ci_hdrc *ci)
>  	 */
>  	if (hw_read(ci, OP_ENDPTLISTADDR, ~0) == 0)
>  		hw_write(ci, OP_ENDPTLISTADDR, ~0, ~0);
> +
> +	if (ci->gadget.connected &&
> +	    (!ci->suspended || !device_may_wakeup(ci->dev)))
> +		usb_gadget_disconnect(&ci->gadget);
>  }

Don't we need to notify gadget(class) side the disconnect event?

Peter
>  
>  static void udc_resume(struct ci_hdrc *ci, bool power_lost)
> @@ -2384,6 +2388,9 @@ static void udc_resume(struct ci_hdrc *ci, bool power_lost)
>  					OTGSC_BSVIS | OTGSC_BSVIE);
>  		if (ci->vbus_active)
>  			usb_gadget_vbus_disconnect(&ci->gadget);
> +	} else if (ci->vbus_active && ci->driver &&
> +		   !ci->gadget.connected) {
> +		usb_gadget_connect(&ci->gadget);
>  	}
>  
>  	/* Restore value 0 if it was set for power lost check */
> -- 
> 2.34.1
> 

-- 

Best regards,
Peter