suspend() and runtime_suspend()

suspend() and runtime_suspend()

[Martin, LoicX]

[-- Attachment #1.1: Type: text/plain, Size: 2363 bytes --]

Hi

I found in the linux kernel documentation :
In /power/pci.h
2.4.1. System Suspend

PCI device drivers (that don't implement legacy power management callbacks) are
generally not expected to prepare devices for signaling wakeup or to put them
into low-power states.  However, if one of the driver's suspend callbacks
(pm->suspend() or pm->suspend_noirq()) saves the device's standard configuration
registers, pci_pm_suspend_noirq() will assume that the device has been prepared
to signal wakeup and put into a low-power state by the driver (the driver is
then assumed to have used the helper functions provided by the PCI subsystem for
this purpose).  PCI device drivers are not encouraged to do that, but in some
rare cases doing that in the driver may be the optimum approach.

3.1.2 suspend()

It is not required (in fact it even is
not recommended) that a PCI driver's suspend() callback save the standard
configuration registers of the device, prepare it for waking up the system, or
put it into a low-power state.  All of these operations can very well be taken
care of by the PCI subsystem, without the driver's participation.

2.3. Runtime Device Power Management

It is expected that the device driver's pm->runtime_suspend() callback will
not attempt to prepare the device for signaling wakeup or to put it into a
low-power state.  The driver ought to leave these tasks to the PCI subsystem
that has all of the information necessary to perform them.


So I was wondering why in the kernel sources, most of the PCI drivers were using pci_set_power_state, as well pci_save_state either in suspend() callbacks either in runtime_suspend() callbacks.
Why should we not use those functions in a driver suspend callback implementation ?

Thx

Loic


---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

[-- Attachment #1.2: Type: text/html, Size: 29073 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: suspend() and runtime_suspend()

[Alan Stern]

On Mon, 21 Mar 2011, Martin, LoicX wrote:

> Hi
> 
> I found in the linux kernel documentation :
> In /power/pci.h
> 2.4.1. System Suspend
> 
> PCI device drivers (that don't implement legacy power management callbacks) are
> generally not expected to prepare devices for signaling wakeup or to put them
> into low-power states.  However, if one of the driver's suspend callbacks
> (pm->suspend() or pm->suspend_noirq()) saves the device's standard configuration
> registers, pci_pm_suspend_noirq() will assume that the device has been prepared
> to signal wakeup and put into a low-power state by the driver (the driver is
> then assumed to have used the helper functions provided by the PCI subsystem for
> this purpose).  PCI device drivers are not encouraged to do that, but in some
> rare cases doing that in the driver may be the optimum approach.
> 
> 3.1.2 suspend()
> 
> It is not required (in fact it even is
> not recommended) that a PCI driver's suspend() callback save the standard
> configuration registers of the device, prepare it for waking up the system, or
> put it into a low-power state.  All of these operations can very well be taken
> care of by the PCI subsystem, without the driver's participation.
> 
> 2.3. Runtime Device Power Management
> 
> It is expected that the device driver's pm->runtime_suspend() callback will
> not attempt to prepare the device for signaling wakeup or to put it into a
> low-power state.  The driver ought to leave these tasks to the PCI subsystem
> that has all of the information necessary to perform them.
> 
> 
> So I was wondering why in the kernel sources, most of the PCI drivers were using pci_set_power_state, as well pci_save_state either in suspend() callbacks

Perhaps because those PCI drivers were written before the
documentation, using legacy power management.

>  either in runtime_suspend() callbacks.

There should not be any drivers doing that, because runtime_suspend is 
a relatively recent addition.  Can you provide examples?

> Why should we not use those functions in a driver suspend callback implementation ?

Because it would duplicate code that is already present in the PCI
core.

Alan Stern

Re: suspend() and runtime_suspend()

[Martin, LoicX]

Hi 

I found those drivers:

drivers/dma/intel_mid_dma.c
drivers/i2c/busses/i2c-intel-mid.c
drivers/staging/gma500/psb_drv.c
drivers/staging/gma500/psb_powermgmt.c
drivers/staging/intel_sst/intel_sst.c

We are currently working on this purpose (runtime suspend/resume implementation), and our first understanding was not perfectly clear.
That's why I was asking about the correct implementation of those callback.

Thanks

I have one more question about this part of the documentation:
>>However, the driver should not call the pm_runtime_allow() helper function unblocking
>>the runtime PM of the device. Instead, it should allow user space or some
>>platform-specific code to do that

Is there any reason concerning the system stability or something else to do it that way instead of doing it in the driver during the probe ?


Loïc

-----Original Message-----
From: Alan Stern [mailto:stern@rowland.harvard.edu] 
Sent: Monday, March 21, 2011 3:28 PM
To: Martin, LoicX
Cc: linux-pm@lists.linux-foundation.org
Subject: Re: [linux-pm] suspend() and runtime_suspend()

On Mon, 21 Mar 2011, Martin, LoicX wrote:

> Hi
> 
> I found in the linux kernel documentation :
> In /power/pci.h
> 2.4.1. System Suspend
> 
> PCI device drivers (that don't implement legacy power management callbacks) are
> generally not expected to prepare devices for signaling wakeup or to put them
> into low-power states.  However, if one of the driver's suspend callbacks
> (pm->suspend() or pm->suspend_noirq()) saves the device's standard configuration
> registers, pci_pm_suspend_noirq() will assume that the device has been prepared
> to signal wakeup and put into a low-power state by the driver (the driver is
> then assumed to have used the helper functions provided by the PCI subsystem for
> this purpose).  PCI device drivers are not encouraged to do that, but in some
> rare cases doing that in the driver may be the optimum approach.
> 
> 3.1.2 suspend()
> 
> It is not required (in fact it even is
> not recommended) that a PCI driver's suspend() callback save the standard
> configuration registers of the device, prepare it for waking up the system, or
> put it into a low-power state.  All of these operations can very well be taken
> care of by the PCI subsystem, without the driver's participation.
> 
> 2.3. Runtime Device Power Management
> 
> It is expected that the device driver's pm->runtime_suspend() callback will
> not attempt to prepare the device for signaling wakeup or to put it into a
> low-power state.  The driver ought to leave these tasks to the PCI subsystem
> that has all of the information necessary to perform them.
> 
> 
> So I was wondering why in the kernel sources, most of the PCI drivers were using pci_set_power_state, as well pci_save_state either in suspend() callbacks

Perhaps because those PCI drivers were written before the
documentation, using legacy power management.

>  either in runtime_suspend() callbacks.

There should not be any drivers doing that, because runtime_suspend is 
a relatively recent addition.  Can you provide examples?

> Why should we not use those functions in a driver suspend callback implementation ?

Because it would duplicate code that is already present in the PCI
core.

Alan Stern

---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

Re: suspend() and runtime_suspend()

[Alan Stern]

On Tue, 22 Mar 2011, Martin, LoicX wrote:

> Hi 
> 
> I found those drivers:
> 
> drivers/dma/intel_mid_dma.c

This driver uses the legacy PM interface, and it appears that the 
runtime_suspend method was piggybacked on top of that.  Consequently it 
was not done properly.

> drivers/i2c/busses/i2c-intel-mid.c

This driver simply needs to be changed.

You could write to the maintainers of these drivers, informing them of 
the problem.

> drivers/staging/gma500/psb_drv.c
> drivers/staging/gma500/psb_powermgmt.c
> drivers/staging/intel_sst/intel_sst.c

Nobody expects code in drivers/staging to be a paragon of good style.  
Again, you could write to the maintainers.

> We are currently working on this purpose (runtime suspend/resume implementation), and our first understanding was not perfectly clear.
> That's why I was asking about the correct implementation of those callback.

There are plenty of other examples showing the right way to do it.

> Thanks
> 
> I have one more question about this part of the documentation:
> >>However, the driver should not call the pm_runtime_allow() helper function unblocking
> >>the runtime PM of the device. Instead, it should allow user space or some
> >>platform-specific code to do that
> 
> Is there any reason concerning the system stability or something else to do it that way instead of doing it in the driver during the probe ?

People have had bad experiences with devices that don't work correctly
with runtime-suspend.  I don't know how well-behaved most PCI devices
are in this regard, but a lot of USB devices fail miserably.  Hence the
decision to have some subsystems forbid runtime-suspend by default,
leaving userspace to decide whether runtime-suspend should be allowed.

If you've got a driver for a device that you _know_ will work correctly 
with runtime-suspend, you can go ahead and call pm_runtime_allow() 
during probe.

Alan Stern

Re: suspend() and runtime_suspend()

[Rafael J. Wysocki]

On Wednesday, March 23, 2011, Alan Stern wrote:
> On Tue, 22 Mar 2011, Martin, LoicX wrote:
> 
> > Hi 
> > 
> > I found those drivers:
> > 
> > drivers/dma/intel_mid_dma.c
> 
> This driver uses the legacy PM interface, and it appears that the 
> runtime_suspend method was piggybacked on top of that.  Consequently it 
> was not done properly.
> 
> > drivers/i2c/busses/i2c-intel-mid.c
> 
> This driver simply needs to be changed.
> 
> You could write to the maintainers of these drivers, informing them of 
> the problem.
> 
> > drivers/staging/gma500/psb_drv.c
> > drivers/staging/gma500/psb_powermgmt.c
> > drivers/staging/intel_sst/intel_sst.c
> 
> Nobody expects code in drivers/staging to be a paragon of good style.  
> Again, you could write to the maintainers.
> 
> > We are currently working on this purpose (runtime suspend/resume implementation), and our first understanding was not perfectly clear.
> > That's why I was asking about the correct implementation of those callback.
> 
> There are plenty of other examples showing the right way to do it.
> 
> > Thanks
> > 
> > I have one more question about this part of the documentation:
> > >>However, the driver should not call the pm_runtime_allow() helper function unblocking
> > >>the runtime PM of the device. Instead, it should allow user space or some
> > >>platform-specific code to do that
> > 
> > Is there any reason concerning the system stability or something else to do it that way instead of doing it in the driver during the probe ?
> 
> People have had bad experiences with devices that don't work correctly
> with runtime-suspend.  I don't know how well-behaved most PCI devices
> are in this regard,

Not very well. :-(

> but a lot of USB devices fail miserably.  Hence the
> decision to have some subsystems forbid runtime-suspend by default,
> leaving userspace to decide whether runtime-suspend should be allowed.
> 
> If you've got a driver for a device that you _know_ will work correctly 
> with runtime-suspend, you can go ahead and call pm_runtime_allow() 
> during probe.

Exactly.

Thanks for your patience. :-)

Rafael