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


Groups > linux.kernel > #1290233

Re: [PATCH 01/10] usb: host: ehci-sched: refactor scan_isoc function

From "Geyslan G. Bem" <geyslan@gmail.com>
Newsgroups linux.kernel
Subject Re: [PATCH 01/10] usb: host: ehci-sched: refactor scan_isoc function
Date 2015-12-12 18:00 +0100
Message-ID <qEVGy-4Eb-19@gated-at.bofh.it> (permalink)
References <qEHay-3O5-13@gated-at.bofh.it> <qEV3P-4oQ-11@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


2015-12-12 13:15 GMT-03:00 Alan Stern <stern@rowland.harvard.edu>:
> On Fri, 11 Dec 2015, Geyslan G. Bem wrote:
>
>> This patch removes an infinite 'for' loop and makes use of the already
>> existing 'restart' tag instead, 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 <geyslan@gmail.com>
>> ---
>>  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;
>
> These new declarations should line up with the ones above.  And there
> shouldn't be a blank line between.
Ok, I'll fix it.

>
>> +     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))
>
> The style used in this source file does not align continuation lines
> with open parens on the line above.  Instead, the continuation lines
> are indented by 2 extra tabs (or maybe 1 extra if 2 would pushd them
> beyond 80 characters).
My Emacs is configured for linux indentation. I'll fix it manually. Or
have you a ready lisp for this style? :-)
Is there a doc like the Documentation/CodingStyle with this different rules?

>
>> +                             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);
>
> Ditto.
>
>> +                     /*
>> +                      * 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))
>
> Ditto.
>
>> +                             *hw_p = q.itd->hw_next;
>> +                     else
>> +                             *hw_p = cpu_to_hc32(ehci,
>> +                                                 ehci->dummy->qh_dma);
>
> Ditto.
>
>> +             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))) {
>
> Although these lines didn't follow the style used in the rest of the
> file, they should do so now.
Ok.

>
>> +
>> +                             q_p = &q.sitd->sitd_next;
>> +                             hw_p = &q.sitd->hw_next;
>> +                             type = Q_NEXT_TYPE(ehci,
>> +                                                q.sitd->hw_next);
>
> Ditto
>
>> +                      * 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))
>
> Ditto.
>
>> +                             *hw_p = q.sitd->hw_next;
>> +                     else
>> +                             *hw_p = cpu_to_hc32(ehci,
>> +                                                 ehci->dummy->qh_dma);
>
> Ditto.
>
>> +                     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);
>
> Ditto.
>
> Alan Stern
>



-- 
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/

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 00/10] usb: host: ehci-sched: cleanup "Geyslan G. Bem" <geyslan@gmail.com> - 2015-12-12 02:20 +0100
  [PATCH 04/10] usb: host: ehci-sched: add spaces around operators "Geyslan G. Bem" <geyslan@gmail.com> - 2015-12-12 02:30 +0100
  [PATCH 08/10] usb: host: ehci-sched: add line after declarations "Geyslan G. Bem" <geyslan@gmail.com> - 2015-12-12 02:30 +0100
  [PATCH 10/10] usb: host: ehci-sched: remove unnecessary braces "Geyslan G. Bem" <geyslan@gmail.com> - 2015-12-12 02:30 +0100
    Re: [PATCH 10/10] usb: host: ehci-sched: remove unnecessary braces Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2015-12-12 16:00 +0100
      Re: [PATCH 10/10] usb: host: ehci-sched: remove unnecessary braces "Geyslan G. Bem" <geyslan@gmail.com> - 2015-12-12 16:50 +0100
  [PATCH 03/10] usb: host: ehci-sched: remove useless assignments "Geyslan G. Bem" <geyslan@gmail.com> - 2015-12-12 02:30 +0100
    Re: [PATCH 03/10] usb: host: ehci-sched: remove useless assignments Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2015-12-12 15:50 +0100
      Re: [PATCH 03/10] usb: host: ehci-sched: remove useless assignments "Geyslan G. Bem" <geyslan@gmail.com> - 2015-12-12 16:50 +0100
  [PATCH 06/10] usb: host: ehci-sched: remove useless else branch "Geyslan G. Bem" <geyslan@gmail.com> - 2015-12-12 02:30 +0100
  [PATCH 01/10] usb: host: ehci-sched: refactor scan_isoc function "Geyslan G. Bem" <geyslan@gmail.com> - 2015-12-12 02:30 +0100
    Re: [PATCH 01/10] usb: host: ehci-sched: refactor scan_isoc function Alan Stern <stern@rowland.harvard.edu> - 2015-12-12 17:20 +0100
      Re: [PATCH 01/10] usb: host: ehci-sched: refactor scan_isoc function "Geyslan G. Bem" <geyslan@gmail.com> - 2015-12-12 18:00 +0100
        Re: [PATCH 01/10] usb: host: ehci-sched: refactor scan_isoc function Alan Stern <stern@rowland.harvard.edu> - 2015-12-12 18:20 +0100
  [PATCH 07/10] usb: host: ehci-sched: use C89-style comments "Geyslan G. Bem" <geyslan@gmail.com> - 2015-12-12 02:30 +0100
  [PATCH 05/10] usb: host: ehci-sched: remove prohibited spaces "Geyslan G. Bem" <geyslan@gmail.com> - 2015-12-12 02:30 +0100
  [PATCH 09/10] usb: host: ehci-sched: use sizeof operator with parens "Geyslan G. Bem" <geyslan@gmail.com> - 2015-12-12 02:30 +0100
  [PATCH 02/10] usb: host: ehci-sched: move constants to right "Geyslan G. Bem" <geyslan@gmail.com> - 2015-12-12 02:30 +0100

csiph-web