Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1415187 > unrolled thread

[PATCH 0/2] mwifiex: add .get_tx_power and .get_antenna cfg80211 operations

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/2] mwifiex: add .get_tx_power and .get_antenna cfg80211 operations Javier Martinez Canillas <javier@osg.samsung.com> - 2016-06-06 17:00 +0200
    [PATCH 1/2] mwifiex: add a cfg80211 .get_tx_power operation callback Javier Martinez Canillas <javier@osg.samsung.com> - 2016-06-06 17:00 +0200
    [PATCH 2/2] mwifiex: add a cfg80211 .get_antenna operation callback Javier Martinez Canillas <javier@osg.samsung.com> - 2016-06-06 17:10 +0200

#1415187 — [PATCH 0/2] mwifiex: add .get_tx_power and .get_antenna cfg80211 operations

Hello,

Since commit de3bb771f471 ("cfg80211: add more warnings for inconsistent
ops") the wireless core warns if a driver implements a cfg80211 callback
but doesn't implements the inverse operation. The mwifiex driver has two
set operations defined without the get counterpat so warnings are shown:

WARNING: CPU: 3 PID: 127 at net/wireless/core.c:366 wiphy_new_nm+0x66c/0x6ac
Modules linked in: mwifiex_sdio mwifiex
CPU: 3 PID: 127 Comm: kworker/3:1 Tainted: G        W       4.7.0-rc1-next-20160531-00006-g569df5b983f3
Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
Workqueue: events request_firmware_work_func
[<c010e1ac>] (unwind_backtrace) from [<c010af38>] (show_stack+0x10/0x14)
[<c010af38>] (show_stack) from [<c0323b9c>] (dump_stack+0x88/0x9c)
[<c0323b9c>] (dump_stack) from [<c011a828>] (__warn+0xe8/0x100)
[<c011a828>] (__warn) from [<c011a8f0>] (warn_slowpath_null+0x20/0x28)
[<c011a8f0>] (warn_slowpath_null) from [<c06a42d4>] (wiphy_new_nm+0x66c/0x6ac)
[<c06a42d4>] (wiphy_new_nm) from [<bf1c24cc>] (mwifiex_register_cfg80211+0x28/0x3f0 [mwifiex])
[<bf1c24cc>] (mwifiex_register_cfg80211 [mwifiex]) from [<bf1a0018>] (mwifiex_fw_dpc+0x2b0/0x474 [mwifiex])
[<bf1a0018>] (mwifiex_fw_dpc [mwifiex]) from [<c040eb74>] (request_firmware_work_func+0x30/0x58)
[<c040eb74>] (request_firmware_work_func) from [<c012fe90>] (process_one_work+0x124/0x338)
[<c012fe90>] (process_one_work) from [<c01300dc>] (worker_thread+0x38/0x4d4)
[<c01300dc>] (worker_thread) from [<c01353b8>] (kthread+0xdc/0xf4)
[<c01353b8>] (kthread) from [<c0107978>] (ret_from_fork+0x14/0x3c)

This patch series fixes the warnings by implementing callbacks for the
get operations.

There isn't public documentation about the firmware interface so the
data is only queried to the firmware if the driver already supports
it as is the case of the Tx power. For the antenna the driver only
supports the get command so the get returns the cached values that
were previously set. This can be changed to query the firmware as a
followup by someone with access to the docs, if this is supported.

Best regards,
Javier


Javier Martinez Canillas (2):
  mwifiex: add a cfg80211 .get_tx_power operation callback
  mwifiex: add a cfg80211 .get_antenna operation callback

 drivers/net/wireless/marvell/mwifiex/cfg80211.c | 50 ++++++++++++++++++++++++-
 drivers/net/wireless/marvell/mwifiex/main.h     |  1 +
 2 files changed, 49 insertions(+), 2 deletions(-)

-- 
2.5.5

[toc] | [next] | [standalone]

#1415194 — [PATCH 1/2] mwifiex: add a cfg80211 .get_tx_power operation callback

The mwifiex driver implements a cfg80211 .set_tx_power operation handler
but doesn't have the inverse .get_tx_power callback.

This not only has the effect that the Tx power can't be reported to user
space tools such as iwconfig and iwlist but also that the wireless core
prints a warning when a new wiphy is created due an cfg80211 operation
being implemented without its counterpart.

After this patch, the Tx power is properly reported to user-space tools:

$ iwlist mlan0 txpower
mlan0     unknown transmit-power information.

          Current Tx-Power=13 dBm       (19 mW)

and also the following warning isn't shown anymore on the driver probe:

WARNING: CPU: 3 PID: 127 at net/wireless/core.c:366 wiphy_new_nm+0x66c/0x6ac
Modules linked in: mwifiex_sdio mwifiex
CPU: 3 PID: 127 Comm: kworker/3:1 Tainted: G        W       4.7.0-rc1-next-20160531-00006-g569df5b983f3
Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
Workqueue: events request_firmware_work_func
[<c010e1ac>] (unwind_backtrace) from [<c010af38>] (show_stack+0x10/0x14)
[<c010af38>] (show_stack) from [<c0323b9c>] (dump_stack+0x88/0x9c)
[<c0323b9c>] (dump_stack) from [<c011a828>] (__warn+0xe8/0x100)
[<c011a828>] (__warn) from [<c011a8f0>] (warn_slowpath_null+0x20/0x28)
[<c011a8f0>] (warn_slowpath_null) from [<c06a42d4>] (wiphy_new_nm+0x66c/0x6ac)
[<c06a42d4>] (wiphy_new_nm) from [<bf1c24cc>] (mwifiex_register_cfg80211+0x28/0x3f0 [mwifiex])
[<bf1c24cc>] (mwifiex_register_cfg80211 [mwifiex]) from [<bf1a0018>] (mwifiex_fw_dpc+0x2b0/0x474 [mwifiex])
[<bf1a0018>] (mwifiex_fw_dpc [mwifiex]) from [<c040eb74>] (request_firmware_work_func+0x30/0x58)
[<c040eb74>] (request_firmware_work_func) from [<c012fe90>] (process_one_work+0x124/0x338)
[<c012fe90>] (process_one_work) from [<c01300dc>] (worker_thread+0x38/0x4d4)
[<c01300dc>] (worker_thread) from [<c01353b8>] (kthread+0xdc/0xf4)
[<c01353b8>] (kthread) from [<c0107978>] (ret_from_fork+0x14/0x3c)

Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
---

 drivers/net/wireless/marvell/mwifiex/cfg80211.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index ff948a922222..b17f3d09a2c7 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -377,6 +377,29 @@ mwifiex_cfg80211_set_tx_power(struct wiphy *wiphy,
 }
 
 /*
+ * CFG802.11 operation handler to get Tx power.
+ */
+static int
+mwifiex_cfg80211_get_tx_power(struct wiphy *wiphy,
+			      struct wireless_dev *wdev,
+			      int *dbm)
+{
+	struct mwifiex_adapter *adapter = mwifiex_cfg80211_get_adapter(wiphy);
+	struct mwifiex_private *priv = mwifiex_get_priv(adapter,
+							MWIFIEX_BSS_ROLE_ANY);
+	int ret = mwifiex_send_cmd(priv, HostCmd_CMD_RF_TX_PWR,
+				   HostCmd_ACT_GEN_GET, 0, NULL, true);
+
+	if (ret < 0)
+		return ret;
+
+	/* tx_power_level is set in HostCmd_CMD_RF_TX_PWR command handler */
+	*dbm = priv->tx_power_level;
+
+	return 0;
+}
+
+/*
  * CFG802.11 operation handler to set Power Save option.
  *
  * The timeout value, if provided, is currently ignored.
@@ -3940,6 +3963,7 @@ static struct cfg80211_ops mwifiex_cfg80211_ops = {
 	.set_default_key = mwifiex_cfg80211_set_default_key,
 	.set_power_mgmt = mwifiex_cfg80211_set_power_mgmt,
 	.set_tx_power = mwifiex_cfg80211_set_tx_power,
+	.get_tx_power = mwifiex_cfg80211_get_tx_power,
 	.set_bitrate_mask = mwifiex_cfg80211_set_bitrate_mask,
 	.start_ap = mwifiex_cfg80211_start_ap,
 	.stop_ap = mwifiex_cfg80211_stop_ap,
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1415201 — [PATCH 2/2] mwifiex: add a cfg80211 .get_antenna operation callback

Since commit de3bb771f471 ("cfg80211: add more warnings for inconsistent
ops") the wireless core warns if a driver implements a cfg80211 callback
but doesn't implements the inverse operation.

The mwifiex driver defines a .set_antenna handler but not a .get_antenna
so this not only makes the core to print a warning when creating a new
wiphy but also the antenna isn't reported to user-space apps such as iw.

Unfortunately the driver doesn't have support to query the antena to the
firmware and there isn't public documentation about the interface so this
patch caches the values set in .set_antenna and reports those back in get.

With this patch, the wireless core does not warn anymore and the antenna
is properly reported once has been set:

$ iw phy phy0 set antenna 0x1

$ iw phy phy0 info | grep Antennas
        Available Antennas: TX 0x3 RX 0x3
        Configured Antennas: TX 0x1 RX 0x1

Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>

---
Hello,

Even though this approach prevents the warning and allows to reports the
antenna values, it would be better if instead of caching the set values,
these are asked to the firmware for each .get_antenna callback.

But the driver currently only implements a set antenna command and the
firmware interface documentation is not public so isn't clear if the
firmware doesn't support or is just that the driver didn't implement it.

Best regards,
Javier

 drivers/net/wireless/marvell/mwifiex/cfg80211.c | 26 +++++++++++++++++++++++--
 drivers/net/wireless/marvell/mwifiex/main.h     |  1 +
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index b17f3d09a2c7..e9efbc852d23 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -1771,6 +1771,7 @@ mwifiex_cfg80211_set_antenna(struct wiphy *wiphy, u32 tx_ant, u32 rx_ant)
 	struct mwifiex_private *priv = mwifiex_get_priv(adapter,
 							MWIFIEX_BSS_ROLE_ANY);
 	struct mwifiex_ds_ant_cfg ant_cfg;
+	int ret;
 
 	if (!tx_ant || !rx_ant)
 		return -EOPNOTSUPP;
@@ -1823,8 +1824,28 @@ mwifiex_cfg80211_set_antenna(struct wiphy *wiphy, u32 tx_ant, u32 rx_ant)
 	ant_cfg.tx_ant = tx_ant;
 	ant_cfg.rx_ant = rx_ant;
 
-	return mwifiex_send_cmd(priv, HostCmd_CMD_RF_ANTENNA,
-				HostCmd_ACT_GEN_SET, 0, &ant_cfg, true);
+	ret = mwifiex_send_cmd(priv, HostCmd_CMD_RF_ANTENNA,
+			       HostCmd_ACT_GEN_SET, 0, &ant_cfg, true);
+
+	if (ret < 0)
+		return ret;
+
+	priv->ant_cfg.tx_ant = tx_ant;
+	priv->ant_cfg.rx_ant = rx_ant;
+
+	return 0;
+}
+
+static int
+mwifiex_cfg80211_get_antenna(struct wiphy *wiphy, u32 *tx_ant, u32 *rx_ant)
+{
+	struct mwifiex_adapter *adapter = mwifiex_cfg80211_get_adapter(wiphy);
+	struct mwifiex_private *priv = mwifiex_get_priv(adapter,
+							MWIFIEX_BSS_ROLE_ANY);
+	*tx_ant = priv->ant_cfg.tx_ant;
+	*rx_ant = priv->ant_cfg.rx_ant;
+
+	return 0;
 }
 
 /* cfg80211 operation handler for stop ap.
@@ -3970,6 +3991,7 @@ static struct cfg80211_ops mwifiex_cfg80211_ops = {
 	.change_beacon = mwifiex_cfg80211_change_beacon,
 	.set_cqm_rssi_config = mwifiex_cfg80211_set_cqm_rssi_config,
 	.set_antenna = mwifiex_cfg80211_set_antenna,
+	.get_antenna = mwifiex_cfg80211_get_antenna,
 	.del_station = mwifiex_cfg80211_del_station,
 	.sched_scan_start = mwifiex_cfg80211_sched_scan_start,
 	.sched_scan_stop = mwifiex_cfg80211_sched_scan_stop,
diff --git a/drivers/net/wireless/marvell/mwifiex/main.h b/drivers/net/wireless/marvell/mwifiex/main.h
index 0207af00be42..b4fe10cbb34d 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.h
+++ b/drivers/net/wireless/marvell/mwifiex/main.h
@@ -668,6 +668,7 @@ struct mwifiex_private {
 	struct delayed_work dfs_chan_sw_work;
 	struct cfg80211_beacon_data beacon_after;
 	struct mwifiex_11h_intf_state state_11h;
+	struct mwifiex_ds_ant_cfg ant_cfg;
 	struct mwifiex_ds_mem_rw mem_rw;
 	struct sk_buff_head bypass_txq;
 	struct mwifiex_user_scan_chan hidden_chan[MWIFIEX_USER_SCAN_CHAN_MAX];
-- 
2.5.5

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web