Hi Paul, >>> _enable_wakeup() and _disable_wakeup() are expected to program the >>> OCP_SYSCONFIG.ENAWAKEUP bit. >> >> These functions were originally intended to take care of everything needed >> for the IP block to wake up the chip, including the PRCM WKEN programming. >> ENAWAKEUP is simply one part of that. >> >>> Get rid of the additional sidle/mstandby programming in them, as its >>> confusing (this is expected to be handled elsewhere as part of >>> _enable_sysc()/__idle_sysc()) >> >> Sorry, why does the expectation exist for the code to enable and disable >> device wakeup to be part of _enable_sysc()/_idle_sysc(), rather than in >> functions called by _enable_sysc()/_idle_sysc()? > > It all comes down to if SIDLE_SMART_WKUP/MSTANDBY_SMART_WKUP programming > be considered as 'idlemode' programming or 'enwakeup' programming. > If you consider these are being part of 'enwakeup' progrmming, these should > certainly be handled as part of _enable_wakeup() and _disable_wakeup(). > > Today, in some cases, these are *also* handled as part of _enable_sysc() > and _idle_sysc(). The way _enable_wakeup() is invoked from _enable_sysc() > is also very inconsistent. For instance, for any IP which supports > SYSC_HAS_MIDLEMODE and SYSC_HAS_ENAWAKEUP, we invoke _enable_wakeup() > regardless of the MIDLEMODE programmmed. > While in case of the IP supporting SYSC_HAS_SIDLEMODE, _enable_wakeup() is > invoked only when the SIDLEMODE programmed is SMART or SMART_WKUP. > > I understand the cleanups you are suggesting below as part of the movement > of some of these things outside of mach-omap2. > I was more looking at fixing the existing piece so its readable and does > things more consistently. Do you have any further thoughts on how we should do about this? regards, Rajendra > > regards, > Rajendra > >> >>> and unnecessary. >> >> So here's part of the reason why the module wakeup control functions >> should remain separate. When the kernel boots, all the PM features should >> be disabled. Then mach-omap2/pm*.c should enables PM features where >> they're needed. >> >> Right now, mach-omap2/pm34xx.c sets module WKEN bits. (These direct >> register accesses need to be moved as part of the cleanup work, of moving >> the PM/PRM/CM code into drivers.) But the list of IP blocks that >> should be allowed to wake the system is board-dependent. >> >> So really, what mach-omap2/pm34xx.c should do is to call into the hwmod >> enable-wakeups code to enable MPU wakeups for all the IP blocks that have >> some DT property set, something like 'enable-wakeups'. Then the hwmod >> code should ensure that the PRM wakeup-enable and GRPSEL bits are >> programmed (by calling into the PRM driver code) and should then either >> set the ENAWAKEUP bits or put the module into smart_wkup MSTANDBY/MIDLE. >> >> Similarly, when the PM driver is unloaded, it should set no-idle on all >> the IP blocks that were marked as wakeup-capable and disable the PRCM >> wakeup control bits, by calling some hwmod disable-wakeups code. >> >>> Well, the _enable_sysc()/_idle_sysc() handled only the mstandby modes >>> correctly. So fix them so they also handle the midle modes correctly >> >> If there's a fix here, please split that out into a separate patch. >> >> >> - Paul >> > -- 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
