Hi Shahar, 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. > >> -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. > > > >> 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__". > >> 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. > >> 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. -- Cheers, Luca. -- 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