Search Linux Wireless

Re: [PATCH V2] Add iwlwifi wireless drivers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]
  Powered by Linux