[RFC v2 16/18] ARM: OMAP2+: AM33XX: Basic suspend resume support

Bedia, Vaibhav vaibhav.bedia at ti.com
Wed Feb 20 04:21:41 EST 2013

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?

> > 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?


More information about the linux-arm-kernel mailing list