| Message ID | 8b417edae18f3b4f35a11aa3796c1512dfcc4c91.1527163379.git.hminas@synopsys.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Minas Harutyunyan <Minas.Harutyunyan@synopsys.com> writes: > To start ISOC transfers in handlers dwc2_gadget_handle_nak() and > dwc2_gadget_handle_out_token_ep_disabled() driver reads current frame > number, based on which, set target frame number to start first ISOC > transfer. > > In case if system's high IRQ latency and multiple EP's asserted > interrupt in same frame, there are high probability that when reading > current frame number in EP's handlers, actual frame number can be > increased. As result for bInterval > 1, starting target frame > will be set wrongly and all ISOC packets will be dropped. > > In patch "usb: dwc2: Change reading of current frame number flow" > reading of current frame number done ASAP in common interrupt handler. > This frame number stored in frame_number variable which used as > starting frame number for ISOC EP's in above mentioned handlers. > > Signed-off-by: Minas Harutyunyan <hminas@synopsys.com> please rebase on top of testing/next after a couple hours: checking file drivers/usb/dwc2/gadget.c Hunk #1 FAILED at 2749. Hunk #2 succeeded at 2772 (offset -1 lines). Hunk #3 FAILED at 2811. 2 out of 3 hunks FAILED
Hi Felipe, This patch should be applied after follow patch from Artur Petrosyan: "[PATCH] usb: dwc2: Change reading of current frame number flow." Thanks, Minas On 7/26/2018 2:45 PM, Felipe Balbi wrote: > Minas Harutyunyan <Minas.Harutyunyan@synopsys.com> writes: > >> To start ISOC transfers in handlers dwc2_gadget_handle_nak() and >> dwc2_gadget_handle_out_token_ep_disabled() driver reads current frame >> number, based on which, set target frame number to start first ISOC >> transfer. >> >> In case if system's high IRQ latency and multiple EP's asserted >> interrupt in same frame, there are high probability that when reading >> current frame number in EP's handlers, actual frame number can be >> increased. As result for bInterval > 1, starting target frame >> will be set wrongly and all ISOC packets will be dropped. >> >> In patch "usb: dwc2: Change reading of current frame number flow" >> reading of current frame number done ASAP in common interrupt handler. >> This frame number stored in frame_number variable which used as >> starting frame number for ISOC EP's in above mentioned handlers. >> >> Signed-off-by: Minas Harutyunyan <hminas@synopsys.com> > > please rebase on top of testing/next after a couple hours: > > checking file drivers/usb/dwc2/gadget.c > Hunk #1 FAILED at 2749. > Hunk #2 succeeded at 2772 (offset -1 lines). > Hunk #3 FAILED at 2811. > 2 out of 3 hunks FAILED > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
No top posting ;) Minas Harutyunyan <Minas.Harutyunyan@synopsys.com> writes: > Hi Felipe, > > This patch should be applied after follow patch from Artur Petrosyan: > > "[PATCH] usb: dwc2: Change reading of current frame number flow." Hasn't this been part of mainline for a while already? commit c7c24e7a047652c558e7aa4b0f54aae3a61aacc4 Author: Artur Petrosyan <Arthur.Petrosyan@synopsys.com> Date: Sat May 5 09:46:26 2018 -0400 usb: dwc2: Change reading of current frame number flow. The current frame_number is read from core for both device and host modes. Reading of the current frame number needs to be performed ASAP due to IRQ latency's. This is why, it is moved to common interrupt handler. Accordingly updated dwc2_gadget_target_frame_elapsed() function which uses stored frame_number instead of reading frame number. In cases when target frame value is incremented the frame_number is required to read again. Signed-off-by: Artur Petrosyan <arturp@synopsys.com> Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c index 676712159980..fedf4dd65cc5 100644 --- a/drivers/usb/dwc2/gadget.c +++ b/drivers/usb/dwc2/gadget.c @@ -2749,23 +2749,16 @@ static void dwc2_gadget_handle_out_token_ep_disabled(struct dwc2_hsotg_ep *ep) struct dwc2_hsotg *hsotg = ep->parent; int dir_in = ep->dir_in; u32 doepmsk; - u32 tmp; if (dir_in || !ep->isochronous) return; - /* - * Store frame in which irq was asserted here, as - * it can change while completing request below. - */ - tmp = dwc2_hsotg_read_frameno(hsotg); - dwc2_hsotg_complete_request(hsotg, ep, get_ep_head(ep), 0); if (using_desc_dma(hsotg)) { if (ep->target_frame == TARGET_FRAME_INITIAL) { /* Start first ISO Out */ - ep->target_frame = tmp; + ep->target_frame = hsotg->frame_number; dwc2_gadget_start_isoc_ddma(ep); } return; @@ -2773,11 +2766,9 @@ static void dwc2_gadget_handle_out_token_ep_disabled(struct dwc2_hsotg_ep *ep) if (ep->interval > 1 && ep->target_frame == TARGET_FRAME_INITIAL) { - u32 dsts; u32 ctrl; - dsts = dwc2_readl(hsotg->regs + DSTS); - ep->target_frame = dwc2_hsotg_read_frameno(hsotg); + ep->target_frame = hsotg->frame_number; dwc2_gadget_incr_frame_num(ep); ctrl = dwc2_readl(hsotg->regs + DOEPCTL(ep->index)); @@ -2813,25 +2804,23 @@ static void dwc2_gadget_handle_nak(struct dwc2_hsotg_ep *hs_ep) { struct dwc2_hsotg *hsotg = hs_ep->parent; int dir_in = hs_ep->dir_in; - u32 tmp; if (!dir_in || !hs_ep->isochronous) return; if (hs_ep->target_frame == TARGET_FRAME_INITIAL) { - tmp = dwc2_hsotg_read_frameno(hsotg); if (using_desc_dma(hsotg)) { dwc2_hsotg_complete_request(hsotg, hs_ep, get_ep_head(hs_ep), 0); - hs_ep->target_frame = tmp; + hs_ep->target_frame = hsotg->frame_number; dwc2_gadget_incr_frame_num(hs_ep); dwc2_gadget_start_isoc_ddma(hs_ep); return; } - hs_ep->target_frame = tmp; + hs_ep->target_frame = hsotg->frame_number; if (hs_ep->interval > 1) { u32 ctrl = dwc2_readl(hsotg->regs + DIEPCTL(hs_ep->index));
To start ISOC transfers in handlers dwc2_gadget_handle_nak() and dwc2_gadget_handle_out_token_ep_disabled() driver reads current frame number, based on which, set target frame number to start first ISOC transfer. In case if system's high IRQ latency and multiple EP's asserted interrupt in same frame, there are high probability that when reading current frame number in EP's handlers, actual frame number can be increased. As result for bInterval > 1, starting target frame will be set wrongly and all ISOC packets will be dropped. In patch "usb: dwc2: Change reading of current frame number flow" reading of current frame number done ASAP in common interrupt handler. This frame number stored in frame_number variable which used as starting frame number for ISOC EP's in above mentioned handlers. Signed-off-by: Minas Harutyunyan <hminas@synopsys.com> --- drivers/usb/dwc2/gadget.c | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-)