Re: [PATCH] PCI / PM: Disable wakeup during shutdown for devices not enabled to wake up

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

 



On Friday, March 02, 2012, Alan Stern wrote:
> On Fri, 2 Mar 2012, Jesse Barnes wrote:
> 
> > On Fri, 2 Mar 2012 15:44:27 +0100
> > Francois Romieu <romieu@xxxxxxxxxxxxx> wrote:
> > 
> > > Rafael J. Wysocki <rjw@xxxxxxx> :
> > > [...]
> > > > If a PCI device is enabled to generate wakeup signals (PME) when put
> > > > into a low-power state by runtime PM, it will be still enabled to
> > > > generate those signals after the system shutdown, unless its driver's
> > > > .shutdown() callback takes care of the wakeup signals generation
> > > > setting.  Moreover, there are devices that are not enabled to wake
> > > > up the system and that are configured by runtime PM to generate
> > > > wakeup signals so that (runtime) remote wakeup works with them.
> > > > Those devices should be reconfigured during system shutdown so that
> > > > they don't generate wakeup signals, but at least some drivers don't
> > > > do that.  However, that very well may be done by the PCI core so
> > > > that drivers don't have to worry about it.  For this reason, modify
> > > > pci_device_shutdown() to disable the generation of wakeup events for
> > > > devices not supposed to wake up the system.
> > > 
> > > On a tangent side, if register writes are ignored by the PCI device
> > > after its .runtime_suspend() handler has been called, whose
> > > responsibility is it to bring back the device in a state from where
> > > the .shutdown() callback can operate ?
> > > 
> > > The r8169 driver exhibits this problem. Its .runtime_xyz() callbacks
> > > are called on link activity. Thus, after a link loss - or no link at
> > > all - .runtime_suspend() and an ineffective .shutdown(), the device is
> > > brought back up as soon as the link recovers.
> > > 
> > > The device driver can monitor .runtime_{suspend / resume} activity
> > > and take appropriate action in .shutdown(). Sameer Nanda has sent
> > > such a patch (see below). I am not sure this is the adequate fix:
> > > 1. the driver ought essentially to perform the hardware part of a
> > >    .runtime_resume() on its own behalf. Mildly engaging.
> > > 2. by the same line as above, could it make more sense to call
> > >    pm_runtime_resume() in driver/base/core.c::device_shutdown ?
> > 
> > I thought there would be a more elegant way of increasing the device
> > use count on entering .shutdown, either from the wifi core or at the
> > driver level, so you don't have to track things with a separate flag.
> 
> That's right.  For example, instead of keeping its own flag to know
> whether or not the device is runtime-suspended, the shutdown routine
> could just always call pm_runtime_resume_sync().  Or it could ask the
> PCI core to put the device into D0, regardless of the device's current
> state.
> 
> The driver core doesn't do this because most devices don't need it, and
> it would slow down the whole shutdown procedure if every suspended
> device were always resumed.

For devices known to have this problem I think calling pm_runtime_resume_sync()
unconditionally at the beginning of .suspend() would be a good approach in
general.

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux