Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1611015 > unrolled thread

[PATCH] dwc2: gadget: Fix in control write transfers

Back to article view | Back to linux.kernel

Contents

  [PATCH] dwc2: gadget: Fix in control write transfers Minas Harutyunyan <Minas.Harutyunyan@synopsys.com> - 2017-03-28 16:20 +0200
    Re: [PATCH] dwc2: gadget: Fix in control write transfers Felipe Balbi <balbi@kernel.org> - 2017-03-30 12:50 +0200
      Re: [PATCH] dwc2: gadget: Fix in control write transfers Minas Harutyunyan <Minas.Harutyunyan@synopsys.com> - 2017-04-03 07:40 +0200
        Re: [PATCH] dwc2: gadget: Fix in control write transfers Felipe Balbi <balbi@kernel.org> - 2017-04-03 09:10 +0200

#1611015 — [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 <hminas@synopsys.com>
---
 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

[toc] | [next] | [standalone]

#1612942

[Multipart message — attachments visible in raw view] — view raw

Hi,

Minas Harutyunyan <Minas.Harutyunyan@synopsys.com> 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

[toc] | [prev] | [next] | [standalone]

#1614895

Hi,

On 3/30/2017 2:42 PM, Felipe Balbi wrote:
>
> Hi,
>
> Minas Harutyunyan <Minas.Harutyunyan@synopsys.com> 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

[toc] | [prev] | [next] | [standalone]

#1614916

Hi,

Minas Harutyunyan <Minas.Harutyunyan@synopsys.com> writes:
>> Minas Harutyunyan <Minas.Harutyunyan@synopsys.com> 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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web