| Message ID | CAPDyKFras1o13WBZzaZ_Snnj5TBQQFWKr=PtCjUvwe+yGPQn9w@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 2 Feb 2016, Ulf Hansson wrote: > That's not what we want. So I moved the checks to the actual > ->runtime_resume() callback in the PM domain, instead of in the > omap_device_enable() function. A version 2 is attached. Please give it > at try. > > [...] > > From: Ulf Hansson <ulf.hansson@linaro.org> > Date: Tue, 2 Feb 2016 10:05:39 +0100 > Subject: [PATCH V2] ARM: OMAP2+: omap-device: Allow devices to be pre-enabled > > Commit 5de85b9d57ab ("PM / runtime: Re-init runtime PM states at probe > error and driver unbind") may re-initialize the runtime PM status of the > device to RPM_SUSPENDED at driver probe failures. > > For the omap_hsmmc and likely also other omap drivers, which needs more > than one attempt to ->probe() (returning -EPROBE_DEFER), this commit > causes a regression at the PM domain level (omap hwmod). > > The reason is that the drivers don't put back the device into low power > state while bailing out in ->probe to return -EPROBE_DEFER. This leads to > that pm_runtime_reinit() in driver core, is re-initializing the runtime PM > status from RPM_ACTIVE to RPM_SUSPENDED. But isn't that okay? When the system starts up, the PM core doesn't know the actual state of any device. It just marks each device as RPM_SUSPENDED and disabled as it is registered. It's the job of subsystems and drivers to make sure the stored state agrees with the actual physical state. So why shouldn't pm_runtime_reinit() leave the structure the same as it would be if the device had just been registered? > The next ->probe() attempt then triggers the ->runtime_resume() callback > to be invoked, which means this happens two times in a row. At the PM > domain level (omap hwmod) this is being treated as an error and thus the > runtime PM status of the device isn't correctly synchronized with the > runtime PM core. Why doesn't the same problem arise the first time the device is probed? Or to put it another, why should a re-probe be any different from the initial probe? > In the end, ->probe() anyway succeeds (as the driver don't checks the > error code from the runtime PM APIs), but results in that the PM domain > always stays powered on. This because of the runtime PM core believes the > device is RPM_SUSPENDED. > > Fix this regression by allowing devices to be pre-enabled when the PM > domain's (omap hwmod) ->runtime_resume() callback is requested to enable > the device. In such cases, return zero to synchronize the runtime PM > status with the runtime PM core. Is this really the right way to fix the problem? Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/mach-omap2/omap_device.c b/arch/arm/mach-omap2/omap_device.c index 0437537..1ad390b 100644 --- a/arch/arm/mach-omap2/omap_device.c +++ b/arch/arm/mach-omap2/omap_device.c @@ -599,8 +599,17 @@ static int _od_runtime_suspend(struct device *dev) static int _od_runtime_resume(struct device *dev) { struct platform_device *pdev = to_platform_device(dev); + struct omap_device *od = to_omap_device(pdev); int ret; + /* + * From the PM domain perspective the device may already be enabled. + * In such case, let's return zero to synchronize the state with the + * runtime PM core. + */ + if (od->_state == OMAP_DEVICE_STATE_ENABLED) + return 0; + ret = omap_device_enable(pdev); if (ret) return ret;