public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Roland Dreier <rdreier@cisco.com>
To: Adrian Bunk <bunk@kernel.org>
Cc: Glenn Streiff <gstreiff@NetEffect.com>,
	Faisal Latif <flatif@NetEffect.com>,
	linux-kernel@vger.kernel.org, general@lists.openfabrics.org
Subject: Re: [ofa-general] [2.6 patch] infiniband/hw/nes/nes_verbs.c: fix off-by-one
Date: Thu, 21 Feb 2008 12:28:55 -0800	[thread overview]
Message-ID: <adahcg2awag.fsf@cisco.com> (raw)
In-Reply-To: <20080221154951.GA28328@cs181133002.pp.htv.fi> (Adrian Bunk's message of "Thu, 21 Feb 2008 17:49:51 +0200")

 > This driver should really have gotten some review before being included 
 > in the kernel.

 > Even a simple checkpatch run finds more than > 250 stylistic errors
 > (not code bugs but cases where the driver violates the standard code 
 > formatting rules of kernel code).

Linus has strongly stated that we should merge hardware drivers early,
and I agree: although the nes driver clearly needs more work, there's
no advantage to users with the hardware in forcing them to wait for
2.6.26 to merge the driver, since they'll just have to patch the
grungy code in themselves anyway.  And by merging the driver early, we
get fixed up for any tree-wide changes and allow janitors to help with
the cleanup.

(By the way, the code is not that pretty but it a lot closer to
upstream style than most driver submissions)

 > And these are just comments from someone with zero knowledge about 
 > InfiniBand, but I'd expect InfiniBand-specifig bugs might be found 
 > before they hit users if an InfiniBand maintainer would review the 
 > complete driver.

Just for the record, although this driver is under drivers/infiniband,
it is actually for a device that does iWARP/10 Gb ethernet.  At some
point we may want to rename drivers/infiniband to drivers/rdma, but so
far the churn hasn't seemed worth it for what is basically a cosmetic
issue.

 - R.

  reply	other threads:[~2008-02-21 20:29 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-19 22:59 [2.6 patch] infiniband/hw/nes/nes_verbs.c: fix off-by-one Adrian Bunk
2008-02-20  4:23 ` [ofa-general] " Roland Dreier
2008-02-20  5:57   ` Adrian Bunk
2008-02-20 23:21     ` Roland Dreier
2008-02-20 23:27       ` Glenn Streiff
2008-02-21 12:39         ` Glenn Streiff
2008-02-21 15:49           ` Adrian Bunk
2008-02-21 20:28             ` Roland Dreier [this message]
2008-02-21 21:01               ` Merging of completely unreviewed drivers Adrian Bunk
2008-02-21 21:09                 ` Roland Dreier
2008-02-21 21:14                 ` Linus Torvalds
2008-02-21 22:33                   ` Alexey Dobriyan
2008-02-21 22:43                     ` Greg KH
2008-02-21 22:57                       ` Jeff Garzik
2008-02-21 22:58                       ` Alexey Dobriyan
2008-02-21 23:31                       ` Jan Engelhardt
2008-02-21 23:38                   ` Krzysztof Halasa
2008-02-21 23:31                     ` Alan Cox
2008-02-22  0:29                       ` Adrian Bunk
2008-02-21 23:41                     ` Jeff Garzik
2008-02-22  0:05                       ` Krzysztof Halasa
2008-02-22  0:44                         ` Jeff Garzik
2008-02-22  2:02                           ` Krzysztof Halasa
2008-02-22 10:04                         ` Alan Cox
2008-02-22 18:45                         ` Pavel Machek
2008-02-22 22:44                           ` Krzysztof Halasa
2008-02-23  9:43                             ` Pavel Machek
2008-02-23 12:38                               ` David Newall
2008-02-23 15:25                                 ` Pavel Machek
2008-02-24  3:18                                   ` David Newall
2008-02-23 17:33                                 ` Linus Torvalds
2008-02-24  3:26                                   ` David Newall
2008-02-24  4:47                                     ` Linus Torvalds
2008-02-23 13:58                               ` Krzysztof Halasa
2008-02-22  1:46                     ` David Newall
2008-02-22  2:06                       ` Al Viro
2008-02-22  2:23                         ` Krzysztof Halasa
2008-02-22  3:13                           ` Al Viro
2008-02-22 22:28                             ` Krzysztof Halasa
2008-02-24  7:47                               ` Jörn Engel
2008-02-24 14:47                                 ` Krzysztof Halasa
2008-02-22  3:13                         ` Linus Torvalds
2008-02-22  6:29                           ` [ofa-general] " Junio C Hamano
2008-02-22  9:02                             ` Adrian Bunk
2008-02-22  6:37                           ` Ray Lee
2008-02-23 15:31                             ` Jan Engelhardt
2008-02-24  3:22                               ` David Newall
2008-02-22 22:37                           ` Krzysztof Halasa
2008-02-22 12:29                       ` [ofa-general] " Bart Van Assche
2008-02-22 14:25                         ` David Newall
2008-02-22 15:17                           ` Peter Zijlstra
2008-02-22 16:48                             ` John W. Linville
2008-02-22 22:59                             ` Krzysztof Halasa
2008-02-22 23:14                               ` Al Viro
2008-02-22 15:48                           ` John W. Linville
2008-02-22 18:54                   ` Ingo Molnar
2008-02-22 19:11                     ` [ofa-general] " Bart Van Assche
2008-02-22 19:20                     ` Jeff Garzik
2008-02-22 19:44                       ` Greg KH
2008-02-21 21:30                 ` Greg KH
2008-02-22  1:06                   ` Adrian Bunk
2008-02-21 22:08                 ` Arjan van de Ven
2008-02-21 22:33                   ` Jeff Garzik
2008-02-21 23:40                     ` Adrian Bunk
2008-02-22 18:40                   ` Pavel Machek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=adahcg2awag.fsf@cisco.com \
    --to=rdreier@cisco.com \
    --cc=bunk@kernel.org \
    --cc=flatif@NetEffect.com \
    --cc=general@lists.openfabrics.org \
    --cc=gstreiff@NetEffect.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox