On Mon, Feb 18, 2013 at 21:41:49, Kevin Hilman wrote: [...] > > By default these IPs don't have MSTANDBY asserted. > > When you say "by default", I guess you mean after reset (and/or context > loss), right? > Yes > > When a low power transition happens, the peripheral power domain loses > > context and hence the forced MSTANDBY configuration in the IP is > > lost. To work around this problem we need to assert MSTANDBY in every > > suspend-resume iteration. > > Yuck. More clever hardware. ;) We are getting this gradually :) > > > I agree that this might hide PM bugs in the driver but to solve this problem we > > need some mechanism for the PM code to know whether or not a driver is bound > > to the corresponding platform devices. Any suggestions on this? > > Driver bound status can be tracked easily using bus notifiers. You can > see an example in the omap_device core. Ok. I'll try to use the driver bound status in the next version. [...] > >> > + /* > >> > + * GFX_L4LS clock domain needs to be woken up to > >> > + * ensure thet L4LS clock domain does not get stuck in transition > >> > + * If that happens L3 module does not get disabled, thereby leading > >> > + * to PER power domain transition failing > >> > + * > >> > + * The clock framework should take care of ensuring > >> > + * that the clock domain is in the right state when > >> > + * GFX driver is active. > >> > >> Are you suggesting that the clock framework is not doing this already? > >> > > > > No. This clkdm_*() calls are here to work-around an issue that I observed > > when implementing suspend-resume. The force wakeup and sleep of the gfx_l4ls > > clock domain across every suspend-resume is something I don't think can > > be pushed to the clock framework. > > I still don't follow what you're suggesting the clock framework "should" > do. Are you describing the case when there is a GFX driver vs. when > there isn't a driver? If so, it needs to be more clear. > No. The issue with GFX_L4LS happens irrespective of the state of the GFX driver and needs to be handled in the suspend-resume sequence. I guess the second part of comment is what created the confusion. I'll get rid of that. > Also, some more description about why the device gets 'stuck in > transition' would be helpful. Is this an erratum workaround? > I'll follow up with the design folks to find out more. From some past discussions this is not expected so looks like we need to an erratum published for this issue. > > I see, then probably a TODO here with more description would be more > helpful. > > So, IIUC, without implemeting this, the drivers will never be able to > detect loss of context, correct? Sounds like something that should be > decribed in the changelog as that's a rather important limitation to > this implementaion. > Ok. I'll address this limitation in the next version and improve the changelog. > >> > + > >> > + /* Give some time to M3 to respond. 500msec is a random value here */ > >> > >> random? really? > > > > Sort of. I don't have any numbers from the h/w guys on the worst > > case delay in getting an interrupt from M3 to MPU. At the same time > > I want to handle the scenario where something goes wrong on the M3 > > side and it doesn't respond. > > OK, then it's not random. You have some reasoning behind the number > that should be documented. Will do. > > That being said, in the absence of numbers from HW folks, can't you > measure the typical times so you know roughly what's "normal". I'll do some timer based profiling and get rough numbers for this. [...] > >> > >> Why not omap_device_enable(pdev) ? > >> > > > > The objective is to leverage the hwmod code to get the WKUP-M3 > > functional and not have OMAP runtime PM code coming in the way. > > FWIW, it is not runtime PM getting in the way. > > > Using omap_device_enable() triggers the following dev_warn() > > from omap_device_enable(). > > Looking closer at the trace, you'll see it's not omap_device_enable() > that is triggering this warning. What is happening is that omap_device > has a late_initcall which forcibly idles omap_devices that have been > enabled, but that don't have a driver. You're hacking around that. > Thanks for the explanation. I should have looked closer :( > IMO, this would be a much cleaner implementation if you just created a > small driver for the wkup_m3. You're already doing a bunch of > driver-like stuff for it (requesting base/IRQs, mapping regions, > firmware, etc.) I think this should separated out into a real driver. > Then it will be bound to the right omap_device, and normal PM operations > will work as expected. > > Also, doing it this way will be more flexible for those wanting to use > their own firmware on the M3, or customize the current firmware. Hmm... that definitely sounds more flexible. It should also help in the next SoC AM437x which has a similar PM architecture. Where would you suggest placing this minimal driver? Regards, Vaibhav -- 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
