Re: [Ipw2100-devel] [RFC] Runtime power management on ipw2100

2007-02-08 Thread Zhu Yi
On Tue, 2007-02-06 at 21:44 +, 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
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Ipw2100-devel] [RFC] Runtime power management on ipw2100

2007-02-06 Thread Matthew Garrett
On Thu, Feb 01, 2007 at 09:47:05AM +0800, Zhu Yi wrote:
 On Wed, 2007-01-31 at 07:52 +, 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 [EMAIL PROTECTED]

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  0xff00) != 0)
+   pci_write_config_dword(priv-pci_dev, 0x40, val  0x00ff);
+
+   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  0x00ff);
 
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,
-

Re: [Ipw2100-devel] [RFC] Runtime power management on ipw2100

2007-01-31 Thread Zhu Yi
On Wed, 2007-01-31 at 07:52 +, 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
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html