[PATCH] cw1200: fix some obvious mistakes
From: Arnd Bergmann
Date: Sat Jun 01 2013 - 18:37:58 EST
I got compile errors with the cw1200, which has lead me to take a
closer look. It seems the driver still has a number of issues,
and this addresses some of them and marks others as FIXME:
* The cw1200_sagrad.c file should not be there, hardcoding
hardware configuration in .c files has been obsolete since
Linux-2.4, so we should just remove it. Most of that file
was already commented out since it does not compile.
* Move the parts of cw1200_sagrad.c that actually got used into
cw1200_sdio.c, because it doesn't build otherwise.
* Make the platform_data for the sdio driver private to
cw1200_sdio.c. The platform that was referenced here is
going to use device tree based probing only in the future,
which means the platform data has to go away anyway.
* Move the platform_data for the spi driver to
include/linux/platform_data/net-cw1200.h where all other
drivers have their platform_data.
* Add comments about passing GPIO numbers in platform_data:
You should not use IORESOURCE_IO, which is for legacy ISA
I/O ports on PCs, not for GPIOs.
It may actually be easier to remove the sdio driver entirely,
since it clearly doesn't work and requires a lot of cleanup.
Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
Cc: Solomon Peachy <pizza@xxxxxxxxxxxx>
Cc: John W. Linville <linville@xxxxxxxxxxxxx>
Cc: Wei Yongjun <yongjun_wei@xxxxxxxxxxxxxxxxx>
Cc: Dmitry Tarnyagin <dmitry.tarnyagin@xxxxxxxxxxx>
Cc: Ajitpal Singh <ajitpal.singh@xxxxxxxxxxxxxx>
Cc: linux-wireless@xxxxxxxxxxxxxxx
---
drivers/net/wireless/cw1200/Kconfig | 8 --
drivers/net/wireless/cw1200/Makefile | 2 -
drivers/net/wireless/cw1200/cw1200_sagrad.c | 145 ---------------------
drivers/net/wireless/cw1200/cw1200_sdio.c | 72 ++++++++--
drivers/net/wireless/cw1200/cw1200_spi.c | 2 +-
.../net-cw1200.h} | 20 +--
6 files changed, 62 insertions(+), 187 deletions(-)
delete mode 100644 drivers/net/wireless/cw1200/cw1200_sagrad.c
rename include/linux/{cw1200_platform.h => platform_data/net-cw1200.h} (53%)
diff --git a/drivers/net/wireless/cw1200/Kconfig b/drivers/net/wireless/cw1200/Kconfig
index 13e3611..c3142d4 100644
--- a/drivers/net/wireless/cw1200/Kconfig
+++ b/drivers/net/wireless/cw1200/Kconfig
@@ -20,14 +20,6 @@ config CW1200_WLAN_SPI
help
Enables support for the CW1200 connected via a SPI bus.
-config CW1200_WLAN_SAGRAD
- tristate "Support Sagrad SG901-1091/1098 modules"
- depends on CW1200_WLAN_SDIO
- help
- This provides the platform data glue to support the
- Sagrad SG901-1091/1098 modules in their standard SDIO EVK.
- It also includes example SPI platform data.
-
menu "Driver debug features"
depends on CW1200 && DEBUG_FS
diff --git a/drivers/net/wireless/cw1200/Makefile b/drivers/net/wireless/cw1200/Makefile
index 1aa3682..bc6cbf9 100644
--- a/drivers/net/wireless/cw1200/Makefile
+++ b/drivers/net/wireless/cw1200/Makefile
@@ -16,9 +16,7 @@ cw1200_core-$(CONFIG_PM) += pm.o
cw1200_wlan_sdio-y := cw1200_sdio.o
cw1200_wlan_spi-y := cw1200_spi.o
-cw1200_wlan_sagrad-y := cw1200_sagrad.o
obj-$(CONFIG_CW1200) += cw1200_core.o
obj-$(CONFIG_CW1200_WLAN_SDIO) += cw1200_wlan_sdio.o
obj-$(CONFIG_CW1200_WLAN_SPI) += cw1200_wlan_spi.o
-obj-$(CONFIG_CW1200_WLAN_SAGRAD) += cw1200_wlan_sagrad.o
diff --git a/drivers/net/wireless/cw1200/cw1200_sagrad.c b/drivers/net/wireless/cw1200/cw1200_sagrad.c
deleted file mode 100644
index a5ada0e..0000000
--- a/drivers/net/wireless/cw1200/cw1200_sagrad.c
+++ /dev/null
@@ -1,145 +0,0 @@
-/*
- * Platform glue data for ST-Ericsson CW1200 driver
- *
- * Copyright (c) 2013, Sagrad, Inc
- * Author: Solomon Peachy <speachy@xxxxxxxxxx>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-
-#include <linux/module.h>
-#include <linux/cw1200_platform.h>
-
-MODULE_AUTHOR("Solomon Peachy <speachy@xxxxxxxxxx>");
-MODULE_DESCRIPTION("ST-Ericsson CW1200 Platform glue driver");
-MODULE_LICENSE("GPL");
-
-/* Define just one of these. Feel free to customize as needed */
-#define SAGRAD_1091_1098_EVK_SDIO
-/* #define SAGRAD_1091_1098_EVK_SPI */
-
-#ifdef SAGRAD_1091_1098_EVK_SDIO
-#if 0
-static struct resource cw1200_href_resources[] = {
- {
- .start = 215, /* fix me as appropriate */
- .end = 215, /* ditto */
- .flags = IORESOURCE_IO,
- .name = "cw1200_wlan_reset",
- },
- {
- .start = 216, /* fix me as appropriate */
- .end = 216, /* ditto */
- .flags = IORESOURCE_IO,
- .name = "cw1200_wlan_powerup",
- },
- {
- .start = NOMADIK_GPIO_TO_IRQ(216), /* fix me as appropriate */
- .end = NOMADIK_GPIO_TO_IRQ(216), /* ditto */
- .flags = IORESOURCE_IRQ,
- .name = "cw1200_wlan_irq",
- },
-};
-#endif
-
-static int cw1200_power_ctrl(const struct cw1200_platform_data_sdio *pdata,
- bool enable)
-{
- /* Control 3v3 and 1v8 to hardware as appropriate */
- /* Note this is not needed if it's controlled elsewhere or always on */
-
- /* May require delay for power to stabilize */
- return 0;
-}
-
-static int cw1200_clk_ctrl(const struct cw1200_platform_data_sdio *pdata,
- bool enable)
-{
- /* Turn CLK_32K off and on as appropriate. */
- /* Note this is not needed if it's always on */
-
- /* May require delay for clock to stabilize */
- return 0;
-}
-
-static struct cw1200_platform_data_sdio cw1200_platform_data = {
- .ref_clk = 38400,
- .have_5ghz = false,
-#if 0
- .reset = &cw1200_href_resources[0],
- .powerup = &cw1200_href_resources[1],
- .irq = &cw1200_href_resources[2],
-#endif
- .power_ctrl = cw1200_power_ctrl,
- .clk_ctrl = cw1200_clk_ctrl,
-/* .macaddr = ??? */
- .sdd_file = "sdd_sagrad_1091_1098.bin",
-};
-#endif
-
-#ifdef SAGRAD_1091_1098_EVK_SPI
-/* Note that this is an example of integrating into your board support file */
-static struct resource cw1200_href_resources[] = {
- {
- .start = GPIO_RF_RESET,
- .end = GPIO_RF_RESET,
- .flags = IORESOURCE_IO,
- .name = "cw1200_wlan_reset",
- },
- {
- .start = GPIO_RF_POWERUP,
- .end = GPIO_RF_POWERUP,
- .flags = IORESOURCE_IO,
- .name = "cw1200_wlan_powerup",
- },
-};
-
-static int cw1200_power_ctrl(const struct cw1200_platform_data_spi *pdata,
- bool enable)
-{
- /* Control 3v3 and 1v8 to hardware as appropriate */
- /* Note this is not needed if it's controlled elsewhere or always on */
-
- /* May require delay for power to stabilize */
- return 0;
-}
-static int cw1200_clk_ctrl(const struct cw1200_platform_data_spi *pdata,
- bool enable)
-{
- /* Turn CLK_32K off and on as appropriate. */
- /* Note this is not needed if it's always on */
-
- /* May require delay for clock to stabilize */
- return 0;
-}
-
-static struct cw1200_platform_data_spi cw1200_platform_data = {
- .ref_clk = 38400,
- .spi_bits_per_word = 16,
- .reset = &cw1200_href_resources[0],
- .powerup = &cw1200_href_resources[1],
- .power_ctrl = cw1200_power_ctrl,
- .clk_ctrl = cw1200_clk_ctrl,
-/* .macaddr = ??? */
- .sdd_file = "sdd_sagrad_1091_1098.bin",
-};
-static struct spi_board_info myboard_spi_devices[] __initdata = {
- {
- .modalias = "cw1200_wlan_spi",
- .max_speed_hz = 10000000, /* 52MHz Max */
- .bus_num = 0,
- .irq = WIFI_IRQ,
- .platform_data = &cw1200_platform_data,
- .chip_select = 0,
- },
-};
-#endif
-
-
-const void *cw1200_get_platform_data(void)
-{
- return &cw1200_platform_data;
-}
-EXPORT_SYMBOL_GPL(cw1200_get_platform_data);
diff --git a/drivers/net/wireless/cw1200/cw1200_sdio.c b/drivers/net/wireless/cw1200/cw1200_sdio.c
index 863510d..721e392 100644
--- a/drivers/net/wireless/cw1200/cw1200_sdio.c
+++ b/drivers/net/wireless/cw1200/cw1200_sdio.c
@@ -20,7 +20,6 @@
#include "cw1200.h"
#include "sbus.h"
-#include <linux/cw1200_platform.h>
#include "hwio.h"
MODULE_AUTHOR("Dmitry Tarnyagin <dmitry.tarnyagin@xxxxxxxxxxx>");
@@ -29,6 +28,27 @@ MODULE_LICENSE("GPL");
#define SDIO_BLOCK_SIZE (512)
+/* FIXME: include this into sbus_priv */
+struct cw1200_platform_data_sdio {
+ u16 ref_clk; /* REQUIRED (in KHz) */
+
+ /* All others are optional */
+ /* FIXME: just do gpio_to_irq */
+ const struct resource *irq; /* if using GPIO for IRQ */
+ bool have_5ghz;
+ bool no_nptb; /* SDIO hardware does not support non-power-of-2-blocksizes */
+ /* FIXME: GPIO numbers are not resources */
+ const struct resource *reset; /* GPIO to RSTn signal */
+ const struct resource *powerup; /* GPIO to POWERUP signal */
+ int (*power_ctrl)(const struct cw1200_platform_data_sdio *pdata,
+ bool enable); /* Control 3v3 / 1v8 supply */
+ int (*clk_ctrl)(const struct cw1200_platform_data_sdio *pdata,
+ bool enable); /* Control CLK32K */
+ /* FIXME: us of_get_mac_address */
+ const u8 *macaddr; /* if NULL, use cw1200_mac_template module parameter */
+ const char *sdd_file; /* if NULL, will use default for detected hw type */
+};
+
struct sbus_priv {
struct sdio_func *func;
struct cw1200_common *core;
@@ -265,6 +285,38 @@ static struct sbus_ops cw1200_sdio_sbus_ops = {
.power_mgmt = cw1200_sdio_pm,
};
+static int cw1200_power_ctrl(const struct cw1200_platform_data_sdio *pdata,
+ bool enable)
+{
+ /* Control 3v3 and 1v8 to hardware as appropriate */
+ /* Note this is not needed if it's controlled elsewhere or always on */
+
+ /* May require delay for power to stabilize */
+ return 0;
+}
+
+static int cw1200_clk_ctrl(const struct cw1200_platform_data_sdio *pdata,
+ bool enable)
+{
+ /* Turn CLK_32K off and on as appropriate. */
+ /* Note this is not needed if it's always on */
+
+ /* May require delay for clock to stabilize */
+ return 0;
+}
+
+/*
+ * FIXME: These are just defaults, the driver needs a proper DT binding
+ * to extract the other values and override these if necessary
+ */
+static struct cw1200_platform_data_sdio cw1200_platform_data = {
+ .ref_clk = 38400,
+ .have_5ghz = false,
+ .power_ctrl = cw1200_power_ctrl,
+ .clk_ctrl = cw1200_clk_ctrl,
+ .sdd_file = "sdd_sagrad_1091_1098.bin",
+};
+
/* Probe Function to be called by SDIO stack when device is discovered */
static int cw1200_sdio_probe(struct sdio_func *func,
const struct sdio_device_id *id)
@@ -286,7 +338,8 @@ static int cw1200_sdio_probe(struct sdio_func *func,
func->card->quirks |= MMC_QUIRK_LENIENT_FN0;
- self->pdata = cw1200_get_platform_data();
+ /* FIXME: get this from DT */
+ self->pdata = &cw1200_platform_data;
self->func = func;
sdio_set_drvdata(func, self);
sdio_claim_host(func);
@@ -377,13 +430,11 @@ static struct sdio_driver sdio_driver = {
/* Init Module function -> Called by insmod */
static int __init cw1200_sdio_init(void)
{
- const struct cw1200_platform_data_sdio *pdata;
int ret;
- pdata = cw1200_get_platform_data();
-
- if (cw1200_sdio_on(pdata)) {
- ret = -1;
+ /* FIXME: must not touch hardware until we know the hardware is present */
+ if (cw1200_sdio_on(&cw1200_platform_data)) {
+ ret = -ENODEV;
goto err;
}
@@ -394,19 +445,16 @@ static int __init cw1200_sdio_init(void)
return 0;
err:
- cw1200_sdio_off(pdata);
+ cw1200_sdio_off(&cw1200_platform_data);
return ret;
}
/* Called at Driver Unloading */
static void __exit cw1200_sdio_exit(void)
{
- const struct cw1200_platform_data_sdio *pdata;
- pdata = cw1200_get_platform_data();
sdio_unregister_driver(&sdio_driver);
- cw1200_sdio_off(pdata);
+ cw1200_sdio_off(&cw1200_platform_data);
}
-
module_init(cw1200_sdio_init);
module_exit(cw1200_sdio_exit);
diff --git a/drivers/net/wireless/cw1200/cw1200_spi.c b/drivers/net/wireless/cw1200/cw1200_spi.c
index 75adef0..ef05916 100644
--- a/drivers/net/wireless/cw1200/cw1200_spi.c
+++ b/drivers/net/wireless/cw1200/cw1200_spi.c
@@ -25,7 +25,7 @@
#include "cw1200.h"
#include "sbus.h"
-#include <linux/cw1200_platform.h>
+#include <linux/platform_data/net-cw1200.h>
#include "hwio.h"
MODULE_AUTHOR("Solomon Peachy <speachy@xxxxxxxxxx>");
diff --git a/include/linux/cw1200_platform.h b/include/linux/platform_data/net-cw1200.h
similarity index 53%
rename from include/linux/cw1200_platform.h
rename to include/linux/platform_data/net-cw1200.h
index c168fa5..8f006ac 100644
--- a/include/linux/cw1200_platform.h
+++ b/include/linux/platform_data/net-cw1200.h
@@ -14,6 +14,7 @@ struct cw1200_platform_data_spi {
/* All others are optional */
bool have_5ghz;
+ /* FIXME: use simple numbers rather than resources for GPIO */
const struct resource *reset; /* GPIO to RSTn signal */
const struct resource *powerup; /* GPIO to POWERUP signal */
int (*power_ctrl)(const struct cw1200_platform_data_spi *pdata,
@@ -24,23 +25,4 @@ struct cw1200_platform_data_spi {
const char *sdd_file; /* if NULL, will use default for detected hw type */
};
-struct cw1200_platform_data_sdio {
- u16 ref_clk; /* REQUIRED (in KHz) */
-
- /* All others are optional */
- const struct resource *irq; /* if using GPIO for IRQ */
- bool have_5ghz;
- bool no_nptb; /* SDIO hardware does not support non-power-of-2-blocksizes */
- const struct resource *reset; /* GPIO to RSTn signal */
- const struct resource *powerup; /* GPIO to POWERUP signal */
- int (*power_ctrl)(const struct cw1200_platform_data_sdio *pdata,
- bool enable); /* Control 3v3 / 1v8 supply */
- int (*clk_ctrl)(const struct cw1200_platform_data_sdio *pdata,
- bool enable); /* Control CLK32K */
- const u8 *macaddr; /* if NULL, use cw1200_mac_template module parameter */
- const char *sdd_file; /* if NULL, will use default for detected hw type */
-};
-
-const void *cw1200_get_platform_data(void);
-
#endif /* CW1200_PLAT_H_INCLUDED */
--
1.8.1.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/