Re: MMC runtime PM patches break libertas probe

[Ohad Ben-Cohen]

On Wed, Nov 17, 2010 at 8:46 AM, Mike Rapoport  wrote:
> On our platforms we just keep it powered on after boot with the reset line 
> held
> high. (e.g. arch/arm/mach-pxa/cm-x300.c, arch/arm/mach-omap/board-cm-t35.c).
> We don't bother much for power savings, though.

Thanks, Mike !

This paves the road for MMC_CAP_POWER_OFF_CARD.

SDIO core will enable runtime PM for the card only if that cap is set.
As a result, the card will be powered down after boot, and will only
be powered up again when a driver is loaded (and then it's up to the
driver whether power will be kept or not).
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: MMC runtime PM patches break libertas probe

[Mike Rapoport]

On 11/16/10 16:49, Ohad Ben-Cohen wrote:
> On Tue, Nov 16, 2010 at 4:29 PM, Daniel Drake  wrote:
>> On 16 November 2010 13:22, Ohad Ben-Cohen  wrote:
>>> Just to update the list, the problem with the XO-1.5 was because the
>>> sd8686 has an external reset gpio line which is currently being
>>> manipulated manually by an out-of-tree kernel patch:
>>>
>>> http://dev.laptop.org/git/olpc-2.6/commit/?h=olpc-2.6.35&id=e9bee721fb0cc303286d1fe5df4930ce79b0b1e0
>>>
>>> ... which makes me wonder whether we really want to take that
>>> MMC_CAP_RUNTIME_PM road. I'm not sure anymore.
>>
>> OLPC is not the only user of the sd8686.
>> Every other user will face the same problem.
>>
>> Other users may not have the luxury of having a GPIO hooked up to the
>> reset line.
> 
> Agree; those users will need a MMC_CAP_RUNTIME_PM (or maybe call it
> with the capability it really stands for which is something like
> MMC_CAP_POWER_OFF_CARD).
> 
> But I want to be positively sure we have such users (or is it that obvious?).
> 
> How is the sd8686's reset line manipulated on other platforms ? Or is
> the sd8686 usually just kept powered on after boot ?

On our platforms we just keep it powered on after boot with the reset line held
high. (e.g. arch/arm/mach-pxa/cm-x300.c, arch/arm/mach-omap/board-cm-t35.c). We
don't bother much for power savings, though.

> I'm looping in libertas-dev.
> 
> Thanks,
> Ohad.
> 
> ___
> libertas-dev mailing list
> libertas-...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/libertas-dev


-- 
Sincerely yours,
Mike.
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/7] mmc: sdhci-of: fix build on non-powerpc platforms

[Grant Likely]

On Tue, Nov 16, 2010 at 04:34:56PM -0600, Rob Herring wrote:
> On 11/16/2010 03:44 PM, Wolfram Sang wrote:
> >On Tue, Nov 16, 2010 at 02:33:52PM -0600, Rob Herring wrote:
> >>From: Rob Herring
> >>
> >>Explicitly include err.h, of_address.h and of_irq.h.
> >>Make use of machine_is() conditional on PPC.
> >>
> >>Signed-off-by: Rob Herring
> >
> >Hmm, sins of the past :/ I wonder if we can get away with less #ifdeffery, 
> >will
> >think about it...
> >
> 
> I don't want to start a long debate, but is updating a kernel
> without updating the dtb really something to worry about?

Yes, once a .dtb is merged we try very hard not to break it.  It may
need to be updated to enable more features, but the goal is to not
regress.  One of the reason being that firmware may provide a default,
but old, dtb and it is important to still be able to boot on those
systems, even if the dtb is immediately going to be updated.

That's one of the reasons why it is so important to document and
review bindings up front and make sure they make sense before we
commit to them.

That being said, there are other ways to deal with old dtbs, like
fixing up the data at platform setup time.

> Isn't a year enough of a transition period.

No.

g.

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

Re: [PATCH] mmci: handle clock frequency 0 properly

[Chris Ball]

Hi Linus,

On Tue, Nov 16, 2010 at 06:17:49PM +0100, Linus Walleij wrote:
> This removes the default clocking for the MMCI controller so that
> the external MCI card clock does not activate until the first
> .set_ios() call is issued. It will further handle the transitions
> from a clock != 0 to 0 and vice versa by gating/ungating the
> clock with clk_disable()/clk_enable().
> 
> This assures that the MCI clock will not be active unless there
> is a card in the MMC slot.
> 
> By default the MMC core will not gate off the clock to a card
> once it's enabled, but with the separate patch for aggressive
> clocking this can optionally be enabled for the system.
> 
> Cc: Chris Ball 
> Cc: Russell King 
> Signed-off-by: Linus Walleij 
> ---
> Changes since v8:
> 
> The frequency registers shall be set with mmci_set_clkreg()
> no matter whether the clock gets enabled or disabled, systems
> without a clk framework will need this so that the clock
> dividers are set to the apropriate values for clock 0 as
> well, and that will probably mitigate power consumption
> somewhat on these systems.
> 
> Chris: this is a new version after Russell found an error in
> it. Can you please take the old version of this patch out of
> the MMC tree so I can merge it through Russells ARM tree
> instead? The patches are perfectly orthogonal so it doesn't
> need to live in the MMC tree.

Okay, this patch has been removed from mmc-next now.

(I'm holding off on pulling the rest of your latest patchset until
you reply to David Vrabel's review of [2/3] -- let me know if you
think it should be pulled now instead.)

Thanks,

-- 
Chris Ball  
One Laptop Per Child
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/7] mmc: sdhci-of: fix build on non-powerpc platforms

[Rob Herring]

On 11/16/2010 03:44 PM, Wolfram Sang wrote:

On Tue, Nov 16, 2010 at 02:33:52PM -0600, Rob Herring wrote:

From: Rob Herring

Explicitly include err.h, of_address.h and of_irq.h.
Make use of machine_is() conditional on PPC.

Signed-off-by: Rob Herring


Hmm, sins of the past :/ I wonder if we can get away with less #ifdeffery, will
think about it...



I don't want to start a long debate, but is updating a kernel without 
updating the dtb really something to worry about? Isn't a year enough of 
a transition period.


Do these machines have a machine level compatible property that could be 
used instead?


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

Re: MMC runtime PM patches break libertas probe

[Ohad Ben-Cohen]

On Tue, Nov 16, 2010 at 11:16 PM, Arnd Hannemann  wrote:
> Yes, just plugged it in. No extra wire whatsover.

I wonder - when you suspend/resume the host, with that card plugged
in, do you see any errors (particularly in the resume phase) ?

After suspend/resume was completed, can you still work with that card
(wlan is still functional) ?

I'm asking because SDIO suspend/resume is very similar to what the new
2.6.37-rc1 SDIO runtime pm code does.

Thanks,
Ohad.

>
> Regards,
> Arnd
>
>> Thanks!
>> Ohad.
>>
>>
>>> I was able to work with 2.6.35 and .36 plus some out-of-tree patches
>>> for the sh_mobile_sdhi mfd, which are now in mainline:
>>> mmc: Allow 2 byte requests in 4-bit mode for tmio_mmc
>>> tmio_mmc: don't clear unhandled pending interrupts
>>> tmio_mmc: don't clear unhandled pending interrupts
>>>
>>>
> From 6b5691bdd8184cc0876d209c69d38844ea10f678 Mon Sep 17 00:00:00 2001
> From: Ohad Ben-Cohen 
> Date: Mon, 1 Nov 2010 09:41:44 +0200
> Subject: [PATCH] mmc: add MMC_CAP_RUNTIME_PM
>
> Some board/card/host configurations are not capable of powering off/on
> on the card during runtime.
>
> To support such configurations, and to allow smoother transition to
> runtime PM behavior, MMC_CAP_RUNTIME_PM is added, so hosts need to
> explicitly indicate that it's OK to power off their cards after boot.
>
> This will prevent sdio_bus_probe() from failing to power on the card
> when the driver is loaded on such setups.
>
> Signed-off-by: Ohad Ben-Cohen 
> ---
>  drivers/mmc/core/sdio.c     |   37 +++--
>  drivers/mmc/core/sdio_bus.c |   33 ++---
>  include/linux/mmc/host.h    |    1 +
>  3 files changed, 46 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
> index 6a9ad40..373d56d 100644
> --- a/drivers/mmc/core/sdio.c
> +++ b/drivers/mmc/core/sdio.c
> @@ -547,9 +547,11 @@ static void mmc_sdio_detect(struct mmc_host *host)
>        BUG_ON(!host->card);
>
>        /* Make sure card is powered before detecting it */
> -       err = pm_runtime_get_sync(&host->card->dev);
> -       if (err < 0)
> -               goto out;
> +       if (host->caps & MMC_CAP_RUNTIME_PM) {
> +               err = pm_runtime_get_sync(&host->card->dev);
> +               if (err < 0)
> +                       goto out;
> +       }
>
>        mmc_claim_host(host);
>
> @@ -570,7 +572,8 @@ out:
>        }
>
>        /* Tell PM core that we're done */
> -       pm_runtime_put(&host->card->dev);
> +       if (host->caps & MMC_CAP_RUNTIME_PM)
> +               pm_runtime_put(&host->card->dev);
>  }
>
>  /*
> @@ -720,16 +723,21 @@ int mmc_attach_sdio(struct mmc_host *host, u32 ocr)
>        card = host->card;
>
>        /*
> -        * Let runtime PM core know our card is active
> +        * Enable runtime PM only if supported by host+card+board
>         */
> -       err = pm_runtime_set_active(&card->dev);
> -       if (err)
> -               goto remove;
> +       if (host->caps & MMC_CAP_RUNTIME_PM) {
> +               /*
> +                * Let runtime PM core know our card is active
> +                */
> +               err = pm_runtime_set_active(&card->dev);
> +               if (err)
> +                       goto remove;
>
> -       /*
> -        * Enable runtime PM for this card
> -        */
> -       pm_runtime_enable(&card->dev);
> +               /*
> +                * Enable runtime PM for this card
> +                */
> +               pm_runtime_enable(&card->dev);
> +       }
>
>        /*
>         * The number of functions on the card is encoded inside
> @@ -747,9 +755,10 @@ int mmc_attach_sdio(struct mmc_host *host, u32 ocr)
>                        goto remove;
>
>                /*
> -                * Enable Runtime PM for this func
> +                * Enable Runtime PM for this func (if supported)
>                 */
> -               pm_runtime_enable(&card->sdio_func[i]->dev);
> +               if (host->caps & MMC_CAP_RUNTIME_PM)
> +                       pm_runtime_enable(&card->sdio_func[i]->dev);
>        }
>
>        mmc_release_host(host);
> diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c
> index 2716c7a..f3ce21b 100644
> --- a/drivers/mmc/core/sdio_bus.c
> +++ b/drivers/mmc/core/sdio_bus.c
> @@ -17,6 +17,7 @@
>  #include 
>
>  #include 
> +#include 
>  #include 
>
>  #include "sdio_cis.h"
> @@ -132,9 +133,11 @@ static int sdio_bus_probe(struct device *dev)
>         * it should call pm_runtime_put_noidle() in its probe routine and
>         *

Re: [PATCH 4/7] mmc: sdhci-of: fix build on non-powerpc platforms

[Wolfram Sang]

On Tue, Nov 16, 2010 at 02:33:52PM -0600, Rob Herring wrote:
> From: Rob Herring 
> 
> Explicitly include err.h, of_address.h and of_irq.h.
> Make use of machine_is() conditional on PPC.
> 
> Signed-off-by: Rob Herring 

Hmm, sins of the past :/ I wonder if we can get away with less #ifdeffery, will
think about it...

> ---
>  drivers/mmc/host/sdhci-of-core.c |9 +
>  1 files changed, 9 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-of-core.c 
> b/drivers/mmc/host/sdhci-of-core.c
> index fa19d84..dd84124 100644
> --- a/drivers/mmc/host/sdhci-of-core.c
> +++ b/drivers/mmc/host/sdhci-of-core.c
> @@ -13,6 +13,7 @@
>   * your option) any later version.
>   */
>  
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -20,8 +21,12 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  #include 
> +#ifdef CONFIG_PPC
>  #include 
> +#endif
>  #include "sdhci-of.h"
>  #include "sdhci.h"
>  
> @@ -112,7 +117,11 @@ static bool __devinit sdhci_of_wp_inverted(struct 
> device_node *np)
>   return true;
>  
>   /* Old device trees don't have the wp-inverted property. */
> +#ifdef CONFIG_PPC
>   return machine_is(mpc837x_rdb) || machine_is(mpc837x_mds);
> +#else
> + return false;
> +#endif
>  }
>  
>  static int __devinit sdhci_of_probe(struct platform_device *ofdev,
> -- 
> 1.7.1
> 
> ___
> devicetree-discuss mailing list
> devicetree-disc...@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature

Re: [PATCH 5/7] mmc: sdhci-of: support generic OF controllers

[Wolfram Sang]

On Tue, Nov 16, 2010 at 02:33:53PM -0600, Rob Herring wrote:
> From: Rob Herring 
> 
> The base sdhci driver requires a valid ops struct. Add empty struct to
> sdhci-of to allow generic controllers which don't need custom ops functions.
> 
> Signed-off-by: Rob Herring 

Acked-by: Wolfram Sang 

For the sdhci-patches, you should add linux-mmc@vger.kernel.org to the CC-list.
Done now...

> ---
>  drivers/mmc/host/sdhci-of-core.c |5 +
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-of-core.c 
> b/drivers/mmc/host/sdhci-of-core.c
> index dd84124..de292fe 100644
> --- a/drivers/mmc/host/sdhci-of-core.c
> +++ b/drivers/mmc/host/sdhci-of-core.c
> @@ -30,6 +30,9 @@
>  #include "sdhci-of.h"
>  #include "sdhci.h"
>  
> +static struct sdhci_ops sdhci_of_ops = {
> +};
> +
>  #ifdef CONFIG_MMC_SDHCI_BIG_ENDIAN_32BIT_BYTE_SWAPPER
>  
>  /*
> @@ -161,6 +164,8 @@ static int __devinit sdhci_of_probe(struct 
> platform_device *ofdev,
>   if (sdhci_of_data) {
>   host->quirks = sdhci_of_data->quirks;
>   host->ops = &sdhci_of_data->ops;
> + } else {
> + host->ops = &sdhci_of_ops;
>   }
>  
>   if (of_get_property(np, "sdhci,auto-cmd12", NULL))
> -- 
> 1.7.1
> 
> ___
> devicetree-discuss mailing list
> devicetree-disc...@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature

Re: MMC runtime PM patches break libertas probe

[Arnd Hannemann]

Am 16.11.2010 21:58, schrieb Ohad Ben-Cohen:
> On Tue, Nov 16, 2010 at 7:17 PM, Arnd Hannemann
>  >> > Its an AP4 (SH7372) evaluation
> board from renesas. It has an SD-Slot,
>   
>> where you plug the SDIO card into it. No special wiring or something
>> like this. So I doubt some external GPIOs are involved.
>> I have no idea how the wifi card gets its power, but I hope its over
>> some well defined pins within the SD slot...
>> 
> Can you please specify what was the wlan card you used ? is it a
> commercial card or an evaluation board of some sort ?
>   
It says: c-guys sd link11b driver is b43.
It's an commercial card.

> You said it, but just to be sure - I assume there is no external power
> supply involved with that card, right ? it's just plugged into the SD
> slot with no extra wire whatsoever ?
>   
Yes, just plugged it in. No extra wire whatsover.

Regards,
Arnd

> Thanks!
> Ohad.
>
>   
>> I was able to work with 2.6.35 and .36 plus some out-of-tree patches
>> for the sh_mobile_sdhi mfd, which are now in mainline:
>> mmc: Allow 2 byte requests in 4-bit mode for tmio_mmc
>> tmio_mmc: don't clear unhandled pending interrupts
>> tmio_mmc: don't clear unhandled pending interrupts
>>
>> 
 From 6b5691bdd8184cc0876d209c69d38844ea10f678 Mon Sep 17 00:00:00 2001
 From: Ohad Ben-Cohen 
 Date: Mon, 1 Nov 2010 09:41:44 +0200
 Subject: [PATCH] mmc: add MMC_CAP_RUNTIME_PM

 Some board/card/host configurations are not capable of powering off/on
 on the card during runtime.

 To support such configurations, and to allow smoother transition to
 runtime PM behavior, MMC_CAP_RUNTIME_PM is added, so hosts need to
 explicitly indicate that it's OK to power off their cards after boot.

 This will prevent sdio_bus_probe() from failing to power on the card
 when the driver is loaded on such setups.

 Signed-off-by: Ohad Ben-Cohen 
 ---
  drivers/mmc/core/sdio.c |   37 +++--
  drivers/mmc/core/sdio_bus.c |   33 ++---
  include/linux/mmc/host.h|1 +
  3 files changed, 46 insertions(+), 25 deletions(-)

 diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
 index 6a9ad40..373d56d 100644
 --- a/drivers/mmc/core/sdio.c
 +++ b/drivers/mmc/core/sdio.c
 @@ -547,9 +547,11 @@ static void mmc_sdio_detect(struct mmc_host *host)
BUG_ON(!host->card);

/* Make sure card is powered before detecting it */
 -   err = pm_runtime_get_sync(&host->card->dev);
 -   if (err < 0)
 -   goto out;
 +   if (host->caps & MMC_CAP_RUNTIME_PM) {
 +   err = pm_runtime_get_sync(&host->card->dev);
 +   if (err < 0)
 +   goto out;
 +   }

mmc_claim_host(host);

 @@ -570,7 +572,8 @@ out:
}

/* Tell PM core that we're done */
 -   pm_runtime_put(&host->card->dev);
 +   if (host->caps & MMC_CAP_RUNTIME_PM)
 +   pm_runtime_put(&host->card->dev);
  }

  /*
 @@ -720,16 +723,21 @@ int mmc_attach_sdio(struct mmc_host *host, u32 ocr)
card = host->card;

/*
 -* Let runtime PM core know our card is active
 +* Enable runtime PM only if supported by host+card+board
 */
 -   err = pm_runtime_set_active(&card->dev);
 -   if (err)
 -   goto remove;
 +   if (host->caps & MMC_CAP_RUNTIME_PM) {
 +   /*
 +* Let runtime PM core know our card is active
 +*/
 +   err = pm_runtime_set_active(&card->dev);
 +   if (err)
 +   goto remove;

 -   /*
 -* Enable runtime PM for this card
 -*/
 -   pm_runtime_enable(&card->dev);
 +   /*
 +* Enable runtime PM for this card
 +*/
 +   pm_runtime_enable(&card->dev);
 +   }

/*
 * The number of functions on the card is encoded inside
 @@ -747,9 +755,10 @@ int mmc_attach_sdio(struct mmc_host *host, u32 ocr)
goto remove;

/*
 -* Enable Runtime PM for this func
 +* Enable Runtime PM for this func (if supported)
 */
 -   pm_runtime_enable(&card->sdio_func[i]->dev);
 +   if (host->caps & MMC_CAP_RUNTIME_PM)
 +   pm_runtime_enable(&card->sdio_func[i]->dev);
}

mmc_release_host(host);
 diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c
 index 2716c7a..f3ce21b 100644
 --- a/drivers/mmc/core/sdio_bus.c
 +++ b/drivers/mmc/core/sdio_bus.c
>

Re: MMC runtime PM patches break libertas probe

[Ohad Ben-Cohen]

On Tue, Nov 16, 2010 at 7:17 PM, Arnd Hannemann
 >> > Its an AP4 (SH7372) evaluation
board from renesas. It has an SD-Slot,
> where you plug the SDIO card into it. No special wiring or something
> like this. So I doubt some external GPIOs are involved.
> I have no idea how the wifi card gets its power, but I hope its over
> some well defined pins within the SD slot...

Can you please specify what was the wlan card you used ? is it a
commercial card or an evaluation board of some sort ?

You said it, but just to be sure - I assume there is no external power
supply involved with that card, right ? it's just plugged into the SD
slot with no extra wire whatsoever ?

Thanks!
Ohad.

> I was able to work with 2.6.35 and .36 plus some out-of-tree patches
> for the sh_mobile_sdhi mfd, which are now in mainline:
> mmc: Allow 2 byte requests in 4-bit mode for tmio_mmc
> tmio_mmc: don't clear unhandled pending interrupts
> tmio_mmc: don't clear unhandled pending interrupts
>
>>> From 6b5691bdd8184cc0876d209c69d38844ea10f678 Mon Sep 17 00:00:00 2001
>>> From: Ohad Ben-Cohen 
>>> Date: Mon, 1 Nov 2010 09:41:44 +0200
>>> Subject: [PATCH] mmc: add MMC_CAP_RUNTIME_PM
>>>
>>> Some board/card/host configurations are not capable of powering off/on
>>> on the card during runtime.
>>>
>>> To support such configurations, and to allow smoother transition to
>>> runtime PM behavior, MMC_CAP_RUNTIME_PM is added, so hosts need to
>>> explicitly indicate that it's OK to power off their cards after boot.
>>>
>>> This will prevent sdio_bus_probe() from failing to power on the card
>>> when the driver is loaded on such setups.
>>>
>>> Signed-off-by: Ohad Ben-Cohen 
>>> ---
>>>  drivers/mmc/core/sdio.c     |   37 +++--
>>>  drivers/mmc/core/sdio_bus.c |   33 ++---
>>>  include/linux/mmc/host.h    |    1 +
>>>  3 files changed, 46 insertions(+), 25 deletions(-)
>>>
>>> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
>>> index 6a9ad40..373d56d 100644
>>> --- a/drivers/mmc/core/sdio.c
>>> +++ b/drivers/mmc/core/sdio.c
>>> @@ -547,9 +547,11 @@ static void mmc_sdio_detect(struct mmc_host *host)
>>>        BUG_ON(!host->card);
>>>
>>>        /* Make sure card is powered before detecting it */
>>> -       err = pm_runtime_get_sync(&host->card->dev);
>>> -       if (err < 0)
>>> -               goto out;
>>> +       if (host->caps & MMC_CAP_RUNTIME_PM) {
>>> +               err = pm_runtime_get_sync(&host->card->dev);
>>> +               if (err < 0)
>>> +                       goto out;
>>> +       }
>>>
>>>        mmc_claim_host(host);
>>>
>>> @@ -570,7 +572,8 @@ out:
>>>        }
>>>
>>>        /* Tell PM core that we're done */
>>> -       pm_runtime_put(&host->card->dev);
>>> +       if (host->caps & MMC_CAP_RUNTIME_PM)
>>> +               pm_runtime_put(&host->card->dev);
>>>  }
>>>
>>>  /*
>>> @@ -720,16 +723,21 @@ int mmc_attach_sdio(struct mmc_host *host, u32 ocr)
>>>        card = host->card;
>>>
>>>        /*
>>> -        * Let runtime PM core know our card is active
>>> +        * Enable runtime PM only if supported by host+card+board
>>>         */
>>> -       err = pm_runtime_set_active(&card->dev);
>>> -       if (err)
>>> -               goto remove;
>>> +       if (host->caps & MMC_CAP_RUNTIME_PM) {
>>> +               /*
>>> +                * Let runtime PM core know our card is active
>>> +                */
>>> +               err = pm_runtime_set_active(&card->dev);
>>> +               if (err)
>>> +                       goto remove;
>>>
>>> -       /*
>>> -        * Enable runtime PM for this card
>>> -        */
>>> -       pm_runtime_enable(&card->dev);
>>> +               /*
>>> +                * Enable runtime PM for this card
>>> +                */
>>> +               pm_runtime_enable(&card->dev);
>>> +       }
>>>
>>>        /*
>>>         * The number of functions on the card is encoded inside
>>> @@ -747,9 +755,10 @@ int mmc_attach_sdio(struct mmc_host *host, u32 ocr)
>>>                        goto remove;
>>>
>>>                /*
>>> -                * Enable Runtime PM for this func
>>> +                * Enable Runtime PM for this func (if supported)
>>>                 */
>>> -               pm_runtime_enable(&card->sdio_func[i]->dev);
>>> +               if (host->caps & MMC_CAP_RUNTIME_PM)
>>> +                       pm_runtime_enable(&card->sdio_func[i]->dev);
>>>        }
>>>
>>>        mmc_release_host(host);
>>> diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c
>>> index 2716c7a..f3ce21b 100644
>>> --- a/drivers/mmc/core/sdio_bus.c
>>> +++ b/drivers/mmc/core/sdio_bus.c
>>> @@ -17,6 +17,7 @@
>>>  #include 
>>>
>>>  #include 
>>> +#include 
>>>  #include 
>>>
>>>  #include "sdio_cis.h"
>>> @@ -132,9 +133,11 @@ static int sdio_bus_probe(struct device *dev)
>>>         * it should call pm_runtime_put_noidle() in its probe routine and
>>>         * pm_runtime_get_noresume() in its remove routine.
>>>         *

Re: omap4: hsmmc: Fix improper card detection while booting

[Tony Lindgren]

* kishore kadiyala  [101115 00:59]:
> While booting OMAP4 ES2.0 boards, cards on MMC1 and MMC2 controllers
> are not getting detected some times.
> 
> During reset of command/data line, wrong pointer to base address
> was passed while read operation to SYSCTL register, thus impacting
> the updated reset logic.
> Passing correct base address fixes the issue.
> 
> Signed-off-by: Kishore Kadiyala 
> Acked-by: Felipie Balbi 
> Cc: Tony Lindgren 
> Cc: Chris Ball   
> Cc: Madhusudhan Chikkature 

Good find:

Acked-by: Tony Lindgren 

> ---
>  drivers/mmc/host/omap_hsmmc.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index 82a1079..5d46021 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -1002,7 +1002,7 @@ static inline void 
> omap_hsmmc_reset_controller_fsm(struct omap_hsmmc_host
> *host,
>* Monitor a 0->1 transition first
>*/
>   if (mmc_slot(host).features & HSMMC_HAS_UPDATED_RESET) {
> - while ((!(OMAP_HSMMC_READ(host, SYSCTL) & bit))
> + while ((!(OMAP_HSMMC_READ(host->base, SYSCTL) & bit))
>   && (i++ < limit))
>   cpu_relax();
>   }
> -- 
> 1.7.1
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] mmc: protect against clockgate races

[David Vrabel]

Linus Walleij wrote:
> This adds a mutex to protect from races between gate and ungate
> calls colliding.

Can you rework this to not require both host->clk_gate_mutex and
host->clk_lock?  Is host->clk_lock necessary any more?

David

> Cc: Ohad Ben-Cohen 
> Signed-off-by: Linus Walleij 
> ---
> This should fix the race observed by Ohad earlier.
> ---
>  drivers/mmc/core/host.c  |   11 ---
>  include/linux/mmc/host.h |1 +
>  2 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> index f74b386..92e3370 100644
> --- a/drivers/mmc/core/host.c
> +++ b/drivers/mmc/core/host.c
> @@ -68,9 +68,9 @@ static void mmc_host_clk_gate_delayed(struct mmc_host *host)
>   unsigned long flags;
>  
>   if (!freq) {
> - pr_err("%s: frequency set to 0 in disable function, "
> -"this means the clock is already disabled.\n",
> -mmc_hostname(host));
> + pr_debug("%s: frequency set to 0 in disable function, "
> +  "this means the clock is already disabled.\n",
> +  mmc_hostname(host));
>   return;
>   }
>   /*
> @@ -94,6 +94,7 @@ static void mmc_host_clk_gate_delayed(struct mmc_host *host)
>   spin_unlock_irqrestore(&host->clk_lock, flags);
>   return;
>   }
> + mutex_lock(&host->clk_gate_mutex);
>   spin_lock_irqsave(&host->clk_lock, flags);
>   if (!host->clk_requests) {
>   spin_unlock_irqrestore(&host->clk_lock, flags);
> @@ -103,6 +104,7 @@ static void mmc_host_clk_gate_delayed(struct mmc_host 
> *host)
>   pr_debug("%s: gated MCI clock\n", mmc_hostname(host));
>   }
>   spin_unlock_irqrestore(&host->clk_lock, flags);
> + mutex_unlock(&host->clk_gate_mutex);
>  }
>  
>  /*
> @@ -128,6 +130,7 @@ void mmc_host_clk_ungate(struct mmc_host *host)
>  {
>   unsigned long flags;
>  
> + mutex_lock(&host->clk_gate_mutex);
>   spin_lock_irqsave(&host->clk_lock, flags);
>   if (host->clk_gated) {
>   spin_unlock_irqrestore(&host->clk_lock, flags);
> @@ -137,6 +140,7 @@ void mmc_host_clk_ungate(struct mmc_host *host)
>   }
>   host->clk_requests++;
>   spin_unlock_irqrestore(&host->clk_lock, flags);
> + mutex_unlock(&host->clk_gate_mutex);
>  }
>  
>  /**
> @@ -214,6 +218,7 @@ static inline void mmc_host_clk_init(struct mmc_host 
> *host)
>   host->clk_gated = false;
>   INIT_WORK(&host->clk_gate_work, mmc_host_clk_gate_work);
>   spin_lock_init(&host->clk_lock);
> + mutex_init(&host->clk_gate_mutex);
>  }
>  
>  /**
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 955bc10..53496bb 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -178,6 +178,7 @@ struct mmc_host {
>   struct work_struct  clk_gate_work; /* delayed clock gate */
>   unsigned intclk_old;/* old clock value cache */
>   spinlock_t  clk_lock;   /* lock for clk fields */
> + struct mutexclk_gate_mutex; /* mutex for clock gating */
>  #endif
>  
>   /* host specific block data */


-- 
David Vrabel, Senior Software Engineer, Drivers
CSR, Churchill House, Cambridge Business Park,  Tel: +44 (0)1223 692562
Cowley Road, Cambridge, CB4 0WZ http://www.csr.com/


Member of the CSR plc group of companies. CSR plc registered in England and 
Wales, registered number 4187346, registered office Churchill House, Cambridge 
Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] mmci: handle clock frequency 0 properly

[Linus Walleij]

This removes the default clocking for the MMCI controller so that
the external MCI card clock does not activate until the first
.set_ios() call is issued. It will further handle the transitions
from a clock != 0 to 0 and vice versa by gating/ungating the
clock with clk_disable()/clk_enable().

This assures that the MCI clock will not be active unless there
is a card in the MMC slot.

By default the MMC core will not gate off the clock to a card
once it's enabled, but with the separate patch for aggressive
clocking this can optionally be enabled for the system.

Cc: Chris Ball 
Cc: Russell King 
Signed-off-by: Linus Walleij 
---
Changes since v8:

The frequency registers shall be set with mmci_set_clkreg()
no matter whether the clock gets enabled or disabled, systems
without a clk framework will need this so that the clock
dividers are set to the apropriate values for clock 0 as
well, and that will probably mitigate power consumption
somewhat on these systems.

Chris: this is a new version after Russell found an error in
it. Can you please take the old version of this patch out of
the MMC tree so I can merge it through Russells ARM tree
instead? The patches are perfectly orthogonal so it doesn't
need to live in the MMC tree.
---
 drivers/mmc/host/mmci.c |   33 ++---
 drivers/mmc/host/mmci.h |1 +
 2 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 0814b88..3709ab3 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -689,6 +689,22 @@ static void mmci_set_ios(struct mmc_host *mmc, struct 
mmc_ios *ios)
 
mmci_set_clkreg(host, ios->clock);
 
+   /*
+* Turn on clock whenever ios->clock transitions
+* from 0 to !=0 and gate it off whenever ios->clock
+* transitions from !=0 to 0.
+*/
+   if (host->iosclock == 0 && ios->clock != 0) {
+   dev_dbg(mmc_dev(mmc), "enable clock f=%d\n", ios->clock);
+   clk_enable(host->clk);
+   } else if (host->iosclock != 0 && ios->clock == 0) {
+   dev_dbg(mmc_dev(mmc), "disable clock\n");
+   clk_disable(host->clk);
+   } else if (ios->clock != 0) {
+   dev_dbg(mmc_dev(mmc), "set clock f=%d\n", ios->clock);
+   }
+   host->iosclock = ios->clock;
+
if (host->pwr != pwr) {
host->pwr = pwr;
writel(pwr, host->base + MMCIPOWER);
@@ -772,6 +788,8 @@ static int __devinit mmci_probe(struct amba_device *dev, 
struct amba_id *id)
 
host = mmc_priv(mmc);
host->mmc = mmc;
+   host->plat = plat;
+   host->variant = variant;
 
host->gpio_wp = -ENOSYS;
host->gpio_cd = -ENOSYS;
@@ -782,19 +800,14 @@ static int __devinit mmci_probe(struct amba_device *dev, 
struct amba_id *id)
dev_dbg(mmc_dev(mmc), "designer ID = 0x%02x\n", host->hw_designer);
dev_dbg(mmc_dev(mmc), "revision = 0x%01x\n", host->hw_revision);
 
+   /* This clock will be enabled/disabled by set_ios() calls later */
host->clk = clk_get(&dev->dev, NULL);
if (IS_ERR(host->clk)) {
ret = PTR_ERR(host->clk);
host->clk = NULL;
goto host_free;
}
-
-   ret = clk_enable(host->clk);
-   if (ret)
-   goto clk_free;
-
-   host->plat = plat;
-   host->variant = variant;
+   host->iosclock = 0;
host->mclk = clk_get_rate(host->clk);
/*
 * According to the spec, mclk is max 100 MHz,
@@ -804,7 +817,7 @@ static int __devinit mmci_probe(struct amba_device *dev, 
struct amba_id *id)
if (host->mclk > 1) {
ret = clk_set_rate(host->clk, 1);
if (ret < 0)
-   goto clk_disable;
+   goto clk_free;
host->mclk = clk_get_rate(host->clk);
dev_dbg(mmc_dev(mmc), "eventual mclk rate: %u Hz\n",
host->mclk);
@@ -812,7 +825,7 @@ static int __devinit mmci_probe(struct amba_device *dev, 
struct amba_id *id)
host->base = ioremap(dev->res.start, resource_size(&dev->res));
if (!host->base) {
ret = -ENOMEM;
-   goto clk_disable;
+   goto clk_free;
}
 
mmc->ops = &mmci_ops;
@@ -961,8 +974,6 @@ static int __devinit mmci_probe(struct amba_device *dev, 
struct amba_id *id)
gpio_free(host->gpio_cd);
  err_gpio_cd:
iounmap(host->base);
- clk_disable:
-   clk_disable(host->clk);
  clk_free:
clk_put(host->clk);
  host_free:
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index df06f01..4791a2b 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -167,6 +167,7 @@ struct mmci_host {
 
unsigned intmclk;
unsigned intcclk;
+   unsigned intiosclock;
u32 pwr;

Re: MMC runtime PM patches break libertas probe

[Arnd Hannemann]

Am 16.11.2010 14:22, schrieb Ohad Ben-Cohen:
> On Mon, Nov 1, 2010 at 10:27 AM, Ohad Ben-Cohen  wrote:
>   
>> On Sun, Oct 31, 2010 at 9:06 PM, Ohad Ben-Cohen  wrote:
>> ...
>> 
>>> we need to support boards with controllers/cards
>>> which we can't power off in runtime.
>>>
>>> On such boards, the right thing to do would be to disable runtime PM 
>>> altogether.
>>>   
>> The patch below (/attached) should trivially fix the problem - can you
>> please check it out ?
>>
>> It's very simple: it just requires hosts to explicitly indicate they
>> support runtime powering off/on their cards (by means of
>> MMC_CAP_RUNTIME_PM).
>>
>> There is no way around this I'm afraid: it is legitimate for a
>> board/host/card configuration not to support powering off the card
>> after boot, and we must allow it.
>>
>> In addition having an explicit MMC_CAP_RUNTIME_PM will also allow us
>> smoother transition to runtime PM behavior. Developers will have to
>> explicitly turn it on, and will not be surprised if things won't
>> immediately work.
>>
>> Please note that this cap is not interchangeable, and can't be replace
>> with CONFIG_PM_RUNTIME.
>>
>> Having said that, I still think we need to understand why CMD3 timed
>> out on the XO-1.5.
>> 
> Just to update the list, the problem with the XO-1.5 was because the
> sd8686 has an external reset gpio line which is currently being
> manipulated manually by an out-of-tree kernel patch:
>
> http://dev.laptop.org/git/olpc-2.6/commit/?h=olpc-2.6.35&id=e9bee721fb0cc303286d1fe5df4930ce79b0b1e0
>
> ... which makes me wonder whether we really want to take that
> MMC_CAP_RUNTIME_PM road. I'm not sure anymore.
>
> We need a demonstrated hardware limitation before we take that
> approach (or a setup which worked using vanilla kernels and now
> doesn't), because frankly this MMC_CAP_RUNTIME_PM approach is
> cumbersome and involving. I would not like to introduce it just to fix
> out-of-tree software issues, and it seems that at least the XO-1.5
> case can be cleanly solved by software (e.g. by letting the regulator
> handle that sd8686 quirk).
>
> I'm looping in Arnd, who reported similar problems with b43-sdio on
> AP4EVB (arm) with tmio_mmc.
>
> Arnd,
>
> We're trying to exactly understand the reasons behind the reported
> SDIO runtime pm failures (we had two, yours, and OLPC). So far it
> seems that the OLPC had a software issue, and I'm wondering about
> yours.
>
> Can you please supply additional information about your board ? where
> does your wifi card gets its power from ? is there an external gpio
> involved ? Were you able to work with vanilla kernels (prior to
> 2.6.37) or do you carry some out-of-tree patches ?
>   
Its an AP4 (SH7372) evaluation board from renesas. It has an SD-Slot,
where you plug the SDIO card into it. No special wiring or something
like this. So I doubt some external GPIOs are involved.
I have no idea how the wifi card gets its power, but I hope its over
some well defined pins within the SD slot...
I was able to work with 2.6.35 and .36 plus some out-of-tree patches
for the sh_mobile_sdhi mfd, which are now in mainline:
mmc: Allow 2 byte requests in 4-bit mode for tmio_mmc
tmio_mmc: don't clear unhandled pending interrupts
tmio_mmc: don't clear unhandled pending interrupts

>> From 6b5691bdd8184cc0876d209c69d38844ea10f678 Mon Sep 17 00:00:00 2001
>> From: Ohad Ben-Cohen 
>> Date: Mon, 1 Nov 2010 09:41:44 +0200
>> Subject: [PATCH] mmc: add MMC_CAP_RUNTIME_PM
>>
>> Some board/card/host configurations are not capable of powering off/on
>> on the card during runtime.
>>
>> To support such configurations, and to allow smoother transition to
>> runtime PM behavior, MMC_CAP_RUNTIME_PM is added, so hosts need to
>> explicitly indicate that it's OK to power off their cards after boot.
>>
>> This will prevent sdio_bus_probe() from failing to power on the card
>> when the driver is loaded on such setups.
>>
>> Signed-off-by: Ohad Ben-Cohen 
>> ---
>>  drivers/mmc/core/sdio.c |   37 +++--
>>  drivers/mmc/core/sdio_bus.c |   33 ++---
>>  include/linux/mmc/host.h|1 +
>>  3 files changed, 46 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
>> index 6a9ad40..373d56d 100644
>> --- a/drivers/mmc/core/sdio.c
>> +++ b/drivers/mmc/core/sdio.c
>> @@ -547,9 +547,11 @@ static void mmc_sdio_detect(struct mmc_host *host)
>>BUG_ON(!host->card);
>>
>>/* Make sure card is powered before detecting it */
>> -   err = pm_runtime_get_sync(&host->card->dev);
>> -   if (err < 0)
>> -   goto out;
>> +   if (host->caps & MMC_CAP_RUNTIME_PM) {
>> +   err = pm_runtime_get_sync(&host->card->dev);
>> +   if (err < 0)
>> +   goto out;
>> +   }
>>
>>mmc_claim_host(host);
>>
>> @@ -570,7 +572,8 @@ out:
>>}
>>
>>/* Tell PM core that we're done */
>> -  

[PATCH 2/3] mmc: protect against clockgate races

[Linus Walleij]

This adds a mutex to protect from races between gate and ungate
calls colliding.

Cc: Ohad Ben-Cohen 
Signed-off-by: Linus Walleij 
---
This should fix the race observed by Ohad earlier.
---
 drivers/mmc/core/host.c  |   11 ---
 include/linux/mmc/host.h |1 +
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index f74b386..92e3370 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -68,9 +68,9 @@ static void mmc_host_clk_gate_delayed(struct mmc_host *host)
unsigned long flags;
 
if (!freq) {
-   pr_err("%s: frequency set to 0 in disable function, "
-  "this means the clock is already disabled.\n",
-  mmc_hostname(host));
+   pr_debug("%s: frequency set to 0 in disable function, "
+"this means the clock is already disabled.\n",
+mmc_hostname(host));
return;
}
/*
@@ -94,6 +94,7 @@ static void mmc_host_clk_gate_delayed(struct mmc_host *host)
spin_unlock_irqrestore(&host->clk_lock, flags);
return;
}
+   mutex_lock(&host->clk_gate_mutex);
spin_lock_irqsave(&host->clk_lock, flags);
if (!host->clk_requests) {
spin_unlock_irqrestore(&host->clk_lock, flags);
@@ -103,6 +104,7 @@ static void mmc_host_clk_gate_delayed(struct mmc_host *host)
pr_debug("%s: gated MCI clock\n", mmc_hostname(host));
}
spin_unlock_irqrestore(&host->clk_lock, flags);
+   mutex_unlock(&host->clk_gate_mutex);
 }
 
 /*
@@ -128,6 +130,7 @@ void mmc_host_clk_ungate(struct mmc_host *host)
 {
unsigned long flags;
 
+   mutex_lock(&host->clk_gate_mutex);
spin_lock_irqsave(&host->clk_lock, flags);
if (host->clk_gated) {
spin_unlock_irqrestore(&host->clk_lock, flags);
@@ -137,6 +140,7 @@ void mmc_host_clk_ungate(struct mmc_host *host)
}
host->clk_requests++;
spin_unlock_irqrestore(&host->clk_lock, flags);
+   mutex_unlock(&host->clk_gate_mutex);
 }
 
 /**
@@ -214,6 +218,7 @@ static inline void mmc_host_clk_init(struct mmc_host *host)
host->clk_gated = false;
INIT_WORK(&host->clk_gate_work, mmc_host_clk_gate_work);
spin_lock_init(&host->clk_lock);
+   mutex_init(&host->clk_gate_mutex);
 }
 
 /**
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 955bc10..53496bb 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -178,6 +178,7 @@ struct mmc_host {
struct work_struct  clk_gate_work; /* delayed clock gate */
unsigned intclk_old;/* old clock value cache */
spinlock_t  clk_lock;   /* lock for clk fields */
+   struct mutexclk_gate_mutex; /* mutex for clock gating */
 #endif
 
/* host specific block data */
-- 
1.6.3.3

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

[PATCH 1/3] mmc: clean up clockgating code

[Linus Walleij]

This rids the unused boolean clk_pending_gate from the code
and renames the clk_disable_work to clk_gate_work in
analogy with the other names used for gating code.

Signed-off-by: Linus Walleij 
---
 drivers/mmc/core/host.c  |   19 ++-
 include/linux/mmc/host.h |3 +--
 2 files changed, 7 insertions(+), 15 deletions(-)

diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index aacd9c5..f74b386 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -66,7 +66,6 @@ static void mmc_host_clk_gate_delayed(struct mmc_host *host)
unsigned long tick_ns;
unsigned long freq = host->ios.clock;
unsigned long flags;
-   int users;
 
if (!freq) {
pr_err("%s: frequency set to 0 in disable function, "
@@ -80,20 +79,18 @@ static void mmc_host_clk_gate_delayed(struct mmc_host *host)
 * clk_disable().
 */
spin_lock_irqsave(&host->clk_lock, flags);
-   users = host->clk_requests;
 
/*
 * Delay n bus cycles (at least 8 from MMC spec) before attempting
 * to disable the MCI block clock. The reference count may have
 * gone up again after this delay due to rescheduling!
 */
-   if (!users) {
+   if (!host->clk_requests) {
spin_unlock_irqrestore(&host->clk_lock, flags);
tick_ns = DIV_ROUND_UP(10, freq);
ndelay(host->clk_delay * tick_ns);
} else {
/* New users appeared while waiting for this work */
-   host->clk_pending_gate = false;
spin_unlock_irqrestore(&host->clk_lock, flags);
return;
}
@@ -105,7 +102,6 @@ static void mmc_host_clk_gate_delayed(struct mmc_host *host)
spin_lock_irqsave(&host->clk_lock, flags);
pr_debug("%s: gated MCI clock\n", mmc_hostname(host));
}
-   host->clk_pending_gate = false;
spin_unlock_irqrestore(&host->clk_lock, flags);
 }
 
@@ -115,7 +111,7 @@ static void mmc_host_clk_gate_delayed(struct mmc_host *host)
 static void mmc_host_clk_gate_work(struct work_struct *work)
 {
struct mmc_host *host = container_of(work, struct mmc_host,
- clk_disable_work);
+ clk_gate_work);
 
mmc_host_clk_gate_delayed(host);
 }
@@ -181,10 +177,8 @@ void mmc_host_clk_gate(struct mmc_host *host)
spin_lock_irqsave(&host->clk_lock, flags);
host->clk_requests--;
if (mmc_host_may_gate_card(host->card) &&
-   !host->clk_requests) {
-   host->clk_pending_gate = true;
-   schedule_work(&host->clk_disable_work);
-   }
+   !host->clk_requests)
+   schedule_work(&host->clk_gate_work);
spin_unlock_irqrestore(&host->clk_lock, flags);
 }
 
@@ -218,8 +212,7 @@ static inline void mmc_host_clk_init(struct mmc_host *host)
/* Hold MCI clock for 8 cycles by default */
host->clk_delay = 8;
host->clk_gated = false;
-   host->clk_pending_gate = false;
-   INIT_WORK(&host->clk_disable_work, mmc_host_clk_gate_work);
+   INIT_WORK(&host->clk_gate_work, mmc_host_clk_gate_work);
spin_lock_init(&host->clk_lock);
 }
 
@@ -233,7 +226,7 @@ static inline void mmc_host_clk_exit(struct mmc_host *host)
 * Wait for any outstanding gate and then make sure we're
 * ungated before exiting.
 */
-   if (cancel_work_sync(&host->clk_disable_work))
+   if (cancel_work_sync(&host->clk_gate_work))
mmc_host_clk_gate_delayed(host);
if (host->clk_gated)
mmc_host_clk_ungate(host);
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index f108cee..955bc10 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -175,8 +175,7 @@ struct mmc_host {
int clk_requests;   /* internal reference counter */
unsigned intclk_delay;  /* number of MCI clk hold 
cycles */
boolclk_gated;  /* clock gated */
-   boolclk_pending_gate; /* pending clock gating */
-   struct work_struct  clk_disable_work; /* delayed clock disable */
+   struct work_struct  clk_gate_work; /* delayed clock gate */
unsigned intclk_old;/* old clock value cache */
spinlock_t  clk_lock;   /* lock for clk fields */
 #endif
-- 
1.6.3.3

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

[PATCH 3/3] mmc: protect some gating variables

[Linus Walleij]

This adds some more picky variable protection to the state holders
for the clock gating code. This is particularly important when
ordinary .set_ios() calls would race with the .set_ios() call
resulting from a delayed gate operation.

Signed-off-by: Linus Walleij 
---
 drivers/mmc/core/core.c |   33 -
 drivers/mmc/core/core.h |1 +
 2 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 8afc620..c5ae489 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -617,15 +617,8 @@ static inline void mmc_set_ios(struct mmc_host *host)
 ios->power_mode, ios->chip_select, ios->vdd,
 ios->bus_width, ios->timing);
 
-#ifdef CONFIG_MMC_CLKGATE
-   /*
-* We've been given a new frequency while the clock is gated,
-* so make sure we regard this as ungating it.
-*/
-   if (ios->clock > 0 && host->clk_gated)
-   host->clk_gated = false;
-#endif
-
+   if (ios->clock > 0)
+   mmc_set_ungated(host);
host->ops->set_ios(host, ios);
 }
 
@@ -659,9 +652,13 @@ void mmc_set_clock(struct mmc_host *host, unsigned int hz)
  */
 void mmc_gate_clock(struct mmc_host *host)
 {
+   unsigned long flags;
+
+   spin_lock_irqsave(&host->clk_lock, flags);
host->clk_old = host->ios.clock;
host->ios.clock = 0;
host->clk_gated = true;
+   spin_unlock_irqrestore(&host->clk_lock, flags);
mmc_set_ios(host);
 }
 
@@ -680,9 +677,27 @@ void mmc_ungate_clock(struct mmc_host *host)
 */
if (host->clk_old) {
BUG_ON(host->ios.clock);
+   /* This call will also set host->clk_gated to false */
mmc_set_clock(host, host->clk_old);
}
+}
+
+void mmc_set_ungated(struct mmc_host *host)
+{
+   unsigned long flags;
+
+   /*
+* We've been given a new frequency while the clock is gated,
+* so make sure we regard this as ungating it.
+*/
+   spin_lock_irqsave(&host->clk_lock, flags);
host->clk_gated = false;
+   spin_unlock_irqrestore(&host->clk_lock, flags);
+}
+
+#else
+void mmc_set_ungated(struct mmc_host *host)
+{
 }
 #endif
 
diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
index 9972808..026c975 100644
--- a/drivers/mmc/core/core.h
+++ b/drivers/mmc/core/core.h
@@ -35,6 +35,7 @@ void mmc_set_chip_select(struct mmc_host *host, int mode);
 void mmc_set_clock(struct mmc_host *host, unsigned int hz);
 void mmc_gate_clock(struct mmc_host *host);
 void mmc_ungate_clock(struct mmc_host *host);
+void mmc_set_ungated(struct mmc_host *host);
 void mmc_set_bus_mode(struct mmc_host *host, unsigned int mode);
 void mmc_set_bus_width(struct mmc_host *host, unsigned int width);
 void mmc_set_bus_width_ddr(struct mmc_host *host, unsigned int width,
-- 
1.6.3.3

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

Re: MMC runtime PM patches break libertas probe

[Ohad Ben-Cohen]

On Mon, Nov 1, 2010 at 10:27 AM, Ohad Ben-Cohen  wrote:
> On Sun, Oct 31, 2010 at 9:06 PM, Ohad Ben-Cohen  wrote:
> ...
>> we need to support boards with controllers/cards
>> which we can't power off in runtime.
>>
>> On such boards, the right thing to do would be to disable runtime PM 
>> altogether.
>
> The patch below (/attached) should trivially fix the problem - can you
> please check it out ?
>
> It's very simple: it just requires hosts to explicitly indicate they
> support runtime powering off/on their cards (by means of
> MMC_CAP_RUNTIME_PM).
>
> There is no way around this I'm afraid: it is legitimate for a
> board/host/card configuration not to support powering off the card
> after boot, and we must allow it.
>
> In addition having an explicit MMC_CAP_RUNTIME_PM will also allow us
> smoother transition to runtime PM behavior. Developers will have to
> explicitly turn it on, and will not be surprised if things won't
> immediately work.
>
> Please note that this cap is not interchangeable, and can't be replace
> with CONFIG_PM_RUNTIME.
>
> Having said that, I still think we need to understand why CMD3 timed
> out on the XO-1.5.

Just to update the list, the problem with the XO-1.5 was because the
sd8686 has an external reset gpio line which is currently being
manipulated manually by an out-of-tree kernel patch:

http://dev.laptop.org/git/olpc-2.6/commit/?h=olpc-2.6.35&id=e9bee721fb0cc303286d1fe5df4930ce79b0b1e0

... which makes me wonder whether we really want to take that
MMC_CAP_RUNTIME_PM road. I'm not sure anymore.

We need a demonstrated hardware limitation before we take that
approach (or a setup which worked using vanilla kernels and now
doesn't), because frankly this MMC_CAP_RUNTIME_PM approach is
cumbersome and involving. I would not like to introduce it just to fix
out-of-tree software issues, and it seems that at least the XO-1.5
case can be cleanly solved by software (e.g. by letting the regulator
handle that sd8686 quirk).

I'm looping in Arnd, who reported similar problems with b43-sdio on
AP4EVB (arm) with tmio_mmc.

Arnd,

We're trying to exactly understand the reasons behind the reported
SDIO runtime pm failures (we had two, yours, and OLPC). So far it
seems that the OLPC had a software issue, and I'm wondering about
yours.

Can you please supply additional information about your board ? where
does your wifi card gets its power from ? is there an external gpio
involved ? Were you able to work with vanilla kernels (prior to
2.6.37) or do you carry some out-of-tree patches ?

Thanks,
Ohad.

>
> From 6b5691bdd8184cc0876d209c69d38844ea10f678 Mon Sep 17 00:00:00 2001
> From: Ohad Ben-Cohen 
> Date: Mon, 1 Nov 2010 09:41:44 +0200
> Subject: [PATCH] mmc: add MMC_CAP_RUNTIME_PM
>
> Some board/card/host configurations are not capable of powering off/on
> on the card during runtime.
>
> To support such configurations, and to allow smoother transition to
> runtime PM behavior, MMC_CAP_RUNTIME_PM is added, so hosts need to
> explicitly indicate that it's OK to power off their cards after boot.
>
> This will prevent sdio_bus_probe() from failing to power on the card
> when the driver is loaded on such setups.
>
> Signed-off-by: Ohad Ben-Cohen 
> ---
>  drivers/mmc/core/sdio.c     |   37 +++--
>  drivers/mmc/core/sdio_bus.c |   33 ++---
>  include/linux/mmc/host.h    |    1 +
>  3 files changed, 46 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
> index 6a9ad40..373d56d 100644
> --- a/drivers/mmc/core/sdio.c
> +++ b/drivers/mmc/core/sdio.c
> @@ -547,9 +547,11 @@ static void mmc_sdio_detect(struct mmc_host *host)
>        BUG_ON(!host->card);
>
>        /* Make sure card is powered before detecting it */
> -       err = pm_runtime_get_sync(&host->card->dev);
> -       if (err < 0)
> -               goto out;
> +       if (host->caps & MMC_CAP_RUNTIME_PM) {
> +               err = pm_runtime_get_sync(&host->card->dev);
> +               if (err < 0)
> +                       goto out;
> +       }
>
>        mmc_claim_host(host);
>
> @@ -570,7 +572,8 @@ out:
>        }
>
>        /* Tell PM core that we're done */
> -       pm_runtime_put(&host->card->dev);
> +       if (host->caps & MMC_CAP_RUNTIME_PM)
> +               pm_runtime_put(&host->card->dev);
>  }
>
>  /*
> @@ -720,16 +723,21 @@ int mmc_attach_sdio(struct mmc_host *host, u32 ocr)
>        card = host->card;
>
>        /*
> -        * Let runtime PM core know our card is active
> +        * Enable runtime PM only if supported by host+card+board
>         */
> -       err = pm_runtime_set_active(&card->dev);
> -       if (err)
> -               goto remove;
> +       if (host->caps & MMC_CAP_RUNTIME_PM) {
> +               /*
> +                * Let runtime PM core know our card is active
> +                */
> +               err = pm_runtime_set_active(&card->dev);
> +               if (err)
> +                       goto remo