2007-11-16 23:07:24

by Miguel Botón

[permalink] [raw]
Subject: [PATCH 2/2] iwlwifi: add power management support -v2

This patch adds power management support in iwl3945 and iwl4965 drivers.

Signed-off-by: Miguel Botón <[email protected]>

diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
index 9baf8de..5c7b422 100644
--- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
+++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
@@ -6951,6 +7187,7 @@ static int iwl3945_mac_config(struct ieee80211_hw *hw, struct ieee80211_conf *co
struct iwl3945_priv *priv = hw->priv;
const struct iwl3945_channel_info *ch_info;
unsigned long flags;
+ int power_mode;

mutex_lock(&priv->mutex);
IWL_DEBUG_MAC80211("enter to channel %d\n", conf->channel);
@@ -7001,6 +7238,23 @@ static int iwl3945_mac_config(struct ieee80211_hw *hw, struct ieee80211_conf *co
}
#endif

+ if (conf->power_management_enable)
+ power_mode = IWL_POWER_BATTERY | IWL_POWER_ENABLED;
+ else
+ power_mode = IWL_POWER_AC;
+
+ if (priv->power_mode != power_mode) {
+ int rc;
+
+ rc = iwl3945_send_power_mode(priv, power_mode);
+ if (rc) {
+ IWL_DEBUG_MAC80211("failed setting power mode.\n");
+ mutex_unlock(&priv->mutex);
+ return -EINVAL;
+ }
+ priv->power_mode = power_mode;
+ }
+
iwl3945_radio_kill_sw(priv, !conf->radio_enabled);

if (!conf->radio_enabled) {
diff --git a/drivers/net/wireless/iwlwifi/iwl4965-base.c b/drivers/net/wireless/iwlwifi/iwl4965-base.c
index df011ea..8d303c5 100644
--- a/drivers/net/wireless/iwlwifi/iwl4965-base.c
+++ b/drivers/net/wireless/iwlwifi/iwl4965-base.c
@@ -7352,6 +7352,7 @@ static int iwl4965_mac_config(struct ieee80211_hw *hw, struct ieee80211_conf *co
struct iwl4965_priv *priv = hw->priv;
const struct iwl4965_channel_info *ch_info;
unsigned long flags;
+ int power_mode;

mutex_lock(&priv->mutex);
IWL_DEBUG_MAC80211("enter to channel %d\n", conf->channel);
@@ -7414,6 +7415,23 @@ static int iwl4965_mac_config(struct ieee80211_hw *hw, struct ieee80211_conf *co
}
#endif

+ if (conf->power_management_enable)
+ power_mode = IWL_POWER_BATTERY | IWL_POWER_ENABLED;
+ else
+ power_mode = IWL_POWER_AC;
+
+ if (priv->power_mode != power_mode) {
+ int rc;
+
+ rc = iwl4965_send_power_mode(priv, power_mode);
+ if (rc) {
+ IWL_DEBUG_MAC80211("failed setting power mode.\n");
+ mutex_unlock(&priv->mutex);
+ return -EINVAL;
+ }
+ priv->power_mode = power_mode;
+ }
+
iwl4965_radio_kill_sw(priv, !conf->radio_enabled);

if (!conf->radio_enabled) {


--
Miguel Botón


2007-11-26 16:18:27

by Miguel Botón

[permalink] [raw]
Subject: Re: [PATCH 2/2] iwlwifi: add power management support -v2

On Saturday 17 November 2007 07:15:05 Tomas Winkler wrote:
> Why power management shouldn't be enabled while in AC? The semantic of this
> ioctls is quite unclear.

IWL_POWER_AC and IWL_POWER_BATTERY are just two power modes. IWL_POWER_AC
would be the default power mode when we're in AC (no power saving) and
IWL_POWER_BATTERY would be the default power mode when we're in battery
(power saving mode). That's why we set IWL_POWER_ENABLED flag with
IWL_POWER_BATTERY, because it is the only power mode that saves power.

We can change to IWL_POWER_BATTERY or IWL_POWER_AC in any moment.

This patch, depending if power management is enabled or not, sets which power
mode we should use, Then, it checks if we're already using this mode or not.

--
Miguel Bot?n

2007-11-26 17:12:12

by Tomas Winkler

[permalink] [raw]
Subject: Re: [PATCH 2/2] iwlwifi: add power management support -v2

On Nov 26, 2007 6:18 PM, Miguel Bot?n <[email protected]> wrote:
>
> On Saturday 17 November 2007 07:15:05 Tomas Winkler wrote:
> > Why power management shouldn't be enabled while in AC? The semantic of this
> > ioctls is quite unclear.
>
I
> IWL_POWER_AC and IWL_POWER_BATTERY are just two power modes. IWL_POWER_AC
> would be the default power mode when we're in AC (no power saving) and
> IWL_POWER_BATTERY would be the default power mode when we're in battery
> (power saving mode). That's why we set IWL_POWER_ENABLED flag with
> IWL_POWER_BATTERY, because it is the only power mode that saves power.
>
> We can change to IWL_POWER_BATTERY or IWL_POWER_AC in any moment.
>
> This patch, depending if power management is enabled or not, sets which power
> mode we should use, Then, it checks if we're already using this mode or not.
>
I'm not sure who introduced this names (lazy to look to history) but
that's very misleading. Nothing says that while in
AC we cannot do power saving and vice versa. This naming is scattered
all over the code, there should be only one place where AC and BATTERY
are translated into appropriate (maybe configurable) power levels. We
have 5 power levels defined for iwlwifi.

Tomas

> --
> Miguel Bot?n
>

2007-11-26 17:19:37

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH 2/2] iwlwifi: add power management support -v2

On Mon, 2007-11-26 at 19:11 +0200, Tomas Winkler wrote:
> On Nov 26, 2007 6:18 PM, Miguel Botón <[email protected]> wrote:
> >
> > On Saturday 17 November 2007 07:15:05 Tomas Winkler wrote:
> > > Why power management shouldn't be enabled while in AC? The semantic of this
> > > ioctls is quite unclear.
> >
> I
> > IWL_POWER_AC and IWL_POWER_BATTERY are just two power modes. IWL_POWER_AC
> > would be the default power mode when we're in AC (no power saving) and
> > IWL_POWER_BATTERY would be the default power mode when we're in battery
> > (power saving mode). That's why we set IWL_POWER_ENABLED flag with
> > IWL_POWER_BATTERY, because it is the only power mode that saves power.
> >
> > We can change to IWL_POWER_BATTERY or IWL_POWER_AC in any moment.
> >
> > This patch, depending if power management is enabled or not, sets which power
> > mode we should use, Then, it checks if we're already using this mode or not.
> >
> I'm not sure who introduced this names (lazy to look to history) but
> that's very misleading. Nothing says that while in
> AC we cannot do power saving and vice versa. This naming is scattered
> all over the code, there should be only one place where AC and BATTERY
> are translated into appropriate (maybe configurable) power levels. We
> have 5 power levels defined for iwlwifi.

At some point everyone needs to standardize on power levels so that
userland has a hope of mapping the right state to the right power level
in the driver. Otherwise, we get into a situation where broadcom power
levels don't map the same way iwl powerlevels do, and then we can't ever
do the right thing.

dan

> Tomas
>
> > --
> > Miguel Botón
> >
> -
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2007-11-26 17:33:30

by Tomas Winkler

[permalink] [raw]
Subject: Re: [PATCH 2/2] iwlwifi: add power management support -v2

>
> At some point everyone needs to standardize on power levels so that
> userland has a hope of mapping the right state to the right power level
> in the driver. Otherwise, we get into a situation where broadcom power
> levels don't map the same way iwl powerlevels do, and then we can't ever
> do the right thing.
>
Agree, these are just misleading names IMHO.

> dan
>
> > Tomas
>
> >
> > > --
> > > Miguel Bot?n
> > >
> > -
> > To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>