Skip to contentSkip to conversation

Power: call save_state on PCI devices along with suspend

PATCH In review Benjamin Herrenschmidt 3 participants 5 messages Sep 9, 2003 PCI

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Power: call save_state on PCI devices along with suspend

Hi Patrick !

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

Ben.

drivers/pci/pci-driver.c +3 -0
diff -urN linux-2.5/drivers/pci/pci-driver.c linuxppc-2.5-benh/drivers/pci/pci-driver.c
@@ -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;
#1 lore raw
Quoted 3 lines

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

#2 lore raw
Quoted 11 lines · Patrick Mochel
Quoted 2 lines

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

Quoted 4 lines

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.

#3 lore raw
Quoted 3 lines · Benjamin Herrenschmidt
Quoted 2 lines · Patrick Mochel

->save_state()? From a quick look, it looks like:
4. drivers/serial/8250_pci.c

This doesn't use save_state anymore.

Signature · 5 lines
Russell King ([email protected])	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
#4 lore raw
In reply to Benjamin Herrenschmidt
Quoted 2 lines

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

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

Quoted 4 lines

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.

Quoted 2 lines

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