diff mbox

[11/15] wireless: wl1271: introduce platform device support

Message ID 1278376666-3509-12-git-send-email-ohad@wizery.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Ohad Ben Cohen July 6, 2010, 12:37 a.m. UTC
None

Comments

Ohad Ben Cohen July 8, 2010, 8:10 p.m. UTC | #1
Hi Nicolas and Roger,

On Tue, Jul 6, 2010 at 8:42 PM, Nicolas Pitre <nico@fluxnic.net> wrote:
> On Tue, 6 Jul 2010, Roger Quadros wrote:
> > If the Power enable GPIO can be treated as SDIO slot supply (i.e. vmmc), then
> > the SDIO/MMC core should tackle it, just like it deals with supply for slots
> > with removable cards.
>
...
> Another function pair would be needed instead, which would do almost
> like the suspend/resume code is already doing.  Something like:

Thanks a lot for your review and comments, and for taking the time to
present your approach.

I like it !

It'd allow us to lose the software (or fake if you want ;) card detect
mechanism, which is something that should have been added to each
platform we wanted to support.

We would only need to make it possible to deliver board-specific data
to the function driver (e.g., in the case of the wl1271, we need irq
and board_ref_clock data).  That would require some board-level
platform-data configuration, which will be specific to the controller
to which the device is hardwired to. This data should propagate
through the host controller to the SDIO core so it would eventually be
accessible by the function driver (e.g. via func->dev.pdata).

We'll adapt and post follow-up patches.

Thanks again,
Ohad.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Roger Quadros July 9, 2010, 8:12 a.m. UTC | #2
Hi Ohad,

On 07/08/2010 11:10 PM, ext Ohad Ben-Cohen wrote:
> Hi Nicolas and Roger,
>
> On Tue, Jul 6, 2010 at 8:42 PM, Nicolas Pitre<nico@fluxnic.net>  wrote:
>> On Tue, 6 Jul 2010, Roger Quadros wrote:
>>> If the Power enable GPIO can be treated as SDIO slot supply (i.e. vmmc), then
>>> the SDIO/MMC core should tackle it, just like it deals with supply for slots
>>> with removable cards.
>>
> ...
>> Another function pair would be needed instead, which would do almost
>> like the suspend/resume code is already doing.  Something like:
>
> Thanks a lot for your review and comments, and for taking the time to
> present your approach.

you're welcome :)
>
> I like it !
>
> It'd allow us to lose the software (or fake if you want ;) card detect
> mechanism, which is something that should have been added to each
> platform we wanted to support.
>
> We would only need to make it possible to deliver board-specific data
> to the function driver (e.g., in the case of the wl1271, we need irq
> and board_ref_clock data).  That would require some board-level
> platform-data configuration, which will be specific to the controller
> to which the device is hardwired to. This data should propagate
> through the host controller to the SDIO core so it would eventually be

why should platform data go through the host controller? You are already 
creating a new platform device for wl1271_sdio just to pass platform_data so the 
platform data for that can be passed directly from board files to this platform 
device. It doesn't need to go via host controller.

I think this case is specific to wl1271. Ideally wl1271 should have used the 
SDIO's in-band interrupt delivery mechanism and SDIO Configuration space to 
provide this information to make it truly plug n play.

br,
-roger
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ohad Ben Cohen July 9, 2010, 8:32 a.m. UTC | #3
Hi Roger,

On Fri, Jul 9, 2010 at 11:12 AM, Roger Quadros <roger.quadros@nokia.com> wrote:
> You are already
> creating a new platform device for wl1271_sdio just to pass platform_data

With the new approach, I can remove this platform device altogether,
and completely rely on the sdio function device for driver probing (as
it was before). I just need to add this little support of delivering
small bits of platform data - the mechanism should be simple and
generic, and would considerably simplify the driver.

> I think this case is specific to wl1271.

Yes, it is.

There's an hw issue with the wl1271's in-band interrupt, so we can't
use it (the issue is fixed in the wl1281 btw).

But we also have to deliver the reference clock info to the driver,
which is board-specific data, and would be relevant to all wl12xx
devices.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Grazvydas Ignotas July 9, 2010, 9:24 a.m. UTC | #4
On Fri, Jul 9, 2010 at 11:32 AM, Ohad Ben-Cohen <ohad@wizery.com> wrote:
> Hi Roger,
>
> On Fri, Jul 9, 2010 at 11:12 AM, Roger Quadros <roger.quadros@nokia.com> wrote:
>> You are already
>> creating a new platform device for wl1271_sdio just to pass platform_data
>
> With the new approach, I can remove this platform device altogether,
> and completely rely on the sdio function device for driver probing (as
> it was before). I just need to add this little support of delivering
> small bits of platform data - the mechanism should be simple and
> generic, and would considerably simplify the driver.
>
>> I think this case is specific to wl1271.
>
> Yes, it is.

wl1251 also has this issue. In fact it's even worse there as it's
missing all SDIO registers and cannot be probed at all.. Those devices
are not plug n play at all.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/wireless/wl12xx/Kconfig b/drivers/net/wireless/wl12xx/Kconfig
index 337fc7b..8fb7b5a 100644
--- a/drivers/net/wireless/wl12xx/Kconfig
+++ b/drivers/net/wireless/wl12xx/Kconfig
@@ -66,6 +66,7 @@  config WL1271_SPI
 config WL1271_SDIO
 	tristate "TI wl1271 SDIO support"
 	depends on WL1271 && MMC && ARM
+	select MMC_EMBEDDED_SDIO
 	---help---
 	  This module adds support for the SDIO interface of adapters using
 	  TI wl1271 chipset.  Select this if your platform is using
diff --git a/drivers/net/wireless/wl12xx/wl1271.h b/drivers/net/wireless/wl12xx/wl1271.h
index 5250361..2df57cc 100644
--- a/drivers/net/wireless/wl12xx/wl1271.h
+++ b/drivers/net/wireless/wl12xx/wl1271.h
@@ -352,6 +352,7 @@  struct wl1271 {
 	bool mac80211_registered;
 
 	void *if_priv;
+	void *if_plat_priv;
 
 	struct wl1271_if_operations *if_ops;
 
diff --git a/drivers/net/wireless/wl12xx/wl1271_sdio.c b/drivers/net/wireless/wl12xx/wl1271_sdio.c
index 571c6b9..96b8fc3 100644
--- a/drivers/net/wireless/wl12xx/wl1271_sdio.c
+++ b/drivers/net/wireless/wl12xx/wl1271_sdio.c
@@ -28,6 +28,10 @@ 
 #include <linux/mmc/sdio_func.h>
 #include <linux/mmc/sdio_ids.h>
 #include <linux/mmc/card.h>
+#include <linux/mmc/host.h>
+#include <linux/completion.h>
+#include <linux/wl12xx.h>
+#include <linux/platform_device.h>
 #include <plat/gpio.h>
 
 #include "wl1271.h"
@@ -36,6 +40,7 @@ 
 
 
 #define RX71_WL1271_IRQ_GPIO		42
+static DECLARE_COMPLETION(wl1271_sdio_ready);
 
 static const struct sdio_device_id wl1271_devices[] = {
 	{ SDIO_DEVICE(SDIO_VENDOR_ID_TI, SDIO_DEVICE_ID_TI_WL1271) },
@@ -50,7 +55,9 @@  static inline struct sdio_func *wl_to_func(struct wl1271 *wl)
 
 static struct device *wl1271_sdio_wl_to_dev(struct wl1271 *wl)
 {
-	return &(wl_to_func(wl)->dev);
+	struct platform_device *pdev = wl->if_plat_priv;
+
+	return &pdev->dev;
 }
 
 static irqreturn_t wl1271_irq(int irq, void *cookie)
@@ -146,21 +153,44 @@  static void wl1271_sdio_raw_write(struct wl1271 *wl, int addr, void *buf,
 
 static int wl1271_sdio_set_power(struct wl1271 *wl, bool enable)
 {
-	struct sdio_func *func = wl_to_func(wl);
+	int ret = 0, timeleft;
 
-	/* Let the SDIO stack handle wlan_enable control, so we
-	 * keep host claimed while wlan is in use to keep wl1271
-	 * alive.
-	 */
 	if (enable) {
-		sdio_claim_host(func);
-		sdio_enable_func(func);
+		/* save our wl struct as private mmc data */
+		wl->set_embedded_data(wl);
+
+		INIT_COMPLETION(wl1271_sdio_ready);
+		/* 1271 Power Up Sequence */
+		wl->set_power(true);
+		mdelay(15);
+		wl->set_power(false);
+		mdelay(1);
+		wl->set_power(true);
+		mdelay(70);
+
+		/* emulate card detect event */
+		wl->set_carddetect(true);
+
 	} else {
-		sdio_disable_func(func);
-		sdio_release_host(func);
+		wl->set_embedded_data(NULL);
+
+		INIT_COMPLETION(wl1271_sdio_ready);
+
+		wl->set_power(false);
+		mdelay(10);
+
+		wl->set_carddetect(false);
 	}
 
-	return 0;
+	timeleft = wait_for_completion_interruptible_timeout(&wl1271_sdio_ready,
+						msecs_to_jiffies(5000));
+	if (timeleft <= 0) {
+		wl1271_error("%s: unsuccessful SDIO %s (%d)", __func__,
+				enable ? "init" : "deinit", timeleft);
+		ret = (timeleft == 0 ? -EAGAIN : timeleft);
+	}
+
+	return ret;
 }
 
 static struct wl1271_if_operations sdio_ops = {
@@ -174,30 +204,97 @@  static struct wl1271_if_operations sdio_ops = {
 	.disable_irq	= wl1271_sdio_disable_interrupts
 };
 
-static int __devinit wl1271_probe(struct sdio_func *func,
+static int wl1271_sdio_probe(struct sdio_func *func,
 				  const struct sdio_device_id *id)
 {
+	struct wl1271 *wl = mmc_get_embedded_data(func->card->host);
+
+	/* 2nd func is WLAN, make sure wl context is received */
+	if (func->num != 0x02 || !wl)
+		return -ENODEV;
+
+	sdio_set_drvdata(func, wl);
+
+	wl->if_priv = func;
+
+	/* Grab access to FN0 for ELP reg. */
+	func->card->quirks |= MMC_QUIRK_LENIENT_FN0;
+
+	sdio_claim_host(func);
+	sdio_enable_func(func);
+
+	complete(&wl1271_sdio_ready);
+
+	return 0;
+}
+
+static void wl1271_sdio_remove(struct sdio_func *func)
+{
+	struct wl1271 *wl = sdio_get_drvdata(func);
+
+	wl->if_priv = NULL;
+
+	sdio_disable_func(func);
+	sdio_release_host(func);
+
+	complete(&wl1271_sdio_ready);
+}
+
+static struct sdio_driver wl1271_sdio_driver = {
+	.name		= "wl1271_sdio_func",
+	.id_table	= wl1271_devices,
+	.probe		= wl1271_sdio_probe,
+	.remove		= wl1271_sdio_remove,
+};
+
+static int wl1271_plat_probe(struct platform_device *pdev)
+{
+	struct wl12xx_platform_data *pdata;
 	struct ieee80211_hw *hw;
 	struct wl1271 *wl;
 	int ret;
 
-	/* We are only able to handle the wlan function */
-	if (func->num != 0x02)
+	pdata = pdev->dev.platform_data;
+	if (!pdata) {
+		wl1271_error("no platform data");
 		return -ENODEV;
+	}
 
 	hw = wl1271_alloc_hw();
-	if (IS_ERR(hw))
+	if (IS_ERR(hw)) {
+		wl1271_error("wl1271_alloc_hw failed: %ld", PTR_ERR(hw));
 		return PTR_ERR(hw);
+	}
 
 	wl = hw->priv;
 
-	wl->if_priv = func;
 	wl->if_ops = &sdio_ops;
+	wl->if_plat_priv = pdev;
+	platform_set_drvdata(pdev, wl);
 
-	/* Grab access to FN0 for ELP reg. */
-	func->card->quirks |= MMC_QUIRK_LENIENT_FN0;
+	wl->set_embedded_data = pdata->set_embedded_data;
+	if (!wl->set_embedded_data) {
+		wl1271_error("set embedded_data func missing in platform data");
+		ret = -ENODEV;
+		goto out_free;
+	}
+
+	wl->set_carddetect = pdata->set_carddetect;
+	if (!wl->set_carddetect) {
+		wl1271_error("set carddetect func missing in platform data");
+		ret = -ENODEV;
+		goto out_free;
+	}
 
 	wl->irq = gpio_to_irq(RX71_WL1271_IRQ_GPIO);
+
+	wl->set_power = pdata->set_power;
+	if (!wl->set_power) {
+		wl1271_error("set power function missing in platform data");
+		ret = -ENODEV;
+		goto out_free;
+	}
+
 	if (wl->irq < 0) {
 		ret = wl->irq;
 		wl1271_error("could not get irq!");
@@ -215,53 +312,67 @@  static int __devinit wl1271_probe(struct sdio_func *func,
 	disable_irq(wl->irq);
 
 	ret = wl1271_init_ieee80211(wl);
-	if (ret)
+	if (ret) {
+		wl1271_error("wl1271_init_ieee80211 failed: %d", ret);
 		goto out_irq;
+	}
 
 	ret = wl1271_register_hw(wl);
-	if (ret)
+	if (ret) {
+		wl1271_error("wl1271_register_hw failed: %d", ret);
 		goto out_irq;
+	}
 
-	sdio_set_drvdata(func, wl);
+	ret = sdio_register_driver(&wl1271_sdio_driver);
+	if (ret < 0) {
+		wl1271_error("failed to register sdio driver: %d", ret);
+		goto out_hw;
+	}
 
 	wl1271_notice("initialized");
 
 	return 0;
 
- out_irq:
+out_hw:
+	wl1271_unregister_hw(wl);
+out_irq:
 	free_irq(wl->irq, wl);
-
-
- out_free:
+out_free:
 	wl1271_free_hw(wl);
-
 	return ret;
 }
 
-static void __devexit wl1271_remove(struct sdio_func *func)
+static int __devexit wl1271_plat_remove(struct platform_device *pdev)
 {
-	struct wl1271 *wl = sdio_get_drvdata(func);
+	struct wl1271 *wl = platform_get_drvdata(pdev);
 
-	free_irq(wl->irq, wl);
+	sdio_unregister_driver(&wl1271_sdio_driver);
 
 	wl1271_unregister_hw(wl);
+
+	free_irq(wl->irq, wl);
+
 	wl1271_free_hw(wl);
+
+	return 0;
 }
 
-static struct sdio_driver wl1271_sdio_driver = {
-	.name		= "wl1271_sdio",
-	.id_table	= wl1271_devices,
-	.probe		= wl1271_probe,
-	.remove		= __devexit_p(wl1271_remove),
+static struct platform_driver wl1271_plat_driver = {
+	.probe = wl1271_plat_probe,
+	.remove = __devexit_p(wl1271_plat_remove),
+	.driver = {
+		.name		= "wl1271_sdio",
+		.owner		= THIS_MODULE,
+	},
 };
 
 static int __init wl1271_init(void)
 {
 	int ret;
 
-	ret = sdio_register_driver(&wl1271_sdio_driver);
+	ret = platform_driver_register(&wl1271_plat_driver);
 	if (ret < 0) {
-		wl1271_error("failed to register sdio driver: %d", ret);
+		wl1271_error("failed to register plat driver: %d", ret);
 		goto out;
 	}
 
@@ -271,7 +382,7 @@  out:
 
 static void __exit wl1271_exit(void)
 {
-	sdio_unregister_driver(&wl1271_sdio_driver);
+	platform_driver_unregister(&wl1271_plat_driver);
 
 	wl1271_notice("unloaded");
 }