On Mon, 23 Mar 2009 22:30:09 +0100 "Rafael J. Wysocki" <rjw@xxxxxxx> wrote: > On Sunday 22 March 2009, Rafael J. Wysocki wrote: > > On Saturday 21 March 2009, Rafael J. Wysocki wrote: > > > From: Rafael J. Wysocki <rjw@xxxxxxx> > > > > > > The story in http://bugzilla.kernel.org/show_bug.cgi?id=12846 > > > shows that setting the power state of a PCI device by > > > pci_raw_set_power_state() may sometimes fail. For this reason, > > > pci_raw_set_power_state() should not assume that the power state > > > of the device has actually changed after writing into its PMCSR. > > > Instead, it should read the value from there and use it to update > > > dev->current_state. It also is useful to print a warning if the > > > device's power state hasn't changed as expected. > > > > > > Signed-off-by: Rafael J. Wysocki <rjw@xxxxxxx> > > > --- > > OK, since the Ben's radeonfb fix for bug #12846 has been merged, > > I'd like to do something a bit different. > > > > Patch 1/2 introduces __pci_set_power_state() that will allow the > > radeonfb driver not to open code PCI PM operations. > > > > Patch 2/2 makes the driver use __pci_set_power_state(). > > > > Comments welcome. > > Well, Jesse doesn't like these patches very much, so here's an > alternative. > > 1/2 changes platform_pci_set_power_state() into an exported function > (and uses it to simplify pci_set_power_state() a bit) so that the > radeonfb driver can use it. > > 2/2 modifies the radeonfb driver itself. The thing I didn't like was that it made the radeon driver use an internal interface; I'd really prefer a proper return value from pci_set_power_state, which in turn means auditing all its current callers. But that doesn't seem worth it unless we see other drivers needing something similar... And if we did go with something like your first patch, I'd still rather see the timeout done in the driver, rather than having the attempts & delay included in the function... -- Jesse Barnes, Intel Open Source Technology Center -- 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
