[PATCH] Power: call save_state on PCI devices along with suspend

[PATCH] Power: call save_state on PCI devices along with suspend

[Benjamin Herrenschmidt]

Hi Patrick !

Don't we want that ? It will help if any driver currently relies on
the save_state callback to be called...

Ben.

diff -urN linux-2.5/drivers/pci/pci-driver.c linuxppc-2.5-benh/drivers/pci/pci-driver.c
--- linux-2.5/drivers/pci/pci-driver.c	2003-09-09 20:16:09.000000000 +0200
+++ linuxppc-2.5-benh/drivers/pci/pci-driver.c	2003-09-09 20:05:59.000000000 +0200
@@ -163,6 +163,9 @@
 	struct pci_dev * pci_dev = to_pci_dev(dev);
 	struct pci_driver * drv = pci_dev->driver;
 
+	/* Compatibility with drivers using obsolete save_state */
+	if (drv && drv->save_state)
+		return drv->save_state(pci_dev,state);
 	if (drv && drv->suspend)
 		return drv->suspend(pci_dev,state);
 	return 0;

Re: [PATCH] Power: call save_state on PCI devices along with suspend

[Patrick Mochel]

> Don't we want that ? It will help if any driver currently relies on
> the save_state callback to be called...

Bah, this patch slipped my mind. How many drivers actually use 
->save_state()? From a quick look, it looks like: 

1. drivers/ide/pci/sc1200.c
2. drivers/net/irda/vlsi_ir.c
3. drivers/scsi/nsp32.c
4. drivers/serial/8250_pci.c

Of those, only (1) actually does anything interesting. (2) and (3) only 
print a message, and (4) appears to be trivial to fold into ->suspend(). 

What do you think about just fixing those up? 



	Pat

Re: [PATCH] Power: call save_state on PCI devices along with suspend

[Benjamin Herrenschmidt]

On Tue, 2003-09-09 at 23:04, Patrick Mochel wrote:
> > Don't we want that ? It will help if any driver currently relies on
> > the save_state callback to be called...
> 
> Bah, this patch slipped my mind. How many drivers actually use 
> ->save_state()? From a quick look, it looks like: 
> 
> 1. drivers/ide/pci/sc1200.c
> 2. drivers/net/irda/vlsi_ir.c
> 3. drivers/scsi/nsp32.c
> 4. drivers/serial/8250_pci.c
> 
> Of those, only (1) actually does anything interesting. (2) and (3) only 
> print a message, and (4) appears to be trivial to fold into ->suspend(). 
> 
> What do you think about just fixing those up? 

Well... that wouldn't help with off-tree drivers...

What I mean here is that our PCI driver API defines save_state, we shall
either "support" it some way, or get rid of it completely... but then we
lose the ability to move a PCI driver back & forth with 2.4 ... (do we
care ?)

If you prefer just fixing those 4 ones, then let's get rid of the
save_state field in pci_driver completely...

Ben.

Re: [PATCH] Power: call save_state on PCI devices along with suspend

[Russell King]

On Tue, Sep 09, 2003 at 11:09:32PM +0200, Benjamin Herrenschmidt wrote:
> On Tue, 2003-09-09 at 23:04, Patrick Mochel wrote:
> > ->save_state()? From a quick look, it looks like: 
> > 4. drivers/serial/8250_pci.c

This doesn't use save_state anymore.

-- 
Russell King (rmk@arm.linux.org.uk)	http://www.arm.linux.org.uk/personal/
Linux kernel maintainer of:
  2.6 ARM Linux   - http://www.arm.linux.org.uk/
  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
  2.6 Serial core

Re: [PATCH] Power: call save_state on PCI devices along with suspend

[Patrick Mochel]

> Well... that wouldn't help with off-tree drivers...

But, we don't care about out-of-tree drivers, right? :)

> What I mean here is that our PCI driver API defines save_state, we shall
> either "support" it some way, or get rid of it completely... but then we
> lose the ability to move a PCI driver back & forth with 2.4 ... (do we
> care ?)

I think the addition of the method was a mistake and it should be fixed 
up. The fact there are only those 4 users should make it trivial in both 
2.6 and 2.4 to do so. 

> If you prefer just fixing those 4 ones, then let's get rid of the
> save_state field in pci_driver completely...

I completely agree. 


	Pat