ath10k.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ath10k:add support for multicast rate control
@ 2018-04-12  4:04 Pradeep Kumar Chitrapu
  2018-04-12  8:00 ` Sven Eckelmann
  2020-12-21 17:23 ` Kalle Valo
  0 siblings, 2 replies; 5+ messages in thread
From: Pradeep Kumar Chitrapu @ 2018-04-12  4:04 UTC (permalink / raw)
  To: ath10k; +Cc: Pradeep Kumar Chitrapu, linux-wireless

Issues wmi command to firmware when multicast rate change is received
with the new BSS_CHANGED_MCAST_RATE flag.

Signed-off-by: Pradeep Kumar Chitrapu <pradeepc@codeaurora.org>
---
 drivers/net/wireless/ath/ath10k/mac.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index bf05a36..63af46f 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -5419,8 +5419,12 @@ static void ath10k_bss_info_changed(struct ieee80211_hw *hw,
 {
 	struct ath10k *ar = hw->priv;
 	struct ath10k_vif *arvif = (void *)vif->drv_priv;
-	int ret = 0;
+	struct ieee80211_supported_band *sband;
+	struct cfg80211_chan_def def;
 	u32 vdev_param, pdev_param, slottime, preamble;
+	int rate_index, ret = 0;
+	u8 rate;
+	enum nl80211_band band;
 
 	mutex_lock(&ar->conf_mutex);
 
@@ -5588,6 +5592,25 @@ static void ath10k_bss_info_changed(struct ieee80211_hw *hw,
 				    arvif->vdev_id, ret);
 	}
 
+	if (changed & BSS_CHANGED_MCAST_RATE &&
+	    !WARN_ON(ath10k_mac_vif_chan(arvif->vif, &def))) {
+		band = def.chan->band;
+		sband = &ar->mac.sbands[band];
+		vdev_param = ar->wmi.vdev_param->mcast_data_rate;
+		rate_index = vif->bss_conf.mcast_rate[band] - 1;
+		rate = ATH10K_HW_MCS_RATE(sband->bitrates[rate_index].hw_value);
+		ath10k_dbg(ar, ATH10K_DBG_MAC,
+			   "mac vdev %d mcast_rate %d\n",
+			   arvif->vdev_id, rate);
+
+		ret = ath10k_wmi_vdev_set_param(ar, arvif->vdev_id,
+						vdev_param, rate);
+		if (ret)
+			ath10k_warn(ar,
+				    "failed to set mcast rate on vdev"
+				    " %i: %d\n", arvif->vdev_id,  ret);
+	}
+
 	mutex_unlock(&ar->conf_mutex);
 }
 
-- 
1.9.1


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

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

* Re: [PATCH] ath10k:add support for multicast rate control
  2018-04-12  4:04 [PATCH] ath10k:add support for multicast rate control Pradeep Kumar Chitrapu
@ 2018-04-12  8:00 ` Sven Eckelmann
  2020-12-21 17:23 ` Kalle Valo
  1 sibling, 0 replies; 5+ messages in thread
From: Sven Eckelmann @ 2018-04-12  8:00 UTC (permalink / raw)
  To: ath10k; +Cc: Pradeep Kumar Chitrapu, linux-wireless


[-- Attachment #1.1.1: Type: text/plain, Size: 2293 bytes --]

On Mittwoch, 11. April 2018 21:04:46 CEST Pradeep Kumar Chitrapu wrote:
> Issues wmi command to firmware when multicast rate change is received
> with the new BSS_CHANGED_MCAST_RATE flag.
[...]
>  
> +	if (changed & BSS_CHANGED_MCAST_RATE &&
> +	    !WARN_ON(ath10k_mac_vif_chan(arvif->vif, &def))) {
> +		band = def.chan->band;
> +		sband = &ar->mac.sbands[band];
> +		vdev_param = ar->wmi.vdev_param->mcast_data_rate;
> +		rate_index = vif->bss_conf.mcast_rate[band] - 1;
> +		rate = ATH10K_HW_MCS_RATE(sband->bitrates[rate_index].hw_value);
> +		ath10k_dbg(ar, ATH10K_DBG_MAC,
> +			   "mac vdev %d mcast_rate %d\n",
> +			   arvif->vdev_id, rate);
> +
> +		ret = ath10k_wmi_vdev_set_param(ar, arvif->vdev_id,
> +						vdev_param, rate);
> +		if (ret)
> +			ath10k_warn(ar,
> +				    "failed to set mcast rate on vdev"
> +				    " %i: %d\n", arvif->vdev_id,  ret);
> +	}
> +
>  	mutex_unlock(&ar->conf_mutex);
>  }
>  
> 

I see two major problems here without checking the actual implementation 
details:

* hw_value is incorrect for a couple of devices. Some devices use a different 
  mapping when they receive rates inforamtion (hw_value) then the ones you use
  for the mcast/bcast/beacon rate setting. I've handled in my POC patch like
  this:

    +		if (ath10k_mac_bitrate_is_cck(sband->bitrates[i].bitrate)) {
    +			preamble = WMI_RATE_PREAMBLE_CCK;
    +
    +			/* QCA didn't use the correct rate values for CA99x0
    +			 * and above (ath10k_g_rates_rev2)
    +			 */
    +			switch (sband->bitrates[i].bitrate) {
    +			case 10:
    +				hw_value = ATH10K_HW_RATE_CCK_LP_1M;
    +				break;
    +			case 20:
    +				hw_value = ATH10K_HW_RATE_CCK_LP_2M;
    +				break;
    +			case 55:
    +				hw_value = ATH10K_HW_RATE_CCK_LP_5_5M;
    +				break;
    +			case 110:
    +				hw_value = ATH10K_HW_RATE_CCK_LP_11M;
    +				break;
    +			}
    +		} else {
    +			preamble = WMI_RATE_PREAMBLE_OFDM;
    +		}

* bcast + mcast (+ mgmt) have to be set separately

I have attached my POC patch (which I was using for packet loss based mesh 
metrics) and to work around (using debugfs) the silly mgmt vs. mcast/bcast 
settings of the QCA fw for APs.

Many of the information came from Ben Greears ath10k-ct driver 
https://github.com/greearb/ath10k-ct 

Kind regards,	
	Sven

[-- Attachment #1.1.2: 9536-ath10k-Allow-to-configure-bcast-mcast-rate.patch --]
[-- Type: text/x-patch, Size: 9561 bytes --]

From: Sven Eckelmann <sven@narfation.org>
Date: Fri, 16 Feb 2018 13:49:51 +0100
Subject: [PATCH] ath10k: Allow to configure bcast/mcast rate

TODO:

 * find better way to get mcast_rate
 * better get the lowest configured basic rate for APs
 * remove netifd WAR

Signed-off-by: Sven Eckelmann <sven@narfation.org>

Forwarded: no
 not yet in the correct shape
---
 drivers/net/wireless/ath/ath10k/core.h  |   1 +
 drivers/net/wireless/ath/ath10k/debug.c |  78 ++++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/debug.h |  11 ++++
 drivers/net/wireless/ath/ath10k/mac.c   | 113 ++++++++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/mac.h   |   1 +
 5 files changed, 204 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 3ff4a423c4d873d322deebd3c498ef0688e84e05..a53f7b2042f4a80bbd358b00d4d26d6fabe5df6e 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -423,6 +423,7 @@ struct ath10k_vif {
 	bool nohwcrypt;
 	int num_legacy_stations;
 	int txpower;
+	u16 mcast_rate;
 	struct wmi_wmm_params_all_arg wmm_params;
 	struct work_struct ap_csa_work;
 	struct delayed_work connection_loss_work;
diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
index fa72ef54605e74f501ab655a6afd0a50b89b6b7e..fc59f214d2a5288f2ac41824e584def787b4799c 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -23,6 +23,7 @@
 #include <linux/firmware.h>
 
 #include "core.h"
+#include "mac.h"
 #include "debug.h"
 #include "hif.h"
 #include "wmi-ops.h"
@@ -2454,6 +2455,83 @@ void ath10k_debug_unregister(struct ath10k *ar)
 	cancel_delayed_work_sync(&ar->debug.htt_stats_dwork);
 }
 
+
+
+static ssize_t ath10k_write_mcast_rate(struct file *file,
+				       const char __user *ubuf,
+				       size_t count, loff_t *ppos)
+{
+	struct ath10k_vif *arvif = file->private_data;
+	struct ath10k *ar = arvif->ar;
+	ssize_t ret = 0;
+	u32 mcast_rate;
+
+	ret = kstrtou32_from_user(ubuf, count, 0, &mcast_rate);
+	if (ret)
+		return ret;
+
+	mutex_lock(&ar->conf_mutex);
+
+	arvif->mcast_rate = mcast_rate;
+	ret = ath10k_mac_set_mcast_rate(arvif);
+	if (ret)
+		goto unlock;
+
+	ret = count;
+unlock:
+	mutex_unlock(&ar->conf_mutex);
+
+	return ret;
+}
+
+static ssize_t ath10k_read_mcast_rate(struct file *file, char __user *ubuf,
+				      size_t count, loff_t *ppos)
+
+{
+	struct ath10k_vif *arvif = file->private_data;
+	struct ath10k *ar = arvif->ar;
+	char buf[32];
+	int len = 0;
+	u16 mcast_rate;
+
+	mutex_lock(&ar->conf_mutex);
+	mcast_rate = arvif->mcast_rate;
+	mutex_unlock(&ar->conf_mutex);
+
+	len = scnprintf(buf, sizeof(buf) - len, "%u\n", mcast_rate);
+
+	return simple_read_from_buffer(ubuf, count, ppos, buf, len);
+}
+
+static const struct file_operations fops_mcast_rate = {
+	.read = ath10k_read_mcast_rate,
+	.write = ath10k_write_mcast_rate,
+	.open = simple_open
+};
+
+int ath10k_debug_register_netdev(struct ath10k_vif *arvif)
+{
+	struct dentry *debugfs_netdev;
+
+	debugfs_netdev = debugfs_create_dir("ath10k", arvif->vif->debugfs_dir);
+	if (IS_ERR_OR_NULL(debugfs_netdev)) {
+		if (IS_ERR(debugfs_netdev))
+			return PTR_ERR(debugfs_netdev);
+
+		return -ENOMEM;
+	}
+
+	debugfs_create_file("mcast_rate", S_IRUGO | S_IWUSR,
+			    debugfs_netdev, arvif,
+			    &fops_mcast_rate);
+
+	return 0;
+}
+
+void ath10k_debug_unregister_netdev(struct ath10k_vif *arvif)
+{
+}
+
 #endif /* CPTCFG_ATH10K_DEBUGFS */
 
 #ifdef CPTCFG_ATH10K_DEBUG
diff --git a/drivers/net/wireless/ath/ath10k/debug.h b/drivers/net/wireless/ath/ath10k/debug.h
index 43dbcdbb3e1bb7bb495377f6a6c1d487453a0a0d..24edf6cf15de1bf22c1126e22051b0aad604d6d7 100644
--- a/drivers/net/wireless/ath/ath10k/debug.h
+++ b/drivers/net/wireless/ath/ath10k/debug.h
@@ -77,6 +77,8 @@ int ath10k_debug_create(struct ath10k *ar);
 void ath10k_debug_destroy(struct ath10k *ar);
 int ath10k_debug_register(struct ath10k *ar);
 void ath10k_debug_unregister(struct ath10k *ar);
+int ath10k_debug_register_netdev(struct ath10k_vif *arvif);
+void ath10k_debug_unregister_netdev(struct ath10k_vif *arvif);
 void ath10k_debug_fw_stats_process(struct ath10k *ar, struct sk_buff *skb);
 void ath10k_debug_tpc_stats_process(struct ath10k *ar,
 				    struct ath10k_tpc_stats *tpc_stats);
@@ -134,6 +136,15 @@ static inline void ath10k_debug_unregister(struct ath10k *ar)
 {
 }
 
+static inline int ath10k_debug_register_netdev(struct ath10k_vif *arvif)
+{
+	return 0;
+}
+
+static inline void ath10k_debug_unregister_netdev(struct ath10k_vif *arvif)
+{
+}
+
 static inline void ath10k_debug_fw_stats_process(struct ath10k *ar,
 						 struct sk_buff *skb)
 {
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 938b8c303312ffaccc14c46cdabbb90e80077359..2bea5e8a6e42cacfa289f0e05052a822c591b05c 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -152,6 +152,101 @@ u8 ath10k_mac_bitrate_to_idx(const struct ieee80211_supported_band *sband,
 	return 0;
 }
 
+int ath10k_mac_set_mcast_rate(struct ath10k_vif *arvif)
+{
+	const struct ieee80211_supported_band *sband;
+	struct ath10k *ar = arvif->ar;
+	struct cfg80211_chan_def def;
+	enum nl80211_band band;
+	u16 best_bitrate = 0;
+	u16 hw_value;
+	u32 ratemask;
+	u8 rate_code;
+	u8 preamble;
+	int i;
+	int ret;
+
+	lockdep_assert_held(&ar->conf_mutex);
+
+	if (ath10k_mac_vif_chan(arvif->vif, &def))
+		return -EPERM;
+
+	band = def.chan->band;
+	sband = ar->hw->wiphy->bands[band];
+	ratemask = arvif->bitrate_mask.control[band].legacy;
+
+	/* it seems that arvif is lost on every fw crash
+	 * read the lowest mcast read of bss
+	 */
+	if (!arvif->mcast_rate && arvif->vif->bss_conf.mcast_rate[band])
+		arvif->mcast_rate = sband->bitrates[arvif->vif->bss_conf.mcast_rate[band] - 1].bitrate;
+
+	for (i = 0; i < sband->n_bitrates; i++) {
+		if (!(ratemask & BIT(i)))
+			continue;
+
+		if (best_bitrate &&
+		    arvif->mcast_rate != sband->bitrates[i].bitrate)
+			continue;
+
+		best_bitrate = sband->bitrates[i].bitrate;
+		hw_value = sband->bitrates[i].hw_value;
+
+		if (ath10k_mac_bitrate_is_cck(sband->bitrates[i].bitrate)) {
+			preamble = WMI_RATE_PREAMBLE_CCK;
+
+			/* QCA didn't use the correct rate values for CA99x0
+			 * and above (ath10k_g_rates_rev2)
+			 */
+			switch (sband->bitrates[i].bitrate) {
+			case 10:
+				hw_value = ATH10K_HW_RATE_CCK_LP_1M;
+				break;
+			case 20:
+				hw_value = ATH10K_HW_RATE_CCK_LP_2M;
+				break;
+			case 55:
+				hw_value = ATH10K_HW_RATE_CCK_LP_5_5M;
+				break;
+			case 110:
+				hw_value = ATH10K_HW_RATE_CCK_LP_11M;
+				break;
+			}
+		} else {
+			preamble = WMI_RATE_PREAMBLE_OFDM;
+		}
+
+		rate_code = ATH10K_HW_RATECODE(hw_value, 0, preamble);
+	}
+
+	if (!best_bitrate) {
+		ath10k_warn(ar, "failed to select multicast rate\n");
+		return -EINVAL;
+	}
+
+	arvif->mcast_rate = best_bitrate;
+
+	ret = ath10k_wmi_vdev_set_param(ar, arvif->vdev_id,
+					ar->wmi.vdev_param->mgmt_rate,
+					rate_code);
+	if (ret)
+		ath10k_warn(ar, "failed to set mgmt fixed rate: %d\n",ret);
+
+	ret = ath10k_wmi_vdev_set_param(ar, arvif->vdev_id,
+					ar->wmi.vdev_param->bcast_data_rate,
+					rate_code);
+	if (ret)
+		ath10k_warn(ar, "failed to set bcast fixed rate: %d\n",ret);
+
+	ret = ath10k_wmi_vdev_set_param(ar, arvif->vdev_id,
+					ar->wmi.vdev_param->mcast_data_rate,
+					rate_code);
+	if (ret)
+		ath10k_warn(ar, "failed to set mcast fixed rate: %d\n",ret);
+
+	return 0;
+}
+
 static int ath10k_mac_get_max_vht_mcs_map(u16 mcs_map, int nss)
 {
 	switch ((mcs_map >> (2 * nss)) & 0x3) {
@@ -5133,6 +5228,9 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
 	spin_unlock_bh(&ar->htt.tx_lock);
 
 	mutex_unlock(&ar->conf_mutex);
+
+	ath10k_debug_register_netdev(arvif);
+
 	return 0;
 
 err_peer_delete:
@@ -5179,6 +5277,8 @@ static void ath10k_remove_interface(struct ieee80211_hw *hw,
 	cancel_work_sync(&arvif->ap_csa_work);
 	cancel_delayed_work_sync(&arvif->connection_loss_work);
 
+	ath10k_debug_unregister_netdev(arvif);
+
 	mutex_lock(&ar->conf_mutex);
 
 	spin_lock_bh(&ar->data_lock);
@@ -5474,6 +5574,12 @@ static void ath10k_bss_info_changed(struct ieee80211_hw *hw,
 				    arvif->vdev_id, ret);
 	}
 
+	/* TODO when should we actually call that */
+	ret = ath10k_mac_set_mcast_rate(arvif);
+	if (ret)
+		ath10k_warn(ar, "failed to set multicast rate params on vdev %i: %d\n",
+			    arvif->vdev_id, ret);
+
 	mutex_unlock(&ar->conf_mutex);
 }
 
@@ -6958,6 +7064,13 @@ static int ath10k_mac_op_set_bitrate_mask(struct ieee80211_hw *hw,
 		goto exit;
 	}
 
+	ret = ath10k_mac_set_mcast_rate(arvif);
+	if (ret) {
+		ath10k_warn(ar, "failed to set multicast rate params on vdev %i: %d\n",
+			    arvif->vdev_id, ret);
+		goto exit;
+	}
+
 exit:
 	mutex_unlock(&ar->conf_mutex);
 
diff --git a/drivers/net/wireless/ath/ath10k/mac.h b/drivers/net/wireless/ath/ath10k/mac.h
index 1bd29ecfcdcc913ff8d3e447eb0d85c4d3c56ec2..db3966028c629b29bfeea3222597e8ba8d203556 100644
--- a/drivers/net/wireless/ath/ath10k/mac.h
+++ b/drivers/net/wireless/ath/ath10k/mac.h
@@ -69,6 +69,7 @@ u8 ath10k_mac_hw_rate_to_idx(const struct ieee80211_supported_band *sband,
 			     u8 hw_rate, bool cck);
 u8 ath10k_mac_bitrate_to_idx(const struct ieee80211_supported_band *sband,
 			     u32 bitrate);
+int ath10k_mac_set_mcast_rate(struct ath10k_vif *arvif);
 
 void ath10k_mac_tx_lock(struct ath10k *ar, int reason);
 void ath10k_mac_tx_unlock(struct ath10k *ar, int reason);

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

[-- Attachment #2: Type: text/plain, Size: 146 bytes --]

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

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

* Re: [PATCH] ath10k:add support for multicast rate control
  2018-04-12  4:04 [PATCH] ath10k:add support for multicast rate control Pradeep Kumar Chitrapu
  2018-04-12  8:00 ` Sven Eckelmann
@ 2020-12-21 17:23 ` Kalle Valo
  1 sibling, 0 replies; 5+ messages in thread
From: Kalle Valo @ 2020-12-21 17:23 UTC (permalink / raw)
  To: Pradeep Kumar Chitrapu; +Cc: Pradeep Kumar Chitrapu, linux-wireless, ath10k

Pradeep Kumar Chitrapu <pradeepc@codeaurora.org> wrote:

> Issues wmi command to firmware when multicast rate change is received
> with the new BSS_CHANGED_MCAST_RATE flag.
> 
> Signed-off-by: Pradeep Kumar Chitrapu <pradeepc@codeaurora.org>

(Cleaning up my backlog, this is from 2018.) I don't understand the
issue from commit log, please clarify and resend if this is still valid.

Patch set to Changes Requested.

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/1523505886-14695-1-git-send-email-pradeepc@codeaurora.org/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

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

* Re: [PATCH] ath10k:add support for multicast rate control
  2018-04-17  5:10 pradeepc
@ 2018-04-17  6:33 ` Sven Eckelmann
  0 siblings, 0 replies; 5+ messages in thread
From: Sven Eckelmann @ 2018-04-17  6:33 UTC (permalink / raw)
  To: pradeepc; +Cc: linux-wireless, ath10k


[-- Attachment #1.1: Type: text/plain, Size: 1416 bytes --]

On Montag, 16. April 2018 22:10:50 CEST pradeepc@codeaurora.org wrote:
[...]
> > I see two major problems here without checking the actual 
> > implementation
> > details:
> > 
> > * hw_value is incorrect for a couple of devices. Some devices use a 
> > different
> >   mapping when they receive rates inforamtion (hw_value) then the ones 
> > you use
> >   for the mcast/bcast/beacon rate setting. I've handled in my POC patch 
> > like
[...]
> Isn't this already fixed in https://patchwork.kernel.org/patch/9150145/ 
> ?

No, this is exactly the patch which causes the problem for you. The parameter 
for these settings stayed the same but this patch changes the order in 
ieee80211_rate to make it easier for the driver to parse the (reordered) rx 
status information for CCK rates (maybe also tx - not sure about that right 
now).

> > * bcast + mcast (+ mgmt) have to be set separately
> 
> Can you please let me know why this would be necessary?
> Although I see minstrel rate control is currently seems using the 
> mcast_rate
> setting to set MGMT/BCAST/MCAST rates, will this not be misleading to 
> user
> passed value with 'iw set mcast_rate' for MGMT traffic as well?

What about broadcast? MGMT was only mentioned here because it is the third 
item which can be manipulated the same way. But yes, it would be good to get 
the mgmt rates in sync with the non-hw-rate-control drivers.

Kind regards,
	Sven

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

[-- Attachment #2: Type: text/plain, Size: 146 bytes --]

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

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

* Re: [PATCH] ath10k:add support for multicast rate control
@ 2018-04-17  5:10 pradeepc
  2018-04-17  6:33 ` Sven Eckelmann
  0 siblings, 1 reply; 5+ messages in thread
From: pradeepc @ 2018-04-17  5:10 UTC (permalink / raw)
  To: Sven Eckelmann; +Cc: linux-wireless, ath10k

On 2018-04-12 01:00, Sven Eckelmann wrote:
> On Mittwoch, 11. April 2018 21:04:46 CEST Pradeep Kumar Chitrapu wrote:
>> Issues wmi command to firmware when multicast rate change is received
>> with the new BSS_CHANGED_MCAST_RATE flag.
> [...]
>> 
>> +	if (changed & BSS_CHANGED_MCAST_RATE &&
>> +	    !WARN_ON(ath10k_mac_vif_chan(arvif->vif, &def))) {
>> +		band = def.chan->band;
>> +		sband = &ar->mac.sbands[band];
>> +		vdev_param = ar->wmi.vdev_param->mcast_data_rate;
>> +		rate_index = vif->bss_conf.mcast_rate[band] - 1;
>> +		rate = ATH10K_HW_MCS_RATE(sband->bitrates[rate_index].hw_value);
>> +		ath10k_dbg(ar, ATH10K_DBG_MAC,
>> +			   "mac vdev %d mcast_rate %d\n",
>> +			   arvif->vdev_id, rate);
>> +
>> +		ret = ath10k_wmi_vdev_set_param(ar, arvif->vdev_id,
>> +						vdev_param, rate);
>> +		if (ret)
>> +			ath10k_warn(ar,
>> +				    "failed to set mcast rate on vdev"
>> +				    " %i: %d\n", arvif->vdev_id,  ret);
>> +	}
>> +
>>  	mutex_unlock(&ar->conf_mutex);
>>  }
>> 
>> 
> 
> I see two major problems here without checking the actual 
> implementation
> details:
> 
> * hw_value is incorrect for a couple of devices. Some devices use a 
> different
>   mapping when they receive rates inforamtion (hw_value) then the ones 
> you use
>   for the mcast/bcast/beacon rate setting. I've handled in my POC patch 
> like
>   this:
> 
>     +		if (ath10k_mac_bitrate_is_cck(sband->bitrates[i].bitrate)) {
>     +			preamble = WMI_RATE_PREAMBLE_CCK;
>     +
>     +			/* QCA didn't use the correct rate values for CA99x0
>     +			 * and above (ath10k_g_rates_rev2)
>     +			 */
>     +			switch (sband->bitrates[i].bitrate) {
>     +			case 10:
>     +				hw_value = ATH10K_HW_RATE_CCK_LP_1M;
>     +				break;
>     +			case 20:
>     +				hw_value = ATH10K_HW_RATE_CCK_LP_2M;
>     +				break;
>     +			case 55:
>     +				hw_value = ATH10K_HW_RATE_CCK_LP_5_5M;
>     +				break;
>     +			case 110:
>     +				hw_value = ATH10K_HW_RATE_CCK_LP_11M;
>     +				break;
>     +			}
>     +		} else {
>     +			preamble = WMI_RATE_PREAMBLE_OFDM;
>     +		}
> 

Isn't this already fixed in https://patchwork.kernel.org/patch/9150145/ 
?

> * bcast + mcast (+ mgmt) have to be set separately

Can you please let me know why this would be necessary?
Although I see minstrel rate control is currently seems using the 
mcast_rate
setting to set MGMT/BCAST/MCAST rates, will this not be misleading to 
user
passed value with 'iw set mcast_rate' for MGMT traffic as well?

> 
> I have attached my POC patch (which I was using for packet loss based 
> mesh
> metrics) and to work around (using debugfs) the silly mgmt vs. 
> mcast/bcast
> settings of the QCA fw for APs.
> 
> Many of the information came from Ben Greears ath10k-ct driver
> https://github.com/greearb/ath10k-ct
> 
> Kind regards,
> 	Sven

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

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

end of thread, other threads:[~2020-12-21 17:24 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-12  4:04 [PATCH] ath10k:add support for multicast rate control Pradeep Kumar Chitrapu
2018-04-12  8:00 ` Sven Eckelmann
2020-12-21 17:23 ` Kalle Valo
2018-04-17  5:10 pradeepc
2018-04-17  6:33 ` Sven Eckelmann

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).