Re: [PATCH v5 3/3] usb: dwc3: gadget: Refactor EP0 forced stall/restart into a separate API

From: Thinh Nguyen
Date: Mon Apr 17 2023 - 22:07:10 EST


On Thu, Apr 13, 2023, Wesley Cheng wrote:
> Several sequences utilize the same routine for forcing the control endpoint
> back into the SETUP phase. This is required, because those operations need
> to ensure that EP0 is back in the default state.
>
> Signed-off-by: Wesley Cheng <quic_wcheng@xxxxxxxxxxx>
> ---
> drivers/usb/dwc3/gadget.c | 53 ++++++++++++++++-----------------------
> 1 file changed, 21 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 0bb4926933b6..9715de8e99bc 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -139,6 +139,24 @@ int dwc3_gadget_set_link_state(struct dwc3 *dwc, enum dwc3_link_state state)
> return -ETIMEDOUT;
> }
>
> +static void dwc3_ep0_reset_state(struct dwc3 *dwc)
> +{
> + unsigned int dir;
> +
> + if (dwc->ep0state != EP0_SETUP_PHASE) {
> + dir = !!dwc->ep0_expect_in;
> + if (dwc->ep0state == EP0_DATA_PHASE)
> + dwc3_ep0_end_control_data(dwc, dwc->eps[dir]);
> + else
> + dwc3_ep0_end_control_data(dwc, dwc->eps[!dir]);
> +
> + dwc->eps[0]->trb_enqueue = 0;
> + dwc->eps[1]->trb_enqueue = 0;
> +
> + dwc3_ep0_stall_and_restart(dwc);
> + }
> +}
> +
> /**
> * dwc3_ep_inc_trb - increment a trb index.
> * @index: Pointer to the TRB index to increment.
> @@ -2563,16 +2581,9 @@ static int dwc3_gadget_soft_disconnect(struct dwc3 *dwc)
> ret = wait_for_completion_timeout(&dwc->ep0_in_setup,
> msecs_to_jiffies(DWC3_PULL_UP_TIMEOUT));
> if (ret == 0) {
> - unsigned int dir;
> -
> dev_warn(dwc->dev, "wait for SETUP phase timed out\n");
> spin_lock_irqsave(&dwc->lock, flags);
> - dir = !!dwc->ep0_expect_in;
> - if (dwc->ep0state == EP0_DATA_PHASE)
> - dwc3_ep0_end_control_data(dwc, dwc->eps[dir]);
> - else
> - dwc3_ep0_end_control_data(dwc, dwc->eps[!dir]);
> - dwc3_ep0_stall_and_restart(dwc);
> + dwc3_ep0_reset_state(dwc);
> spin_unlock_irqrestore(&dwc->lock, flags);
> }
> }
> @@ -3847,16 +3858,7 @@ static void dwc3_gadget_disconnect_interrupt(struct dwc3 *dwc)
> dwc->setup_packet_pending = false;
> usb_gadget_set_state(dwc->gadget, USB_STATE_NOTATTACHED);
>
> - if (dwc->ep0state != EP0_SETUP_PHASE) {
> - unsigned int dir;
> -
> - dir = !!dwc->ep0_expect_in;
> - if (dwc->ep0state == EP0_DATA_PHASE)
> - dwc3_ep0_end_control_data(dwc, dwc->eps[dir]);
> - else
> - dwc3_ep0_end_control_data(dwc, dwc->eps[!dir]);
> - dwc3_ep0_stall_and_restart(dwc);
> - }
> + dwc3_ep0_reset_state(dwc);
> }
>
> static void dwc3_gadget_reset_interrupt(struct dwc3 *dwc)
> @@ -3910,20 +3912,7 @@ static void dwc3_gadget_reset_interrupt(struct dwc3 *dwc)
> * phase. So ensure that EP0 is in setup phase by issuing a stall
> * and restart if EP0 is not in setup phase.
> */
> - if (dwc->ep0state != EP0_SETUP_PHASE) {
> - unsigned int dir;
> -
> - dir = !!dwc->ep0_expect_in;
> - if (dwc->ep0state == EP0_DATA_PHASE)
> - dwc3_ep0_end_control_data(dwc, dwc->eps[dir]);
> - else
> - dwc3_ep0_end_control_data(dwc, dwc->eps[!dir]);
> -
> - dwc->eps[0]->trb_enqueue = 0;
> - dwc->eps[1]->trb_enqueue = 0;
> -
> - dwc3_ep0_stall_and_restart(dwc);
> - }
> + dwc3_ep0_reset_state(dwc);
>
> /*
> * In the Synopsis DesignWare Cores USB3 Databook Rev. 3.30a

Acked-by: Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx>

Thanks,
Thinh