All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC] Runtime power management on ipw2100
@ 2007-01-31  7:52 Matthew Garrett
  2007-01-31  9:13 ` Amit Kucheria
                   ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Matthew Garrett @ 2007-01-31  7:52 UTC (permalink / raw)
  To: ipw2100-devel, netdev, linux-pm

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?

---

diff --git a/drivers/net/wireless/ipw2100.c b/drivers/net/wireless/ipw2100.c
index 24ef52a..0e3c7e8 100644
--- a/drivers/net/wireless/ipw2100.c
+++ b/drivers/net/wireless/ipw2100.c
@@ -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;

-- 
Matthew Garrett | mjg59@srcf.ucam.org

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [RFC] Runtime power management on ipw2100
  2007-01-31  7:52 [RFC] Runtime power management on ipw2100 Matthew Garrett
@ 2007-01-31  9:13 ` Amit Kucheria
  2007-01-31  9:48   ` [linux-pm] " Matthew Garrett
  2007-01-31 10:39 ` Pavel Machek
  2007-02-01  1:47 ` [Ipw2100-devel] " Zhu Yi
  2 siblings, 1 reply; 19+ messages in thread
From: Amit Kucheria @ 2007-01-31  9:13 UTC (permalink / raw)
  To: ext Matthew Garrett; +Cc: ipw2100-devel, linux-pm, netdev

On Wed, 2007-01-31 at 07:52 +0000, ext Matthew Garrett wrote:
> 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>

> 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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [linux-pm] [RFC] Runtime power management on ipw2100
  2007-01-31  9:13 ` Amit Kucheria
@ 2007-01-31  9:48   ` Matthew Garrett
  2007-01-31 11:04     ` Amit Kucheria
  2007-01-31 11:13     ` Andi Kleen
  0 siblings, 2 replies; 19+ messages in thread
From: Matthew Garrett @ 2007-01-31  9:48 UTC (permalink / raw)
  To: Amit Kucheria; +Cc: ipw2100-devel, linux-pm, netdev

On Wed, Jan 31, 2007 at 11:13:07AM +0200, Amit Kucheria wrote:

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

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

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

-- 
Matthew Garrett | mjg59@srcf.ucam.org

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [linux-pm] [RFC] Runtime power management on ipw2100
  2007-01-31 11:13     ` Andi Kleen
@ 2007-01-31 10:27       ` Matthew Garrett
  2007-01-31 10:48         ` Andi Kleen
  0 siblings, 1 reply; 19+ messages in thread
From: Matthew Garrett @ 2007-01-31 10:27 UTC (permalink / raw)
  To: Andi Kleen; +Cc: ipw2100-devel, netdev, linux-pm, Amit Kucheria

On Wed, Jan 31, 2007 at 12:13:04PM +0100, Andi Kleen wrote:
> Matthew Garrett <mjg59@srcf.ucam.org> writes:
> > 
> > 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?

-- 
Matthew Garrett | mjg59@srcf.ucam.org

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [linux-pm] [RFC] Runtime power management on ipw2100
  2007-01-31  7:52 [RFC] Runtime power management on ipw2100 Matthew Garrett
  2007-01-31  9:13 ` Amit Kucheria
@ 2007-01-31 10:39 ` Pavel Machek
  2007-02-01  1:47 ` [Ipw2100-devel] " Zhu Yi
  2 siblings, 0 replies; 19+ messages in thread
From: Pavel Machek @ 2007-01-31 10:39 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: ipw2100-devel, netdev, linux-pm

Hi!

> 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
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [linux-pm] [RFC] Runtime power management on ipw2100
  2007-01-31 10:27       ` [linux-pm] " Matthew Garrett
@ 2007-01-31 10:48         ` Andi Kleen
  2007-01-31 11:53           ` Amit Kucheria
  0 siblings, 1 reply; 19+ messages in thread
From: Andi Kleen @ 2007-01-31 10:48 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: Amit Kucheria, ipw2100-devel, linux-pm, netdev

On Wednesday 31 January 2007 11:27, Matthew Garrett wrote:
> On Wed, Jan 31, 2007 at 12:13:04PM +0100, Andi Kleen wrote:
> > Matthew Garrett <mjg59@srcf.ucam.org> writes:
> > > 
> > > 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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] Runtime power management on ipw2100
  2007-01-31  9:48   ` [linux-pm] " Matthew Garrett
@ 2007-01-31 11:04     ` Amit Kucheria
  2007-01-31 11:13     ` Andi Kleen
  1 sibling, 0 replies; 19+ messages in thread
From: Amit Kucheria @ 2007-01-31 11:04 UTC (permalink / raw)
  To: ext Matthew Garrett; +Cc: ipw2100-devel, linux-pm, netdev

On Wed, 2007-01-31 at 09:48 +0000, ext Matthew Garrett wrote:
> On Wed, Jan 31, 2007 at 11:13:07AM +0200, Amit Kucheria wrote:
> 
> > 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>

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

> 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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] Runtime power management on ipw2100
  2007-01-31  9:48   ` [linux-pm] " Matthew Garrett
  2007-01-31 11:04     ` Amit Kucheria
@ 2007-01-31 11:13     ` Andi Kleen
  2007-01-31 10:27       ` [linux-pm] " Matthew Garrett
  1 sibling, 1 reply; 19+ messages in thread
From: Andi Kleen @ 2007-01-31 11:13 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: ipw2100-devel, netdev, linux-pm

Matthew Garrett <mjg59@srcf.ucam.org> writes:
> 
> 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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] Runtime power management on ipw2100
  2007-01-31 10:48         ` Andi Kleen
@ 2007-01-31 11:53           ` Amit Kucheria
  2007-01-31 13:04             ` Pavel Machek
  0 siblings, 1 reply; 19+ messages in thread
From: Amit Kucheria @ 2007-01-31 11:53 UTC (permalink / raw)
  To: ext Andi Kleen; +Cc: Matthew Garrett, netdev, linux-pm, ipw2100-devel

On Wed, 2007-01-31 at 11:48 +0100, ext Andi Kleen wrote:
> On Wednesday 31 January 2007 11:27, Matthew Garrett wrote:
> > On Wed, Jan 31, 2007 at 12:13:04PM +0100, Andi Kleen wrote:
> > > Matthew Garrett <mjg59@srcf.ucam.org> writes:
> > > > 
> > > > 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.

Yes. Low power states without ability to respond to wakeup interrupts
would be broken behaviour generally.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] Runtime power management on ipw2100
  2007-01-31 11:53           ` Amit Kucheria
@ 2007-01-31 13:04             ` Pavel Machek
  2007-01-31 13:12               ` [linux-pm] " Oliver Neukum
                                 ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Pavel Machek @ 2007-01-31 13:04 UTC (permalink / raw)
  To: Amit Kucheria
  Cc: Matthew Garrett, ipw2100-devel, linux-pm, ext Andi Kleen, netdev

On Wed 2007-01-31 13:53:20, Amit Kucheria wrote:
> On Wed, 2007-01-31 at 11:48 +0100, ext Andi Kleen wrote:
> > On Wednesday 31 January 2007 11:27, Matthew Garrett wrote:
> > > On Wed, Jan 31, 2007 at 12:13:04PM +0100, Andi Kleen wrote:
> > > > Matthew Garrett <mjg59@srcf.ucam.org> writes:
> > > > > 
> > > > > 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.
> 
> 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
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [linux-pm] [RFC] Runtime power management on ipw2100
  2007-01-31 13:04             ` Pavel Machek
@ 2007-01-31 13:12               ` Oliver Neukum
  2007-01-31 13:13               ` samuel
  2007-01-31 13:24               ` Amit Kucheria
  2 siblings, 0 replies; 19+ messages in thread
From: Oliver Neukum @ 2007-01-31 13:12 UTC (permalink / raw)
  To: linux-pm
  Cc: Pavel Machek, Amit Kucheria, Matthew Garrett, ipw2100-devel,
	ext Andi Kleen, netdev

Am Mittwoch, 31. Januar 2007 14:04 schrieb Pavel Machek:
> On Wed 2007-01-31 13:53:20, Amit Kucheria wrote:
> > On Wed, 2007-01-31 at 11:48 +0100, ext Andi Kleen wrote:
> > > On Wednesday 31 January 2007 11:27, Matthew Garrett wrote:
> > > > On Wed, Jan 31, 2007 at 12:13:04PM +0100, Andi Kleen wrote:
> > > > > Matthew Garrett <mjg59@srcf.ucam.org> writes:
> > > > > > 
> > > > > > 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.
> > 
> > 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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] Runtime power management on ipw2100
  2007-01-31 13:04             ` Pavel Machek
  2007-01-31 13:12               ` [linux-pm] " Oliver Neukum
@ 2007-01-31 13:13               ` samuel
  2007-01-31 13:24               ` Amit Kucheria
  2 siblings, 0 replies; 19+ messages in thread
From: samuel @ 2007-01-31 13:13 UTC (permalink / raw)
  To: pavel, amit.kucheria
  Cc: Matthew Garrett, ipw2100-devel, linux-pm, ext Andi Kleen, netdev


On 1/31/2007, "Pavel Machek" <pavel@suse.cz> wrote:

>On Wed 2007-01-31 13:53:20, Amit Kucheria wrote:
>> On Wed, 2007-01-31 at 11:48 +0100, ext Andi Kleen wrote:
>> > On Wednesday 31 January 2007 11:27, Matthew Garrett wrote:
>> > > On Wed, Jan 31, 2007 at 12:13:04PM +0100, Andi Kleen wrote:
>> > > > Matthew Garrett <mjg59@srcf.ucam.org> writes:
>> > > > >
>> > > > > 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.
>>
>> 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.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] Runtime power management on ipw2100
  2007-01-31 13:04             ` Pavel Machek
  2007-01-31 13:12               ` [linux-pm] " Oliver Neukum
  2007-01-31 13:13               ` samuel
@ 2007-01-31 13:24               ` Amit Kucheria
  2007-01-31 13:44                 ` [linux-pm] " Pavel Machek
  2 siblings, 1 reply; 19+ messages in thread
From: Amit Kucheria @ 2007-01-31 13:24 UTC (permalink / raw)
  To: ext Pavel Machek
  Cc: Matthew Garrett, ipw2100-devel, linux-pm, ext Andi Kleen, netdev

On Wed, 2007-01-31 at 14:04 +0100, ext Pavel Machek wrote:
> On Wed 2007-01-31 13:53:20, Amit Kucheria wrote:
> > 
> > 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

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

So with the right hardware, yes, you can listen and still save power.

Regards,
Amit

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [linux-pm] [RFC] Runtime power management on ipw2100
  2007-01-31 13:24               ` Amit Kucheria
@ 2007-01-31 13:44                 ` Pavel Machek
  2007-01-31 14:11                   ` Matthew Garrett
  0 siblings, 1 reply; 19+ messages in thread
From: Pavel Machek @ 2007-01-31 13:44 UTC (permalink / raw)
  To: Amit Kucheria
  Cc: ext Andi Kleen, Matthew Garrett, netdev, linux-pm, ipw2100-devel

Hi!

> > > 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?
> 
> >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 days
> 
> So 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

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [linux-pm] [RFC] Runtime power management on ipw2100
  2007-01-31 13:44                 ` [linux-pm] " Pavel Machek
@ 2007-01-31 14:11                   ` Matthew Garrett
  0 siblings, 0 replies; 19+ messages in thread
From: Matthew Garrett @ 2007-01-31 14:11 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Amit Kucheria, ext Andi Kleen, netdev, linux-pm, ipw2100-devel

On Wed, Jan 31, 2007 at 02:44:16PM +0100, Pavel Machek wrote:

> 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.
-- 
Matthew Garrett | mjg59@srcf.ucam.org

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Ipw2100-devel] [RFC] Runtime power management on ipw2100
  2007-01-31  7:52 [RFC] Runtime power management on ipw2100 Matthew Garrett
  2007-01-31  9:13 ` Amit Kucheria
  2007-01-31 10:39 ` Pavel Machek
@ 2007-02-01  1:47 ` Zhu Yi
  2007-02-06 21:44   ` Matthew Garrett
  2 siblings, 1 reply; 19+ messages in thread
From: Zhu Yi @ 2007-02-01  1:47 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: ipw2100-devel, netdev, linux-pm

On Wed, 2007-01-31 at 07:52 +0000, Matthew Garrett wrote:
> 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

>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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Ipw2100-devel] [RFC] Runtime power management on ipw2100
  2007-02-01  1:47 ` [Ipw2100-devel] " Zhu Yi
@ 2007-02-06 21:44   ` Matthew Garrett
  2007-02-08  9:01     ` Zhu Yi
  0 siblings, 1 reply; 19+ messages in thread
From: Matthew Garrett @ 2007-02-06 21:44 UTC (permalink / raw)
  To: Zhu Yi; +Cc: ipw2100-devel, netdev, linux-pm

On Thu, Feb 01, 2007 at 09:47:05AM +0800, Zhu Yi wrote:
> On Wed, 2007-01-31 at 07:52 +0000, Matthew Garrett wrote:

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

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

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?

Signed-off-by: Matthew Garrett <mjg59@srcf.ucam.org>

diff --git a/drivers/net/wireless/ipw2100.c b/drivers/net/wireless/ipw2100.c
index 24ef52a..679946e 100644
--- a/drivers/net/wireless/ipw2100.c
+++ b/drivers/net/wireless/ipw2100.c
@@ -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;

-- 
Matthew Garrett | mjg59@srcf.ucam.org

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [RFC] Runtime power management on ipw2100
  2007-02-06 21:44   ` Matthew Garrett
@ 2007-02-08  9:01     ` Zhu Yi
  2007-02-19 21:08       ` [linux-pm] [Ipw2100-devel] " David Brownell
  0 siblings, 1 reply; 19+ messages in thread
From: Zhu Yi @ 2007-02-08  9:01 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: ipw2100-devel, linux-pm, netdev

On Tue, 2007-02-06 at 21:44 +0000, Matthew Garrett wrote:
> 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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [linux-pm] [Ipw2100-devel] [RFC] Runtime power management on ipw2100
  2007-02-08  9:01     ` Zhu Yi
@ 2007-02-19 21:08       ` David Brownell
  0 siblings, 0 replies; 19+ messages in thread
From: David Brownell @ 2007-02-19 21:08 UTC (permalink / raw)
  To: linux-pm; +Cc: Zhu Yi, Matthew Garrett, ipw2100-devel, netdev

On Thursday 08 February 2007 1:01 am, Zhu Yi wrote:

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

> 	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

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2007-02-19 21:08 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-01-31  7:52 [RFC] Runtime power management on ipw2100 Matthew Garrett
2007-01-31  9:13 ` Amit Kucheria
2007-01-31  9:48   ` [linux-pm] " Matthew Garrett
2007-01-31 11:04     ` Amit Kucheria
2007-01-31 11:13     ` Andi Kleen
2007-01-31 10:27       ` [linux-pm] " Matthew Garrett
2007-01-31 10:48         ` Andi Kleen
2007-01-31 11:53           ` Amit Kucheria
2007-01-31 13:04             ` Pavel Machek
2007-01-31 13:12               ` [linux-pm] " Oliver Neukum
2007-01-31 13:13               ` samuel
2007-01-31 13:24               ` Amit Kucheria
2007-01-31 13:44                 ` [linux-pm] " Pavel Machek
2007-01-31 14:11                   ` Matthew Garrett
2007-01-31 10:39 ` Pavel Machek
2007-02-01  1:47 ` [Ipw2100-devel] " Zhu Yi
2007-02-06 21:44   ` Matthew Garrett
2007-02-08  9:01     ` Zhu Yi
2007-02-19 21:08       ` [linux-pm] [Ipw2100-devel] " David Brownell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.