Jon Hunter [off-list ref] writes:

Hi Benoit,

On 05/08/2012 06:01 AM, Cousson, Benoit wrote:

[...]

quoted

quoted

quoted

quoted

P.S. Please note there is also already a different fix in mainline for
the EMU clkdm data from Paul which adds the force wakeup flag and
removes the DISABLE_AUTO flag[1] (but leaves the ENABLE_AUTO flag,
because the hardware is capable.)

Hmmm ... yes saw this, and you will have to excuse me as I don't fully
follow the logic here. In fact, I am thinking we want the opposite ;-)

 From looking, into this it seems to me that when PMU is running we want
the EMU clock domain in software-wakeup state and when PMU is not
running we want in the hardware auto state.

So far, I'm with you.

quoted

By keeping the ENABLE_AUTO flag set, as soon as we enable the clock
domain it is put right back into the HW_AUTO state

This is only because it was in the HWSUP state when _enable was called.
If clkdm_deny_idle() is used, that behavior will change.

quoted

and hence PMU is
not working (see _enable() function in
arch/arm/mach-omap2/omap_hwmod.c)

So really what I think we want is to remove the ENABLE_AUTO flag to keep
the clock domain in software wake-up and use the DISABLE_AUTO flag to
put the clock domain back in HW_AUTO (note this requires a patch to
perform this 2nd part).

Well, Paul will have to comment here for the final word, but IIUC, the
hwmod flags are supposed to indicate only what the HW is capable of.  If
we want to change the runtime behavior, we nee to use (or add) APIs to
change the beahvior.  In this case, clkdm_allow_idle(),
clkdm_deny_idle() are probably what is needed here.

Yes, indeed, we should not hack the flags to fix that kind of issue. The
flags describe what the HW is capable of, and the EMU CD can support
HW_AUTO and SW_WAKEUP. AFAIK, the issue with that EMU CD is that the
only valid next power state is OFF, meaning that no retention mode is
supported. So any transition to idle will go to OFF and lead to a reset
upon wakeup.

No hacking intended here, just getting the flags correct ;-)

So let me start from the beginning ...

1. I agree that for the EMU CD that the valid HW states are HW_AUTO and
SW_WKUP.

2. When the EMU CD is active (due to something like PMU), we want to
keep the CD in the SW_WKUP state, otherwise we can automatically
transition to idle and reset the IP (at least for omap4430).

3. When the EMU CD is inactive, we want to keep the CD in the HW_AUTO
state because SW_SLEEP is NOT supported.

In the current code, we have the CLKDM_CAN_DISABLE_AUTO flag disabled
and the CLKDM_CAN_ENABLE_AUTO flag enabled. If CLKDM_CAN_ENABLE_AUTO is
set then the omap_pm_clkdms_setup() function will place the CD into
HW_AUTO regardless of CLKDM_CAN_DISABLE_AUTO, and the next time the
hwmod _enable() is called it is in the HW_AUTO state and so it is
allowed to idle. This is not what we want. Do you agree?

If I set CLKDM_CAN_DISABLE_AUTO flag and disable CLKDM_CAN_ENABLE_AUTO,
then I do not have the above problem.

To be honest, with you the more I look and test the code, the more
confused I am by the definition of the CLKDM_CAN_HWSUP ...

#define CLKDM_CAN_HWSUP	(CLKDM_CAN_ENABLE_AUTO | CLKDM_CAN_DISABLE_AUTO)

When I look at where these flags are used, I see that
CLKDM_CAN_ENABLE_AUTO is used in clkdm_allow_idle and
CLKDM_CAN_DISABLE_AUTO is used in clkdm_deny_idle. So this implies that ...

CLKDM_CAN_ENABLE_AUTO = Supports HW_AUTO state when CD is active
CLKDM_CAN_DISABLE_AUTO = Does NOT supports HW_AUTO state when CD is active

Are the above the correct definitions? 

Not quite. 

These flags describe the capabilities as defined in CLKTRCTRL field of
the CLKSTCTRL register (e.g. CM_EMU_CLKSTCTRL)

CLKDM_CAN_ENABLE_AUTO: IP supports HW_AUTO state (and it can be enabled)
CLKDM_CAN_DISABLE_AUTO: HW_AUTO feature can be disabled (a.k.a. NO_SLEEP)

Note that in OMAP4, the latter called NO_SLEEP in the TRM, but in OMAP3
it's described as "The automatic hardware-supervised mode is disabled"

What is confusing to me is that the OMAP4 TRM doesn't list the NO_SLEEP
mode as supported by the EMU.   It seems to me that if the IP supports
HW_AUTO, it should be able to be enabled *and* disabled. 

Maybe Paul/Benoit can clarify.

Kevin