More powermanagment hooks for pci

More powermanagment hooks for pci

[Pavel Machek]

Hi!

Apparently, some pci driver (8390too) need to do something at poweron
before interrupts are enabled. Please apply,
								Pavel 

--- /usr/src/tmp/linux/drivers/pci/pci-driver.c	2003-07-06 20:07:38.000000000 +0200
+++ /usr/src/linux/drivers/pci/pci-driver.c	2003-07-20 22:42:26.000000000 +0200
@@ -179,11 +179,9 @@
 	struct pci_dev * pci_dev = to_pci_dev(dev);
 
 	if (pci_dev->driver) {
-		/* We may not call PCI drivers resume at
-		   RESUME_POWER_ON because interrupts are not yet
-		   working at that point. Calling resume at
-		   RESUME_RESTORE_STATE seems like solution. */
-		if (level == RESUME_RESTORE_STATE && pci_dev->driver->resume)
+	        if (level == RESUME_POWER_ON && pci_dev->driver->power_on)
+			pci_dev->driver->power_on(pci_dev);
+		else if (level == RESUME_RESTORE_STATE && pci_dev->driver->resume)
 			pci_dev->driver->resume(pci_dev);
 	}
 	return 0;
--- /usr/src/tmp/linux/include/linux/pci.h	2003-07-11 21:38:47.000000000 +0200
+++ /usr/src/linux/include/linux/pci.h	2003-07-20 22:42:26.000000000 +0200
@@ -512,6 +512,7 @@
 	void (*remove) (struct pci_dev *dev);	/* Device removed (NULL if not a hot-plug capable driver) */
 	int  (*save_state) (struct pci_dev *dev, u32 state);    /* Save Device Context */
 	int  (*suspend) (struct pci_dev *dev, u32 state);	/* Device suspended */
+	int  (*power_on) (struct pci_dev *dev);	                /* Device power on */
 	int  (*resume) (struct pci_dev *dev);	                /* Device woken up */
 	int  (*enable_wake) (struct pci_dev *dev, u32 state, int enable);   /* Enable wake event */
 

-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: More powermanagment hooks for pci

[Greg KH]

On Sun, Jul 20, 2003 at 11:29:43PM +0200, Pavel Machek wrote:
> Hi!
> 
> Apparently, some pci driver (8390too) need to do something at poweron
> before interrupts are enabled. Please apply,

Sorry, but I'm not going to apply this.  I'm pretty sure that Pat has
some changes like this pending in his power management stuff, that I
think we should wait for.

And yes, I have the same laptop that would benifit from this patch, but
a change like this for just one driver isn't ok.

thanks,

greg k-h

Re: More powermanagment hooks for pci

[Pavel Machek]

Hi!

> > Apparently, some pci driver (8390too) need to do something at poweron
> > before interrupts are enabled. Please apply,
> 
> Sorry, but I'm not going to apply this.  I'm pretty sure that Pat has
> some changes like this pending in his power management stuff, that I
> think we should wait for.
> 
> And yes, I have the same laptop that would benifit from this patch, but
> a change like this for just one driver isn't ok.

Well, there are likely more drivers that need to quiesce PCI card
before resume. (I was wrong, 8390too does *not* need it, radeonfb
does). It looks like bug not to have the hook in the first place...

Patrick, can you comment? I was trying to add power_on hook to PCI
devices...

								Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: More powermanagment hooks for pci

[Patrick Mochel]

> Well, there are likely more drivers that need to quiesce PCI card
> before resume. (I was wrong, 8390too does *not* need it, radeonfb
> does). It looks like bug not to have the hook in the first place...

You also didn't credit the original author.. 

> Patrick, can you comment? I was trying to add power_on hook to PCI
> devices...

I know. I'm thinking of adding power_{off,on} to the core and getting rid
of the level parameter to the suspend/resume functions (like how I changed
system devices). That would require an additional hook to the PCI drivers
so that the call is propogated down to the low-level driver. If that's the
case, then we should add both to struct pci_driver at once.


	-pat

Re: More powermanagment hooks for pci

[Pavel Machek]

Hi!

> > Well, there are likely more drivers that need to quiesce PCI card
> > before resume. (I was wrong, 8390too does *not* need it, radeonfb
> > does). It looks like bug not to have the hook in the first place...
> 
> You also didn't credit the original author.. 

Sorry, Ole was the original author AFAIK.

> > Patrick, can you comment? I was trying to add power_on hook to PCI
> > devices...
> 
> I know. I'm thinking of adding power_{off,on} to the core and getting rid
> of the level parameter to the suspend/resume functions (like how I changed
> system devices). That would require an additional hook to the PCI drivers
> so that the call is propogated down to the low-level driver. If that's the
> case, then we should add both to struct pci_driver at once.

Should I modify patch to add both?
								Pavel

-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: More powermanagment hooks for pci

[Patrick Mochel]

> > I know. I'm thinking of adding power_{off,on} to the core and getting rid
> > of the level parameter to the suspend/resume functions (like how I changed
> > system devices). That would require an additional hook to the PCI drivers
> > so that the call is propogated down to the low-level driver. If that's the
> > case, then we should add both to struct pci_driver at once.
> 
> Should I modify patch to add both?

No, don't bother yet. 


	-pat