[PATCH 1/2] usb: ci: Fix gadget reinit

[Marek Vasut]

On 11/19/23 02:04, Simon Holesch wrote:
> The ChipIdea device controller wasn't properly cleaned up when disabled.
> So enabling it again left it in a broken state. The problem occurred for
> example when the host unbinds the driver and binds it again.
> 
> During the first setup, when the out request is queued, the endpoint is
> primed (`epprime`). If the endpoint is then disabled, it stayed primed
> with the initial buffer. So after the endpoint is re-enabled, the device
> controller and device driver were out of sync: the new out request was
> in the driver queue head, yet not submitted, but the "complete" function
> was still called, since the endpoint was primed with the old buffer.
> 
> With the fastboot function this error led to the (rather confusing)
> error message "buffer overflow".
> 
> Fixed by clearing the primed buffers with the `epflush` (`ENDPTFLUSH`)
> register.
> 
> Signed-off-by: Simon Holesch <simon at holesch.de>
> ---
>   drivers/usb/gadget/ci_udc.c | 33 +++++++++++++++++++++++++++++++++
>   1 file changed, 33 insertions(+)
> 
> diff --git a/drivers/usb/gadget/ci_udc.c b/drivers/usb/gadget/ci_udc.c
> index 2bfacfe59f..4ad991c72b 100644
> --- a/drivers/usb/gadget/ci_udc.c
> +++ b/drivers/usb/gadget/ci_udc.c
> @@ -354,12 +354,45 @@ static int ci_ep_enable(struct usb_ep *ep,
>   	return 0;
>   }
>   
> +static void ep_disable(int num, int in)
> +{
> +	struct ci_udc *udc = (struct ci_udc *)controller.ctrl->hcor;
> +	unsigned int ep_bit, n, enable_bit;
> +
> +	if (in) {
> +		ep_bit = EPT_TX(num);
> +		enable_bit = CTRL_TXE;
> +	} else {
> +		ep_bit = EPT_RX(num);
> +		enable_bit = CTRL_RXE;
> +	}

Please use setbits_le32()

> +	/* clear primed buffers */
> +	do {
> +		writel(ep_bit, &udc->epflush);
> +		while (readl(&udc->epflush) & ep_bit)
> +			;
> +	} while (readl(&udc->epstat) & ep_bit);

wait_for_bit_le32() with suitable timeout

> +	/* clear enable bit */
> +	n = readl(&udc->epctrl[num]);
> +	n &= ~enable_bit;
> +	writel(n, &udc->epctrl[num]);

clrbits_le32()

> +}
> +
>   static int ci_ep_disable(struct usb_ep *ep)
>   {
>   	struct ci_ep *ci_ep = container_of(ep, struct ci_ep, ep);
> +	int num, in;
> +
> +	num = ci_ep->desc->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK;
> +	in = (ci_ep->desc->bEndpointAddress & USB_DIR_IN) != 0;
> +
> +	ep_disable(num, in);
>   
>   	ci_ep->desc = NULL;
>   	ep->desc = NULL;
> +	ci_ep->req_primed = false;
>   	return 0;
>   }

With those fixed:

Reviewed-by: Marek Vasut <marex at denx.de>

Thanks !