LKML Archive on lore.kernel.org
 help / Atom feed
* [RFC PATCH v2 1/2] mwifiex: refactor mwifiex_parse_htinfo() for reuse
@ 2018-12-07  0:12 Brian Norris
  2018-12-07  0:12 ` [RFC PATCH v2 2/2] mwifiex: add NL80211_STA_INFO_RX_BITRATE support Brian Norris
  2018-12-13 14:55 ` [RFC PATCH v2 1/2] mwifiex: refactor mwifiex_parse_htinfo() for reuse Kalle Valo
  0 siblings, 2 replies; 5+ messages in thread
From: Brian Norris @ 2018-12-07  0:12 UTC (permalink / raw)
  To: Kalle Valo
  Cc: linux-kernel, Amitkumar Karwar, Nishant Sarmukadam,
	Ganapathi Bhat, Xinming Hu, linux-wireless, Brian Norris

This function converts some firmware-specific parameters into cfg80211
'rate_info' structures. It currently assumes it's dealing only with TX
bitrate, but the RX bitrate looks to be the same, so refactor this
function to be reusable.

Signed-off-by: Brian Norris <briannorris@chromium.org>
---
v2:
 * no change - just split unrelated (debugfs) patch to its own series
---
 .../net/wireless/marvell/mwifiex/cfg80211.c   | 36 ++++++++++---------
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index adc88433faa8..02b80ea232a7 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -1275,27 +1275,27 @@ mwifiex_cfg80211_change_virtual_intf(struct wiphy *wiphy,
 }
 
 static void
-mwifiex_parse_htinfo(struct mwifiex_private *priv, u8 tx_htinfo,
+mwifiex_parse_htinfo(struct mwifiex_private *priv, u8 rateinfo, u8 htinfo,
 		     struct rate_info *rate)
 {
 	struct mwifiex_adapter *adapter = priv->adapter;
 
 	if (adapter->is_hw_11ac_capable) {
 		/* bit[1-0]: 00=LG 01=HT 10=VHT */
-		if (tx_htinfo & BIT(0)) {
+		if (htinfo & BIT(0)) {
 			/* HT */
-			rate->mcs = priv->tx_rate;
+			rate->mcs = rateinfo;
 			rate->flags |= RATE_INFO_FLAGS_MCS;
 		}
-		if (tx_htinfo & BIT(1)) {
+		if (htinfo & BIT(1)) {
 			/* VHT */
-			rate->mcs = priv->tx_rate & 0x0F;
+			rate->mcs = rateinfo & 0x0F;
 			rate->flags |= RATE_INFO_FLAGS_VHT_MCS;
 		}
 
-		if (tx_htinfo & (BIT(1) | BIT(0))) {
+		if (htinfo & (BIT(1) | BIT(0))) {
 			/* HT or VHT */
-			switch (tx_htinfo & (BIT(3) | BIT(2))) {
+			switch (htinfo & (BIT(3) | BIT(2))) {
 			case 0:
 				rate->bw = RATE_INFO_BW_20;
 				break;
@@ -1310,26 +1310,26 @@ mwifiex_parse_htinfo(struct mwifiex_private *priv, u8 tx_htinfo,
 				break;
 			}
 
-			if (tx_htinfo & BIT(4))
+			if (htinfo & BIT(4))
 				rate->flags |= RATE_INFO_FLAGS_SHORT_GI;
 
-			if ((priv->tx_rate >> 4) == 1)
+			if ((rateinfo >> 4) == 1)
 				rate->nss = 2;
 			else
 				rate->nss = 1;
 		}
 	} else {
 		/*
-		 * Bit 0 in tx_htinfo indicates that current Tx rate
-		 * is 11n rate. Valid MCS index values for us are 0 to 15.
+		 * Bit 0 in htinfo indicates that current rate is 11n. Valid
+		 * MCS index values for us are 0 to 15.
 		 */
-		if ((tx_htinfo & BIT(0)) && (priv->tx_rate < 16)) {
-			rate->mcs = priv->tx_rate;
+		if ((htinfo & BIT(0)) && (rateinfo < 16)) {
+			rate->mcs = rateinfo;
 			rate->flags |= RATE_INFO_FLAGS_MCS;
 			rate->bw = RATE_INFO_BW_20;
-			if (tx_htinfo & BIT(1))
+			if (htinfo & BIT(1))
 				rate->bw = RATE_INFO_BW_40;
-			if (tx_htinfo & BIT(2))
+			if (htinfo & BIT(2))
 				rate->flags |= RATE_INFO_FLAGS_SHORT_GI;
 		}
 	}
@@ -1375,7 +1375,8 @@ mwifiex_dump_station_info(struct mwifiex_private *priv,
 		sinfo->tx_packets = node->stats.tx_packets;
 		sinfo->tx_failed = node->stats.tx_failed;
 
-		mwifiex_parse_htinfo(priv, node->stats.last_tx_htinfo,
+		mwifiex_parse_htinfo(priv, priv->tx_rate,
+				     node->stats.last_tx_htinfo,
 				     &sinfo->txrate);
 		sinfo->txrate.legacy = node->stats.last_tx_rate * 5;
 
@@ -1401,7 +1402,8 @@ mwifiex_dump_station_info(struct mwifiex_private *priv,
 			 HostCmd_ACT_GEN_GET, DTIM_PERIOD_I,
 			 &priv->dtim_period, true);
 
-	mwifiex_parse_htinfo(priv, priv->tx_htinfo, &sinfo->txrate);
+	mwifiex_parse_htinfo(priv, priv->tx_rate, priv->tx_htinfo,
+			     &sinfo->txrate);
 
 	sinfo->signal_avg = priv->bcn_rssi_avg;
 	sinfo->rx_bytes = priv->stats.rx_bytes;
-- 
2.20.0.rc2.403.gdbc3b29805-goog


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

* [RFC PATCH v2 2/2] mwifiex: add NL80211_STA_INFO_RX_BITRATE support
  2018-12-07  0:12 [RFC PATCH v2 1/2] mwifiex: refactor mwifiex_parse_htinfo() for reuse Brian Norris
@ 2018-12-07  0:12 ` Brian Norris
  2018-12-07 10:50   ` Kalle Valo
  2018-12-13 14:55 ` [RFC PATCH v2 1/2] mwifiex: refactor mwifiex_parse_htinfo() for reuse Kalle Valo
  1 sibling, 1 reply; 5+ messages in thread
From: Brian Norris @ 2018-12-07  0:12 UTC (permalink / raw)
  To: Kalle Valo
  Cc: linux-kernel, Amitkumar Karwar, Nishant Sarmukadam,
	Ganapathi Bhat, Xinming Hu, linux-wireless, Brian Norris

Comparing the existing TX_BITRATE parsing code (in
mwifiex_parse_htinfo()) with the RX bitrate histograms in debugfs.c, it
appears that the rxpd_rate and rxpd_htinfo fields have the same format.
At least, they give reasonable results when I parse them this way.

So this patch adds support for RX_BITRATE to our station info dump.

Along the way, I add legacy bitrate parsing into the same function,
using the debugfs code (mwifiex_histogram_read() and
mwifiex_adjust_data_rate()) as reference.

Additionally, to satisfy the requirements of
NL80211_STA_INFO_RX_BITRATE, I skip logging the bitrate of multicast
packets. This shouldn't add a lot of overhead to the RX path, as there
are already several similar 802.3 header checks in this same codepath.
We can also bias the branch behavior to favor unicast, as that's the
common performance-sensitive case.

I'd consider this support somewhat experimental, as I have zero
documentation from Marvell. But the existing driver code gives me good
reason to think this is correct.

I've tested this on a few different 802.11{a,b,g,n,ac} networks, and the
reported bitrates look good to me.

Signed-off-by: Brian Norris <briannorris@chromium.org>
---
RFC: I'd appreciate it if someone from Marvell could double check my work
here.

v2:
 * no change - just split unrelated (debugfs) patch to its own series
---
 .../net/wireless/marvell/mwifiex/cfg80211.c   | 26 +++++++++++++++++++
 drivers/net/wireless/marvell/mwifiex/sta_rx.c | 13 ++++++----
 2 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index 02b80ea232a7..1467af22e394 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -1333,6 +1333,28 @@ mwifiex_parse_htinfo(struct mwifiex_private *priv, u8 rateinfo, u8 htinfo,
 				rate->flags |= RATE_INFO_FLAGS_SHORT_GI;
 		}
 	}
+
+	/* Decode legacy rates for non-HT. */
+	if (!(htinfo & (BIT(0) | BIT(1)))) {
+		/* Bitrates in multiples of 100kb/s. */
+		static const int legacy_rates[] = {
+			[0] = 10,
+			[1] = 20,
+			[2] = 55,
+			[3] = 110,
+			[4] = 60, /* MWIFIEX_RATE_INDEX_OFDM0 */
+			[5] = 60,
+			[6] = 90,
+			[7] = 120,
+			[8] = 180,
+			[9] = 240,
+			[10] = 360,
+			[11] = 480,
+			[12] = 540,
+		};
+		if (rateinfo < ARRAY_SIZE(legacy_rates))
+			rate->legacy = legacy_rates[rateinfo];
+	}
 }
 
 /*
@@ -1414,6 +1436,10 @@ mwifiex_dump_station_info(struct mwifiex_private *priv,
 	/* bit rate is in 500 kb/s units. Convert it to 100kb/s units */
 	sinfo->txrate.legacy = rate * 5;
 
+	sinfo->filled |= BIT(NL80211_STA_INFO_RX_BITRATE);
+	mwifiex_parse_htinfo(priv, priv->rxpd_rate, priv->rxpd_htinfo,
+			     &sinfo->rxrate);
+
 	if (priv->bss_mode == NL80211_IFTYPE_STATION) {
 		sinfo->filled |= BIT_ULL(NL80211_STA_INFO_BSS_PARAM);
 		sinfo->bss_param.flags = 0;
diff --git a/drivers/net/wireless/marvell/mwifiex/sta_rx.c b/drivers/net/wireless/marvell/mwifiex/sta_rx.c
index 00fcbda09349..fb28a5c7f441 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_rx.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_rx.c
@@ -152,14 +152,17 @@ int mwifiex_process_rx_packet(struct mwifiex_private *priv,
 		mwifiex_process_tdls_action_frame(priv, offset, rx_pkt_len);
 	}
 
-	priv->rxpd_rate = local_rx_pd->rx_rate;
-
-	priv->rxpd_htinfo = local_rx_pd->ht_info;
+	/* Only stash RX bitrate for unicast packets. */
+	if (likely(!is_multicast_ether_addr(rx_pkt_hdr->eth803_hdr.h_dest))) {
+		priv->rxpd_rate = local_rx_pd->rx_rate;
+		priv->rxpd_htinfo = local_rx_pd->ht_info;
+	}
 
 	if (GET_BSS_ROLE(priv) == MWIFIEX_BSS_ROLE_STA ||
 	    GET_BSS_ROLE(priv) == MWIFIEX_BSS_ROLE_UAP) {
-		adj_rx_rate = mwifiex_adjust_data_rate(priv, priv->rxpd_rate,
-						       priv->rxpd_htinfo);
+		adj_rx_rate = mwifiex_adjust_data_rate(priv,
+						       local_rx_pd->rx_rate,
+						       local_rx_pd->ht_info);
 		mwifiex_hist_data_add(priv, adj_rx_rate, local_rx_pd->snr,
 				      local_rx_pd->nf);
 	}
-- 
2.20.0.rc2.403.gdbc3b29805-goog


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

* Re: [RFC PATCH v2 2/2] mwifiex: add NL80211_STA_INFO_RX_BITRATE support
  2018-12-07  0:12 ` [RFC PATCH v2 2/2] mwifiex: add NL80211_STA_INFO_RX_BITRATE support Brian Norris
@ 2018-12-07 10:50   ` Kalle Valo
  2018-12-07 11:55     ` [EXT] " Ganapathi Bhat
  0 siblings, 1 reply; 5+ messages in thread
From: Kalle Valo @ 2018-12-07 10:50 UTC (permalink / raw)
  To: Brian Norris
  Cc: linux-kernel, Amitkumar Karwar, Nishant Sarmukadam,
	Ganapathi Bhat, Xinming Hu, linux-wireless

Brian Norris <briannorris@chromium.org> writes:

> Comparing the existing TX_BITRATE parsing code (in
> mwifiex_parse_htinfo()) with the RX bitrate histograms in debugfs.c, it
> appears that the rxpd_rate and rxpd_htinfo fields have the same format.
> At least, they give reasonable results when I parse them this way.
>
> So this patch adds support for RX_BITRATE to our station info dump.
>
> Along the way, I add legacy bitrate parsing into the same function,
> using the debugfs code (mwifiex_histogram_read() and
> mwifiex_adjust_data_rate()) as reference.
>
> Additionally, to satisfy the requirements of
> NL80211_STA_INFO_RX_BITRATE, I skip logging the bitrate of multicast
> packets. This shouldn't add a lot of overhead to the RX path, as there
> are already several similar 802.3 header checks in this same codepath.
> We can also bias the branch behavior to favor unicast, as that's the
> common performance-sensitive case.
>
> I'd consider this support somewhat experimental, as I have zero
> documentation from Marvell. But the existing driver code gives me good
> reason to think this is correct.
>
> I've tested this on a few different 802.11{a,b,g,n,ac} networks, and the
> reported bitrates look good to me.
>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
> RFC: I'd appreciate it if someone from Marvell could double check my work
> here.

BTW, if we don't hear anything from Marvell I'm going to apply these
anyway.

-- 
Kalle Valo

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

* Re: [EXT] Re: [RFC PATCH v2 2/2] mwifiex: add NL80211_STA_INFO_RX_BITRATE support
  2018-12-07 10:50   ` Kalle Valo
@ 2018-12-07 11:55     ` " Ganapathi Bhat
  0 siblings, 0 replies; 5+ messages in thread
From: Ganapathi Bhat @ 2018-12-07 11:55 UTC (permalink / raw)
  To: Kalle Valo, Brian Norris
  Cc: linux-kernel, Amitkumar Karwar, Nishant Sarmukadam, Xinming Hu,
	linux-wireless

Hi Kalle/Brian,

>> RFC: I'd appreciate it if someone from Marvell could double check my work
>> here.

>BTW, if we don't hear anything from Marvell I'm going to apply these
>anyway.

This patch series looks good to us.

Thanks,
Ganapathi 

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

* Re: [RFC PATCH v2 1/2] mwifiex: refactor mwifiex_parse_htinfo() for reuse
  2018-12-07  0:12 [RFC PATCH v2 1/2] mwifiex: refactor mwifiex_parse_htinfo() for reuse Brian Norris
  2018-12-07  0:12 ` [RFC PATCH v2 2/2] mwifiex: add NL80211_STA_INFO_RX_BITRATE support Brian Norris
@ 2018-12-13 14:55 ` Kalle Valo
  1 sibling, 0 replies; 5+ messages in thread
From: Kalle Valo @ 2018-12-13 14:55 UTC (permalink / raw)
  To: Brian Norris
  Cc: linux-kernel, Amitkumar Karwar, Nishant Sarmukadam,
	Ganapathi Bhat, Xinming Hu, linux-wireless, Brian Norris

Brian Norris <briannorris@chromium.org> wrote:

> This function converts some firmware-specific parameters into cfg80211
> 'rate_info' structures. It currently assumes it's dealing only with TX
> bitrate, but the RX bitrate looks to be the same, so refactor this
> function to be reusable.
> 
> Signed-off-by: Brian Norris <briannorris@chromium.org>

2 patches applied to wireless-drivers-next.git, thanks.

a256707fbd4b mwifiex: refactor mwifiex_parse_htinfo() for reuse
ed0b2b067bad mwifiex: add NL80211_STA_INFO_RX_BITRATE support

-- 
https://patchwork.kernel.org/patch/10717275/

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


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

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-07  0:12 [RFC PATCH v2 1/2] mwifiex: refactor mwifiex_parse_htinfo() for reuse Brian Norris
2018-12-07  0:12 ` [RFC PATCH v2 2/2] mwifiex: add NL80211_STA_INFO_RX_BITRATE support Brian Norris
2018-12-07 10:50   ` Kalle Valo
2018-12-07 11:55     ` [EXT] " Ganapathi Bhat
2018-12-13 14:55 ` [RFC PATCH v2 1/2] mwifiex: refactor mwifiex_parse_htinfo() for reuse Kalle Valo

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org linux-kernel@archiver.kernel.org
	public-inbox-index lkml


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox