diff mbox series

[v2] usb: dwc3: gadget: Do link recovery for SS and SSP

Message ID 20c05d4e60c97b03314ede8d7f2d7c29b34f665f.1576028945.git.thinhn@synopsys.com (mailing list archive)
State Mainlined
Commit d0550cd20e52558ecf6847a0f96ebd5d944c17e4
Headers show
Series [v2] usb: dwc3: gadget: Do link recovery for SS and SSP | expand

Commit Message

Thinh Nguyen Dec. 11, 2019, 2:26 a.m. UTC
The controller always supports link recovery for device in SS and SSP.
Remove the speed limit check. Also, when the device is in RESUME or
RESET state, it means the controller received the resume/reset request.
The driver must send the link recovery to acknowledge the request. They
are valid states for the driver to send link recovery.

Fixes: 72246da40f37 ("usb: Introduce DesignWare USB3 DRD Driver")
Fixes: ee5cd41c9117 ("usb: dwc3: Update speed checks for SuperSpeedPlus")
Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
---
Changes in v2 -
  Added Fixes tags

 drivers/usb/dwc3/gadget.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

Comments

Thinh Nguyen March 31, 2020, 2:12 a.m. UTC | #1
Hi,

Thinh Nguyen wrote:
> The controller always supports link recovery for device in SS and SSP.
> Remove the speed limit check. Also, when the device is in RESUME or
> RESET state, it means the controller received the resume/reset request.
> The driver must send the link recovery to acknowledge the request. They
> are valid states for the driver to send link recovery.
>
> Fixes: 72246da40f37 ("usb: Introduce DesignWare USB3 DRD Driver")
> Fixes: ee5cd41c9117 ("usb: dwc3: Update speed checks for SuperSpeedPlus")
> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
> ---
>
> Resend note -
>    This was on Felipe's next branch some time ago,
>    but it was lost somehow.
>
> Changes in v2 -
>    Added Fixes tags
>
>   drivers/usb/dwc3/gadget.c | 8 ++------
>   1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index a9aba716bf80..3dcdde9080f5 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -1712,7 +1712,6 @@ static int __dwc3_gadget_wakeup(struct dwc3 *dwc)
>   	u32			reg;
>   
>   	u8			link_state;
> -	u8			speed;
>   
>   	/*
>   	 * According to the Databook Remote wakeup request should
> @@ -1722,16 +1721,13 @@ static int __dwc3_gadget_wakeup(struct dwc3 *dwc)
>   	 */
>   	reg = dwc3_readl(dwc->regs, DWC3_DSTS);
>   
> -	speed = reg & DWC3_DSTS_CONNECTSPD;
> -	if ((speed == DWC3_DSTS_SUPERSPEED) ||
> -	    (speed == DWC3_DSTS_SUPERSPEED_PLUS))
> -		return 0;
> -
>   	link_state = DWC3_DSTS_USBLNKST(reg);
>   
>   	switch (link_state) {
> +	case DWC3_LINK_STATE_RESET:
>   	case DWC3_LINK_STATE_RX_DET:	/* in HS, means Early Suspend */
>   	case DWC3_LINK_STATE_U3:	/* in HS, means SUSPEND */
> +	case DWC3_LINK_STATE_RESUME:
>   		break;
>   	default:
>   		return -EINVAL;

Maybe this patch was lost somewhere. Let me know if there's any issue 
with this patch.

Thanks,
Thinh
Felipe Balbi March 31, 2020, 8:11 a.m. UTC | #2
Hi,

Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:
> Thinh Nguyen wrote:
>> The controller always supports link recovery for device in SS and SSP.
>> Remove the speed limit check. Also, when the device is in RESUME or
>> RESET state, it means the controller received the resume/reset request.
>> The driver must send the link recovery to acknowledge the request. They
>> are valid states for the driver to send link recovery.
>>
>> Fixes: 72246da40f37 ("usb: Introduce DesignWare USB3 DRD Driver")
>> Fixes: ee5cd41c9117 ("usb: dwc3: Update speed checks for SuperSpeedPlus")
>> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
>> ---
>>
>> Resend note -
>>    This was on Felipe's next branch some time ago,
>>    but it was lost somehow.
>>
>> Changes in v2 -
>>    Added Fixes tags
>>
>>   drivers/usb/dwc3/gadget.c | 8 ++------
>>   1 file changed, 2 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index a9aba716bf80..3dcdde9080f5 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -1712,7 +1712,6 @@ static int __dwc3_gadget_wakeup(struct dwc3 *dwc)
>>   	u32			reg;
>>   
>>   	u8			link_state;
>> -	u8			speed;
>>   
>>   	/*
>>   	 * According to the Databook Remote wakeup request should
>> @@ -1722,16 +1721,13 @@ static int __dwc3_gadget_wakeup(struct dwc3 *dwc)
>>   	 */
>>   	reg = dwc3_readl(dwc->regs, DWC3_DSTS);
>>   
>> -	speed = reg & DWC3_DSTS_CONNECTSPD;
>> -	if ((speed == DWC3_DSTS_SUPERSPEED) ||
>> -	    (speed == DWC3_DSTS_SUPERSPEED_PLUS))
>> -		return 0;
>> -
>>   	link_state = DWC3_DSTS_USBLNKST(reg);
>>   
>>   	switch (link_state) {
>> +	case DWC3_LINK_STATE_RESET:
>>   	case DWC3_LINK_STATE_RX_DET:	/* in HS, means Early Suspend */
>>   	case DWC3_LINK_STATE_U3:	/* in HS, means SUSPEND */
>> +	case DWC3_LINK_STATE_RESUME:
>>   		break;
>>   	default:
>>   		return -EINVAL;
>
> Maybe this patch was lost somewhere. Let me know if there's any issue 
> with this patch.

It's now in my testing/fixes
diff mbox series

Patch

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index a9aba716bf80..3dcdde9080f5 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1712,7 +1712,6 @@  static int __dwc3_gadget_wakeup(struct dwc3 *dwc)
 	u32			reg;
 
 	u8			link_state;
-	u8			speed;
 
 	/*
 	 * According to the Databook Remote wakeup request should
@@ -1722,16 +1721,13 @@  static int __dwc3_gadget_wakeup(struct dwc3 *dwc)
 	 */
 	reg = dwc3_readl(dwc->regs, DWC3_DSTS);
 
-	speed = reg & DWC3_DSTS_CONNECTSPD;
-	if ((speed == DWC3_DSTS_SUPERSPEED) ||
-	    (speed == DWC3_DSTS_SUPERSPEED_PLUS))
-		return 0;
-
 	link_state = DWC3_DSTS_USBLNKST(reg);
 
 	switch (link_state) {
+	case DWC3_LINK_STATE_RESET:
 	case DWC3_LINK_STATE_RX_DET:	/* in HS, means Early Suspend */
 	case DWC3_LINK_STATE_U3:	/* in HS, means SUSPEND */
+	case DWC3_LINK_STATE_RESUME:
 		break;
 	default:
 		return -EINVAL;