Skip to contentSkip to conversation

More powermanagment hooks for pci

PATCH Pavel Machek 4 participants 6 messages Jul 20, 2003 – Jul 22, 2003 Kbuild

lore
Share & tools
lore

Pavel Machek

author
#0 lore raw

Hi!

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

usr/src/linux/drivers/pci/pci-driver.c renamed from usr/src/tmp/linux/drivers/pci/pci-driver.c +3 -5
@@ -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/linux/include/linux/pci.h renamed from usr/src/tmp/linux/include/linux/pci.h +1 -0
@@ -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 */
Signature · 2 lines
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]
#1 lore raw
Quoted 4 lines · 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.

thanks,

greg k-h

#2 lore raw

Hi!

Quoted 9 lines
Quoted 2 lines

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

Signature · 2 lines
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]
#3 lore raw
Quoted 4 lines

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..

Quoted 2 lines

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

#4 lore raw

Hi!

Quoted 5 lines
Quoted 3 lines

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.

Quoted 8 lines
Quoted 2 lines

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

Signature · 2 lines
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]
#5 lore raw
Quoted 8 lines
Quoted 6 lines

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