Re: [PATCH 1/2] usb: chipidea: udc: limit usb request length to max 16KB

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

On 24-09-12 12:51:49, Xu Yang wrote:
> Currently, the deivice controller has below limitations:
> 1. can't generate short packet interrupt if IOC not set in dTD. So if one
>    request span more than one dTDs and only the last dTD set IOC, the usb
>    request will pending there if no more data comes.
> 2. the controller can't accurately deliver data to differtent usb requests
>    in some cases due to short packet. For example: one usb request span 3
>    dTDs, then if the controller received a short packet the next packet
>    will go to 2nd dTD of current request rather than the first dTD of next
>    request.
> 

Are there any IP errata for it?

> To let the device controller work properly, one usb request should only
> correspond to one dTD. Then every dTD will set IOC. In theory, each dTD
> support up to 20KB data transfer if the offset is 0. Due to we cannot
> predetermine the offset, this will limit the usb request length to max
> 16KB. This should be fine since most of the user transfer data based on
> this size policy.
> 
> Although these limitations found on OUT eps, we can put the request to IN
> eps too, this will benefit the following patches.

Since IN endpoints have not found the problem, please limit the changes
only for OUT endpoints.

> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> ---
>  drivers/usb/chipidea/ci.h  | 1 +
>  drivers/usb/chipidea/udc.c | 5 +++++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> index 2a38e1eb6546..f8deccfc8713 100644
> --- a/drivers/usb/chipidea/ci.h
> +++ b/drivers/usb/chipidea/ci.h
> @@ -25,6 +25,7 @@
>  #define TD_PAGE_COUNT      5
>  #define CI_HDRC_PAGE_SIZE  4096ul /* page size for TD's */
>  #define ENDPT_MAX          32
> +#define CI_MAX_REQ_SIZE	(4 * CI_HDRC_PAGE_SIZE)
>  #define CI_MAX_BUF_SIZE	(TD_PAGE_COUNT * CI_HDRC_PAGE_SIZE)
>  
>  /******************************************************************************
> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> index b1a1be6439b6..5d9369d01065 100644
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -969,6 +969,11 @@ static int _ep_queue(struct usb_ep *ep, struct usb_request *req,
>  		return -EMSGSIZE;
>  	}
>  
> +	if (hwreq->req.length > CI_MAX_REQ_SIZE) {
> +		dev_err(hwep->ci->dev, "request length too big (max 16KB)\n");
> +		return -EMSGSIZE;
> +	}
> +

Since this IP is used by many vendors, it may fix by others.
I prefer add flag like CI_HDRC_SHORT_PACKET_NOT_SUPPORT, 
and set in imx platform file.

Peter