On Mon, Oct 13, 2014 at 11:13:40AM +0200, Sebastian Andrzej Siewior wrote: > On 2014-09-15 09:39:09 [-0500], Felipe Balbi wrote: > > if we don't make sure to kill the timer, it could > > expire after we have already gated our clocks. > > > > That will trigger a Data Abort exception because > > we would try to access register while clock is gated. > > That timer deserves to be killed because nobody wants it to wakeup the > system from suspend. However the Data Abort wouldn't happen if the timer > would use pm_runtime_get_sync() right? correct, but we want to suspend ;-) there is a race here: mod_timer(); /* before mod_timer() expires */ "echo mem > /sys/power/suspend" ->runtime_suspend() /* clocks are now gated */ /* mod_timer() expires and BOOM! */ > > --- a/drivers/usb/musb/musb_dsps.c > > +++ b/drivers/usb/musb/musb_dsps.c > > @@ -870,6 +870,7 @@ static int dsps_suspend(struct device *dev) > > struct musb *musb = platform_get_drvdata(glue->musb); > > void __iomem *mbase = musb->ctrl_base; > > > > + del_timer_sync(&glue->timer); > > glue->context.control = dsps_readl(mbase, wrp->control); > > glue->context.epintr = dsps_readl(mbase, wrp->epintr_set); > > glue->context.coreintr = dsps_readl(mbase, wrp->coreintr_set); > > @@ -895,6 +896,7 @@ static int dsps_resume(struct device *dev) > > dsps_writel(mbase, wrp->mode, glue->context.mode); > > dsps_writel(mbase, wrp->tx_mode, glue->context.tx_mode); > > dsps_writel(mbase, wrp->rx_mode, glue->context.rx_mode); > > + setup_timer(&glue->timer, otg_timer, (unsigned long) musb); > > You need a mod_timer() here instead. I will send a patch in a few. yeah, I was confused if it should be setup_timer() or mod_timer(). Since I deleted the timer on suspend, I thought setup_timer() was more appropriate, no ? -- balbi
Attachment:
signature.asc
Description: Digital signature
