Runtime power management on ipw2100
Subsystems (3)
RFC 1/1 Runtime power management on ipw2100
Based on previous discussions, I've implemented a rough attempt at providing some level of basic runtime power management on the ipw2100 chipset. This patch does the following:
1) On load, it initialises the hardware and then quiesces it again
2) On interface up, it powers the hardware back up
3) On interface down, it powers the hardware down and puts the chip in D3
4) It attempts to behave correctly over suspend/resume - ie, if the interface was down beforehand, it will ensure that the chip is powered down
The degree of bouncing between power states is probably unnecessary, but I don't have access to any docs so I've just gone for the most conservative approach that still seems to work. It's also possible that kill-switch handling isn't quite right - my machine doesn't have a working kill switch, so I haven't been able to test that.
Anyway, does this approach look broadly correct to people? If so, I'll look into doing something similar for the other hardware I have access to.
The situation is slightly more complicated for wired interfaces. As previously discussed, we potentially want three interface states (on, low power, off) with the intermediate one powering down as much of the hardware as possible while still implementing link detection. Is there an existing state flag that can be appropriated for this?
---
drivers/net/wireless/ipw2100.c +58 -26
@@ -5771,8 +5771,36 @@ static int ipw2100_open(struct net_device *dev) {struct ipw2100_priv *priv =ieee80211_priv (dev);unsigned long flags; +int err, val;IPW_DEBUG_INFO ("dev->open\n" ); +mutex_lock (&priv->action_mutex); + +pci_set_power_state (priv->pci_dev,PCI_D0 ); + err =pci_enable_device (priv->pci_dev); + +if (err) { + printk(KERN_WARNING DRV_NAME + "Error calling pci_enable_device.\n"); +return err; + } + +pci_restore_state (priv->pci_dev); + +pci_read_config_dword (priv->pci_dev,0x40 , &val); +if ((val &0x0000ff00 ) !=0 ) +pci_write_config_dword (priv->pci_dev,0x40 , val &0xffff00ff ); + + err =ipw2100_up (priv,0 ); + +if (err) { + printk(KERN_WARNING DRV_NAME + " Error bringing card up.\n"); +pci_disable_device (priv->pci_dev); +pci_set_power_state (priv->pci_dev, PCI_D3hot); +return err; + } +spin_lock_irqsave (&priv->low_lock, flags);if (priv->status &STATUS_ASSOCIATED ) {netif_carrier_on (dev);
@@ -5780,6 +5808,8 @@ static int ipw2100_open(struct net_device *dev) }spin_unlock_irqrestore (&priv->low_lock, flags); +mutex_unlock (&priv->action_mutex); +return 0 ; }
@@ -5814,6 +5844,19 @@ static int ipw2100_close(struct net_device *dev) }spin_unlock_irqrestore (&priv->low_lock, flags); +ipw2100_down (priv); + +#ifdef ACPI_CSTATE_LIMIT_DEFINED +if (priv->config &CFG_C3_DISABLED ) { +IPW_DEBUG_INFO (": Resetting C3 transitions.\n" ); +acpi_set_cstate_limit (priv->cstate_limit); + priv->config &= ~CFG_C3_DISABLED ; + } +#endif + +pci_disable_device (priv->pci_dev); +pci_set_power_state (priv->pci_dev, PCI_D3hot); +IPW_DEBUG_INFO ("exit\n" );return 0 ;
@@ -6208,6 +6251,7 @@ static int ipw2100_pci_init_one(struct pci_dev *pci_dev,pci_write_config_dword (pci_dev,0x40 , val &0xffff00ff );pci_set_power_state (pci_dev,PCI_D0 ); +pci_save_state (pci_dev);if (!ipw2100_hw_is_adapter_in_system (dev)) { printk(KERN_WARNING DRV_NAME
@@ -6274,23 +6318,8 @@ static int ipw2100_pci_init_one(struct pci_dev *pci_dev,if (err) goto fail_unlock; -/* If the RF Kill switch is disabled, go ahead and complete the -* startup sequence */ -if (!(priv->status &STATUS_RF_KILL_MASK )) { -/* Enable the adapter - sends HOST_COMPLETE */ -if (ipw2100_enable_adapter (priv)) { - printk(KERN_WARNING DRV_NAME -": %s: failed in call to enable adapter.\n" , - priv->net_dev->name); -ipw2100_hw_stop_adapter (priv); - err = -EIO ; - goto fail_unlock; - } - -/* Start a scan . . . */ -ipw2100_set_scan_options (priv); -ipw2100_start_scan (priv); - } +pci_disable_device (pci_dev); +pci_set_power_state (pci_dev, PCI_D3hot);IPW_DEBUG_INFO ("exit\n" );
@@ -6353,8 +6382,6 @@ static void __devexit ipw2100_pci_remove_one(struct pci_dev *pci_dev)if (ipw2100_firmware.version)ipw2100_release_firmware (priv, &ipw2100_firmware);#endif -/* Take down the hardware */ -ipw2100_down (priv);/* Release the mutex so that the network subsystem can * complete any needed calls into the driver... */
@@ -6384,7 +6411,6 @@ static void __devexit ipw2100_pci_remove_one(struct pci_dev *pci_dev) }pci_release_regions (pci_dev); -pci_disable_device (pci_dev);IPW_DEBUG_INFO ("exit\n" ); }
@@ -6398,7 +6424,7 @@ static int ipw2100_suspend(struct pci_dev *pci_dev, pm_message_t state)IPW_DEBUG_INFO ("%s: Going into suspend...\n" , dev->name);mutex_lock (&priv->action_mutex); -if (priv->status &STATUS_INITIALIZED ) { +if (priv->status &STATUS_INITIALIZED && dev->flags &IFF_UP ) {/* Take down the device; powers it off, etc. */ ipw2100_down (priv); }
@@ -6406,9 +6432,11 @@ static int ipw2100_suspend(struct pci_dev *pci_dev, pm_message_t state)/* Remove the PRESENT state of the device */ netif_device_detach (dev); -pci_save_state (pci_dev); -pci_disable_device (pci_dev); -pci_set_power_state (pci_dev, PCI_D3hot); +if (dev->flags &IFF_UP ) { +pci_save_state (pci_dev); +pci_disable_device (pci_dev); +pci_set_power_state (pci_dev, PCI_D3hot); + }mutex_unlock (&priv->action_mutex);
@@ -6453,9 +6481,13 @@ static int ipw2100_resume(struct pci_dev *pci_dev)netif_device_attach (dev);/* Bring the device back up */ -if (!(priv->status &STATUS_RF_KILL_SW )) +if (!(priv->status &STATUS_RF_KILL_SW ) && (dev->flags &IFF_UP ))ipw2100_up (priv,0 ); - +else { +pci_disable_device (pci_dev); +pci_set_power_state (pci_dev, PCI_D3hot); + } +mutex_unlock (&priv->action_mutex);return 0 ;
Signature · 7 lines
Matthew Garrett | [email protected] ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys - and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
Quoted 11 lines · ext Matthew Garrett
Based on previous discussions, I've implemented a rough attempt at providing some level of basic runtime power management on the ipw2100 chipset. This patch does the following:
1) On load, it initialises the hardware and then quiesces it again
2) On interface up, it powers the hardware back up
3) On interface down, it powers the hardware down and puts the chip in D3
4) It attempts to behave correctly over suspend/resume - ie, if the interface was down beforehand, it will ensure that the chip is powered down
This isn't quite runtime power management in my way of thinking. I sort of expected this to be a fundamental characteristic of well-behaved drivers, but then I don't have too much experience with PCI on x86 platforms.
With runtime power management, I would try to put the device into low power states when it is UP. That's what drivers on the OMAP platform (N800, 770) do.
What is the latency in changing between different PCI power states for peripherals?
Would it be possible e.g. to put the peripheral into a low power state after each Tx/Rx (with reasonable hyteresis)?
<snip>
Quoted 4 lines
The situation is slightly more complicated for wired interfaces. As previously discussed, we potentially want three interface states (on, low power, off) with the intermediate one powering down as much of the hardware as possible while still implementing link detection.
And this low power state is what the HW should be in all the time, except when it has work to do.
Regards,
Amit
Quoted 2 lines · Amit Kucheria
What is the latency in changing between different PCI power states for peripherals?
I'm not sure in the general case, but the power-down path for the ipw2100 involves a static wait of 100ms in ipw2100_hw_stop_adapter().
Quoted 2 lines
Would it be possible e.g. to put the peripheral into a low power state after each Tx/Rx (with reasonable hyteresis)?
Most wireless drivers support some degree of power management at this scale, but (in ipw2100 at least) it's implemented in the firmware so I have absolutely no idea what it's actually doing.
Quoted 9 lines
<snip>
Quoted 4 lines
The situation is slightly more complicated for wired interfaces. As previously discussed, we potentially want three interface states (on, low power, off) with the intermediate one powering down as much of the hardware as possible while still implementing link detection.
And this low power state is what the HW should be in all the time, except when it has work to do.
PCI seems to require a delay of 10ms when sequencing from D3 to D0, which probably isn't acceptable latency for an "up" state. While there's definitely a benefit to the sort of PM you're describing (it's a model we've already started using on the desktop as far as the CPU goes), I think we still want to be able to expose as much power saving as possible.
Signature · 7 lines
Matthew Garrett | [email protected] ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys - and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
Quoted 3 lines · Matthew Garrett
PCI seems to require a delay of 10ms when sequencing from D3 to D0, which probably isn't acceptable latency for an "up" state.
It might be if the interface has been idle for some time (and the delay is not busy looping of course)
How idle should be user configurable.
But the idle detection needed for that should be probably higher up stack or at least some common library functions.
-Andi
Quoted 7 lines · Andi Kleen
Quoted 3 lines · Matthew Garrett
PCI seems to require a delay of 10ms when sequencing from D3 to D0, which probably isn't acceptable latency for an "up" state.
It might be if the interface has been idle for some time (and the delay is not busy looping of course)
Hm. How would this interact with receiving packets?
Signature · 7 lines
Matthew Garrett | [email protected] ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys - and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
Quoted 10 lines · Matthew Garrett
Quoted 7 lines · Andi Kleen
Quoted 3 lines · Matthew Garrett
PCI seems to require a delay of 10ms when sequencing from D3 to D0, which probably isn't acceptable latency for an "up" state.
It might be if the interface has been idle for some time (and the delay is not busy looping of course)
Hm. How would this interact with receiving packets?
The hardware will hopefully have support to wake itself up when that happens.
-Andi
Quoted 7 lines · ext Andi Kleen
Quoted 6 lines · Matthew Garrett
Quoted 5 lines · Andi Kleen
Quoted 3 lines · Matthew Garrett
PCI seems to require a delay of 10ms when sequencing from D3 to D0, which probably isn't acceptable latency for an "up" state.
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
It might be if the interface has been idle for some time (and the delay is not busy looping of course)
Hm. How would this interact with receiving packets?
The hardware will hopefully have support to wake itself up when that happens.
Yes. Low power states without ability to respond to wakeup interrupts would be broken behaviour generally.
Quoted 11 lines · Amit Kucheria
Quoted 10 lines · ext Andi Kleen
Quoted 9 lines · Matthew Garrett
Quoted 7 lines · Andi Kleen
Quoted 3 lines · Matthew Garrett
PCI seems to require a delay of 10ms when sequencing from D3 to D0, which probably isn't acceptable latency for an "up" state.
It might be if the interface has been idle for some time (and the delay is not busy looping of course)
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
Hm. How would this interact with receiving packets?
The hardware will hopefully have support to wake itself up when that happens.
Yes. Low power states without ability to respond to wakeup interrupts would be broken behaviour generally.
Do you realy expect wifi to save significant ammount of power, while still listening for packets on wireless network?
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Am Mittwoch, 31. Januar 2007 14:04 schrieb Pavel Machek:
Quoted 14 lines · Amit Kucheria
Quoted 13 lines · ext Andi Kleen
Quoted 12 lines · Matthew Garrett
Quoted 10 lines · Andi Kleen
Quoted 7 lines
Quoted 3 lines · Matthew Garrett
PCI seems to require a delay of 10ms when sequencing from D3 to D0, which probably isn't acceptable latency for an "up" state.
It might be if the interface has been idle for some time (and the delay is not busy looping of course)
Hm. How would this interact with receiving packets?
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
The hardware will hopefully have support to wake itself up when that happens.
Yes. Low power states without ability to respond to wakeup interrupts would be broken behaviour generally.
Do you realy expect wifi to save significant ammount of power, while still listening for packets on wireless network?
It has a managed mode which gives each station a timeslot. Outside those slots you could power down, if you can do it quickly.
Regards
Oliver
Quoted 14 lines · Pavel Machek
Quoted 13 lines · Amit Kucheria
Quoted 12 lines · ext Andi Kleen
Quoted 10 lines · Matthew Garrett
Quoted 7 lines · Andi Kleen
Quoted 3 lines · Matthew Garrett
PCI seems to require a delay of 10ms when sequencing from D3 to D0, which probably isn't acceptable latency for an "up" state.
It might be if the interface has been idle for some time (and the delay is not busy looping of course)
Hm. How would this interact with receiving packets?
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
The hardware will hopefully have support to wake itself up when that happens.
Yes. Low power states without ability to respond to wakeup interrupts would be broken behaviour generally.
Do you realy expect wifi to save significant ammount of power, while still listening for packets on wireless network?
Yes, that's what 802.11 PSM is for: your 802.11 hardware can sleep for significant amounts of time and wake up periodically, then check from the beacons if the AP has been buffering some frames for it. The duty cycle can be quite low, depending on how often you decide to wake up and catch beacon frames.
Cheers,
Samuel.
Quoted 10 lines · ext Pavel Machek
Quoted 3 lines · Amit Kucheria
Yes. Low power states without ability to respond to wakeup interrupts would be broken behaviour generally.
Do you realy expect wifi to save significant ammount of power, while still listening for packets on wireless network?
PavelFrom N800 specs (not meant as a shameless plug):
Continuous WLAN browsing: upto 3.5 hrs
Always Online WLAN (idle but respond to VoIP calls): upto 2 days Standby time(no WLAN): upto 12 days
So with the right hardware, yes, you can listen and still save power.
Regards,
Amit
Hi!
Quoted 6 lines
Quoted 5 lines
Quoted 2 lines
Yes. Low power states without ability to respond to wakeup interrupts would be broken behaviour generally.
Do you realy expect wifi to save significant ammount of power, while still listening for packets on wireless network?
Quoted 7 lines
Quoted 1 line
From N800 specs (not meant as a shameless plug):
Continuous WLAN browsing: upto 3.5 hrs
Always Online WLAN (idle but respond to VoIP calls): upto 2 days Standby time(no WLAN): upto 12 daysSo with the right hardware, yes, you can listen and still save power.
Okay, but I'm somehow not sure if ipw2100 can do this kind of tricks. Yes, n800 is likely very nice toy :-).
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Quoted 2 lines · Pavel Machek
Okay, but I'm somehow not sure if ipw2100 can do this kind of tricks. Yes, n800 is likely very nice toy :-).
Well, to an extent - if you use the wext power features, you can get it into a relatively low-power state at the cost of some performance. It's not as good as powering down the hardware completely, though.
Signature · 1 line
Matthew Garrett | [email protected]
Quoted 7 lines · ext Matthew Garrett
Quoted 2 lines · Amit Kucheria
What is the latency in changing between different PCI power states for peripherals?
I'm not sure in the general case, but the power-down path for the ipw2100 involves a static wait of 100ms in
ipw2100_hw_stop_adapter().
Ouch!
<snip>
Quoted 2 lines
PCI seems to require a delay of 10ms when sequencing from D3 to D0, which probably isn't acceptable latency for an "up" state. While there's
It might be acceptable to users who are just browsing the web, but not to users who are streaming music from internet radio stations. IOW, expose the interface (disabled by default) and leave policy to userspace through a library.
Quoted 4 lines
definitely a benefit to the sort of PM you're describing (it's a model we've already started using on the desktop as far as the CPU goes), I think we still want to be able to expose as much power saving as possible.
I agree that this is a good start.
Regards,
Amit
Hi!
Quoted 11 lines
Based on previous discussions, I've implemented a rough attempt at providing some level of basic runtime power management on the ipw2100 chipset. This patch does the following:
1) On load, it initialises the hardware and then quiesces it again
2) On interface up, it powers the hardware back up
3) On interface down, it powers the hardware down and puts the chip in D3
4) It attempts to behave correctly over suspend/resume - ie, if the interface was down beforehand, it will ensure that the chip is powered down
I'm not a wireless guru, but patch certainly looks okay to me.
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Quoted 12 lines · Matthew Garrett
Based on previous discussions, I've implemented a rough attempt at providing some level of basic runtime power management on the ipw2100 chipset. This patch does the following:
1) On load, it initialises the hardware and then quiesces it again
2) On interface up, it powers the hardware back up
3) On interface down, it powers the hardware down and puts the chip in D3
4) It attempts to behave correctly over suspend/resume - ie, if the interface was down beforehand, it will ensure that the chip is powered down
Quoted 1 line
From my understanding, the intention of this patch is to defer the
device self-initialization work (including firmware loading) from netdev initialization time to netdev open time (ifconfig up) and de-initialize the device when it is not being used (ifconfig down). This saves power during the time the driver is loaded but the interface is not open.
You should remove ipw2100_up() from ipw2100_net_init() which is netdev->init() since it will be called in ->open() in your patch. I'd also suggest you to request_irq()/free_irq() in the netdev ->open() and ->close() in case the device shares IRQ with other devices, the interrupt handler should not be invoked anyway.
Thanks,
-yi
Quoted 7 lines · Zhu Yi
Quoted 1 line · Matthew Garrett
From my understanding, the intention of this patch is to defer the
device self-initialization work (including firmware loading) from netdev initialization time to netdev open time (ifconfig up) and de-initialize the device when it is not being used (ifconfig down). This saves power during the time the driver is loaded but the interface is not open.
That's correct.
Quoted 5 lines
You should remove
ipw2100_up()fromipw2100_net_init()which isnetdev->init()since it will be called in ->open() in your patch. I'd also suggest you to request_irq()/free_irq() in the netdev ->open() and ->close() in case the device shares IRQ with other devices, the interrupt handler should not be invoked anyway.
I've removed net_init() entirely, since all it did was call ipw2100_up(). I'm reluctant to drop the IRQ because PCMCIA seems to have a nasty habit of grabbing free looking IRQs and setting them to be edge triggered, which would obviously be bad.
How's this patch?
drivers/net/wireless/ipw2100.c +61 -34
@@ -1809,13 +1809,6 @@ static int ipw2100_up(struct ipw2100_priv *priv, int deferred)return rc; } -/* Called by register_netdev() */ -static int ipw2100_net_init (struct net_device *dev) -{ -struct ipw2100_priv *priv =ieee80211_priv (dev); -return ipw2100_up (priv,1 ); -} -static void ipw2100_down (struct ipw2100_priv *priv) {unsigned long flags;
@@ -5771,8 +5764,38 @@ static int ipw2100_open(struct net_device *dev) {struct ipw2100_priv *priv =ieee80211_priv (dev);unsigned long flags; +int err, val;IPW_DEBUG_INFO ("dev->open\n" ); +mutex_lock (&priv->action_mutex); + +pci_set_power_state (priv->pci_dev,PCI_D0 ); + err =pci_enable_device (priv->pci_dev); + +if (err) { + printk(KERN_WARNING DRV_NAME + "Error calling pci_enable_device.\n"); +mutex_unlock (&priv->action_mutex); +return err; + } + +pci_restore_state (priv->pci_dev); + +pci_read_config_dword (priv->pci_dev,0x40 , &val); +if ((val &0x0000ff00 ) !=0 ) +pci_write_config_dword (priv->pci_dev,0x40 , val &0xffff00ff ); + + err =ipw2100_up (priv,0 ); + +if (err) { + printk(KERN_WARNING DRV_NAME + " Error bringing card up.\n"); +pci_disable_device (priv->pci_dev); +pci_set_power_state (priv->pci_dev, PCI_D3hot); +mutex_unlock (&priv->action_mutex); +return err; + } +spin_lock_irqsave (&priv->low_lock, flags);if (priv->status &STATUS_ASSOCIATED ) {netif_carrier_on (dev);
@@ -5780,6 +5803,8 @@ static int ipw2100_open(struct net_device *dev) }spin_unlock_irqrestore (&priv->low_lock, flags); +mutex_unlock (&priv->action_mutex); +return 0 ; }
@@ -5814,6 +5839,19 @@ static int ipw2100_close(struct net_device *dev) }spin_unlock_irqrestore (&priv->low_lock, flags); +ipw2100_down (priv); + +#ifdef ACPI_CSTATE_LIMIT_DEFINED +if (priv->config &CFG_C3_DISABLED ) { +IPW_DEBUG_INFO (": Resetting C3 transitions.\n" ); +acpi_set_cstate_limit (priv->cstate_limit); + priv->config &= ~CFG_C3_DISABLED ; + } +#endif + +pci_disable_device (priv->pci_dev); +pci_set_power_state (priv->pci_dev, PCI_D3hot); +IPW_DEBUG_INFO ("exit\n" );return 0 ;
@@ -6021,7 +6059,6 @@ static struct net_device *ipw2100_alloc_device(struct pci_dev *pci_dev, dev->open = ipw2100_open; dev->stop = ipw2100_close; - dev->init = ipw2100_net_init; dev->ethtool_ops = &ipw2100_ethtool_ops; dev->tx_timeout = ipw2100_tx_timeout; dev->wireless_handlers = &ipw2100_wx_handler_def;
@@ -6208,6 +6245,7 @@ static int ipw2100_pci_init_one(struct pci_dev *pci_dev,pci_write_config_dword (pci_dev,0x40 , val &0xffff00ff );pci_set_power_state (pci_dev,PCI_D0 ); +pci_save_state (pci_dev);if (!ipw2100_hw_is_adapter_in_system (dev)) { printk(KERN_WARNING DRV_NAME
@@ -6274,23 +6312,9 @@ static int ipw2100_pci_init_one(struct pci_dev *pci_dev,if (err) goto fail_unlock; -/* If the RF Kill switch is disabled, go ahead and complete the -* startup sequence */ -if (!(priv->status &STATUS_RF_KILL_MASK )) { -/* Enable the adapter - sends HOST_COMPLETE */ -if (ipw2100_enable_adapter (priv)) { - printk(KERN_WARNING DRV_NAME -": %s: failed in call to enable adapter.\n" , - priv->net_dev->name); -ipw2100_hw_stop_adapter (priv); - err = -EIO ; - goto fail_unlock; - } - -/* Start a scan . . . */ -ipw2100_set_scan_options (priv); -ipw2100_start_scan (priv); - } +ipw2100_down (priv); +pci_disable_device (pci_dev); +pci_set_power_state (pci_dev, PCI_D3hot);IPW_DEBUG_INFO ("exit\n" );
@@ -6353,8 +6377,6 @@ static void __devexit ipw2100_pci_remove_one(struct pci_dev *pci_dev)if (ipw2100_firmware.version)ipw2100_release_firmware (priv, &ipw2100_firmware);#endif -/* Take down the hardware */ -ipw2100_down (priv);/* Release the mutex so that the network subsystem can * complete any needed calls into the driver... */
@@ -6384,7 +6406,6 @@ static void __devexit ipw2100_pci_remove_one(struct pci_dev *pci_dev) }pci_release_regions (pci_dev); -pci_disable_device (pci_dev);IPW_DEBUG_INFO ("exit\n" ); }
@@ -6398,7 +6419,7 @@ static int ipw2100_suspend(struct pci_dev *pci_dev, pm_message_t state)IPW_DEBUG_INFO ("%s: Going into suspend...\n" , dev->name);mutex_lock (&priv->action_mutex); -if (priv->status &STATUS_INITIALIZED ) { +if (priv->status &STATUS_INITIALIZED && dev->flags &IFF_UP ) {/* Take down the device; powers it off, etc. */ ipw2100_down (priv); }
@@ -6406,9 +6427,11 @@ static int ipw2100_suspend(struct pci_dev *pci_dev, pm_message_t state)/* Remove the PRESENT state of the device */ netif_device_detach (dev); -pci_save_state (pci_dev); -pci_disable_device (pci_dev); -pci_set_power_state (pci_dev, PCI_D3hot); +if (dev->flags &IFF_UP ) { +pci_save_state (pci_dev); +pci_disable_device (pci_dev); +pci_set_power_state (pci_dev, PCI_D3hot); + }mutex_unlock (&priv->action_mutex);
@@ -6453,9 +6476,13 @@ static int ipw2100_resume(struct pci_dev *pci_dev)netif_device_attach (dev);/* Bring the device back up */ -if (!(priv->status &STATUS_RF_KILL_SW )) +if (!(priv->status &STATUS_RF_KILL_SW ) && (dev->flags &IFF_UP ))ipw2100_up (priv,0 ); - +else { +pci_disable_device (pci_dev); +pci_set_power_state (pci_dev, PCI_D3hot); + } +mutex_unlock (&priv->action_mutex);return 0 ;
Signature · 1 line
Matthew Garrett | [email protected]
Quoted 3 lines · Matthew Garrett
I'm reluctant to drop the IRQ because PCMCIA seems to have a nasty habit of grabbing free looking IRQs and setting them to be edge triggered, which would obviously be bad.
Can you provide more details for this problem and why we should workaround it here instead of fixing the PCMCIA driver (hardware)?
A generic requirement for dynamic power management is the hardware resource should not be touched when you put it in a low power state. In this case, ipw2100 interrupt handler is possibly called (for example, it shares the same IRQ with other devies) and it touches the card register, which will cause problem. A possible workaround is to check the suspend state in the beginning of the ISR and returns IRQ_NONE. But I think freeing the irq handler before suspend should be the right way to go.
Thanks,
-yi
-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier. Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
Quoted 2 lines · Zhu Yi
A generic requirement for dynamic power management is the hardware resource should not be touched when you put it in a low power state.
That is in no way a "generic" requirement. It might apply specifically to one ipw2100 low power state ... but "in general" devices may support more than one low power state, with different levels of functionality. Not all of those levels necessarily disallow touching the hardware.
Quoted 2 lines
But I think
freeing the irq handler before suspend should be the right way to go.
Some folk like that model a lot for shared IRQs. It shouldn't matter for non-sharable ones.
- Dave