From: "Yan, Zheng" <zheng.z.yan@intel•com>
To: wwguy <wey-yi.w.guy@intel•com>
Cc: ilw@linux•intel.com, "netdev@vger•kernel.org" <netdev@vger•kernel.org>
Subject: Re: [PATCH RFC] iwlwifi: add basic runtime PM support
Date: Mon, 09 Jan 2012 09:37:07 +0800 [thread overview]
Message-ID: <4F0A44C3.5050307@intel.com> (raw)
In-Reply-To: <1325862848.13419.16.camel@wwguy-ubuntu>
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
>
prev parent reply other threads:[~2012-01-09 1:37 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-01-06 2:41 [PATCH RFC] iwlwifi: add basic runtime PM support Yan, Zheng
[not found] ` <4F065F59.2070107-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2012-01-06 9:47 ` Johannes Berg
2012-01-09 1:01 ` Yan, Zheng
2012-01-09 0:34 ` Guy, Wey-Yi
2012-01-09 1:55 ` Yan, Zheng
2012-01-09 1:05 ` Guy, Wey-Yi
2012-01-09 9:04 ` Yan, Zheng
2012-01-09 9:11 ` Johannes Berg
[not found] ` <1326100270.3451.5.camel-8upI4CBIZJIJvtFkdXX2HixXY32XiHfO@public.gmane.org>
2012-01-09 14:39 ` Guy, Wey-Yi
2012-01-09 9:10 ` Johannes Berg
2012-01-06 15:14 ` wwguy
2012-01-09 1:37 ` Yan, Zheng [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4F0A44C3.5050307@intel.com \
--to=zheng.z.yan@intel$(echo .)com \
--cc=ilw@linux$(echo .)intel.com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=wey-yi.w.guy@intel$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox