Re: [PATCH 1/9] NetEffect 10Gb RNIC Driver: kernel Kconfig and makefiles

[Roland Dreier <rdreier@cisco.com>]

 > +source "drivers/infiniband/hw/nes/Kconfig"
 > +
 >  source "drivers/infiniband/hw/cxgb3/Kconfig"

This patch seems to be against some non-standard tree, since cxgb3
isn't upstream yet.  And if cxgb3 were already upstream, it might be
polite to add yourself after it rather than before ;)

 > +config INFINIBAND_NES_DEBUG
 > +	bool "Verbose debugging output"
 > +	depends on INFINIBAND_NES
 > +	default n
 > +	---help---
 > +	  This option causes the NetEffect RNIC driver to produce debug
 > +	  messages.  Select this if you are developing the driver
 > +	  or trying to diagnose a problem.

I recommend making this option invisible unless EMBEDDED is set, and
having the default be 'y', and making your debugging level changeable
at run-time.  That way everyone (in particular distros) will have this
turned on and you'll be able to figure out problems without making
end-users rebuild a kernel.

 > +EXTRA_CFLAGS += -Idrivers/infiniband/include

Not needed in the kernel tree.

 > -Idrivers/infiniband/hw/nes/nes_tcpip/include

I guess this is the mysterious TCP stack module.  Anyway if you need
this in the end, I would suggest removing the C flag and using
#include "nes_tcpip/blah.h" in your source.

 > +ifdef CONFIG_INFINIBAND_NES_DEBUG
 > +EXTRA_CFLAGS += -DNES_DEBUG
 > +endif

There's no point to this -- just test CONFIG_INFINIBAND_NES_DEBUG directly.

 > +ifneq ($(KERNELRELEASE),)
 > +	obj-$(CONFIG_INFINIBAND_NES) += iw_nes.o	
 > +
 > +	iw_nes-objs := \
 > +		nes.o \
 > +		nes_hw.o \
 > +		nes_nic.o \
 > +		nes_cm.o \
 > +		nes_utils.o \
 > +		nes_verbs.o 
 > +else

This should be your whole Makefile -- we're not going to merge stuff
into the kernel tree to build your module out of the kernel tree.
Also it's more idiomatic to put all your component objects onto one
(or a few) lines.

 - R.