On 07/18/2013 11:14 AM, Tony Lindgren wrote:

* Grygorii Strashko [off-list ref] [130717 10:11]:

quoted

On 07/17/2013 06:38 PM, Tony Lindgren wrote:

quoted

* Grygorii Strashko [off-list ref] [130717 04:49]:

quoted

Before switching to DT pinctrl states of OMAP IPs have been handled by hwmod
framework. After switching to DT-boot the pinctrl handling was dropped from
hwmod framework and, as it was recommended, OMAP IP's drivers have to be updated
to handle pinctrl states by itself using pinctrl_pm_select_xx() helpers
(see http://lists.infradead.org/pipermail/linux-arm-kernel/2013-June/173514.html)

But this is not right for OMAP2+ SoC where real IPs state is controlled
by omap_device core which enables/disables modules & clocks actually.

I'm not convinced we should try to handle this in a generic way
as only some devices need dynamic remuxing of some pins.

quoted

For example, if OMAP I2C driver will handle pinctrl state during system wide
suspend the following issue may occure:
- suspend_noirq - I2C device can be still active because of PM auto-suspend
   |-_od_suspend_noirq
      |- omap_i2c_suspend_noirq
         |- PINs state set to SLEEP
   |- pm_generic_runtime_suspend
      |- omap_i2c_runtime_suspend()
         |- PINs state set to IDLE  <--- *oops* PINs state is IDLE and not SLEEP
   |- omap_device_idle()
      |- omap_hwmod_idle()
         |- _idle()
            |- disbale module (sysc&clocks)

Above call sequence explains what's going on when .suspend_noirq() is 
called - .suspend_noirq() handler ==> _od_suspend_noirq() and it's set 
for all OMAP2+ devices.

quoted

quoted

And in this example you are assuming that you need separate idle and
sleep states, which is not true at least for most cases I've seen.

I don't need both states (at least right now) :), but
- if any OMAP2+ driver will have two states defined: "idle" and "sleep"
- and if it will try to manage them from drivers callbacks only using
   pure calls to pinctrl_pm_select_xx() helpers

the "idle" state will be selected during suspend and *not* "sleep".

But the drivers have separate calls for runtime PM and suspend/resume?

Yes, but for OMAP2+ everything is handled by OMAP device framework :)

quoted

quoted

It is possible that am33xx needs separate idle and sleep states, but
most likely only for some pins. For omap[345] we can get away with
just the default state for most cases.

In case, if only "default" state is defined for device - nothing will
  be done by OMAP device framework for it (I mean any call to
pinctrl_pm_select_xx() will do nothing - it just checks that there is
no state and returns 0).

If we want to automate something, it should be done at Linux generic
level rather than at omap "bus" level. That sort of relates to drivers
needing to know when they've lost context too, which should be
implemented in Linux generic way.

Regards,

Tony