Adapt drivers to type-safe pci

Adapt drivers to type-safe pci

[Pavel Machek]

Hi!

This adapts few drivers to type-safe pci powermanagment. I introduced
device_to_pci_state helper that will be usefull to few drivers... Does
it look okay? If so, Patrick, please apply.

[device_to_pci_power will have to change in near future. Still that's
better than changing 30 drivers.] 
								Pavel

--- linux.middle/drivers/media/video/bttv-driver.c	2004-10-19 14:38:02.000000000 +0200
+++ linux/drivers/media/video/bttv-driver.c	2004-11-01 21:25:09.000000000 +0100
@@ -3945,7 +3945,7 @@
 
 	/* save pci state */
 	pci_save_state(pci_dev, btv->state.pci_cfg);
-	if (0 != pci_set_power_state(pci_dev, state)) {
+	if (0 != pci_set_power_state(pci_dev, device_to_pci_power(pci_dev, state))) {
 		pci_disable_device(pci_dev);
 		btv->state.disabled = 1;
 	}
@@ -3964,7 +3964,7 @@
 		pci_enable_device(pci_dev);
 		btv->state.disabled = 0;
 	}
-	pci_set_power_state(pci_dev, 0);
+	pci_set_power_state(pci_dev, PCI_D0);
 	pci_restore_state(pci_dev, btv->state.pci_cfg);
 
 	/* restore bt878 state */
--- linux.middle/drivers/net/8139too.c	2004-10-19 14:38:06.000000000 +0200
+++ linux/drivers/net/8139too.c	2004-11-01 20:41:29.000000000 +0100
@@ -2608,7 +2608,7 @@
 
 	spin_unlock_irqrestore (&tp->lock, flags);
 
-	pci_set_power_state (pdev, 3);
+	pci_set_power_state (pdev, PCI_D3hot);
 
 	return 0;
 }
@@ -2622,7 +2622,7 @@
 	pci_restore_state (pdev, tp->pci_state);
 	if (!netif_running (dev))
 		return 0;
-	pci_set_power_state (pdev, 0);
+	pci_set_power_state (pdev, PCI_D0);
 	rtl8139_init_ring (dev);
 	rtl8139_hw_start (dev);
 	netif_device_attach (dev);
--- linux.middle/drivers/net/via-rhine.c	2004-10-21 12:32:56.000000000 +0200
+++ linux/drivers/net/via-rhine.c	2004-11-01 20:35:30.000000000 +0100
@@ -1974,7 +1974,7 @@
         if (request_irq(dev->irq, rhine_interrupt, SA_SHIRQ, dev->name, dev))
 		printk(KERN_ERR "via-rhine %s: request_irq failed\n", dev->name);
 
-	ret = pci_set_power_state(pdev, 0);
+	ret = pci_set_power_state(pdev, PCI_D0);
 	if (debug > 1)
 		printk(KERN_INFO "%s: Entering power state D0 %s (%d).\n",
 			dev->name, ret ? "failed" : "succeeded", ret);
--- linux.middle/include/linux/pci.h	2004-10-25 23:25:41.000000000 +0200
+++ linux/include/linux/pci.h	2004-11-01 21:24:17.000000000 +0100
@@ -488,6 +488,16 @@
 #define PCI_D3hot	((pci_power_t __force) 3)
 #define PCI_D3cold	((pci_power_t __force) 4)
 
+static inline pci_power_t device_to_pci_power(struct pci_dev *dev, u32 state)
+{
+	switch (state) {
+	case 0:	return PCI_D0;
+	case 2: return PCI_D2;
+	case 3: return PCI_D3hot;
+	default: BUG();
+	}
+}
+
 /*
  * The pci_dev structure is used to describe PCI devices.
  */

-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!

Re: [linux-pm] Adapt drivers to type-safe pci

[David Brownell]

[-- Attachment #1: Type: text/plain, Size: 457 bytes --]

On Monday 01 November 2004 12:26, Pavel Machek wrote:
> Hi!
> 
> This adapts few drivers to type-safe pci powermanagment. I introduced
> device_to_pci_state helper that will be usefull to few drivers... Does
> it look okay? 

I'd rather have something like the attached patch.
Fixed policy mappings are generically broken; what
about devices that don't support PCI_D2?

This should I guess use the new "pci_power_t", but
I'm not that current yet.  

- Dave

[-- Attachment #2: Diff --]
[-- Type: text/x-diff, Size: 1972 bytes --]

--- 1.73/drivers/pci/pci.c	2004-10-06 09:42:55 -07:00
+++ edited/drivers/pci/pci.c	2004-11-01 16:55:48 -08:00
@@ -229,7 +229,7 @@
 /**
  * pci_set_power_state - Set the power state of a PCI device
  * @dev: PCI device to be suspended
- * @state: Power state we're entering
+ * @state: PCI power state (D0, D1, D2, D3hot, D3cold) we're entering
  *
  * Transition a device to a new power state, using the Power Management 
  * Capabilities in the device's config space.
@@ -298,6 +298,54 @@
 
 	return 0;
 }
+
+/**
+ * pci_choose_state - Choose the power state of a PCI device
+ * @dev: PCI device to be suspended
+ * @state: target sleep state for the whole system
+ *
+ * If the device doesn't support PCI power management, this does nothing.
+ * Otherwise it puts the device in a PCI power state that it supports,
+ * and which for most devices is appropriate to that system sleep state.
+ * Some drivers will need to choose the PCI state themselves, calling
+ * pci_set_power_state() directly.
+ */
+void
+pci_choose_state(struct pci_dev *pdev, suspend_state_t sleepstate)
+{
+	int pm, state;
+	u16 pmc;
+
+	/* nothing to do for legacy PCI devices */
+	pm = pci_find_capability(pdev, PCI_CAP_ID_PM);
+	if (!pm)
+		return;
+
+	/* map to a PCI PM state this device supports
+	 * FIXME ACPI may know what states to use; we should probably
+	 * prefer that policy to this one.
+	 */
+	state = 3;
+	switch (sleepstate) {
+	case PM_SUSPEND_ON:
+		state = 0;
+		break;
+	case PM_SUSPEND_STANDBY:
+	case PM_SUSPEND_MEM:
+		pci_read_config_word(pdev, pm + PCI_PM_PMC, &pmc);
+		if (sleepstate == PM_SUSPEND_STANDBY
+				&& (pmc & PCI_PM_CAP_D1) != 0)
+			state = 1;
+		else if ((pmc & PCI_PM_CAP_D2) != 0)
+			state = 2;
+		break;
+	}
+
+	/* maybe go to a deeper power state */
+	if (pdev->current_state < state)
+		pci_set_power_state(pdev, state);
+}
+EXPORT_SYMBOL(pci_choose_state);
 
 /**
  * pci_save_state - save the PCI configuration space of a device before suspending

Re: [linux-pm] Adapt drivers to type-safe pci

[Benjamin Herrenschmidt]

On Mon, 2004-11-01 at 18:00 -0700, David Brownell wrote:
> On Monday 01 November 2004 12:26, Pavel Machek wrote:
> > Hi!
> > 
> > This adapts few drivers to type-safe pci powermanagment. I introduced
> > device_to_pci_state helper that will be usefull to few drivers... Does
> > it look okay? 
> 
> I'd rather have something like the attached patch.
> Fixed policy mappings are generically broken; what
> about devices that don't support PCI_D2?
> 
> This should I guess use the new "pci_power_t", but
> I'm not that current yet.  

Your snipped (please, inline, don't attach) would mean D2 is to be
preferred over D3 at all times ?

Ben.

Re: [linux-pm] Adapt drivers to type-safe pci

[Pavel Machek]

Hi!

> > This adapts few drivers to type-safe pci powermanagment. I introduced
> > device_to_pci_state helper that will be usefull to few drivers... Does
> > it look okay? 
> 
> I'd rather have something like the attached patch.
> Fixed policy mappings are generically broken; what
> about devices that don't support PCI_D2?

Point taken, this function is probably too big to be in header. 

> This should I guess use the new "pci_power_t", but
> I'm not that current yet.  

I'd still like this function to return suitable state (and do nothing
else), to make converting drivers easier.

For now suspend() only gets 0 or 3; I'd rather not change code for
now. Do we really enter PCI_D1 on standby? I believe PCI transitions
are fast enough to enter PCI_D3hot even for standby / suspend-to-ram.

								Pavel

> +void
> +pci_choose_state(struct pci_dev *pdev, suspend_state_t sleepstate)
> +{
> +	int pm, state;
> +	u16 pmc;
> +
> +	/* nothing to do for legacy PCI devices */
> +	pm = pci_find_capability(pdev, PCI_CAP_ID_PM);
> +	if (!pm)
> +		return;
> +
> +	/* map to a PCI PM state this device supports
> +	 * FIXME ACPI may know what states to use; we should probably
> +	 * prefer that policy to this one.
> +	 */
> +	state = 3;
> +	switch (sleepstate) {
> +	case PM_SUSPEND_ON:
> +		state = 0;
> +		break;
> +	case PM_SUSPEND_STANDBY:
> +	case PM_SUSPEND_MEM:
> +		pci_read_config_word(pdev, pm + PCI_PM_PMC, &pmc);
> +		if (sleepstate == PM_SUSPEND_STANDBY
> +				&& (pmc & PCI_PM_CAP_D1) != 0)
> +			state = 1;
> +		else if ((pmc & PCI_PM_CAP_D2) != 0)
> +			state = 2;
> +		break;
> +	}
> +
> +	/* maybe go to a deeper power state */
> +	if (pdev->current_state < state)
> +		pci_set_power_state(pdev, state);
> +}
> +EXPORT_SYMBOL(pci_choose_state);
>  
>  /**
>   * pci_save_state - save the PCI configuration space of a device before suspending


-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!