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 > 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(struct > brcmf_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.c > b/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, enum > brcmf_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;