Re: [PATCH 3/3] usb: chipidea: udc: make isoc endpoint a bit error tolerant

[Peter Chen <peter.chen@kernel.org>]

On 24-09-12 11:35:51, Xu Yang wrote:
> When a endpoint met errors, the usb controller will firstly assert
> related error bit in status filed of dTD, then ENDPTCOMPLETE will be
> asserted. Finally, USBSTS.UEI will be set.
> 
> Due to isoc transfers are error-tolerant transfers, we can make isoc
> endpoint a bit error tolerant on device mode too. In case of error,
> it's possilbe to resume the endpoint by reprime the corresponding
> endpoint.
> 
> When error occurs, this will allow error dTD be deleted from dQH and
> giveback request to user. Then, a reprime/prime operation is executed
> depends on whether dQH is empty or not. If dQH is not empty, reprime
> will be done during dequeue process. If dQH is empty, prime will be
> done when new dTD is linked. In this way, isoc transfer can be recovered
> from a small number of errors.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> ---
>  drivers/usb/chipidea/udc.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> index 0ab57b87b07b..b1a1be6439b6 100644
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -683,6 +683,7 @@ static int _hardware_dequeue(struct ci_hw_ep *hwep, struct ci_hw_req *hwreq)
>  	unsigned remaining_length;
>  	unsigned actual = hwreq->req.length;
>  	struct ci_hdrc *ci = hwep->ci;
> +	bool is_isoc = hwep->type == USB_ENDPOINT_XFER_ISOC;
>  
>  	if (hwreq->req.status != -EALREADY)
>  		return -EINVAL;
> @@ -696,7 +697,7 @@ static int _hardware_dequeue(struct ci_hw_ep *hwep, struct ci_hw_req *hwreq)
>  			int n = hw_ep_bit(hwep->num, hwep->dir);
>  
>  			if (ci->rev == CI_REVISION_24 ||
> -			    ci->rev == CI_REVISION_22)
> +			    ci->rev == CI_REVISION_22 || is_isoc)
>  				if (!hw_read(ci, OP_ENDPTSTAT, BIT(n)))
>  					reprime_dtd(ci, hwep, node);
>  			hwreq->req.status = -EALREADY;
> @@ -707,7 +708,6 @@ static int _hardware_dequeue(struct ci_hw_ep *hwep, struct ci_hw_req *hwreq)
>  		remaining_length >>= __ffs(TD_TOTAL_BYTES);
>  		actual -= remaining_length;
>  
> -		hwreq->req.status = tmptoken & TD_STATUS;

Non-ISO dTD may need error status? You may refine code by ISO and
non-ISOC.

Peter

>  		if ((TD_STATUS_HALTED & hwreq->req.status)) {
>  			hwreq->req.status = -EPIPE;
>  			break;
> @@ -715,11 +715,13 @@ static int _hardware_dequeue(struct ci_hw_ep *hwep, struct ci_hw_req *hwreq)
>  			hwreq->req.status = -EPROTO;
>  			break;
>  		} else if ((TD_STATUS_TR_ERR & hwreq->req.status)) {
> -			hwreq->req.status = -EILSEQ;
> -			break;
> +			if (!is_isoc) {
> +				hwreq->req.status = -EILSEQ;
> +				break;
> +			}
>  		}
>  
> -		if (remaining_length) {
> +		if (remaining_length && !is_isoc) {
>  			if (hwep->dir == TX) {
>  				hwreq->req.status = -EPROTO;
>  				break;
> -- 
> 2.34.1
>