Re: various vmbus review comments

From: Greg KH
Date: Wed May 04 2011 - 14:29:11 EST


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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/