All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC/PATCH 00/13] wl12xx re-factor
@ 2011-05-13 21:26 Felipe Balbi
  2011-05-13 21:26 ` [RFC/PATCH 01/13] net: wl12xx: sdio: id_tables should be __devinitconst Felipe Balbi
                   ` (13 more replies)
  0 siblings, 14 replies; 32+ messages in thread
From: Felipe Balbi @ 2011-05-13 21:26 UTC (permalink / raw)
  To: Luciano Coelho; +Cc: linux-wireless, netdev, linux-kernel, Felipe Balbi

Hi Luca,

this is the re-factor I was talking to you
about. Please have a look and give your
comments.

It probably won't work as is, I compile
tested only, but it shows the idea.

Felipe Balbi (13):
  net: wl12xx: sdio: id_tables should be __devinitconst
  net: wl12xx: sdio: add a context structure
  net: wl12xx: remove some unnecessary prints
  net: wl12xx: care for optional operations
  net: wl12xx: remove the nops
  net: wl12xx: remove unnecessary prints
  net: wl12xx: spi: add a context structure
  net: wl12xx: spi: add a platform_device
  net: wl12xx: sdio: add a platform_device
  net: wl12xx: main: add platform device
  net: wireless: wl12xx: re-factor all drivers
  net: wireless: wl12xx: mark some symbols static
  net: wl12xx: main: drop unneded plat_dev

 drivers/net/wireless/wl12xx/io.c                   |   10 +-
 drivers/net/wireless/wl12xx/io.h                   |   21 +--
 drivers/net/wireless/wl12xx/main.c                 |  191 +++++++++++-----
 drivers/net/wireless/wl12xx/sdio.c                 |  220 +++++++-----------
 drivers/net/wireless/wl12xx/spi.c                  |  246 ++++++++------------
 drivers/net/wireless/wl12xx/wl12xx.h               |   16 +-
 drivers/net/wireless/wl12xx/wl12xx_platform_data.c |    4 +-
 include/linux/wl12xx.h                             |    4 +-
 8 files changed, 336 insertions(+), 376 deletions(-)

-- 
1.7.4.1.343.ga91df


^ permalink raw reply	[flat|nested] 32+ messages in thread

* [RFC/PATCH 01/13] net: wl12xx: sdio: id_tables should be __devinitconst
  2011-05-13 21:26 [RFC/PATCH 00/13] wl12xx re-factor Felipe Balbi
@ 2011-05-13 21:26 ` Felipe Balbi
  2011-05-20 12:02   ` Luciano Coelho
  2011-05-20 16:02   ` Ohad Ben-Cohen
  2011-05-13 21:26 ` [RFC/PATCH 02/13] net: wl12xx: sdio: add a context structure Felipe Balbi
                   ` (12 subsequent siblings)
  13 siblings, 2 replies; 32+ messages in thread
From: Felipe Balbi @ 2011-05-13 21:26 UTC (permalink / raw)
  To: Luciano Coelho; +Cc: linux-wireless, netdev, linux-kernel, Felipe Balbi

That's only needed during init anyway, let's free
some space after we're done probing.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/net/wireless/wl12xx/sdio.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/sdio.c b/drivers/net/wireless/wl12xx/sdio.c
index b1c7d03..1268efe 100644
--- a/drivers/net/wireless/wl12xx/sdio.c
+++ b/drivers/net/wireless/wl12xx/sdio.c
@@ -45,7 +45,7 @@
 #define SDIO_DEVICE_ID_TI_WL1271	0x4076
 #endif
 
-static const struct sdio_device_id wl1271_devices[] = {
+static const struct sdio_device_id wl1271_devices[] __devinitconst = {
 	{ SDIO_DEVICE(SDIO_VENDOR_ID_TI, SDIO_DEVICE_ID_TI_WL1271) },
 	{}
 };
-- 
1.7.4.1.343.ga91df


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC/PATCH 02/13] net: wl12xx: sdio: add a context structure
  2011-05-13 21:26 [RFC/PATCH 00/13] wl12xx re-factor Felipe Balbi
  2011-05-13 21:26 ` [RFC/PATCH 01/13] net: wl12xx: sdio: id_tables should be __devinitconst Felipe Balbi
@ 2011-05-13 21:26 ` Felipe Balbi
  2011-05-13 21:26   ` Felipe Balbi
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 32+ messages in thread
From: Felipe Balbi @ 2011-05-13 21:26 UTC (permalink / raw)
  To: Luciano Coelho; +Cc: linux-wireless, netdev, linux-kernel, Felipe Balbi

this will help re-structuring the driver so
that we avoid all duplications which are
currently on this driver.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/net/wireless/wl12xx/sdio.c |   78 +++++++++++++++++++++++++-----------
 1 files changed, 54 insertions(+), 24 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/sdio.c b/drivers/net/wireless/wl12xx/sdio.c
index 1268efe..fe775d3 100644
--- a/drivers/net/wireless/wl12xx/sdio.c
+++ b/drivers/net/wireless/wl12xx/sdio.c
@@ -45,20 +45,25 @@
 #define SDIO_DEVICE_ID_TI_WL1271	0x4076
 #endif
 
+struct wl12xx_sdio_glue {
+	struct device		*dev;
+	struct wl1271		*wl;
+};
+
 static const struct sdio_device_id wl1271_devices[] __devinitconst = {
 	{ SDIO_DEVICE(SDIO_VENDOR_ID_TI, SDIO_DEVICE_ID_TI_WL1271) },
 	{}
 };
 MODULE_DEVICE_TABLE(sdio, wl1271_devices);
 
-static inline struct sdio_func *wl_to_func(struct wl1271 *wl)
+static inline struct wl12xx_sdio_glue *wl_to_glue(struct wl1271 *wl)
 {
 	return wl->if_priv;
 }
 
 static struct device *wl1271_sdio_wl_to_dev(struct wl1271 *wl)
 {
-	return &(wl_to_func(wl)->dev);
+	return wl_to_glue(wl)->dev;
 }
 
 static irqreturn_t wl1271_hardirq(int irq, void *cookie)
@@ -101,8 +106,9 @@ static void wl1271_sdio_init(struct wl1271 *wl)
 static void wl1271_sdio_raw_read(struct wl1271 *wl, int addr, void *buf,
 				 size_t len, bool fixed)
 {
-	int ret;
-	struct sdio_func *func = wl_to_func(wl);
+	struct wl12xx_sdio_glue	*glue = wl_to_glue(wl);
+	struct sdio_func	*func = dev_to_sdio_func(glue->dev);
+	int			ret;
 
 	if (unlikely(addr == HW_ACCESS_ELP_CTRL_REG_ADDR)) {
 		((u8 *)buf)[0] = sdio_f0_readb(func, addr, &ret);
@@ -126,8 +132,9 @@ static void wl1271_sdio_raw_read(struct wl1271 *wl, int addr, void *buf,
 static void wl1271_sdio_raw_write(struct wl1271 *wl, int addr, void *buf,
 				  size_t len, bool fixed)
 {
-	int ret;
-	struct sdio_func *func = wl_to_func(wl);
+	struct wl12xx_sdio_glue	*glue = wl_to_glue(wl);
+	struct sdio_func	*func = dev_to_sdio_func(glue->dev);
+	int			ret;
 
 	if (unlikely(addr == HW_ACCESS_ELP_CTRL_REG_ADDR)) {
 		sdio_f0_writeb(func, ((u8 *)buf)[0], addr, &ret);
@@ -150,8 +157,9 @@ static void wl1271_sdio_raw_write(struct wl1271 *wl, int addr, void *buf,
 
 static int wl1271_sdio_power_on(struct wl1271 *wl)
 {
-	struct sdio_func *func = wl_to_func(wl);
-	int ret;
+	struct wl12xx_sdio_glue	*glue = wl_to_glue(wl);
+	struct sdio_func	*func = dev_to_sdio_func(glue->dev);
+	int			ret;
 
 	/* Make sure the card will not be powered off by runtime PM */
 	ret = pm_runtime_get_sync(&func->dev);
@@ -172,8 +180,9 @@ out:
 
 static int wl1271_sdio_power_off(struct wl1271 *wl)
 {
-	struct sdio_func *func = wl_to_func(wl);
-	int ret;
+	struct wl12xx_sdio_glue	*glue = wl_to_glue(wl);
+	struct sdio_func	*func = dev_to_sdio_func(glue->dev);
+	int			ret;
 
 	sdio_disable_func(func);
 	sdio_release_host(func);
@@ -209,24 +218,39 @@ static struct wl1271_if_operations sdio_ops = {
 static int __devinit wl1271_probe(struct sdio_func *func,
 				  const struct sdio_device_id *id)
 {
-	struct ieee80211_hw *hw;
 	const struct wl12xx_platform_data *wlan_data;
-	struct wl1271 *wl;
-	int ret;
+
+	struct wl12xx_sdio_glue		*glue;
+	struct ieee80211_hw		*hw;
+	struct wl1271			*wl;
+
+	int				ret = -ENOMEM;
 
 	/* We are only able to handle the wlan function */
 	if (func->num != 0x02)
 		return -ENODEV;
 
+	glue = kzalloc(sizeof(*glue), GFP_KERNEL);
+	if (!glue) {
+		dev_err(&func->dev, "not enough memory\n");
+		goto err0;
+	}
+
 	hw = wl1271_alloc_hw();
-	if (IS_ERR(hw))
-		return PTR_ERR(hw);
+	if (IS_ERR(hw)) {
+		dev_err(&func->dev, "can't allocate hw\n");
+		ret = PTR_ERR(hw);
+		goto err1;
+	}
 
 	wl = hw->priv;
 
-	wl->if_priv = func;
+	wl->if_priv = glue;
 	wl->if_ops = &sdio_ops;
 
+	glue->dev = &func->dev;
+	glue->wl = wl;
+
 	/* Grab access to FN0 for ELP reg. */
 	func->card->quirks |= MMC_QUIRK_LENIENT_FN0;
 
@@ -234,7 +258,7 @@ static int __devinit wl1271_probe(struct sdio_func *func,
 	if (IS_ERR(wlan_data)) {
 		ret = PTR_ERR(wlan_data);
 		wl1271_error("missing wlan platform data: %d", ret);
-		goto out_free;
+		goto err2;
 	}
 
 	wl->irq = wlan_data->irq;
@@ -245,20 +269,20 @@ static int __devinit wl1271_probe(struct sdio_func *func,
 				   DRIVER_NAME, wl);
 	if (ret < 0) {
 		wl1271_error("request_irq() failed: %d", ret);
-		goto out_free;
+		goto err2;
 	}
 
 	disable_irq(wl->irq);
 
 	ret = wl1271_init_ieee80211(wl);
 	if (ret)
-		goto out_irq;
+		goto err3;
 
 	ret = wl1271_register_hw(wl);
 	if (ret)
-		goto out_irq;
+		goto err3;
 
-	sdio_set_drvdata(func, wl);
+	sdio_set_drvdata(func, glue);
 
 	/* Tell PM core that we don't need the card to be powered now */
 	pm_runtime_put_noidle(&func->dev);
@@ -267,18 +291,23 @@ static int __devinit wl1271_probe(struct sdio_func *func,
 
 	return 0;
 
- out_irq:
+err3:
 	free_irq(wl->irq, wl);
 
- out_free:
+err2:
 	wl1271_free_hw(wl);
 
+err1:
+	kfree(glue);
+
+err0:
 	return ret;
 }
 
 static void __devexit wl1271_remove(struct sdio_func *func)
 {
-	struct wl1271 *wl = sdio_get_drvdata(func);
+	struct wl12xx_sdio_glue	*glue = sdio_get_drvdata(func);
+	struct wl1271		*wl = glue->wl;
 
 	/* Undo decrement done above in wl1271_probe */
 	pm_runtime_get_noresume(&func->dev);
@@ -286,6 +315,7 @@ static void __devexit wl1271_remove(struct sdio_func *func)
 	wl1271_unregister_hw(wl);
 	free_irq(wl->irq, wl);
 	wl1271_free_hw(wl);
+	kfree(glue);
 }
 
 static int wl1271_suspend(struct device *dev)
-- 
1.7.4.1.343.ga91df


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC/PATCH 03/13] net: wl12xx: remove some unnecessary prints
@ 2011-05-13 21:26   ` Felipe Balbi
  0 siblings, 0 replies; 32+ messages in thread
From: Felipe Balbi @ 2011-05-13 21:26 UTC (permalink / raw)
  To: Luciano Coelho; +Cc: linux-wireless, netdev, linux-kernel, Felipe Balbi

Those have little value. Remove those to make
the driver less noisy.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/net/wireless/wl12xx/sdio.c |   15 +--------------
 1 files changed, 1 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/sdio.c b/drivers/net/wireless/wl12xx/sdio.c
index fe775d3..0832b80 100644
--- a/drivers/net/wireless/wl12xx/sdio.c
+++ b/drivers/net/wireless/wl12xx/sdio.c
@@ -287,8 +287,6 @@ static int __devinit wl1271_probe(struct sdio_func *func,
 	/* Tell PM core that we don't need the card to be powered now */
 	pm_runtime_put_noidle(&func->dev);
 
-	wl1271_notice("initialized");
-
 	return 0;
 
 err3:
@@ -347,23 +345,12 @@ static struct sdio_driver wl1271_sdio_driver = {
 
 static int __init wl1271_init(void)
 {
-	int ret;
-
-	ret = sdio_register_driver(&wl1271_sdio_driver);
-	if (ret < 0) {
-		wl1271_error("failed to register sdio driver: %d", ret);
-		goto out;
-	}
-
-out:
-	return ret;
+	return sdio_register_driver(&wl1271_sdio_driver);
 }
 
 static void __exit wl1271_exit(void)
 {
 	sdio_unregister_driver(&wl1271_sdio_driver);
-
-	wl1271_notice("unloaded");
 }
 
 module_init(wl1271_init);
-- 
1.7.4.1.343.ga91df


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC/PATCH 03/13] net: wl12xx: remove some unnecessary prints
@ 2011-05-13 21:26   ` Felipe Balbi
  0 siblings, 0 replies; 32+ messages in thread
From: Felipe Balbi @ 2011-05-13 21:26 UTC (permalink / raw)
  To: Luciano Coelho
  Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Felipe Balbi

Those have little value. Remove those to make
the driver less noisy.

Signed-off-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
---
 drivers/net/wireless/wl12xx/sdio.c |   15 +--------------
 1 files changed, 1 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/sdio.c b/drivers/net/wireless/wl12xx/sdio.c
index fe775d3..0832b80 100644
--- a/drivers/net/wireless/wl12xx/sdio.c
+++ b/drivers/net/wireless/wl12xx/sdio.c
@@ -287,8 +287,6 @@ static int __devinit wl1271_probe(struct sdio_func *func,
 	/* Tell PM core that we don't need the card to be powered now */
 	pm_runtime_put_noidle(&func->dev);
 
-	wl1271_notice("initialized");
-
 	return 0;
 
 err3:
@@ -347,23 +345,12 @@ static struct sdio_driver wl1271_sdio_driver = {
 
 static int __init wl1271_init(void)
 {
-	int ret;
-
-	ret = sdio_register_driver(&wl1271_sdio_driver);
-	if (ret < 0) {
-		wl1271_error("failed to register sdio driver: %d", ret);
-		goto out;
-	}
-
-out:
-	return ret;
+	return sdio_register_driver(&wl1271_sdio_driver);
 }
 
 static void __exit wl1271_exit(void)
 {
 	sdio_unregister_driver(&wl1271_sdio_driver);

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC/PATCH 04/13] net: wl12xx: care for optional operations
  2011-05-13 21:26 [RFC/PATCH 00/13] wl12xx re-factor Felipe Balbi
                   ` (2 preceding siblings ...)
  2011-05-13 21:26   ` Felipe Balbi
@ 2011-05-13 21:26 ` Felipe Balbi
  2011-05-20 12:03   ` Luciano Coelho
  2011-05-13 21:26 ` [RFC/PATCH 05/13] net: wl12xx: remove the nops Felipe Balbi
                   ` (9 subsequent siblings)
  13 siblings, 1 reply; 32+ messages in thread
From: Felipe Balbi @ 2011-05-13 21:26 UTC (permalink / raw)
  To: Luciano Coelho; +Cc: linux-wireless, netdev, linux-kernel, Felipe Balbi

->init and ->reset are optional - at least
sdio.c doesn't implement them - so allow those
pointers to be NULL.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/net/wireless/wl12xx/io.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/io.c b/drivers/net/wireless/wl12xx/io.c
index d557f73..57bc646 100644
--- a/drivers/net/wireless/wl12xx/io.c
+++ b/drivers/net/wireless/wl12xx/io.c
@@ -117,12 +117,14 @@ EXPORT_SYMBOL_GPL(wl1271_set_partition);
 
 void wl1271_io_reset(struct wl1271 *wl)
 {
-	wl->if_ops->reset(wl);
+	if (wl->if_ops->reset)
+		wl->if_ops->reset(wl);
 }
 
 void wl1271_io_init(struct wl1271 *wl)
 {
-	wl->if_ops->init(wl);
+	if (wl->if_ops->init)
+		wl->if_ops->init(wl);
 }
 
 void wl1271_top_reg_write(struct wl1271 *wl, int addr, u16 val)
-- 
1.7.4.1.343.ga91df


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC/PATCH 05/13] net: wl12xx: remove the nops
  2011-05-13 21:26 [RFC/PATCH 00/13] wl12xx re-factor Felipe Balbi
                   ` (3 preceding siblings ...)
  2011-05-13 21:26 ` [RFC/PATCH 04/13] net: wl12xx: care for optional operations Felipe Balbi
@ 2011-05-13 21:26 ` Felipe Balbi
  2011-05-20 12:04   ` Luciano Coelho
  2011-05-13 21:26 ` [RFC/PATCH 06/13] net: wl12xx: remove unnecessary prints Felipe Balbi
                   ` (8 subsequent siblings)
  13 siblings, 1 reply; 32+ messages in thread
From: Felipe Balbi @ 2011-05-13 21:26 UTC (permalink / raw)
  To: Luciano Coelho; +Cc: linux-wireless, netdev, linux-kernel, Felipe Balbi

Nops aren't needed. When we actually need
those calls, then we add them with meat
and barbecue sauce.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/net/wireless/wl12xx/sdio.c |   30 ------------------------------
 1 files changed, 0 insertions(+), 30 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/sdio.c b/drivers/net/wireless/wl12xx/sdio.c
index 0832b80..bb7569c 100644
--- a/drivers/net/wireless/wl12xx/sdio.c
+++ b/drivers/net/wireless/wl12xx/sdio.c
@@ -95,14 +95,6 @@ static void wl1271_sdio_enable_interrupts(struct wl1271 *wl)
 	enable_irq(wl->irq);
 }
 
-static void wl1271_sdio_reset(struct wl1271 *wl)
-{
-}
-
-static void wl1271_sdio_init(struct wl1271 *wl)
-{
-}
-
 static void wl1271_sdio_raw_read(struct wl1271 *wl, int addr, void *buf,
 				 size_t len, bool fixed)
 {
@@ -207,8 +199,6 @@ static int wl1271_sdio_set_power(struct wl1271 *wl, bool enable)
 static struct wl1271_if_operations sdio_ops = {
 	.read		= wl1271_sdio_raw_read,
 	.write		= wl1271_sdio_raw_write,
-	.reset		= wl1271_sdio_reset,
-	.init		= wl1271_sdio_init,
 	.power		= wl1271_sdio_set_power,
 	.dev		= wl1271_sdio_wl_to_dev,
 	.enable_irq	= wl1271_sdio_enable_interrupts,
@@ -316,31 +306,11 @@ static void __devexit wl1271_remove(struct sdio_func *func)
 	kfree(glue);
 }
 
-static int wl1271_suspend(struct device *dev)
-{
-	/* Tell MMC/SDIO core it's OK to power down the card
-	 * (if it isn't already), but not to remove it completely */
-	return 0;
-}
-
-static int wl1271_resume(struct device *dev)
-{
-	return 0;
-}
-
-static const struct dev_pm_ops wl1271_sdio_pm_ops = {
-	.suspend	= wl1271_suspend,
-	.resume		= wl1271_resume,
-};
-
 static struct sdio_driver wl1271_sdio_driver = {
 	.name		= "wl1271_sdio",
 	.id_table	= wl1271_devices,
 	.probe		= wl1271_probe,
 	.remove		= __devexit_p(wl1271_remove),
-	.drv = {
-		.pm = &wl1271_sdio_pm_ops,
-	},
 };
 
 static int __init wl1271_init(void)
-- 
1.7.4.1.343.ga91df


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC/PATCH 06/13] net: wl12xx: remove unnecessary prints
  2011-05-13 21:26 [RFC/PATCH 00/13] wl12xx re-factor Felipe Balbi
                   ` (4 preceding siblings ...)
  2011-05-13 21:26 ` [RFC/PATCH 05/13] net: wl12xx: remove the nops Felipe Balbi
@ 2011-05-13 21:26 ` Felipe Balbi
  2011-05-20 12:04   ` Luciano Coelho
  2011-05-13 21:26 ` [RFC/PATCH 07/13] net: wl12xx: spi: add a context structure Felipe Balbi
                   ` (7 subsequent siblings)
  13 siblings, 1 reply; 32+ messages in thread
From: Felipe Balbi @ 2011-05-13 21:26 UTC (permalink / raw)
  To: Luciano Coelho; +Cc: linux-wireless, netdev, linux-kernel, Felipe Balbi

Those have little value. Remove those to
make the driver less noisy.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/net/wireless/wl12xx/spi.c |   15 +--------------
 1 files changed, 1 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/spi.c b/drivers/net/wireless/wl12xx/spi.c
index ffc745b..382b79d 100644
--- a/drivers/net/wireless/wl12xx/spi.c
+++ b/drivers/net/wireless/wl12xx/spi.c
@@ -426,8 +426,6 @@ static int __devinit wl1271_probe(struct spi_device *spi)
 	if (ret)
 		goto out_irq;
 
-	wl1271_notice("initialized");
-
 	return 0;
 
  out_irq:
@@ -464,23 +462,12 @@ static struct spi_driver wl1271_spi_driver = {
 
 static int __init wl1271_init(void)
 {
-	int ret;
-
-	ret = spi_register_driver(&wl1271_spi_driver);
-	if (ret < 0) {
-		wl1271_error("failed to register spi driver: %d", ret);
-		goto out;
-	}
-
-out:
-	return ret;
+	return spi_register_driver(&wl1271_spi_driver);
 }
 
 static void __exit wl1271_exit(void)
 {
 	spi_unregister_driver(&wl1271_spi_driver);
-
-	wl1271_notice("unloaded");
 }
 
 module_init(wl1271_init);
-- 
1.7.4.1.343.ga91df


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC/PATCH 07/13] net: wl12xx: spi: add a context structure
  2011-05-13 21:26 [RFC/PATCH 00/13] wl12xx re-factor Felipe Balbi
                   ` (5 preceding siblings ...)
  2011-05-13 21:26 ` [RFC/PATCH 06/13] net: wl12xx: remove unnecessary prints Felipe Balbi
@ 2011-05-13 21:26 ` Felipe Balbi
  2011-05-13 21:26 ` [RFC/PATCH 08/13] net: wl12xx: spi: add a platform_device Felipe Balbi
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 32+ messages in thread
From: Felipe Balbi @ 2011-05-13 21:26 UTC (permalink / raw)
  To: Luciano Coelho; +Cc: linux-wireless, netdev, linux-kernel, Felipe Balbi

this will help re-structuring the driver so
that we avoid all duplications which are
currently on this driver.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/net/wireless/wl12xx/spi.c |  128 +++++++++++++++++++++++-------------
 1 files changed, 82 insertions(+), 46 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/spi.c b/drivers/net/wireless/wl12xx/spi.c
index 382b79d..5dbd5b3 100644
--- a/drivers/net/wireless/wl12xx/spi.c
+++ b/drivers/net/wireless/wl12xx/spi.c
@@ -68,14 +68,19 @@
 
 #define WSPI_MAX_NUM_OF_CHUNKS (WL1271_AGGR_BUFFER_SIZE / WSPI_MAX_CHUNK_SIZE)
 
-static inline struct spi_device *wl_to_spi(struct wl1271 *wl)
+struct wl12xx_spi_glue {
+	struct device	*dev;
+	struct wl1271	*wl;
+};
+
+static inline struct wl12xx_spi_glue *wl_to_glue(struct wl1271 *wl)
 {
 	return wl->if_priv;
 }
 
 static struct device *wl1271_spi_wl_to_dev(struct wl1271 *wl)
 {
-	return &(wl_to_spi(wl)->dev);
+	return wl_to_glue(wl)->dev;
 }
 
 static void wl1271_spi_disable_interrupts(struct wl1271 *wl)
@@ -90,9 +95,10 @@ static void wl1271_spi_enable_interrupts(struct wl1271 *wl)
 
 static void wl1271_spi_reset(struct wl1271 *wl)
 {
-	u8 *cmd;
-	struct spi_transfer t;
-	struct spi_message m;
+	struct wl12xx_spi_glue	*glue = wl_to_glue(wl);
+	struct spi_transfer	t;
+	struct spi_message	m;
+	u8			*cmd;
 
 	cmd = kzalloc(WSPI_INIT_CMD_LEN, GFP_KERNEL);
 	if (!cmd) {
@@ -109,7 +115,7 @@ static void wl1271_spi_reset(struct wl1271 *wl)
 	t.len = WSPI_INIT_CMD_LEN;
 	spi_message_add_tail(&t, &m);
 
-	spi_sync(wl_to_spi(wl), &m);
+	spi_sync(to_spi_device(glue->dev), &m);
 
 	wl1271_dump(DEBUG_SPI, "spi reset -> ", cmd, WSPI_INIT_CMD_LEN);
 	kfree(cmd);
@@ -117,9 +123,12 @@ static void wl1271_spi_reset(struct wl1271 *wl)
 
 static void wl1271_spi_init(struct wl1271 *wl)
 {
-	u8 crc[WSPI_INIT_CMD_CRC_LEN], *cmd;
-	struct spi_transfer t;
-	struct spi_message m;
+	struct wl12xx_spi_glue	*glue = wl_to_glue(wl);
+	struct spi_transfer	t;
+	struct spi_message	m;
+
+	u8			crc[WSPI_INIT_CMD_CRC_LEN];
+	u8			*cmd;
 
 	cmd = kzalloc(WSPI_INIT_CMD_LEN, GFP_KERNEL);
 	if (!cmd) {
@@ -164,7 +173,7 @@ static void wl1271_spi_init(struct wl1271 *wl)
 	t.len = WSPI_INIT_CMD_LEN;
 	spi_message_add_tail(&t, &m);
 
-	spi_sync(wl_to_spi(wl), &m);
+	spi_sync(to_spi_device(glue->dev), &m);
 	wl1271_dump(DEBUG_SPI, "spi init -> ", cmd, WSPI_INIT_CMD_LEN);
 	kfree(cmd);
 }
@@ -173,10 +182,12 @@ static void wl1271_spi_init(struct wl1271 *wl)
 
 static int wl1271_spi_read_busy(struct wl1271 *wl)
 {
-	struct spi_transfer t[1];
-	struct spi_message m;
-	u32 *busy_buf;
-	int num_busy_bytes = 0;
+	struct wl12xx_spi_glue	*glue = wl_to_glue(wl);
+	struct spi_transfer	t[1];
+	struct spi_message	m;
+
+	int			num_busy_bytes = 0;
+	u32			*busy_buf;
 
 	/*
 	 * Read further busy words from SPI until a non-busy word is
@@ -193,7 +204,7 @@ static int wl1271_spi_read_busy(struct wl1271 *wl)
 		t[0].len = sizeof(u32);
 		t[0].cs_change = true;
 		spi_message_add_tail(&t[0], &m);
-		spi_sync(wl_to_spi(wl), &m);
+		spi_sync(to_spi_device(glue->dev), &m);
 
 		if (*busy_buf & 0x1)
 			return 0;
@@ -207,11 +218,13 @@ static int wl1271_spi_read_busy(struct wl1271 *wl)
 static void wl1271_spi_raw_read(struct wl1271 *wl, int addr, void *buf,
 				size_t len, bool fixed)
 {
-	struct spi_transfer t[2];
-	struct spi_message m;
-	u32 *busy_buf;
-	u32 *cmd;
-	u32 chunk_len;
+	struct wl12xx_spi_glue	*glue = wl_to_glue(wl);
+	struct spi_transfer	t[2];
+	struct spi_message	m;
+
+	u32			*busy_buf;
+	u32			*cmd;
+	u32			chunk_len;
 
 	while (len > 0) {
 		chunk_len = min((size_t)WSPI_MAX_CHUNK_SIZE, len);
@@ -242,7 +255,7 @@ static void wl1271_spi_raw_read(struct wl1271 *wl, int addr, void *buf,
 		t[1].cs_change = true;
 		spi_message_add_tail(&t[1], &m);
 
-		spi_sync(wl_to_spi(wl), &m);
+		spi_sync(to_spi_device(glue->dev), &m);
 
 		if (!(busy_buf[WL1271_BUSY_WORD_CNT - 1] & 0x1) &&
 		    wl1271_spi_read_busy(wl)) {
@@ -258,7 +271,7 @@ static void wl1271_spi_raw_read(struct wl1271 *wl, int addr, void *buf,
 		t[0].cs_change = true;
 		spi_message_add_tail(&t[0], &m);
 
-		spi_sync(wl_to_spi(wl), &m);
+		spi_sync(to_spi_device(glue->dev), &m);
 
 		wl1271_dump(DEBUG_SPI, "spi_read cmd -> ", cmd, sizeof(*cmd));
 		wl1271_dump(DEBUG_SPI, "spi_read buf <- ", buf, chunk_len);
@@ -273,12 +286,15 @@ static void wl1271_spi_raw_read(struct wl1271 *wl, int addr, void *buf,
 static void wl1271_spi_raw_write(struct wl1271 *wl, int addr, void *buf,
 			  size_t len, bool fixed)
 {
-	struct spi_transfer t[2 * WSPI_MAX_NUM_OF_CHUNKS];
-	struct spi_message m;
-	u32 commands[WSPI_MAX_NUM_OF_CHUNKS];
-	u32 *cmd;
-	u32 chunk_len;
-	int i;
+	struct wl12xx_spi_glue	*glue = wl_to_glue(wl);
+	struct spi_transfer	t[2 * WSPI_MAX_NUM_OF_CHUNKS];
+	struct spi_message	m;
+
+	u32			commands[WSPI_MAX_NUM_OF_CHUNKS];
+	u32			chunk_len;
+	u32			*cmd;
+
+	int			i;
 
 	WARN_ON(len > WL1271_AGGR_BUFFER_SIZE);
 
@@ -317,7 +333,7 @@ static void wl1271_spi_raw_write(struct wl1271 *wl, int addr, void *buf,
 		cmd++;
 	}
 
-	spi_sync(wl_to_spi(wl), &m);
+	spi_sync(to_spi_device(glue->dev), &m);
 }
 
 static irqreturn_t wl1271_hardirq(int irq, void *cookie)
@@ -360,10 +376,13 @@ static struct wl1271_if_operations spi_ops = {
 
 static int __devinit wl1271_probe(struct spi_device *spi)
 {
-	struct wl12xx_platform_data *pdata;
-	struct ieee80211_hw *hw;
-	struct wl1271 *wl;
-	int ret;
+	struct wl12xx_platform_data	*pdata;
+
+	struct wl12xx_spi_glue		*glue;
+	struct ieee80211_hw		*hw;
+	struct wl1271			*wl;
+
+	int				ret = -ENOMEM;
 
 	pdata = spi->dev.platform_data;
 	if (!pdata) {
@@ -371,14 +390,25 @@ static int __devinit wl1271_probe(struct spi_device *spi)
 		return -ENODEV;
 	}
 
+	glue = kzalloc(sizeof(*glue), GFP_KERNEL);
+	if (!glue) {
+		dev_err(&spi->dev, "not enought memory\n");
+		goto err0;
+	}
+
 	hw = wl1271_alloc_hw();
-	if (IS_ERR(hw))
-		return PTR_ERR(hw);
+	if (IS_ERR(hw)) {
+		ret = PTR_ERR(hw);
+		goto err1;
+	}
 
 	wl = hw->priv;
 
-	dev_set_drvdata(&spi->dev, wl);
-	wl->if_priv = spi;
+	glue->dev = &spi->dev;
+	glue->wl = wl;
+
+	spi_set_drvdata(spi, glue);
+	wl->if_priv = glue;
 
 	wl->if_ops = &spi_ops;
 
@@ -389,14 +419,14 @@ static int __devinit wl1271_probe(struct spi_device *spi)
 	ret = spi_setup(spi);
 	if (ret < 0) {
 		wl1271_error("spi_setup failed");
-		goto out_free;
+		goto err2;
 	}
 
 	wl->set_power = pdata->set_power;
 	if (!wl->set_power) {
 		wl1271_error("set power function missing in platform data");
 		ret = -ENODEV;
-		goto out_free;
+		goto err2;
 	}
 
 	wl->ref_clock = pdata->board_ref_clock;
@@ -405,7 +435,7 @@ static int __devinit wl1271_probe(struct spi_device *spi)
 	if (wl->irq < 0) {
 		wl1271_error("irq missing in platform data");
 		ret = -ENODEV;
-		goto out_free;
+		goto err2;
 	}
 
 	ret = request_threaded_irq(wl->irq, wl1271_hardirq, wl1271_irq,
@@ -413,37 +443,43 @@ static int __devinit wl1271_probe(struct spi_device *spi)
 				   DRIVER_NAME, wl);
 	if (ret < 0) {
 		wl1271_error("request_irq() failed: %d", ret);
-		goto out_free;
+		goto err2;
 	}
 
 	disable_irq(wl->irq);
 
 	ret = wl1271_init_ieee80211(wl);
 	if (ret)
-		goto out_irq;
+		goto err3;
 
 	ret = wl1271_register_hw(wl);
 	if (ret)
-		goto out_irq;
+		goto err3;
 
 	return 0;
 
- out_irq:
+err3:
 	free_irq(wl->irq, wl);
 
- out_free:
+err2:
 	wl1271_free_hw(wl);
 
+err1:
+	kfree(glue);
+
+err0:
 	return ret;
 }
 
 static int __devexit wl1271_remove(struct spi_device *spi)
 {
-	struct wl1271 *wl = dev_get_drvdata(&spi->dev);
+	struct wl12xx_spi_glue	*glue = spi_get_drvdata(spi);
+	struct wl1271		*wl = glue->wl;
 
 	wl1271_unregister_hw(wl);
 	free_irq(wl->irq, wl);
 	wl1271_free_hw(wl);
+	kfree(glue);
 
 	return 0;
 }
-- 
1.7.4.1.343.ga91df


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC/PATCH 08/13] net: wl12xx: spi: add a platform_device
  2011-05-13 21:26 [RFC/PATCH 00/13] wl12xx re-factor Felipe Balbi
                   ` (6 preceding siblings ...)
  2011-05-13 21:26 ` [RFC/PATCH 07/13] net: wl12xx: spi: add a context structure Felipe Balbi
@ 2011-05-13 21:26 ` Felipe Balbi
  2011-05-13 21:26 ` [RFC/PATCH 09/13] net: wl12xx: sdio: " Felipe Balbi
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 32+ messages in thread
From: Felipe Balbi @ 2011-05-13 21:26 UTC (permalink / raw)
  To: Luciano Coelho; +Cc: linux-wireless, netdev, linux-kernel, Felipe Balbi

that device will match with a driver to
be added to the core driver.

that driver will be added once the other
glue layer (sdio.c) is converted to this
new scheme.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/net/wireless/wl12xx/spi.c |   50 ++++++++++++++++++++++++++++++++++--
 1 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/spi.c b/drivers/net/wireless/wl12xx/spi.c
index 5dbd5b3..6644996 100644
--- a/drivers/net/wireless/wl12xx/spi.c
+++ b/drivers/net/wireless/wl12xx/spi.c
@@ -26,6 +26,7 @@
 #include <linux/crc7.h>
 #include <linux/spi/spi.h>
 #include <linux/wl12xx.h>
+#include <linux/platform_device.h>
 #include <linux/slab.h>
 
 #include "wl12xx.h"
@@ -69,8 +70,9 @@
 #define WSPI_MAX_NUM_OF_CHUNKS (WL1271_AGGR_BUFFER_SIZE / WSPI_MAX_CHUNK_SIZE)
 
 struct wl12xx_spi_glue {
-	struct device	*dev;
-	struct wl1271	*wl;
+	struct device		*dev;
+	struct wl1271		*wl;
+	struct platform_device	*core;
 };
 
 static inline struct wl12xx_spi_glue *wl_to_glue(struct wl1271 *wl)
@@ -377,6 +379,8 @@ static struct wl1271_if_operations spi_ops = {
 static int __devinit wl1271_probe(struct spi_device *spi)
 {
 	struct wl12xx_platform_data	*pdata;
+	struct platform_device		*core;
+	struct resource			res[1];
 
 	struct wl12xx_spi_glue		*glue;
 	struct ieee80211_hw		*hw;
@@ -456,8 +460,47 @@ static int __devinit wl1271_probe(struct spi_device *spi)
 	if (ret)
 		goto err3;
 
+	core = platform_device_alloc("wl12xx-spi", -1);
+	if (!core) {
+		dev_err(&spi->dev, "can't allocate platform_device\n");
+		ret = -ENOMEM;
+		goto err4;
+	}
+
+	core->dev.parent = &spi->dev;
+
+	memset(res, 0x00, sizeof(res) * ARRAY_SIZE(res));
+
+	res[0].start	= spi->irq;
+	res[0].flags	= IORESOURCE_IRQ;
+	res[0].name	= "irq";
+
+	ret = platform_device_add_resources(core, res, ARRAY_SIZE(res));
+	if (ret) {
+		dev_err(&spi->dev, "can't add resources\n");
+		goto err5;
+	}
+
+	ret = platform_device_add_data(core, pdata, sizeof(*pdata));
+	if (ret) {
+		dev_err(&spi->dev, "can't add platform data\n");
+		goto err5;
+	}
+
+	ret = platform_device_register(core);
+	if (ret) {
+		dev_err(&spi->dev, "can't register platform device\n");
+		goto err5;
+	}
+
 	return 0;
 
+err5:
+	platform_device_put(core);
+
+err4:
+	wl1271_unregister_hw(wl);
+
 err3:
 	free_irq(wl->irq, wl);
 
@@ -479,12 +522,13 @@ static int __devexit wl1271_remove(struct spi_device *spi)
 	wl1271_unregister_hw(wl);
 	free_irq(wl->irq, wl);
 	wl1271_free_hw(wl);
+	platform_device_del(glue->core);
+	platform_device_put(glue->core);
 	kfree(glue);
 
 	return 0;
 }
 
-
 static struct spi_driver wl1271_spi_driver = {
 	.driver = {
 		.name		= "wl1271_spi",
-- 
1.7.4.1.343.ga91df


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC/PATCH 09/13] net: wl12xx: sdio: add a platform_device
  2011-05-13 21:26 [RFC/PATCH 00/13] wl12xx re-factor Felipe Balbi
                   ` (7 preceding siblings ...)
  2011-05-13 21:26 ` [RFC/PATCH 08/13] net: wl12xx: spi: add a platform_device Felipe Balbi
@ 2011-05-13 21:26 ` Felipe Balbi
  2011-05-13 21:26   ` Felipe Balbi
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 32+ messages in thread
From: Felipe Balbi @ 2011-05-13 21:26 UTC (permalink / raw)
  To: Luciano Coelho; +Cc: linux-wireless, netdev, linux-kernel, Felipe Balbi

that device will match with a driver to
be added to the core driver.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/net/wireless/wl12xx/sdio.c |   45 ++++++++++++++++++++++++++++++++++++
 1 files changed, 45 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/sdio.c b/drivers/net/wireless/wl12xx/sdio.c
index bb7569c..5c5e4f2 100644
--- a/drivers/net/wireless/wl12xx/sdio.c
+++ b/drivers/net/wireless/wl12xx/sdio.c
@@ -25,6 +25,7 @@
 #include <linux/module.h>
 #include <linux/crc7.h>
 #include <linux/vmalloc.h>
+#include <linux/platform_device.h>
 #include <linux/mmc/sdio_func.h>
 #include <linux/mmc/sdio_ids.h>
 #include <linux/mmc/card.h>
@@ -48,6 +49,7 @@
 struct wl12xx_sdio_glue {
 	struct device		*dev;
 	struct wl1271		*wl;
+	struct platform_device	*core;
 };
 
 static const struct sdio_device_id wl1271_devices[] __devinitconst = {
@@ -210,6 +212,8 @@ static int __devinit wl1271_probe(struct sdio_func *func,
 {
 	const struct wl12xx_platform_data *wlan_data;
 
+	struct platform_device		*core;
+	struct resource			res[1];
 	struct wl12xx_sdio_glue		*glue;
 	struct ieee80211_hw		*hw;
 	struct wl1271			*wl;
@@ -277,8 +281,47 @@ static int __devinit wl1271_probe(struct sdio_func *func,
 	/* Tell PM core that we don't need the card to be powered now */
 	pm_runtime_put_noidle(&func->dev);
 
+	core = platform_device_alloc("wl12xx-sdio", -1);
+	if (!core) {
+		dev_err(&func->dev, "can't allocate platform_device\n");
+		ret = -ENOMEM;
+		goto err4;
+	}
+
+	core->dev.parent = &func->dev;
+
+	memset(res, 0x00, sizeof(res) * ARRAY_SIZE(res));
+
+	res[0].start	= wlan_data->irq;
+	res[0].flags	= IORESOURCE_IRQ;
+	res[0].name	= "irq";
+
+	ret = platform_device_add_resources(core, res, ARRAY_SIZE(res));
+	if (ret) {
+		dev_err(&func->dev, "can't add resources\n");
+		goto err5;
+	}
+
+	ret = platform_device_add_data(core, wlan_data, sizeof(*wlan_data));
+	if (ret) {
+		dev_err(&func->dev, "can't add platform data\n");
+		goto err5;
+	}
+
+	ret = platform_device_register(core);
+	if (ret) {
+		dev_err(&func->dev, "can't register platform device\n");
+		goto err5;
+	}
+
 	return 0;
 
+err5:
+	platform_device_put(core);
+
+err4:
+	wl1271_unregister_hw(wl);
+
 err3:
 	free_irq(wl->irq, wl);
 
@@ -303,6 +346,8 @@ static void __devexit wl1271_remove(struct sdio_func *func)
 	wl1271_unregister_hw(wl);
 	free_irq(wl->irq, wl);
 	wl1271_free_hw(wl);
+	platform_device_del(glue->core);
+	platform_device_put(glue->core);
 	kfree(glue);
 }
 
-- 
1.7.4.1.343.ga91df


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC/PATCH 10/13] net: wl12xx: main: add platform device
@ 2011-05-13 21:26   ` Felipe Balbi
  0 siblings, 0 replies; 32+ messages in thread
From: Felipe Balbi @ 2011-05-13 21:26 UTC (permalink / raw)
  To: Luciano Coelho; +Cc: linux-wireless, netdev, linux-kernel, Felipe Balbi

now that we have a platform_driver on both
glue layers, add a platform_device to the
core driver.

It's currently an empty platform_driver but
more functionality will be added on later
patches.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/net/wireless/wl12xx/main.c |   35 +++++++++++++++++++++++++++++++++++
 1 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index 8b3c8d1..d377e1f 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -3547,6 +3547,41 @@ int wl1271_free_hw(struct wl1271 *wl)
 }
 EXPORT_SYMBOL_GPL(wl1271_free_hw);
 
+static int __devinit wl12xx_probe(struct platform_device *pdev)
+{
+	return 0;
+}
+
+static int __devexit wl12xx_remove(struct platform_device *pdev)
+{
+	return 0;
+}
+
+static const struct platform_device_id wl12xx_id_table[] __devinitconst = {
+	{ "wl12xx-sdio", 0 },
+	{ "wl12xx-spi", 0 },
+	{  }	/* Terminating Entry */
+};
+MODULE_DEVICE_TABLE(platform, wl12xx_id_table);
+
+static struct platform_driver wl12xx_driver = {
+	.probe		= wl12xx_probe,
+	.remove		= __devexit_p(wl12xx_remove),
+	.id_table	= wl12xx_id_table,
+};
+
+static int __init wl12xx_init(void)
+{
+	return platform_driver_register(&wl12xx_driver);
+}
+module_init(wl12xx_init);
+
+static void __exit wl12xx_exit(void)
+{
+	platform_driver_unregister(&wl12xx_driver);
+}
+module_exit(wl12xx_exit);
+
 u32 wl12xx_debug_level = DEBUG_NONE;
 EXPORT_SYMBOL_GPL(wl12xx_debug_level);
 module_param_named(debug_level, wl12xx_debug_level, uint, S_IRUSR | S_IWUSR);
-- 
1.7.4.1.343.ga91df


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC/PATCH 10/13] net: wl12xx: main: add platform device
@ 2011-05-13 21:26   ` Felipe Balbi
  0 siblings, 0 replies; 32+ messages in thread
From: Felipe Balbi @ 2011-05-13 21:26 UTC (permalink / raw)
  To: Luciano Coelho
  Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Felipe Balbi

now that we have a platform_driver on both
glue layers, add a platform_device to the
core driver.

It's currently an empty platform_driver but
more functionality will be added on later
patches.

Signed-off-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
---
 drivers/net/wireless/wl12xx/main.c |   35 +++++++++++++++++++++++++++++++++++
 1 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index 8b3c8d1..d377e1f 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -3547,6 +3547,41 @@ int wl1271_free_hw(struct wl1271 *wl)
 }
 EXPORT_SYMBOL_GPL(wl1271_free_hw);
 
+static int __devinit wl12xx_probe(struct platform_device *pdev)
+{
+	return 0;
+}
+
+static int __devexit wl12xx_remove(struct platform_device *pdev)
+{
+	return 0;
+}
+
+static const struct platform_device_id wl12xx_id_table[] __devinitconst = {
+	{ "wl12xx-sdio", 0 },
+	{ "wl12xx-spi", 0 },
+	{  }	/* Terminating Entry */
+};
+MODULE_DEVICE_TABLE(platform, wl12xx_id_table);
+
+static struct platform_driver wl12xx_driver = {
+	.probe		= wl12xx_probe,
+	.remove		= __devexit_p(wl12xx_remove),
+	.id_table	= wl12xx_id_table,
+};
+
+static int __init wl12xx_init(void)
+{
+	return platform_driver_register(&wl12xx_driver);
+}
+module_init(wl12xx_init);
+
+static void __exit wl12xx_exit(void)
+{
+	platform_driver_unregister(&wl12xx_driver);
+}
+module_exit(wl12xx_exit);
+
 u32 wl12xx_debug_level = DEBUG_NONE;
 EXPORT_SYMBOL_GPL(wl12xx_debug_level);
 module_param_named(debug_level, wl12xx_debug_level, uint, S_IRUSR | S_IWUSR);
-- 
1.7.4.1.343.ga91df

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC/PATCH 11/13] net: wireless: wl12xx: re-factor all drivers
  2011-05-13 21:26 [RFC/PATCH 00/13] wl12xx re-factor Felipe Balbi
                   ` (9 preceding siblings ...)
  2011-05-13 21:26   ` Felipe Balbi
@ 2011-05-13 21:26 ` Felipe Balbi
  2011-05-13 21:26   ` Felipe Balbi
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 32+ messages in thread
From: Felipe Balbi @ 2011-05-13 21:26 UTC (permalink / raw)
  To: Luciano Coelho; +Cc: linux-wireless, netdev, linux-kernel, Felipe Balbi

move all common parts to where they belong,
namely main.c.

Ok this is a big patch but I didn't find
ways to make it so that we don't break
compilation. A few more patches will come
to make static whatever should be static.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/net/wireless/wl12xx/io.c                   |    8 +-
 drivers/net/wireless/wl12xx/io.h                   |   14 +--
 drivers/net/wireless/wl12xx/main.c                 |   90 +++++++++++-
 drivers/net/wireless/wl12xx/sdio.c                 |  128 +++--------------
 drivers/net/wireless/wl12xx/spi.c                  |  153 +++-----------------
 drivers/net/wireless/wl12xx/wl12xx.h               |   15 +-
 drivers/net/wireless/wl12xx/wl12xx_platform_data.c |    4 +-
 include/linux/wl12xx.h                             |    4 +-
 8 files changed, 147 insertions(+), 269 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/io.c b/drivers/net/wireless/wl12xx/io.c
index 57bc646..95a9a16 100644
--- a/drivers/net/wireless/wl12xx/io.c
+++ b/drivers/net/wireless/wl12xx/io.c
@@ -45,12 +45,12 @@
 
 void wl1271_disable_interrupts(struct wl1271 *wl)
 {
-	wl->if_ops->disable_irq(wl);
+	disable_irq(wl->irq);
 }
 
 void wl1271_enable_interrupts(struct wl1271 *wl)
 {
-	wl->if_ops->enable_irq(wl);
+	enable_irq(wl->irq);
 }
 
 /* Set the SPI partitions to access the chip addresses
@@ -118,13 +118,13 @@ EXPORT_SYMBOL_GPL(wl1271_set_partition);
 void wl1271_io_reset(struct wl1271 *wl)
 {
 	if (wl->if_ops->reset)
-		wl->if_ops->reset(wl);
+		wl->if_ops->reset(wl->dev);
 }
 
 void wl1271_io_init(struct wl1271 *wl)
 {
 	if (wl->if_ops->init)
-		wl->if_ops->init(wl);
+		wl->if_ops->init(wl->dev);
 }
 
 void wl1271_top_reg_write(struct wl1271 *wl, int addr, u16 val)
diff --git a/drivers/net/wireless/wl12xx/io.h b/drivers/net/wireless/wl12xx/io.h
index 00c771e..2bb2295 100644
--- a/drivers/net/wireless/wl12xx/io.h
+++ b/drivers/net/wireless/wl12xx/io.h
@@ -50,23 +50,17 @@ void wl1271_enable_interrupts(struct wl1271 *wl);
 void wl1271_io_reset(struct wl1271 *wl);
 void wl1271_io_init(struct wl1271 *wl);
 
-static inline struct device *wl1271_wl_to_dev(struct wl1271 *wl)
-{
-	return wl->if_ops->dev(wl);
-}
-
-
 /* Raw target IO, address is not translated */
 static inline void wl1271_raw_write(struct wl1271 *wl, int addr, void *buf,
 				    size_t len, bool fixed)
 {
-	wl->if_ops->write(wl, addr, buf, len, fixed);
+	wl->if_ops->write(wl->dev, addr, buf, len, fixed);
 }
 
 static inline void wl1271_raw_read(struct wl1271 *wl, int addr, void *buf,
 				   size_t len, bool fixed)
 {
-	wl->if_ops->read(wl, addr, buf, len, fixed);
+	wl->if_ops->read(wl->dev, addr, buf, len, fixed);
 }
 
 static inline u32 wl1271_raw_read32(struct wl1271 *wl, int addr)
@@ -140,13 +134,13 @@ static inline void wl1271_write32(struct wl1271 *wl, int addr, u32 val)
 
 static inline void wl1271_power_off(struct wl1271 *wl)
 {
-	wl->if_ops->power(wl, false);
+	wl->if_ops->power(wl->dev, false);
 	clear_bit(WL1271_FLAG_GPIO_POWER, &wl->flags);
 }
 
 static inline int wl1271_power_on(struct wl1271 *wl)
 {
-	int ret = wl->if_ops->power(wl, true);
+	int ret = wl->if_ops->power(wl->dev, true);
 	if (ret == 0)
 		set_bit(WL1271_FLAG_GPIO_POWER, &wl->flags);
 
diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index d377e1f..628f7a3 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -30,6 +30,7 @@
 #include <linux/vmalloc.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
+#include <linux/wl12xx.h>
 
 #include "wl12xx.h"
 #include "wl12xx_80211.h"
@@ -799,7 +800,7 @@ static int wl1271_fetch_firmware(struct wl1271 *wl)
 
 	wl1271_debug(DEBUG_BOOT, "booting firmware %s", fw_name);
 
-	ret = request_firmware(&fw, fw_name, wl1271_wl_to_dev(wl));
+	ret = request_firmware(&fw, fw_name, wl->dev);
 
 	if (ret < 0) {
 		wl1271_error("could not get firmware: %d", ret);
@@ -838,7 +839,7 @@ static int wl1271_fetch_nvs(struct wl1271 *wl)
 	const struct firmware *fw;
 	int ret;
 
-	ret = request_firmware(&fw, WL1271_NVS_NAME, wl1271_wl_to_dev(wl));
+	ret = request_firmware(&fw, WL1271_NVS_NAME, wl->dev);
 
 	if (ret < 0) {
 		wl1271_error("could not get nvs file: %d", ret);
@@ -3377,7 +3378,7 @@ int wl1271_init_ieee80211(struct wl1271 *wl)
 
 	wl->hw->wiphy->reg_notifier = wl1271_reg_notify;
 
-	SET_IEEE80211_DEV(wl->hw, wl1271_wl_to_dev(wl));
+	SET_IEEE80211_DEV(wl->hw, wl->dev);
 
 	wl->hw->sta_data_size = sizeof(struct wl1271_station);
 
@@ -3547,13 +3548,96 @@ int wl1271_free_hw(struct wl1271 *wl)
 }
 EXPORT_SYMBOL_GPL(wl1271_free_hw);
 
+static irqreturn_t wl12xx_hardirq(int irq, void *_wl)
+{
+	struct wl1271		*wl = _wl;
+	unsigned long		flags;
+
+	wl1271_debug(DEBUG_IRQ, "IRQ");
+
+	/* complete the ELP completion */
+	spin_lock_irqsave(&wl->wl_lock, flags);
+	set_bit(WL1271_FLAG_IRQ_RUNNING, &wl->flags);
+	if (wl->elp_compl) {
+		complete(wl->elp_compl);
+		wl->elp_compl = NULL;
+	}
+	spin_unlock_irqrestore(&wl->wl_lock, flags);
+
+	return IRQ_WAKE_THREAD;
+}
+
 static int __devinit wl12xx_probe(struct platform_device *pdev)
 {
+	struct wl12xx_platform_data *pdata = pdev->dev.platform_data;
+	struct ieee80211_hw	*hw;
+	struct wl1271		*wl;
+
+	int			ret = -ENODEV;
+	int			irq;
+
+	hw = wl1271_alloc_hw();
+	if (IS_ERR(hw)) {
+		dev_err(&pdev->dev, "can't allocate hw\n");
+		goto err0;
+	}
+
+	irq = platform_get_irq(pdev, 0);
+
+	wl = hw->priv;
+
+	wl->ref_clock	= pdata->board_ref_clock;
+	wl->set_power	= pdata->set_power;
+	wl->irq		= irq;
+	wl->dev		= &pdev->dev;
+
+	platform_set_drvdata(pdev, wl);
+
+	ret = request_threaded_irq(wl->irq, wl12xx_hardirq, wl1271_irq,
+			IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
+			pdev->name, wl);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to request irq --> %d\n", ret);
+		goto err1;
+	}
+
+	disable_irq(wl->irq);
+
+	ret = wl1271_init_ieee80211(wl);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to initialize hw\n");
+		goto err2;
+	}
+
+	ret = wl1271_register_hw(wl);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to register hw\n");
+		goto err3;
+	}
+
 	return 0;
+
+err3:
+	wl1271_free_hw(wl);
+
+err2:
+	free_irq(wl->irq, wl);
+
+err1:
+	wl1271_free_hw(wl);
+
+err0:
+	return ret;
 }
 
 static int __devexit wl12xx_remove(struct platform_device *pdev)
 {
+	struct wl1271		*wl = platform_get_drvdata(pdev);
+
+	wl1271_unregister_hw(wl);
+	free_irq(wl->irq, wl);
+	wl1271_free_hw(wl);
+
 	return 0;
 }
 
diff --git a/drivers/net/wireless/wl12xx/sdio.c b/drivers/net/wireless/wl12xx/sdio.c
index 5c5e4f2..a0db640 100644
--- a/drivers/net/wireless/wl12xx/sdio.c
+++ b/drivers/net/wireless/wl12xx/sdio.c
@@ -48,7 +48,6 @@
 
 struct wl12xx_sdio_glue {
 	struct device		*dev;
-	struct wl1271		*wl;
 	struct platform_device	*core;
 };
 
@@ -58,49 +57,10 @@ static const struct sdio_device_id wl1271_devices[] __devinitconst = {
 };
 MODULE_DEVICE_TABLE(sdio, wl1271_devices);
 
-static inline struct wl12xx_sdio_glue *wl_to_glue(struct wl1271 *wl)
-{
-	return wl->if_priv;
-}
-
-static struct device *wl1271_sdio_wl_to_dev(struct wl1271 *wl)
-{
-	return wl_to_glue(wl)->dev;
-}
-
-static irqreturn_t wl1271_hardirq(int irq, void *cookie)
-{
-	struct wl1271 *wl = cookie;
-	unsigned long flags;
-
-	wl1271_debug(DEBUG_IRQ, "IRQ");
-
-	/* complete the ELP completion */
-	spin_lock_irqsave(&wl->wl_lock, flags);
-	set_bit(WL1271_FLAG_IRQ_RUNNING, &wl->flags);
-	if (wl->elp_compl) {
-		complete(wl->elp_compl);
-		wl->elp_compl = NULL;
-	}
-	spin_unlock_irqrestore(&wl->wl_lock, flags);
-
-	return IRQ_WAKE_THREAD;
-}
-
-static void wl1271_sdio_disable_interrupts(struct wl1271 *wl)
-{
-	disable_irq(wl->irq);
-}
-
-static void wl1271_sdio_enable_interrupts(struct wl1271 *wl)
-{
-	enable_irq(wl->irq);
-}
-
-static void wl1271_sdio_raw_read(struct wl1271 *wl, int addr, void *buf,
+static void wl1271_sdio_raw_read(struct device *child, int addr, void *buf,
 				 size_t len, bool fixed)
 {
-	struct wl12xx_sdio_glue	*glue = wl_to_glue(wl);
+	struct wl12xx_sdio_glue	*glue = dev_get_drvdata(child->parent);
 	struct sdio_func	*func = dev_to_sdio_func(glue->dev);
 	int			ret;
 
@@ -123,10 +83,10 @@ static void wl1271_sdio_raw_read(struct wl1271 *wl, int addr, void *buf,
 		wl1271_error("sdio read failed (%d)", ret);
 }
 
-static void wl1271_sdio_raw_write(struct wl1271 *wl, int addr, void *buf,
+static void wl1271_sdio_raw_write(struct device *child, int addr, void *buf,
 				  size_t len, bool fixed)
 {
-	struct wl12xx_sdio_glue	*glue = wl_to_glue(wl);
+	struct wl12xx_sdio_glue	*glue = dev_get_drvdata(child->parent);
 	struct sdio_func	*func = dev_to_sdio_func(glue->dev);
 	int			ret;
 
@@ -149,9 +109,8 @@ static void wl1271_sdio_raw_write(struct wl1271 *wl, int addr, void *buf,
 		wl1271_error("sdio write failed (%d)", ret);
 }
 
-static int wl1271_sdio_power_on(struct wl1271 *wl)
+static int wl1271_sdio_power_on(struct wl12xx_sdio_glue *glue)
 {
-	struct wl12xx_sdio_glue	*glue = wl_to_glue(wl);
 	struct sdio_func	*func = dev_to_sdio_func(glue->dev);
 	int			ret;
 
@@ -172,9 +131,8 @@ out:
 	return ret;
 }
 
-static int wl1271_sdio_power_off(struct wl1271 *wl)
+static int wl1271_sdio_power_off(struct wl12xx_sdio_glue *glue)
 {
-	struct wl12xx_sdio_glue	*glue = wl_to_glue(wl);
 	struct sdio_func	*func = dev_to_sdio_func(glue->dev);
 	int			ret;
 
@@ -190,33 +148,30 @@ static int wl1271_sdio_power_off(struct wl1271 *wl)
 	return pm_runtime_put_sync(&func->dev);
 }
 
-static int wl1271_sdio_set_power(struct wl1271 *wl, bool enable)
+static int wl1271_sdio_set_power(struct device *child, bool enable)
 {
+	struct wl12xx_sdio_glue	*glue = dev_get_drvdata(child->parent);
+
 	if (enable)
-		return wl1271_sdio_power_on(wl);
+		return wl1271_sdio_power_on(glue);
 	else
-		return wl1271_sdio_power_off(wl);
+		return wl1271_sdio_power_off(glue);
 }
 
 static struct wl1271_if_operations sdio_ops = {
 	.read		= wl1271_sdio_raw_read,
 	.write		= wl1271_sdio_raw_write,
 	.power		= wl1271_sdio_set_power,
-	.dev		= wl1271_sdio_wl_to_dev,
-	.enable_irq	= wl1271_sdio_enable_interrupts,
-	.disable_irq	= wl1271_sdio_disable_interrupts
 };
 
 static int __devinit wl1271_probe(struct sdio_func *func,
 				  const struct sdio_device_id *id)
 {
-	const struct wl12xx_platform_data *wlan_data;
+	struct wl12xx_platform_data	*wlan_data;
 
 	struct platform_device		*core;
 	struct resource			res[1];
 	struct wl12xx_sdio_glue		*glue;
-	struct ieee80211_hw		*hw;
-	struct wl1271			*wl;
 
 	int				ret = -ENOMEM;
 
@@ -230,20 +185,7 @@ static int __devinit wl1271_probe(struct sdio_func *func,
 		goto err0;
 	}
 
-	hw = wl1271_alloc_hw();
-	if (IS_ERR(hw)) {
-		dev_err(&func->dev, "can't allocate hw\n");
-		ret = PTR_ERR(hw);
-		goto err1;
-	}
-
-	wl = hw->priv;
-
-	wl->if_priv = glue;
-	wl->if_ops = &sdio_ops;
-
 	glue->dev = &func->dev;
-	glue->wl = wl;
 
 	/* Grab access to FN0 for ELP reg. */
 	func->card->quirks |= MMC_QUIRK_LENIENT_FN0;
@@ -252,29 +194,10 @@ static int __devinit wl1271_probe(struct sdio_func *func,
 	if (IS_ERR(wlan_data)) {
 		ret = PTR_ERR(wlan_data);
 		wl1271_error("missing wlan platform data: %d", ret);
-		goto err2;
-	}
-
-	wl->irq = wlan_data->irq;
-	wl->ref_clock = wlan_data->board_ref_clock;
-
-	ret = request_threaded_irq(wl->irq, wl1271_hardirq, wl1271_irq,
-				   IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
-				   DRIVER_NAME, wl);
-	if (ret < 0) {
-		wl1271_error("request_irq() failed: %d", ret);
-		goto err2;
+		goto err1;
 	}
 
-	disable_irq(wl->irq);
-
-	ret = wl1271_init_ieee80211(wl);
-	if (ret)
-		goto err3;
-
-	ret = wl1271_register_hw(wl);
-	if (ret)
-		goto err3;
+	wlan_data->ops = &sdio_ops;
 
 	sdio_set_drvdata(func, glue);
 
@@ -285,7 +208,7 @@ static int __devinit wl1271_probe(struct sdio_func *func,
 	if (!core) {
 		dev_err(&func->dev, "can't allocate platform_device\n");
 		ret = -ENOMEM;
-		goto err4;
+		goto err1;
 	}
 
 	core->dev.parent = &func->dev;
@@ -299,34 +222,25 @@ static int __devinit wl1271_probe(struct sdio_func *func,
 	ret = platform_device_add_resources(core, res, ARRAY_SIZE(res));
 	if (ret) {
 		dev_err(&func->dev, "can't add resources\n");
-		goto err5;
+		goto err2;
 	}
 
 	ret = platform_device_add_data(core, wlan_data, sizeof(*wlan_data));
 	if (ret) {
 		dev_err(&func->dev, "can't add platform data\n");
-		goto err5;
+		goto err2;
 	}
 
 	ret = platform_device_register(core);
 	if (ret) {
 		dev_err(&func->dev, "can't register platform device\n");
-		goto err5;
+		goto err2;
 	}
 
 	return 0;
 
-err5:
-	platform_device_put(core);
-
-err4:
-	wl1271_unregister_hw(wl);
-
-err3:
-	free_irq(wl->irq, wl);
-
 err2:
-	wl1271_free_hw(wl);
+	platform_device_put(core);
 
 err1:
 	kfree(glue);
@@ -338,14 +252,10 @@ err0:
 static void __devexit wl1271_remove(struct sdio_func *func)
 {
 	struct wl12xx_sdio_glue	*glue = sdio_get_drvdata(func);
-	struct wl1271		*wl = glue->wl;
 
 	/* Undo decrement done above in wl1271_probe */
 	pm_runtime_get_noresume(&func->dev);
 
-	wl1271_unregister_hw(wl);
-	free_irq(wl->irq, wl);
-	wl1271_free_hw(wl);
 	platform_device_del(glue->core);
 	platform_device_put(glue->core);
 	kfree(glue);
diff --git a/drivers/net/wireless/wl12xx/spi.c b/drivers/net/wireless/wl12xx/spi.c
index 6644996..edc2c64 100644
--- a/drivers/net/wireless/wl12xx/spi.c
+++ b/drivers/net/wireless/wl12xx/spi.c
@@ -71,33 +71,12 @@
 
 struct wl12xx_spi_glue {
 	struct device		*dev;
-	struct wl1271		*wl;
 	struct platform_device	*core;
 };
 
-static inline struct wl12xx_spi_glue *wl_to_glue(struct wl1271 *wl)
+static void wl1271_spi_reset(struct device *child)
 {
-	return wl->if_priv;
-}
-
-static struct device *wl1271_spi_wl_to_dev(struct wl1271 *wl)
-{
-	return wl_to_glue(wl)->dev;
-}
-
-static void wl1271_spi_disable_interrupts(struct wl1271 *wl)
-{
-	disable_irq(wl->irq);
-}
-
-static void wl1271_spi_enable_interrupts(struct wl1271 *wl)
-{
-	enable_irq(wl->irq);
-}
-
-static void wl1271_spi_reset(struct wl1271 *wl)
-{
-	struct wl12xx_spi_glue	*glue = wl_to_glue(wl);
+	struct wl12xx_spi_glue	*glue = dev_get_drvdata(child->parent);
 	struct spi_transfer	t;
 	struct spi_message	m;
 	u8			*cmd;
@@ -123,9 +102,9 @@ static void wl1271_spi_reset(struct wl1271 *wl)
 	kfree(cmd);
 }
 
-static void wl1271_spi_init(struct wl1271 *wl)
+static void wl1271_spi_init(struct device *child)
 {
-	struct wl12xx_spi_glue	*glue = wl_to_glue(wl);
+	struct wl12xx_spi_glue	*glue = dev_get_drvdata(child->parent);
 	struct spi_transfer	t;
 	struct spi_message	m;
 
@@ -182,9 +161,10 @@ static void wl1271_spi_init(struct wl1271 *wl)
 
 #define WL1271_BUSY_WORD_TIMEOUT 1000
 
-static int wl1271_spi_read_busy(struct wl1271 *wl)
+static int wl1271_spi_read_busy(struct device *child)
 {
-	struct wl12xx_spi_glue	*glue = wl_to_glue(wl);
+	struct wl12xx_spi_glue	*glue = dev_get_drvdata(child->parent);
+	struct wl1271		*wl = dev_get_drvdata(child);
 	struct spi_transfer	t[1];
 	struct spi_message	m;
 
@@ -217,10 +197,11 @@ static int wl1271_spi_read_busy(struct wl1271 *wl)
 	return -ETIMEDOUT;
 }
 
-static void wl1271_spi_raw_read(struct wl1271 *wl, int addr, void *buf,
+static void wl1271_spi_raw_read(struct device *child, int addr, void *buf,
 				size_t len, bool fixed)
 {
-	struct wl12xx_spi_glue	*glue = wl_to_glue(wl);
+	struct wl12xx_spi_glue	*glue = dev_get_drvdata(child->parent);
+	struct wl1271		*wl = dev_get_drvdata(child);
 	struct spi_transfer	t[2];
 	struct spi_message	m;
 
@@ -260,7 +241,7 @@ static void wl1271_spi_raw_read(struct wl1271 *wl, int addr, void *buf,
 		spi_sync(to_spi_device(glue->dev), &m);
 
 		if (!(busy_buf[WL1271_BUSY_WORD_CNT - 1] & 0x1) &&
-		    wl1271_spi_read_busy(wl)) {
+		    wl1271_spi_read_busy(child)) {
 			memset(buf, 0, chunk_len);
 			return;
 		}
@@ -285,10 +266,10 @@ static void wl1271_spi_raw_read(struct wl1271 *wl, int addr, void *buf,
 	}
 }
 
-static void wl1271_spi_raw_write(struct wl1271 *wl, int addr, void *buf,
+static void wl1271_spi_raw_write(struct device *child, int addr, void *buf,
 			  size_t len, bool fixed)
 {
-	struct wl12xx_spi_glue	*glue = wl_to_glue(wl);
+	struct wl12xx_spi_glue	*glue = dev_get_drvdata(child->parent);
 	struct spi_transfer	t[2 * WSPI_MAX_NUM_OF_CHUNKS];
 	struct spi_message	m;
 
@@ -338,42 +319,11 @@ static void wl1271_spi_raw_write(struct wl1271 *wl, int addr, void *buf,
 	spi_sync(to_spi_device(glue->dev), &m);
 }
 
-static irqreturn_t wl1271_hardirq(int irq, void *cookie)
-{
-	struct wl1271 *wl = cookie;
-	unsigned long flags;
-
-	wl1271_debug(DEBUG_IRQ, "IRQ");
-
-	/* complete the ELP completion */
-	spin_lock_irqsave(&wl->wl_lock, flags);
-	set_bit(WL1271_FLAG_IRQ_RUNNING, &wl->flags);
-	if (wl->elp_compl) {
-		complete(wl->elp_compl);
-		wl->elp_compl = NULL;
-	}
-	spin_unlock_irqrestore(&wl->wl_lock, flags);
-
-	return IRQ_WAKE_THREAD;
-}
-
-static int wl1271_spi_set_power(struct wl1271 *wl, bool enable)
-{
-	if (wl->set_power)
-		wl->set_power(enable);
-
-	return 0;
-}
-
 static struct wl1271_if_operations spi_ops = {
 	.read		= wl1271_spi_raw_read,
 	.write		= wl1271_spi_raw_write,
 	.reset		= wl1271_spi_reset,
 	.init		= wl1271_spi_init,
-	.power		= wl1271_spi_set_power,
-	.dev		= wl1271_spi_wl_to_dev,
-	.enable_irq	= wl1271_spi_enable_interrupts,
-	.disable_irq	= wl1271_spi_disable_interrupts
 };
 
 static int __devinit wl1271_probe(struct spi_device *spi)
@@ -381,10 +331,7 @@ static int __devinit wl1271_probe(struct spi_device *spi)
 	struct wl12xx_platform_data	*pdata;
 	struct platform_device		*core;
 	struct resource			res[1];
-
 	struct wl12xx_spi_glue		*glue;
-	struct ieee80211_hw		*hw;
-	struct wl1271			*wl;
 
 	int				ret = -ENOMEM;
 
@@ -394,27 +341,16 @@ static int __devinit wl1271_probe(struct spi_device *spi)
 		return -ENODEV;
 	}
 
+	pdata->ops = &spi_ops;
+
 	glue = kzalloc(sizeof(*glue), GFP_KERNEL);
 	if (!glue) {
 		dev_err(&spi->dev, "not enought memory\n");
 		goto err0;
 	}
 
-	hw = wl1271_alloc_hw();
-	if (IS_ERR(hw)) {
-		ret = PTR_ERR(hw);
-		goto err1;
-	}
-
-	wl = hw->priv;
-
 	glue->dev = &spi->dev;
-	glue->wl = wl;
-
 	spi_set_drvdata(spi, glue);
-	wl->if_priv = glue;
-
-	wl->if_ops = &spi_ops;
 
 	/* This is the only SPI value that we need to set here, the rest
 	 * comes from the board-peripherals file */
@@ -423,48 +359,14 @@ static int __devinit wl1271_probe(struct spi_device *spi)
 	ret = spi_setup(spi);
 	if (ret < 0) {
 		wl1271_error("spi_setup failed");
-		goto err2;
-	}
-
-	wl->set_power = pdata->set_power;
-	if (!wl->set_power) {
-		wl1271_error("set power function missing in platform data");
-		ret = -ENODEV;
-		goto err2;
-	}
-
-	wl->ref_clock = pdata->board_ref_clock;
-
-	wl->irq = spi->irq;
-	if (wl->irq < 0) {
-		wl1271_error("irq missing in platform data");
-		ret = -ENODEV;
-		goto err2;
-	}
-
-	ret = request_threaded_irq(wl->irq, wl1271_hardirq, wl1271_irq,
-				   IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
-				   DRIVER_NAME, wl);
-	if (ret < 0) {
-		wl1271_error("request_irq() failed: %d", ret);
-		goto err2;
+		goto err1;
 	}
 
-	disable_irq(wl->irq);
-
-	ret = wl1271_init_ieee80211(wl);
-	if (ret)
-		goto err3;
-
-	ret = wl1271_register_hw(wl);
-	if (ret)
-		goto err3;
-
 	core = platform_device_alloc("wl12xx-spi", -1);
 	if (!core) {
 		dev_err(&spi->dev, "can't allocate platform_device\n");
 		ret = -ENOMEM;
-		goto err4;
+		goto err1;
 	}
 
 	core->dev.parent = &spi->dev;
@@ -478,34 +380,25 @@ static int __devinit wl1271_probe(struct spi_device *spi)
 	ret = platform_device_add_resources(core, res, ARRAY_SIZE(res));
 	if (ret) {
 		dev_err(&spi->dev, "can't add resources\n");
-		goto err5;
+		goto err2;
 	}
 
 	ret = platform_device_add_data(core, pdata, sizeof(*pdata));
 	if (ret) {
 		dev_err(&spi->dev, "can't add platform data\n");
-		goto err5;
+		goto err2;
 	}
 
 	ret = platform_device_register(core);
 	if (ret) {
 		dev_err(&spi->dev, "can't register platform device\n");
-		goto err5;
+		goto err2;
 	}
 
 	return 0;
 
-err5:
-	platform_device_put(core);
-
-err4:
-	wl1271_unregister_hw(wl);
-
-err3:
-	free_irq(wl->irq, wl);
-
 err2:
-	wl1271_free_hw(wl);
+	platform_device_put(core);
 
 err1:
 	kfree(glue);
@@ -517,11 +410,7 @@ err0:
 static int __devexit wl1271_remove(struct spi_device *spi)
 {
 	struct wl12xx_spi_glue	*glue = spi_get_drvdata(spi);
-	struct wl1271		*wl = glue->wl;
 
-	wl1271_unregister_hw(wl);
-	free_irq(wl->irq, wl);
-	wl1271_free_hw(wl);
 	platform_device_del(glue->core);
 	platform_device_put(glue->core);
 	kfree(glue);
diff --git a/drivers/net/wireless/wl12xx/wl12xx.h b/drivers/net/wireless/wl12xx/wl12xx.h
index 86be83e..eb3d3fc 100644
--- a/drivers/net/wireless/wl12xx/wl12xx.h
+++ b/drivers/net/wireless/wl12xx/wl12xx.h
@@ -287,16 +287,13 @@ struct wl1271_scan {
 };
 
 struct wl1271_if_operations {
-	void (*read)(struct wl1271 *wl, int addr, void *buf, size_t len,
+	void (*read)(struct device *child, int addr, void *buf, size_t len,
 		     bool fixed);
-	void (*write)(struct wl1271 *wl, int addr, void *buf, size_t len,
+	void (*write)(struct device *child, int addr, void *buf, size_t len,
 		     bool fixed);
-	void (*reset)(struct wl1271 *wl);
-	void (*init)(struct wl1271 *wl);
-	int (*power)(struct wl1271 *wl, bool enable);
-	struct device* (*dev)(struct wl1271 *wl);
-	void (*enable_irq)(struct wl1271 *wl);
-	void (*disable_irq)(struct wl1271 *wl);
+	void (*reset)(struct device *child);
+	void (*init)(struct device *child);
+	int (*power)(struct device *child, bool enable);
 };
 
 #define MAX_NUM_KEYS 14
@@ -346,6 +343,8 @@ struct wl1271 {
 	struct ieee80211_hw *hw;
 	bool mac80211_registered;
 
+	struct device *dev;
+
 	void *if_priv;
 
 	struct wl1271_if_operations *if_ops;
diff --git a/drivers/net/wireless/wl12xx/wl12xx_platform_data.c b/drivers/net/wireless/wl12xx/wl12xx_platform_data.c
index 973b110..3c96b33 100644
--- a/drivers/net/wireless/wl12xx/wl12xx_platform_data.c
+++ b/drivers/net/wireless/wl12xx/wl12xx_platform_data.c
@@ -2,7 +2,7 @@
 #include <linux/err.h>
 #include <linux/wl12xx.h>
 
-static const struct wl12xx_platform_data *platform_data;
+static struct wl12xx_platform_data *platform_data;
 
 int __init wl12xx_set_platform_data(const struct wl12xx_platform_data *data)
 {
@@ -18,7 +18,7 @@ int __init wl12xx_set_platform_data(const struct wl12xx_platform_data *data)
 	return 0;
 }
 
-const struct wl12xx_platform_data *wl12xx_get_platform_data(void)
+struct wl12xx_platform_data *wl12xx_get_platform_data(void)
 {
 	if (!platform_data)
 		return ERR_PTR(-ENODEV);
diff --git a/include/linux/wl12xx.h b/include/linux/wl12xx.h
index bebb8ef..98c17e3 100644
--- a/include/linux/wl12xx.h
+++ b/include/linux/wl12xx.h
@@ -38,6 +38,8 @@ struct wl12xx_platform_data {
 	int irq;
 	bool use_eeprom;
 	int board_ref_clock;
+
+	struct wl1271_if_operations *ops;
 };
 
 #ifdef CONFIG_WL12XX_PLATFORM_DATA
@@ -54,6 +56,6 @@ int wl12xx_set_platform_data(const struct wl12xx_platform_data *data)
 
 #endif
 
-const struct wl12xx_platform_data *wl12xx_get_platform_data(void);
+struct wl12xx_platform_data *wl12xx_get_platform_data(void);
 
 #endif
-- 
1.7.4.1.343.ga91df


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC/PATCH 12/13] net: wireless: wl12xx: mark some symbols static
@ 2011-05-13 21:26   ` Felipe Balbi
  0 siblings, 0 replies; 32+ messages in thread
From: Felipe Balbi @ 2011-05-13 21:26 UTC (permalink / raw)
  To: Luciano Coelho; +Cc: linux-wireless, netdev, linux-kernel, Felipe Balbi

after re-factoring a bunch of symbols are only
used inside main.c which allows us to mark
them as static.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/net/wireless/wl12xx/io.h   |    7 -------
 drivers/net/wireless/wl12xx/main.c |   18 ++++++------------
 2 files changed, 6 insertions(+), 19 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/io.h b/drivers/net/wireless/wl12xx/io.h
index 2bb2295..34ead79 100644
--- a/drivers/net/wireless/wl12xx/io.h
+++ b/drivers/net/wireless/wl12xx/io.h
@@ -157,11 +157,4 @@ int wl1271_set_partition(struct wl1271 *wl,
 
 /* Functions from wl1271_main.c */
 
-int wl1271_register_hw(struct wl1271 *wl);
-void wl1271_unregister_hw(struct wl1271 *wl);
-int wl1271_init_ieee80211(struct wl1271 *wl);
-struct ieee80211_hw *wl1271_alloc_hw(void);
-int wl1271_free_hw(struct wl1271 *wl);
-irqreturn_t wl1271_irq(int irq, void *data);
-
 #endif
diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index 628f7a3..d3c056d 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -661,7 +661,7 @@ static void wl1271_netstack_work(struct work_struct *work)
 
 #define WL1271_IRQ_MAX_LOOPS 256
 
-irqreturn_t wl1271_irq(int irq, void *cookie)
+static irqreturn_t wl1271_irq(int irq, void *cookie)
 {
 	int ret;
 	u32 intr;
@@ -776,7 +776,6 @@ out:
 
 	return IRQ_HANDLED;
 }
-EXPORT_SYMBOL_GPL(wl1271_irq);
 
 static int wl1271_fetch_firmware(struct wl1271 *wl)
 {
@@ -3265,7 +3264,7 @@ static ssize_t wl1271_sysfs_show_hw_pg_ver(struct device *dev,
 static DEVICE_ATTR(hw_pg_ver, S_IRUGO | S_IWUSR,
 		   wl1271_sysfs_show_hw_pg_ver, NULL);
 
-int wl1271_register_hw(struct wl1271 *wl)
+static int wl1271_register_hw(struct wl1271 *wl)
 {
 	int ret;
 
@@ -3302,9 +3301,8 @@ int wl1271_register_hw(struct wl1271 *wl)
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(wl1271_register_hw);
 
-void wl1271_unregister_hw(struct wl1271 *wl)
+static void wl1271_unregister_hw(struct wl1271 *wl)
 {
 	if (wl->state == WL1271_STATE_PLT)
 		__wl1271_plt_stop(wl);
@@ -3314,9 +3312,8 @@ void wl1271_unregister_hw(struct wl1271 *wl)
 	wl->mac80211_registered = false;
 
 }
-EXPORT_SYMBOL_GPL(wl1271_unregister_hw);
 
-int wl1271_init_ieee80211(struct wl1271 *wl)
+static int wl1271_init_ieee80211(struct wl1271 *wl)
 {
 	static const u32 cipher_suites[] = {
 		WLAN_CIPHER_SUITE_WEP40,
@@ -3386,11 +3383,10 @@ int wl1271_init_ieee80211(struct wl1271 *wl)
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(wl1271_init_ieee80211);
 
 #define WL1271_DEFAULT_CHANNEL 0
 
-struct ieee80211_hw *wl1271_alloc_hw(void)
+static struct ieee80211_hw *wl1271_alloc_hw(void)
 {
 	struct ieee80211_hw *hw;
 	struct platform_device *plat_dev = NULL;
@@ -3523,9 +3519,8 @@ err_hw_alloc:
 
 	return ERR_PTR(ret);
 }
-EXPORT_SYMBOL_GPL(wl1271_alloc_hw);
 
-int wl1271_free_hw(struct wl1271 *wl)
+static int wl1271_free_hw(struct wl1271 *wl)
 {
 	platform_device_unregister(wl->plat_dev);
 	free_pages((unsigned long)wl->aggr_buf,
@@ -3546,7 +3541,6 @@ int wl1271_free_hw(struct wl1271 *wl)
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(wl1271_free_hw);
 
 static irqreturn_t wl12xx_hardirq(int irq, void *_wl)
 {
-- 
1.7.4.1.343.ga91df


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC/PATCH 12/13] net: wireless: wl12xx: mark some symbols static
@ 2011-05-13 21:26   ` Felipe Balbi
  0 siblings, 0 replies; 32+ messages in thread
From: Felipe Balbi @ 2011-05-13 21:26 UTC (permalink / raw)
  To: Luciano Coelho
  Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Felipe Balbi

after re-factoring a bunch of symbols are only
used inside main.c which allows us to mark
them as static.

Signed-off-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
---
 drivers/net/wireless/wl12xx/io.h   |    7 -------
 drivers/net/wireless/wl12xx/main.c |   18 ++++++------------
 2 files changed, 6 insertions(+), 19 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/io.h b/drivers/net/wireless/wl12xx/io.h
index 2bb2295..34ead79 100644
--- a/drivers/net/wireless/wl12xx/io.h
+++ b/drivers/net/wireless/wl12xx/io.h
@@ -157,11 +157,4 @@ int wl1271_set_partition(struct wl1271 *wl,
 
 /* Functions from wl1271_main.c */
 
-int wl1271_register_hw(struct wl1271 *wl);
-void wl1271_unregister_hw(struct wl1271 *wl);
-int wl1271_init_ieee80211(struct wl1271 *wl);
-struct ieee80211_hw *wl1271_alloc_hw(void);
-int wl1271_free_hw(struct wl1271 *wl);
-irqreturn_t wl1271_irq(int irq, void *data);
-
 #endif
diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index 628f7a3..d3c056d 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -661,7 +661,7 @@ static void wl1271_netstack_work(struct work_struct *work)
 
 #define WL1271_IRQ_MAX_LOOPS 256
 
-irqreturn_t wl1271_irq(int irq, void *cookie)
+static irqreturn_t wl1271_irq(int irq, void *cookie)
 {
 	int ret;
 	u32 intr;
@@ -776,7 +776,6 @@ out:
 
 	return IRQ_HANDLED;
 }
-EXPORT_SYMBOL_GPL(wl1271_irq);
 
 static int wl1271_fetch_firmware(struct wl1271 *wl)
 {
@@ -3265,7 +3264,7 @@ static ssize_t wl1271_sysfs_show_hw_pg_ver(struct device *dev,
 static DEVICE_ATTR(hw_pg_ver, S_IRUGO | S_IWUSR,
 		   wl1271_sysfs_show_hw_pg_ver, NULL);
 
-int wl1271_register_hw(struct wl1271 *wl)
+static int wl1271_register_hw(struct wl1271 *wl)
 {
 	int ret;
 
@@ -3302,9 +3301,8 @@ int wl1271_register_hw(struct wl1271 *wl)
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(wl1271_register_hw);
 
-void wl1271_unregister_hw(struct wl1271 *wl)
+static void wl1271_unregister_hw(struct wl1271 *wl)
 {
 	if (wl->state == WL1271_STATE_PLT)
 		__wl1271_plt_stop(wl);
@@ -3314,9 +3312,8 @@ void wl1271_unregister_hw(struct wl1271 *wl)
 	wl->mac80211_registered = false;
 
 }
-EXPORT_SYMBOL_GPL(wl1271_unregister_hw);
 
-int wl1271_init_ieee80211(struct wl1271 *wl)
+static int wl1271_init_ieee80211(struct wl1271 *wl)
 {
 	static const u32 cipher_suites[] = {
 		WLAN_CIPHER_SUITE_WEP40,
@@ -3386,11 +3383,10 @@ int wl1271_init_ieee80211(struct wl1271 *wl)
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(wl1271_init_ieee80211);
 
 #define WL1271_DEFAULT_CHANNEL 0
 
-struct ieee80211_hw *wl1271_alloc_hw(void)
+static struct ieee80211_hw *wl1271_alloc_hw(void)
 {
 	struct ieee80211_hw *hw;
 	struct platform_device *plat_dev = NULL;
@@ -3523,9 +3519,8 @@ err_hw_alloc:
 
 	return ERR_PTR(ret);
 }
-EXPORT_SYMBOL_GPL(wl1271_alloc_hw);
 
-int wl1271_free_hw(struct wl1271 *wl)
+static int wl1271_free_hw(struct wl1271 *wl)
 {
 	platform_device_unregister(wl->plat_dev);
 	free_pages((unsigned long)wl->aggr_buf,
@@ -3546,7 +3541,6 @@ int wl1271_free_hw(struct wl1271 *wl)
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(wl1271_free_hw);
 
 static irqreturn_t wl12xx_hardirq(int irq, void *_wl)
 {
-- 
1.7.4.1.343.ga91df

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC/PATCH 13/13] net: wl12xx: main: drop unneded plat_dev
  2011-05-13 21:26 [RFC/PATCH 00/13] wl12xx re-factor Felipe Balbi
                   ` (11 preceding siblings ...)
  2011-05-13 21:26   ` Felipe Balbi
@ 2011-05-13 21:26 ` Felipe Balbi
  2011-05-19 12:49 ` [RFC/PATCH 00/13] wl12xx re-factor Luciano Coelho
  13 siblings, 0 replies; 32+ messages in thread
From: Felipe Balbi @ 2011-05-13 21:26 UTC (permalink / raw)
  To: Luciano Coelho; +Cc: linux-wireless, netdev, linux-kernel, Felipe Balbi

now that useless plat_dev is unnecessary,
we can remove it.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/net/wireless/wl12xx/main.c   |   48 +++-------------------------------
 drivers/net/wireless/wl12xx/wl12xx.h |    1 -
 2 files changed, 4 insertions(+), 45 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index d3c056d..55cedcd 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -315,21 +315,6 @@ static void __wl1271_op_remove_interface(struct wl1271 *wl);
 static void wl1271_free_ap_keys(struct wl1271 *wl);
 
 
-static void wl1271_device_release(struct device *dev)
-{
-
-}
-
-static struct platform_device wl1271_device = {
-	.name           = "wl1271",
-	.id             = -1,
-
-	/* device model insists to have a release function */
-	.dev            = {
-		.release = wl1271_device_release,
-	},
-};
-
 static LIST_HEAD(wl_list);
 
 static int wl1271_dev_notify(struct notifier_block *me, unsigned long what,
@@ -3389,7 +3374,6 @@ static int wl1271_init_ieee80211(struct wl1271 *wl)
 static struct ieee80211_hw *wl1271_alloc_hw(void)
 {
 	struct ieee80211_hw *hw;
-	struct platform_device *plat_dev = NULL;
 	struct wl1271 *wl;
 	int i, j, ret;
 	unsigned int order;
@@ -3401,20 +3385,12 @@ static struct ieee80211_hw *wl1271_alloc_hw(void)
 		goto err_hw_alloc;
 	}
 
-	plat_dev = kmemdup(&wl1271_device, sizeof(wl1271_device), GFP_KERNEL);
-	if (!plat_dev) {
-		wl1271_error("could not allocate platform_device");
-		ret = -ENOMEM;
-		goto err_plat_alloc;
-	}
-
 	wl = hw->priv;
 	memset(wl, 0, sizeof(*wl));
 
 	INIT_LIST_HEAD(&wl->list);
 
 	wl->hw = hw;
-	wl->plat_dev = plat_dev;
 
 	for (i = 0; i < NUM_TX_QUEUES; i++)
 		skb_queue_head_init(&wl->tx_queue[i]);
@@ -3475,23 +3451,15 @@ static struct ieee80211_hw *wl1271_alloc_hw(void)
 		goto err_hw;
 	}
 
-	/* Register platform device */
-	ret = platform_device_register(wl->plat_dev);
-	if (ret) {
-		wl1271_error("couldn't register platform device");
-		goto err_aggr;
-	}
-	dev_set_drvdata(&wl->plat_dev->dev, wl);
-
 	/* Create sysfs file to control bt coex state */
-	ret = device_create_file(&wl->plat_dev->dev, &dev_attr_bt_coex_state);
+	ret = device_create_file(wl->dev, &dev_attr_bt_coex_state);
 	if (ret < 0) {
 		wl1271_error("failed to create sysfs file bt_coex_state");
-		goto err_platform;
+		goto err_aggr;
 	}
 
 	/* Create sysfs file to get HW PG version */
-	ret = device_create_file(&wl->plat_dev->dev, &dev_attr_hw_pg_ver);
+	ret = device_create_file(wl->dev, &dev_attr_hw_pg_ver);
 	if (ret < 0) {
 		wl1271_error("failed to create sysfs file hw_pg_ver");
 		goto err_bt_coex_state;
@@ -3500,19 +3468,13 @@ static struct ieee80211_hw *wl1271_alloc_hw(void)
 	return hw;
 
 err_bt_coex_state:
-	device_remove_file(&wl->plat_dev->dev, &dev_attr_bt_coex_state);
-
-err_platform:
-	platform_device_unregister(wl->plat_dev);
+	device_remove_file(wl->dev, &dev_attr_bt_coex_state);
 
 err_aggr:
 	free_pages((unsigned long)wl->aggr_buf, order);
 
 err_hw:
 	wl1271_debugfs_exit(wl);
-	kfree(plat_dev);
-
-err_plat_alloc:
 	ieee80211_free_hw(hw);
 
 err_hw_alloc:
@@ -3522,10 +3484,8 @@ err_hw_alloc:
 
 static int wl1271_free_hw(struct wl1271 *wl)
 {
-	platform_device_unregister(wl->plat_dev);
 	free_pages((unsigned long)wl->aggr_buf,
 			get_order(WL1271_AGGR_BUFFER_SIZE));
-	kfree(wl->plat_dev);
 
 	wl1271_debugfs_exit(wl);
 
diff --git a/drivers/net/wireless/wl12xx/wl12xx.h b/drivers/net/wireless/wl12xx/wl12xx.h
index eb3d3fc..279bd82 100644
--- a/drivers/net/wireless/wl12xx/wl12xx.h
+++ b/drivers/net/wireless/wl12xx/wl12xx.h
@@ -339,7 +339,6 @@ struct wl1271_link {
 };
 
 struct wl1271 {
-	struct platform_device *plat_dev;
 	struct ieee80211_hw *hw;
 	bool mac80211_registered;
 
-- 
1.7.4.1.343.ga91df


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [RFC/PATCH 00/13] wl12xx re-factor
  2011-05-13 21:26 [RFC/PATCH 00/13] wl12xx re-factor Felipe Balbi
                   ` (12 preceding siblings ...)
  2011-05-13 21:26 ` [RFC/PATCH 13/13] net: wl12xx: main: drop unneded plat_dev Felipe Balbi
@ 2011-05-19 12:49 ` Luciano Coelho
  2011-05-19 14:06   ` Felipe Balbi
  13 siblings, 1 reply; 32+ messages in thread
From: Luciano Coelho @ 2011-05-19 12:49 UTC (permalink / raw)
  To: Felipe Balbi; +Cc: linux-wireless, netdev, linux-kernel

On Sat, 2011-05-14 at 00:26 +0300, Felipe Balbi wrote:
> Hi Luca,

Hi Felipe,


> this is the re-factor I was talking to you
> about. Please have a look and give your
> comments.
> 
> It probably won't work as is, I compile
> tested only, but it shows the idea.

This looks very good! I think we should do something like this to avoid
the code that is duplicated in the bus modules.

But, as I already mentioned briefly on IRC, there is a problem with the
way you changed the platform data structure, because it will break
compat-wireless.  The actual memory and data that is used by the
platform data is in the board components and not part of the wireless
subsystem.  With compat-wireless, we need to make sure that new stuff
works with older kernels.  In your patches you modify the platform data
structure, so when we run an old kernel with new compat-wireless, things
will break.

We already found a similar bug due to a previous change in the platform
data structure, so I don't want this to happen again.  So for now, I'll
keep these patches aside, but as soon as we find a good solution, I'll
definitely use your ideas here (or ask you to rebase :P).

I'll probably apply some of the patches that are not related to the
platform data change.  I'll respond to those specific patches
separately.

-- 
Cheers,
Luca.


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC/PATCH 00/13] wl12xx re-factor
  2011-05-19 12:49 ` [RFC/PATCH 00/13] wl12xx re-factor Luciano Coelho
@ 2011-05-19 14:06   ` Felipe Balbi
  0 siblings, 0 replies; 32+ messages in thread
From: Felipe Balbi @ 2011-05-19 14:06 UTC (permalink / raw)
  To: Luciano Coelho; +Cc: Felipe Balbi, linux-wireless, netdev, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1589 bytes --]

Hi,

On Thu, May 19, 2011 at 03:49:05PM +0300, Luciano Coelho wrote:
> > this is the re-factor I was talking to you
> > about. Please have a look and give your
> > comments.
> > 
> > It probably won't work as is, I compile
> > tested only, but it shows the idea.
> 
> This looks very good! I think we should do something like this to avoid
> the code that is duplicated in the bus modules.
> 
> But, as I already mentioned briefly on IRC, there is a problem with the
> way you changed the platform data structure, because it will break
> compat-wireless.  The actual memory and data that is used by the
> platform data is in the board components and not part of the wireless
> subsystem.  With compat-wireless, we need to make sure that new stuff
> works with older kernels.  In your patches you modify the platform data
> structure, so when we run an old kernel with new compat-wireless, things
> will break.
> 
> We already found a similar bug due to a previous change in the platform
> data structure, so I don't want this to happen again.  So for now, I'll
> keep these patches aside, but as soon as we find a good solution, I'll
> definitely use your ideas here (or ask you to rebase :P).
> 
> I'll probably apply some of the patches that are not related to the
> platform data change.  I'll respond to those specific patches
> separately.

ok good. I have stated my POV WRT compatibility layers for in-tree
drivers on a separate thread. In summary, I think those shouldn't exist
at all :-)

Just let me know if you need anything ;-)

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC/PATCH 01/13] net: wl12xx: sdio: id_tables should be __devinitconst
  2011-05-13 21:26 ` [RFC/PATCH 01/13] net: wl12xx: sdio: id_tables should be __devinitconst Felipe Balbi
@ 2011-05-20 12:02   ` Luciano Coelho
  2011-05-20 16:02   ` Ohad Ben-Cohen
  1 sibling, 0 replies; 32+ messages in thread
From: Luciano Coelho @ 2011-05-20 12:02 UTC (permalink / raw)
  To: Felipe Balbi; +Cc: linux-wireless, netdev, linux-kernel

On Sat, 2011-05-14 at 00:26 +0300, Felipe Balbi wrote:
> That's only needed during init anyway, let's free
> some space after we're done probing.
> 
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> ---

Applied, this one.  Thanks, Felipe!

-- 
Cheers,
Luca.


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC/PATCH 03/13] net: wl12xx: remove some unnecessary prints
  2011-05-13 21:26   ` Felipe Balbi
  (?)
@ 2011-05-20 12:03   ` Luciano Coelho
  -1 siblings, 0 replies; 32+ messages in thread
From: Luciano Coelho @ 2011-05-20 12:03 UTC (permalink / raw)
  To: Felipe Balbi; +Cc: linux-wireless, netdev, linux-kernel

On Sat, 2011-05-14 at 00:26 +0300, Felipe Balbi wrote:
> Those have little value. Remove those to make
> the driver less noisy.
> 
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> ---

Applied also this one.  Thanks!

-- 
Cheers,
Luca.


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC/PATCH 04/13] net: wl12xx: care for optional operations
  2011-05-13 21:26 ` [RFC/PATCH 04/13] net: wl12xx: care for optional operations Felipe Balbi
@ 2011-05-20 12:03   ` Luciano Coelho
  0 siblings, 0 replies; 32+ messages in thread
From: Luciano Coelho @ 2011-05-20 12:03 UTC (permalink / raw)
  To: Felipe Balbi; +Cc: linux-wireless, netdev, linux-kernel

On Sat, 2011-05-14 at 00:26 +0300, Felipe Balbi wrote:
> ->init and ->reset are optional - at least
> sdio.c doesn't implement them - so allow those
> pointers to be NULL.
> 
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> ---

Applied.

-- 
Cheers,
Luca.


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC/PATCH 05/13] net: wl12xx: remove the nops
  2011-05-13 21:26 ` [RFC/PATCH 05/13] net: wl12xx: remove the nops Felipe Balbi
@ 2011-05-20 12:04   ` Luciano Coelho
  0 siblings, 0 replies; 32+ messages in thread
From: Luciano Coelho @ 2011-05-20 12:04 UTC (permalink / raw)
  To: Felipe Balbi; +Cc: linux-wireless, netdev, linux-kernel

On Sat, 2011-05-14 at 00:26 +0300, Felipe Balbi wrote:
> Nops aren't needed. When we actually need
> those calls, then we add them with meat
> and barbecue sauce.
> 
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> ---

Applied the parts that are still relevant (some of the calls already
have saucy beef in them ;).

-- 
Cheers,
Luca.


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC/PATCH 06/13] net: wl12xx: remove unnecessary prints
  2011-05-13 21:26 ` [RFC/PATCH 06/13] net: wl12xx: remove unnecessary prints Felipe Balbi
@ 2011-05-20 12:04   ` Luciano Coelho
  0 siblings, 0 replies; 32+ messages in thread
From: Luciano Coelho @ 2011-05-20 12:04 UTC (permalink / raw)
  To: Felipe Balbi; +Cc: linux-wireless, netdev, linux-kernel

On Sat, 2011-05-14 at 00:26 +0300, Felipe Balbi wrote:
> Those have little value. Remove those to
> make the driver less noisy.
> 
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> ---

Applied, thanks dude!

-- 
Cheers,
Luca.


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC/PATCH 01/13] net: wl12xx: sdio: id_tables should be __devinitconst
  2011-05-13 21:26 ` [RFC/PATCH 01/13] net: wl12xx: sdio: id_tables should be __devinitconst Felipe Balbi
  2011-05-20 12:02   ` Luciano Coelho
@ 2011-05-20 16:02   ` Ohad Ben-Cohen
  2011-05-20 16:14     ` Michał Mirosław
  1 sibling, 1 reply; 32+ messages in thread
From: Ohad Ben-Cohen @ 2011-05-20 16:02 UTC (permalink / raw)
  To: Felipe Balbi; +Cc: Luciano Coelho, linux-wireless, netdev, linux-kernel

On Sat, May 14, 2011 at 12:26 AM, Felipe Balbi <balbi@ti.com> wrote:
> That's only needed during init anyway, let's free
> some space after we're done probing.

sdio devices are dynamically created whenever the hw is plugged into
the mmc slot by the user. that can happen anytime while the system is
up, not only during init.

> -static const struct sdio_device_id wl1271_devices[] = {
> +static const struct sdio_device_id wl1271_devices[] __devinitconst = {

it looks to me that sdio_match_device is going to be surprised if
->id_table won't be valid.

i wouldn't do this, unless you have a good explanation otherwise.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC/PATCH 01/13] net: wl12xx: sdio: id_tables should be __devinitconst
  2011-05-20 16:02   ` Ohad Ben-Cohen
@ 2011-05-20 16:14     ` Michał Mirosław
  0 siblings, 0 replies; 32+ messages in thread
From: Michał Mirosław @ 2011-05-20 16:14 UTC (permalink / raw)
  To: Ohad Ben-Cohen
  Cc: Felipe Balbi, Luciano Coelho, linux-wireless, netdev, linux-kernel

2011/5/20 Ohad Ben-Cohen <ohad@wizery.com>:
> On Sat, May 14, 2011 at 12:26 AM, Felipe Balbi <balbi@ti.com> wrote:
>> That's only needed during init anyway, let's free
>> some space after we're done probing.
> sdio devices are dynamically created whenever the hw is plugged into
> the mmc slot by the user. that can happen anytime while the system is
> up, not only during init.
>> -static const struct sdio_device_id wl1271_devices[] = {
>> +static const struct sdio_device_id wl1271_devices[] __devinitconst = {
> it looks to me that sdio_match_device is going to be surprised if
> ->id_table won't be valid.
>
> i wouldn't do this, unless you have a good explanation otherwise.

devinit sections are not freed if device hotplug is enabled.

Best Regards,
Michał Mirosław

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC/PATCH 03/13] net: wl12xx: remove some unnecessary prints
  2011-05-13 21:26   ` Felipe Balbi
  (?)
  (?)
@ 2011-05-22 12:34   ` Eliad Peller
  2011-05-22 12:37     ` Eliad Peller
  -1 siblings, 1 reply; 32+ messages in thread
From: Eliad Peller @ 2011-05-22 12:34 UTC (permalink / raw)
  To: Felipe Balbi; +Cc: Luciano Coelho, linux-wireless

On Sat, May 14, 2011 at 12:26 AM, Felipe Balbi <balbi@ti.com> wrote:
> Those have little value. Remove those to make
> the driver less noisy.
>
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> ---
[...]
> @@ -287,8 +287,6 @@ static int __devinit wl1271_probe(struct sdio_func *func,
>        /* Tell PM core that we don't need the card to be powered now */
>        pm_runtime_put_noidle(&func->dev);
>
> -       wl1271_notice("initialized");
> -
>        return 0;
>

>  static void __exit wl1271_exit(void)
>  {
>        sdio_unregister_driver(&wl1271_sdio_driver);
> -
> -       wl1271_notice("unloaded");
>  }
>

in fact, i find these prints pretty useful.
does changing wl1271_notice to wl1271_debug(DEBUG_MAC80211) will solve
the "nosiness"?
(i use DEBUG_MAC80211 rather than DEBUG_SDIO, as DEBUG_SDIO is really
*very* noisy)

(i'll send it as a new patch as the original patch was already applied)

Eliad.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC/PATCH 03/13] net: wl12xx: remove some unnecessary prints
  2011-05-22 12:34   ` Eliad Peller
@ 2011-05-22 12:37     ` Eliad Peller
  2011-05-22 17:30       ` Luciano Coelho
  0 siblings, 1 reply; 32+ messages in thread
From: Eliad Peller @ 2011-05-22 12:37 UTC (permalink / raw)
  To: Felipe Balbi; +Cc: Luciano Coelho, linux-wireless

On Sun, May 22, 2011 at 3:34 PM, Eliad Peller <eliad@wizery.com> wrote:
> On Sat, May 14, 2011 at 12:26 AM, Felipe Balbi <balbi@ti.com> wrote:
>> Those have little value. Remove those to make
>> the driver less noisy.
>>
>> Signed-off-by: Felipe Balbi <balbi@ti.com>
>> ---
> [...]
>> @@ -287,8 +287,6 @@ static int __devinit wl1271_probe(struct sdio_func *func,
>>        /* Tell PM core that we don't need the card to be powered now */
>>        pm_runtime_put_noidle(&func->dev);
>>
>> -       wl1271_notice("initialized");
>> -
>>        return 0;
>>
>
>>  static void __exit wl1271_exit(void)
>>  {
>>        sdio_unregister_driver(&wl1271_sdio_driver);
>> -
>> -       wl1271_notice("unloaded");
>>  }
>>
>
> in fact, i find these prints pretty useful.
> does changing wl1271_notice to wl1271_debug(DEBUG_MAC80211) will solve
> the "nosiness"?
> (i use DEBUG_MAC80211 rather than DEBUG_SDIO, as DEBUG_SDIO is really
> *very* noisy)
>
> (i'll send it as a new patch as the original patch was already applied)
>
> Eliad.
>

err... s/nosiness/noisiness/

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC/PATCH 03/13] net: wl12xx: remove some unnecessary prints
  2011-05-22 12:37     ` Eliad Peller
@ 2011-05-22 17:30       ` Luciano Coelho
  2011-05-22 21:57         ` Eliad Peller
  0 siblings, 1 reply; 32+ messages in thread
From: Luciano Coelho @ 2011-05-22 17:30 UTC (permalink / raw)
  To: Eliad Peller; +Cc: Felipe Balbi, linux-wireless

On Sun, 2011-05-22 at 15:37 +0300, Eliad Peller wrote:
> On Sun, May 22, 2011 at 3:34 PM, Eliad Peller <eliad@wizery.com> wrote:
> > On Sat, May 14, 2011 at 12:26 AM, Felipe Balbi <balbi@ti.com> wrote:
> >> Those have little value. Remove those to make
> >> the driver less noisy.
> >>
> >> Signed-off-by: Felipe Balbi <balbi@ti.com>
> >> ---
> > [...]
> >> @@ -287,8 +287,6 @@ static int __devinit wl1271_probe(struct sdio_func *func,
> >>        /* Tell PM core that we don't need the card to be powered now */
> >>        pm_runtime_put_noidle(&func->dev);
> >>
> >> -       wl1271_notice("initialized");
> >> -
> >>        return 0;
> >>
> >
> >>  static void __exit wl1271_exit(void)
> >>  {
> >>        sdio_unregister_driver(&wl1271_sdio_driver);
> >> -
> >> -       wl1271_notice("unloaded");
> >>  }
> >>
> >
> > in fact, i find these prints pretty useful.
> > does changing wl1271_notice to wl1271_debug(DEBUG_MAC80211) will solve
> > the "nosiness"?
> > (i use DEBUG_MAC80211 rather than DEBUG_SDIO, as DEBUG_SDIO is really
> > *very* noisy)
> >
> > (i'll send it as a new patch as the original patch was already applied)
> >
> > Eliad.
> >
> 
> err... s/nosiness/noisiness/

I think these are pretty useless.  You can see whether the driver is
loaded or not by lsmod'ing.  You can also use ftrace to get the same
stuff, if you want to know whether the driver is loaded or not offline.
Or what is the scenario where you think this is useful?

I'm reworking the whole way our traces are handled, so I don't think
reintroducing them is a good thing.


-- 
Cheers,
Luca.


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC/PATCH 03/13] net: wl12xx: remove some unnecessary prints
  2011-05-22 17:30       ` Luciano Coelho
@ 2011-05-22 21:57         ` Eliad Peller
  2011-05-23  5:32           ` Luciano Coelho
  0 siblings, 1 reply; 32+ messages in thread
From: Eliad Peller @ 2011-05-22 21:57 UTC (permalink / raw)
  To: Luciano Coelho; +Cc: Felipe Balbi, linux-wireless

On Sun, May 22, 2011 at 8:30 PM, Luciano Coelho <coelho@ti.com> wrote:
> On Sun, 2011-05-22 at 15:37 +0300, Eliad Peller wrote:
>> On Sun, May 22, 2011 at 3:34 PM, Eliad Peller <eliad@wizery.com> wrote:
>> > On Sat, May 14, 2011 at 12:26 AM, Felipe Balbi <balbi@ti.com> wrote:
>> >> Those have little value. Remove those to make
>> >> the driver less noisy.
>> >>
>> >> Signed-off-by: Felipe Balbi <balbi@ti.com>
>> >> ---
>> > [...]
>> >> @@ -287,8 +287,6 @@ static int __devinit wl1271_probe(struct sdio_func *func,
>> >>        /* Tell PM core that we don't need the card to be powered now */
>> >>        pm_runtime_put_noidle(&func->dev);
>> >>
>> >> -       wl1271_notice("initialized");
>> >> -
>> >>        return 0;
>> >>
>> >
>> >>  static void __exit wl1271_exit(void)
>> >>  {
>> >>        sdio_unregister_driver(&wl1271_sdio_driver);
>> >> -
>> >> -       wl1271_notice("unloaded");
>> >>  }
>> >>
>> >
>> > in fact, i find these prints pretty useful.
>> > does changing wl1271_notice to wl1271_debug(DEBUG_MAC80211) will solve
>> > the "nosiness"?
>> > (i use DEBUG_MAC80211 rather than DEBUG_SDIO, as DEBUG_SDIO is really
>> > *very* noisy)
>> >
>> > (i'll send it as a new patch as the original patch was already applied)
>> >
>> > Eliad.
>> >
>>
>> err... s/nosiness/noisiness/
>
> I think these are pretty useless.  You can see whether the driver is
> loaded or not by lsmod'ing.  You can also use ftrace to get the same
> stuff, if you want to know whether the driver is loaded or not offline.
> Or what is the scenario where you think this is useful?
>
i was thinking about a simple offline log analysis, where it's pretty
useful to know when the wl12xx_sdio was insmod'ed/rmmod'ed.
i haven't tried ftrace yet. i'll give it a look.
anyway, the whole wl12xx driver is full of similar logs that get
called multiple times, so i don't see the real advantage of removing 2
prints that get called only once.

> I'm reworking the whole way our traces are handled, so I don't think
> reintroducing them is a good thing.

ok. so i'll just wait for your rework :)

thanks,
Eliad.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC/PATCH 03/13] net: wl12xx: remove some unnecessary prints
  2011-05-22 21:57         ` Eliad Peller
@ 2011-05-23  5:32           ` Luciano Coelho
  2011-05-23  6:07             ` Felipe Balbi
  0 siblings, 1 reply; 32+ messages in thread
From: Luciano Coelho @ 2011-05-23  5:32 UTC (permalink / raw)
  To: Eliad Peller; +Cc: Felipe Balbi, linux-wireless

On Mon, 2011-05-23 at 00:57 +0300, Eliad Peller wrote:
> On Sun, May 22, 2011 at 8:30 PM, Luciano Coelho <coelho@ti.com> wrote:
> > On Sun, 2011-05-22 at 15:37 +0300, Eliad Peller wrote:
> >> On Sun, May 22, 2011 at 3:34 PM, Eliad Peller <eliad@wizery.com> wrote:
> >> > On Sat, May 14, 2011 at 12:26 AM, Felipe Balbi <balbi@ti.com> wrote:
> >> >> Those have little value. Remove those to make
> >> >> the driver less noisy.
> >> >>
> >> >> Signed-off-by: Felipe Balbi <balbi@ti.com>
> >> >> ---
> >> > [...]
> >> >> @@ -287,8 +287,6 @@ static int __devinit wl1271_probe(struct sdio_func *func,
> >> >>        /* Tell PM core that we don't need the card to be powered now */
> >> >>        pm_runtime_put_noidle(&func->dev);
> >> >>
> >> >> -       wl1271_notice("initialized");
> >> >> -
> >> >>        return 0;
> >> >>
> >> >
> >> >>  static void __exit wl1271_exit(void)
> >> >>  {
> >> >>        sdio_unregister_driver(&wl1271_sdio_driver);
> >> >> -
> >> >> -       wl1271_notice("unloaded");
> >> >>  }
> >> >>
> >> >
> >> > in fact, i find these prints pretty useful.
> >> > does changing wl1271_notice to wl1271_debug(DEBUG_MAC80211) will solve
> >> > the "nosiness"?
> >> > (i use DEBUG_MAC80211 rather than DEBUG_SDIO, as DEBUG_SDIO is really
> >> > *very* noisy)
> >> >
> >> > (i'll send it as a new patch as the original patch was already applied)
> >> >
> >> > Eliad.
> >> >
> >>
> >> err... s/nosiness/noisiness/
> >
> > I think these are pretty useless.  You can see whether the driver is
> > loaded or not by lsmod'ing.  You can also use ftrace to get the same
> > stuff, if you want to know whether the driver is loaded or not offline.
> > Or what is the scenario where you think this is useful?
> >
> i was thinking about a simple offline log analysis, where it's pretty
> useful to know when the wl12xx_sdio was insmod'ed/rmmod'ed.

Have you really had to know when the modules are insmodded or rmmoded?


> i haven't tried ftrace yet. i'll give it a look.
> anyway, the whole wl12xx driver is full of similar logs that get
> called multiple times, so i don't see the real advantage of removing 2
> prints that get called only once.

Yeah, the driver is full of useless logs.  That's why I'm going to
rework it.  My idea is to use more standard tracing (nobody needs to
learn wl12xx debug bitmask and how to set it), using ftrace and friends.

These two were removed now because Felipe has reworked the SDIO/SPI
modules and, at the same time, cleaned it up a little.  I think it's
good to clean up things little by little, especially on the parts that
are being touched anyway.


> > I'm reworking the whole way our traces are handled, so I don't think
> > reintroducing them is a good thing.
> 
> ok. so i'll just wait for your rework :)

It may still take a bit of time, because it's not my highest priority
right now and the changes are quite intrusive (in the sense that they
will touch every file), but when it's ready, I think it's going to be
cool, you'll be able to use trace-cmd, kernelshark etc.

-- 
Cheers,
Luca.


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC/PATCH 03/13] net: wl12xx: remove some unnecessary prints
  2011-05-23  5:32           ` Luciano Coelho
@ 2011-05-23  6:07             ` Felipe Balbi
  0 siblings, 0 replies; 32+ messages in thread
From: Felipe Balbi @ 2011-05-23  6:07 UTC (permalink / raw)
  To: Luciano Coelho; +Cc: Eliad Peller, Felipe Balbi, linux-wireless

[-- Attachment #1: Type: text/plain, Size: 2700 bytes --]

Hi,

On Mon, May 23, 2011 at 08:32:53AM +0300, Luciano Coelho wrote:
> > >> > in fact, i find these prints pretty useful.
> > >> > does changing wl1271_notice to wl1271_debug(DEBUG_MAC80211) will solve
> > >> > the "nosiness"?
> > >> > (i use DEBUG_MAC80211 rather than DEBUG_SDIO, as DEBUG_SDIO is really
> > >> > *very* noisy)
> > >> >
> > >> > (i'll send it as a new patch as the original patch was already applied)
> > >> >
> > >> > Eliad.
> > >> >
> > >>
> > >> err... s/nosiness/noisiness/
> > >
> > > I think these are pretty useless.  You can see whether the driver is
> > > loaded or not by lsmod'ing.  You can also use ftrace to get the same
> > > stuff, if you want to know whether the driver is loaded or not offline.
> > > Or what is the scenario where you think this is useful?
> > >
> > i was thinking about a simple offline log analysis, where it's pretty
> > useful to know when the wl12xx_sdio was insmod'ed/rmmod'ed.

It's quite weird statement IMHO. Drivers will load either if you
modprobe by hand of a device is created and matches a driver, in which
case udev will load it for you. In both cases, either you know driver is
loaded because you manually loaded it or, if you're connected to web, or
ifconfig -a shows something, or you can ping around etc...

Besides, if the driver doesn't load because e.g. it failed to allocate
memory, you should get a failure report on dmesg.

The best way to handle this is to be as silent as possible on the
working case and only bother people on failures ;-)

> > i haven't tried ftrace yet. i'll give it a look.
> > anyway, the whole wl12xx driver is full of similar logs that get
> > called multiple times, so i don't see the real advantage of removing 2
> > prints that get called only once.
> 
> Yeah, the driver is full of useless logs.  That's why I'm going to
> rework it.  My idea is to use more standard tracing (nobody needs to
> learn wl12xx debug bitmask and how to set it), using ftrace and friends.

you can also play around with dynamic printk ;-) You use standard
dev_dbg() macros but can enable disable those by line, file, module, or
some simple regexes.

> > > I'm reworking the whole way our traces are handled, so I don't think
> > > reintroducing them is a good thing.
> > 
> > ok. so i'll just wait for your rework :)
> 
> It may still take a bit of time, because it's not my highest priority
> right now and the changes are quite intrusive (in the sense that they
> will touch every file), but when it's ready, I think it's going to be
> cool, you'll be able to use trace-cmd, kernelshark etc.

that's neat. I should do the same for musb ;-)

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2011-05-23  6:07 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-13 21:26 [RFC/PATCH 00/13] wl12xx re-factor Felipe Balbi
2011-05-13 21:26 ` [RFC/PATCH 01/13] net: wl12xx: sdio: id_tables should be __devinitconst Felipe Balbi
2011-05-20 12:02   ` Luciano Coelho
2011-05-20 16:02   ` Ohad Ben-Cohen
2011-05-20 16:14     ` Michał Mirosław
2011-05-13 21:26 ` [RFC/PATCH 02/13] net: wl12xx: sdio: add a context structure Felipe Balbi
2011-05-13 21:26 ` [RFC/PATCH 03/13] net: wl12xx: remove some unnecessary prints Felipe Balbi
2011-05-13 21:26   ` Felipe Balbi
2011-05-20 12:03   ` Luciano Coelho
2011-05-22 12:34   ` Eliad Peller
2011-05-22 12:37     ` Eliad Peller
2011-05-22 17:30       ` Luciano Coelho
2011-05-22 21:57         ` Eliad Peller
2011-05-23  5:32           ` Luciano Coelho
2011-05-23  6:07             ` Felipe Balbi
2011-05-13 21:26 ` [RFC/PATCH 04/13] net: wl12xx: care for optional operations Felipe Balbi
2011-05-20 12:03   ` Luciano Coelho
2011-05-13 21:26 ` [RFC/PATCH 05/13] net: wl12xx: remove the nops Felipe Balbi
2011-05-20 12:04   ` Luciano Coelho
2011-05-13 21:26 ` [RFC/PATCH 06/13] net: wl12xx: remove unnecessary prints Felipe Balbi
2011-05-20 12:04   ` Luciano Coelho
2011-05-13 21:26 ` [RFC/PATCH 07/13] net: wl12xx: spi: add a context structure Felipe Balbi
2011-05-13 21:26 ` [RFC/PATCH 08/13] net: wl12xx: spi: add a platform_device Felipe Balbi
2011-05-13 21:26 ` [RFC/PATCH 09/13] net: wl12xx: sdio: " Felipe Balbi
2011-05-13 21:26 ` [RFC/PATCH 10/13] net: wl12xx: main: add platform device Felipe Balbi
2011-05-13 21:26   ` Felipe Balbi
2011-05-13 21:26 ` [RFC/PATCH 11/13] net: wireless: wl12xx: re-factor all drivers Felipe Balbi
2011-05-13 21:26 ` [RFC/PATCH 12/13] net: wireless: wl12xx: mark some symbols static Felipe Balbi
2011-05-13 21:26   ` Felipe Balbi
2011-05-13 21:26 ` [RFC/PATCH 13/13] net: wl12xx: main: drop unneded plat_dev Felipe Balbi
2011-05-19 12:49 ` [RFC/PATCH 00/13] wl12xx re-factor Luciano Coelho
2011-05-19 14:06   ` Felipe Balbi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.