Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S967898AbdD0KUu (ORCPT ); Thu, 27 Apr 2017 06:20:50 -0400 Received: from mail-lf0-f66.google.com ([209.85.215.66]:36390 "EHLO mail-lf0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935456AbdD0KUl (ORCPT ); Thu, 27 Apr 2017 06:20:41 -0400 From: "Matwey V. Kornilov" To: b-liu@ti.com, gregkh@linuxfoundation.org Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, "Matwey V. Kornilov" Subject: [PATCH] usb: musb: musb_host: Introduce postponed URB giveback Date: Thu, 27 Apr 2017 13:20:33 +0300 Message-Id: <20170427102033.5500-1-matwey@sai.msu.ru> X-Mailer: git-send-email 2.12.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5045 Lines: 130 This commit changes the order of actions undertaken in musb_advance_schedule() in order to overcome issue with broken isochronous transfer [1]. There is no harm to split musb_giveback into two pieces. The first unlinks finished urb, the second givebacks it. The issue here that givebacking may be quite time-consuming due to urb->complete() call. As it happens in case of pwc-driven web cameras. It may take about 0.5 ms to call __musb_giveback() that calls urb->callback() internally. Under specific circumstances setting MUSB_RXCSR_H_REQPKT in subsequent musb_start_urb() for the next urb will be too late to produce physical IN packet. Since auto req is not used by this module the exchange would be as the following: [ ] 7.220456 d= 0.000997 [182 + 3.667] [ 3] IN : 4.5 [ T ] 7.220459 d= 0.000003 [182 + 7.000] [800] DATA0: [skipped] [ ] 7.222456 d= 0.001997 [184 + 3.667] [ 3] IN : 4.5 [ ] 7.222459 d= 0.000003 [184 + 7.000] [ 3] DATA0: 00 00 It is known that missed IN in isochronous mode makes some perepherial broken. For instance, pwc-driven or uvc-driven web cameras. In order to workaround this issue we postpone calling urb->callback() after setting MUSB_RXCSR_H_REQPKT for the next urb if there is the next urb pending in queue. [1] https://www.spinics.net/lists/linux-usb/msg145747.html Fixes: f551e1352983 ("Revert "usb: musb: musb_host: Enable HCD_BH flag to handle urb return in bottom half"") Signed-off-by: Matwey V. Kornilov --- drivers/usb/musb/musb_host.c | 54 +++++++++++++++++++++++++++++++++++++------- 1 file changed, 46 insertions(+), 8 deletions(-) diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c index ac3a4952abb4..b590c2555dab 100644 --- a/drivers/usb/musb/musb_host.c +++ b/drivers/usb/musb/musb_host.c @@ -299,19 +299,24 @@ musb_start_urb(struct musb *musb, int is_in, struct musb_qh *qh) } } -/* Context: caller owns controller lock, IRQs are blocked */ -static void musb_giveback(struct musb *musb, struct urb *urb, int status) +static void __musb_giveback(struct musb *musb, struct urb *urb, int status) __releases(musb->lock) __acquires(musb->lock) { - trace_musb_urb_gb(musb, urb); - - usb_hcd_unlink_urb_from_ep(musb->hcd, urb); spin_unlock(&musb->lock); usb_hcd_giveback_urb(musb->hcd, urb, status); spin_lock(&musb->lock); } +/* Context: caller owns controller lock, IRQs are blocked */ +static void musb_giveback(struct musb *musb, struct urb *urb, int status) +{ + trace_musb_urb_gb(musb, urb); + + usb_hcd_unlink_urb_from_ep(musb->hcd, urb); + __musb_giveback(musb, urb, status); +} + /* For bulk/interrupt endpoints only */ static inline void musb_save_toggle(struct musb_qh *qh, int is_in, struct urb *urb) @@ -346,6 +351,7 @@ static void musb_advance_schedule(struct musb *musb, struct urb *urb, struct musb_hw_ep *ep = qh->hw_ep; int ready = qh->is_ready; int status; + int postponed_giveback = 0; status = (urb->status == -EINPROGRESS) ? 0 : urb->status; @@ -361,9 +367,35 @@ static void musb_advance_schedule(struct musb *musb, struct urb *urb, break; } - qh->is_ready = 0; - musb_giveback(musb, urb, status); - qh->is_ready = ready; + usb_hcd_unlink_urb_from_ep(musb->hcd, urb); + + /* It may take about 0.5 ms to call __musb_giveback() that + * calls urb->callback() internally. Under specific circumstances + * setting MUSB_RXCSR_H_REQPKT in subsequent musb_start_urb() for the + * next urb will be too late to produce physical IN packet. Since + * auto req is not used by this module the exchange would be as the + * following: + * + * [ ] 7.220456 d= 0.000997 [182 + 3.667] [ 3] IN : 4.5 + * [ T ] 7.220459 d= 0.000003 [182 + 7.000] [800] DATA0: [skipped] + * [ ] 7.222456 d= 0.001997 [184 + 3.667] [ 3] IN : 4.5 + * [ ] 7.222459 d= 0.000003 [184 + 7.000] [ 3] DATA0: 00 00 + * + * It is known that missed IN in isochronous mode makes some + * perepherial broken. For instance, pwc-driven or uvc-driven + * web cameras. + * In order to workaround this issue we postpone calling + * urb->callback() after setting MUSB_RXCSR_H_REQPKT for the + * next urb if there is the next urb pending in queue. + */ + if (is_in && qh->type == USB_ENDPOINT_XFER_ISOC + && !list_empty(&qh->hep->urb_list)) { + postponed_giveback = 1; + } else { + qh->is_ready = 0; + __musb_giveback(musb, urb, status); + qh->is_ready = ready; + } /* reclaim resources (and bandwidth) ASAP; deschedule it, and * invalidate qh as soon as list_empty(&hep->urb_list) @@ -428,6 +460,12 @@ static void musb_advance_schedule(struct musb *musb, struct urb *urb, hw_ep->epnum, is_in ? 'R' : 'T', next_urb(qh)); musb_start_urb(musb, is_in, qh); } + + if (postponed_giveback) { + qh->is_ready = 0; + __musb_giveback(musb, urb, status); + qh->is_ready = ready; + } } static u16 musb_h_flush_rxfifo(struct musb_hw_ep *hw_ep, u16 csr) -- 2.12.0