Re: [PATCH 2/3] wl1251: add runtime PM support for SDIO

[Ohad Ben-Cohen <ohad@wizery.com>]

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.
>