On Mon, 2006-02-20 at 17:31 -0800, Patrick Mochel wrote:
> On Mon, 20 Feb 2006, Andrew Morton wrote:
>
> > Andrew Morton <[EMAIL PROTECTED]> wrote:
> > >
> > > Shaohua Li <[EMAIL PROTECTED]> wrote:
> > > >
> > > > Hi,
> > > > Considering below scenario:
> > > > 1.Unload a PCI device's driver, the device ->current remains in PCI_D0.
> > > > 2.Do suspend/resume circle. After that, BIOS puts the device to D3.
> > > > 3.Reload the device driver. The calling pci_set_power_state in the
> > > > driver can't change the state to D0, as set_power_state thinks the
> > > > device is already in D0.
> > > > This patch assumes drivers call pci_disable_device at unloading and sets
> > > > device's state to unknown.
> > > > A bug is reported at http://bugzilla.kernel.org/show_bug.cgi?id=6024
> > > >
> > > > Signed-off-by: Shaohua Li <[EMAIL PROTECTED]>
> > > > ---
> > > >
> > > > linux-2.6.16-rc3-root/drivers/pci/pci.c | 1 +
> > > > 1 files changed, 1 insertion(+)
> > > >
> > > > diff -puN drivers/pci/pci.c~pci_device_state drivers/pci/pci.c
> > > > --- linux-2.6.16-rc3/drivers/pci/pci.c~pci_device_state 2006-02-14
> > > > 13:59:25.000000000 +0800
> > > > +++ linux-2.6.16-rc3-root/drivers/pci/pci.c 2006-02-14
> > > > 14:00:12.000000000 +0800
> > > > @@ -532,6 +532,7 @@ pci_disable_device(struct pci_dev *dev)
> > > > pci_write_config_word(dev, PCI_COMMAND, pci_command);
> > > > }
> > > > dev->is_busmaster = 0;
> > > > + dev->current_state = PCI_UNKNOWN;
> > > >
> > > > pcibios_disable_device(dev);
> > > > dev->is_enabled = 0;
> > > > _
> > >
> > > waaah.
> > >
> > > For a long time my laptop wouldn't suspend because something in the
> > > ehci_hcd module is returning -EINVAL.
> > >
> > > ACPI: PCI interrupt for device 0000:00:1d.7 disabled
> > > ACPI (acpi_bus-0199): Device is not power manageable [20060210]
> > > ACPI: PCI Interrupt 0000:00:1d.7[D] -> GSI 23 (level, low) -> IRQ 22
> > > PCI: Setting latency timer of device 0000:00:1d.7 to 64
> > > Could not suspend device 0000:00:1d.7: error -22
> > >
> > > but a few weeks ago, suspend-to-ram started working. And I was happy.
> > >
> > > The above patch puts it back into not-working mode again.
> > >
> > > So I kinda liked the bug which you've gone and fixed.
> > >
> > > This shows that ehci can actually suspend and resume if it really wants
> > > to;
> > > problem is, it doesn't. How come?
> >
> > OK, the failure is in:
> >
> > suspend_device
> > ->pci_device_suspend
> > ->usb_hcd_pci_suspend
> > ->pci_set_power_state
> >
> > pci_set_power_state() is failing here:
> >
> > if (state != PCI_D0 && dev->current_state > state)
> > return -EINVAL;
> >
> > Because state=3 (PCI_D3hot) and current state=5 (PCI_UNKNOWN)
> >
> > I note that usb_hcd_pci_suspend() calls pci_disable_device() _before_
> > calling pci_set_power_state(). I don't know if that's kosher, but it's
> > what caused this failure.
>
> Ugh, didn't think about that the first time around.
>
> Does the following look sane? If you're feeling adventurous, give it a
> try..
>
> With the patch below, we don't touch the pci_disable_device() path, since
> the device may need to enter a low-power state afterwards (whether it's
> because of a system suspend or a module unload). Instead, we set the power
> state of an unbound device to PCI_UNKNOWN on resume and do not try to
> power it on or restore any of its config space.
>
> This is different than current behavior, where the config space is always
> restored, even if the device is not bound to a driver. The fact that we
> were doing this without putting the device into D0 is bad in itself - the
> restored config space may not hold if the device is in D3.
This might not work. We haven't driver for pci bridges, and we need
restore bridges's config space.
Or another solution is we could simply set device's current_state to
unknown in pci_device_remove.
Thanks,
Shaohua
>
> To behave identically, albeit wastefully, try the patch below, with these
> additional lines:
>
>
> + pci_dev->current_state = PCI_UNKNOWN;
> + pci_set_power_state(pci_dev, PCI_D0);
> + pci_default_resume(pci_dev);
>
> Thanks,
>
>
> Pat
>
>
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index 0aa14c9..1866811 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -303,10 +303,21 @@ static int pci_device_resume(struct devi
> struct pci_dev * pci_dev = to_pci_dev(dev);
> struct pci_driver * drv = pci_dev->driver;
>
> - if (drv && drv->resume)
> - drv->resume(pci_dev);
> - else
> - pci_default_resume(pci_dev);
> + if (drv) {
> + if (drv->resume)
> + drv->resume(pci_dev);
> + else
> + pci_default_resume(pci_dev);
> + } else {
> + /*
> + * A device w/o a driver has an unknown power state - the BIOS
> + * could leave it in D3 after a suspend/resume cycle, but it
> also
> + * could have put it back in D0. Who knows?
> + * So, we simply set the state to PCI_UNKNOWN so that it
> + * always gets put into D0 when a driver is loaded again.
> + */
> + pci_dev->current_state = PCI_UNKNOWN;
> + }
> return 0;
> }
-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems? Stop! Download the new AJAX search engine that makes
searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
