2017-03-28 14:12:46

by Minas Harutyunyan

[permalink] [raw]
Subject: [PATCH] dwc2: gadget: Fix in control write transfers

After data out stage gadget driver should not initate ZLP on control EP,
because it is up to function driver.

Signed-off-by: Minas Harutyunyan <[email protected]>
---
drivers/usb/dwc2/gadget.c | 20 ++++++--------------
1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 98a4a79e7f6e..b7520fa3725b 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -1369,6 +1369,11 @@ static int dwc2_hsotg_ep_queue(struct usb_ep *ep, struct usb_request *req,
return 0;
}

+ /* Change EP direction if status phase request is after data out */
+ if (!hs_ep->index && !req->length && !hs_ep->dir_in &&
+ (hs->ep0_state == DWC2_EP0_DATA_OUT))
+ hs_ep->dir_in = 1;
+
if (first) {
if (!hs_ep->isochronous) {
dwc2_hsotg_start_req(hs, hs_ep, hs_req, false);
@@ -2327,14 +2332,6 @@ static void dwc2_hsotg_handle_outdone(struct dwc2_hsotg *hsotg, int epnum)
*/
}

- /* DDMA IN status phase will start from StsPhseRcvd interrupt */
- if (!using_desc_dma(hsotg) && epnum == 0 &&
- hsotg->ep0_state == DWC2_EP0_DATA_OUT) {
- /* Move to STATUS IN */
- dwc2_hsotg_ep0_zlp(hsotg, true);
- return;
- }
-
/*
* Slave mode OUT transfers do not go through XferComplete so
* adjust the ISOC parity here.
@@ -2997,14 +2994,9 @@ static void dwc2_hsotg_epint(struct dwc2_hsotg *hsotg, unsigned int idx,
}
}

- if (ints & DXEPINT_STSPHSERCVD) {
+ if (ints & DXEPINT_STSPHSERCVD)
dev_dbg(hsotg->dev, "%s: StsPhseRcvd\n", __func__);

- /* Move to STATUS IN for DDMA */
- if (using_desc_dma(hsotg))
- dwc2_hsotg_ep0_zlp(hsotg, true);
- }
-
if (ints & DXEPINT_BACK2BACKSETUP)
dev_dbg(hsotg->dev, "%s: B2BSetup/INEPNakEff\n", __func__);

--
2.11.0


2017-03-30 10:42:50

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH] dwc2: gadget: Fix in control write transfers


Hi,

Minas Harutyunyan <[email protected]> writes:
> After data out stage gadget driver should not initate ZLP on control EP,
> because it is up to function driver.

not true always, depends on return value from ->setup(). Which problem
did you have? Which gadget driver? How did you reproduce? Which other
tests did you run on this patch?

--
balbi


Attachments:
signature.asc (832.00 B)

2017-04-03 05:32:29

by Minas Harutyunyan

[permalink] [raw]
Subject: Re: [PATCH] dwc2: gadget: Fix in control write transfers

Hi,

On 3/30/2017 2:42 PM, Felipe Balbi wrote:
>
> Hi,
>
> Minas Harutyunyan <[email protected]> writes:
>> After data out stage gadget driver should not initate ZLP on control EP,
>> because it is up to function driver.
>
> not true always, depends on return value from ->setup(). Which problem
> did you have? Which gadget driver? How did you reproduce? Which other
> tests did you run on this patch?
>

This required for delayed status support. Tested with Synopsys test
gadget. As host used USB tracer traffic generator (different control
transfers scenarios). Also performed smoke tests with mass storage
function to detect any side effects.

Thanks,
Minas

2017-04-03 07:06:11

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH] dwc2: gadget: Fix in control write transfers


Hi,

Minas Harutyunyan <[email protected]> writes:
>> Minas Harutyunyan <[email protected]> writes:
>>> After data out stage gadget driver should not initate ZLP on control EP,
>>> because it is up to function driver.
>>
>> not true always, depends on return value from ->setup(). Which problem
>> did you have? Which gadget driver? How did you reproduce? Which other
>> tests did you run on this patch?
>>
>
> This required for delayed status support. Tested with Synopsys test
> gadget. As host used USB tracer traffic generator (different control
> transfers scenarios). Also performed smoke tests with mass storage
> function to detect any side effects.

so you didn't test any gadget driver that doesn't rely on
delayed_status, right? Care to test one of those?

The situation here is a little too complex (and we're trying to change
it). Here's how it goes:

if (ctrl->wLength == 0) /* 2-stage ctrl */ {
gadget_driver always queues STATUS;
} else {
gadget_driver queues DATA;
if (result == DELAYED_STATUS)
gadgdet_driver queues STATUS
else
UDC handles STATUS
}

It seems to me, you're not handling this properly as of yet. Please make
sure several gadget drivers work for you. Try g_mass_storage and g_zero
at least. :-)

--
balbi