Re: brcfmac: add possibility to turn off powersave on wiphy
From: Dan Williams <hidden>
Date: 2017-07-21 16:51:58
On Fri, 2017-07-21 at 16:56 +0200, Rafal wrote:
I have a board with ap6212 chip and I have encountered two problems with the brcmfmac driver operating with this device. First problem I describe below, second one in separate mail. Namely, I have noticed quite poor connection with the device despite good signal strength. Ping to the device was very uneven, about 50 - 100 ms in average, 10 - 20% of packets loss. After some investigations I have found that problem is caused by powersave feature on wiphy (BRCMF_C_SET_PM command). When the value is set to PM_OFF, the problem does not appear - ping is quite stable, ~2ms. When set to PM_FAST, the ping is bad. The brcmfmac driver currently does not have possibility to turn off the powersave feature. I have added the possibility on 4.11.6 kernel
I don't think that's true? The driver implements the nl80211 set_power_mgmt hook, which lets you turn off powersave with 'iw'. That seems like a much better option than a module parameter. I know other drivers have the module parameter, but I personally consider that legacy/holdover, given that we have runtime toggles for this kind of thing. brcmf_cfg80211_set_power_mgmt() should do what you need, right? Dan
quoted hunk ↗ jump to hunk
in my sandbox, but maybe the change is worth to add in general. I'm providing my patch for reference. This change allows to turn off powersave in two ways. First one, by specify "brcm,powersave-default- off" option in OF devicetree. Second one - as a module parameter. The parameter is named powersave_default and is tri-state: value >0 enables powersave value =0 disables powersave value <0 (the default) does not override powersave value, i.e. value from devicetree or driver default is in effect. Rafal diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c index 944b83c..86cd1f7 100644--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c@@ -5702,7 +5702,7 @@ static s32 wl_init_priv(structbrcmf_cfg80211_info *cfg) s32 err = 0; cfg->scan_request = NULL; - cfg->pwr_save = true; + cfg->pwr_save = !cfg->pub->settings->powersave_default_off; cfg->active_scan = true; /* we do active scan per default */ cfg->dongle_up = false; /* dongle is not up yet */ err = brcmf_init_priv_mem(cfg);@@ -6450,8 +6450,9 @@ static int brcmf_setup_wiphy(struct wiphy*wiphy, struct brcmf_if *ifp) BIT(NL80211_BSS_SELECT_ATTR_BAN D_PREF) | BIT(NL80211_BSS_SELECT_ATTR_RSS I_ADJUST); - wiphy->flags |= WIPHY_FLAG_PS_ON_BY_DEFAULT | - WIPHY_FLAG_OFFCHAN_TX | + if( ! ifp->drvr->settings->powersave_default_off ) + wiphy->flags |= WIPHY_FLAG_PS_ON_BY_DEFAULT; + wiphy->flags |= WIPHY_FLAG_OFFCHAN_TX | WIPHY_FLAG_HAS_REMAIN_ON_CHANNEL; if (brcmf_feat_is_enabled(ifp, BRCMF_FEAT_TDLS)) wiphy->flags |= WIPHY_FLAG_SUPPORTS_TDLS; diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c index 33b133f..191424e 100644--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c@@ -80,6 +80,10 @@ module_param_named(ignore_probe_fail,brcmf_ignore_probe_fail, int, 0); MODULE_PARM_DESC(ignore_probe_fail, "always succeed probe for debugging"); #endif +static int brcmf_powersave_default = -1; +module_param_named(powersave_default, brcmf_powersave_default, int, 0); +MODULE_PARM_DESC(powersave_default, "Set powersave default on/off on wiphy"); + static struct brcmfmac_platform_data *brcmfmac_pdata; struct brcmf_mp_global_t brcmf_mp_global;@@ -319,6 +323,8 @@ struct brcmf_mp_device*brcmf_get_module_param(struct device *dev, /* No platform data for this device, try OF (Open Firwmare) */ brcmf_of_probe(dev, bus_type, settings); } + if( brcmf_powersave_default >= 0 ) + settings->powersave_default_off = !brcmf_powersave_default; return settings; } diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h index a62f8e7..ab67fcf 100644--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h@@ -59,6 +59,7 @@ struct brcmf_mp_device {int fcmode; bool roamoff; bool ignore_probe_fail; + bool powersave_default_off; struct brcmfmac_pd_cc *country_codes; union { struct brcmfmac_sdio_pd sdio;diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.cb/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c index aee6e59..904ba11 100644--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c@@ -39,6 +39,9 @@ void brcmf_of_probe(struct device *dev, enumbrcmf_bus_type bus_type, if (of_property_read_u32(np, "brcm,drive-strength", &val) == 0) sdio->drive_strength = val; + settings->powersave_default_off = of_property_read_bool(np, + "brcm,powersave-default-off"); + /* make sure there are interrupts defined in the node */ if (!of_find_property(np, "interrupts", NULL)) return;