[PATCH RFC] iwlwifi: add basic runtime PM support

[PATCH RFC] iwlwifi: add basic runtime PM support

[Yan, Zheng]

This simple patch adds open/close based runtime PM support to the iwlwifi driver.
Namely, make the driver suspend the device after shutting down the interface and
resume the device when activating the interface. In my test, suspending the device
can save about 0.4 watt power. The shortcoming is that the device no longer generate
rfkill changes interrupt.

Signed-off-by: Zheng Yan <zheng.z.yan@intel.com>
---
diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.c b/drivers/net/wireless/iwlwifi/iwl-agn.c
index e0e9a3d..c0b7b85 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn.c
@@ -38,6 +38,7 @@
 #include <linux/firmware.h>
 #include <linux/etherdevice.h>
 #include <linux/if_arp.h>
+#include <linux/pm_runtime.h>
 
 #include <net/mac80211.h>
 
@@ -1755,6 +1756,8 @@ static int iwlagn_mac_start(struct ieee80211_hw *hw)
 	struct iwl_priv *priv = hw->priv;
 	int ret;
 
+	pm_runtime_get_sync(bus(priv)->dev);
+
 	IWL_DEBUG_MAC80211(priv, "enter\n");
 
 	/* we should be verifying the device is ready to be opened */
@@ -1762,19 +1765,24 @@ static int iwlagn_mac_start(struct ieee80211_hw *hw)
 	ret = __iwl_up(priv);
 	mutex_unlock(&priv->shrd->mutex);
 	if (ret)
-		return ret;
+		goto out;
 
 	IWL_DEBUG_INFO(priv, "Start UP work done.\n");
 
 	/* Now we should be done, and the READY bit should be set. */
-	if (WARN_ON(!test_bit(STATUS_READY, &priv->shrd->status)))
+	if (WARN_ON(!test_bit(STATUS_READY, &priv->shrd->status))) {
 		ret = -EIO;
+		goto out;
+	}
 
 	iwlagn_led_enable(priv);
 
 	priv->is_open = 1;
 	IWL_DEBUG_MAC80211(priv, "leave\n");
-	return 0;
+	ret = 0;
+out:
+	pm_runtime_put(bus(priv)->dev);
+	return ret;
 }
 
 static void iwlagn_mac_stop(struct ieee80211_hw *hw)
@@ -1786,6 +1794,8 @@ static void iwlagn_mac_stop(struct ieee80211_hw *hw)
 	if (!priv->is_open)
 		return;
 
+	pm_runtime_get_sync(bus(priv)->dev);
+
 	priv->is_open = 0;
 
 	iwl_down(priv);
@@ -1798,6 +1808,8 @@ static void iwlagn_mac_stop(struct ieee80211_hw *hw)
 	iwl_enable_rfkill_int(priv);
 
 	IWL_DEBUG_MAC80211(priv, "leave\n");
+
+	pm_runtime_put(bus(priv)->dev);
 }
 
 #ifdef CONFIG_PM_SLEEP
diff --git a/drivers/net/wireless/iwlwifi/iwl-pci.c b/drivers/net/wireless/iwlwifi/iwl-pci.c
index 1800029..d3f39b2 100644
--- a/drivers/net/wireless/iwlwifi/iwl-pci.c
+++ b/drivers/net/wireless/iwlwifi/iwl-pci.c
@@ -63,6 +63,7 @@
 #include <linux/module.h>
 #include <linux/pci.h>
 #include <linux/pci-aspm.h>
+#include <linux/pm_runtime.h>
 
 #include "iwl-bus.h"
 #include "iwl-io.h"
@@ -465,6 +466,8 @@ static int iwl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	err = iwl_probe(bus, &trans_ops_pcie, cfg);
 	if (err)
 		goto out_disable_msi;
+
+	pm_runtime_put_noidle(&pdev->dev);
 	return 0;
 
 out_disable_msi:
@@ -487,6 +490,8 @@ static void __devexit iwl_pci_remove(struct pci_dev *pdev)
 	struct pci_dev *pci_dev = IWL_BUS_GET_PCI_DEV(bus);
 	struct iwl_shared *shrd = bus->shrd;
 
+	pm_runtime_get_noresume(&pdev->dev);
+
 	iwl_remove(shrd->priv);
 
 	pci_disable_msi(pci_dev);
@@ -534,7 +539,21 @@ static int iwl_pci_resume(struct device *device)
 	return iwl_trans_resume(shrd->trans);
 }
 
-static SIMPLE_DEV_PM_OPS(iwl_dev_pm_ops, iwl_pci_suspend, iwl_pci_resume);
+
+static int iwl_pci_runtime_idle(struct device *dev)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	struct iwl_bus *bus = pci_get_drvdata(pdev);
+	struct iwl_shared *shrd = bus->shrd;
+
+	if (!test_bit(STATUS_ALIVE, &shrd->status))
+		return 0;
+
+	return -EBUSY;
+}
+
+static UNIVERSAL_DEV_PM_OPS(iwl_dev_pm_ops, iwl_pci_suspend,
+			iwl_pci_resume, iwl_pci_runtime_idle);
 
 #define IWL_PM_OPS	(&iwl_dev_pm_ops)
 

Re: [PATCH RFC] iwlwifi: add basic runtime PM support

[Johannes Berg]

[add linux-wireless]

On Fri, 2012-01-06 at 10:41 +0800, Yan, Zheng wrote:
> This simple patch adds open/close based runtime PM support to the iwlwifi driver.
> Namely, make the driver suspend the device after shutting down the interface and
> resume the device when activating the interface. In my test, suspending the device
> can save about 0.4 watt power. The shortcoming is that the device no longer generate
> rfkill changes interrupt.

NACK due to that last sentence. There's no way we can live with that in
the general case -- and your patch isn't even configurable afaict. And
I'm sure polling the rfkill flag would use just as much energy.

There might be some value in this in a system that doesn't have a hard
rfkill line, but that means this needs to be configurable since the
device can't know whether there's a button or not [1].

johannes

[1] actually in theory it might be possible to determine whether or not
the pin is floating or not? I doubt even that is possible with the HW we
have though

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC] iwlwifi: add basic runtime PM support

[Yan, Zheng]

On 01/06/2012 05:47 PM, Johannes Berg wrote:
> [add linux-wireless]
> 
> On Fri, 2012-01-06 at 10:41 +0800, Yan, Zheng wrote:
>> This simple patch adds open/close based runtime PM support to the iwlwifi driver.
>> Namely, make the driver suspend the device after shutting down the interface and
>> resume the device when activating the interface. In my test, suspending the device
>> can save about 0.4 watt power. The shortcoming is that the device no longer generate
>> rfkill changes interrupt.
> 
> NACK due to that last sentence. There's no way we can live with that in
> the general case -- and your patch isn't even configurable afaict. And
> I'm sure polling the rfkill flag would use just as much energy.
> 
It's configurable, runtime PM is disabled by default.

> There might be some value in this in a system that doesn't have a hard
> rfkill line, but that means this needs to be configurable since the
> device can't know whether there's a button or not [1].
> 
The patch targets system that only use software rfkill

Regards
Yan, Zheng


> johannes
> 
> [1] actually in theory it might be possible to determine whether or not
> the pin is floating or not? I doubt even that is possible with the HW we
> have though
> 

Re: [PATCH RFC] iwlwifi: add basic runtime PM support

[Guy, Wey-Yi]

On Mon, 2012-01-09 at 09:01 +0800, Yan, Zheng wrote:
> On 01/06/2012 05:47 PM, Johannes Berg wrote:
> > [add linux-wireless]
> > 
> > On Fri, 2012-01-06 at 10:41 +0800, Yan, Zheng wrote:
> >> This simple patch adds open/close based runtime PM support to the iwlwifi driver.
> >> Namely, make the driver suspend the device after shutting down the interface and
> >> resume the device when activating the interface. In my test, suspending the device
> >> can save about 0.4 watt power. The shortcoming is that the device no longer generate
> >> rfkill changes interrupt.
> > 
> > NACK due to that last sentence. There's no way we can live with that in
> > the general case -- and your patch isn't even configurable afaict. And
> > I'm sure polling the rfkill flag would use just as much energy.
> > 
> It's configurable, runtime PM is disabled by default.

Somehow I miss it, how you configure it?

> 
> > There might be some value in this in a system that doesn't have a hard
> > rfkill line, but that means this needs to be configurable since the
> > device can't know whether there's a button or not [1].
> > 
> The patch targets system that only use software rfkill

How you control that?

Wey
> 

Re: [PATCH RFC] iwlwifi: add basic runtime PM support

[Yan, Zheng]

On 01/09/2012 08:34 AM, Guy, Wey-Yi wrote:
> On Mon, 2012-01-09 at 09:01 +0800, Yan, Zheng wrote:
>> On 01/06/2012 05:47 PM, Johannes Berg wrote:
>>> [add linux-wireless]
>>>
>>> On Fri, 2012-01-06 at 10:41 +0800, Yan, Zheng wrote:
>>>> This simple patch adds open/close based runtime PM support to the iwlwifi driver.
>>>> Namely, make the driver suspend the device after shutting down the interface and
>>>> resume the device when activating the interface. In my test, suspending the device
>>>> can save about 0.4 watt power. The shortcoming is that the device no longer generate
>>>> rfkill changes interrupt.
>>>
>>> NACK due to that last sentence. There's no way we can live with that in
>>> the general case -- and your patch isn't even configurable afaict. And
>>> I'm sure polling the rfkill flag would use just as much energy.
>>>
>> It's configurable, runtime PM is disabled by default.
> 
> Somehow I miss it, how you configure it?
> 
change the value of /sys/devices/.../power/control to auto to enable the runtime PM.
(e.g echo auto > /sys/devices/pci0000:00/0000:00:1c.3/0000:02:00.0/power/control)

>>
>>> There might be some value in this in a system that doesn't have a hard
>>> rfkill line, but that means this needs to be configurable since the
>>> device can't know whether there's a button or not [1].
>>>
>> The patch targets system that only use software rfkill
> 
> How you control that?  
I can't. Our team is working on runtime PM project, the purpose of the patch is
more or less to demonstrate how much power can be saved.

Regards
Yan, Zheng

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC] iwlwifi: add basic runtime PM support

[Guy, Wey-Yi]

On Mon, 2012-01-09 at 09:55 +0800, Yan, Zheng wrote:
> On 01/09/2012 08:34 AM, Guy, Wey-Yi wrote:
> > On Mon, 2012-01-09 at 09:01 +0800, Yan, Zheng wrote:
> >> On 01/06/2012 05:47 PM, Johannes Berg wrote:
> >>> [add linux-wireless]
> >>>
> >>> On Fri, 2012-01-06 at 10:41 +0800, Yan, Zheng wrote:
> >>>> This simple patch adds open/close based runtime PM support to the iwlwifi driver.
> >>>> Namely, make the driver suspend the device after shutting down the interface and
> >>>> resume the device when activating the interface. In my test, suspending the device
> >>>> can save about 0.4 watt power. The shortcoming is that the device no longer generate
> >>>> rfkill changes interrupt.
> >>>
> >>> NACK due to that last sentence. There's no way we can live with that in
> >>> the general case -- and your patch isn't even configurable afaict. And
> >>> I'm sure polling the rfkill flag would use just as much energy.
> >>>
> >> It's configurable, runtime PM is disabled by default.
> > 
> > Somehow I miss it, how you configure it?
> > 
> change the value of /sys/devices/.../power/control to auto to enable the runtime PM.
> (e.g echo auto > /sys/devices/pci0000:00/0000:00:1c.3/0000:02:00.0/power/control)

I am not sure it is acceptable, how you expect user figure out the pci
space especially the NIC can be in any of the PCI slots.

> 
> >>
> >>> There might be some value in this in a system that doesn't have a hard
> >>> rfkill line, but that means this needs to be configurable since the
> >>> device can't know whether there's a button or not [1].
> >>>
> >> The patch targets system that only use software rfkill
> > 
> > How you control that?  
> I can't. Our team is working on runtime PM project, the purpose of the patch is
> more or less to demonstrate how much power can be saved.
> 
I understand, but unless we figure out either make rkill interrupt works
in runtime PM, or figure out the platform does not has HW RFKILL
automatically, I don't see how this patch can upstream without generate
a lot of issues and bug reports.

Thanks
Wey
> 

Re: [PATCH RFC] iwlwifi: add basic runtime PM support

[Yan, Zheng]

On 01/09/2012 09:05 AM, Guy, Wey-Yi wrote:
> On Mon, 2012-01-09 at 09:55 +0800, Yan, Zheng wrote:
>> On 01/09/2012 08:34 AM, Guy, Wey-Yi wrote:
>>> On Mon, 2012-01-09 at 09:01 +0800, Yan, Zheng wrote:
>>>> On 01/06/2012 05:47 PM, Johannes Berg wrote:
>>>>> [add linux-wireless]
>>>>>
>>>>> On Fri, 2012-01-06 at 10:41 +0800, Yan, Zheng wrote:
>>>>>> This simple patch adds open/close based runtime PM support to the iwlwifi driver.
>>>>>> Namely, make the driver suspend the device after shutting down the interface and
>>>>>> resume the device when activating the interface. In my test, suspending the device
>>>>>> can save about 0.4 watt power. The shortcoming is that the device no longer generate
>>>>>> rfkill changes interrupt.
>>>>>
>>>>> NACK due to that last sentence. There's no way we can live with that in
>>>>> the general case -- and your patch isn't even configurable afaict. And
>>>>> I'm sure polling the rfkill flag would use just as much energy.
>>>>>
>>>> It's configurable, runtime PM is disabled by default.
>>>
>>> Somehow I miss it, how you configure it?
>>>
>> change the value of /sys/devices/.../power/control to auto to enable the runtime PM.
>> (e.g echo auto > /sys/devices/pci0000:00/0000:00:1c.3/0000:02:00.0/power/control)
> 
> I am not sure it is acceptable, how you expect user figure out the pci
> space especially the NIC can be in any of the PCI slots.
> 
>>
>>>>
>>>>> There might be some value in this in a system that doesn't have a hard
>>>>> rfkill line, but that means this needs to be configurable since the
>>>>> device can't know whether there's a button or not [1].
>>>>>
>>>> The patch targets system that only use software rfkill
>>>
>>> How you control that?  
>> I can't. Our team is working on runtime PM project, the purpose of the patch is
>> more or less to demonstrate how much power can be saved.
>>
> I understand, but unless we figure out either make rkill interrupt works
> in runtime PM, or figure out the platform does not has HW RFKILL
> automatically, I don't see how this patch can upstream without generate
> a lot of issues and bug reports.
> 
Thank you for the suggestion.

Yan, Zheng

Re: [PATCH RFC] iwlwifi: add basic runtime PM support

[Johannes Berg]

On Sun, 2012-01-08 at 17:05 -0800, Guy, Wey-Yi wrote:


> I understand, but unless we figure out either make rkill interrupt works
> in runtime PM, or figure out the platform does not has HW RFKILL
> automatically, I don't see how this patch can upstream without generate
> a lot of issues and bug reports.

I suppose the question is -- will any tools/users enable it and then
scream about their rfkill?

Maybe we can use pm_runtime_forbid() to disable it completely, and add a
module parameter or something to enable it -- just so people are aware
of the tradeoffs?

johannes

Re: [PATCH RFC] iwlwifi: add basic runtime PM support

[Guy, Wey-Yi]

On Mon, 2012-01-09 at 10:11 +0100, Johannes Berg wrote:
> On Sun, 2012-01-08 at 17:05 -0800, Guy, Wey-Yi wrote:
> 
> 
> > I understand, but unless we figure out either make rkill interrupt works
> > in runtime PM, or figure out the platform does not has HW RFKILL
> > automatically, I don't see how this patch can upstream without generate
> > a lot of issues and bug reports.
> 
> I suppose the question is -- will any tools/users enable it and then
> scream about their rfkill?
> 
> Maybe we can use pm_runtime_forbid() to disable it completely, and add a
> module parameter or something to enable it -- just so people are aware
> of the tradeoffs?

module parameter shall works, we just need a clear and easy way to allow
user enable/disable this runtime PM feature and not making mistake

Wey
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC] iwlwifi: add basic runtime PM support

[Johannes Berg]

On Mon, 2012-01-09 at 09:01 +0800, Yan, Zheng wrote:

> > NACK due to that last sentence. There's no way we can live with that in
> > the general case -- and your patch isn't even configurable afaict. And
> > I'm sure polling the rfkill flag would use just as much energy.
> > 
> It's configurable, runtime PM is disabled by default.

Oh ok, that's not so bad then -- we just need the default case to
continue working I think.

johannes

Re: [PATCH RFC] iwlwifi: add basic runtime PM support

[wwguy]

Hi Zheng,

On Fri, 2012-01-06 at 10:41 +0800, Yan, Zheng wrote:
> This simple patch adds open/close based runtime PM support to the iwlwifi driver.
> Namely, make the driver suspend the device after shutting down the interface and
> resume the device when activating the interface. In my test, suspending the device
> can save about 0.4 watt power. The shortcoming is that the device no longer generate
> rfkill changes interrupt.
> 
> Signed-off-by: Zheng Yan <zheng.z.yan@intel.com>
> ---
> diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.c b/drivers/net/wireless/iwlwifi/iwl-agn.c
> index e0e9a3d..c0b7b85 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-agn.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-agn.c
> @@ -38,6 +38,7 @@
>  #include <linux/firmware.h>
>  #include <linux/etherdevice.h>
>  #include <linux/if_arp.h>
> +#include <linux/pm_runtime.h>
>  
>  #include <net/mac80211.h>
>  
> @@ -1755,6 +1756,8 @@ static int iwlagn_mac_start(struct ieee80211_hw *hw)
>  	struct iwl_priv *priv = hw->priv;
>  	int ret;
why not just set the ret = 0 here and save eht work later
>  
> +	pm_runtime_get_sync(bus(priv)->dev);
> +
>  	IWL_DEBUG_MAC80211(priv, "enter\n");
>  
>  	/* we should be verifying the device is ready to be opened */
> @@ -1762,19 +1765,24 @@ static int iwlagn_mac_start(struct ieee80211_hw *hw)
>  	ret = __iwl_up(priv);
>  	mutex_unlock(&priv->shrd->mutex);
>  	if (ret)
> -		return ret;
> +		goto out;
>  
>  	IWL_DEBUG_INFO(priv, "Start UP work done.\n");
>  
>  	/* Now we should be done, and the READY bit should be set. */
> -	if (WARN_ON(!test_bit(STATUS_READY, &priv->shrd->status)))
> +	if (WARN_ON(!test_bit(STATUS_READY, &priv->shrd->status))) {
>  		ret = -EIO;
> +		goto out;
> +	}
>  
>  	iwlagn_led_enable(priv);
>  
>  	priv->is_open = 1;
>  	IWL_DEBUG_MAC80211(priv, "leave\n");
> -	return 0;
> +	ret = 0;
if set it earlier, do have to do this

> +out:
> +	pm_runtime_put(bus(priv)->dev);
> +	return ret;
>  }
>  
>  static void iwlagn_mac_stop(struct ieee80211_hw *hw)
> @@ -1786,6 +1794,8 @@ static void iwlagn_mac_stop(struct ieee80211_hw *hw)
>  	if (!priv->is_open)
>  		return;
>  
> +	pm_runtime_get_sync(bus(priv)->dev);
> +
>  	priv->is_open = 0;
>  
>  	iwl_down(priv);
> @@ -1798,6 +1808,8 @@ static void iwlagn_mac_stop(struct ieee80211_hw *hw)
>  	iwl_enable_rfkill_int(priv);
>  
>  	IWL_DEBUG_MAC80211(priv, "leave\n");
> +
> +	pm_runtime_put(bus(priv)->dev);
>  }
>  
>  #ifdef CONFIG_PM_SLEEP
> diff --git a/drivers/net/wireless/iwlwifi/iwl-pci.c b/drivers/net/wireless/iwlwifi/iwl-pci.c
> index 1800029..d3f39b2 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-pci.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-pci.c
> @@ -63,6 +63,7 @@
>  #include <linux/module.h>
>  #include <linux/pci.h>
>  #include <linux/pci-aspm.h>
> +#include <linux/pm_runtime.h>
>  
>  #include "iwl-bus.h"
>  #include "iwl-io.h"
> @@ -465,6 +466,8 @@ static int iwl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	err = iwl_probe(bus, &trans_ops_pcie, cfg);
>  	if (err)
>  		goto out_disable_msi;
> +
> +	pm_runtime_put_noidle(&pdev->dev);
>  	return 0;
>  
>  out_disable_msi:
> @@ -487,6 +490,8 @@ static void __devexit iwl_pci_remove(struct pci_dev *pdev)
>  	struct pci_dev *pci_dev = IWL_BUS_GET_PCI_DEV(bus);
>  	struct iwl_shared *shrd = bus->shrd;
>  
> +	pm_runtime_get_noresume(&pdev->dev);
> +
>  	iwl_remove(shrd->priv);
>  
>  	pci_disable_msi(pci_dev);
> @@ -534,7 +539,21 @@ static int iwl_pci_resume(struct device *device)
>  	return iwl_trans_resume(shrd->trans);
>  }
>  
> -static SIMPLE_DEV_PM_OPS(iwl_dev_pm_ops, iwl_pci_suspend, iwl_pci_resume);
> +
> +static int iwl_pci_runtime_idle(struct device *dev)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	struct iwl_bus *bus = pci_get_drvdata(pdev);
> +	struct iwl_shared *shrd = bus->shrd;
> +
> +	if (!test_bit(STATUS_ALIVE, &shrd->status))
> +		return 0;
> +
> +	return -EBUSY;
> +}
> +
> +static UNIVERSAL_DEV_PM_OPS(iwl_dev_pm_ops, iwl_pci_suspend,
> +			iwl_pci_resume, iwl_pci_runtime_idle);
>  
>  #define IWL_PM_OPS	(&iwl_dev_pm_ops)
>  

it looks good, what NIC and system you test with?
also you mention it no longer generate rfkill changes interrupt, could
you elaborate?  

btw, could you send the patch to <internal-wifi-devel@linux.intel.com>
first if ok for you.

Thanks
Wey

Re: [PATCH RFC] iwlwifi: add basic runtime PM support

[Yan, Zheng]

On 01/06/2012 11:14 PM, wwguy wrote:
> Hi Zheng,
> 
> On Fri, 2012-01-06 at 10:41 +0800, Yan, Zheng wrote:
>> This simple patch adds open/close based runtime PM support to the iwlwifi driver.
>> Namely, make the driver suspend the device after shutting down the interface and
>> resume the device when activating the interface. In my test, suspending the device
>> can save about 0.4 watt power. The shortcoming is that the device no longer generate
>> rfkill changes interrupt.
>>
>> Signed-off-by: Zheng Yan <zheng.z.yan@intel.com>
>> ---
>> diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.c b/drivers/net/wireless/iwlwifi/iwl-agn.c
>> index e0e9a3d..c0b7b85 100644
>> --- a/drivers/net/wireless/iwlwifi/iwl-agn.c
>> +++ b/drivers/net/wireless/iwlwifi/iwl-agn.c
>> @@ -38,6 +38,7 @@
>>  #include <linux/firmware.h>
>>  #include <linux/etherdevice.h>
>>  #include <linux/if_arp.h>
>> +#include <linux/pm_runtime.h>
>>  
>>  #include <net/mac80211.h>
>>  
>> @@ -1755,6 +1756,8 @@ static int iwlagn_mac_start(struct ieee80211_hw *hw)
>>  	struct iwl_priv *priv = hw->priv;
>>  	int ret;
> why not just set the ret = 0 here and save eht work later
>>  
>> +	pm_runtime_get_sync(bus(priv)->dev);
>> +
>>  	IWL_DEBUG_MAC80211(priv, "enter\n");
>>  
>>  	/* we should be verifying the device is ready to be opened */
>> @@ -1762,19 +1765,24 @@ static int iwlagn_mac_start(struct ieee80211_hw *hw)
>>  	ret = __iwl_up(priv);
>>  	mutex_unlock(&priv->shrd->mutex);
>>  	if (ret)
>> -		return ret;
>> +		goto out;
>>  
>>  	IWL_DEBUG_INFO(priv, "Start UP work done.\n");
>>  
>>  	/* Now we should be done, and the READY bit should be set. */
>> -	if (WARN_ON(!test_bit(STATUS_READY, &priv->shrd->status)))
>> +	if (WARN_ON(!test_bit(STATUS_READY, &priv->shrd->status))) {
>>  		ret = -EIO;
>> +		goto out;
>> +	}
>>  
>>  	iwlagn_led_enable(priv);
>>  
>>  	priv->is_open = 1;
>>  	IWL_DEBUG_MAC80211(priv, "leave\n");
>> -	return 0;
>> +	ret = 0;
> if set it earlier, do have to do this
> 
>> +out:
>> +	pm_runtime_put(bus(priv)->dev);
>> +	return ret;
>>  }
>>  
>>  static void iwlagn_mac_stop(struct ieee80211_hw *hw)
>> @@ -1786,6 +1794,8 @@ static void iwlagn_mac_stop(struct ieee80211_hw *hw)
>>  	if (!priv->is_open)
>>  		return;
>>  
>> +	pm_runtime_get_sync(bus(priv)->dev);
>> +
>>  	priv->is_open = 0;
>>  
>>  	iwl_down(priv);
>> @@ -1798,6 +1808,8 @@ static void iwlagn_mac_stop(struct ieee80211_hw *hw)
>>  	iwl_enable_rfkill_int(priv);
>>  
>>  	IWL_DEBUG_MAC80211(priv, "leave\n");
>> +
>> +	pm_runtime_put(bus(priv)->dev);
>>  }
>>  
>>  #ifdef CONFIG_PM_SLEEP
>> diff --git a/drivers/net/wireless/iwlwifi/iwl-pci.c b/drivers/net/wireless/iwlwifi/iwl-pci.c
>> index 1800029..d3f39b2 100644
>> --- a/drivers/net/wireless/iwlwifi/iwl-pci.c
>> +++ b/drivers/net/wireless/iwlwifi/iwl-pci.c
>> @@ -63,6 +63,7 @@
>>  #include <linux/module.h>
>>  #include <linux/pci.h>
>>  #include <linux/pci-aspm.h>
>> +#include <linux/pm_runtime.h>
>>  
>>  #include "iwl-bus.h"
>>  #include "iwl-io.h"
>> @@ -465,6 +466,8 @@ static int iwl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>>  	err = iwl_probe(bus, &trans_ops_pcie, cfg);
>>  	if (err)
>>  		goto out_disable_msi;
>> +
>> +	pm_runtime_put_noidle(&pdev->dev);
>>  	return 0;
>>  
>>  out_disable_msi:
>> @@ -487,6 +490,8 @@ static void __devexit iwl_pci_remove(struct pci_dev *pdev)
>>  	struct pci_dev *pci_dev = IWL_BUS_GET_PCI_DEV(bus);
>>  	struct iwl_shared *shrd = bus->shrd;
>>  
>> +	pm_runtime_get_noresume(&pdev->dev);
>> +
>>  	iwl_remove(shrd->priv);
>>  
>>  	pci_disable_msi(pci_dev);
>> @@ -534,7 +539,21 @@ static int iwl_pci_resume(struct device *device)
>>  	return iwl_trans_resume(shrd->trans);
>>  }
>>  
>> -static SIMPLE_DEV_PM_OPS(iwl_dev_pm_ops, iwl_pci_suspend, iwl_pci_resume);
>> +
>> +static int iwl_pci_runtime_idle(struct device *dev)
>> +{
>> +	struct pci_dev *pdev = to_pci_dev(dev);
>> +	struct iwl_bus *bus = pci_get_drvdata(pdev);
>> +	struct iwl_shared *shrd = bus->shrd;
>> +
>> +	if (!test_bit(STATUS_ALIVE, &shrd->status))
>> +		return 0;
>> +
>> +	return -EBUSY;
>> +}
>> +
>> +static UNIVERSAL_DEV_PM_OPS(iwl_dev_pm_ops, iwl_pci_suspend,
>> +			iwl_pci_resume, iwl_pci_runtime_idle);
>>  
>>  #define IWL_PM_OPS	(&iwl_dev_pm_ops)
>>  
> 
> it looks good, what NIC and system you test with?
Intel Centrino Advanced-N 6230

> also you mention it no longer generate rfkill changes interrupt, could
> you elaborate?
After putting the device into D3hot, the device doesn't generate rfkill
changes interrupt. I also tried check CSR_GP_CNTRL register when the device
is suspended, it doesn't reflect rfkill changes neither.

> 
> btw, could you send the patch to <internal-wifi-devel@linux.intel.com>
> first if ok for you.
> 
Ok

Regards
Yan, Zheng

> Thanks
> Wey
>