On Tuesday 23 April 2013 01:49 PM, Paul Walmsley wrote: > Hi Rajendra, > > On Thu, 18 Apr 2013, Rajendra Nayak wrote: > >>>>> _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? > > Is it possible to implement a solution that preserves _enable_wakeup() and > _disable_wakeup() as distinct functions, that can be called by separate > wakeup control entry points? > > If it makes sense to change _enable_sysc() so that it doesn't call > _enable_wakeup(), but does similar work itself, that's fine with me, as > long as there's not too much code duplication. > > It will be good to have the inconsistencies fixed. Sure, I'll post something on those lines shortly. regards, Rajendra > > > - 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
