public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <greg@kroah.com>
To: KY Srinivasan <kys@microsoft.com>
Cc: "gregkh@suse.de" <gregkh@suse.de>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"devel@linuxdriverproject.org" <devel@linuxdriverproject.org>,
	"virtualization@lists.osdl.org" <virtualization@lists.osdl.org>
Subject: Re: various vmbus review comments
Date: Wed, 4 May 2011 11:28:55 -0700	[thread overview]
Message-ID: <20110504182855.GB8911@kroah.com> (raw)
In-Reply-To: <6E21E5352C11B742B20C142EB499E0481DFD6B@TK5EX14MBXC124.redmond.corp.microsoft.com>

On Wed, May 04, 2011 at 04:58:39PM +0000, KY Srinivasan wrote:
> > > 2) hyperv_storage.h - this has all the definitions needed to build storage
> > > drivers for Hyper-V. Storage drivers will include hyperv.h and
> > > hyperv_storage.h.
> > >
> > > 3) hyperv_net.h - this has all the definitions needed to build the network
> > > driver for Hyper-V. The netvsc driver will include hyperv.h and hyperv_net.h.
> > >
> > > 4) hyperv_utils.h - this has all the definitions needed to build the util driver.
> > > The util driver would include hyperv.h and hyperv_utils.h.
> > 
> > No for all 3 of these.  Why would a simple driver need a .h file at all?
> > Just include the structures within the .c file, making it nice and
> > self-contained.  There's no need to clutter things up, and individual
> > driver .h files should _never_ be in include/linux.
> 
> Agreed - we don't need to clutter up  include/linux directory with individual
> driver.h files. However, including all the definitions in the .c file would also be
> ugly. For instance there are multiple *.c files for the storage drivers and we don't
> want to replicate the contents of  hyperv_storage.h in each of these .c files.
> The same is true for the netvsc driver - there are multiple .c files where we would need
> to replicate the contents of hyperv_net.h. 
> 
> How about I keep these driver specific header files; but these could be local to the 
> directory where these drivers land.

Yes, that is fine.  I still find it odd that you need multiple files for
the network driver.  We have much more complex and longer drivers for
common cards these days than this one, all in one file.  But that's for
you and the network developers to argue over :)

thanks,

greg k-h

  reply	other threads:[~2011-05-04 18:29 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-03 20:46 various vmbus review comments Greg KH
2011-05-03 21:00 ` KY Srinivasan
2011-05-03 22:03   ` Greg KH
2011-05-03 22:49     ` KY Srinivasan
2011-05-04 16:20 ` KY Srinivasan
2011-05-04 16:32   ` Greg KH
2011-05-04 16:58     ` KY Srinivasan
2011-05-04 18:28       ` Greg KH [this message]
2011-05-06 13:10     ` KY Srinivasan
2011-05-06 14:59       ` Greg KH
2011-05-06 17:34         ` KY Srinivasan
2011-05-09 14:33       ` Christoph Hellwig
2011-05-09 14:56         ` KY Srinivasan
2011-05-10  5:24           ` Christoph Hellwig
2011-05-10 13:00             ` KY Srinivasan
2011-05-10 13:07               ` Christoph Hellwig
2011-05-10 13:16               ` Greg KH
2011-05-09  1:46 ` KY Srinivasan
2011-05-09  3:04   ` Greg KH
2011-05-09 12:35     ` KY Srinivasan

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=20110504182855.GB8911@kroah.com \
    --to=greg@kroah.com \
    --cc=devel@linuxdriverproject.org \
    --cc=gregkh@suse.de \
    --cc=kys@microsoft.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=virtualization@lists.osdl.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