On Wed, Feb 04, 2015 at 10:56:00AM +0100, Geert Uytterhoeven wrote: > >>> I can't find that sh_dmae_ctl_stop() is invoked from the runtime PM > >>> suspend callback. That means the device will be "removed" differently, > >>> depending on it's runtime PM status (due to the upper check for > >>> pm_runtime_suspended() ) . Is that really what you want? > >> I think the patch description is the key. "During system reboot, the > >> sh-dma-engine device may be runtime-suspended, causing a crash" > >> > >> The runtime-suspended device is already idle and has removed its clock. > > > > That's not my point. The device will be shutdown differently, > > depending if it's runtime PM suspended or not. > > You're right. > > > So, if it's only about gating clocks then why do even bother invoking > > sh_dmae_ctl_stop() in this path. > > sh_dmae_ctl_stop() stops the DMA controller. But this is the "master stop". > At this time, the individual channels should have been stopped by dmae_halt(). > > So you prefer my V1 patch instead, which unconditionally runtime-resumed > the device, so sh_dmae_ctl_stop() can be called? > http://www.spinics.net/lists/dmaengine/msg02954.html That forces device to resume when we are doinga shutdown... > > Alternatively... > > Both sh_dmae_resume() and sh_dmae_runtime_resume() call sh_dmae_rst(), > which re-initializes the DMAOR register. > To make it symmetric, we can move the call to sh_dmae_ctl_stop() to > sh_dmae_suspend() and sh_dmae_runtime_suspend() instead? I think thats a btter idea.. -- ~Vinod -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
