On 11/14/2013 12:55 PM, Felipe Balbi wrote: > Hi, > > On Thu, Nov 14, 2013 at 11:05:16AM -0600, Nishanth Menon wrote: >> OMAP device hooks around suspend|resume_noirq ensures that hwmod >> devices are forced to idle using omap_device_idle/enable as part of >> the last stage of suspend activity. >> >> For a device such as i2c who uses autosuspend, it is possible to enter >> the suspend path with dev->power.runtime_status = RPM_ACTIVE. >> >> As part of the suspend flow, the generic runtime logic would increment >> it's dev->power.disable_depth to 1. This should prevent further >> pm_runtime_get_sync from succeeding once the runtime_status has been >> set to RPM_SUSPENDED. >> >> Now, as part of the suspend_noirq handler in omap_device, we force the >> following: if the device status is !suspended, we force the device >> to idle using omap_device_idle (clocks are cut etc..). This ensures >> that from a hardware perspective, the device is "suspended". However, >> runtime_status is left to be active. >> >> *if* an operation is attempted after this point to >> pm_runtime_get_sync, runtime framework depends on runtime_status to >> indicate accurately the device status, and since it sees it to be >> ACTIVE, it assumes the module is functional and returns a non-error >> value. As a result the user will see pm_runtime_get succeed, however a >> register access will crash due to the lack of clocks. >> >> To prevent this from happening, we should ensure that runtime_status >> exactly indicates the device status. As a result of this change >> any further calls to pm_runtime_get* would return -EACCES (since >> disable_depth is 1). On resume, we restore the clocks and runtime >> status exactly as we suspended with. These operations are not expected >> to fail as we update the states after the core runtime framework has >> suspended itself and restore before the core runtime framework has >> resumed. >> >> Reported-by: J Keerthy <j-keerthy@xxxxxx> >> Signed-off-by: Nishanth Menon <nm@xxxxxx> >> Acked-by: Rajendra Nayak <rnayak@xxxxxx> >> Acked-by: Kevin Hilman <khilman@xxxxxxxxxx> >> --- >> Changes in V3: >> - Added WARN in case of unexpected failure of runtime pm status restore >> v2: https://patchwork.kernel.org/patch/3176501/ >> v1: https://patchwork.kernel.org/patch/3154501/ >> >> patch baseline: V3.12 tag (also applies on linux-next next-20131107 tag) >> >> arch/arm/mach-omap2/omap_device.c | 13 +++++++++++-- >> 1 file changed, 11 insertions(+), 2 deletions(-) >> >> diff --git a/arch/arm/mach-omap2/omap_device.c b/arch/arm/mach-omap2/omap_device.c >> index b69dd9a..53f0735 100644 >> --- a/arch/arm/mach-omap2/omap_device.c >> +++ b/arch/arm/mach-omap2/omap_device.c >> @@ -621,6 +621,7 @@ static int _od_suspend_noirq(struct device *dev) >> >> if (!ret && !pm_runtime_status_suspended(dev)) { >> if (pm_generic_runtime_suspend(dev) == 0) { >> + pm_runtime_set_suspended(dev); >> omap_device_idle(pdev); >> od->flags |= OMAP_DEVICE_SUSPENDED; >> } >> @@ -634,10 +635,18 @@ static int _od_resume_noirq(struct device *dev) >> struct platform_device *pdev = to_platform_device(dev); >> struct omap_device *od = to_omap_device(pdev); >> >> - if ((od->flags & OMAP_DEVICE_SUSPENDED) && >> - !pm_runtime_status_suspended(dev)) { >> + if (od->flags & OMAP_DEVICE_SUSPENDED) { >> od->flags &= ~OMAP_DEVICE_SUSPENDED; >> omap_device_enable(pdev); >> + /* >> + * XXX: we run before core runtime pm has resumed itself. At >> + * this point in time, we just restore the runtime pm state and >> + * considering symmetric operations in resume, we donot expect >> + * to fail. If we failed, something changed in core runtime_pm >> + * framework OR some device driver messed things up, hence, WARN >> + */ >> + WARN(pm_runtime_set_active(dev), >> + "Could not set %s runtime state active\n", dev_name(dev)); > > if you want to print the device name, how about dev_WARN() ? > > no strong feelings though: I would like to stick with WARN as dev_WARN would probably need an condition check.. unless someone has a strong opinion, I dont see it adding value here. > > Reviewed-by: Felipe Balbi <balbi@xxxxxx> > -- Regards, Nishanth Menon -- 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
