Re: From: Carlos Mart??n <carlos@cmartin.tk>

["Carlos Martín" <carlos@cmartin.tk>]

On Tuesday 21 February 2006 20:26, Christoph Hellwig wrote:
> On Sat, Feb 18, 2006 at 11:35:21PM +0100, Carlos Martin wrote:
> > [PATCH] acxsm: Fix Kconfig option check
> > 
> > This check never actually worked because CONFIG_ACX_{ACX,USB} are
> > tristate. With Adrian Bunk's patch to the Kconfig, this works with the
> > _BOOL hidden Kconfig options.
> > Also update error message adding that this shouldn't happen anymore.
> 
> All the fixes discussed so far are not very nice at all.  The right
> fix is the following:
> 
> 
>  - split the module into three:
> 	acx-common.ko
> 	acx-pci.ko
> 	acx-usb.ko

I don't really like this method. It's not for any real reason though, so I'll 
go with it if people want it. I've used this for the rt2x00 driver by loading 
it manually, which was really annoying, but I suppose when it's properly 
installed it works well.

> 
>  - make CONFIG_ACX_PCI and CONFIG_ACX_USB user-visible tristate
>    varibles in the Kconfig.
>  - build acx-common.ko when either of those is selected, with the
>    following makefile:
> 
> ---- snip ----
> acx-common-y		+= wlan.o conv.o ioctl.o common.o
> acx-pci-y		+= pci.o
> acx-usb-y		+= usb.o
> 
> obj-$(CONFIG_ACX_PCI)	+= acx-common.o acx-pci.o
> obj-$(CONFIG_ACX_USB)	+= acx-common.o acx-usb.o

This is how we had it before, which leads to having a lot of the same code on 
both modules. The unified driver is not much larger so it was made this way, 
and that's where all the problem comes from.

Wouldn't this lead to duplicated function definitions at link time if both are 
compiled-in? From your module split above I understood you wanted acx-common 
to be another module, but here I see it goes into the modules.

> ---- snip ----
> 
>  - kill the IS_PCI/IS_USB macros and add a acx_operations structure that
>    handles the different hardware without branches all over and allows
>    the hw-specific code to be in separate modules.

There aren't that many IS_{PCI,USB} uses and most if not all are justified 
(extra step for one case). It might be a good idea to do that for the 
IS_ACX{100,111} macros instead of calling the generic function which then 
calls the chip-specific one.

   cmn
-- 
Carlos Martín Nieto    |   http://www.cmartin.tk
Hobbyist programmer    |


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid\x103432&bid#0486&dat\x121642