driverdev-devel.linuxdriverproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/10] staging: wfx: introduce nl80211 vendor extensions
@ 2020-05-26 17:18 Jerome Pouiller
  2020-05-26 17:18 ` [PATCH 01/10] staging: wfx: drop unused variable Jerome Pouiller
                   ` (11 more replies)
  0 siblings, 12 replies; 15+ messages in thread
From: Jerome Pouiller @ 2020-05-26 17:18 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, David S . Miller, Kalle Valo

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

Hello,

This series introduces some nl80211 vendor extensions to the wfx driver.

This series may lead to some discussions:

  1. Patch 7 allows to change the dynamic PS timeout. I have found
     an API in wext (cfg80211_wext_siwpower()) that do more or less the
     same thing. However, I have not found any equivalent in nl80211. Is it
     expected or this API should be ported to nl80211?

  2. The device The device allows to do Packet Traffic Arbitration (PTA or
     also Coex). This feature allows the device to communicate with another
     RF device in order to share the access to the RF. The patch 9 provides
     a way to configure that. However, I think that this chip is not the
     only one to provide this feature. Maybe a standard way to change
     these parameters should be provided?

  3. For these vendor extensions, I have used the new policy introduced by
     the commit 901bb989185516 ("nl80211: require and validate vendor
     command policy"). However, it seems that my version of 'iw' is not
     able to follow this new policy (it does not pack the netlink
     attributes into a NLA_NESTED). I could develop a tool specifically for
     that API, but it is not very handy. So, in patch 10, I have also
     introduced an API for compatibility with iw. Any comments about this?


Jérôme Pouiller (10):
  staging: wfx: drop unused variable
  staging: wfx: do not declare variables inside loops
  staging: wfx: drop unused function wfx_pending_requeue()
  staging: wfx: add support for tx_power_loop
  staging: wfx: retrieve the PS status from the vif
  staging: wfx: split wfx_get_ps_timeout() from wfx_update_pm()
  staging: wfx: add support for set/get ps_timeout
  staging: wfx: allow to burn prevent rollback bit
  staging: wfx: allow to set PTA settings
  staging: wfx: allow to run nl80211 vendor commands with 'iw'

 drivers/staging/wfx/Makefile          |   3 +-
 drivers/staging/wfx/data_tx.c         |  11 +-
 drivers/staging/wfx/debug.c           |  26 +++++
 drivers/staging/wfx/hif_api_general.h |  67 +++++++++++-
 drivers/staging/wfx/hif_rx.c          |   7 ++
 drivers/staging/wfx/hif_tx.c          |  64 ++++++++++++
 drivers/staging/wfx/hif_tx.h          |   6 ++
 drivers/staging/wfx/main.c            |   6 ++
 drivers/staging/wfx/nl80211_vendor.c  | 143 ++++++++++++++++++++++++++
 drivers/staging/wfx/nl80211_vendor.h  |  93 +++++++++++++++++
 drivers/staging/wfx/queue.c           |  13 ---
 drivers/staging/wfx/queue.h           |   1 -
 drivers/staging/wfx/sta.c             |  56 ++++++----
 drivers/staging/wfx/sta.h             |   2 +
 drivers/staging/wfx/wfx.h             |   7 ++
 15 files changed, 459 insertions(+), 46 deletions(-)
 create mode 100644 drivers/staging/wfx/nl80211_vendor.c
 create mode 100644 drivers/staging/wfx/nl80211_vendor.h

-- 
2.26.2

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 01/10] staging: wfx: drop unused variable
  2020-05-26 17:18 [PATCH 00/10] staging: wfx: introduce nl80211 vendor extensions Jerome Pouiller
@ 2020-05-26 17:18 ` Jerome Pouiller
  2020-05-26 17:18 ` [PATCH 02/10] staging: wfx: do not declare variables inside loops Jerome Pouiller
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Jerome Pouiller @ 2020-05-26 17:18 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, David S . Miller, Kalle Valo

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

Since the commit 3f84adfe1d7ae ("staging: wfx: remove hack about tx_rate
policies"), the variable "count" is no more used in wfx_tx_policy_build().

Notice that there were two instances of the variable "count" in
wfx_tx_policy_build(). This patch also solves this cosmetic issue.

Reported-by: kbuild test robot <lkp@intel.com>
Fixes: 3f84adfe1d7ae ("staging: wfx: remove hack about tx_rate policies")
Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/data_tx.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index a82f00f8f17bd..a9eddd6db2b5d 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -43,15 +43,10 @@ static void wfx_tx_policy_build(struct wfx_vif *wvif, struct tx_policy *policy,
 				struct ieee80211_tx_rate *rates)
 {
 	int i;
-	size_t count;
 	struct wfx_dev *wdev = wvif->wdev;
 
 	WARN(rates[0].idx < 0, "invalid rate policy");
 	memset(policy, 0, sizeof(*policy));
-	for (i = 1; i < IEEE80211_TX_MAX_RATES; i++)
-		if (rates[i].idx < 0)
-			break;
-	count = i;
 	for (i = 0; i < IEEE80211_TX_MAX_RATES; ++i) {
 		int rateid;
 		u8 count;
-- 
2.26.2

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 02/10] staging: wfx: do not declare variables inside loops
  2020-05-26 17:18 [PATCH 00/10] staging: wfx: introduce nl80211 vendor extensions Jerome Pouiller
  2020-05-26 17:18 ` [PATCH 01/10] staging: wfx: drop unused variable Jerome Pouiller
@ 2020-05-26 17:18 ` Jerome Pouiller
  2020-05-26 17:18 ` [PATCH 03/10] staging: wfx: drop unused function wfx_pending_requeue() Jerome Pouiller
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Jerome Pouiller @ 2020-05-26 17:18 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, David S . Miller, Kalle Valo

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

The local variables should be declared at beginning of the functions.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/data_tx.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index a9eddd6db2b5d..f042ef36b408e 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -42,15 +42,13 @@ static int wfx_get_hw_rate(struct wfx_dev *wdev,
 static void wfx_tx_policy_build(struct wfx_vif *wvif, struct tx_policy *policy,
 				struct ieee80211_tx_rate *rates)
 {
-	int i;
 	struct wfx_dev *wdev = wvif->wdev;
+	int i, rateid;
+	u8 count;
 
 	WARN(rates[0].idx < 0, "invalid rate policy");
 	memset(policy, 0, sizeof(*policy));
 	for (i = 0; i < IEEE80211_TX_MAX_RATES; ++i) {
-		int rateid;
-		u8 count;
-
 		if (rates[i].idx < 0)
 			break;
 		WARN_ON(rates[i].count > 15);
-- 
2.26.2

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 03/10] staging: wfx: drop unused function wfx_pending_requeue()
  2020-05-26 17:18 [PATCH 00/10] staging: wfx: introduce nl80211 vendor extensions Jerome Pouiller
  2020-05-26 17:18 ` [PATCH 01/10] staging: wfx: drop unused variable Jerome Pouiller
  2020-05-26 17:18 ` [PATCH 02/10] staging: wfx: do not declare variables inside loops Jerome Pouiller
@ 2020-05-26 17:18 ` Jerome Pouiller
  2020-05-26 17:18 ` [PATCH 04/10] staging: wfx: add support for tx_power_loop Jerome Pouiller
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Jerome Pouiller @ 2020-05-26 17:18 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, David S . Miller, Kalle Valo

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

The function wfx_pending_requeue() is not used anymore since the
commit 7a44644c9379e ("staging: wfx: introduce
wfx_set_default_unicast_key()")

Fixes: 7a44644c9379e ("staging: wfx: introduce wfx_set_default_unicast_key()")
Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/queue.c | 13 -------------
 drivers/staging/wfx/queue.h |  1 -
 2 files changed, 14 deletions(-)

diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c
index 26b141cbd3035..3248ecefda564 100644
--- a/drivers/staging/wfx/queue.c
+++ b/drivers/staging/wfx/queue.c
@@ -143,19 +143,6 @@ void wfx_tx_queues_put(struct wfx_dev *wdev, struct sk_buff *skb)
 		skb_queue_tail(&queue->normal, skb);
 }
 
-int wfx_pending_requeue(struct wfx_dev *wdev, struct sk_buff *skb)
-{
-	struct wfx_queue *queue = &wdev->tx_queue[skb_get_queue_mapping(skb)];
-
-	WARN_ON(skb_get_queue_mapping(skb) > 3);
-	WARN_ON(!atomic_read(&queue->pending_frames));
-
-	atomic_dec(&queue->pending_frames);
-	skb_unlink(skb, &wdev->tx_pending);
-	wfx_tx_queues_put(wdev, skb);
-	return 0;
-}
-
 void wfx_pending_drop(struct wfx_dev *wdev, struct sk_buff_head *dropped)
 {
 	struct wfx_queue *queue;
diff --git a/drivers/staging/wfx/queue.h b/drivers/staging/wfx/queue.h
index 0cbe5f4b06f24..0c3b7244498e3 100644
--- a/drivers/staging/wfx/queue.h
+++ b/drivers/staging/wfx/queue.h
@@ -38,7 +38,6 @@ void wfx_tx_queue_drop(struct wfx_dev *wdev, struct wfx_queue *queue,
 
 struct sk_buff *wfx_pending_get(struct wfx_dev *wdev, u32 packet_id);
 void wfx_pending_drop(struct wfx_dev *wdev, struct sk_buff_head *dropped);
-int wfx_pending_requeue(struct wfx_dev *wdev, struct sk_buff *skb);
 unsigned int wfx_pending_get_pkt_us_delay(struct wfx_dev *wdev,
 					  struct sk_buff *skb);
 void wfx_pending_dump_old_frames(struct wfx_dev *wdev, unsigned int limit_ms);
-- 
2.26.2

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 04/10] staging: wfx: add support for tx_power_loop
  2020-05-26 17:18 [PATCH 00/10] staging: wfx: introduce nl80211 vendor extensions Jerome Pouiller
                   ` (2 preceding siblings ...)
  2020-05-26 17:18 ` [PATCH 03/10] staging: wfx: drop unused function wfx_pending_requeue() Jerome Pouiller
@ 2020-05-26 17:18 ` Jerome Pouiller
  2020-05-26 17:18 ` [PATCH 05/10] staging: wfx: retrieve the PS status from the vif Jerome Pouiller
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Jerome Pouiller @ 2020-05-26 17:18 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, David S . Miller, Kalle Valo

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

During the calibration of the RF amplifier, the device is able to provide
some data about the status of the amplifier.

Record these data and expose them in debugfs.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/debug.c           | 26 ++++++++++++++++++++++++++
 drivers/staging/wfx/hif_api_general.h | 18 +++++++++++++++---
 drivers/staging/wfx/hif_rx.c          |  7 +++++++
 drivers/staging/wfx/main.c            |  2 ++
 drivers/staging/wfx/wfx.h             |  2 ++
 5 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/wfx/debug.c b/drivers/staging/wfx/debug.c
index f52e7cf885cba..10d649985696a 100644
--- a/drivers/staging/wfx/debug.c
+++ b/drivers/staging/wfx/debug.c
@@ -178,6 +178,30 @@ static int wfx_rx_stats_show(struct seq_file *seq, void *v)
 }
 DEFINE_SHOW_ATTRIBUTE(wfx_rx_stats);
 
+static int wfx_tx_power_loop_show(struct seq_file *seq, void *v)
+{
+	struct wfx_dev *wdev = seq->private;
+	struct hif_tx_power_loop_info *st = &wdev->tx_power_loop_info;
+	int tmp;
+
+	mutex_lock(&wdev->tx_power_loop_info_lock);
+	tmp = le16_to_cpu(st->tx_gain_dig);
+	seq_printf(seq, "Tx gain digital: %d\n", tmp);
+	tmp = le16_to_cpu(st->tx_gain_pa);
+	seq_printf(seq, "Tx gain PA: %d\n", tmp);
+	tmp = (s16)le16_to_cpu(st->target_pout);
+	seq_printf(seq, "Target Pout: %d.%02d dBm\n", tmp / 4, (tmp % 4) * 25);
+	tmp = (s16)le16_to_cpu(st->p_estimation);
+	seq_printf(seq, "FEM Pout: %d.%02d dBm\n", tmp / 4, (tmp % 4) * 25);
+	tmp = le16_to_cpu(st->vpdet);
+	seq_printf(seq, "Vpdet: %d mV\n", tmp);
+	seq_printf(seq, "Measure index: %d\n", st->measurement_index);
+	mutex_unlock(&wdev->tx_power_loop_info_lock);
+
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(wfx_tx_power_loop);
+
 static ssize_t wfx_send_pds_write(struct file *file,
 				  const char __user *user_buf,
 				  size_t count, loff_t *ppos)
@@ -317,6 +341,8 @@ int wfx_debug_init(struct wfx_dev *wdev)
 	d = debugfs_create_dir("wfx", wdev->hw->wiphy->debugfsdir);
 	debugfs_create_file("counters", 0444, d, wdev, &wfx_counters_fops);
 	debugfs_create_file("rx_stats", 0444, d, wdev, &wfx_rx_stats_fops);
+	debugfs_create_file("tx_power_loop", 0444, d, wdev,
+			    &wfx_tx_power_loop_fops);
 	debugfs_create_file("send_pds", 0200, d, wdev, &wfx_send_pds_fops);
 	debugfs_create_file("burn_slk_key", 0200, d, wdev,
 			    &wfx_burn_slk_key_fops);
diff --git a/drivers/staging/wfx/hif_api_general.h b/drivers/staging/wfx/hif_api_general.h
index f5abd81747069..dba18a7ae9194 100644
--- a/drivers/staging/wfx/hif_api_general.h
+++ b/drivers/staging/wfx/hif_api_general.h
@@ -201,9 +201,10 @@ struct hif_cnf_control_gpio {
 } __packed;
 
 enum hif_generic_indication_type {
-	HIF_GENERIC_INDICATION_TYPE_RAW      = 0x0,
-	HIF_GENERIC_INDICATION_TYPE_STRING   = 0x1,
-	HIF_GENERIC_INDICATION_TYPE_RX_STATS = 0x2
+	HIF_GENERIC_INDICATION_TYPE_RAW                = 0x0,
+	HIF_GENERIC_INDICATION_TYPE_STRING             = 0x1,
+	HIF_GENERIC_INDICATION_TYPE_RX_STATS           = 0x2,
+	HIF_GENERIC_INDICATION_TYPE_TX_POWER_LOOP_INFO = 0x3,
 };
 
 struct hif_rx_stats {
@@ -222,8 +223,19 @@ struct hif_rx_stats {
 	s8     current_temp;
 } __packed;
 
+struct hif_tx_power_loop_info {
+	__le16 tx_gain_dig;
+	__le16 tx_gain_pa;
+	__le16 target_pout; // signed value
+	__le16 p_estimation; // signed value
+	__le16 vpdet;
+	u8     measurement_index;
+	u8     reserved;
+} __packed;
+
 union hif_indication_data {
 	struct hif_rx_stats rx_stats;
+	struct hif_tx_power_loop_info tx_power_loop_info;
 	u8     raw_data[1];
 };
 
diff --git a/drivers/staging/wfx/hif_rx.c b/drivers/staging/wfx/hif_rx.c
index 88466063cc427..bb156033d1e16 100644
--- a/drivers/staging/wfx/hif_rx.c
+++ b/drivers/staging/wfx/hif_rx.c
@@ -278,6 +278,13 @@ static int hif_generic_indication(struct wfx_dev *wdev,
 		       sizeof(wdev->rx_stats));
 		mutex_unlock(&wdev->rx_stats_lock);
 		return 0;
+	case HIF_GENERIC_INDICATION_TYPE_TX_POWER_LOOP_INFO:
+		mutex_lock(&wdev->tx_power_loop_info_lock);
+		memcpy(&wdev->tx_power_loop_info,
+		       &body->indication_data.tx_power_loop_info,
+		       sizeof(wdev->tx_power_loop_info));
+		mutex_unlock(&wdev->tx_power_loop_info_lock);
+		return 0;
 	default:
 		dev_err(wdev->dev, "generic_indication: unknown indication type: %#.8x\n",
 			type);
diff --git a/drivers/staging/wfx/main.c b/drivers/staging/wfx/main.c
index ae23a56f50e05..6bd96f4763884 100644
--- a/drivers/staging/wfx/main.c
+++ b/drivers/staging/wfx/main.c
@@ -274,6 +274,7 @@ static void wfx_free_common(void *data)
 {
 	struct wfx_dev *wdev = data;
 
+	mutex_destroy(&wdev->tx_power_loop_info_lock);
 	mutex_destroy(&wdev->rx_stats_lock);
 	mutex_destroy(&wdev->conf_mutex);
 	ieee80211_free_hw(wdev->hw);
@@ -344,6 +345,7 @@ struct wfx_dev *wfx_init_common(struct device *dev,
 
 	mutex_init(&wdev->conf_mutex);
 	mutex_init(&wdev->rx_stats_lock);
+	mutex_init(&wdev->tx_power_loop_info_lock);
 	init_completion(&wdev->firmware_ready);
 	INIT_DELAYED_WORK(&wdev->cooling_timeout_work,
 			  wfx_cooling_timeout_work);
diff --git a/drivers/staging/wfx/wfx.h b/drivers/staging/wfx/wfx.h
index cc9f7d16ee8b7..73e216733ce4f 100644
--- a/drivers/staging/wfx/wfx.h
+++ b/drivers/staging/wfx/wfx.h
@@ -58,6 +58,8 @@ struct wfx_dev {
 
 	struct hif_rx_stats	rx_stats;
 	struct mutex		rx_stats_lock;
+	struct hif_tx_power_loop_info tx_power_loop_info;
+	struct mutex		tx_power_loop_info_lock;
 };
 
 struct wfx_vif {
-- 
2.26.2

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 05/10] staging: wfx: retrieve the PS status from the vif
  2020-05-26 17:18 [PATCH 00/10] staging: wfx: introduce nl80211 vendor extensions Jerome Pouiller
                   ` (3 preceding siblings ...)
  2020-05-26 17:18 ` [PATCH 04/10] staging: wfx: add support for tx_power_loop Jerome Pouiller
@ 2020-05-26 17:18 ` Jerome Pouiller
  2020-05-26 17:18 ` [PATCH 06/10] staging: wfx: split wfx_get_ps_timeout() from wfx_update_pm() Jerome Pouiller
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Jerome Pouiller @ 2020-05-26 17:18 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, David S . Miller, Kalle Valo

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

The Power Save status is stored for each virtual interface and for the
whole device. The WF200 is able to handle power saving per interface, so
use the value stored in vif.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/sta.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index 6015cd2c4d8ae..d0ab0b8dc404e 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -203,7 +203,7 @@ void wfx_configure_filter(struct ieee80211_hw *hw, unsigned int changed_flags,
 static int wfx_update_pm(struct wfx_vif *wvif)
 {
 	struct ieee80211_conf *conf = &wvif->wdev->hw->conf;
-	bool ps = conf->flags & IEEE80211_CONF_PS;
+	bool ps = wvif->vif->bss_conf.ps;
 	int ps_timeout = conf->dynamic_ps_timeout;
 	struct ieee80211_channel *chan0 = NULL, *chan1 = NULL;
 
-- 
2.26.2

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 06/10] staging: wfx: split wfx_get_ps_timeout() from wfx_update_pm()
  2020-05-26 17:18 [PATCH 00/10] staging: wfx: introduce nl80211 vendor extensions Jerome Pouiller
                   ` (4 preceding siblings ...)
  2020-05-26 17:18 ` [PATCH 05/10] staging: wfx: retrieve the PS status from the vif Jerome Pouiller
@ 2020-05-26 17:18 ` Jerome Pouiller
  2020-05-26 17:18 ` [PATCH 07/10] staging: wfx: add support for set/get ps_timeout Jerome Pouiller
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Jerome Pouiller @ 2020-05-26 17:18 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, David S . Miller, Kalle Valo

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

In the next commit, we will have to compute the PS timeout without
changing the power save status of the device. This patch introduces
wfx_get_ps_timeout() for that job and make wfx_update_pm() relies on it.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/sta.c | 49 +++++++++++++++++++++++++--------------
 1 file changed, 31 insertions(+), 18 deletions(-)

diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index d0ab0b8dc404e..12e8a5b638f11 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -200,36 +200,49 @@ void wfx_configure_filter(struct ieee80211_hw *hw, unsigned int changed_flags,
 	mutex_unlock(&wdev->conf_mutex);
 }
 
-static int wfx_update_pm(struct wfx_vif *wvif)
+int wfx_get_ps_timeout(struct wfx_vif *wvif, bool *enable_ps)
 {
-	struct ieee80211_conf *conf = &wvif->wdev->hw->conf;
-	bool ps = wvif->vif->bss_conf.ps;
-	int ps_timeout = conf->dynamic_ps_timeout;
 	struct ieee80211_channel *chan0 = NULL, *chan1 = NULL;
+	struct ieee80211_conf *conf = &wvif->wdev->hw->conf;
 
-	WARN_ON(conf->dynamic_ps_timeout < 0);
-	if (!wvif->vif->bss_conf.assoc)
-		return 0;
-	if (!ps)
-		ps_timeout = 0;
-	if (wvif->uapsd_mask)
-		ps_timeout = 0;
-
-	// Kernel disable powersave when an AP is in use. In contrary, it is
-	// absolutely necessary to enable legacy powersave for WF200 if channels
-	// are differents.
+	WARN(!wvif->vif->bss_conf.assoc && enable_ps,
+	     "enable_ps is reliable only if associated");
 	if (wdev_to_wvif(wvif->wdev, 0))
 		chan0 = wdev_to_wvif(wvif->wdev, 0)->vif->bss_conf.chandef.chan;
 	if (wdev_to_wvif(wvif->wdev, 1))
 		chan1 = wdev_to_wvif(wvif->wdev, 1)->vif->bss_conf.chandef.chan;
 	if (chan0 && chan1 && chan0->hw_value != chan1->hw_value &&
 	    wvif->vif->type != NL80211_IFTYPE_AP) {
-		ps = true;
+		// It is necessary to enable powersave if channels
+		// are differents.
+		if (enable_ps)
+			*enable_ps = true;
 		if (wvif->bss_not_support_ps_poll)
-			ps_timeout = 30;
+			return 30;
 		else
-			ps_timeout = 0;
+			return 0;
 	}
+	if (enable_ps)
+		*enable_ps = wvif->vif->bss_conf.ps;
+	if (wvif->vif->bss_conf.assoc && wvif->vif->bss_conf.ps)
+		return conf->dynamic_ps_timeout;
+	else
+		return -1;
+}
+
+int wfx_update_pm(struct wfx_vif *wvif)
+{
+	int ps_timeout;
+	bool ps;
+
+	if (!wvif->vif->bss_conf.assoc)
+		return 0;
+	ps_timeout = wfx_get_ps_timeout(wvif, &ps);
+	if (!ps)
+		ps_timeout = 0;
+	WARN_ON(ps_timeout < 0);
+	if (wvif->uapsd_mask)
+		ps_timeout = 0;
 
 	if (!wait_for_completion_timeout(&wvif->set_pm_mode_complete,
 					 TU_TO_JIFFIES(512)))
-- 
2.26.2

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 07/10] staging: wfx: add support for set/get ps_timeout
  2020-05-26 17:18 [PATCH 00/10] staging: wfx: introduce nl80211 vendor extensions Jerome Pouiller
                   ` (5 preceding siblings ...)
  2020-05-26 17:18 ` [PATCH 06/10] staging: wfx: split wfx_get_ps_timeout() from wfx_update_pm() Jerome Pouiller
@ 2020-05-26 17:18 ` Jerome Pouiller
  2020-05-26 17:18 ` [PATCH 08/10] staging: wfx: allow to burn prevent rollback bit Jerome Pouiller
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Jerome Pouiller @ 2020-05-26 17:18 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, David S . Miller, Kalle Valo

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

In some advanced usage or debug scenarios, it could interesting to
change the value of ps_timeout or eventually to force use of PS-Poll
frames.

The wext API (used by iwconfig) provide a way to change ps_timeout.
However, this API is obsolete and it seems a little weird to use (it
seems it doesn't apply the changes, so the user have to disable then
re-enable de power save)

On side of nl80211, there is no way to change the ps_timeout.

This patch provides a vendor extension to nl80211 to change the value
of ps_timeout.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/Makefile         |  3 +-
 drivers/staging/wfx/main.c           |  4 +++
 drivers/staging/wfx/nl80211_vendor.c | 49 ++++++++++++++++++++++++++++
 drivers/staging/wfx/nl80211_vendor.h | 44 +++++++++++++++++++++++++
 drivers/staging/wfx/sta.c            |  9 +++--
 drivers/staging/wfx/sta.h            |  2 ++
 drivers/staging/wfx/wfx.h            |  1 +
 7 files changed, 109 insertions(+), 3 deletions(-)
 create mode 100644 drivers/staging/wfx/nl80211_vendor.c
 create mode 100644 drivers/staging/wfx/nl80211_vendor.h

diff --git a/drivers/staging/wfx/Makefile b/drivers/staging/wfx/Makefile
index 0e0cc982ceab2..2d34a02853226 100644
--- a/drivers/staging/wfx/Makefile
+++ b/drivers/staging/wfx/Makefile
@@ -18,7 +18,8 @@ wfx-y := \
 	key.o \
 	main.o \
 	sta.o \
-	debug.o
+	debug.o \
+	nl80211_vendor.o
 wfx-$(CONFIG_SPI) += bus_spi.o
 wfx-$(subst m,y,$(CONFIG_MMC)) += bus_sdio.o
 
diff --git a/drivers/staging/wfx/main.c b/drivers/staging/wfx/main.c
index 6bd96f4763884..11f6bc6fa3394 100644
--- a/drivers/staging/wfx/main.c
+++ b/drivers/staging/wfx/main.c
@@ -34,6 +34,7 @@
 #include "secure_link.h"
 #include "hif_tx_mib.h"
 #include "hif_api_cmd.h"
+#include "nl80211_vendor.h"
 
 #define WFX_PDS_MAX_SIZE 1500
 
@@ -312,6 +313,9 @@ struct wfx_dev *wfx_init_common(struct device *dev,
 				sizeof(struct hif_msg)
 				+ sizeof(struct hif_req_tx)
 				+ 4 /* alignment */ + 8 /* TKIP IV */;
+
+	hw->wiphy->n_vendor_commands = ARRAY_SIZE(wfx_nl80211_vendor_commands);
+	hw->wiphy->vendor_commands = wfx_nl80211_vendor_commands;
 	hw->wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION) |
 				     BIT(NL80211_IFTYPE_ADHOC) |
 				     BIT(NL80211_IFTYPE_AP);
diff --git a/drivers/staging/wfx/nl80211_vendor.c b/drivers/staging/wfx/nl80211_vendor.c
new file mode 100644
index 0000000000000..ec2fd2d73885f
--- /dev/null
+++ b/drivers/staging/wfx/nl80211_vendor.c
@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Extra commands for nl80211 interface.
+ *
+ * Copyright (c) 2020, Silicon Laboratories, Inc.
+ */
+#include "nl80211_vendor.h"
+#include "wfx.h"
+#include "sta.h"
+
+int wfx_nl_ps_timeout(struct wiphy *wiphy, struct wireless_dev *widev,
+		      const void *data, int data_len)
+{
+	int reply_size = nla_total_size(sizeof(u32));
+	struct nlattr *tb[WFX_NL80211_ATTR_MAX];
+	struct ieee80211_vif *vif;
+	struct wfx_vif *wvif;
+	struct sk_buff *msg;
+	int rc, ps_timeout;
+
+	rc = nla_parse(tb, WFX_NL80211_ATTR_MAX - 1, data, data_len,
+		       wfx_nl_policy, NULL);
+	if (rc)
+		return rc;
+	vif = wdev_to_ieee80211_vif(widev);
+	if (!vif)
+		return -ENODEV;
+	wvif = (struct wfx_vif *)vif->drv_priv;
+
+	if (tb[WFX_NL80211_ATTR_PS_TIMEOUT]) {
+		wvif->force_ps_timeout =
+			nla_get_s32(tb[WFX_NL80211_ATTR_PS_TIMEOUT]);
+		wfx_update_pm(wvif);
+	}
+
+	msg = cfg80211_vendor_cmd_alloc_reply_skb(wiphy, reply_size);
+	if (!msg)
+		return -ENOMEM;
+	ps_timeout = wfx_get_ps_timeout(wvif, NULL);
+	rc = nla_put_s32(msg, WFX_NL80211_ATTR_PS_TIMEOUT, ps_timeout);
+	if (rc)
+		goto error;
+	return cfg80211_vendor_cmd_reply(msg);
+
+error:
+	kfree_skb(msg);
+	return rc;
+}
+
diff --git a/drivers/staging/wfx/nl80211_vendor.h b/drivers/staging/wfx/nl80211_vendor.h
new file mode 100644
index 0000000000000..c069330e240a9
--- /dev/null
+++ b/drivers/staging/wfx/nl80211_vendor.h
@@ -0,0 +1,44 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Extra commands for nl80211 interface.
+ *
+ * Copyright (c) 2020, Silicon Laboratories, Inc.
+ */
+#ifndef WFX_NL80211_VENDOR_H
+#define WFX_NL80211_VENDOR_H
+
+#include <net/netlink.h>
+#include <net/cfg80211.h>
+
+#include "hif_api_general.h"
+
+#define WFX_NL80211_ID 0x90fd9f
+
+int wfx_nl_ps_timeout(struct wiphy *wiphy, struct wireless_dev *widev,
+		      const void *data, int data_len);
+
+enum {
+	WFX_NL80211_SUBCMD_PS_TIMEOUT                   = 0x10,
+};
+
+enum {
+	WFX_NL80211_ATTR_PS_TIMEOUT     = 1,
+	WFX_NL80211_ATTR_MAX
+};
+
+static const struct nla_policy wfx_nl_policy[WFX_NL80211_ATTR_MAX] = {
+	[WFX_NL80211_ATTR_PS_TIMEOUT]     = NLA_POLICY_RANGE(NLA_S32, -1, 127),
+};
+
+static const struct wiphy_vendor_command wfx_nl80211_vendor_commands[] = {
+	{
+		.info.vendor_id = WFX_NL80211_ID,
+		.info.subcmd = WFX_NL80211_SUBCMD_PS_TIMEOUT,
+		.flags = WIPHY_VENDOR_CMD_NEED_WDEV,
+		.policy = wfx_nl_policy,
+		.doit = wfx_nl_ps_timeout,
+		.maxattr = WFX_NL80211_ATTR_MAX - 1,
+	},
+};
+
+#endif /* WFX_NL80211_VENDOR_H */
diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index 12e8a5b638f11..7f0bb8eb78660 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -217,14 +217,18 @@ int wfx_get_ps_timeout(struct wfx_vif *wvif, bool *enable_ps)
 		// are differents.
 		if (enable_ps)
 			*enable_ps = true;
-		if (wvif->bss_not_support_ps_poll)
+		if (wvif->force_ps_timeout > -1)
+			return wvif->force_ps_timeout;
+		else if (wvif->bss_not_support_ps_poll)
 			return 30;
 		else
 			return 0;
 	}
 	if (enable_ps)
 		*enable_ps = wvif->vif->bss_conf.ps;
-	if (wvif->vif->bss_conf.assoc && wvif->vif->bss_conf.ps)
+	if (wvif->force_ps_timeout > -1)
+		return wvif->force_ps_timeout;
+	else if (wvif->vif->bss_conf.assoc && wvif->vif->bss_conf.ps)
 		return conf->dynamic_ps_timeout;
 	else
 		return -1;
@@ -788,6 +792,7 @@ int wfx_add_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
 	wvif->vif = vif;
 	wvif->wdev = wdev;
 
+	wvif->force_ps_timeout = -1;
 	wvif->link_id_map = 1; // link-id 0 is reserved for multicast
 	INIT_WORK(&wvif->update_tim_work, wfx_update_tim_work);
 	INIT_DELAYED_WORK(&wvif->beacon_loss_work, wfx_beacon_loss_work);
diff --git a/drivers/staging/wfx/sta.h b/drivers/staging/wfx/sta.h
index 8a20ad9ae017e..8220d31184c8c 100644
--- a/drivers/staging/wfx/sta.h
+++ b/drivers/staging/wfx/sta.h
@@ -69,9 +69,11 @@ void wfx_cooling_timeout_work(struct work_struct *work);
 void wfx_suspend_hot_dev(struct wfx_dev *wdev, enum sta_notify_cmd cmd);
 void wfx_suspend_resume_mc(struct wfx_vif *wvif, enum sta_notify_cmd cmd);
 void wfx_event_report_rssi(struct wfx_vif *wvif, u8 raw_rcpi_rssi);
+int wfx_update_pm(struct wfx_vif *wvif);
 
 // Other Helpers
 void wfx_reset(struct wfx_vif *wvif);
+int wfx_get_ps_timeout(struct wfx_vif *wvif, bool *force_ps);
 u32 wfx_rate_mask_to_hw(struct wfx_dev *wdev, u32 rates);
 
 #endif /* WFX_STA_H */
diff --git a/drivers/staging/wfx/wfx.h b/drivers/staging/wfx/wfx.h
index 73e216733ce4f..ef68aa4086e01 100644
--- a/drivers/staging/wfx/wfx.h
+++ b/drivers/staging/wfx/wfx.h
@@ -92,6 +92,7 @@ struct wfx_vif {
 	bool			scan_abort;
 	struct ieee80211_scan_request *scan_req;
 
+	int			force_ps_timeout;
 	bool			bss_not_support_ps_poll;
 	struct work_struct	update_pm_work;
 	struct completion	set_pm_mode_complete;
-- 
2.26.2

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 08/10] staging: wfx: allow to burn prevent rollback bit
  2020-05-26 17:18 [PATCH 00/10] staging: wfx: introduce nl80211 vendor extensions Jerome Pouiller
                   ` (6 preceding siblings ...)
  2020-05-26 17:18 ` [PATCH 07/10] staging: wfx: add support for set/get ps_timeout Jerome Pouiller
@ 2020-05-26 17:18 ` Jerome Pouiller
  2020-05-26 17:18 ` [PATCH 09/10] staging: wfx: allow to set PTA settings Jerome Pouiller
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Jerome Pouiller @ 2020-05-26 17:18 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, David S . Miller, Kalle Valo

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

In case a security flaw is found in a version of firmware, the device
offers a way to disallow the loading an older firmware.

This patch provides a vendor extension to nl80211 to enable this
feature.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/hif_api_general.h |  8 ++++++++
 drivers/staging/wfx/hif_tx.c          | 18 ++++++++++++++++++
 drivers/staging/wfx/hif_tx.h          |  1 +
 drivers/staging/wfx/nl80211_vendor.c  | 23 +++++++++++++++++++++++
 drivers/staging/wfx/nl80211_vendor.h  | 11 +++++++++++
 5 files changed, 61 insertions(+)

diff --git a/drivers/staging/wfx/hif_api_general.h b/drivers/staging/wfx/hif_api_general.h
index dba18a7ae9194..c8af3534700ca 100644
--- a/drivers/staging/wfx/hif_api_general.h
+++ b/drivers/staging/wfx/hif_api_general.h
@@ -361,4 +361,12 @@ struct hif_cnf_sl_configure {
 	__le32 status;
 } __packed;
 
+struct hif_req_prevent_rollback {
+	__le32 magic_word; // Return an error if not equal to 0x5C8912F3
+} __packed;
+
+struct hif_cnf_prevent_rollback {
+	__le32 status;
+} __packed;
+
 #endif
diff --git a/drivers/staging/wfx/hif_tx.c b/drivers/staging/wfx/hif_tx.c
index 6db41587cc7a5..899e1eb71a44b 100644
--- a/drivers/staging/wfx/hif_tx.c
+++ b/drivers/staging/wfx/hif_tx.c
@@ -535,6 +535,24 @@ int hif_update_ie_beacon(struct wfx_vif *wvif, const u8 *ies, size_t ies_len)
 	return ret;
 }
 
+int hif_burn_prevent_rollback(struct wfx_dev *wdev, u32 magic_word)
+{
+	int ret;
+	struct hif_msg *hif;
+	struct hif_req_prevent_rollback *body = wfx_alloc_hif(sizeof(*body),
+							      &hif);
+
+	if (!hif)
+		return -ENOMEM;
+	body->magic_word = cpu_to_le32(magic_word);
+	wfx_fill_header(hif, -1, HIF_REQ_ID_PREVENT_ROLLBACK, sizeof(*body));
+	ret = wfx_cmd_send(wdev, hif, NULL, 0, false);
+	if (ret == le32_to_cpu(HIF_STATUS_ROLLBACK_SUCCESS))
+		ret = 0;
+	kfree(hif);
+	return ret;
+}
+
 int hif_sl_send_pub_keys(struct wfx_dev *wdev,
 			 const u8 *pubkey, const u8 *pubkey_hmac)
 {
diff --git a/drivers/staging/wfx/hif_tx.h b/drivers/staging/wfx/hif_tx.h
index e1da28aef706e..d29c72d94789a 100644
--- a/drivers/staging/wfx/hif_tx.h
+++ b/drivers/staging/wfx/hif_tx.h
@@ -57,6 +57,7 @@ int hif_start(struct wfx_vif *wvif, const struct ieee80211_bss_conf *conf,
 int hif_beacon_transmit(struct wfx_vif *wvif, bool enable);
 int hif_map_link(struct wfx_vif *wvif, u8 *mac_addr, int flags, int sta_id);
 int hif_update_ie_beacon(struct wfx_vif *wvif, const u8 *ies, size_t ies_len);
+int hif_burn_prevent_rollback(struct wfx_dev *wdev, u32 magic_word);
 int hif_sl_set_mac_key(struct wfx_dev *wdev,
 		       const u8 *slk_key, int destination);
 int hif_sl_config(struct wfx_dev *wdev, const unsigned long *bitmap);
diff --git a/drivers/staging/wfx/nl80211_vendor.c b/drivers/staging/wfx/nl80211_vendor.c
index ec2fd2d73885f..1a9d411718a73 100644
--- a/drivers/staging/wfx/nl80211_vendor.c
+++ b/drivers/staging/wfx/nl80211_vendor.c
@@ -7,6 +7,7 @@
 #include "nl80211_vendor.h"
 #include "wfx.h"
 #include "sta.h"
+#include "hif_tx.h"
 
 int wfx_nl_ps_timeout(struct wiphy *wiphy, struct wireless_dev *widev,
 		      const void *data, int data_len)
@@ -47,3 +48,25 @@ int wfx_nl_ps_timeout(struct wiphy *wiphy, struct wireless_dev *widev,
 	return rc;
 }
 
+int wfx_nl_burn_antirollback(struct wiphy *wiphy, struct wireless_dev *widev,
+			     const void *data, int data_len)
+{
+	struct ieee80211_hw *hw = wiphy_to_ieee80211_hw(wiphy);
+	struct wfx_dev *wdev = (struct wfx_dev *)hw->priv;
+	struct nlattr *tb[WFX_NL80211_ATTR_MAX];
+	u32 magic;
+	int rc;
+
+	rc = nla_parse(tb, WFX_NL80211_ATTR_MAX - 1, data, data_len,
+		       wfx_nl_policy, NULL);
+	if (rc)
+		return rc;
+	if (!tb[WFX_NL80211_ATTR_ROLLBACK_MAGIC])
+		return -EINVAL;
+	magic = nla_get_u32(tb[WFX_NL80211_ATTR_ROLLBACK_MAGIC]);
+	rc = hif_burn_prevent_rollback(wdev, magic);
+	if (rc)
+		return -EINVAL;
+	return 0;
+}
+
diff --git a/drivers/staging/wfx/nl80211_vendor.h b/drivers/staging/wfx/nl80211_vendor.h
index c069330e240a9..49efe8716a654 100644
--- a/drivers/staging/wfx/nl80211_vendor.h
+++ b/drivers/staging/wfx/nl80211_vendor.h
@@ -16,18 +16,23 @@
 
 int wfx_nl_ps_timeout(struct wiphy *wiphy, struct wireless_dev *widev,
 		      const void *data, int data_len);
+int wfx_nl_burn_antirollback(struct wiphy *wiphy, struct wireless_dev *widev,
+			     const void *data, int data_len);
 
 enum {
 	WFX_NL80211_SUBCMD_PS_TIMEOUT                   = 0x10,
+	WFX_NL80211_SUBCMD_BURN_PREVENT_ROLLBACK        = 0x20,
 };
 
 enum {
 	WFX_NL80211_ATTR_PS_TIMEOUT     = 1,
+	WFX_NL80211_ATTR_ROLLBACK_MAGIC = 2,
 	WFX_NL80211_ATTR_MAX
 };
 
 static const struct nla_policy wfx_nl_policy[WFX_NL80211_ATTR_MAX] = {
 	[WFX_NL80211_ATTR_PS_TIMEOUT]     = NLA_POLICY_RANGE(NLA_S32, -1, 127),
+	[WFX_NL80211_ATTR_ROLLBACK_MAGIC] = { .type = NLA_U32 },
 };
 
 static const struct wiphy_vendor_command wfx_nl80211_vendor_commands[] = {
@@ -38,6 +43,12 @@ static const struct wiphy_vendor_command wfx_nl80211_vendor_commands[] = {
 		.policy = wfx_nl_policy,
 		.doit = wfx_nl_ps_timeout,
 		.maxattr = WFX_NL80211_ATTR_MAX - 1,
+	}, {
+		.info.vendor_id = WFX_NL80211_ID,
+		.info.subcmd = WFX_NL80211_SUBCMD_BURN_PREVENT_ROLLBACK,
+		.policy = wfx_nl_policy,
+		.doit = wfx_nl_burn_antirollback,
+		.maxattr = WFX_NL80211_ATTR_MAX - 1,
 	},
 };
 
-- 
2.26.2

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 09/10] staging: wfx: allow to set PTA settings
  2020-05-26 17:18 [PATCH 00/10] staging: wfx: introduce nl80211 vendor extensions Jerome Pouiller
                   ` (7 preceding siblings ...)
  2020-05-26 17:18 ` [PATCH 08/10] staging: wfx: allow to burn prevent rollback bit Jerome Pouiller
@ 2020-05-26 17:18 ` Jerome Pouiller
  2020-05-26 17:18 ` [PATCH 10/10] staging: wfx: allow to run nl80211 vendor commands with 'iw' Jerome Pouiller
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Jerome Pouiller @ 2020-05-26 17:18 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, David S . Miller, Kalle Valo

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

The device allows to do Packet Traffic Arbitration (PTA or also Coex)
with other RF chips.

Currently, there is no API to manage the PTA parameters. This patch
provides a vendor extension to nl80211 to change the PTA parameters.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/hif_api_general.h | 41 ++++++++++++++++
 drivers/staging/wfx/hif_tx.c          | 46 +++++++++++++++++
 drivers/staging/wfx/hif_tx.h          |  5 ++
 drivers/staging/wfx/nl80211_vendor.c  | 71 +++++++++++++++++++++++++++
 drivers/staging/wfx/nl80211_vendor.h  | 16 ++++++
 drivers/staging/wfx/wfx.h             |  4 ++
 6 files changed, 183 insertions(+)

diff --git a/drivers/staging/wfx/hif_api_general.h b/drivers/staging/wfx/hif_api_general.h
index c8af3534700ca..eb90164ab87c7 100644
--- a/drivers/staging/wfx/hif_api_general.h
+++ b/drivers/staging/wfx/hif_api_general.h
@@ -369,4 +369,45 @@ struct hif_cnf_prevent_rollback {
 	__le32 status;
 } __packed;
 
+struct hif_req_pta_settings {
+	u8     PtaMode;
+	u8     RequestSignalActiveLevel;
+	u8     PrioritySignalActiveLevel;
+	u8     FreqSignalActiveLevel;
+	u8     GrantSignalActiveLevel;
+	u8     CoexType;
+	u8     DefaultGrantState;
+	u8     SimultaneousRxAccesses;
+	u8     PrioritySamplingTime;
+	u8     TxRxSamplingTime;
+	u8     FreqSamplingTime;
+	u8     GrantValidTime;
+	u8     FemControlTime;
+	u8     FirstSlotTime;
+	__le16 PeriodicTxRxSamplingTime;
+	__le16 CoexQuota;
+	__le16 WlanQuota;
+} __packed;
+
+struct hif_cnf_pta_settings {
+	__le32 status;
+} __packed;
+
+struct hif_req_pta_priority {
+	__le32 priority;
+} __packed;
+
+struct hif_cnf_pta_priority {
+	__le32 status;
+} __packed;
+
+struct hif_req_pta_enable {
+	u8     enable;
+	u8     reserved[3];
+} __packed;
+
+struct hif_cnf_pta_enable {
+	__le32 status;
+} __packed;
+
 #endif
diff --git a/drivers/staging/wfx/hif_tx.c b/drivers/staging/wfx/hif_tx.c
index 899e1eb71a44b..4cb8fe865e58f 100644
--- a/drivers/staging/wfx/hif_tx.c
+++ b/drivers/staging/wfx/hif_tx.c
@@ -535,6 +535,52 @@ int hif_update_ie_beacon(struct wfx_vif *wvif, const u8 *ies, size_t ies_len)
 	return ret;
 }
 
+int hif_pta_settings(struct wfx_dev *wdev,
+		     const struct hif_req_pta_settings *parms)
+{
+	int ret;
+	struct hif_msg *hif;
+	struct hif_req_pta_settings *body = wfx_alloc_hif(sizeof(*body), &hif);
+
+	if (!hif)
+		return -ENOMEM;
+	memcpy(body, parms, sizeof(*body));
+	wfx_fill_header(hif, -1, HIF_REQ_ID_PTA_SETTINGS, sizeof(*body));
+	ret = wfx_cmd_send(wdev, hif, NULL, 0, false);
+	kfree(hif);
+	return ret;
+}
+
+int hif_pta_priority(struct wfx_dev *wdev, u32 priority)
+{
+	int ret;
+	struct hif_msg *hif;
+	struct hif_req_pta_priority *body = wfx_alloc_hif(sizeof(*body), &hif);
+
+	if (!hif)
+		return -ENOMEM;
+	body->priority = cpu_to_le32(priority);
+	wfx_fill_header(hif, -1, HIF_REQ_ID_PTA_PRIORITY, sizeof(*body));
+	ret = wfx_cmd_send(wdev, hif, NULL, 0, false);
+	kfree(hif);
+	return ret;
+}
+
+int hif_pta_enable(struct wfx_dev *wdev, bool enable)
+{
+	int ret;
+	struct hif_msg *hif;
+	struct hif_req_pta_enable *body = wfx_alloc_hif(sizeof(*body), &hif);
+
+	if (!hif)
+		return -ENOMEM;
+	body->enable = enable;
+	wfx_fill_header(hif, -1, HIF_REQ_ID_PTA_STATE, sizeof(*body));
+	ret = wfx_cmd_send(wdev, hif, NULL, 0, false);
+	kfree(hif);
+	return ret;
+}
+
 int hif_burn_prevent_rollback(struct wfx_dev *wdev, u32 magic_word)
 {
 	int ret;
diff --git a/drivers/staging/wfx/hif_tx.h b/drivers/staging/wfx/hif_tx.h
index d29c72d94789a..f7202be4e7fc6 100644
--- a/drivers/staging/wfx/hif_tx.h
+++ b/drivers/staging/wfx/hif_tx.h
@@ -15,6 +15,7 @@ struct ieee80211_bss_conf;
 struct ieee80211_tx_queue_params;
 struct cfg80211_scan_request;
 struct hif_req_add_key;
+struct hif_req_pta_settings;
 struct wfx_dev;
 struct wfx_vif;
 
@@ -57,6 +58,10 @@ int hif_start(struct wfx_vif *wvif, const struct ieee80211_bss_conf *conf,
 int hif_beacon_transmit(struct wfx_vif *wvif, bool enable);
 int hif_map_link(struct wfx_vif *wvif, u8 *mac_addr, int flags, int sta_id);
 int hif_update_ie_beacon(struct wfx_vif *wvif, const u8 *ies, size_t ies_len);
+int hif_pta_settings(struct wfx_dev *wdev,
+		     const struct hif_req_pta_settings *parms);
+int hif_pta_priority(struct wfx_dev *wdev, u32 priority);
+int hif_pta_enable(struct wfx_dev *wdev, bool enable);
 int hif_burn_prevent_rollback(struct wfx_dev *wdev, u32 magic_word);
 int hif_sl_set_mac_key(struct wfx_dev *wdev,
 		       const u8 *slk_key, int destination);
diff --git a/drivers/staging/wfx/nl80211_vendor.c b/drivers/staging/wfx/nl80211_vendor.c
index 1a9d411718a73..d08072adaf9d6 100644
--- a/drivers/staging/wfx/nl80211_vendor.c
+++ b/drivers/staging/wfx/nl80211_vendor.c
@@ -70,3 +70,74 @@ int wfx_nl_burn_antirollback(struct wiphy *wiphy, struct wireless_dev *widev,
 	return 0;
 }
 
+int wfx_nl_pta_params(struct wiphy *wiphy, struct wireless_dev *widev,
+		      const void *data, int data_len)
+{
+	struct ieee80211_hw *hw = wiphy_to_ieee80211_hw(wiphy);
+	struct wfx_dev *wdev = (struct wfx_dev *)hw->priv;
+	int reply_size = nla_total_size(sizeof(wdev->pta_settings)) +
+			 nla_total_size(sizeof(u8)) +
+			 nla_total_size(sizeof(u32));
+	struct nlattr *tb[WFX_NL80211_ATTR_MAX];
+	bool do_enable = false;
+	struct sk_buff *msg;
+	struct nlattr *nla;
+	int rc;
+
+	rc = nla_parse(tb, WFX_NL80211_ATTR_MAX - 1, data, data_len,
+		       wfx_nl_policy, NULL);
+	if (rc)
+		return rc;
+	nla = tb[WFX_NL80211_ATTR_PTA_ENABLE];
+	if (nla) {
+		do_enable = true;
+		wdev->pta_enable = nla_get_u8(tb[WFX_NL80211_ATTR_PTA_ENABLE]);
+	}
+	if (do_enable && !wdev->pta_enable)
+		rc = hif_pta_enable(wdev, wdev->pta_enable);
+	if (rc)
+		return rc;
+	nla = tb[WFX_NL80211_ATTR_PTA_SETTINGS];
+	if (nla) {
+		// User has to care about endianness of data it send.
+		memcpy(&wdev->pta_settings, nla_data(nla),
+		       sizeof(wdev->pta_settings));
+		rc = hif_pta_settings(wdev, &wdev->pta_settings);
+	}
+	if (rc)
+		return rc;
+	nla = tb[WFX_NL80211_ATTR_PTA_PRIORITY];
+	if (nla) {
+		wdev->pta_priority =
+			nla_get_u32(tb[WFX_NL80211_ATTR_PTA_PRIORITY]);
+		rc = hif_pta_priority(wdev, wdev->pta_priority);
+	}
+	if (rc)
+		return rc;
+	if (do_enable && wdev->pta_enable)
+		rc = hif_pta_enable(wdev, wdev->pta_enable);
+	if (rc)
+		return rc;
+
+	msg = cfg80211_vendor_cmd_alloc_reply_skb(wiphy, reply_size);
+	if (!msg)
+		return -ENOMEM;
+	rc = nla_put(msg, WFX_NL80211_ATTR_PTA_SETTINGS,
+		     sizeof(wdev->pta_settings), &wdev->pta_settings);
+	if (rc)
+		goto error;
+	rc = nla_put_u32(msg, WFX_NL80211_ATTR_PTA_PRIORITY,
+			 wdev->pta_priority);
+	if (rc)
+		goto error;
+	rc = nla_put_u8(msg, WFX_NL80211_ATTR_PTA_ENABLE,
+			wdev->pta_enable ? 1 : 0);
+	if (rc)
+		goto error;
+	return cfg80211_vendor_cmd_reply(msg);
+
+error:
+	kfree_skb(msg);
+	return rc;
+}
+
diff --git a/drivers/staging/wfx/nl80211_vendor.h b/drivers/staging/wfx/nl80211_vendor.h
index 49efe8716a654..0ff3bf73f0ad3 100644
--- a/drivers/staging/wfx/nl80211_vendor.h
+++ b/drivers/staging/wfx/nl80211_vendor.h
@@ -18,21 +18,31 @@ int wfx_nl_ps_timeout(struct wiphy *wiphy, struct wireless_dev *widev,
 		      const void *data, int data_len);
 int wfx_nl_burn_antirollback(struct wiphy *wiphy, struct wireless_dev *widev,
 			     const void *data, int data_len);
+int wfx_nl_pta_params(struct wiphy *wiphy, struct wireless_dev *widev,
+		      const void *data, int data_len);
 
 enum {
 	WFX_NL80211_SUBCMD_PS_TIMEOUT                   = 0x10,
 	WFX_NL80211_SUBCMD_BURN_PREVENT_ROLLBACK        = 0x20,
+	WFX_NL80211_SUBCMD_PTA_PARMS                    = 0x30,
 };
 
 enum {
 	WFX_NL80211_ATTR_PS_TIMEOUT     = 1,
 	WFX_NL80211_ATTR_ROLLBACK_MAGIC = 2,
+	WFX_NL80211_ATTR_PTA_ENABLE     = 3,
+	WFX_NL80211_ATTR_PTA_PRIORITY   = 4,
+	WFX_NL80211_ATTR_PTA_SETTINGS   = 5,
 	WFX_NL80211_ATTR_MAX
 };
 
 static const struct nla_policy wfx_nl_policy[WFX_NL80211_ATTR_MAX] = {
 	[WFX_NL80211_ATTR_PS_TIMEOUT]     = NLA_POLICY_RANGE(NLA_S32, -1, 127),
 	[WFX_NL80211_ATTR_ROLLBACK_MAGIC] = { .type = NLA_U32 },
+	[WFX_NL80211_ATTR_PTA_ENABLE]     = NLA_POLICY_MAX(NLA_U8, 1),
+	[WFX_NL80211_ATTR_PTA_PRIORITY]   = { .type = NLA_U32 },
+	[WFX_NL80211_ATTR_PTA_SETTINGS]   =
+		NLA_POLICY_EXACT_LEN(sizeof(struct hif_req_pta_settings)),
 };
 
 static const struct wiphy_vendor_command wfx_nl80211_vendor_commands[] = {
@@ -49,6 +59,12 @@ static const struct wiphy_vendor_command wfx_nl80211_vendor_commands[] = {
 		.policy = wfx_nl_policy,
 		.doit = wfx_nl_burn_antirollback,
 		.maxattr = WFX_NL80211_ATTR_MAX - 1,
+	}, {
+		.info.vendor_id = WFX_NL80211_ID,
+		.info.subcmd = WFX_NL80211_SUBCMD_PTA_PARMS,
+		.policy = wfx_nl_policy,
+		.doit = wfx_nl_pta_params,
+		.maxattr = WFX_NL80211_ATTR_MAX - 1,
 	},
 };
 
diff --git a/drivers/staging/wfx/wfx.h b/drivers/staging/wfx/wfx.h
index ef68aa4086e01..078f7885bf2fa 100644
--- a/drivers/staging/wfx/wfx.h
+++ b/drivers/staging/wfx/wfx.h
@@ -60,6 +60,10 @@ struct wfx_dev {
 	struct mutex		rx_stats_lock;
 	struct hif_tx_power_loop_info tx_power_loop_info;
 	struct mutex		tx_power_loop_info_lock;
+
+	bool			pta_enable;
+	u32			pta_priority;
+	struct hif_req_pta_settings pta_settings;
 };
 
 struct wfx_vif {
-- 
2.26.2

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 10/10] staging: wfx: allow to run nl80211 vendor commands with 'iw'
  2020-05-26 17:18 [PATCH 00/10] staging: wfx: introduce nl80211 vendor extensions Jerome Pouiller
                   ` (8 preceding siblings ...)
  2020-05-26 17:18 ` [PATCH 09/10] staging: wfx: allow to set PTA settings Jerome Pouiller
@ 2020-05-26 17:18 ` Jerome Pouiller
  2020-05-27  8:22 ` [PATCH 00/10] staging: wfx: introduce nl80211 vendor extensions Greg Kroah-Hartman
  2020-05-27 12:34 ` Kalle Valo
  11 siblings, 0 replies; 15+ messages in thread
From: Jerome Pouiller @ 2020-05-26 17:18 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, David S . Miller, Kalle Valo

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

In current code, the nl80211 vendor extensions provided by the driver
use the new API[1]. It requires to pack the netlink attributes into a
NLA_NESTED.

Unfortunately, it is not the way the command 'iw vendor' works.

This patch, add extra vendor commands that can be called with
'iw vendor'.

[1] see commit 901bb989185516 ("nl80211: require and validate vendor
    command policy")

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/nl80211_vendor.h | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/staging/wfx/nl80211_vendor.h b/drivers/staging/wfx/nl80211_vendor.h
index 0ff3bf73f0ad3..b805b4aa951a0 100644
--- a/drivers/staging/wfx/nl80211_vendor.h
+++ b/drivers/staging/wfx/nl80211_vendor.h
@@ -23,8 +23,11 @@ int wfx_nl_pta_params(struct wiphy *wiphy, struct wireless_dev *widev,
 
 enum {
 	WFX_NL80211_SUBCMD_PS_TIMEOUT                   = 0x10,
+	WFX_NL80211_SUBCMD_PS_TIMEOUT_COMPAT            = 0x11,
 	WFX_NL80211_SUBCMD_BURN_PREVENT_ROLLBACK        = 0x20,
+	WFX_NL80211_SUBCMD_BURN_PREVENT_ROLLBACK_COMPAT = 0x21,
 	WFX_NL80211_SUBCMD_PTA_PARMS                    = 0x30,
+	WFX_NL80211_SUBCMD_PTA_PARMS_COMPAT             = 0x31,
 };
 
 enum {
@@ -53,18 +56,37 @@ static const struct wiphy_vendor_command wfx_nl80211_vendor_commands[] = {
 		.policy = wfx_nl_policy,
 		.doit = wfx_nl_ps_timeout,
 		.maxattr = WFX_NL80211_ATTR_MAX - 1,
+	}, {
+		// Compat with iw
+		.info.vendor_id = WFX_NL80211_ID,
+		.info.subcmd = WFX_NL80211_SUBCMD_PS_TIMEOUT_COMPAT,
+		.flags = WIPHY_VENDOR_CMD_NEED_WDEV,
+		.policy = VENDOR_CMD_RAW_DATA,
+		.doit = wfx_nl_ps_timeout,
 	}, {
 		.info.vendor_id = WFX_NL80211_ID,
 		.info.subcmd = WFX_NL80211_SUBCMD_BURN_PREVENT_ROLLBACK,
 		.policy = wfx_nl_policy,
 		.doit = wfx_nl_burn_antirollback,
 		.maxattr = WFX_NL80211_ATTR_MAX - 1,
+	}, {
+		// Compat with iw
+		.info.vendor_id = WFX_NL80211_ID,
+		.info.subcmd = WFX_NL80211_SUBCMD_BURN_PREVENT_ROLLBACK_COMPAT,
+		.policy = VENDOR_CMD_RAW_DATA,
+		.doit = wfx_nl_burn_antirollback,
 	}, {
 		.info.vendor_id = WFX_NL80211_ID,
 		.info.subcmd = WFX_NL80211_SUBCMD_PTA_PARMS,
 		.policy = wfx_nl_policy,
 		.doit = wfx_nl_pta_params,
 		.maxattr = WFX_NL80211_ATTR_MAX - 1,
+	}, {
+		// Compat with iw
+		.info.vendor_id = WFX_NL80211_ID,
+		.info.subcmd = WFX_NL80211_SUBCMD_PTA_PARMS_COMPAT,
+		.policy = VENDOR_CMD_RAW_DATA,
+		.doit = wfx_nl_pta_params,
 	},
 };
 
-- 
2.26.2

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 00/10] staging: wfx: introduce nl80211 vendor extensions
  2020-05-26 17:18 [PATCH 00/10] staging: wfx: introduce nl80211 vendor extensions Jerome Pouiller
                   ` (9 preceding siblings ...)
  2020-05-26 17:18 ` [PATCH 10/10] staging: wfx: allow to run nl80211 vendor commands with 'iw' Jerome Pouiller
@ 2020-05-27  8:22 ` Greg Kroah-Hartman
  2020-05-27 12:34 ` Kalle Valo
  11 siblings, 0 replies; 15+ messages in thread
From: Greg Kroah-Hartman @ 2020-05-27  8:22 UTC (permalink / raw)
  To: Jerome Pouiller
  Cc: devel, netdev, linux-wireless, linux-kernel, David S . Miller,
	Kalle Valo

On Tue, May 26, 2020 at 07:18:11PM +0200, Jerome Pouiller wrote:
> From: Jérôme Pouiller <jerome.pouiller@silabs.com>
> 
> Hello,
> 
> This series introduces some nl80211 vendor extensions to the wfx driver.
> 
> This series may lead to some discussions:

I've applied the first 6 patches here, until you get some answers from
the wifi developers about what to do with the rest.  Once you do, please
fix up / change them to meet their requirements, and then resend.

thanks,

greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 00/10] staging: wfx: introduce nl80211 vendor extensions
  2020-05-26 17:18 [PATCH 00/10] staging: wfx: introduce nl80211 vendor extensions Jerome Pouiller
                   ` (10 preceding siblings ...)
  2020-05-27  8:22 ` [PATCH 00/10] staging: wfx: introduce nl80211 vendor extensions Greg Kroah-Hartman
@ 2020-05-27 12:34 ` Kalle Valo
  2020-05-27 13:05   ` Jérôme Pouiller
  11 siblings, 1 reply; 15+ messages in thread
From: Kalle Valo @ 2020-05-27 12:34 UTC (permalink / raw)
  To: Jerome Pouiller
  Cc: devel, netdev, linux-wireless, linux-kernel, Greg Kroah-Hartman,
	David S . Miller

Jerome Pouiller <Jerome.Pouiller@silabs.com> writes:

> This series introduces some nl80211 vendor extensions to the wfx driver.
>
> This series may lead to some discussions:
>
>   1. Patch 7 allows to change the dynamic PS timeout. I have found
>      an API in wext (cfg80211_wext_siwpower()) that do more or less the
>      same thing. However, I have not found any equivalent in nl80211. Is it
>      expected or this API should be ported to nl80211?

struct wireless_dev::ps_timeout doesn't work for you?

>
>   2. The device The device allows to do Packet Traffic Arbitration (PTA or
>      also Coex). This feature allows the device to communicate with another
>      RF device in order to share the access to the RF. The patch 9 provides
>      a way to configure that. However, I think that this chip is not the
>      only one to provide this feature. Maybe a standard way to change
>      these parameters should be provided?
>
>   3. For these vendor extensions, I have used the new policy introduced by
>      the commit 901bb989185516 ("nl80211: require and validate vendor
>      command policy"). However, it seems that my version of 'iw' is not
>      able to follow this new policy (it does not pack the netlink
>      attributes into a NLA_NESTED). I could develop a tool specifically for
>      that API, but it is not very handy. So, in patch 10, I have also
>      introduced an API for compatibility with iw. Any comments about this?

If you want the driver out of staging I recommend not adding any vendor
commands until the driver is moved to drivers/net/wireless. Also do note
that we have special rules for nl80211 vendor commands:

https://wireless.wiki.kernel.org/en/developers/documentation/nl80211#vendor-specific_api

-- 
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 00/10] staging: wfx: introduce nl80211 vendor extensions
  2020-05-27 12:34 ` Kalle Valo
@ 2020-05-27 13:05   ` Jérôme Pouiller
  2020-05-29 15:13     ` Kalle Valo
  0 siblings, 1 reply; 15+ messages in thread
From: Jérôme Pouiller @ 2020-05-27 13:05 UTC (permalink / raw)
  To: Kalle Valo
  Cc: devel, netdev, linux-wireless, linux-kernel, Greg Kroah-Hartman,
	David S . Miller

On Wednesday 27 May 2020 14:34:37 CEST Kalle Valo wrote:
> Jerome Pouiller <Jerome.Pouiller@silabs.com> writes:
> 
> > This series introduces some nl80211 vendor extensions to the wfx driver.
> >
> > This series may lead to some discussions:
> >
> >   1. Patch 7 allows to change the dynamic PS timeout. I have found
> >      an API in wext (cfg80211_wext_siwpower()) that do more or less the
> >      same thing. However, I have not found any equivalent in nl80211. Is it
> >      expected or this API should be ported to nl80211?
> 
> struct wireless_dev::ps_timeout doesn't work for you?

Indeed, cfg80211_wext_siwpower() modify wireless_dev::ps_timeout, but
there is no equivalent in nl80211, no?

Else, I choose to not directly change wireless_dev::ps_timeout because I
worried about interactions with other parts of cfg80211/mac80211.


> >
> >   2. The device The device allows to do Packet Traffic Arbitration (PTA or
> >      also Coex). This feature allows the device to communicate with another
> >      RF device in order to share the access to the RF. The patch 9 provides
> >      a way to configure that. However, I think that this chip is not the
> >      only one to provide this feature. Maybe a standard way to change
> >      these parameters should be provided?
> >
> >   3. For these vendor extensions, I have used the new policy introduced by
> >      the commit 901bb989185516 ("nl80211: require and validate vendor
> >      command policy"). However, it seems that my version of 'iw' is not
> >      able to follow this new policy (it does not pack the netlink
> >      attributes into a NLA_NESTED). I could develop a tool specifically for
> >      that API, but it is not very handy. So, in patch 10, I have also
> >      introduced an API for compatibility with iw. Any comments about this?
> 
> If you want the driver out of staging I recommend not adding any vendor
> commands until the driver is moved to drivers/net/wireless. Also do note
> that we have special rules for nl80211 vendor commands:
> 
> https://wireless.wiki.kernel.org/en/developers/documentation/nl80211#vendor-specific_api

I hoped to suggest the move of this driver outside of staging in some
weeks (the last items in TODO list are either non-essential or easy to
fix). So, you suggest me to resend these patches after that change?

-- 
Jérôme Pouiller


_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 00/10] staging: wfx: introduce nl80211 vendor extensions
  2020-05-27 13:05   ` Jérôme Pouiller
@ 2020-05-29 15:13     ` Kalle Valo
  0 siblings, 0 replies; 15+ messages in thread
From: Kalle Valo @ 2020-05-29 15:13 UTC (permalink / raw)
  To: Jérôme Pouiller
  Cc: devel, netdev, linux-wireless, linux-kernel, Greg Kroah-Hartman,
	David S . Miller

Jérôme Pouiller <jerome.pouiller@silabs.com> writes:

> On Wednesday 27 May 2020 14:34:37 CEST Kalle Valo wrote:
>> Jerome Pouiller <Jerome.Pouiller@silabs.com> writes:
>> 
>> > This series introduces some nl80211 vendor extensions to the wfx driver.
>> >
>> > This series may lead to some discussions:
>> >
>> >   1. Patch 7 allows to change the dynamic PS timeout. I have found
>> >      an API in wext (cfg80211_wext_siwpower()) that do more or less the
>> >      same thing. However, I have not found any equivalent in nl80211. Is it
>> >      expected or this API should be ported to nl80211?
>> 
>> struct wireless_dev::ps_timeout doesn't work for you?
>
> Indeed, cfg80211_wext_siwpower() modify wireless_dev::ps_timeout, but
> there is no equivalent in nl80211, no?

Ah, I remember now. Something like 10 years ago there was a discussion
about using qos-pm framework for modifying the timeout (or something
like that, can't remember the details anymore) but no recollection what
was the end result.

> Else, I choose to not directly change wireless_dev::ps_timeout because I
> worried about interactions with other parts of cfg80211/mac80211.

This is exactly why we have strict rules for nl80211 vendor commands. We
want to have generic interfaces as much as possible, not each driver
coming up with their own interfaces.

>> >   2. The device The device allows to do Packet Traffic Arbitration (PTA or
>> >      also Coex). This feature allows the device to communicate with another
>> >      RF device in order to share the access to the RF. The patch 9 provides
>> >      a way to configure that. However, I think that this chip is not the
>> >      only one to provide this feature. Maybe a standard way to change
>> >      these parameters should be provided?
>> >
>> >   3. For these vendor extensions, I have used the new policy introduced by
>> >      the commit 901bb989185516 ("nl80211: require and validate vendor
>> >      command policy"). However, it seems that my version of 'iw' is not
>> >      able to follow this new policy (it does not pack the netlink
>> >      attributes into a NLA_NESTED). I could develop a tool specifically for
>> >      that API, but it is not very handy. So, in patch 10, I have also
>> >      introduced an API for compatibility with iw. Any comments about this?
>> 
>> If you want the driver out of staging I recommend not adding any vendor
>> commands until the driver is moved to drivers/net/wireless. Also do note
>> that we have special rules for nl80211 vendor commands:
>> 
>> https://wireless.wiki.kernel.org/en/developers/documentation/nl80211#vendor-specific_api
>
> I hoped to suggest the move of this driver outside of staging in some
> weeks (the last items in TODO list are either non-essential or easy to
> fix). So, you suggest me to resend these patches after that change?

It makes a lot easier for the review if there are no nl80211 vendor
commands in the driver, most likely you would need to remove them. So
yes, don't add anything unless absolutely essential until the driver is
accepted upstream. The smaller the driver the faster the review.

-- 
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2020-05-29 15:14 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-26 17:18 [PATCH 00/10] staging: wfx: introduce nl80211 vendor extensions Jerome Pouiller
2020-05-26 17:18 ` [PATCH 01/10] staging: wfx: drop unused variable Jerome Pouiller
2020-05-26 17:18 ` [PATCH 02/10] staging: wfx: do not declare variables inside loops Jerome Pouiller
2020-05-26 17:18 ` [PATCH 03/10] staging: wfx: drop unused function wfx_pending_requeue() Jerome Pouiller
2020-05-26 17:18 ` [PATCH 04/10] staging: wfx: add support for tx_power_loop Jerome Pouiller
2020-05-26 17:18 ` [PATCH 05/10] staging: wfx: retrieve the PS status from the vif Jerome Pouiller
2020-05-26 17:18 ` [PATCH 06/10] staging: wfx: split wfx_get_ps_timeout() from wfx_update_pm() Jerome Pouiller
2020-05-26 17:18 ` [PATCH 07/10] staging: wfx: add support for set/get ps_timeout Jerome Pouiller
2020-05-26 17:18 ` [PATCH 08/10] staging: wfx: allow to burn prevent rollback bit Jerome Pouiller
2020-05-26 17:18 ` [PATCH 09/10] staging: wfx: allow to set PTA settings Jerome Pouiller
2020-05-26 17:18 ` [PATCH 10/10] staging: wfx: allow to run nl80211 vendor commands with 'iw' Jerome Pouiller
2020-05-27  8:22 ` [PATCH 00/10] staging: wfx: introduce nl80211 vendor extensions Greg Kroah-Hartman
2020-05-27 12:34 ` Kalle Valo
2020-05-27 13:05   ` Jérôme Pouiller
2020-05-29 15:13     ` Kalle Valo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).