On Thu, Feb 14, 2013 at 02:22:47PM -0800, Tony Lindgren wrote: > * Paul Walmsley <paul@xxxxxxxxx> [130214 12:51]: > > Hi, > > > > On Thu, 14 Feb 2013, Tony Lindgren wrote: > > > > > I don't think so as hwmod should only touch the sysconfig space > > > when no driver has claimed it. > > > > hwmod does need to touch the SYSCONFIG register during pm_runtime suspend > > and resume operations, and during device enable and disable operations. > > Here's the relevant code: > > > > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=arch/arm/mach-omap2/omap_hwmod.c;h=4653efb87a2721ea20a9fe06a30a6c204d6d2282;hb=HEAD#l2157 > > > > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=arch/arm/mach-omap2/omap_hwmod.c;h=4653efb87a2721ea20a9fe06a30a6c204d6d2282;hb=HEAD#l2195 > > But that's triggered by runtime PM from the device driver, right? > > I think it's fine for hwmod to do that as requested by the device > driver via runtime PM since hwmod is the bus glue making the drivers arch > independent. > > I think the only piece we're missing for that is for driver to run > something like pm_runtime_init() that initializes the address space > to hwmod. Or just bus specific ioremap like you're suggesting later > on. > > Just to recap what we've discussed earlier, the reasons why we want > reset and idle functions should be in the driver specific header are: > > 1. There's often driver specific logic needed in addition to the > syconfig tinkering in the reset/idle functions. how about introducing generic device_reset() and device_idle() hooks which driver can implement and can be called by bus glue ? Something like: diff --git a/include/linux/pm.h b/include/linux/pm.h index 03d7bb1..9c921e2 100644 --- a/include/linux/pm.h +++ b/include/linux/pm.h @@ -256,6 +256,8 @@ typedef struct pm_message { * these conditions and handle the device as appropriate, possibly queueing * a suspend request for it. The return value is ignored by the PM core. * + * @runtime_reset: Resets the device and puts it in a known state. + * * Refer to Documentation/power/runtime_pm.txt for more information about the * role of the above callbacks in device runtime power management. * @@ -285,6 +287,7 @@ struct dev_pm_ops { int (*runtime_suspend)(struct device *dev); int (*runtime_resume)(struct device *dev); int (*runtime_idle)(struct device *dev); + int (*runtime_reset)(struct device *dev); }; #ifdef CONFIG_PM_SLEEP We already have ->runtime_idle() which we could be using... > 2. We need to be able to reset and idle some hardware even if the > driver is not compiled in. yeah, this will be tricky. Even if you try late_initcall() there could still be issues with -EPROBE_DEFER. Maybe you don't need to reset the IPs but rather put those which have status = "disabled" to FORCE_IDLE. Wouldn't that be enough ? If a driver claims the device later, then pm_runtime_enable() + pm_runtime_get() will change that. -- balbi
Attachment:
signature.asc
Description: Digital signature
