[RFC] MMC: Proposals on reworking clock and power management

[RFC] MMC: Proposals on reworking clock and power management

[Ghorai, Sukumar]

Hi Chris, Linus, Nicolas, Adrian and all,

It has been identified after several discussions [1], [2], that
MMC/SD/SDIO driver need to re-work on clock and power management.
I am trying to list all the design considerations which are part
of this study/discussion are following:
 
1. Remove clock handling APIs currently implemented through claim_host/
   release_host

2. Ideally host-driver driver should not tell the core to shut down
   the upper layers, instead the core should decide when to
   "disabled clock or cut the power". 

   Going in details of different state handlings - Clock disable,
   Card sleep, card power cutoff;
   
   2.a) clock disable: 
     Option-1: can be handled in the host itself using a timer based
               on some idle period, OR
     Option-2: let the core do enabling/ disabling clks based on
               existing enable/ disable provided in the host, OR
     Option-3: with the *existing* API calls, namely the set_ios()
               method as proposed by Linus Walleij  [4]

   2.a) Card sleep, Power cutoff to the card:
     This should be handled in the core where based on block layer
     activity, calling corresponding API's from bus layer:
     mmc_power_save_host/ mmc_power_restore_host 

3. MMC_CAP_DISABLE - should not be a host capability.  This should be
   a core feature that should be _transparent_  to all hosts with no
   changes to any of the host drivers.  

Also it's discussed that different groups need to work together [3].

Few Patches posted by Linus Wallej in July 2009 [4], but they are
not merged yet. They would require some rework.

Therefore would like to know -
a). if the work can be taken forward from where Linus Walleij left.
b). does anyone started working on it and need to do any shearing
    to make bit faster.
c). if anything else also need to take care at re-design.

Regards,
Ghorai

	

[1] [PATCH 11/15] wireless: wl1271: introduce platform device support
    http://www.mail-archive.com/linux-omap@vger.kernel.org/msg31703.html

[2] [RFC RESEND] sdhci-s3c: support clock enable/disable (clock-gating)
    http://www.mail-archive.com/linux-mmc@vger.kernel.org/msg03532.html

[3] http://www.mail-archive.com/linux-mmc@vger.kernel.org/msg03956.html

[4] [PATCH 1/2] MMC Agressive clocking framework v5
    http://www.gossamer-threads.com/lists/linux/kernel/1106880?do=post_view_threaded#1106880

Re: [RFC] MMC: Proposals on reworking clock and power management

[Alan Cox]

> 2. Ideally host-driver driver should not tell the core to shut down
>    the upper layers, instead the core should decide when to
>    "disabled clock or cut the power". 

How will the core know ?

I actually think you are simply perpetuating the mess in the existing MMC
driver where everything is driven by a core which as a result is a twisty
maze of exceptions and flags to say "except for this" "except for that".

The core MMC code only knows when the *core* parts are no longer
requiring power. It shouldn't know about anything deeper and if you are
using the runtime PM layer the runtime PM refcounting can do all your
work.

Alan

Re: [RFC] MMC: Proposals on reworking clock and power management

[Kishore Kadiyala]

On Wed, Oct 27, 2010 at 8:02 PM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
>> 2. Ideally host-driver driver should not tell the core to shut down
>>    the upper layers, instead the core should decide when to
>>    "disabled clock or cut the power".
>
> How will the core know ?
>
> I actually think you are simply perpetuating the mess in the existing MMC
> driver where everything is driven by a core which as a result is a twisty
> maze of exceptions and flags to say "except for this" "except for that".
>
> The core MMC code only knows when the *core* parts are no longer
> requiring power. It shouldn't know about anything deeper and if you are
> using the runtime PM layer the runtime PM refcounting can do all your
> work.

Agree Alan,
Its better adapting the driver to Runtime PM instead fixing some thing
in the core.

Regards,
Kishore

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

RE: [RFC] MMC: Proposals on reworking clock and power management

[Madhusudhan]

> -----Original Message-----
> From: Alan Cox [mailto:alan@lxorguk.ukuu.org.uk]
> Sent: Wednesday, October 27, 2010 9:32 AM
> To: Ghorai, Sukumar
> Cc: Chris Ball; Linus Walleij; Nicolas Pitre; Adrian Hunter; linux-
> mmc@vger.kernel.org; Kadiyala, Kishore; Chikkature Rajashekar,
> Madhusudhan; S, Venkatraman
> Subject: Re: [RFC] MMC: Proposals on reworking clock and power management
> 
> > 2. Ideally host-driver driver should not tell the core to shut down
> >    the upper layers, instead the core should decide when to
> >    "disabled clock or cut the power".
> 
> How will the core know ?
> 

Are you saying that MMC core needs to be adapted first to RUNTIME PM?


> I actually think you are simply perpetuating the mess in the existing MMC
> driver where everything is driven by a core which as a result is a twisty
> maze of exceptions and flags to say "except for this" "except for that".
> 
> The core MMC code only knows when the *core* parts are no longer
> requiring power. It shouldn't know about anything deeper and if you are
> using the runtime PM layer the runtime PM refcounting can do all your
> work.
> 
> Alan

Re: [RFC] MMC: Proposals on reworking clock and power management

[Alan Cox]

> > > 2. Ideally host-driver driver should not tell the core to shut down
> > >    the upper layers, instead the core should decide when to
> > >    "disabled clock or cut the power".
> > 
> > How will the core know ?
> > 
> 
> Are you saying that MMC core needs to be adapted first to RUNTIME PM?

I think the runtime pm layer actually does help solve the problem -
because it allows both the driver and the core to defer power off.

To me the fundamental problem is that the MMC/SDHCI core thinks it is
all knowing, when in reality silicon designers are far too good at
producing weird, whacky, broken and otherwise bizarre implementations.

Eventually it will all break because a design comes along which is
common, and sufficiently different.

(PS: this should be discussed on the list not via some random collection
of email addresses)

Alan

Re: [RFC] MMC: Proposals on reworking clock and power management

[Nicolas Pitre]

On Wed, 27 Oct 2010, Alan Cox wrote:

> > > > 2. Ideally host-driver driver should not tell the core to shut down
> > > >    the upper layers, instead the core should decide when to
> > > >    "disabled clock or cut the power".
> > > 
> > > How will the core know ?
> > > 
> > 
> > Are you saying that MMC core needs to be adapted first to RUNTIME PM?
> 
> I think the runtime pm layer actually does help solve the problem -
> because it allows both the driver and the core to defer power off.
> 
> To me the fundamental problem is that the MMC/SDHCI core thinks it is
> all knowing, when in reality silicon designers are far too good at
> producing weird, whacky, broken and otherwise bizarre implementations.

There are 2 levels of knowledge here:

1) The host controller doesn't know and must not try to know what the 
upper layer is doing with the card.  This means that the host controller 
layer must not decide on its own to turn off the clock and/or cut power 
_to_ the card.  Emphasis again: the host controller must not policy what 
happens to the card.  Card issues and quirks are common to all 
controllers and best dealt with in the core.  Duplicating that knowledge 
in all controller drivers is silly.

2) Host controller driver may optimize power usage further according to 
its own characteristics with actions that are transparent to both the 
core code and the card.  For example, it is a damn good idea to shut 
down power to the _controller_ if power is also cut to the _card_.  Or, 
the clock to the controller block may be gated while still keeping the 
clock to the card active when no activity has happened after a certain 
delay and that is compatible with the fact that card clock is still 
active, etc.  I think tat the OMAP controller hardware is highly 
flexible and configurable in that regard.

> Eventually it will all break because a design comes along which is
> common, and sufficiently different.

There are still things that are clearly belonging to different 
abstraction layers.  And at the moment the OMAP controller driver is 
abusing those layers.

> (PS: this should be discussed on the list not via some random collection
> of email addresses)

Indeed.


Nicolas

Re: [RFC] MMC: Proposals on reworking clock and power management

[Nicolas Pitre]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1896 bytes --]

On Wed, 27 Oct 2010, Ghorai, Sukumar wrote:

> Hi Chris, Linus, Nicolas, Adrian and all,
> 
> It has been identified after several discussions [1], [2], that
> MMC/SD/SDIO driver need to re-work on clock and power management.
> I am trying to list all the design considerations which are part
> of this study/discussion are following:
>  
> 1. Remove clock handling APIs currently implemented through claim_host/
>    release_host
> 
> 2. Ideally host-driver driver should not tell the core to shut down
>    the upper layers, instead the core should decide when to
>    "disabled clock or cut the power". 
> 
>    Going in details of different state handlings - Clock disable,
>    Card sleep, card power cutoff;
>    
>    2.a) clock disable: 
>      Option-1: can be handled in the host itself using a timer based
>                on some idle period, OR

only for the clock going into the controller, not the clock going to the 
card.

>                existing enable/ disable provided in the host, OR
>      Option-3: with the *existing* API calls, namely the set_ios()
>                method as proposed by Linus Walleij  [4]

That's for the clock going to the card.  So both options 1 and 3 may 
coexist as they address different clocks.

>    2.a) Card sleep, Power cutoff to the card:
>      This should be handled in the core where based on block layer
>      activity, calling corresponding API's from bus layer:
>      mmc_power_save_host/ mmc_power_restore_host 

Same issue here.  Card power is for the core to decide, but the 
controller driver may perform opportunistic power down of the controller 
itself when it is possible and the core does not need to know about it.

> 3. MMC_CAP_DISABLE - should not be a host capability.  This should be
>    a core feature that should be _transparent_  to all hosts with no
>    changes to any of the host drivers.  

Right.



Nicolas

Re: [RFC] MMC: Proposals on reworking clock and power management

[Kishore Kadiyala]

Nicolas,

<snip >
>
>>    2.a) Card sleep, Power cutoff to the card:
>>      This should be handled in the core where based on block layer
>>      activity, calling corresponding API's from bus layer:
>>      mmc_power_save_host/ mmc_power_restore_host
>
> Same issue here.  Card power is for the core to decide, but the
> controller driver may perform opportunistic power down of the controller
> itself when it is possible and the core does not need to know about it.

I think card power decision is taken care in the core as per below patch :
https://patchwork.kernel.org/patch/226631/

And regarding "opportunistic power down of the controller" can be done by
adapting driver to PM-Runtime

<snip>

Regards,
Kishore

Re: [RFC] MMC: Proposals on reworking clock and power management

[Ohad Ben-Cohen]

On Thu, Oct 28, 2010 at 5:11 PM, Kishore Kadiyala
<kishorek.kadiyala@gmail.com> wrote:
> I think card power decision is taken care in the core as per below patch :
> https://patchwork.kernel.org/patch/226631/

Not really, this is a no-brainer patch to propagate error codes back
from those handlers.

No decision taking is going on there.

Only the card driver can decide when the card can be powered off, and
with SDIO, the function drivers are doing this today using runtime PM
API.

Re: [RFC] MMC: Proposals on reworking clock and power management

[Kishore Kadiyala]

On Thu, Oct 28, 2010 at 8:41 PM, Kishore Kadiyala
<kishorek.kadiyala@gmail.com> wrote:
> Nicolas,
>
> <snip >
>>
>>>    2.a) Card sleep, Power cutoff to the card:
>>>      This should be handled in the core where based on block layer
>>>      activity, calling corresponding API's from bus layer:
>>>      mmc_power_save_host/ mmc_power_restore_host
>>
>> Same issue here.  Card power is for the core to decide, but the
>> controller driver may perform opportunistic power down of the controller
>> itself when it is possible and the core does not need to know about it.
>
> I think card power decision is taken care in the core as per below patch :
> https://patchwork.kernel.org/patch/226631/

Actually I was looking into the series posted by
Ohad - https://patchwork.kernel.org/patch/226611/ .

Looking at the below patches, which takes care of card power off/on
once adapted to PM-Runtime
https://patchwork.kernel.org/patch/226631/
https://patchwork.kernel.org/patch/226671/

Wouldn't adapting the host driver just to PM-Runtime will answer the
above issues?

Regards,
Kishore
>
> And regarding "opportunistic power down of the controller" can be done by
> adapting driver to PM-Runtime
>
> <snip>
>
> Regards,
> Kishore
>

Re: [RFC] MMC: Proposals on reworking clock and power management

[Ohad Ben-Cohen]

On Thu, Oct 28, 2010 at 5:39 PM, Kishore Kadiyala
<kishorek.kadiyala@gmail.com> wrote:
> Wouldn't adapting the host driver just to PM-Runtime will answer the
> above issues?

It's not enough.

When the card is runtime suspended, sure, the host controller will
immediately get idle notification by runtime PM core.

But you may still want to gate the clock to the host controller on bus
inactivity, even if the card is not runtime suspended. The decision to
do this should come from the mmc core.

Re: [RFC] MMC: Proposals on reworking clock and power management

[Nicolas Pitre]

On Thu, 28 Oct 2010, Ohad Ben-Cohen wrote:

> On Thu, Oct 28, 2010 at 5:39 PM, Kishore Kadiyala
> <kishorek.kadiyala@gmail.com> wrote:
> > Wouldn't adapting the host driver just to PM-Runtime will answer the
> > above issues?
> 
> It's not enough.
> 
> When the card is runtime suspended, sure, the host controller will
> immediately get idle notification by runtime PM core.
> 
> But you may still want to gate the clock to the host controller on bus
> inactivity, even if the card is not runtime suspended. The decision to
> do this should come from the mmc core.

This is not clear that this is something that the core can effectively 
help with.  Opportunistic power saving at the host controller level is 
pretty much hardware dependent and may wildly vary in capabilities... 
and bugs.  So in this later case I think the driver for the host 
controller is the best place to just set up a timer and gate the clock 
after a certain period of inactivity for example.


Nicolas

Re: [RFC] MMC: Proposals on reworking clock and power management

[Kishore Kadiyala]

On Thu, Oct 28, 2010 at 9:43 PM, Nicolas Pitre <nico@fluxnic.net> wrote:
> On Thu, 28 Oct 2010, Ohad Ben-Cohen wrote:
>
>> On Thu, Oct 28, 2010 at 5:39 PM, Kishore Kadiyala
>> <kishorek.kadiyala@gmail.com> wrote:
>> > Wouldn't adapting the host driver just to PM-Runtime will answer the
>> > above issues?
>>
>> It's not enough.
>>
>> When the card is runtime suspended, sure, the host controller will
>> immediately get idle notification by runtime PM core.
>>
>> But you may still want to gate the clock to the host controller on bus
>> inactivity, even if the card is not runtime suspended. The decision to
>> do this should come from the mmc core.
>
> This is not clear that this is something that the core can effectively
> help with.  Opportunistic power saving at the host controller level is
> pretty much hardware dependent and may wildly vary in capabilities...
> and bugs.  So in this later case I think the driver for the host
> controller is the best place to just set up a timer and gate the clock
> after a certain period of inactivity for example.

Agree,
So briefly, calling pm_runtime_put_sync using a activity based timer
in the host Controller
driver will gate the clock to the controller .
On idle notification to PM runtime core, which will trigger a call to
the runtime_suspend hook in bus.c
[https://patchwork.kernel.org/patch/226671/ ] which will perform
powering off the Card

Regards,
Kishore

Re: [RFC] MMC: Proposals on reworking clock and power management

[Ohad Ben-Cohen]

On Thu, Oct 28, 2010 at 6:40 PM, Kishore Kadiyala
<kishorek.kadiyala@gmail.com> wrote:
> On Thu, Oct 28, 2010 at 9:43 PM, Nicolas Pitre <nico@fluxnic.net> wrote:
>> This is not clear that this is something that the core can effectively
>> help with.  Opportunistic power saving at the host controller level is
>> pretty much hardware dependent and may wildly vary in capabilities...
>> and bugs.  So in this later case I think the driver for the host
>> controller is the best place to just set up a timer and gate the clock
>> after a certain period of inactivity for example.

makes sense, thanks.

> Agree,
> So briefly, calling pm_runtime_put_sync using a activity based timer
> in the host Controller
> driver will gate the clock to the controller .

Just make sure you call pm_suspend_ignore_children(), because you want
to be able to gate that clock even when the card is still active

> On idle notification to PM runtime core, which will trigger a call to
> the runtime_suspend hook in bus.c
> [https://patchwork.kernel.org/patch/226671/ ] which will perform
> powering off the Card

Kishore, I'm not sure what you meant in the last sentence ?

Re: [RFC] MMC: Proposals on reworking clock and power management

[Roger Quadros]

On 10/28/2010 11:51 PM, ext Ohad Ben-Cohen wrote:
> On Thu, Oct 28, 2010 at 6:40 PM, Kishore Kadiyala
> <kishorek.kadiyala@gmail.com>  wrote:
>> On Thu, Oct 28, 2010 at 9:43 PM, Nicolas Pitre<nico@fluxnic.net>  wrote:
>>> This is not clear that this is something that the core can effectively
>>> help with.  Opportunistic power saving at the host controller level is
>>> pretty much hardware dependent and may wildly vary in capabilities...
>>> and bugs.  So in this later case I think the driver for the host
>>> controller is the best place to just set up a timer and gate the clock
>>> after a certain period of inactivity for example.
>
> makes sense, thanks.

just to be sure, we're talking about controller clock and not card clock. right?

>
>> Agree,
>> So briefly, calling pm_runtime_put_sync using a activity based timer
>> in the host Controller
>> driver will gate the clock to the controller .
>
> Just make sure you call pm_suspend_ignore_children(), because you want
> to be able to gate that clock even when the card is still active

Let's be specific when we say clock. Which clock?

-- 
regards,
-roger

Re: [RFC] MMC: Proposals on reworking clock and power management

[Ohad Ben-Cohen]

On Fri, Oct 29, 2010 at 12:47 PM, Roger Quadros <roger.quadros@nokia.com> wrote:
> just to be sure, we're talking about controller clock and not card clock.
> right?

yes.

>>> driver will gate the clock to the controller .
>>
>> Just make sure you call pm_suspend_ignore_children(), because you want
>> to be able to gate that clock even when the card is still active
>
> Let's be specific when we say clock. Which clock?

controller.

Re: [RFC] MMC: Proposals on reworking clock and power management

[Kishore Kadiyala]

On Fri, Oct 29, 2010 at 2:21 AM, Ohad Ben-Cohen <ohad@wizery.com> wrote:
> On Thu, Oct 28, 2010 at 6:40 PM, Kishore Kadiyala
> <kishorek.kadiyala@gmail.com> wrote:
>> On Thu, Oct 28, 2010 at 9:43 PM, Nicolas Pitre <nico@fluxnic.net> wrote:
>>> This is not clear that this is something that the core can effectively
>>> help with.  Opportunistic power saving at the host controller level is
>>> pretty much hardware dependent and may wildly vary in capabilities...
>>> and bugs.  So in this later case I think the driver for the host
>>> controller is the best place to just set up a timer and gate the clock
>>> after a certain period of inactivity for example.
>
> makes sense, thanks.
>
>> Agree,
>> So briefly, calling pm_runtime_put_sync using a activity based timer
>> in the host Controller
>> driver will gate the clock to the controller .
>
> Just make sure you call pm_suspend_ignore_children(), because you want
> to be able to gate that clock even when the card is still active

Thanks will have a look into this
>
>> On idle notification to PM runtime core, which will trigger a call to
>> the runtime_suspend hook in bus.c
>> [https://patchwork.kernel.org/patch/226671/ ] which will perform
>> powering off the Card
>
> Kishore, I'm not sure what you meant in the last sentence ?

I meant --> put_sync/get_sync [host driver]
                  --> runtime_suspend/runtime_resume
                        -->mmc_bus_suspend/resume [core.c]
                             --> card power OFF/ON [thorugh set_ios]
Regards,
Kishore

Re: [RFC] MMC: Proposals on reworking clock and power management

[Linus Walleij]

2010/10/27 Ghorai, Sukumar <s-ghorai@ti.com>:

>     Option-3: with the *existing* API calls, namely the set_ios()
>               method as proposed by Linus Walleij  [4]

OK do you want to pick this up and fix it Sukumar? I will be
happy to review and test it on our hardware and forward-port
the second patch with the MMCI hooks.

I have this patchset on my TODO list, sadly with everything
else, I may find time on a flight tonite to look at it if there is
a lot of interest.

>   2.a) Card sleep, Power cutoff to the card:
>     This should be handled in the core where based on block layer
>     activity, calling corresponding API's from bus layer:
>     mmc_power_save_host/ mmc_power_restore_host

I think this is already discussed. What needs to be done in the
core is in the first patch, the driver then knows if the core is
happy with you shutting off the clock to the card, whether you
do it or not and under what circumstances is up to the driver.

The driver can then play around with it block clock and PM
hooks and whatnot, AFAICT the core does not need to be
aware of such stuff.

> 3. MMC_CAP_DISABLE - should not be a host capability.  This should be
>   a core feature that should be _transparent_  to all hosts with no
>   changes to any of the host drivers.

In my patchset the .set_ios(0) stuff is a Kconfig option. It cannot
be on per default until we are sure that all drivers will properly
deal with a zero clock argument from .set_ios(). If you're
confident with all host drivers handling the occasional .set_ios(0)
by all means just default-activate it.

Yours,
Linus Walleij