On Friday 07 August 2009, Alan Stern wrote: > On Sat, 8 Aug 2009, Magnus Damm wrote: > > > On Fri, Aug 7, 2009 at 11:32 PM, Alan Stern<stern@xxxxxxxxxxxxxxxxxxx> wrote: > > > On Fri, 7 Aug 2009, Magnus Damm wrote: > > > > > >> PM: Runtime PM v13 for Platform Devices 20090807 > > >> > > >> [PATCH 01/05] PM: Runtime PM v13 - add dev_pm_ops helpers > > > > > > This patch doesn't do anything much, besides reverting a change I asked > > > Rafael to make. I don't see how it helps platform-specific code do > > > anything. > > > > It's helping different bus types to implement the runtime part of the > > dev_pm_ops in a consistent way. I suggest that all bus types should > > return -ENOSYS if the callback is missing. > > Doesn't Rafael's code already do this? > > > And they can do so by using > > the helper functions. The change is not platform specific, but my > > latest SuperH platform Runtime PM prototype makes use of it. > > > > The latest SuperH specific Runtime PM implementation require > > dev_pm_ops even though there is no work to be done for the driver. The > > code works in a sort of opt-in way at this point, so callbacks are > > explicitly required. I'd like us to standardize on this behaviour if > > possible, so runtime pm enabled platform drivers can be shared between > > different platform bus implementations. So my LCDC platform driver > > will work fine on both SuperH SoCs and ARM SoCs. > > I still don't see the connection. Why are helper functions useful? > > > Sorry for reverting your change, but I couldn't see any clear benefit > > with the v11->v13 lock-drop-inside-the-if-case change. Isn't it just > > avoiding dropping the lock in the uncommon error case? Maybe I'm > > misunderstanding. > > Basically you are right, except for one thing: The error case might not > be so uncommon. That's the benefit. > > Rafael, along these lines, I suspect we might not want to go into an > error state if a runtime suspend fails because there is no callback > function. Returning -ENOSYS to the caller is fine, but leave > runtime_error set to 0. Maybe do the same for runtime resume. Yes, I've just made this change. > > >> [PATCH 02/05] PM: Runtime PM v13 - let bus-less devices succeed > > > > > > This could be added without 01/05. But why do you want it? Busless > > > devices don't have PM runtime callbacks, so whether the core thinks the > > > callbacks succeed or not doesn't make any difference. > > > > > > You say that "Runtime suspend and resume of devices on the platform bus > > > is impossible without this change", but you don't explain why -- or why > > > the patch makes runtime suspend and resume of these devices possible. > > > > Right now, in the standard upstream kernel all platform devices get > > assigned a parent device unless one exists are registration time. The > > shared parent device is parent-less. Since the Runtime PM code resumes > > the parent before the child, the resume operation will fail because > > there is no dev_pm_ops for the shared parent. > > Not if the parent is disabled for runtime PM, which it is in this case, > right? > > > So I wonder which way that is the best to allow resuming platform > > devices. Patch [02/05] is one way, but maybe there are more elegant > > ways to handle it? Should the platform code be modified instead? If > > so, how? I suppose root hubs for USB may have a similar issue, no? > > If necessary, I would suggest adding appropriate dummy runtime PM > routines for that catch-all parent device. But it may not be > necessary. > > > > >> [PATCH 03/05] PM: Runtime PM v13 - add debug printouts > > > > > > This looks good. > > > > Thanks. Maybe it's a good plan to add similar printouts to other > > functions as well? > > Perhaps so. When writing similar code for USB I found that lots of > debugging printouts were needed all over the place, to get everything > working right. Once it was working, they were a nuisance. I've added such messages to __pm_runtime_idle(), for consistency. Thanks, Rafael _______________________________________________ linux-pm mailing list linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/linux-pm
