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;
