Re: [PATCH v2] usb: cdnsp: Fix issue with detecting command completion event

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

On 25-05-13 05:30:09, Pawel Laszczak wrote:
> In some cases, there is a small-time gap in which CMD_RING_BUSY
> can be cleared by controller but adding command completion event
> to event ring will be delayed. As the result driver will return
> error code.
> This behavior has been detected on usbtest driver (test 9) with
> configuration including ep1in/ep1out bulk and ep2in/ep2out isoc
> endpoint.
> Probably this gap occurred because controller was busy with adding
> some other events to event ring.
> The CMD_RING_BUSY is cleared to '0' when the Command Descriptor
> has been executed and not when command completion event has been
> added to event ring.
> 
> To fix this issue for this test the small delay is sufficient
> less than 10us) but to make sure the problem doesn't happen again
> in the future the patch introduces 10 retries to check with delay
> about 20us before returning error code.
> 
> Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence USBSSP DRD Driver")
> cc: stable@vger.kernel.org
> Signed-off-by: Pawel Laszczak <pawell@cadence.com>

Acked-by: Peter Chen <peter.chen@kernel.org>

Peter
> ---
> Changelog:
> v2:
> - replaced usleep_range with udelay
> - increased retry counter and decreased the udelay value
> 
>  drivers/usb/cdns3/cdnsp-gadget.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/cdns3/cdnsp-gadget.c b/drivers/usb/cdns3/cdnsp-gadget.c
> index 4824a10df07e..58650b7f4173 100644
> --- a/drivers/usb/cdns3/cdnsp-gadget.c
> +++ b/drivers/usb/cdns3/cdnsp-gadget.c
> @@ -547,6 +547,7 @@ int cdnsp_wait_for_cmd_compl(struct cdnsp_device *pdev)
>  	dma_addr_t cmd_deq_dma;
>  	union cdnsp_trb *event;
>  	u32 cycle_state;
> +	u32 retry = 10;
>  	int ret, val;
>  	u64 cmd_dma;
>  	u32  flags;
> @@ -578,8 +579,23 @@ int cdnsp_wait_for_cmd_compl(struct cdnsp_device *pdev)
>  		flags = le32_to_cpu(event->event_cmd.flags);
>  
>  		/* Check the owner of the TRB. */
> -		if ((flags & TRB_CYCLE) != cycle_state)
> +		if ((flags & TRB_CYCLE) != cycle_state) {
> +			/*
> +			 * Give some extra time to get chance controller
> +			 * to finish command before returning error code.
> +			 * Checking CMD_RING_BUSY is not sufficient because
> +			 * this bit is cleared to '0' when the Command
> +			 * Descriptor has been executed by controller
> +			 * and not when command completion event has
> +			 * be added to event ring.
> +			 */
> +			if (retry--) {
> +				udelay(20);
> +				continue;
> +			}
> +
>  			return -EINVAL;
> +		}
>  
>  		cmd_dma = le64_to_cpu(event->event_cmd.cmd_trb);
>  
> -- 
> 2.43.0
> 

-- 

Best regards,
Peter