2010-04-12 22:22:56

by Grazvydas Ignotas

[permalink] [raw]
Subject: [PATCH] wl1251: register platform_device to pass board data

wl1251 is embedded chip that can be connected using SDIO bus, and is not
an actual SDIO card. For this reason there is a need to pass some board
specific data, like 'EEPROM is attached' flag or power control callback.
However there is no way to pass this data through SDIO subsystem, so
this patch registers dummy platform_device to allow that.

Signed-off-by: Grazvydas Ignotas <[email protected]>
---
drivers/net/wireless/wl12xx/wl1251_sdio.c | 37 +++++++++++++++++++++++++++++
1 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/wl1251_sdio.c b/drivers/net/wireless/wl12xx/wl1251_sdio.c
index 2051ef0..b05ecc0 100644
--- a/drivers/net/wireless/wl12xx/wl1251_sdio.c
+++ b/drivers/net/wireless/wl12xx/wl1251_sdio.c
@@ -23,6 +23,8 @@
#include <linux/mod_devicetable.h>
#include <linux/mmc/sdio_func.h>
#include <linux/mmc/sdio_ids.h>
+#include <linux/platform_device.h>
+#include <linux/spi/wl12xx.h>

#include "wl1251.h"

@@ -34,6 +36,8 @@
#define SDIO_DEVICE_ID_TI_WL1251 0x9066
#endif

+struct wl12xx_platform_data *wl12xx_board_data;
+
static struct sdio_func *wl_to_func(struct wl1251 *wl)
{
return wl->if_priv;
@@ -144,6 +148,29 @@ static const struct wl1251_if_operations wl1251_sdio_ops = {
.disable_irq = wl1251_sdio_disable_irq,
};

+static int wl1251_platform_probe(struct platform_device *pdev)
+{
+ if (pdev->id != -1) {
+ wl1251_error("can only handle single device");
+ return -ENODEV;
+ }
+
+ wl12xx_board_data = pdev->dev.platform_data;
+ return 0;
+}
+
+/*
+ * Dummy platform_driver for passing platform_data to this driver,
+ * used because SDIO subsystem doesn't provide a way to do this.
+ */
+static struct platform_driver wl1251_platform_driver = {
+ .driver = {
+ .name = "wl1251_data",
+ .owner = THIS_MODULE,
+ },
+ .probe = wl1251_platform_probe,
+};
+
static int wl1251_sdio_probe(struct sdio_func *func,
const struct sdio_device_id *id)
{
@@ -169,6 +196,11 @@ static int wl1251_sdio_probe(struct sdio_func *func,
wl->if_ops = &wl1251_sdio_ops;
wl->set_power = wl1251_sdio_set_power;

+ if (wl12xx_board_data != NULL) {
+ wl->set_power = wl12xx_board_data->set_power;
+ wl->use_eeprom = wl12xx_board_data->use_eeprom;
+ }
+
sdio_release_host(func);
ret = wl1251_init_ieee80211(wl);
if (ret)
@@ -208,6 +240,10 @@ static int __init wl1251_sdio_init(void)
{
int err;

+ err = platform_driver_register(&wl1251_platform_driver);
+ if (err)
+ wl1251_error("failed to register platform driver: %d", err);
+
err = sdio_register_driver(&wl1251_sdio_driver);
if (err)
wl1251_error("failed to register sdio driver: %d", err);
@@ -217,6 +253,7 @@ static int __init wl1251_sdio_init(void)
static void __exit wl1251_sdio_exit(void)
{
sdio_unregister_driver(&wl1251_sdio_driver);
+ platform_driver_unregister(&wl1251_platform_driver);
wl1251_notice("unloaded");
}

--
1.6.3.3



2010-04-13 16:55:22

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] wl1251: register platform_device to pass board data

Grazvydas Ignotas <[email protected]> writes:

> wl1251 is embedded chip that can be connected using SDIO bus, and is not
> an actual SDIO card. For this reason there is a need to pass some board
> specific data, like 'EEPROM is attached' flag or power control callback.
> However there is no way to pass this data through SDIO subsystem, so
> this patch registers dummy platform_device to allow that.

Thanks a lot, this is really needed. Few comments though:

> +struct wl12xx_platform_data *wl12xx_board_data;

Shouldn't this be static? Also I assume that there is no way to
allocate this dynamically, am I correct?

> @@ -208,6 +240,10 @@ static int __init wl1251_sdio_init(void)
> {
> int err;
>
> + err = platform_driver_register(&wl1251_platform_driver);
> + if (err)
> + wl1251_error("failed to register platform driver: %d", err);

You should exit the function here.

Can you fix the above and resend, please?

--
Kalle Valo

2010-04-13 17:56:09

by Bob Copeland

[permalink] [raw]
Subject: Re: [PATCH] wl1251: register platform_device to pass board data

On Tue, Apr 13, 2010 at 12:55 PM, Kalle Valo <[email protected]> wrote:
>> However there is no way to pass this data through SDIO subsystem, so
>> this patch registers dummy platform_device to allow that.

No objection to the patch, platforms like android want this so
you can set the gpio for irq handling instead of using the
SDIO interrupts.

But maybe we should invent a way to send it through the SDIO
subsystem?

My hope was that device trees on ARM would solve the issue
eventually.

--
Bob Copeland %% http://www.bobcopeland.com

2010-04-13 22:04:00

by Grazvydas Ignotas

[permalink] [raw]
Subject: Re: [PATCH] wl1251: register platform_device to pass board data

On Tue, Apr 13, 2010 at 7:55 PM, Kalle Valo <[email protected]> wrote:
>> +struct wl12xx_platform_data *wl12xx_board_data;
>
> Shouldn't this be static? Also I assume that there is no way to
> allocate this dynamically, am I correct?

Yes, it should be static. Could not think if a way to avoid global
pointer though..

On Tue, Apr 13, 2010 at 9:36 PM, Kalle Valo <[email protected]> wrote:
> Bob Copeland <[email protected]> writes:
>
>> On Tue, Apr 13, 2010 at 12:55 PM, Kalle Valo <[email protected]> wrote:
>>>> However there is no way to pass this data through SDIO subsystem, so
>>>> this patch registers dummy platform_device to allow that.
>>
>> No objection to the patch, platforms like android want this so
>> you can set the gpio for irq handling instead of using the
>> SDIO interrupts.
>
> I talked with TI about the interrupts. They say SDIO interrupt should
> not be used, instead we should always the external gpio line for the
> interrupt.

I've got some patches for this in the pipeline, just need a way to
pass platform_data..

>> But maybe we should invent a way to send it through the SDIO
>> subsystem?
>
> I agree, that is the best option.

Well there are even more problems with this particular chip as it's
missing essential SDIO registers like CIS and CCCR and can't even
probe without some additional kernel hacks..

>> My hope was that device trees on ARM would solve the issue
>> eventually.
>
> But no luck yet?

There is lot of talk but it's not there yet, and I'm not sure it will
suit this case even. I think we can use my hack for the time being.

2010-04-13 18:36:34

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] wl1251: register platform_device to pass board data

Bob Copeland <[email protected]> writes:

> On Tue, Apr 13, 2010 at 12:55 PM, Kalle Valo <[email protected]> wrote:
>>> However there is no way to pass this data through SDIO subsystem, so
>>> this patch registers dummy platform_device to allow that.
>
> No objection to the patch, platforms like android want this so
> you can set the gpio for irq handling instead of using the
> SDIO interrupts.

I talked with TI about the interrupts. They say SDIO interrupt should
not be used, instead we should always the external gpio line for the
interrupt.

> But maybe we should invent a way to send it through the SDIO
> subsystem?

I agree, that is the best option.

> My hope was that device trees on ARM would solve the issue
> eventually.

But no luck yet?

--
Kalle Valo