On Wednesday 02 June 2010, Alan Stern wrote:
> On Tue, 1 Jun 2010, Rafael J. Wysocki wrote:
>
> > > Therefore the default state for unbound PCI devices should be
> > > runtime-PM-enabled.
> >
> > Why do you think it should?
>
> So that they can be put into a low-power state. If they are disabled
> for runtime PM then they will remain in a high-power state.
>
> > > Something like the patch below, awkward though it is. Does this look
> > > reasonable?
> >
> > I'm not sure if r8169 and e1000e will work with it. Probably not, but I'm
> > too tired to check right now. :-)
>
> You mean because they already include their own runtime power
> management? Then they would need to be updated to match these changes,
> naturally.
>
> > > @@ -365,15 +380,24 @@ static int pci_device_probe(struct devic
> > >
> > > static int pci_device_remove(struct device * dev)
> > > {
> > > + struct device_driver *driver = dev->driver;
> > > struct pci_dev * pci_dev = to_pci_dev(dev);
> > > struct pci_driver * drv = pci_dev->driver;
> > >
> > > if (drv) {
> > > - if (drv->remove)
> > > + if (drv->remove) {
> > > + pm_runtime_get_sync(dev);
> > > drv->remove(pci_dev);
> > > + pm_runtime_put_noidle(dev);
> > > + }
> > > pci_dev->driver = NULL;
> > > }
> >
> > Is "else" missing here or am I missing something?
>
> I don't think an "else" is missing. Rather, the entire "if (drv)"
> line should be eliminated because it can never fail.
>
> > > + /* Undo the pm_runtime_get_sync() in local_pci_probe() */
> > > + dev->driver = NULL;
> > > + pm_runtime_put_sync(dev);
> > > + dev->driver = driver;
> > > +
> > > /*
> > > * If the device is still on, set the power state as "unknown",
> > > * since it might change by the next time we load the driver.
> >
> > Do PCI bus type callbacks need to be reworked to work correctly with unbound
> > devices?
>
> They do; I neglected to take care of them when first writing this
> patch. After everything is working I'll submit it for real. For now,
> the point was to find out whether this is going in the right direction.
I don't have a fundamental problem with it. As long as all things can be made
work along these lines, it's fine by me.
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
