* [PATCH 1/3] wl1251: add power callback to wl1251_if_operations
2010-11-03 22:13 [PATCH 0/3] wl1251 SDIO patches Grazvydas Ignotas
@ 2010-11-03 22:13 ` Grazvydas Ignotas
2010-11-07 10:15 ` Kalle Valo
2010-11-03 22:13 ` [PATCH 2/3] wl1251: add runtime PM support for SDIO Grazvydas Ignotas
2010-11-03 22:13 ` [PATCH 3/3] wl1251: use wl12xx_platform_data to pass data Grazvydas Ignotas
2 siblings, 1 reply; 17+ messages in thread
From: Grazvydas Ignotas @ 2010-11-03 22:13 UTC (permalink / raw)
To: linux-wireless; +Cc: Kalle Valo, John W. Linville, Grazvydas Ignotas
Call interface specific power callback before calling board specific
one. Also allow that callback to fail. This is how it's done for
wl1271 and will be used for runtime_pm support.
Signed-off-by: Grazvydas Ignotas <notasas@gmail.com>
---
drivers/net/wireless/wl1251/main.c | 15 +++++++++------
drivers/net/wireless/wl1251/sdio.c | 8 ++++++--
drivers/net/wireless/wl1251/spi.c | 9 +++++++++
drivers/net/wireless/wl1251/wl1251.h | 1 +
4 files changed, 25 insertions(+), 8 deletions(-)
diff --git a/drivers/net/wireless/wl1251/main.c b/drivers/net/wireless/wl1251/main.c
index 7a87625..012e1a4 100644
--- a/drivers/net/wireless/wl1251/main.c
+++ b/drivers/net/wireless/wl1251/main.c
@@ -52,14 +52,14 @@ void wl1251_disable_interrupts(struct wl1251 *wl)
wl->if_ops->disable_irq(wl);
}
-static void wl1251_power_off(struct wl1251 *wl)
+static int wl1251_power_off(struct wl1251 *wl)
{
- wl->set_power(false);
+ return wl->if_ops->power(wl, false);
}
-static void wl1251_power_on(struct wl1251 *wl)
+static int wl1251_power_on(struct wl1251 *wl)
{
- wl->set_power(true);
+ return wl->if_ops->power(wl, true);
}
static int wl1251_fetch_firmware(struct wl1251 *wl)
@@ -152,9 +152,12 @@ static void wl1251_fw_wakeup(struct wl1251 *wl)
static int wl1251_chip_wakeup(struct wl1251 *wl)
{
- int ret = 0;
+ int ret;
+
+ ret = wl1251_power_on(wl);
+ if (ret < 0)
+ return ret;
- wl1251_power_on(wl);
msleep(WL1251_POWER_ON_SLEEP);
wl->if_ops->reset(wl);
diff --git a/drivers/net/wireless/wl1251/sdio.c b/drivers/net/wireless/wl1251/sdio.c
index 74ba9ce..0285190 100644
--- a/drivers/net/wireless/wl1251/sdio.c
+++ b/drivers/net/wireless/wl1251/sdio.c
@@ -171,8 +171,12 @@ static void wl1251_disable_line_irq(struct wl1251 *wl)
return disable_irq(wl->irq);
}
-static void wl1251_sdio_set_power(bool enable)
+static int wl1251_sdio_set_power(struct wl1251 *wl, bool enable)
{
+ if (wl->set_power)
+ wl->set_power(enable);
+
+ return 0;
}
static struct wl1251_if_operations wl1251_sdio_ops = {
@@ -181,6 +185,7 @@ static struct wl1251_if_operations wl1251_sdio_ops = {
.write_elp = wl1251_sdio_write_elp,
.read_elp = wl1251_sdio_read_elp,
.reset = wl1251_sdio_reset,
+ .power = wl1251_sdio_set_power,
};
static int wl1251_platform_probe(struct platform_device *pdev)
@@ -239,7 +244,6 @@ static int wl1251_sdio_probe(struct sdio_func *func,
wl_sdio->func = func;
wl->if_priv = wl_sdio;
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;
diff --git a/drivers/net/wireless/wl1251/spi.c b/drivers/net/wireless/wl1251/spi.c
index 88fa8e6..ac872b3 100644
--- a/drivers/net/wireless/wl1251/spi.c
+++ b/drivers/net/wireless/wl1251/spi.c
@@ -215,12 +215,21 @@ static void wl1251_spi_disable_irq(struct wl1251 *wl)
return disable_irq(wl->irq);
}
+static int wl1251_spi_set_power(struct wl1251 *wl, bool enable)
+{
+ if (wl->set_power)
+ wl->set_power(enable);
+
+ return 0;
+}
+
static const struct wl1251_if_operations wl1251_spi_ops = {
.read = wl1251_spi_read,
.write = wl1251_spi_write,
.reset = wl1251_spi_reset_wake,
.enable_irq = wl1251_spi_enable_irq,
.disable_irq = wl1251_spi_disable_irq,
+ .power = wl1251_spi_set_power,
};
static int __devinit wl1251_spi_probe(struct spi_device *spi)
diff --git a/drivers/net/wireless/wl1251/wl1251.h b/drivers/net/wireless/wl1251/wl1251.h
index e113d4c..13fbeec 100644
--- a/drivers/net/wireless/wl1251/wl1251.h
+++ b/drivers/net/wireless/wl1251/wl1251.h
@@ -256,6 +256,7 @@ struct wl1251_if_operations {
void (*write)(struct wl1251 *wl, int addr, void *buf, size_t len);
void (*read_elp)(struct wl1251 *wl, int addr, u32 *val);
void (*write_elp)(struct wl1251 *wl, int addr, u32 val);
+ int (*power)(struct wl1251 *wl, bool enable);
void (*reset)(struct wl1251 *wl);
void (*enable_irq)(struct wl1251 *wl);
void (*disable_irq)(struct wl1251 *wl);
--
1.6.3.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 2/3] wl1251: add runtime PM support for SDIO
2010-11-03 22:13 [PATCH 0/3] wl1251 SDIO patches Grazvydas Ignotas
2010-11-03 22:13 ` [PATCH 1/3] wl1251: add power callback to wl1251_if_operations Grazvydas Ignotas
@ 2010-11-03 22:13 ` Grazvydas Ignotas
2010-11-04 13:51 ` Ohad Ben-Cohen
2010-11-07 10:18 ` Kalle Valo
2010-11-03 22:13 ` [PATCH 3/3] wl1251: use wl12xx_platform_data to pass data Grazvydas Ignotas
2 siblings, 2 replies; 17+ messages in thread
From: Grazvydas Ignotas @ 2010-11-03 22:13 UTC (permalink / raw)
To: linux-wireless
Cc: Kalle Valo, John W. Linville, Grazvydas Ignotas, Ohad Ben-Cohen
Add runtime PM support, similar to how it's done for wl1271.
This allows to power down the card when the driver is loaded but
network is not in use.
CC: Ohad Ben-Cohen <ohad@wizery.com>
Signed-off-by: Grazvydas Ignotas <notasas@gmail.com>
---
drivers/net/wireless/wl1251/sdio.c | 62 ++++++++++++++++++++++++++++++++++--
1 files changed, 59 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/wl1251/sdio.c b/drivers/net/wireless/wl1251/sdio.c
index 0285190..0a5db21 100644
--- a/drivers/net/wireless/wl1251/sdio.c
+++ b/drivers/net/wireless/wl1251/sdio.c
@@ -26,6 +26,7 @@
#include <linux/platform_device.h>
#include <linux/wl12xx.h>
#include <linux/irq.h>
+#include <linux/pm_runtime.h>
#include "wl1251.h"
@@ -173,10 +174,36 @@ static void wl1251_disable_line_irq(struct wl1251 *wl)
static int wl1251_sdio_set_power(struct wl1251 *wl, bool enable)
{
- if (wl->set_power)
- wl->set_power(enable);
+ struct sdio_func *func = wl_to_func(wl);
+ int ret;
- return 0;
+ if (enable) {
+ /* Power up the card */
+ if (wl->set_power)
+ wl->set_power(true);
+
+ ret = pm_runtime_get_sync(&func->dev);
+ if (ret < 0)
+ goto out;
+
+ sdio_claim_host(func);
+ sdio_enable_func(func);
+ sdio_release_host(func);
+ } else {
+ sdio_claim_host(func);
+ sdio_disable_func(func);
+ sdio_release_host(func);
+
+ ret = pm_runtime_put_sync(&func->dev);
+ if (ret < 0)
+ goto out;
+
+ if (wl->set_power)
+ wl->set_power(false);
+ }
+
+out:
+ return ret;
}
static struct wl1251_if_operations wl1251_sdio_ops = {
@@ -277,6 +304,10 @@ static int wl1251_sdio_probe(struct sdio_func *func,
goto out_free_irq;
sdio_set_drvdata(func, wl);
+
+ /* Tell PM core that we don't need the card to be powered now */
+ pm_runtime_put_noidle(&func->dev);
+
return ret;
out_free_irq:
@@ -298,6 +329,9 @@ static void __devexit wl1251_sdio_remove(struct sdio_func *func)
struct wl1251 *wl = sdio_get_drvdata(func);
struct wl1251_sdio *wl_sdio = wl->if_priv;
+ /* Undo decrement done above in wl1271_probe */
+ pm_runtime_get_noresume(&func->dev);
+
if (wl->irq)
free_irq(wl->irq, wl);
kfree(wl_sdio);
@@ -309,11 +343,33 @@ static void __devexit wl1251_sdio_remove(struct sdio_func *func)
sdio_release_host(func);
}
+static int wl1251_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 wl1251_resume(struct device *dev)
+{
+ return 0;
+}
+
+static const struct dev_pm_ops wl1251_sdio_pm_ops = {
+ .suspend = wl1251_suspend,
+ .resume = wl1251_resume,
+};
+
static struct sdio_driver wl1251_sdio_driver = {
.name = "wl1251_sdio",
.id_table = wl1251_devices,
.probe = wl1251_sdio_probe,
.remove = __devexit_p(wl1251_sdio_remove),
+ .drv = {
+ .pm = &wl1251_sdio_pm_ops,
+ },
};
static int __init wl1251_sdio_init(void)
--
1.6.3.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 2/3] wl1251: add runtime PM support for SDIO
2010-11-03 22:13 ` [PATCH 2/3] wl1251: add runtime PM support for SDIO Grazvydas Ignotas
@ 2010-11-04 13:51 ` Ohad Ben-Cohen
2010-11-04 14:37 ` Grazvydas Ignotas
2010-11-07 10:18 ` Kalle Valo
1 sibling, 1 reply; 17+ messages in thread
From: Ohad Ben-Cohen @ 2010-11-04 13:51 UTC (permalink / raw)
To: Grazvydas Ignotas; +Cc: linux-wireless, Kalle Valo, John W. Linville
On Wed, Nov 3, 2010 at 6:13 PM, Grazvydas Ignotas <notasas@gmail.com> wrote:
> Add runtime PM support, similar to how it's done for wl1271.
> This allows to power down the card when the driver is loaded but
> network is not in use.
>
> CC: Ohad Ben-Cohen <ohad@wizery.com>
> Signed-off-by: Grazvydas Ignotas <notasas@gmail.com>
> ---
> drivers/net/wireless/wl1251/sdio.c | 62 ++++++++++++++++++++++++++++++++++--
> 1 files changed, 59 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/wireless/wl1251/sdio.c b/drivers/net/wireless/wl1251/sdio.c
> index 0285190..0a5db21 100644
> --- a/drivers/net/wireless/wl1251/sdio.c
> +++ b/drivers/net/wireless/wl1251/sdio.c
> @@ -26,6 +26,7 @@
> #include <linux/platform_device.h>
> #include <linux/wl12xx.h>
> #include <linux/irq.h>
> +#include <linux/pm_runtime.h>
>
> #include "wl1251.h"
>
> @@ -173,10 +174,36 @@ static void wl1251_disable_line_irq(struct wl1251 *wl)
>
> static int wl1251_sdio_set_power(struct wl1251 *wl, bool enable)
> {
> - if (wl->set_power)
> - wl->set_power(enable);
> + struct sdio_func *func = wl_to_func(wl);
> + int ret;
>
> - return 0;
> + if (enable) {
> + /* Power up the card */
> + if (wl->set_power)
> + wl->set_power(true);
Why do you still need that ->set_power() handler ?
> +
> + ret = pm_runtime_get_sync(&func->dev);
This call should do all the power-related work for you (assuming you
have a fixed regulator in place).
> + if (ret < 0)
> + goto out;
> +
> + sdio_claim_host(func);
> + sdio_enable_func(func);
> + sdio_release_host(func);
> + } else {
> + sdio_claim_host(func);
> + sdio_disable_func(func);
> + sdio_release_host(func);
> +
> + ret = pm_runtime_put_sync(&func->dev);
> + if (ret < 0)
> + goto out;
> +
> + if (wl->set_power)
> + wl->set_power(false);
> + }
> +
> +out:
> + return ret;
> }
>
> static struct wl1251_if_operations wl1251_sdio_ops = {
> @@ -277,6 +304,10 @@ static int wl1251_sdio_probe(struct sdio_func *func,
> goto out_free_irq;
>
> sdio_set_drvdata(func, wl);
> +
> + /* Tell PM core that we don't need the card to be powered now */
> + pm_runtime_put_noidle(&func->dev);
> +
> return ret;
>
> out_free_irq:
> @@ -298,6 +329,9 @@ static void __devexit wl1251_sdio_remove(struct sdio_func *func)
> struct wl1251 *wl = sdio_get_drvdata(func);
> struct wl1251_sdio *wl_sdio = wl->if_priv;
>
> + /* Undo decrement done above in wl1271_probe */
> + pm_runtime_get_noresume(&func->dev);
> +
> if (wl->irq)
> free_irq(wl->irq, wl);
> kfree(wl_sdio);
> @@ -309,11 +343,33 @@ static void __devexit wl1251_sdio_remove(struct sdio_func *func)
> sdio_release_host(func);
> }
>
> +static int wl1251_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 wl1251_resume(struct device *dev)
> +{
> + return 0;
> +}
> +
> +static const struct dev_pm_ops wl1251_sdio_pm_ops = {
> + .suspend = wl1251_suspend,
> + .resume = wl1251_resume,
> +};
> +
> static struct sdio_driver wl1251_sdio_driver = {
> .name = "wl1251_sdio",
> .id_table = wl1251_devices,
> .probe = wl1251_sdio_probe,
> .remove = __devexit_p(wl1251_sdio_remove),
> + .drv = {
> + .pm = &wl1251_sdio_pm_ops,
> + },
> };
>
> static int __init wl1251_sdio_init(void)
> --
> 1.6.3.3
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 2/3] wl1251: add runtime PM support for SDIO
2010-11-04 13:51 ` Ohad Ben-Cohen
@ 2010-11-04 14:37 ` Grazvydas Ignotas
2010-11-04 22:04 ` Ohad Ben-Cohen
0 siblings, 1 reply; 17+ messages in thread
From: Grazvydas Ignotas @ 2010-11-04 14:37 UTC (permalink / raw)
To: Ohad Ben-Cohen; +Cc: linux-wireless, Kalle Valo, John W. Linville
On Thu, Nov 4, 2010 at 3:51 PM, Ohad Ben-Cohen <ohad@wizery.com> wrote:
> On Wed, Nov 3, 2010 at 6:13 PM, Grazvydas Ignotas <notasas@gmail.com> wrote:
>> Add runtime PM support, similar to how it's done for wl1271.
>> This allows to power down the card when the driver is loaded but
>> network is not in use.
>>
>> CC: Ohad Ben-Cohen <ohad@wizery.com>
>> Signed-off-by: Grazvydas Ignotas <notasas@gmail.com>
>> ---
>> drivers/net/wireless/wl1251/sdio.c | 62 ++++++++++++++++++++++++++++++++++--
>> 1 files changed, 59 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/wireless/wl1251/sdio.c b/drivers/net/wireless/wl1251/sdio.c
>> index 0285190..0a5db21 100644
>> --- a/drivers/net/wireless/wl1251/sdio.c
>> +++ b/drivers/net/wireless/wl1251/sdio.c
>> @@ -26,6 +26,7 @@
>> #include <linux/platform_device.h>
>> #include <linux/wl12xx.h>
>> #include <linux/irq.h>
>> +#include <linux/pm_runtime.h>
>>
>> #include "wl1251.h"
>>
>> @@ -173,10 +174,36 @@ static void wl1251_disable_line_irq(struct wl1251 *wl)
>>
>> static int wl1251_sdio_set_power(struct wl1251 *wl, bool enable)
>> {
>> - if (wl->set_power)
>> - wl->set_power(enable);
>> + struct sdio_func *func = wl_to_func(wl);
>> + int ret;
>>
>> - return 0;
>> + if (enable) {
>> + /* Power up the card */
>> + if (wl->set_power)
>> + wl->set_power(true);
>
> Why do you still need that ->set_power() handler ?
On pandora besides power, we also have a GPIO to control clock buffer
for wl1251, so I thought I could enable it here. Perhaps it could be
set up as regulator too, but I'm not sure how to set it up so that
clock is enabled before main power. Also it's not really a power
supply so it somehow felt wrong to set up regulator for it.
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 2/3] wl1251: add runtime PM support for SDIO
2010-11-04 14:37 ` Grazvydas Ignotas
@ 2010-11-04 22:04 ` Ohad Ben-Cohen
2010-11-07 10:20 ` Kalle Valo
0 siblings, 1 reply; 17+ messages in thread
From: Ohad Ben-Cohen @ 2010-11-04 22:04 UTC (permalink / raw)
To: Grazvydas Ignotas; +Cc: linux-wireless, Kalle Valo, John W. Linville
On Thu, Nov 4, 2010 at 10:37 AM, Grazvydas Ignotas <notasas@gmail.com> wrote:
> On Thu, Nov 4, 2010 at 3:51 PM, Ohad Ben-Cohen <ohad@wizery.com> wrote:
>> On Wed, Nov 3, 2010 at 6:13 PM, Grazvydas Ignotas <notasas@gmail.com> wrote:
>>> Add runtime PM support, similar to how it's done for wl1271.
>>> This allows to power down the card when the driver is loaded but
>>> network is not in use.
>>>
>>> CC: Ohad Ben-Cohen <ohad@wizery.com>
>>> Signed-off-by: Grazvydas Ignotas <notasas@gmail.com>
>>> ---
>>> drivers/net/wireless/wl1251/sdio.c | 62 ++++++++++++++++++++++++++++++++++--
>>> 1 files changed, 59 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/wl1251/sdio.c b/drivers/net/wireless/wl1251/sdio.c
>>> index 0285190..0a5db21 100644
>>> --- a/drivers/net/wireless/wl1251/sdio.c
>>> +++ b/drivers/net/wireless/wl1251/sdio.c
>>> @@ -26,6 +26,7 @@
>>> #include <linux/platform_device.h>
>>> #include <linux/wl12xx.h>
>>> #include <linux/irq.h>
>>> +#include <linux/pm_runtime.h>
>>>
>>> #include "wl1251.h"
>>>
>>> @@ -173,10 +174,36 @@ static void wl1251_disable_line_irq(struct wl1251 *wl)
>>>
>>> static int wl1251_sdio_set_power(struct wl1251 *wl, bool enable)
>>> {
>>> - if (wl->set_power)
>>> - wl->set_power(enable);
>>> + struct sdio_func *func = wl_to_func(wl);
>>> + int ret;
>>>
>>> - return 0;
>>> + if (enable) {
>>> + /* Power up the card */
>>> + if (wl->set_power)
>>> + wl->set_power(true);
>>
>> Why do you still need that ->set_power() handler ?
>
> On pandora besides power, we also have a GPIO to control clock buffer
> for wl1251, so I thought I could enable it here.
Oh, I see. The name set_power then is a bit misleading.. ;)
> Perhaps it could be
> set up as regulator too, but I'm not sure how to set it up so that
> clock is enabled before main power. Also it's not really a power
> supply so it somehow felt wrong to set up regulator for it.
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 2/3] wl1251: add runtime PM support for SDIO
2010-11-04 22:04 ` Ohad Ben-Cohen
@ 2010-11-07 10:20 ` Kalle Valo
2010-11-07 10:23 ` Ohad Ben-Cohen
0 siblings, 1 reply; 17+ messages in thread
From: Kalle Valo @ 2010-11-07 10:20 UTC (permalink / raw)
To: Ohad Ben-Cohen; +Cc: Grazvydas Ignotas, linux-wireless, John W. Linville
Ohad Ben-Cohen <ohad@wizery.com> writes:
>>> Why do you still need that ->set_power() handler ?
>>
>> On pandora besides power, we also have a GPIO to control clock buffer
>> for wl1251, so I thought I could enable it here.
>
> Oh, I see. The name set_power then is a bit misleading.. ;)
Maybe a comment stating the reason for the callback would make it more
clear?
--
Kalle Valo
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] wl1251: add runtime PM support for SDIO
2010-11-03 22:13 ` [PATCH 2/3] wl1251: add runtime PM support for SDIO Grazvydas Ignotas
2010-11-04 13:51 ` Ohad Ben-Cohen
@ 2010-11-07 10:18 ` Kalle Valo
2010-11-07 10:25 ` Ohad Ben-Cohen
1 sibling, 1 reply; 17+ messages in thread
From: Kalle Valo @ 2010-11-07 10:18 UTC (permalink / raw)
To: Grazvydas Ignotas; +Cc: linux-wireless, John W. Linville, Ohad Ben-Cohen
Grazvydas Ignotas <notasas@gmail.com> writes:
> Add runtime PM support, similar to how it's done for wl1271.
> This allows to power down the card when the driver is loaded but
> network is not in use.
Few minor comments:
> + /* Undo decrement done above in wl1271_probe */
Should be wl1251_probe().
> +static int wl1251_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;
> +}
Sorry, I'm not familiar with pm_ops and I don't fully understand the
comment above. Does the comment mean that by returning 0 we can
accomplish all that? Or instead is it a fixme comment that we should
do that, but it's not implemented yet?
--
Kalle Valo
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 2/3] wl1251: add runtime PM support for SDIO
2010-11-07 10:18 ` Kalle Valo
@ 2010-11-07 10:25 ` Ohad Ben-Cohen
2010-11-07 10:31 ` Kalle Valo
0 siblings, 1 reply; 17+ messages in thread
From: Ohad Ben-Cohen @ 2010-11-07 10:25 UTC (permalink / raw)
To: Kalle Valo; +Cc: Grazvydas Ignotas, linux-wireless, John W. Linville
On Sun, Nov 7, 2010 at 12:18 PM, Kalle Valo <kvalo@adurom.com> wrote:
>> +static int wl1251_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;
>> +}
>
> Sorry, I'm not familiar with pm_ops and I don't fully understand the
> comment above. Does the comment mean that by returning 0 we can
> accomplish all that?
Yes. By returning 0 we let the MMC core power down our card, but not remove it.
> Or instead is it a fixme comment that we should
> do that, but it's not implemented yet?
>
> --
> Kalle Valo
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 2/3] wl1251: add runtime PM support for SDIO
2010-11-07 10:25 ` Ohad Ben-Cohen
@ 2010-11-07 10:31 ` Kalle Valo
2010-11-08 13:26 ` Grazvydas Ignotas
0 siblings, 1 reply; 17+ messages in thread
From: Kalle Valo @ 2010-11-07 10:31 UTC (permalink / raw)
To: Ohad Ben-Cohen; +Cc: Grazvydas Ignotas, linux-wireless, John W. Linville
Ohad Ben-Cohen <ohad@wizery.com> writes:
> On Sun, Nov 7, 2010 at 12:18 PM, Kalle Valo <kvalo@adurom.com> wrote:
>>> +static int wl1251_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;
>>> +}
>>
>> Sorry, I'm not familiar with pm_ops and I don't fully understand the
>> comment above. Does the comment mean that by returning 0 we can
>> accomplish all that?
>
> Yes. By returning 0 we let the MMC core power down our card, but not
> remove it.
Good, thanks for confirming that.
--
Kalle Valo
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 2/3] wl1251: add runtime PM support for SDIO
2010-11-07 10:31 ` Kalle Valo
@ 2010-11-08 13:26 ` Grazvydas Ignotas
0 siblings, 0 replies; 17+ messages in thread
From: Grazvydas Ignotas @ 2010-11-08 13:26 UTC (permalink / raw)
To: Kalle Valo; +Cc: Ohad Ben-Cohen, linux-wireless, John W. Linville
On Sun, Nov 7, 2010 at 12:31 PM, Kalle Valo <kvalo@adurom.com> wrote:
> Ohad Ben-Cohen <ohad@wizery.com> writes:
>
>> On Sun, Nov 7, 2010 at 12:18 PM, Kalle Valo <kvalo@adurom.com> wrote:
>>>> +static int wl1251_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;
>>>> +}
>>>
>>> Sorry, I'm not familiar with pm_ops and I don't fully understand the
>>> comment above. Does the comment mean that by returning 0 we can
>>> accomplish all that?
>>
>> Yes. By returning 0 we let the MMC core power down our card, but not
>> remove it.
>
> Good, thanks for confirming that.
Thanks, updating comments and resending this one in a moment.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/3] wl1251: use wl12xx_platform_data to pass data
2010-11-03 22:13 [PATCH 0/3] wl1251 SDIO patches Grazvydas Ignotas
2010-11-03 22:13 ` [PATCH 1/3] wl1251: add power callback to wl1251_if_operations Grazvydas Ignotas
2010-11-03 22:13 ` [PATCH 2/3] wl1251: add runtime PM support for SDIO Grazvydas Ignotas
@ 2010-11-03 22:13 ` Grazvydas Ignotas
2010-11-07 10:28 ` Kalle Valo
2 siblings, 1 reply; 17+ messages in thread
From: Grazvydas Ignotas @ 2010-11-03 22:13 UTC (permalink / raw)
To: linux-wireless
Cc: Kalle Valo, John W. Linville, Grazvydas Ignotas, Tony Lindgren,
Ohad Ben-Cohen
Make use the newly added method to pass platform data for wl1251 too.
This allows to eliminate some redundant code.
Cc: Tony Lindgren <tony@atomide.com>
Cc: Ohad Ben-Cohen <ohad@wizery.com>
Signed-off-by: Grazvydas Ignotas <notasas@gmail.com>
---
This touches arch/arm/mach-omap2/* but I think it should go through
the wireless tree to avoid cross-tree dependencies, if Tony and others
are ok with this.
arch/arm/mach-omap2/board-omap3pandora.c | 32 +++++++-------------------
drivers/net/wireless/wl1251/sdio.c | 35 +----------------------------
drivers/net/wireless/wl12xx/Kconfig | 2 +-
3 files changed, 12 insertions(+), 57 deletions(-)
diff --git a/arch/arm/mach-omap2/board-omap3pandora.c b/arch/arm/mach-omap2/board-omap3pandora.c
index 89ed1be..8be2615 100644
--- a/arch/arm/mach-omap2/board-omap3pandora.c
+++ b/arch/arm/mach-omap2/board-omap3pandora.c
@@ -642,31 +642,13 @@ static void __init omap3pandora_init_irq(void)
omap_gpio_init();
}
-static void pandora_wl1251_set_power(bool enable)
-{
- /*
- * Keep power always on until wl1251_sdio driver learns to re-init
- * the chip after powering it down and back up.
- */
-}
-
-static struct wl12xx_platform_data pandora_wl1251_pdata = {
- .set_power = pandora_wl1251_set_power,
- .use_eeprom = true,
-};
-
-static struct platform_device pandora_wl1251_data = {
- .name = "wl1251_data",
- .id = -1,
- .dev = {
- .platform_data = &pandora_wl1251_pdata,
- },
-};
-
-static void pandora_wl1251_init(void)
+static void __init pandora_wl1251_init(void)
{
+ struct wl12xx_platform_data pandora_wl1251_pdata;
int ret;
+ memset(&pandora_wl1251_pdata, 0, sizeof(pandora_wl1251_pdata));
+
ret = gpio_request(PANDORA_WIFI_IRQ_GPIO, "wl1251 irq");
if (ret < 0)
goto fail;
@@ -679,6 +661,11 @@ static void pandora_wl1251_init(void)
if (pandora_wl1251_pdata.irq < 0)
goto fail_irq;
+ pandora_wl1251_pdata.use_eeprom = true;
+ ret = wl12xx_set_platform_data(&pandora_wl1251_pdata);
+ if (ret < 0)
+ goto fail_irq;
+
return;
fail_irq:
@@ -691,7 +678,6 @@ static struct platform_device *omap3pandora_devices[] __initdata = {
&pandora_leds_gpio,
&pandora_keys_gpio,
&pandora_dss_device,
- &pandora_wl1251_data,
&pandora_vwlan_device,
};
diff --git a/drivers/net/wireless/wl1251/sdio.c b/drivers/net/wireless/wl1251/sdio.c
index 0a5db21..c27cc8c 100644
--- a/drivers/net/wireless/wl1251/sdio.c
+++ b/drivers/net/wireless/wl1251/sdio.c
@@ -43,8 +43,6 @@ struct wl1251_sdio {
u32 elp_val;
};
-static struct wl12xx_platform_data *wl12xx_board_data;
-
static struct sdio_func *wl_to_func(struct wl1251 *wl)
{
struct wl1251_sdio *wl_sdio = wl->if_priv;
@@ -215,30 +213,6 @@ static struct wl1251_if_operations wl1251_sdio_ops = {
.power = wl1251_sdio_set_power,
};
-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,
- * until we have a way to pass this through SDIO subsystem or
- * some other way.
- */
-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)
{
@@ -246,6 +220,7 @@ static int wl1251_sdio_probe(struct sdio_func *func,
struct wl1251 *wl;
struct ieee80211_hw *hw;
struct wl1251_sdio *wl_sdio;
+ const struct wl12xx_platform_data *wl12xx_board_data;
hw = wl1251_alloc_hw();
if (IS_ERR(hw))
@@ -272,6 +247,7 @@ static int wl1251_sdio_probe(struct sdio_func *func,
wl->if_priv = wl_sdio;
wl->if_ops = &wl1251_sdio_ops;
+ wl12xx_board_data = wl12xx_get_platform_data();
if (wl12xx_board_data != NULL) {
wl->set_power = wl12xx_board_data->set_power;
wl->irq = wl12xx_board_data->irq;
@@ -376,12 +352,6 @@ 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);
- return err;
- }
-
err = sdio_register_driver(&wl1251_sdio_driver);
if (err)
wl1251_error("failed to register sdio driver: %d", err);
@@ -391,7 +361,6 @@ 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");
}
diff --git a/drivers/net/wireless/wl12xx/Kconfig b/drivers/net/wireless/wl12xx/Kconfig
index b447559..1f4d73f 100644
--- a/drivers/net/wireless/wl12xx/Kconfig
+++ b/drivers/net/wireless/wl12xx/Kconfig
@@ -42,5 +42,5 @@ config WL1271_SDIO
config WL12XX_PLATFORM_DATA
bool
- depends on WL1271_SDIO != n
+ depends on WL1271_SDIO != n || WL1251_SDIO != n
default y
--
1.6.3.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 3/3] wl1251: use wl12xx_platform_data to pass data
2010-11-03 22:13 ` [PATCH 3/3] wl1251: use wl12xx_platform_data to pass data Grazvydas Ignotas
@ 2010-11-07 10:28 ` Kalle Valo
2010-11-08 9:14 ` Luciano Coelho
0 siblings, 1 reply; 17+ messages in thread
From: Kalle Valo @ 2010-11-07 10:28 UTC (permalink / raw)
To: Grazvydas Ignotas
Cc: linux-wireless, John W. Linville, Tony Lindgren, Ohad Ben-Cohen,
luciano.coelho
Grazvydas Ignotas <notasas@gmail.com> writes:
> Make use the newly added method to pass platform data for wl1251 too.
> This allows to eliminate some redundant code.
>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Ohad Ben-Cohen <ohad@wizery.com>
> Signed-off-by: Grazvydas Ignotas <notasas@gmail.com>
For the wl1251 part:
Acked-by: Kalle Valo <kvalo@adurom.com>
> ---
> This touches arch/arm/mach-omap2/* but I think it should go through
> the wireless tree to avoid cross-tree dependencies, if Tony and others
> are ok with this.
I agree, better to push this through the wireless tree.
> arch/arm/mach-omap2/board-omap3pandora.c | 32 +++++++-------------------
> drivers/net/wireless/wl1251/sdio.c | 35 +----------------------------
> drivers/net/wireless/wl12xx/Kconfig | 2 +-
We need to CC Luciano for the wl12xx change.
> --- a/drivers/net/wireless/wl12xx/Kconfig
> +++ b/drivers/net/wireless/wl12xx/Kconfig
> @@ -42,5 +42,5 @@ config WL1271_SDIO
>
> config WL12XX_PLATFORM_DATA
> bool
> - depends on WL1271_SDIO != n
> + depends on WL1271_SDIO != n || WL1251_SDIO != n
> default y
Oh, I didn't take this into account when I moved wl1251 out from the
wl12xx directory. Now wl1251 has a dependency to wl12xx stuff.
Oh well, it's a small issue. I guess we can live with that :)
--
Kalle Valo
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] wl1251: use wl12xx_platform_data to pass data
2010-11-07 10:28 ` Kalle Valo
@ 2010-11-08 9:14 ` Luciano Coelho
2010-11-08 15:19 ` Tony Lindgren
0 siblings, 1 reply; 17+ messages in thread
From: Luciano Coelho @ 2010-11-08 9:14 UTC (permalink / raw)
To: ext Kalle Valo, Grazvydas Ignotas
Cc: linux-wireless@vger.kernel.org, John W. Linville, Tony Lindgren,
Ohad Ben-Cohen
On Sun, 2010-11-07 at 11:28 +0100, ext Kalle Valo wrote:
> Grazvydas Ignotas <notasas@gmail.com> writes:
>
> > Make use the newly added method to pass platform data for wl1251 too.
> > This allows to eliminate some redundant code.
> >
> > Cc: Tony Lindgren <tony@atomide.com>
> > Cc: Ohad Ben-Cohen <ohad@wizery.com>
> > Signed-off-by: Grazvydas Ignotas <notasas@gmail.com>
>
> For the wl1251 part:
>
> Acked-by: Kalle Valo <kvalo@adurom.com>
>
> > ---
> > This touches arch/arm/mach-omap2/* but I think it should go through
> > the wireless tree to avoid cross-tree dependencies, if Tony and others
> > are ok with this.
>
> I agree, better to push this through the wireless tree.
>
> > arch/arm/mach-omap2/board-omap3pandora.c | 32 +++++++-------------------
> > drivers/net/wireless/wl1251/sdio.c | 35 +----------------------------
> > drivers/net/wireless/wl12xx/Kconfig | 2 +-
>
> We need to CC Luciano for the wl12xx change.
>
> > --- a/drivers/net/wireless/wl12xx/Kconfig
> > +++ b/drivers/net/wireless/wl12xx/Kconfig
> > @@ -42,5 +42,5 @@ config WL1271_SDIO
> >
> > config WL12XX_PLATFORM_DATA
> > bool
> > - depends on WL1271_SDIO != n
> > + depends on WL1271_SDIO != n || WL1251_SDIO != n
> > default y
>
> Oh, I didn't take this into account when I moved wl1251 out from the
> wl12xx directory. Now wl1251 has a dependency to wl12xx stuff.
>
> Oh well, it's a small issue. I guess we can live with that :)
Yeah, I didn't realize this either. I think we can live with that, but
at some point we should clean this up and either have separate platform
data for wl1251 and wl12xx or move the the platform data one level up,
to wireless. But for now:
Acked-by: Luciano Coelho <luciano.coelho@nokia.com>
And I agree with pushing this through wireless-testing as well, if it's
okay with Tony and John.
--
Cheers,
Luca.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] wl1251: use wl12xx_platform_data to pass data
2010-11-08 9:14 ` Luciano Coelho
@ 2010-11-08 15:19 ` Tony Lindgren
0 siblings, 0 replies; 17+ messages in thread
From: Tony Lindgren @ 2010-11-08 15:19 UTC (permalink / raw)
To: Luciano Coelho
Cc: ext Kalle Valo, Grazvydas Ignotas, linux-wireless@vger.kernel.org,
John W. Linville, Ohad Ben-Cohen
* Luciano Coelho <luciano.coelho@nokia.com> [101108 01:05]:
> On Sun, 2010-11-07 at 11:28 +0100, ext Kalle Valo wrote:
> > Grazvydas Ignotas <notasas@gmail.com> writes:
> >
> > > Make use the newly added method to pass platform data for wl1251 too.
> > > This allows to eliminate some redundant code.
> > >
> > > Cc: Tony Lindgren <tony@atomide.com>
> > > Cc: Ohad Ben-Cohen <ohad@wizery.com>
> > > Signed-off-by: Grazvydas Ignotas <notasas@gmail.com>
> >
> > For the wl1251 part:
> >
> > Acked-by: Kalle Valo <kvalo@adurom.com>
> >
> > > ---
> > > This touches arch/arm/mach-omap2/* but I think it should go through
> > > the wireless tree to avoid cross-tree dependencies, if Tony and others
> > > are ok with this.
> >
> > I agree, better to push this through the wireless tree.
> >
> > > arch/arm/mach-omap2/board-omap3pandora.c | 32 +++++++-------------------
> > > drivers/net/wireless/wl1251/sdio.c | 35 +----------------------------
> > > drivers/net/wireless/wl12xx/Kconfig | 2 +-
> >
> > We need to CC Luciano for the wl12xx change.
> >
> > > --- a/drivers/net/wireless/wl12xx/Kconfig
> > > +++ b/drivers/net/wireless/wl12xx/Kconfig
> > > @@ -42,5 +42,5 @@ config WL1271_SDIO
> > >
> > > config WL12XX_PLATFORM_DATA
> > > bool
> > > - depends on WL1271_SDIO != n
> > > + depends on WL1271_SDIO != n || WL1251_SDIO != n
> > > default y
> >
> > Oh, I didn't take this into account when I moved wl1251 out from the
> > wl12xx directory. Now wl1251 has a dependency to wl12xx stuff.
> >
> > Oh well, it's a small issue. I guess we can live with that :)
>
> Yeah, I didn't realize this either. I think we can live with that, but
> at some point we should clean this up and either have separate platform
> data for wl1251 and wl12xx or move the the platform data one level up,
> to wireless. But for now:
>
> Acked-by: Luciano Coelho <luciano.coelho@nokia.com>
>
> And I agree with pushing this through wireless-testing as well, if it's
> okay with Tony and John.
Yes, ack from me too:
Acked-by: Tony Lindgren <tony@atomide.com>
^ permalink raw reply [flat|nested] 17+ messages in thread