Re: [PATCH v2] wl1251: add sysfs interface for bluetooth coexistence mode configuration

Pali Rohár
2016-01-09T20:33:33Z

[-- Attachment #1: Type: Text/Plain, Size: 10523 bytes --]

On Saturday 26 December 2015 12:45:10 Pali Rohár wrote:
> Port the bt_coex_mode sysfs interface from wl1251 driver version
> included in the Maemo Fremantle kernel to allow bt-coexistence mode
> configuration. This enables userspace applications to set one of the
> modes
> WL1251_BT_COEX_OFF, WL1251_BT_COEX_ENABLE and
> WL1251_BT_COEX_MONOAUDIO. The default mode is WL1251_BT_COEX_OFF.
> It should be noted that this driver always enabled bt-coexistence
> before and enabled bt-coexistence directly affects the receiving
> performance, rendering it unusable in some low-signal situations.
> Especially monitor mode is affected very badly with bt-coexistence
> enabled.
>
> Signed-off-by: David Gnedt <david.gnedt@davizone.at>
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> ---
> I'm resending this patch for review again as after two years there is
> no nl80211 interface for bt coex and wl1251 on Nokia N900 needs it.
> Once there will be common interface for bt coex I can rewrite my
> patches, but I do not want to wait another 2 years...
>
> Changes:
> In v2 is sysfs node attached directly to wl1251 device instead of
> creating new platform device for sysfs node. So sysfs node is now
> available at: /sys/class/net/wlan0/device/bt_coex_mode
> ---

Hello! Can you review or comment this patch?

> drivers/net/wireless/ti/wl1251/acx.c | 43 ++++++++++++++--
> drivers/net/wireless/ti/wl1251/acx.h | 8 +--
> drivers/net/wireless/ti/wl1251/init.c | 6 +--
> drivers/net/wireless/ti/wl1251/main.c | 84
> +++++++++++++++++++++++++++++++
> drivers/net/wireless/ti/wl1251/wl1251.h | 8 +++
> 5 files changed, 137 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/wireless/ti/wl1251/acx.c
> b/drivers/net/wireless/ti/wl1251/acx.c index d6fbdda..a119d77 100644
> --- a/drivers/net/wireless/ti/wl1251/acx.c
> +++ b/drivers/net/wireless/ti/wl1251/acx.c
> @@ -539,7 +539,7 @@ out:
> return ret;
> }
>
> -int wl1251_acx_sg_enable(struct wl1251 *wl)
> +int wl1251_acx_sg_enable(struct wl1251 *wl, u8 mode)
> {
> struct acx_bt_wlan_coex *pta;
> int ret;
> @@ -550,7 +550,7 @@ int wl1251_acx_sg_enable(struct wl1251 *wl)
> if (!pta)
> return -ENOMEM;
>
> - pta->enable = SG_ENABLE;
> + pta->enable = mode;
>
> ret = wl1251_cmd_configure(wl, ACX_SG_ENABLE, pta, sizeof(*pta));
> if (ret < 0) {
> @@ -563,7 +563,7 @@ out:
> return ret;
> }
>
> -int wl1251_acx_sg_cfg(struct wl1251 *wl)
> +int wl1251_acx_sg_cfg(struct wl1251 *wl, u16 wake_up_beacon)
> {
> struct acx_bt_wlan_coex_param *param;
> int ret;
> @@ -586,7 +586,7 @@ int wl1251_acx_sg_cfg(struct wl1251 *wl)
> param->wlan_cycle_fast = PTA_CYCLE_TIME_FAST_DEF;
> param->bt_anti_starvation_period = PTA_ANTI_STARVE_PERIOD_DEF;
> param->next_bt_lp_packet = PTA_TIMEOUT_NEXT_BT_LP_PACKET_DEF;
> - param->wake_up_beacon = PTA_TIME_BEFORE_BEACON_DEF;
> + param->wake_up_beacon = wake_up_beacon;
> param->hp_dm_max_guard_time = PTA_HPDM_MAX_TIME_DEF;
> param->next_wlan_packet = PTA_TIME_OUT_NEXT_WLAN_DEF;
> param->antenna_type = PTA_ANTENNA_TYPE_DEF;
> @@ -615,6 +615,41 @@ out:
> return ret;
> }
>
> +int wl1251_acx_sg_configure(struct wl1251 *wl, bool force)
> +{
> + int ret;
> +
> + if (wl->state == WL1251_STATE_OFF && !force)
> + return 0;
> +
> + switch (wl->bt_coex_mode) {
> + case WL1251_BT_COEX_OFF:
> + ret = wl1251_acx_sg_enable(wl, SG_DISABLE);
> + if (ret)
> + break;
> + ret = wl1251_acx_sg_cfg(wl, 0);
> + break;
> + case WL1251_BT_COEX_ENABLE:
> + ret = wl1251_acx_sg_enable(wl, SG_ENABLE);
> + if (ret)
> + break;
> + ret = wl1251_acx_sg_cfg(wl, PTA_TIME_BEFORE_BEACON_DEF);
> + break;
> + case WL1251_BT_COEX_MONOAUDIO:
> + ret = wl1251_acx_sg_enable(wl, SG_ENABLE);
> + if (ret)
> + break;
> + ret = wl1251_acx_sg_cfg(wl, PTA_TIME_BEFORE_BEACON_MONO_AUDIO);
> + break;
> + default:
> + wl1251_error("Invalid BT co-ex mode!");
> + ret = -EOPNOTSUPP;
> + break;
> + }
> +
> + return ret;
> +}
> +
> int wl1251_acx_cca_threshold(struct wl1251 *wl)
> {
> struct acx_energy_detection *detection;
> diff --git a/drivers/net/wireless/ti/wl1251/acx.h
> b/drivers/net/wireless/ti/wl1251/acx.h index 2bdec38..820573c 100644
> --- a/drivers/net/wireless/ti/wl1251/acx.h
> +++ b/drivers/net/wireless/ti/wl1251/acx.h
> @@ -558,7 +558,8 @@ struct acx_bt_wlan_coex {
> #define PTA_ANTI_STARVE_PERIOD_DEF (500)
> #define PTA_ANTI_STARVE_NUM_CYCLE_DEF (4)
> #define PTA_ALLOW_PA_SD_DEF (1)
> -#define PTA_TIME_BEFORE_BEACON_DEF (6300)
> +#define PTA_TIME_BEFORE_BEACON_DEF (500)
> +#define PTA_TIME_BEFORE_BEACON_MONO_AUDIO (6300)
> #define PTA_HPDM_MAX_TIME_DEF (1600)
> #define PTA_TIME_OUT_NEXT_WLAN_DEF (2550)
> #define PTA_AUTO_MODE_NO_CTS_DEF (0)
> @@ -1470,8 +1471,9 @@ int wl1251_acx_rts_threshold(struct wl1251 *wl,
> u16 rts_threshold); int wl1251_acx_beacon_filter_opt(struct wl1251
> *wl, bool enable_filter); int wl1251_acx_beacon_filter_table(struct
> wl1251 *wl);
> int wl1251_acx_conn_monit_params(struct wl1251 *wl);
> -int wl1251_acx_sg_enable(struct wl1251 *wl);
> -int wl1251_acx_sg_cfg(struct wl1251 *wl);
> +int wl1251_acx_sg_enable(struct wl1251 *wl, u8 mode);
> +int wl1251_acx_sg_cfg(struct wl1251 *wl, u16 wake_up_beacon);
> +int wl1251_acx_sg_configure(struct wl1251 *wl, bool force);
> int wl1251_acx_cca_threshold(struct wl1251 *wl);
> int wl1251_acx_bcn_dtim_options(struct wl1251 *wl);
> int wl1251_acx_aid(struct wl1251 *wl, u16 aid);
> diff --git a/drivers/net/wireless/ti/wl1251/init.c
> b/drivers/net/wireless/ti/wl1251/init.c index 1d799bf..f8a2ea9
> 100644
> --- a/drivers/net/wireless/ti/wl1251/init.c
> +++ b/drivers/net/wireless/ti/wl1251/init.c
> @@ -162,11 +162,7 @@ int wl1251_hw_init_pta(struct wl1251 *wl)
> {
> int ret;
>
> - ret = wl1251_acx_sg_enable(wl);
> - if (ret < 0)
> - return ret;
> -
> - ret = wl1251_acx_sg_cfg(wl);
> + ret = wl1251_acx_sg_configure(wl, true);
> if (ret < 0)
> return ret;
>
> diff --git a/drivers/net/wireless/ti/wl1251/main.c
> b/drivers/net/wireless/ti/wl1251/main.c index cd47779..8f53e43
> 100644
> --- a/drivers/net/wireless/ti/wl1251/main.c
> +++ b/drivers/net/wireless/ti/wl1251/main.c
> @@ -1383,6 +1383,77 @@ static const struct ieee80211_ops wl1251_ops =
> { .get_survey = wl1251_op_get_survey,
> };
>
> +static ssize_t wl1251_sysfs_show_bt_coex_mode(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct wl1251 *wl = dev_get_drvdata(dev);
> + ssize_t len;
> +
> + /* FIXME: what's the maximum length of buf? page size?*/
> + len = 500;
> +
> + mutex_lock(&wl->mutex);
> + len = snprintf(buf, len, "%d\n\n%d - off\n%d - on\n%d -
> monoaudio\n", + wl->bt_coex_mode,
> + WL1251_BT_COEX_OFF,
> + WL1251_BT_COEX_ENABLE,
> + WL1251_BT_COEX_MONOAUDIO);
> + mutex_unlock(&wl->mutex);
> +
> + return len;
> +}
> +
> +static ssize_t wl1251_sysfs_store_bt_coex_mode(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct wl1251 *wl = dev_get_drvdata(dev);
> + unsigned long res;
> + int ret;
> +
> + ret = kstrtoul(buf, 10, &res);
> +
> + if (ret < 0) {
> + wl1251_warning("incorrect value written to bt_coex_mode");
> + return count;
> + }
> +
> + mutex_lock(&wl->mutex);
> +
> + if (res == wl->bt_coex_mode)
> + goto out;
> +
> + switch (res) {
> + case WL1251_BT_COEX_OFF:
> + case WL1251_BT_COEX_ENABLE:
> + case WL1251_BT_COEX_MONOAUDIO:
> + wl->bt_coex_mode = res;
> + break;
> + default:
> + wl1251_warning("incorrect value written to bt_coex_mode");
> + goto out;
> + }
> +
> + if (wl->state == WL1251_STATE_OFF)
> + goto out;
> +
> + ret = wl1251_ps_elp_wakeup(wl);
> + if (ret < 0)
> + goto out;
> +
> + wl1251_acx_sg_configure(wl, false);
> + wl1251_ps_elp_sleep(wl);
> +
> +out:
> + mutex_unlock(&wl->mutex);
> + return count;
> +}
> +
> +static DEVICE_ATTR(bt_coex_mode, S_IRUGO | S_IWUSR,
> + wl1251_sysfs_show_bt_coex_mode,
> + wl1251_sysfs_store_bt_coex_mode);
> +
> static int wl1251_read_eeprom_byte(struct wl1251 *wl, off_t offset,
> u8 *data) {
> unsigned long timeout;
> @@ -1467,6 +1538,7 @@ static int wl1251_register_hw(struct wl1251
> *wl)
>
> int wl1251_init_ieee80211(struct wl1251 *wl)
> {
> + struct device *dev = wiphy_dev(wl->hw->wiphy);
> int ret;
>
> /* The tx descriptor buffer and the TKIP space */
> @@ -1493,6 +1565,13 @@ int wl1251_init_ieee80211(struct wl1251 *wl)
> if (ret)
> goto out;
>
> + /* Create sysfs file to control bt coex state */
> + ret = device_create_file(dev, &dev_attr_bt_coex_mode);
> + if (ret < 0) {
> + wl1251_error("failed to create sysfs file bt_coex_mode");
> + goto out;
> + }
> +
> wl1251_debugfs_init(wl);
> wl1251_notice("initialized");
>
> @@ -1549,6 +1628,7 @@ struct ieee80211_hw *wl1251_alloc_hw(void)
> wl->beacon_int = WL1251_DEFAULT_BEACON_INT;
> wl->dtim_period = WL1251_DEFAULT_DTIM_PERIOD;
> wl->vif = NULL;
> + wl->bt_coex_mode = WL1251_BT_COEX_OFF;
>
> for (i = 0; i < FW_TX_CMPLT_BLOCK_SIZE; i++)
> wl->tx_frames[i] = NULL;
> @@ -1584,6 +1664,10 @@ EXPORT_SYMBOL_GPL(wl1251_alloc_hw);
>
> int wl1251_free_hw(struct wl1251 *wl)
> {
> + struct device *dev = wiphy_dev(wl->hw->wiphy);
> +
> + device_remove_file(dev, &dev_attr_bt_coex_mode);
> +
> ieee80211_unregister_hw(wl->hw);
>
> wl1251_debugfs_exit(wl);
> diff --git a/drivers/net/wireless/ti/wl1251/wl1251.h
> b/drivers/net/wireless/ti/wl1251/wl1251.h index 16dae52..b8b7ab7
> 100644
> --- a/drivers/net/wireless/ti/wl1251/wl1251.h
> +++ b/drivers/net/wireless/ti/wl1251/wl1251.h
> @@ -258,6 +258,12 @@ struct wl1251_debugfs {
> struct dentry *excessive_retries;
> };
>
> +enum wl1251_bt_coex_mode {
> + WL1251_BT_COEX_OFF,
> + WL1251_BT_COEX_ENABLE,
> + WL1251_BT_COEX_MONOAUDIO
> +};
> +
> struct wl1251_if_operations {
> void (*read)(struct wl1251 *wl, int addr, void *buf, size_t len);
> void (*write)(struct wl1251 *wl, int addr, void *buf, size_t len);
> @@ -394,6 +400,8 @@ struct wl1251 {
>
> struct ieee80211_vif *vif;
>
> + enum wl1251_bt_coex_mode bt_coex_mode;
> +
> u32 chip_id;
> char fw_ver[21];

--
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]