| Message ID | cb0375e10909020753o4d314f61k74dd35025f61666a@mail.gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
Hi Andy, On Wed, 2009-09-02 at 07:53 -0700, Andrew Lutomirski wrote: > Right now, enabling power saving on iwlwifi is impossible, because > mac80211 won't tell iwlwifi that power saving is on (since > IEEE80211_HW_SUPPORTS_PS is not set) and iwlwifi will ignore the > user's power_level setting until mac80211 asks for power saving. > Setting this flag allows the user to manually enable power saving if > desired. > > Signed-off-by: Andy Lutomirski <luto@mit.edu> > > --- nack. http://www.intellinuxwireless.org/bugzilla/show_bug.cgi?id=2053 and http://www.intellinuxwireless.org/bugzilla/show_bug.cgi?id=2051 are two examples of what happens if power save support is enabled. In addition to this we are also seeing issues with 4965 that are described in the commit that disabled powersave in 2.6.31 in the first place (286d94906587901851906a5e2ddc09bc1a7ba1d9). > This fixes what looks to me like a regression: power_level used to > work but now fails silently. In 2.6.32 I think this code is going > away, but this patch seems like a safe stopgap measure. 2.6.32 will have power save support disabled also. Reinette -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Sep 2, 2009 at 11:56 AM, reinette chatre<reinette.chatre@intel.com> wrote: > Hi Andy, > > On Wed, 2009-09-02 at 07:53 -0700, Andrew Lutomirski wrote: >> Right now, enabling power saving on iwlwifi is impossible, because >> mac80211 won't tell iwlwifi that power saving is on (since >> IEEE80211_HW_SUPPORTS_PS is not set) and iwlwifi will ignore the >> user's power_level setting until mac80211 asks for power saving. >> Setting this flag allows the user to manually enable power saving if >> desired. >> >> Signed-off-by: Andy Lutomirski <luto@mit.edu> >> >> --- > > nack. > > http://www.intellinuxwireless.org/bugzilla/show_bug.cgi?id=2053 and > http://www.intellinuxwireless.org/bugzilla/show_bug.cgi?id=2051 are two > examples of what happens if power save support is enabled. In addition > to this we are also seeing issues with 4965 that are described in the > commit that disabled powersave in 2.6.31 in the first place > (286d94906587901851906a5e2ddc09bc1a7ba1d9). > Fair enough. Would you accept a patch to remove power_level from sysfs instead? > >> This fixes what looks to me like a regression: power_level used to >> work but now fails silently. Â In 2.6.32 I think this code is going >> away, but this patch seems like a safe stopgap measure. > > 2.6.32 will have power save support disabled also. Any ETA for getting this fixed for real? I like my battery life :) --Andy -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2009-09-02 at 08:56 -0700, reinette chatre wrote:
> 2.6.32 will have power save support disabled also.
Only by default -- there you can actually enable it as a user with
"iwconfig wlan0 power timeout 100ms" to turn it on -- unless that patch
didn't go in?
johannes
Hi Andrew, On Wed, 2009-09-02 at 09:41 -0700, Andrew Lutomirski wrote: > Fair enough. Would you accept a patch to remove power_level from sysfs instead? No. This file is used when users want to manually reduce the power used by device. This is the file you want to change if you want to extend your battery life. > >> This fixes what looks to me like a regression: power_level used to > >> work but now fails silently. In 2.6.32 I think this code is going > >> away, but this patch seems like a safe stopgap measure. > > > > 2.6.32 will have power save support disabled also. > > Any ETA for getting this fixed for real? I like my battery life :) We are working on it. Reinette -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2009-09-02 at 09:48 -0700, Johannes Berg wrote: > On Wed, 2009-09-02 at 08:56 -0700, reinette chatre wrote: > > > 2.6.32 will have power save support disabled also. > > Only by default -- there you can actually enable it as a user with > "iwconfig wlan0 power timeout 100ms" to turn it on -- unless that patch > didn't go in? yes - sorry, I should have added that it is disabled by default but users can enable it using iwconfig. Reinette -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2009-09-02 at 09:54 -0700, reinette chatre wrote: > Hi Andrew, > > On Wed, 2009-09-02 at 09:41 -0700, Andrew Lutomirski wrote: > > > Fair enough. Would you accept a patch to remove power_level from sysfs instead? > > No. This file is used when users want to manually reduce the power used > by device. This is the file you want to change if you want to extend > your battery life. Actually, we have already removed that file for 2.6.32 I think? So it may be fair to remove it for .31 since there you can't use it anyway. johannes
Hi Johannes, On Wed, 2009-09-02 at 10:03 -0700, Johannes Berg wrote: > On Wed, 2009-09-02 at 09:54 -0700, reinette chatre wrote: > > On Wed, 2009-09-02 at 09:41 -0700, Andrew Lutomirski wrote: > > > Fair enough. Would you accept a patch to remove power_level from sysfs instead? > > No. This file is used when users want to manually reduce the power used > > by device. This is the file you want to change if you want to extend > > your battery life. > Actually, we have already removed that file for 2.6.32 I think? The file itself was removed, but from what I understand the same functionality can now be obtained from sleep_level_override. Users who previously used power_level can thus now use sleep_level_override, but with sleep_level_override not existing in 2.6.31 they will have no alternative if we remove power_level. > So it > may be fair to remove it for .31 since there you can't use it anyway. You can still use it to manually set power index used by device to reduce power usage. Reinette -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Reinette, > The file itself was removed, but from what I understand the same > functionality can now be obtained from sleep_level_override. Indeed. > Users who > previously used power_level can thus now use sleep_level_override, but > with sleep_level_override not existing in 2.6.31 they will have no > alternative if we remove power_level. > > > So it > > may be fair to remove it for .31 since there you can't use it anyway. > > You can still use it to manually set power index used by device to > reduce power usage. I was under the impression that Andrew said this actually didn't work. johannes
On Wed, Sep 2, 2009 at 1:30 PM, Johannes Berg<johannes@sipsolutions.net> wrote: > Hi Reinette, > >> The file itself was removed, but from what I understand the same >> functionality can now be obtained from sleep_level_override. > > Indeed. > >> Users who >> previously used power_level can thus now use sleep_level_override, but >> with sleep_level_override not existing in 2.6.31 they will have no >> alternative if we remove power_level. >> >> > So it >> > may be fair to remove it for .31 since there you can't use it anyway. >> >> You can still use it to manually set power index used by device to >> reduce power usage. > > I was under the impression that Andrew said this actually didn't work. Exactly. My patch fixes that, although there might be a better way to do that. --Andy > > johannes > -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2009-09-02 at 10:38 -0700, Andrew Lutomirski wrote: > On Wed, Sep 2, 2009 at 1:30 PM, Johannes Berg<johannes@sipsolutions.net> wrote: > > > > I was under the impression that Andrew said this actually didn't work. > > Exactly. My patch fixes that, although there might be a better way to do that. Seems to be in iwl_power_update_mode() the mode is forced to CAM if power save disabled, and power save is set to disabled if mac considers it disabled (see setting of power_disabled in iwl_mac_config()). One way around this is to always allow user to set power mode, even if it is disabled from mac perspective. Since the power mode is updated in many more spots (not always triggered by user) I do not know the full implications of this change. Considering the unknown of this change and the problems we have encountered when enabling power save I would rather not merge such a patch without significant testing. I think it is too late for 2.6.31. The same problem should not exist in 2.6.32. Reinette -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/wireless/iwlwifi/iwl-core.c b/drivers/net/wireless/iwlwifi/iwl-core.c index 18b135f..be6f1e0 100644 --- a/drivers/net/wireless/iwlwifi/iwl-core.c +++ b/drivers/net/wireless/iwlwifi/iwl-core.c @@ -1325,7 +1325,8 @@ int iwl_setup_mac(struct iwl_priv *priv) hw->flags = IEEE80211_HW_SIGNAL_DBM | IEEE80211_HW_NOISE_DBM | IEEE80211_HW_AMPDU_AGGREGATION | - IEEE80211_HW_SPECTRUM_MGMT; + IEEE80211_HW_SPECTRUM_MGMT | + IEEE80211_HW_SUPPORTS_PS; hw->wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION) | BIT(NL80211_IFTYPE_ADHOC);
Right now, enabling power saving on iwlwifi is impossible, because mac80211 won't tell iwlwifi that power saving is on (since IEEE80211_HW_SUPPORTS_PS is not set) and iwlwifi will ignore the user's power_level setting until mac80211 asks for power saving. Setting this flag allows the user to manually enable power saving if desired. Signed-off-by: Andy Lutomirski <luto@mit.edu> --- This fixes what looks to me like a regression: power_level used to work but now fails silently. In 2.6.32 I think this code is going away, but this patch seems like a safe stopgap measure. iwlwifi mostly ignores mac80211's power settings, but I think advertising IEEE80211_HW_SUPPORTS_PS should be safe, even this late in the 2.6.31 cycle because: 1. This patch won't have any effect until the user requests power saving through wext or nl80211. 2. Even if that happens, the user still has to set power_level in sysfs for anything to change. 3. As far as I can tell, power_level in sysfs wasn't any safer in 2.6.29 or 2.6.30 than it will be with this patch. 4. This fixes a regression. The power_level sysfs entry is still rather odd in that setting and getting don't behave in quite the way that the user would expect (there should probably be power_level_requested and actual_power_level as separate entries), but power_level looks like it's going away soon, so this isn't worth fixing now. -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html