Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754002AbbLKU1U (ORCPT ); Fri, 11 Dec 2015 15:27:20 -0500 Received: from mail-vk0-f45.google.com ([209.85.213.45]:35723 "EHLO mail-vk0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752869AbbLKU1S (ORCPT ); Fri, 11 Dec 2015 15:27:18 -0500 MIME-Version: 1.0 In-Reply-To: <1449795349-1204-1-git-send-email-geyslan@gmail.com> References: <1449795349-1204-1-git-send-email-geyslan@gmail.com> Date: Fri, 11 Dec 2015 17:27:17 -0300 Message-ID: Subject: Re: [PATCH v2] usb: ehci: refactor scan_isoc function From: "Geyslan G. Bem" To: Peter Senna Tschudin Cc: "Geyslan G. Bem" , Alan Stern , Greg Kroah-Hartman , linux-usb@vger.kernel.org, LKML Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 12846 Lines: 268 2015-12-10 21:55 GMT-03:00 Geyslan G. Bem : > This patch removes an infinite for loop and makes use of the already > existing 'restart' tag, reducing one leading tab. > > The comments and code were corrected conforming coding style. > > Tested by compilation only. > Caught by checkpatch: > WARNING: Too many leading tabs - consider code refactoring > > Signed-off-by: Geyslan G. Bem > --- > v2: fixes coding style (removes spaces before parens and open brackets) > --- > drivers/usb/host/ehci-sched.c | 203 ++++++++++++++++++++++-------------------- > 1 file changed, 104 insertions(+), 99 deletions(-) > > diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c > index f9a3327..86b2484 100644 > --- a/drivers/usb/host/ehci-sched.c > +++ b/drivers/usb/host/ehci-sched.c > @@ -2383,6 +2383,9 @@ static void scan_isoc(struct ehci_hcd *ehci) > unsigned fmask = ehci->periodic_size - 1; > bool modified, live; > > + union ehci_shadow q, *q_p; > + __hc32 type, *hw_p; > + > /* > * When running, scan from last scan point up to "now" > * else clean up by scanning everything that's left. > @@ -2399,119 +2402,121 @@ static void scan_isoc(struct ehci_hcd *ehci) > ehci->now_frame = now_frame; > > frame = ehci->last_iso_frame; > - for (;;) { > - union ehci_shadow q, *q_p; > - __hc32 type, *hw_p; > > restart: > - /* scan each element in frame's queue for completions */ > - q_p = &ehci->pshadow [frame]; > - hw_p = &ehci->periodic [frame]; > - q.ptr = q_p->ptr; > - type = Q_NEXT_TYPE(ehci, *hw_p); > - modified = false; > - > - while (q.ptr != NULL) { > - switch (hc32_to_cpu(ehci, type)) { > - case Q_TYPE_ITD: > - /* If this ITD is still active, leave it for > - * later processing ... check the next entry. > - * No need to check for activity unless the > - * frame is current. > - */ > - if (frame == now_frame && live) { > - rmb(); > - for (uf = 0; uf < 8; uf++) { > - if (q.itd->hw_transaction[uf] & > - ITD_ACTIVE(ehci)) > - break; > - } > - if (uf < 8) { > - q_p = &q.itd->itd_next; > - hw_p = &q.itd->hw_next; > - type = Q_NEXT_TYPE(ehci, > - q.itd->hw_next); > - q = *q_p; > + /* Scan each element in frame's queue for completions */ > + q_p = &ehci->pshadow[frame]; > + hw_p = &ehci->periodic[frame]; > + q.ptr = q_p->ptr; > + type = Q_NEXT_TYPE(ehci, *hw_p); > + modified = false; > + > + while (q.ptr != NULL) { > + switch (hc32_to_cpu(ehci, type)) { > + case Q_TYPE_ITD: > + /* > + * If this ITD is still active, leave it for > + * later processing ... check the next entry. > + * No need to check for activity unless the > + * frame is current. > + */ > + if (frame == now_frame && live) { > + rmb(); > + for (uf = 0; uf < 8; uf++) { > + if (q.itd->hw_transaction[uf] & > + ITD_ACTIVE(ehci)) > break; > - } > } > - > - /* Take finished ITDs out of the schedule > - * and process them: recycle, maybe report > - * URB completion. HC won't cache the > - * pointer for much longer, if at all. > - */ > - *q_p = q.itd->itd_next; > - if (!ehci->use_dummy_qh || > - q.itd->hw_next != EHCI_LIST_END(ehci)) > - *hw_p = q.itd->hw_next; > - else > - *hw_p = cpu_to_hc32(ehci, > - ehci->dummy->qh_dma); > - type = Q_NEXT_TYPE(ehci, q.itd->hw_next); > - wmb(); > - modified = itd_complete (ehci, q.itd); > - q = *q_p; > - break; > - case Q_TYPE_SITD: > - /* If this SITD is still active, leave it for > - * later processing ... check the next entry. > - * No need to check for activity unless the > - * frame is current. > - */ > - if (((frame == now_frame) || > - (((frame + 1) & fmask) == now_frame)) > - && live > - && (q.sitd->hw_results & > - SITD_ACTIVE(ehci))) { > - > - q_p = &q.sitd->sitd_next; > - hw_p = &q.sitd->hw_next; > + if (uf < 8) { > + q_p = &q.itd->itd_next; > + hw_p = &q.itd->hw_next; > type = Q_NEXT_TYPE(ehci, > - q.sitd->hw_next); > + q.itd->hw_next); > q = *q_p; > break; > } > + } > > - /* Take finished SITDs out of the schedule > - * and process them: recycle, maybe report > - * URB completion. > - */ > - *q_p = q.sitd->sitd_next; > - if (!ehci->use_dummy_qh || > - q.sitd->hw_next != EHCI_LIST_END(ehci)) > - *hw_p = q.sitd->hw_next; > - else > - *hw_p = cpu_to_hc32(ehci, > - ehci->dummy->qh_dma); > - type = Q_NEXT_TYPE(ehci, q.sitd->hw_next); > - wmb(); > - modified = sitd_complete (ehci, q.sitd); > + /* > + * Take finished ITDs out of the schedule > + * and process them: recycle, maybe report > + * URB completion. HC won't cache the > + * pointer for much longer, if at all. > + */ > + *q_p = q.itd->itd_next; > + if (!ehci->use_dummy_qh || > + q.itd->hw_next != EHCI_LIST_END(ehci)) > + *hw_p = q.itd->hw_next; > + else > + *hw_p = cpu_to_hc32(ehci, > + ehci->dummy->qh_dma); > + type = Q_NEXT_TYPE(ehci, q.itd->hw_next); > + wmb(); > + modified = itd_complete(ehci, q.itd); > + q = *q_p; > + break; > + case Q_TYPE_SITD: > + /* > + * If this SITD is still active, leave it for > + * later processing ... check the next entry. > + * No need to check for activity unless the > + * frame is current. > + */ > + if (((frame == now_frame) || > + (((frame + 1) & fmask) == now_frame)) > + && live > + && (q.sitd->hw_results & > + SITD_ACTIVE(ehci))) { > + > + q_p = &q.sitd->sitd_next; > + hw_p = &q.sitd->hw_next; > + type = Q_NEXT_TYPE(ehci, > + q.sitd->hw_next); > q = *q_p; > break; > - default: > - ehci_dbg(ehci, "corrupt type %d frame %d shadow %p\n", > - type, frame, q.ptr); > - // BUG (); > - /* FALL THROUGH */ > - case Q_TYPE_QH: > - case Q_TYPE_FSTN: > - /* End of the iTDs and siTDs */ > - q.ptr = NULL; > - break; > } > > - /* assume completion callbacks modify the queue */ > - if (unlikely(modified && ehci->isoc_count > 0)) > - goto restart; > - } > - > - /* Stop when we have reached the current frame */ > - if (frame == now_frame) > + /* > + * Take finished SITDs out of the schedule > + * and process them: recycle, maybe report > + * URB completion. > + */ > + *q_p = q.sitd->sitd_next; > + if (!ehci->use_dummy_qh || > + q.sitd->hw_next != EHCI_LIST_END(ehci)) > + *hw_p = q.sitd->hw_next; > + else > + *hw_p = cpu_to_hc32(ehci, > + ehci->dummy->qh_dma); > + type = Q_NEXT_TYPE(ehci, q.sitd->hw_next); > + wmb(); > + modified = sitd_complete(ehci, q.sitd); > + q = *q_p; > break; > + default: > + ehci_dbg(ehci, "corrupt type %d frame %d shadow %p\n", > + type, frame, q.ptr); > + /* BUG(); */ > + /* FALL THROUGH */ > + case Q_TYPE_QH: > + case Q_TYPE_FSTN: > + /* End of the iTDs and siTDs */ > + q.ptr = NULL; > + break; > + } > > - /* The last frame may still have active siTDs */ > - ehci->last_iso_frame = frame; > - frame = (frame + 1) & fmask; > + /* Assume completion callbacks modify the queue */ > + if (unlikely(modified && ehci->isoc_count > 0)) > + goto restart; > } > + > + /* Stop when we have reached the current frame */ > + if (frame == now_frame) > + return; > + > + /* The last frame may still have active siTDs */ > + ehci->last_iso_frame = frame; > + frame = (frame + 1) & fmask; > + > + goto restart; > } > -- > 2.6.3 > I'm doing a cleanup in this file. It's better to wait the whole pack of patches. -- Regards, Geyslan G. Bem hackingbits.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/