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

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 <redacted>
---

 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

From: Shengzhen Li <redacted>

Most cfg80211 operations are just a wrappers to functions defined in the
sta_ioctl.c file, so for consistency move the .get_tx_power logic there.

Signed-off-by: Shengzhen Li <redacted>
Signed-off-by: Amitkumar Karwar <redacted>
[javier: update the subject line and commit message]
Signed-off-by: Javier Martinez Canillas <redacted>

---

 drivers/net/wireless/marvell/mwifiex/cfg80211.c  | 14 +++-----------
 drivers/net/wireless/marvell/mwifiex/main.h      |  2 ++
 drivers/net/wireless/marvell/mwifiex/sta_ioctl.c | 18 ++++++++++++++++++
 3 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index b17f3d09a2c7..ff3f63ed95e1 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c

@@ -385,18 +385,10 @@ mwifiex_cfg80211_get_tx_power(struct wiphy *wiphy,
                              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;
+       struct mwifiex_private *priv;

-       return 0;
+       priv = mwifiex_get_priv(adapter, MWIFIEX_BSS_ROLE_ANY);
+       return mwifiex_get_tx_power(priv, dbm);
 }

 /*

diff --git a/drivers/net/wireless/marvell/mwifiex/main.h b/drivers/net/wireless/marvell/mwifiex/main.h
index 0207af00be42..79c28cfb7780 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.h
+++ b/drivers/net/wireless/marvell/mwifiex/main.h

@@ -1464,6 +1464,8 @@ int mwifiex_drv_get_driver_version(struct mwifiex_adapter *adapter,
 int mwifiex_set_tx_power(struct mwifiex_private *priv,
                         struct mwifiex_power_cfg *power_cfg);

+int mwifiex_get_tx_power(struct mwifiex_private *priv, int *dbm);
+
 int mwifiex_main_process(struct mwifiex_adapter *);

 int mwifiex_queue_tx_pkt(struct mwifiex_private *priv, struct sk_buff *skb);

diff --git a/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c b/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
index 8e0862657122..70ff9b805b5b 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c

@@ -775,6 +775,24 @@ int mwifiex_set_tx_power(struct mwifiex_private *priv,
 }

 /*
+ * IOCTL request handler to get tx power configuration.
+ *
+ * This function prepares the correct firmware command and
+ * issues it.
+ */
+int mwifiex_get_tx_power(struct mwifiex_private *priv, int *dbm)
+{
+       int ret;
+
+       ret = mwifiex_send_cmd(priv, HostCmd_CMD_TXPWR_CFG,
+                              HostCmd_ACT_GEN_GET, 0, NULL, true);
+
+       *dbm = priv->tx_power_level;
+
+       return ret;
+}
+
+/*
  * IOCTL request handler to get power save mode.
  *
  * This function prepares the correct firmware command and
--

2.5.5

From: Shengzhen Li <redacted>

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.

This patch queries the antenna to the firmware so is properly reported to
user-space. With this patch, the wireless core does not warn anymore and:

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

Signed-off-by: Shengzhen Li <redacted>
Signed-off-by: Amitkumar Karwar <redacted>
[javier: expand the commit message]
Signed-off-by: Javier Martinez Canillas <redacted>

---

 drivers/net/wireless/marvell/mwifiex/cfg80211.c    | 16 +++++++
 drivers/net/wireless/marvell/mwifiex/fw.h          |  3 ++
 drivers/net/wireless/marvell/mwifiex/init.c        |  2 +
 drivers/net/wireless/marvell/mwifiex/main.h        |  2 +
 drivers/net/wireless/marvell/mwifiex/sta_cmd.c     | 50 +++++++++++++++-------
 drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c | 10 +++--
 6 files changed, 64 insertions(+), 19 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index ff3f63ed95e1..ff1eefe5087b 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c

@@ -1819,6 +1819,21 @@ mwifiex_cfg80211_set_antenna(struct wiphy *wiphy, u32 tx_ant, u32 rx_ant)
                                HostCmd_ACT_GEN_SET, 0, &ant_cfg, true);
 }

+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);
+       mwifiex_send_cmd(priv, HostCmd_CMD_RF_ANTENNA,
+                        HostCmd_ACT_GEN_GET, 0, NULL, true);
+
+       *tx_ant = priv->tx_ant;
+       *rx_ant = priv->rx_ant;
+
+       return 0;
+}
+
 /* cfg80211 operation handler for stop ap.
  * Function stops BSS running at uAP interface.
  */

@@ -3962,6 +3977,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/fw.h b/drivers/net/wireless/marvell/mwifiex/fw.h
index 8e4145abdbfa..cef72343f5b6 100644
--- a/drivers/net/wireless/marvell/mwifiex/fw.h
+++ b/drivers/net/wireless/marvell/mwifiex/fw.h

@@ -462,6 +462,9 @@ enum P2P_MODES {
 #define HostCmd_ACT_SET_RX              0x0001
 #define HostCmd_ACT_SET_TX              0x0002
 #define HostCmd_ACT_SET_BOTH            0x0003
+#define HostCmd_ACT_GET_RX              0x0004
+#define HostCmd_ACT_GET_TX              0x0008
+#define HostCmd_ACT_GET_BOTH            0x000c

 #define RF_ANTENNA_AUTO                 0xFFFF

diff --git a/drivers/net/wireless/marvell/mwifiex/init.c b/drivers/net/wireless/marvell/mwifiex/init.c
index 78c532f0d286..fbaf49056746 100644
--- a/drivers/net/wireless/marvell/mwifiex/init.c
+++ b/drivers/net/wireless/marvell/mwifiex/init.c

@@ -110,6 +110,8 @@ int mwifiex_init_priv(struct mwifiex_private *priv)
        priv->tx_power_level = 0;
        priv->max_tx_power_level = 0;
        priv->min_tx_power_level = 0;
+       priv->tx_ant = 0;
+       priv->rx_ant = 0;
        priv->tx_rate = 0;
        priv->rxpd_htinfo = 0;
        priv->rxpd_rate = 0;

diff --git a/drivers/net/wireless/marvell/mwifiex/main.h b/drivers/net/wireless/marvell/mwifiex/main.h
index 79c28cfb7780..2ae7ff74e1c6 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.h
+++ b/drivers/net/wireless/marvell/mwifiex/main.h

@@ -533,6 +533,8 @@ struct mwifiex_private {
        u16 tx_power_level;
        u8 max_tx_power_level;
        u8 min_tx_power_level;
+       u32 tx_ant;
+       u32 rx_ant;
        u8 tx_rate;
        u8 tx_htinfo;
        u8 rxpd_htinfo;

diff --git a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
index e436574b1698..8c658495bf66 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c

@@ -313,23 +313,41 @@ static int mwifiex_cmd_rf_antenna(struct mwifiex_private *priv,

        cmd->command = cpu_to_le16(HostCmd_CMD_RF_ANTENNA);

-       if (cmd_action != HostCmd_ACT_GEN_SET)
-               return 0;
-
-       if (priv->adapter->hw_dev_mcs_support == HT_STREAM_2X2) {
-               cmd->size = cpu_to_le16(sizeof(struct host_cmd_ds_rf_ant_mimo) +
-                                       S_DS_GEN);
-               ant_mimo->action_tx = cpu_to_le16(HostCmd_ACT_SET_TX);
-               ant_mimo->tx_ant_mode = cpu_to_le16((u16)ant_cfg->tx_ant);
-               ant_mimo->action_rx = cpu_to_le16(HostCmd_ACT_SET_RX);
-               ant_mimo->rx_ant_mode = cpu_to_le16((u16)ant_cfg->rx_ant);
-       } else {
-               cmd->size = cpu_to_le16(sizeof(struct host_cmd_ds_rf_ant_siso) +
-                                       S_DS_GEN);
-               ant_siso->action = cpu_to_le16(HostCmd_ACT_SET_BOTH);
-               ant_siso->ant_mode = cpu_to_le16((u16)ant_cfg->tx_ant);
+       switch (cmd_action) {
+       case HostCmd_ACT_GEN_SET:
+               if (priv->adapter->hw_dev_mcs_support == HT_STREAM_2X2) {
+                       cmd->size = cpu_to_le16(sizeof(struct
+                                               host_cmd_ds_rf_ant_mimo)
+                                               + S_DS_GEN);
+                       ant_mimo->action_tx = cpu_to_le16(HostCmd_ACT_SET_TX);
+                       ant_mimo->tx_ant_mode = cpu_to_le16((u16)ant_cfg->
+                                                           tx_ant);
+                       ant_mimo->action_rx = cpu_to_le16(HostCmd_ACT_SET_RX);
+                       ant_mimo->rx_ant_mode = cpu_to_le16((u16)ant_cfg->
+                                                           rx_ant);
+               } else {
+                       cmd->size = cpu_to_le16(sizeof(struct
+                                               host_cmd_ds_rf_ant_siso) +
+                                               S_DS_GEN);
+                       ant_siso->action = cpu_to_le16(HostCmd_ACT_SET_BOTH);
+                       ant_siso->ant_mode = cpu_to_le16((u16)ant_cfg->tx_ant);
+               }
+               break;
+       case HostCmd_ACT_GEN_GET:
+               if (priv->adapter->hw_dev_mcs_support == HT_STREAM_2X2) {
+                       cmd->size = cpu_to_le16(sizeof(struct
+                                               host_cmd_ds_rf_ant_mimo) +
+                                               S_DS_GEN);
+                       ant_mimo->action_tx = cpu_to_le16(HostCmd_ACT_GET_TX);
+                       ant_mimo->action_rx = cpu_to_le16(HostCmd_ACT_GET_RX);
+               } else {
+                       cmd->size = cpu_to_le16(sizeof(struct
+                                               host_cmd_ds_rf_ant_siso) +
+                                               S_DS_GEN);
+                       ant_siso->action = cpu_to_le16(HostCmd_ACT_GET_BOTH);
+               }
+               break;
        }
-
        return 0;
 }

diff --git a/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c b/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c
index d18c7979d723..11f19b668e13 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c

@@ -469,7 +469,9 @@ static int mwifiex_ret_rf_antenna(struct mwifiex_private *priv,
        struct host_cmd_ds_rf_ant_siso *ant_siso = &resp->params.ant_siso;
        struct mwifiex_adapter *adapter = priv->adapter;

-       if (adapter->hw_dev_mcs_support == HT_STREAM_2X2)
+       if (adapter->hw_dev_mcs_support == HT_STREAM_2X2) {
+               priv->tx_ant = le16_to_cpu(ant_mimo->tx_ant_mode);
+               priv->rx_ant = le16_to_cpu(ant_mimo->rx_ant_mode);
                mwifiex_dbg(adapter, INFO,
                            "RF_ANT_RESP: Tx action = 0x%x, Tx Mode = 0x%04x\t"
                            "Rx action = 0x%x, Rx Mode = 0x%04x\n",

@@ -477,12 +479,14 @@ static int mwifiex_ret_rf_antenna(struct mwifiex_private *priv,
                            le16_to_cpu(ant_mimo->tx_ant_mode),
                            le16_to_cpu(ant_mimo->action_rx),
                            le16_to_cpu(ant_mimo->rx_ant_mode));
-       else
+       } else {
+               priv->tx_ant = le16_to_cpu(ant_siso->ant_mode);
+               priv->rx_ant = le16_to_cpu(ant_siso->ant_mode);
                mwifiex_dbg(adapter, INFO,
                            "RF_ANT_RESP: action = 0x%x, Mode = 0x%04x\n",
                            le16_to_cpu(ant_siso->action),
                            le16_to_cpu(ant_siso->ant_mode));
-
+       }
        return 0;
 }

--

2.5.5