Search Linux Wireless

Re: [PATCH ] wl1271: Change wl12xx Files Names

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

 



On 10/28/2010 07:01 PM, Luciano Coelho wrote:
Hi Shahar,
Hi Luca,

On Thu, 2010-10-28 at 18:16 +0200, ext Shahar Levi wrote:
On 10/27/2010 09:25 PM, Luciano Coelho wrote:
On Tue, 2010-10-26 at 13:18 +0200, ext Shahar Levi wrote:
-config WL1271
-       tristate "TI wl1271 support"
+config WL127X
+       tristate "TI wl127x support"make -j10 modules&&  (make modules_install ARCH=arm INSTALL_MOD_PATH=/home/nfs/ )

Why not use WL12XX and wl12xx here already? Regarding the wl128x stuff,
as I mentioned before, I think it's best if we do the check at runtime
and not in Kconfig.
In case the 128x will check at runtime you right.
Will be fix in v2, if we will agreed otherwise i will revert.

Yes, as I commented to your wl128x patch, I think we should wait until
we can do it at runtime, so no need to differentiate in the
configuration.


+wl12xx_spi-objs                = spi.o
+wl12xx_sdio-objs       = sdio.o

Here...
Will be fix in v2

There's nothing to fix here.  I just pointed out that the modules are
actually called wl12xx_sdio and wl12xx_spi.  So it's correct here
already.
Due to the fact it is confusing spi and sdio i renamed spi.c file name to wl12xx_spi.c (in v2) and in that case i remove those lines.
The same catch for sdio.c

-wl1271-$(CONFIG_NL80211_TESTMODE)      += wl1271_testmode.o
-obj-$(CONFIG_WL1271)   += wl1271.o
-obj-$(CONFIG_WL1271_SPI)       += wl1271_spi.o
-obj-$(CONFIG_WL1271_SDIO)      += wl1271_sdio.o
+wl1271-$(CONFIG_NL80211_TESTMODE)      += testmode.o
+obj-$(CONFIG_WL127X)                   += wl12xx.o
+obj-$(CONFIG_WL12XX_SPI)               += wl12xx_spi.o
+obj-$(CONFIG_WL12XX_SDIO)              += wl12xx_sdio.o

...and here, you're using wl12xx_spi and wl12xx_sdio correctly, which
reinforces my comment about calling the modules "spi.ko" and "sdio.ko"
in the Kconfig section ;)
Will be fix in v2

Again, nothing to fix here.
you right.

diff --git a/drivers/net/wireless/wl12xx/wl1271_conf.h b/drivers/net/wireless/wl12xx/conf.h
similarity index 100%
rename from drivers/net/wireless/wl12xx/wl1271_conf.h
rename to drivers/net/wireless/wl12xx/conf.h

You forgot to change this:

#ifndef __WL1271_CONF_H__

to:

#ifndef __WL12XX_CONF_H__

And the other appearances of __WL1271_CONF_H__ in that file.  This
applies to all header files.
we agreed that change will be in second stage.
not fix for now.

This is a different thing.  These macros are always directly related to
the file name.  Keeping it as __WL1271_CONF_H__ here would be confusing,
since the file is actually called conf.h.  Actually I was wrong when I
said it should be changed to "__WL12XX_CONF_H__" it should be only
"__CONF_H__".
Will be fix in v2

diff --git a/drivers/net/wireless/wl12xx/wl1271_debugfs.h b/drivers/net/wireless/wl12xx/debugfs.h
similarity index 98%
rename from drivers/net/wireless/wl12xx/wl1271_debugfs.h
rename to drivers/net/wireless/wl12xx/debugfs.h
index 00a45b2..d12bf93 100644
--- a/drivers/net/wireless/wl12xx/wl1271_debugfs.h
+++ b/drivers/net/wireless/wl12xx/debugfs.h
@@ -24,7 +24,7 @@
   #ifndef WL1271_DEBUGFS_H
   #define WL1271_DEBUGFS_H

As mentioned above, this #ifndef and #define needs to be changed too.
we agreed that change will be in second stage.
not fix for now.

Same as above.  Please fix all the #ifndef and #define in all header
files to correctly match the file names.
Will be fix in v2

diff --git a/drivers/net/wireless/wl12xx/wl1271_main.c b/drivers/net/wireless/wl12xx/main.c
similarity index 99%
rename from drivers/net/wireless/wl12xx/wl1271_main.c
rename to drivers/net/wireless/wl12xx/main.c
index 63036b5..dab10a5 100644
--- a/drivers/net/wireless/wl12xx/wl1271_main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -31,20 +31,20 @@

[...]

   #define WL1271_BOOT_RETRIES 3

Did we agree not to change this stuff for now? Yes, now I remember, it's
better to do it in two steps indeed (ie. do the other changes in a
separate patch).  But I'd rather apply all the patches add once.
I believe that patch could stand alone. There isn't any connection
between files names and function+defines names.

Yes, no need to change these macros or function names in this patch.
In that case it can be apply alon, is it? ;-)

--
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