Re: [PATCH] arm: omap2plus: unidle devices which are about to probe

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07/11/2013 01:16 PM, Felipe Balbi wrote:
in order to make HWMOD and pm_runtime agree on the
initial state of the device, we will unidle the device
and call pm_runtime_set_active() to tell pm_runtime
that the device is really active.

By the time driver's probe() is reached, a call to
pm_runtime_get_sync() will not cause driver's
->runtime_resume() method to be called at first, only
after a successful ->runtime_suspend().

Note that we must prevent pm_runtime transitions while
driver is probing otherwise drivers would be suspended
as soon as they call pm_runtime_use_autosuspend(). By
calling pm_runtime_forbid() before probe() and
pm_runtime_allow() after probe() we 'fix' that detail.

Note that this patch was inspired by PCI's pci_pm_init().

NAK. This is a hack.
In addition to what I've mentioned in
http://www.spinics.net/lists/arm-kernel/msg258061.html there are following issues:
1) this patch disables call to PM runtime callbacks for all
OMAP drivers which is wrong - I've found, for example, that
omap-usb-host.c driver enables TLL in some configurations in its
.runtime_resume():

usbhs_runtime_resume()
|-omap_tll_enable()


2) even with this fix the restore context issue will not be fixed for *non* console UARTs. Just try:
#echo 0xDEAD > dev/ttyO3 // checked on OMAP4 SDP

3) I've checked most of OMAP drivers and all of them solve such kind of problem internally (SPI, MMC, I2C, etc.)

4) See inline

Signed-off-by: Felipe Balbi <balbi@xxxxxx>
---

boot tested on top of today's Linus master
6d128e1e72bf082542e85f72e6b7ddd704193588 with OMAP4
panda. Reached console prompt and, after setting a
proper autosuspend delay, consoles autosuspend just
fine.

It needs to be tested on other platforms.

ps: note that we also call pm_runtime_set_suspended(dev)
from our late_initcall() to disable devices so that pm_runtime
and HWMOD continue to aggree on device's state.

  arch/arm/mach-omap2/omap_device.c | 44 +++++++++++++++++++++++++++++++++++----
  1 file changed, 40 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_device.c b/arch/arm/mach-omap2/omap_device.c
index 5cc9287..cb1fc1d 100644
--- a/arch/arm/mach-omap2/omap_device.c
+++ b/arch/arm/mach-omap2/omap_device.c
@@ -178,6 +178,26 @@ odbfd_exit:
  	return ret;
  }

+static void omap_device_pm_init(struct platform_device *pdev)
+{
+	omap_device_enable(pdev);
+	pm_runtime_forbid(&pdev->dev);
It's wrong to use pm_runtime_forbid() - pm_runtime_get_noresume() should be used instead.
pm_runtime_forbid()
|-rpm_resume()


+	pm_runtime_set_active(&pdev->dev);
+	device_enable_async_suspend(&pdev->dev);
why did you use device_enable_async_suspend() - it would enable async suspend for devices - this is not related to the current issue and topic for discussion (currently is used by usb/scsi/pci/ata drivers only).
+}
+
+static void omap_device_pm_allow(struct platform_device *pdev)
+{
+	pm_runtime_allow(&pdev->dev);
+}
+
+static void omap_device_pm_exit(struct platform_device *pdev)
+{
+	device_disable_async_suspend(&pdev->dev);
+	pm_runtime_set_suspended(&pdev->dev);
+	omap_device_idle(pdev);
+}
+
  static int _omap_device_notifier_call(struct notifier_block *nb,
  				      unsigned long event, void *dev)
  {
@@ -189,16 +209,31 @@ static int _omap_device_notifier_call(struct notifier_block *nb,
  		if (pdev->archdata.od)
  			omap_device_delete(pdev->archdata.od);
  		break;
+	case BUS_NOTIFY_BIND_DRIVER:
+		if (pdev->archdata.od)
+			omap_device_pm_init(pdev);
+		break;
+	case BUS_NOTIFY_BOUND_DRIVER:
+		if (pdev->archdata.od)
+			omap_device_pm_allow(pdev);
+		break;
+	case BUS_NOTIFY_UNBOUND_DRIVER:
+		if (pdev->archdata.od)
+			omap_device_pm_exit(pdev);
+		break;
  	case BUS_NOTIFY_ADD_DEVICE:
  		if (pdev->dev.of_node)
  			omap_device_build_from_dt(pdev);
-		/* fall through */
+		break;
  	default:
-		od = to_omap_device(pdev);
-		if (od)
-			od->_driver_status = event;
+		/* nothing */
+		break;
  	}

+	od = to_omap_device(pdev);
+	if (od)
+		od->_driver_status = event;
+
  	return NOTIFY_DONE;
  }

@@ -855,6 +890,7 @@ static int __init omap_device_late_idle(struct device *dev, void *data)
  			dev_warn(dev, "%s: enabled but no driver.  Idling\n",
  				 __func__);
  			omap_device_idle(pdev);
+			pm_runtime_set_suspended(dev);
  		}
  	}



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




[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux