| Message ID | 20090527100642.29671.52231.sendpatchset@rx1.opensource.se (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
On Wednesday 27 May 2009, Magnus Damm wrote: > From: Magnus Damm <damm@igel.co.jp> > > Add platform_device_idle() and platform_device_wakeup() > and allow architectures to implement their own versions > of these if CONFIG_HAVE_PLATFORM_IDLE_WAKEUP is set. > > Signed-off-by: Magnus Damm <damm@igel.co.jp> > --- > > Yes, this needs documentation. See [00/04] for now. > > arch/Kconfig | 3 +++ > include/linux/platform_device.h | 8 ++++++++ > 2 files changed, 11 insertions(+) > > --- 0002/arch/Kconfig > +++ work/arch/Kconfig 2009-05-26 12:31:06.000000000 +0900 > @@ -115,3 +115,6 @@ config HAVE_DEFAULT_NO_SPIN_MUTEXES > > config HAVE_PLATFORM_DEVICE_ARCHDATA > bool > + > +config HAVE_PLATFORM_DEVICE_IDLE_WAKEUP Again, I'm not sure if the new CONFIG_ option is really necessary. > + bool > --- 0002/include/linux/platform_device.h > +++ work/include/linux/platform_device.h 2009-05-26 12:32:40.000000000 +0900 > @@ -57,6 +57,14 @@ extern int platform_device_add(struct pl > extern void platform_device_del(struct platform_device *pdev); > extern void platform_device_put(struct platform_device *pdev); > > +#ifdef CONFIG_HAVE_PLATFORM_DEVICE_IDLE_WAKEUP > +void platform_device_idle(struct platform_device *pdev); > +void platform_device_wakeup(struct platform_device *pdev); > +#else > +static inline void platform_device_idle(struct platform_device *pdev) {} > +static inline void platform_device_wakeup(struct platform_device *pdev) {} > +#endif > + > struct platform_driver { > int (*probe)(struct platform_device *); > int (*remove)(struct platform_device *); -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2009/5/29 Rafael J. Wysocki <rjw@sisk.pl>: > On Wednesday 27 May 2009, Magnus Damm wrote: >> From: Magnus Damm <damm@igel.co.jp> >> >> Add platform_device_idle() and platform_device_wakeup() >> and allow architectures to implement their own versions >> of these if CONFIG_HAVE_PLATFORM_IDLE_WAKEUP is set. >> >> Signed-off-by: Magnus Damm <damm@igel.co.jp> >> --- >> >>  Yes, this needs documentation. See [00/04] for now. >> >>  arch/Kconfig           |   3 +++ >>  include/linux/platform_device.h |   8 ++++++++ >>  2 files changed, 11 insertions(+) >> >> --- 0002/arch/Kconfig >> +++ work/arch/Kconfig 2009-05-26 12:31:06.000000000 +0900 >> @@ -115,3 +115,6 @@ config HAVE_DEFAULT_NO_SPIN_MUTEXES >> >>  config HAVE_PLATFORM_DEVICE_ARCHDATA >>    bool >> + >> +config HAVE_PLATFORM_DEVICE_IDLE_WAKEUP > > Again, I'm not sure if the new CONFIG_ option is really necessary. At least it could be combined into a single option for platform device runtime pm. So no need to have separate kconfig options for [02/04] and [03/04]. We could also drop the #ifdef stuff and go for weak symbols that can be overriden by architecture code. I'm open to suggestions. =) / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Friday 29 May 2009, Magnus Damm wrote: > 2009/5/29 Rafael J. Wysocki <rjw@sisk.pl>: > > On Wednesday 27 May 2009, Magnus Damm wrote: > >> From: Magnus Damm <damm@igel.co.jp> > >> > >> Add platform_device_idle() and platform_device_wakeup() > >> and allow architectures to implement their own versions > >> of these if CONFIG_HAVE_PLATFORM_IDLE_WAKEUP is set. > >> > >> Signed-off-by: Magnus Damm <damm@igel.co.jp> > >> --- > >> > >> Yes, this needs documentation. See [00/04] for now. > >> > >> arch/Kconfig | 3 +++ > >> include/linux/platform_device.h | 8 ++++++++ > >> 2 files changed, 11 insertions(+) > >> > >> --- 0002/arch/Kconfig > >> +++ work/arch/Kconfig 2009-05-26 12:31:06.000000000 +0900 > >> @@ -115,3 +115,6 @@ config HAVE_DEFAULT_NO_SPIN_MUTEXES > >> > >> config HAVE_PLATFORM_DEVICE_ARCHDATA > >> bool > >> + > >> +config HAVE_PLATFORM_DEVICE_IDLE_WAKEUP > > > > Again, I'm not sure if the new CONFIG_ option is really necessary. > > At least it could be combined into a single option for platform device > runtime pm. So no need to have separate kconfig options for [02/04] > and [03/04]. > > We could also drop the #ifdef stuff and go for weak symbols that can > be overriden by architecture code. The weak symbols don't work really AFAICS. People have been replacing them with alternative things recently. > I'm open to suggestions. =) Well, that depends on what exactly you want to achieve. :-) Best, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jun 3, 2009 at 6:05 PM, Rafael J. Wysocki<rjw@sisk.pl> wrote: > On Friday 29 May 2009, Magnus Damm wrote: >> 2009/5/29 Rafael J. Wysocki <rjw@sisk.pl>: >> > On Wednesday 27 May 2009, Magnus Damm wrote: >> >> From: Magnus Damm <damm@igel.co.jp> >> >> >> >> Add platform_device_idle() and platform_device_wakeup() >> >> and allow architectures to implement their own versions >> >> of these if CONFIG_HAVE_PLATFORM_IDLE_WAKEUP is set. >> >> >> >> Signed-off-by: Magnus Damm <damm@igel.co.jp> >> >> --- >> >> >> >>  Yes, this needs documentation. See [00/04] for now. >> >> >> >>  arch/Kconfig           |   3 +++ >> >>  include/linux/platform_device.h |   8 ++++++++ >> >>  2 files changed, 11 insertions(+) >> >> >> >> --- 0002/arch/Kconfig >> >> +++ work/arch/Kconfig 2009-05-26 12:31:06.000000000 +0900 >> >> @@ -115,3 +115,6 @@ config HAVE_DEFAULT_NO_SPIN_MUTEXES >> >> >> >>  config HAVE_PLATFORM_DEVICE_ARCHDATA >> >>    bool >> >> + >> >> +config HAVE_PLATFORM_DEVICE_IDLE_WAKEUP >> > >> > Again, I'm not sure if the new CONFIG_ option is really necessary. >> >> At least it could be combined into a single option for platform device >> runtime pm. So no need to have separate kconfig options for [02/04] >> and [03/04]. >> >> We could also drop the #ifdef stuff and go for weak symbols that can >> be overriden by architecture code. > > The weak symbols don't work really AFAICS.  People have been replacing them > with alternative things recently. > >> I'm open to suggestions. =) > > Well, that depends on what exactly you want to achieve. :-) =) I'd like an architecture independent interface upstream that makes it possible for device drivers to notify the architecture code that they are idle() and that need wakeup(). In the long term I hope we can share a great deal of runtime PM code between architectures, but for now we just need some architecture specific hooks to get started. The wakeup()/idle() code in this patch is one way to solve it on a platform device level. Another more generic way would be to add ->enable() and ->disable() callbacks to struct bus_type and introduce device_enable() and device_disable() that takes struct device and invokes the bus callbacks if set. Any thoughts? Does it make sense? / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Friday 05 June 2009, Magnus Damm wrote: > On Wed, Jun 3, 2009 at 6:05 PM, Rafael J. Wysocki<rjw@sisk.pl> wrote: > > On Friday 29 May 2009, Magnus Damm wrote: > >> 2009/5/29 Rafael J. Wysocki <rjw@sisk.pl>: > >> > On Wednesday 27 May 2009, Magnus Damm wrote: > >> >> From: Magnus Damm <damm@igel.co.jp> > >> >> > >> >> Add platform_device_idle() and platform_device_wakeup() > >> >> and allow architectures to implement their own versions > >> >> of these if CONFIG_HAVE_PLATFORM_IDLE_WAKEUP is set. > >> >> > >> >> Signed-off-by: Magnus Damm <damm@igel.co.jp> > >> >> --- > >> >> > >> >> Yes, this needs documentation. See [00/04] for now. > >> >> > >> >> arch/Kconfig | 3 +++ > >> >> include/linux/platform_device.h | 8 ++++++++ > >> >> 2 files changed, 11 insertions(+) > >> >> > >> >> --- 0002/arch/Kconfig > >> >> +++ work/arch/Kconfig 2009-05-26 12:31:06.000000000 +0900 > >> >> @@ -115,3 +115,6 @@ config HAVE_DEFAULT_NO_SPIN_MUTEXES > >> >> > >> >> config HAVE_PLATFORM_DEVICE_ARCHDATA > >> >> bool > >> >> + > >> >> +config HAVE_PLATFORM_DEVICE_IDLE_WAKEUP > >> > > >> > Again, I'm not sure if the new CONFIG_ option is really necessary. > >> > >> At least it could be combined into a single option for platform device > >> runtime pm. So no need to have separate kconfig options for [02/04] > >> and [03/04]. > >> > >> We could also drop the #ifdef stuff and go for weak symbols that can > >> be overriden by architecture code. > > > > The weak symbols don't work really AFAICS. People have been replacing them > > with alternative things recently. > > > >> I'm open to suggestions. =) > > > > Well, that depends on what exactly you want to achieve. :-) > > =) > > I'd like an architecture independent interface upstream that makes it > possible for device drivers to notify the architecture code that they > are idle() and that need wakeup(). > > In the long term I hope we can share a great deal of runtime PM code > between architectures, but for now we just need some architecture > specific hooks to get started. > > The wakeup()/idle() code in this patch is one way to solve it on a > platform device level. Another more generic way would be to add > ->enable() and ->disable() callbacks to struct bus_type and introduce > device_enable() and device_disable() that takes struct device and > invokes the bus callbacks if set. So, you need a generic mechanism that drivers can use to notify the bus type code that a device is idle and therefore it should schedule an autosuspend request for the device. Also, you want a mechanism by which drivers can notify the platform code that there is a need to wake-up an autosuspended device. Is that correct? Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Jun 6, 2009 at 5:42 AM, Rafael J. Wysocki<rjw@sisk.pl> wrote: > On Friday 05 June 2009, Magnus Damm wrote: >> On Wed, Jun 3, 2009 at 6:05 PM, Rafael J. Wysocki<rjw@sisk.pl> wrote: >> > On Friday 29 May 2009, Magnus Damm wrote: >> >> 2009/5/29 Rafael J. Wysocki <rjw@sisk.pl>: >> >> > On Wednesday 27 May 2009, Magnus Damm wrote: >> >> >> From: Magnus Damm <damm@igel.co.jp> >> >> >> >> >> >> Add platform_device_idle() and platform_device_wakeup() >> >> >> and allow architectures to implement their own versions >> >> >> of these if CONFIG_HAVE_PLATFORM_IDLE_WAKEUP is set. >> >> >> >> >> >> Signed-off-by: Magnus Damm <damm@igel.co.jp> >> >> >> --- >> The wakeup()/idle() code in this patch is one way to solve it on a >> platform device level. Another more generic way would be to add >> ->enable() and ->disable() callbacks to struct bus_type and introduce >> device_enable() and device_disable() that takes struct device and >> invokes the bus callbacks if set. > > So, you need a generic mechanism that drivers can use to notify the bus type > code that a device is idle and therefore it should schedule an autosuspend > request for the device. Â Also, you want a mechanism by which drivers can notify > the platform code that there is a need to wake-up an autosuspended device. > Is that correct? Yes, you are 100% correct that I want drivers to have some way to notify the bus type that a certain device is idle or needs to be woken up. Exactly what should happen when the device is marked as idle is a different question. I guess this is bus specific. Connecting the idle notification directly to autosuspend is not a very good idea IMO since the power management comes with latency restrictions. If we zoom out a bit then I think that we should have something similar to cpuidle but for devices. Maybe the driver should give a list of suspend modes, their latencies and power savings. This per driver (or per device) latency information is important, but even more important IMO is latency information for the bus itself. So for our on-chip SuperH SoC platform devices I'd like to keep track of which devices that are idle, and if all devices within one power domain are idle then i'd like to execute autosuspend() for those and after that power off the bus/domain. But only if this doesn't break any latency requirements. Cheers, / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday 09 June 2009, Magnus Damm wrote: > On Sat, Jun 6, 2009 at 5:42 AM, Rafael J. Wysocki<rjw@sisk.pl> wrote: > > On Friday 05 June 2009, Magnus Damm wrote: > >> On Wed, Jun 3, 2009 at 6:05 PM, Rafael J. Wysocki<rjw@sisk.pl> wrote: > >> > On Friday 29 May 2009, Magnus Damm wrote: > >> >> 2009/5/29 Rafael J. Wysocki <rjw@sisk.pl>: > >> >> > On Wednesday 27 May 2009, Magnus Damm wrote: > >> >> >> From: Magnus Damm <damm@igel.co.jp> > >> >> >> > >> >> >> Add platform_device_idle() and platform_device_wakeup() > >> >> >> and allow architectures to implement their own versions > >> >> >> of these if CONFIG_HAVE_PLATFORM_IDLE_WAKEUP is set. > >> >> >> > >> >> >> Signed-off-by: Magnus Damm <damm@igel.co.jp> > >> >> >> --- > > >> The wakeup()/idle() code in this patch is one way to solve it on a > >> platform device level. Another more generic way would be to add > >> ->enable() and ->disable() callbacks to struct bus_type and introduce > >> device_enable() and device_disable() that takes struct device and > >> invokes the bus callbacks if set. > > > > So, you need a generic mechanism that drivers can use to notify the bus type > > code that a device is idle and therefore it should schedule an autosuspend > > request for the device. Also, you want a mechanism by which drivers can notify > > the platform code that there is a need to wake-up an autosuspended device. > > Is that correct? > > Yes, you are 100% correct that I want drivers to have some way to > notify the bus type that a certain device is idle or needs to be woken > up. > > Exactly what should happen when the device is marked as idle is a > different question. I guess this is bus specific. Connecting the idle > notification directly to autosuspend is not a very good idea IMO since > the power management comes with latency restrictions. > > If we zoom out a bit then I think that we should have something > similar to cpuidle but for devices. Maybe the driver should give a > list of suspend modes, their latencies and power savings. This per > driver (or per device) latency information is important, but even more > important IMO is latency information for the bus itself. > > So for our on-chip SuperH SoC platform devices I'd like to keep track > of which devices that are idle, and if all devices within one power > domain are idle then i'd like to execute autosuspend() for those and > after that power off the bus/domain. But only if this doesn't break > any latency requirements. OK, I think we can add ->idle() and ->wakeup() callbacks to struct bus_type for this purpose. BTW, I'm waiting for a new version of your patch adding the arch data to struct platform_device with a better changelog. Best, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jun 10, 2009 at 8:41 AM, Rafael J. Wysocki<rjw@sisk.pl> wrote: > On Tuesday 09 June 2009, Magnus Damm wrote: >> On Sat, Jun 6, 2009 at 5:42 AM, Rafael J. Wysocki<rjw@sisk.pl> wrote: >> > On Friday 05 June 2009, Magnus Damm wrote: >> >> On Wed, Jun 3, 2009 at 6:05 PM, Rafael J. Wysocki<rjw@sisk.pl> wrote: >> >> > On Friday 29 May 2009, Magnus Damm wrote: >> >> >> 2009/5/29 Rafael J. Wysocki <rjw@sisk.pl>: >> >> >> > On Wednesday 27 May 2009, Magnus Damm wrote: >> >> >> >> From: Magnus Damm <damm@igel.co.jp> >> >> >> >> >> >> >> >> Add platform_device_idle() and platform_device_wakeup() >> >> >> >> and allow architectures to implement their own versions >> >> >> >> of these if CONFIG_HAVE_PLATFORM_IDLE_WAKEUP is set. >> >> >> >> >> >> >> >> Signed-off-by: Magnus Damm <damm@igel.co.jp> >> >> >> >> --- >> >> >> The wakeup()/idle() code in this patch is one way to solve it on a >> >> platform device level. Another more generic way would be to add >> >> ->enable() and ->disable() callbacks to struct bus_type and introduce >> >> device_enable() and device_disable() that takes struct device and >> >> invokes the bus callbacks if set. >> > >> > So, you need a generic mechanism that drivers can use to notify the bus type >> > code that a device is idle and therefore it should schedule an autosuspend >> > request for the device. Â Also, you want a mechanism by which drivers can notify >> > the platform code that there is a need to wake-up an autosuspended device. >> > Is that correct? >> >> Yes, you are 100% correct that I want drivers to have some way to >> notify the bus type that a certain device is idle or needs to be woken >> up. >> >> Exactly what should happen when the device is marked as idle is a >> different question. I guess this is bus specific. Connecting the idle >> notification directly to autosuspend is not a very good idea IMO since >> the power management comes with latency restrictions. >> >> If we zoom out a bit then I think that we should have something >> similar to cpuidle but for devices. Maybe the driver should give a >> list of suspend modes, their latencies and power savings. This per >> driver (or per device) latency information is important, but even more >> important IMO is latency information for the bus itself. >> >> So for our on-chip SuperH SoC platform devices I'd like to keep track >> of which devices that are idle, and if all devices within one power >> domain are idle then i'd like to execute autosuspend() for those and >> after that power off the bus/domain. But only if this doesn't break >> any latency requirements. > > OK, I think we can add ->idle() and ->wakeup() callbacks to struct bus_type > for this purpose. Sounds very good! So unless there are any objections I'll just post a "Driver Core: Add idle and wakeup functions V2" which adds ->idle() and ->wakeup() callbacks to struct bus_type together with inline asm functions device_idle() and device_wakeup(). > BTW, I'm waiting for a new version of your patch adding the arch data to > struct platform_device with a better changelog. Yeah, sorry about the delay. I will post an updated version! / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wednesday 10 June 2009, Magnus Damm wrote: > On Wed, Jun 10, 2009 at 8:41 AM, Rafael J. Wysocki<rjw@sisk.pl> wrote: > > On Tuesday 09 June 2009, Magnus Damm wrote: > >> On Sat, Jun 6, 2009 at 5:42 AM, Rafael J. Wysocki<rjw@sisk.pl> wrote: > >> > On Friday 05 June 2009, Magnus Damm wrote: > >> >> On Wed, Jun 3, 2009 at 6:05 PM, Rafael J. Wysocki<rjw@sisk.pl> wrote: > >> >> > On Friday 29 May 2009, Magnus Damm wrote: > >> >> >> 2009/5/29 Rafael J. Wysocki <rjw@sisk.pl>: > >> >> >> > On Wednesday 27 May 2009, Magnus Damm wrote: > >> >> >> >> From: Magnus Damm <damm@igel.co.jp> > >> >> >> >> > >> >> >> >> Add platform_device_idle() and platform_device_wakeup() > >> >> >> >> and allow architectures to implement their own versions > >> >> >> >> of these if CONFIG_HAVE_PLATFORM_IDLE_WAKEUP is set. > >> >> >> >> > >> >> >> >> Signed-off-by: Magnus Damm <damm@igel.co.jp> > >> >> >> >> --- > >> > >> >> The wakeup()/idle() code in this patch is one way to solve it on a > >> >> platform device level. Another more generic way would be to add > >> >> ->enable() and ->disable() callbacks to struct bus_type and introduce > >> >> device_enable() and device_disable() that takes struct device and > >> >> invokes the bus callbacks if set. > >> > > >> > So, you need a generic mechanism that drivers can use to notify the bus type > >> > code that a device is idle and therefore it should schedule an autosuspend > >> > request for the device. Also, you want a mechanism by which drivers can notify > >> > the platform code that there is a need to wake-up an autosuspended device. > >> > Is that correct? > >> > >> Yes, you are 100% correct that I want drivers to have some way to > >> notify the bus type that a certain device is idle or needs to be woken > >> up. > >> > >> Exactly what should happen when the device is marked as idle is a > >> different question. I guess this is bus specific. Connecting the idle > >> notification directly to autosuspend is not a very good idea IMO since > >> the power management comes with latency restrictions. > >> > >> If we zoom out a bit then I think that we should have something > >> similar to cpuidle but for devices. Maybe the driver should give a > >> list of suspend modes, their latencies and power savings. This per > >> driver (or per device) latency information is important, but even more > >> important IMO is latency information for the bus itself. > >> > >> So for our on-chip SuperH SoC platform devices I'd like to keep track > >> of which devices that are idle, and if all devices within one power > >> domain are idle then i'd like to execute autosuspend() for those and > >> after that power off the bus/domain. But only if this doesn't break > >> any latency requirements. > > > > OK, I think we can add ->idle() and ->wakeup() callbacks to struct bus_type > > for this purpose. > > Sounds very good! So unless there are any objections I'll just post a > "Driver Core: Add idle and wakeup functions V2" which adds ->idle() > and ->wakeup() callbacks to struct bus_type together with inline asm > functions device_idle() and device_wakeup(). Well, I'd prefer to do that along with my run-time PM patch that is currently being discussed with Alan and Oliver (I've sent a Cc of the first message in that thread to you). > > BTW, I'm waiting for a new version of your patch adding the arch data to > > struct platform_device with a better changelog. > > Yeah, sorry about the delay. I will post an updated version! Great, thanks! Best, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- 0002/arch/Kconfig +++ work/arch/Kconfig 2009-05-26 12:31:06.000000000 +0900 @@ -115,3 +115,6 @@ config HAVE_DEFAULT_NO_SPIN_MUTEXES config HAVE_PLATFORM_DEVICE_ARCHDATA bool + +config HAVE_PLATFORM_DEVICE_IDLE_WAKEUP + bool --- 0002/include/linux/platform_device.h +++ work/include/linux/platform_device.h 2009-05-26 12:32:40.000000000 +0900 @@ -57,6 +57,14 @@ extern int platform_device_add(struct pl extern void platform_device_del(struct platform_device *pdev); extern void platform_device_put(struct platform_device *pdev); +#ifdef CONFIG_HAVE_PLATFORM_DEVICE_IDLE_WAKEUP +void platform_device_idle(struct platform_device *pdev); +void platform_device_wakeup(struct platform_device *pdev); +#else +static inline void platform_device_idle(struct platform_device *pdev) {} +static inline void platform_device_wakeup(struct platform_device *pdev) {} +#endif + struct platform_driver { int (*probe)(struct platform_device *); int (*remove)(struct platform_device *);