Jon Hunter <jon-hunter@xxxxxx> writes: > Hi Benoit, > > On 05/08/2012 06:01 AM, Cousson, Benoit wrote: > > [...] > >>>>> 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. >>> >>>> 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. >>> >>>> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
