2003-09-09 18:42:24

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: [PATCH] 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.

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;



2003-09-09 21:10:17

by Benjamin Herrenschmidt

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

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.


2003-09-09 20:57:52

by Patrick Mochel

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


> 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

2003-09-09 21:23:31

by Russell King

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

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 ([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

2003-09-09 21:40:46

by Patrick Mochel

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


> 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