On Mon, 2007-08-27 at 14:10 +0100, Christoph Hellwig wrote: > Btw, strong NACK from me until you sort out the mess with the common > files. Including C files with cpp symbols defined or not is not how we > do driver development in Linux. Please split out really common code > into common files, and build driver specific code into files of it's > own. Having a slight amount of duplicated code is much better than > having such a mess for maintaince. It's not a "slight amount" of duplicated code. $ expr `grep -e '^{$' iwl-base.c |wc -l` - ` sed -e '/^{$/,/^}$/{/#if IWL == /,/^}$/d}' iwl-base.c |grep -e '^}$' |wc -l` 34 Additional 34 functions have to be splited out from iwl-base.c into their own hardware specific files. If you look at the code, most of these functions are only with little difference which are caused by the difference of the hardware. In the future, when we add new hardware support for iwlwifi, it's more maintainable to extend the current functions with new hardware difference than copy all the 34 functions into a new file and only make slight changes to each of them. I know the method is not common used in the Linux drivers, but this is decided by the hardware layout. For 3945 and 4965, 90% of the hardware/firmware layout are the same and only 10% are different. This makes it possible to support two slightly differs hardwares in one driver. If the hardwares differ a lot (i.e 2100 and 2200), I won't even think about to do it in this way. Thanks, -yi - To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html