All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/7] rtl8xxxu: Pending patches
@ 2016-11-18 21:44 Jes.Sorensen
  2016-11-18 21:44 ` [PATCH 1/7] rtl8xxxu: Fix memory leak in handling rxdesc16 packets Jes.Sorensen
                   ` (7 more replies)
  0 siblings, 8 replies; 17+ messages in thread
From: Jes.Sorensen @ 2016-11-18 21:44 UTC (permalink / raw)
  To: linux-wireless; +Cc: kvalo, Larry.Finger, Jes Sorensen

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Kalle,

Please find attached a number of patches for the rtl8xxxu
driver.

The issues reported with wpa_supplicant on 8723bu still needs further
investigation.

Note the memory leak issue has only been seen with 8188eu devices so
far, but it's serious and needs to be plugged.

These are what I currently have in my pending queue. Apologies if I
already posted some of these, trying to get back in sync after
Plumbers.

Cheers,
Jes


Jes Sorensen (6):
  rtl8xxxu: Fix memory leak in handling rxdesc16 packets
  rtl8xxxu: Fix big-endian problem reporting mactime
  rtl8xxxu: Fix rtl8723bu driver reload issue
  rtl8xxxu: Fix rtl8192eu driver reload issue
  rtl8xxxu: Obtain RTS rates from mac80211
  rtl8xxxu: Pass tx_info to fill_txdesc in order to have access to retry
    count

Wei Yongjun (1):
  rtl8xxxu: Fix non static symbol warning

 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h   |  31 +++---
 .../net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c |  10 +-
 .../net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c |   4 +
 .../net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c  | 114 ++++++++++++++-------
 4 files changed, 104 insertions(+), 55 deletions(-)

-- 
2.7.4

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

* [PATCH 1/7] rtl8xxxu: Fix memory leak in handling rxdesc16 packets
  2016-11-18 21:44 [PATCH 0/7] rtl8xxxu: Pending patches Jes.Sorensen
@ 2016-11-18 21:44 ` Jes.Sorensen
  2016-11-25  9:51   ` [1/7] " Kalle Valo
  2016-11-18 21:44 ` [PATCH 2/7] rtl8xxxu: Fix big-endian problem reporting mactime Jes.Sorensen
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Jes.Sorensen @ 2016-11-18 21:44 UTC (permalink / raw)
  To: linux-wireless; +Cc: kvalo, Larry.Finger, Jes Sorensen

From: Jes Sorensen <Jes.Sorensen@redhat.com>

A device running without RX package aggregation could return more data
in the USB packet than the actual network packet. In this case the
could would clone the skb but then determine that that there was no
packet to handle and exit without freeing the cloned skb first.

This has so far only been observed with 8188eu devices, but could
affect others.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index b2d7f6e..a96ff17 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -5197,7 +5197,12 @@ int rtl8xxxu_parse_rxdesc16(struct rtl8xxxu_priv *priv, struct sk_buff *skb)
 		pkt_offset = roundup(pkt_len + drvinfo_sz + desc_shift +
 				     sizeof(struct rtl8xxxu_rxdesc16), 128);
 
-		if (pkt_cnt > 1)
+		/*
+		 * Only clone the skb if there's enough data at the end to
+		 * at least cover the rx descriptor
+		 */
+		if (pkt_cnt > 1 &&
+		    urb_len > (pkt_offset + sizeof(struct rtl8xxxu_rxdesc16)))
 			next_skb = skb_clone(skb, GFP_ATOMIC);
 
 		rx_status = IEEE80211_SKB_RXCB(skb);
-- 
2.7.4

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

* [PATCH 2/7] rtl8xxxu: Fix big-endian problem reporting mactime
  2016-11-18 21:44 [PATCH 0/7] rtl8xxxu: Pending patches Jes.Sorensen
  2016-11-18 21:44 ` [PATCH 1/7] rtl8xxxu: Fix memory leak in handling rxdesc16 packets Jes.Sorensen
@ 2016-11-18 21:44 ` Jes.Sorensen
  2016-11-18 21:44 ` [PATCH 3/7] rtl8xxxu: Fix rtl8723bu driver reload issue Jes.Sorensen
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Jes.Sorensen @ 2016-11-18 21:44 UTC (permalink / raw)
  To: linux-wireless; +Cc: kvalo, Larry.Finger, Jes Sorensen

From: Jes Sorensen <Jes.Sorensen@redhat.com>

The full RX descriptor is converted so converting tsfl again would
return it to it's original endian value.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h      | 4 ++--
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index 10166289..08d587a 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -238,7 +238,7 @@ struct rtl8xxxu_rxdesc16 {
 	u32 pattern1match:1;
 	u32 pattern0match:1;
 #endif
-	__le32 tsfl;
+	u32 tsfl;
 #if 0
 	u32 bassn:12;
 	u32 bavld:1;
@@ -368,7 +368,7 @@ struct rtl8xxxu_rxdesc24 {
 	u32 ldcp:1;
 	u32 splcp:1;
 #endif
-	__le32 tsfl;
+	u32 tsfl;
 };
 
 struct rtl8xxxu_txdesc32 {
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index a96ff17..a5e6ec2 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -5220,7 +5220,7 @@ int rtl8xxxu_parse_rxdesc16(struct rtl8xxxu_priv *priv, struct sk_buff *skb)
 			rtl8xxxu_rx_parse_phystats(priv, rx_status, phy_stats,
 						   rx_desc->rxmcs);
 
-		rx_status->mactime = le32_to_cpu(rx_desc->tsfl);
+		rx_status->mactime = rx_desc->tsfl;
 		rx_status->flag |= RX_FLAG_MACTIME_START;
 
 		if (!rx_desc->swdec)
@@ -5290,7 +5290,7 @@ int rtl8xxxu_parse_rxdesc24(struct rtl8xxxu_priv *priv, struct sk_buff *skb)
 		rtl8xxxu_rx_parse_phystats(priv, rx_status, phy_stats,
 					   rx_desc->rxmcs);
 
-	rx_status->mactime = le32_to_cpu(rx_desc->tsfl);
+	rx_status->mactime = rx_desc->tsfl;
 	rx_status->flag |= RX_FLAG_MACTIME_START;
 
 	if (!rx_desc->swdec)
-- 
2.7.4

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

* [PATCH 3/7] rtl8xxxu: Fix rtl8723bu driver reload issue
  2016-11-18 21:44 [PATCH 0/7] rtl8xxxu: Pending patches Jes.Sorensen
  2016-11-18 21:44 ` [PATCH 1/7] rtl8xxxu: Fix memory leak in handling rxdesc16 packets Jes.Sorensen
  2016-11-18 21:44 ` [PATCH 2/7] rtl8xxxu: Fix big-endian problem reporting mactime Jes.Sorensen
@ 2016-11-18 21:44 ` Jes.Sorensen
  2016-11-18 21:44 ` [PATCH 4/7] rtl8xxxu: Fix rtl8192eu " Jes.Sorensen
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Jes.Sorensen @ 2016-11-18 21:44 UTC (permalink / raw)
  To: linux-wireless; +Cc: kvalo, Larry.Finger, Jes Sorensen

From: Jes Sorensen <Jes.Sorensen@redhat.com>

The generic disable_rf() function clears bits 22 and 23 in
REG_RX_WAIT_CCA, however we did not re-enable them again in
rtl8723b_enable_rf()

This resolves the problem for me with 8723bu devices not working again
after reloading the driver.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
index 6c086b5..02b8ddd 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
@@ -1498,6 +1498,10 @@ static void rtl8723b_enable_rf(struct rtl8xxxu_priv *priv)
 	u32 val32;
 	u8 val8;
 
+	val32 = rtl8xxxu_read32(priv, REG_RX_WAIT_CCA);
+	val32 |= (BIT(22) | BIT(23));
+	rtl8xxxu_write32(priv, REG_RX_WAIT_CCA, val32);
+
 	/*
 	 * No indication anywhere as to what 0x0790 does. The 2 antenna
 	 * vendor code preserves bits 6-7 here.
-- 
2.7.4

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

* [PATCH 4/7] rtl8xxxu: Fix rtl8192eu driver reload issue
  2016-11-18 21:44 [PATCH 0/7] rtl8xxxu: Pending patches Jes.Sorensen
                   ` (2 preceding siblings ...)
  2016-11-18 21:44 ` [PATCH 3/7] rtl8xxxu: Fix rtl8723bu driver reload issue Jes.Sorensen
@ 2016-11-18 21:44 ` Jes.Sorensen
  2016-11-18 21:44 ` [PATCH 5/7] rtl8xxxu: Obtain RTS rates from mac80211 Jes.Sorensen
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Jes.Sorensen @ 2016-11-18 21:44 UTC (permalink / raw)
  To: linux-wireless; +Cc: kvalo, Larry.Finger, Jes Sorensen

From: Jes Sorensen <Jes.Sorensen@redhat.com>

The 8192eu suffered from two issues when reloading the driver.

The same problems as with the 8723bu where REG_RX_WAIT_CCA bits 22 and
23 didn't get set in rtl8192e_enable_rf().

In addition it also seems prone to issues when setting REG_RF_CTRL to
0 intead of just disabling the RF_ENABLE bit. Similar to what was
causing issues with the 8188eu.

With this patch I can successfully reload the driver and reassociate
to an APi with an 8192eu dongle.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
index df54d27..a793fed 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
@@ -1461,7 +1461,9 @@ static int rtl8192eu_active_to_emu(struct rtl8xxxu_priv *priv)
 	int count, ret = 0;
 
 	/* Turn off RF */
-	rtl8xxxu_write8(priv, REG_RF_CTRL, 0);
+	val8 = rtl8xxxu_read8(priv, REG_RF_CTRL);
+	val8 &= ~RF_ENABLE;
+	rtl8xxxu_write8(priv, REG_RF_CTRL, val8);
 
 	/* Switch DPDT_SEL_P output from register 0x65[2] */
 	val8 = rtl8xxxu_read8(priv, REG_LEDCFG2);
@@ -1593,6 +1595,10 @@ static void rtl8192e_enable_rf(struct rtl8xxxu_priv *priv)
 	u32 val32;
 	u8 val8;
 
+	val32 = rtl8xxxu_read32(priv, REG_RX_WAIT_CCA);
+	val32 |= (BIT(22) | BIT(23));
+	rtl8xxxu_write32(priv, REG_RX_WAIT_CCA, val32);
+
 	val8 = rtl8xxxu_read8(priv, REG_GPIO_MUXCFG);
 	val8 |= BIT(5);
 	rtl8xxxu_write8(priv, REG_GPIO_MUXCFG, val8);
-- 
2.7.4

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

* [PATCH 5/7] rtl8xxxu: Obtain RTS rates from mac80211
  2016-11-18 21:44 [PATCH 0/7] rtl8xxxu: Pending patches Jes.Sorensen
                   ` (3 preceding siblings ...)
  2016-11-18 21:44 ` [PATCH 4/7] rtl8xxxu: Fix rtl8192eu " Jes.Sorensen
@ 2016-11-18 21:44 ` Jes.Sorensen
  2016-11-18 21:44 ` [PATCH 6/7] rtl8xxxu: Pass tx_info to fill_txdesc in order to have access to retry count Jes.Sorensen
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Jes.Sorensen @ 2016-11-18 21:44 UTC (permalink / raw)
  To: linux-wireless; +Cc: kvalo, Larry.Finger, Jes Sorensen

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Use the mac80211 provided rate for RTS rather than the hard coded
24Mbps as suggested by the vendor drivers.

Reported-by: Andrea Merello <andrea.merello@gmail.com>
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h   |  6 +--
 .../net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c  | 46 ++++++++++++++--------
 2 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index 08d587a..bc3c990 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -1340,7 +1340,7 @@ struct rtl8xxxu_fileops {
 	void (*fill_txdesc) (struct ieee80211_hdr *hdr,
 			     struct rtl8xxxu_txdesc32 *tx_desc, u32 rate,
 			     u16 rate_flag, bool sgi, bool short_preamble,
-			     bool ampdu_enable);
+			     bool ampdu_enable, u32 rts_rate);
 	int writeN_block_size;
 	int rx_agg_buf_size;
 	char tx_desc_size;
@@ -1437,11 +1437,11 @@ bool rtl8xxxu_gen2_simularity_compare(struct rtl8xxxu_priv *priv,
 void rtl8xxxu_fill_txdesc_v1(struct ieee80211_hdr *hdr,
 			     struct rtl8xxxu_txdesc32 *tx_desc, u32 rate,
 			     u16 rate_flag, bool sgi, bool short_preamble,
-			     bool ampdu_enable);
+			     bool ampdu_enable, u32 rts_rate);
 void rtl8xxxu_fill_txdesc_v2(struct ieee80211_hdr *hdr,
 			     struct rtl8xxxu_txdesc32 *tx_desc32, u32 rate,
 			     u16 rate_flag, bool sgi, bool short_preamble,
-			     bool ampdu_enable);
+			     bool ampdu_enable, u32 rts_rate);
 
 extern struct rtl8xxxu_fileops rtl8192cu_fops;
 extern struct rtl8xxxu_fileops rtl8192eu_fops;
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index a5e6ec2..a4ac557 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -4762,7 +4762,7 @@ void
 rtl8xxxu_fill_txdesc_v1(struct ieee80211_hdr *hdr,
 			struct rtl8xxxu_txdesc32 *tx_desc, u32 rate,
 			u16 rate_flag, bool sgi, bool short_preamble,
-			bool ampdu_enable)
+			bool ampdu_enable, u32 rts_rate)
 {
 	u16 seq_number;
 
@@ -4796,15 +4796,16 @@ rtl8xxxu_fill_txdesc_v1(struct ieee80211_hdr *hdr,
 	if (sgi)
 		tx_desc->txdw5 |= cpu_to_le32(TXDESC32_SHORT_GI);
 
+	/*
+	 * rts_rate is zero if RTS/CTS or CTS to SELF are not enabled
+	 */
+	tx_desc->txdw4 |= cpu_to_le32(rts_rate << TXDESC32_RTS_RATE_SHIFT);
 	if (rate_flag & IEEE80211_TX_RC_USE_RTS_CTS) {
-		/*
-		 * Use RTS rate 24M - does the mac80211 tell
-		 * us which to use?
-		 */
-		tx_desc->txdw4 |= cpu_to_le32(DESC_RATE_24M <<
-					      TXDESC32_RTS_RATE_SHIFT);
 		tx_desc->txdw4 |= cpu_to_le32(TXDESC32_RTS_CTS_ENABLE);
 		tx_desc->txdw4 |= cpu_to_le32(TXDESC32_HW_RTS_ENABLE);
+	} else if (rate_flag & IEEE80211_TX_RC_USE_CTS_PROTECT) {
+		tx_desc->txdw4 |= cpu_to_le32(TXDESC32_CTS_SELF_ENABLE);
+		tx_desc->txdw4 |= cpu_to_le32(TXDESC32_HW_RTS_ENABLE);
 	}
 }
 
@@ -4816,7 +4817,7 @@ void
 rtl8xxxu_fill_txdesc_v2(struct ieee80211_hdr *hdr,
 			struct rtl8xxxu_txdesc32 *tx_desc32, u32 rate,
 			u16 rate_flag, bool sgi, bool short_preamble,
-			bool ampdu_enable)
+			bool ampdu_enable, u32 rts_rate)
 {
 	struct rtl8xxxu_txdesc40 *tx_desc40;
 	u16 seq_number;
@@ -4849,15 +4850,19 @@ rtl8xxxu_fill_txdesc_v2(struct ieee80211_hdr *hdr,
 	if (short_preamble)
 		tx_desc40->txdw5 |= cpu_to_le32(TXDESC40_SHORT_PREAMBLE);
 
+	tx_desc40->txdw4 |= cpu_to_le32(rts_rate << TXDESC40_RTS_RATE_SHIFT);
+	/*
+	 * rts_rate is zero if RTS/CTS or CTS to SELF are not enabled
+	 */
 	if (rate_flag & IEEE80211_TX_RC_USE_RTS_CTS) {
-		/*
-		 * Use RTS rate 24M - does the mac80211 tell
-		 * us which to use?
-		 */
-		tx_desc40->txdw4 |= cpu_to_le32(DESC_RATE_24M <<
-						TXDESC40_RTS_RATE_SHIFT);
 		tx_desc40->txdw3 |= cpu_to_le32(TXDESC40_RTS_CTS_ENABLE);
 		tx_desc40->txdw3 |= cpu_to_le32(TXDESC40_HW_RTS_ENABLE);
+	} else if (rate_flag & IEEE80211_TX_RC_USE_CTS_PROTECT) {
+		/*
+		 * For some reason the vendor driver doesn't set
+		 * TXDESC40_HW_RTS_ENABLE for CTS to SELF
+		 */
+		tx_desc40->txdw3 |= cpu_to_le32(TXDESC40_CTS_SELF_ENABLE);
 	}
 }
 
@@ -4874,7 +4879,7 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
 	struct ieee80211_sta *sta = NULL;
 	struct ieee80211_vif *vif = tx_info->control.vif;
 	struct device *dev = &priv->udev->dev;
-	u32 queue, rate;
+	u32 queue, rate, rts_rate;
 	u16 pktlen = skb->len;
 	u16 seq_number;
 	u16 rate_flag = tx_info->control.rates[0].flags;
@@ -4974,10 +4979,17 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
 	    (sta && vif && vif->bss_conf.use_short_preamble))
 		short_preamble = true;
 
+	if (rate_flag & IEEE80211_TX_RC_USE_RTS_CTS)
+		rts_rate = ieee80211_get_rts_cts_rate(hw, tx_info)->hw_value;
+	else if (rate_flag & IEEE80211_TX_RC_USE_CTS_PROTECT)
+		rts_rate = ieee80211_get_rts_cts_rate(hw, tx_info)->hw_value;
+	else
+		rts_rate = 0;
+
 	seq_number = IEEE80211_SEQ_TO_SN(le16_to_cpu(hdr->seq_ctrl));
 
-	priv->fops->fill_txdesc(hdr, tx_desc, rate, rate_flag,
-				sgi, short_preamble, ampdu_enable);
+	priv->fops->fill_txdesc(hdr, tx_desc, rate, rate_flag, sgi,
+				short_preamble, ampdu_enable, rts_rate);
 
 	rtl8xxxu_calc_tx_desc_csum(tx_desc);
 
-- 
2.7.4

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

* [PATCH 6/7] rtl8xxxu: Pass tx_info to fill_txdesc in order to have access to retry count
  2016-11-18 21:44 [PATCH 0/7] rtl8xxxu: Pending patches Jes.Sorensen
                   ` (4 preceding siblings ...)
  2016-11-18 21:44 ` [PATCH 5/7] rtl8xxxu: Obtain RTS rates from mac80211 Jes.Sorensen
@ 2016-11-18 21:44 ` Jes.Sorensen
  2016-11-18 21:44 ` [PATCH 7/7] rtl8xxxu: Fix non static symbol warning Jes.Sorensen
  2016-11-19  1:46 ` [PATCH 0/7] rtl8xxxu: Pending patches Barry Day
  7 siblings, 0 replies; 17+ messages in thread
From: Jes.Sorensen @ 2016-11-18 21:44 UTC (permalink / raw)
  To: linux-wireless; +Cc: kvalo, Larry.Finger, Jes Sorensen

From: Jes Sorensen <Jes.Sorensen@redhat.com>

In order to obtain retry count for a given rate we need to pass the
full struct ieee80211_tx_info to the function setting the rate in he
TX descriptor.

This uncovered a huge bug where the old code would use struct
ieee80211_rate.flags to test for rate parameters, which is always
zero, instead of the flags value from struct ieee80211_tx_rate.

Time to find a brown paper bag :(

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h   | 27 ++++----
 .../net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c  | 71 ++++++++++++++--------
 2 files changed, 60 insertions(+), 38 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index bc3c990..df551b2 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -1337,10 +1337,11 @@ struct rtl8xxxu_fileops {
 				  u32 ramask, int sgi);
 	void (*report_connect) (struct rtl8xxxu_priv *priv,
 				u8 macid, bool connect);
-	void (*fill_txdesc) (struct ieee80211_hdr *hdr,
-			     struct rtl8xxxu_txdesc32 *tx_desc, u32 rate,
-			     u16 rate_flag, bool sgi, bool short_preamble,
-			     bool ampdu_enable, u32 rts_rate);
+	void (*fill_txdesc) (struct ieee80211_hw *hw, struct ieee80211_hdr *hdr,
+			     struct ieee80211_tx_info *tx_info,
+			     struct rtl8xxxu_txdesc32 *tx_desc, bool sgi,
+			     bool short_preamble, bool ampdu_enable,
+			     u32 rts_rate);
 	int writeN_block_size;
 	int rx_agg_buf_size;
 	char tx_desc_size;
@@ -1434,14 +1435,16 @@ int rtl8xxxu_parse_rxdesc24(struct rtl8xxxu_priv *priv, struct sk_buff *skb);
 int rtl8xxxu_gen2_channel_to_group(int channel);
 bool rtl8xxxu_gen2_simularity_compare(struct rtl8xxxu_priv *priv,
 				      int result[][8], int c1, int c2);
-void rtl8xxxu_fill_txdesc_v1(struct ieee80211_hdr *hdr,
-			     struct rtl8xxxu_txdesc32 *tx_desc, u32 rate,
-			     u16 rate_flag, bool sgi, bool short_preamble,
-			     bool ampdu_enable, u32 rts_rate);
-void rtl8xxxu_fill_txdesc_v2(struct ieee80211_hdr *hdr,
-			     struct rtl8xxxu_txdesc32 *tx_desc32, u32 rate,
-			     u16 rate_flag, bool sgi, bool short_preamble,
-			     bool ampdu_enable, u32 rts_rate);
+void rtl8xxxu_fill_txdesc_v1(struct ieee80211_hw *hw, struct ieee80211_hdr *hdr,
+			     struct ieee80211_tx_info *tx_info,
+			     struct rtl8xxxu_txdesc32 *tx_desc, bool sgi,
+			     bool short_preamble, bool ampdu_enable,
+			     u32 rts_rate);
+void rtl8xxxu_fill_txdesc_v2(struct ieee80211_hw *hw, struct ieee80211_hdr *hdr,
+			     struct ieee80211_tx_info *tx_info,
+			     struct rtl8xxxu_txdesc32 *tx_desc32, bool sgi,
+			     bool short_preamble, bool ampdu_enable,
+			     u32 rts_rate);
 
 extern struct rtl8xxxu_fileops rtl8192cu_fops;
 extern struct rtl8xxxu_fileops rtl8192eu_fops;
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index a4ac557..04141e5 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -4759,13 +4759,28 @@ static void rtl8xxxu_dump_action(struct device *dev,
  * This format is used on 8188cu/8192cu/8723au
  */
 void
-rtl8xxxu_fill_txdesc_v1(struct ieee80211_hdr *hdr,
-			struct rtl8xxxu_txdesc32 *tx_desc, u32 rate,
-			u16 rate_flag, bool sgi, bool short_preamble,
-			bool ampdu_enable, u32 rts_rate)
+rtl8xxxu_fill_txdesc_v1(struct ieee80211_hw *hw, struct ieee80211_hdr *hdr,
+			struct ieee80211_tx_info *tx_info,
+			struct rtl8xxxu_txdesc32 *tx_desc, bool sgi,
+			bool short_preamble, bool ampdu_enable, u32 rts_rate)
 {
+	struct ieee80211_rate *tx_rate = ieee80211_get_tx_rate(hw, tx_info);
+	struct rtl8xxxu_priv *priv = hw->priv;
+	struct device *dev = &priv->udev->dev;
+	u32 rate;
+	u16 rate_flags = tx_info->control.rates[0].flags;
 	u16 seq_number;
 
+	if (rate_flags & IEEE80211_TX_RC_MCS &&
+	    !ieee80211_is_mgmt(hdr->frame_control))
+		rate = tx_info->control.rates[0].idx + DESC_RATE_MCS0;
+	else
+		rate = tx_rate->hw_value;
+
+	if (rtl8xxxu_debug & RTL8XXXU_DEBUG_TX)
+		dev_info(dev, "%s: TX rate: %d, pkt size %d\n",
+			 __func__, rate, cpu_to_le16(tx_desc->pkt_size));
+
 	seq_number = IEEE80211_SEQ_TO_SN(le16_to_cpu(hdr->seq_ctrl));
 
 	tx_desc->txdw5 = cpu_to_le32(rate);
@@ -4800,10 +4815,10 @@ rtl8xxxu_fill_txdesc_v1(struct ieee80211_hdr *hdr,
 	 * rts_rate is zero if RTS/CTS or CTS to SELF are not enabled
 	 */
 	tx_desc->txdw4 |= cpu_to_le32(rts_rate << TXDESC32_RTS_RATE_SHIFT);
-	if (rate_flag & IEEE80211_TX_RC_USE_RTS_CTS) {
+	if (rate_flags & IEEE80211_TX_RC_USE_RTS_CTS) {
 		tx_desc->txdw4 |= cpu_to_le32(TXDESC32_RTS_CTS_ENABLE);
 		tx_desc->txdw4 |= cpu_to_le32(TXDESC32_HW_RTS_ENABLE);
-	} else if (rate_flag & IEEE80211_TX_RC_USE_CTS_PROTECT) {
+	} else if (rate_flags & IEEE80211_TX_RC_USE_CTS_PROTECT) {
 		tx_desc->txdw4 |= cpu_to_le32(TXDESC32_CTS_SELF_ENABLE);
 		tx_desc->txdw4 |= cpu_to_le32(TXDESC32_HW_RTS_ENABLE);
 	}
@@ -4814,16 +4829,31 @@ rtl8xxxu_fill_txdesc_v1(struct ieee80211_hdr *hdr,
  * This format is used on 8192eu/8723bu
  */
 void
-rtl8xxxu_fill_txdesc_v2(struct ieee80211_hdr *hdr,
-			struct rtl8xxxu_txdesc32 *tx_desc32, u32 rate,
-			u16 rate_flag, bool sgi, bool short_preamble,
-			bool ampdu_enable, u32 rts_rate)
+rtl8xxxu_fill_txdesc_v2(struct ieee80211_hw *hw, struct ieee80211_hdr *hdr,
+			struct ieee80211_tx_info *tx_info,
+			struct rtl8xxxu_txdesc32 *tx_desc32, bool sgi,
+			bool short_preamble, bool ampdu_enable, u32 rts_rate)
 {
+	struct ieee80211_rate *tx_rate = ieee80211_get_tx_rate(hw, tx_info);
+	struct rtl8xxxu_priv *priv = hw->priv;
+	struct device *dev = &priv->udev->dev;
 	struct rtl8xxxu_txdesc40 *tx_desc40;
+	u32 rate;
+	u16 rate_flags = tx_info->control.rates[0].flags;
 	u16 seq_number;
 
 	tx_desc40 = (struct rtl8xxxu_txdesc40 *)tx_desc32;
 
+	if (rtl8xxxu_debug & RTL8XXXU_DEBUG_TX)
+		dev_info(dev, "%s: TX rate: %d, pkt size %d\n",
+			 __func__, rate, cpu_to_le16(tx_desc40->pkt_size));
+
+	if (rate_flags & IEEE80211_TX_RC_MCS &&
+	    !ieee80211_is_mgmt(hdr->frame_control))
+		rate = tx_info->control.rates[0].idx + DESC_RATE_MCS0;
+	else
+		rate = tx_rate->hw_value;
+
 	seq_number = IEEE80211_SEQ_TO_SN(le16_to_cpu(hdr->seq_ctrl));
 
 	tx_desc40->txdw4 = cpu_to_le32(rate);
@@ -4854,10 +4884,10 @@ rtl8xxxu_fill_txdesc_v2(struct ieee80211_hdr *hdr,
 	/*
 	 * rts_rate is zero if RTS/CTS or CTS to SELF are not enabled
 	 */
-	if (rate_flag & IEEE80211_TX_RC_USE_RTS_CTS) {
+	if (rate_flags & IEEE80211_TX_RC_USE_RTS_CTS) {
 		tx_desc40->txdw3 |= cpu_to_le32(TXDESC40_RTS_CTS_ENABLE);
 		tx_desc40->txdw3 |= cpu_to_le32(TXDESC40_HW_RTS_ENABLE);
-	} else if (rate_flag & IEEE80211_TX_RC_USE_CTS_PROTECT) {
+	} else if (rate_flags & IEEE80211_TX_RC_USE_CTS_PROTECT) {
 		/*
 		 * For some reason the vendor driver doesn't set
 		 * TXDESC40_HW_RTS_ENABLE for CTS to SELF
@@ -4872,14 +4902,13 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
 {
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
 	struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(skb);
-	struct ieee80211_rate *tx_rate = ieee80211_get_tx_rate(hw, tx_info);
 	struct rtl8xxxu_priv *priv = hw->priv;
 	struct rtl8xxxu_txdesc32 *tx_desc;
 	struct rtl8xxxu_tx_urb *tx_urb;
 	struct ieee80211_sta *sta = NULL;
 	struct ieee80211_vif *vif = tx_info->control.vif;
 	struct device *dev = &priv->udev->dev;
-	u32 queue, rate, rts_rate;
+	u32 queue, rts_rate;
 	u16 pktlen = skb->len;
 	u16 seq_number;
 	u16 rate_flag = tx_info->control.rates[0].flags;
@@ -4906,10 +4935,6 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
 		goto error;
 	}
 
-	if (rtl8xxxu_debug & RTL8XXXU_DEBUG_TX)
-		dev_info(dev, "%s: TX rate: %d (%d), pkt size %d\n",
-			 __func__, tx_rate->bitrate, tx_rate->hw_value, pktlen);
-
 	if (ieee80211_is_action(hdr->frame_control))
 		rtl8xxxu_dump_action(dev, hdr);
 
@@ -4963,12 +4988,6 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
 		}
 	}
 
-	if (rate_flag & IEEE80211_TX_RC_MCS &&
-	    !ieee80211_is_mgmt(hdr->frame_control))
-		rate = tx_info->control.rates[0].idx + DESC_RATE_MCS0;
-	else
-		rate = tx_rate->hw_value;
-
 	if (rate_flag & IEEE80211_TX_RC_SHORT_GI ||
 	    (ieee80211_is_data_qos(hdr->frame_control) &&
 	     sta && sta->ht_cap.cap &
@@ -4988,8 +5007,8 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
 
 	seq_number = IEEE80211_SEQ_TO_SN(le16_to_cpu(hdr->seq_ctrl));
 
-	priv->fops->fill_txdesc(hdr, tx_desc, rate, rate_flag, sgi,
-				short_preamble, ampdu_enable, rts_rate);
+	priv->fops->fill_txdesc(hw, hdr, tx_info, tx_desc, sgi, short_preamble,
+				ampdu_enable, rts_rate);
 
 	rtl8xxxu_calc_tx_desc_csum(tx_desc);
 
-- 
2.7.4

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

* [PATCH 7/7] rtl8xxxu: Fix non static symbol warning
  2016-11-18 21:44 [PATCH 0/7] rtl8xxxu: Pending patches Jes.Sorensen
                   ` (5 preceding siblings ...)
  2016-11-18 21:44 ` [PATCH 6/7] rtl8xxxu: Pass tx_info to fill_txdesc in order to have access to retry count Jes.Sorensen
@ 2016-11-18 21:44 ` Jes.Sorensen
  2016-11-19  1:46 ` [PATCH 0/7] rtl8xxxu: Pending patches Barry Day
  7 siblings, 0 replies; 17+ messages in thread
From: Jes.Sorensen @ 2016-11-18 21:44 UTC (permalink / raw)
  To: linux-wireless; +Cc: kvalo, Larry.Finger, Wei Yongjun, Jes Sorensen

From: Wei Yongjun <weiyongjun1@huawei.com>

Fixes the following sparse warning:

drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c:1559:6: warning:
 symbol 'rtl8192eu_power_off' was not declared. Should it be static?

Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
index a793fed..a1178c5 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
@@ -1556,7 +1556,7 @@ static int rtl8192eu_power_on(struct rtl8xxxu_priv *priv)
 	return ret;
 }
 
-void rtl8192eu_power_off(struct rtl8xxxu_priv *priv)
+static void rtl8192eu_power_off(struct rtl8xxxu_priv *priv)
 {
 	u8 val8;
 	u16 val16;
-- 
2.7.4

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

* Re: [PATCH 0/7] rtl8xxxu: Pending patches
  2016-11-18 21:44 [PATCH 0/7] rtl8xxxu: Pending patches Jes.Sorensen
                   ` (6 preceding siblings ...)
  2016-11-18 21:44 ` [PATCH 7/7] rtl8xxxu: Fix non static symbol warning Jes.Sorensen
@ 2016-11-19  1:46 ` Barry Day
  2016-11-19  2:00   ` Jes Sorensen
  7 siblings, 1 reply; 17+ messages in thread
From: Barry Day @ 2016-11-19  1:46 UTC (permalink / raw)
  To: Jes.Sorensen; +Cc: linux-wireless, kvalo, Larry.Finger

On Fri, Nov 18, 2016 at 04:44:21PM -0500, Jes.Sorensen@redhat.com wrote:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
> 
> Kalle,
> 
> Please find attached a number of patches for the rtl8xxxu
> driver.
> 
> The issues reported with wpa_supplicant on 8723bu still needs further
> investigation.
> 

The patch I posted that you want tested more will also fix the wpa_supplicant
issue.
Currently I'm looking at why the tx rate is not what it should be. I feel
fixing that first will be beneficial for fixing any other issues.

The recent merge has made my local branch of rtl8xxxu-devel 14 commits ahead.
Do I need to do a reset and submit a new patch for the DWA-131 dongle?

Barry

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

* Re: [PATCH 0/7] rtl8xxxu: Pending patches
  2016-11-19  1:46 ` [PATCH 0/7] rtl8xxxu: Pending patches Barry Day
@ 2016-11-19  2:00   ` Jes Sorensen
  2016-11-19  2:38     ` Barry Day
  0 siblings, 1 reply; 17+ messages in thread
From: Jes Sorensen @ 2016-11-19  2:00 UTC (permalink / raw)
  To: Barry Day; +Cc: linux-wireless, kvalo, Larry.Finger

Barry Day <briselec@gmail.com> writes:
> On Fri, Nov 18, 2016 at 04:44:21PM -0500, Jes.Sorensen@redhat.com wrote:
>> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>> 
>> Kalle,
>> 
>> Please find attached a number of patches for the rtl8xxxu
>> driver.
>> 
>> The issues reported with wpa_supplicant on 8723bu still needs further
>> investigation.
>> 
>
> The patch I posted that you want tested more will also fix the
> wpa_supplicant issue.  Currently I'm looking at why the tx rate is not
> what it should be. I feel fixing that first will be beneficial for
> fixing any other issues.

Interesting, I was thinking that might be the case. I do want to dig
into this further to understand it better. If we use your solution I
will want to make sure we cover both gen1 and gen2 parts.

> The recent merge has made my local branch of rtl8xxxu-devel 14 commits ahead.
> Do I need to do a reset and submit a new patch for the DWA-131 dongle?

In general you need to use 'git pull --rebase' on my tree. I rebase it
to stay in sync with Kalle's tree.

The DWA-131 is the 8192eu? Sorry a bit behind and my mind is losing
bits. If it's the patch you posted earlier I can dig it out and play
with it - I am still catching up though, so please be patient.

Cheers,
Jes

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

* Re: [PATCH 0/7] rtl8xxxu: Pending patches
  2016-11-19  2:00   ` Jes Sorensen
@ 2016-11-19  2:38     ` Barry Day
  2016-11-19 23:53       ` Jes Sorensen
  0 siblings, 1 reply; 17+ messages in thread
From: Barry Day @ 2016-11-19  2:38 UTC (permalink / raw)
  To: Jes Sorensen; +Cc: linux-wireless, kvalo, Larry.Finger

On Fri, Nov 18, 2016 at 09:00:10PM -0500, Jes Sorensen wrote:
> Barry Day <briselec@gmail.com> writes:
> > On Fri, Nov 18, 2016 at 04:44:21PM -0500, Jes.Sorensen@redhat.com wrote:
> >> From: Jes Sorensen <Jes.Sorensen@redhat.com>
> >> 
> >> Kalle,
> >> 
> >> Please find attached a number of patches for the rtl8xxxu
> >> driver.
> >> 
> >> The issues reported with wpa_supplicant on 8723bu still needs further
> >> investigation.
> >> 
> >
> > The patch I posted that you want tested more will also fix the
> > wpa_supplicant issue.  Currently I'm looking at why the tx rate is not
> > what it should be. I feel fixing that first will be beneficial for
> > fixing any other issues.
> 
> Interesting, I was thinking that might be the case. I do want to dig
> into this further to understand it better. If we use your solution I
> will want to make sure we cover both gen1 and gen2 parts.
> 
> > The recent merge has made my local branch of rtl8xxxu-devel 14 commits ahead.
> > Do I need to do a reset and submit a new patch for the DWA-131 dongle?
> 
> In general you need to use 'git pull --rebase' on my tree. I rebase it
> to stay in sync with Kalle's tree.
> 
> The DWA-131 is the 8192eu? Sorry a bit behind and my mind is losing
> bits. If it's the patch you posted earlier I can dig it out and play
> with it - I am still catching up though, so please be patient.
> 
> Cheers,
> Jes
> 
> 

yes it's an 8192eu.

Would you accept a patch that adds a struct device pointer to rtl8xxxu_priv
and used as the device pointer in the logging functions? Then all the messages
will start with the driver name making them easier to find.

Barry

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

* Re: [PATCH 0/7] rtl8xxxu: Pending patches
  2016-11-19  2:38     ` Barry Day
@ 2016-11-19 23:53       ` Jes Sorensen
  2016-11-20  2:42         ` Barry Day
  0 siblings, 1 reply; 17+ messages in thread
From: Jes Sorensen @ 2016-11-19 23:53 UTC (permalink / raw)
  To: Barry Day; +Cc: linux-wireless, kvalo, Larry.Finger

Barry Day <briselec@gmail.com> writes:
> On Fri, Nov 18, 2016 at 09:00:10PM -0500, Jes Sorensen wrote:
>> Barry Day <briselec@gmail.com> writes:
>> > On Fri, Nov 18, 2016 at 04:44:21PM -0500, Jes.Sorensen@redhat.com wrote:
>> >> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>> >> 
>> >> Kalle,
>> >> 
>> >> Please find attached a number of patches for the rtl8xxxu
>> >> driver.
>> >> 
>> >> The issues reported with wpa_supplicant on 8723bu still needs further
>> >> investigation.
>> >> 
>> >
>> > The patch I posted that you want tested more will also fix the
>> > wpa_supplicant issue.  Currently I'm looking at why the tx rate is not
>> > what it should be. I feel fixing that first will be beneficial for
>> > fixing any other issues.
>> 
>> Interesting, I was thinking that might be the case. I do want to dig
>> into this further to understand it better. If we use your solution I
>> will want to make sure we cover both gen1 and gen2 parts.
>> 
>> > The recent merge has made my local branch of rtl8xxxu-devel 14
>> > commits ahead.
>> > Do I need to do a reset and submit a new patch for the DWA-131 dongle?
>> 
>> In general you need to use 'git pull --rebase' on my tree. I rebase it
>> to stay in sync with Kalle's tree.
>> 
>> The DWA-131 is the 8192eu? Sorry a bit behind and my mind is losing
>> bits. If it's the patch you posted earlier I can dig it out and play
>> with it - I am still catching up though, so please be patient.
>
> yes it's an 8192eu.

Gotcha - how do you do your testing to reproduce the problem btw? Most
of my testing is using the 8723au as a primary device and the next
device as a follow-on, so I may not see as long a run with the device
active as you see.

> Would you accept a patch that adds a struct device pointer to rtl8xxxu_priv
> and used as the device pointer in the logging functions? Then all the messages
> will start with the driver name making them easier to find.

How do you mean? Right now I have a struct usb_device pointer and
dereference that for ->dev to use with dev_info() messages etc. Do you
want to see more than that?

Cheers,
Jes

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

* Re: [PATCH 0/7] rtl8xxxu: Pending patches
  2016-11-19 23:53       ` Jes Sorensen
@ 2016-11-20  2:42         ` Barry Day
  2016-11-21 16:59           ` Jes Sorensen
  0 siblings, 1 reply; 17+ messages in thread
From: Barry Day @ 2016-11-20  2:42 UTC (permalink / raw)
  To: Jes Sorensen; +Cc: linux-wireless, kvalo, Larry.Finger

On Sat, Nov 19, 2016 at 06:53:42PM -0500, Jes Sorensen wrote:
> Barry Day <briselec@gmail.com> writes:
> > On Fri, Nov 18, 2016 at 09:00:10PM -0500, Jes Sorensen wrote:
> >> Barry Day <briselec@gmail.com> writes:
> >> > On Fri, Nov 18, 2016 at 04:44:21PM -0500, Jes.Sorensen@redhat.com wrote:
> >> >> From: Jes Sorensen <Jes.Sorensen@redhat.com>
> >> >> 
> >> >> Kalle,
> >> >> 
> >> >> Please find attached a number of patches for the rtl8xxxu
> >> >> driver.
> >> >> 
> >> >> The issues reported with wpa_supplicant on 8723bu still needs further
> >> >> investigation.
> >> >> 
> >> >
> >> > The patch I posted that you want tested more will also fix the
> >> > wpa_supplicant issue.  Currently I'm looking at why the tx rate is not
> >> > what it should be. I feel fixing that first will be beneficial for
> >> > fixing any other issues.
> >> 
> >> Interesting, I was thinking that might be the case. I do want to dig
> >> into this further to understand it better. If we use your solution I
> >> will want to make sure we cover both gen1 and gen2 parts.
> >> 
> >> > The recent merge has made my local branch of rtl8xxxu-devel 14
> >> > commits ahead.
> >> > Do I need to do a reset and submit a new patch for the DWA-131 dongle?
> >> 
> >> In general you need to use 'git pull --rebase' on my tree. I rebase it
> >> to stay in sync with Kalle's tree.
> >> 
> >> The DWA-131 is the 8192eu? Sorry a bit behind and my mind is losing
> >> bits. If it's the patch you posted earlier I can dig it out and play
> >> with it - I am still catching up though, so please be patient.
> >
> > yes it's an 8192eu.
> 
> Gotcha - how do you do your testing to reproduce the problem btw? Most
> of my testing is using the 8723au as a primary device and the next
> device as a follow-on, so I may not see as long a run with the device
> active as you see.
> 

Testing is simple. Connect to an AP in the usual manner..disconnect..reconnect.
The 8192eu will fail to reconnect and John Heenan reported the 8723bu also
fails to reconnect. Even though he was directly stopping and restarting
wpa_supplicant, it's the same thing to the driver -
connect..disconnect..reconnect.


> > Would you accept a patch that adds a struct device pointer to rtl8xxxu_priv
> > and used as the device pointer in the logging functions? Then all the messages
> > will start with the driver name making them easier to find.
> 
> How do you mean? Right now I have a struct usb_device pointer and
> dereference that for ->dev to use with dev_info() messages etc. Do you
> want to see more than that?
> 
> Cheers,
> Jes


With using a usb_device pointer, each message starts with the usb bus address.
Plug it into a different port and that address could change. By using a pointer
to the device associated with the wiphy each message will begin with the driver
name. Just makes it easier for the average user to report what's in the log
because he can just grep for "rtl8xxxu".

Barry

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

* Re: [PATCH 0/7] rtl8xxxu: Pending patches
  2016-11-20  2:42         ` Barry Day
@ 2016-11-21 16:59           ` Jes Sorensen
  2016-11-22 12:26             ` Barry Day
  0 siblings, 1 reply; 17+ messages in thread
From: Jes Sorensen @ 2016-11-21 16:59 UTC (permalink / raw)
  To: Barry Day; +Cc: linux-wireless, kvalo, Larry.Finger

Barry Day <briselec@gmail.com> writes:
> On Sat, Nov 19, 2016 at 06:53:42PM -0500, Jes Sorensen wrote:
>> Barry Day <briselec@gmail.com> writes:
>> > On Fri, Nov 18, 2016 at 09:00:10PM -0500, Jes Sorensen wrote:
>> >> Barry Day <briselec@gmail.com> writes:
>> >> > On Fri, Nov 18, 2016 at 04:44:21PM -0500, Jes.Sorensen@redhat.com wrote:
>> >> >> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>> >> >> 
>> >> >> Kalle,
>> >> >> 
>> >> >> Please find attached a number of patches for the rtl8xxxu
>> >> >> driver.
>> >> >> 
>> >> >> The issues reported with wpa_supplicant on 8723bu still needs further
>> >> >> investigation.
>> >> >> 
>> >> >
>> >> > The patch I posted that you want tested more will also fix the
>> >> > wpa_supplicant issue.  Currently I'm looking at why the tx rate is not
>> >> > what it should be. I feel fixing that first will be beneficial for
>> >> > fixing any other issues.
>> >> 
>> >> Interesting, I was thinking that might be the case. I do want to dig
>> >> into this further to understand it better. If we use your solution I
>> >> will want to make sure we cover both gen1 and gen2 parts.
>> >> 
>> >> > The recent merge has made my local branch of rtl8xxxu-devel 14
>> >> > commits ahead.
>> >> > Do I need to do a reset and submit a new patch for the DWA-131 dongle?
>> >> 
>> >> In general you need to use 'git pull --rebase' on my tree. I rebase it
>> >> to stay in sync with Kalle's tree.
>> >> 
>> >> The DWA-131 is the 8192eu? Sorry a bit behind and my mind is losing
>> >> bits. If it's the patch you posted earlier I can dig it out and play
>> >> with it - I am still catching up though, so please be patient.
>> >
>> > yes it's an 8192eu.
>> 
>> Gotcha - how do you do your testing to reproduce the problem btw? Most
>> of my testing is using the 8723au as a primary device and the next
>> device as a follow-on, so I may not see as long a run with the device
>> active as you see.
>> 
>
> Testing is simple. Connect to an AP in the usual
> manner..disconnect..reconnect.  The 8192eu will fail to reconnect and
> John Heenan reported the 8723bu also fails to reconnect. Even though
> he was directly stopping and restarting wpa_supplicant, it's the same
> thing to the driver - connect..disconnect..reconnect.

Thanks for the details - I'll have a look shortly.

> With using a usb_device pointer, each message starts with the usb bus
> address.  Plug it into a different port and that address could
> change. By using a pointer to the device associated with the wiphy
> each message will begin with the driver name. Just makes it easier for
> the average user to report what's in the log because he can just grep
> for "rtl8xxxu".

I see - that would be problematic for me as I quite often have 3-4 of
these things plugged in at the same time. Not knowing which port spits
out the message would make it a lot harder to track. In fact my primary
devel box for this (Lenovo Yoga 13) has an rtl8723au soldered on the
motherboard, so the moment I plug in any other dongle I'll have two.

The alternative would be to add a prefer to the individual messages.

Cheers,
Jes

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

* Re: [PATCH 0/7] rtl8xxxu: Pending patches
  2016-11-21 16:59           ` Jes Sorensen
@ 2016-11-22 12:26             ` Barry Day
  2016-11-29  0:08               ` Jes Sorensen
  0 siblings, 1 reply; 17+ messages in thread
From: Barry Day @ 2016-11-22 12:26 UTC (permalink / raw)
  To: Jes Sorensen; +Cc: linux-wireless, kvalo, Larry.Finger

On Mon, Nov 21, 2016 at 11:59:47AM -0500, Jes Sorensen wrote:
> Barry Day <briselec@gmail.com> writes:
> > On Sat, Nov 19, 2016 at 06:53:42PM -0500, Jes Sorensen wrote:
> >> Barry Day <briselec@gmail.com> writes:
> >> > On Fri, Nov 18, 2016 at 09:00:10PM -0500, Jes Sorensen wrote:
> >> >> Barry Day <briselec@gmail.com> writes:
> >> >> > On Fri, Nov 18, 2016 at 04:44:21PM -0500, Jes.Sorensen@redhat.com wrote:
> >> >> >> From: Jes Sorensen <Jes.Sorensen@redhat.com>
> >> >> >> 
> >> >> >> Kalle,
> >> >> >> 
> >> >> >> Please find attached a number of patches for the rtl8xxxu
> >> >> >> driver.
> >> >> >> 
> >> >> >> The issues reported with wpa_supplicant on 8723bu still needs further
> >> >> >> investigation.
> >> >> >> 
> >> >> >
> >> >> > The patch I posted that you want tested more will also fix the
> >> >> > wpa_supplicant issue.  Currently I'm looking at why the tx rate is not
> >> >> > what it should be. I feel fixing that first will be beneficial for
> >> >> > fixing any other issues.
> >> >> 
> >> >> Interesting, I was thinking that might be the case. I do want to dig
> >> >> into this further to understand it better. If we use your solution I
> >> >> will want to make sure we cover both gen1 and gen2 parts.
> >> >> 
> >> >> > The recent merge has made my local branch of rtl8xxxu-devel 14
> >> >> > commits ahead.
> >> >> > Do I need to do a reset and submit a new patch for the DWA-131 dongle?
> >> >> 
> >> >> In general you need to use 'git pull --rebase' on my tree. I rebase it
> >> >> to stay in sync with Kalle's tree.
> >> >> 
> >> >> The DWA-131 is the 8192eu? Sorry a bit behind and my mind is losing
> >> >> bits. If it's the patch you posted earlier I can dig it out and play
> >> >> with it - I am still catching up though, so please be patient.
> >> >
> >> > yes it's an 8192eu.
> >> 
> >> Gotcha - how do you do your testing to reproduce the problem btw? Most
> >> of my testing is using the 8723au as a primary device and the next
> >> device as a follow-on, so I may not see as long a run with the device
> >> active as you see.
> >> 
> >
> > Testing is simple. Connect to an AP in the usual
> > manner..disconnect..reconnect.  The 8192eu will fail to reconnect and
> > John Heenan reported the 8723bu also fails to reconnect. Even though
> > he was directly stopping and restarting wpa_supplicant, it's the same
> > thing to the driver - connect..disconnect..reconnect.
> 
> Thanks for the details - I'll have a look shortly.
> 
> > With using a usb_device pointer, each message starts with the usb bus
> > address.  Plug it into a different port and that address could
> > change. By using a pointer to the device associated with the wiphy
> > each message will begin with the driver name. Just makes it easier for
> > the average user to report what's in the log because he can just grep
> > for "rtl8xxxu".
> 
> I see - that would be problematic for me as I quite often have 3-4 of
> these things plugged in at the same time. Not knowing which port spits
> out the message would make it a lot harder to track. In fact my primary
> devel box for this (Lenovo Yoga 13) has an rtl8723au soldered on the
> motherboard, so the moment I plug in any other dongle I'll have two.
> 
> The alternative would be to add a prefer to the individual messages.
> 
> Cheers,
> Jes

I should have mentioned it also places the usb address after the driver name.

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

* Re: [1/7] rtl8xxxu: Fix memory leak in handling rxdesc16 packets
  2016-11-18 21:44 ` [PATCH 1/7] rtl8xxxu: Fix memory leak in handling rxdesc16 packets Jes.Sorensen
@ 2016-11-25  9:51   ` Kalle Valo
  0 siblings, 0 replies; 17+ messages in thread
From: Kalle Valo @ 2016-11-25  9:51 UTC (permalink / raw)
  To: Jes Sorensen; +Cc: linux-wireless, Larry.Finger, Jes Sorensen

Jes Sorensen <Jes.Sorensen@redhat.com> wrote:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
> 
> A device running without RX package aggregation could return more data
> in the USB packet than the actual network packet. In this case the
> could would clone the skb but then determine that that there was no
> packet to handle and exit without freeing the cloned skb first.
> 
> This has so far only been observed with 8188eu devices, but could
> affect others.
> 
> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>

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

a0aba89763f8 rtl8xxxu: Fix memory leak in handling rxdesc16 packets
cf7cfef06462 rtl8xxxu: Fix big-endian problem reporting mactime
b9af93551127 rtl8xxxu: Fix rtl8723bu driver reload issue
5d03f882c2fc rtl8xxxu: Fix rtl8192eu driver reload issue
a748a11038f8 rtl8xxxu: Obtain RTS rates from mac80211
b4c3d9cfb607 rtl8xxxu: Pass tx_info to fill_txdesc in order to have access to retry count
13e1349aff82 rtl8xxxu: Fix non static symbol warning

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

Documentation about submitting wireless patches and checking status
from patchwork:

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

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

* Re: [PATCH 0/7] rtl8xxxu: Pending patches
  2016-11-22 12:26             ` Barry Day
@ 2016-11-29  0:08               ` Jes Sorensen
  0 siblings, 0 replies; 17+ messages in thread
From: Jes Sorensen @ 2016-11-29  0:08 UTC (permalink / raw)
  To: Barry Day; +Cc: linux-wireless, kvalo, Larry.Finger

Barry Day <briselec@gmail.com> writes:
> On Mon, Nov 21, 2016 at 11:59:47AM -0500, Jes Sorensen wrote:
>> Barry Day <briselec@gmail.com> writes:
>> > Testing is simple. Connect to an AP in the usual
>> > manner..disconnect..reconnect.  The 8192eu will fail to reconnect and
>> > John Heenan reported the 8723bu also fails to reconnect. Even though
>> > he was directly stopping and restarting wpa_supplicant, it's the same
>> > thing to the driver - connect..disconnect..reconnect.
>> 
>> Thanks for the details - I'll have a look shortly.
>> 
>> > With using a usb_device pointer, each message starts with the usb bus
>> > address.  Plug it into a different port and that address could
>> > change. By using a pointer to the device associated with the wiphy
>> > each message will begin with the driver name. Just makes it easier for
>> > the average user to report what's in the log because he can just grep
>> > for "rtl8xxxu".
>> 
>> I see - that would be problematic for me as I quite often have 3-4 of
>> these things plugged in at the same time. Not knowing which port spits
>> out the message would make it a lot harder to track. In fact my primary
>> devel box for this (Lenovo Yoga 13) has an rtl8723au soldered on the
>> motherboard, so the moment I plug in any other dongle I'll have two.
>> 
>> The alternative would be to add a prefer to the individual messages.
>> 
>> Cheers,
>> Jes
>
> I should have mentioned it also places the usb address after the driver name.

If you're willing to cook up a patch, I'll have a look at it - I want to
see how it behaves though before deciding whether to approve it.

Cheers,
Jes

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

end of thread, other threads:[~2016-11-29  0:08 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-18 21:44 [PATCH 0/7] rtl8xxxu: Pending patches Jes.Sorensen
2016-11-18 21:44 ` [PATCH 1/7] rtl8xxxu: Fix memory leak in handling rxdesc16 packets Jes.Sorensen
2016-11-25  9:51   ` [1/7] " Kalle Valo
2016-11-18 21:44 ` [PATCH 2/7] rtl8xxxu: Fix big-endian problem reporting mactime Jes.Sorensen
2016-11-18 21:44 ` [PATCH 3/7] rtl8xxxu: Fix rtl8723bu driver reload issue Jes.Sorensen
2016-11-18 21:44 ` [PATCH 4/7] rtl8xxxu: Fix rtl8192eu " Jes.Sorensen
2016-11-18 21:44 ` [PATCH 5/7] rtl8xxxu: Obtain RTS rates from mac80211 Jes.Sorensen
2016-11-18 21:44 ` [PATCH 6/7] rtl8xxxu: Pass tx_info to fill_txdesc in order to have access to retry count Jes.Sorensen
2016-11-18 21:44 ` [PATCH 7/7] rtl8xxxu: Fix non static symbol warning Jes.Sorensen
2016-11-19  1:46 ` [PATCH 0/7] rtl8xxxu: Pending patches Barry Day
2016-11-19  2:00   ` Jes Sorensen
2016-11-19  2:38     ` Barry Day
2016-11-19 23:53       ` Jes Sorensen
2016-11-20  2:42         ` Barry Day
2016-11-21 16:59           ` Jes Sorensen
2016-11-22 12:26             ` Barry Day
2016-11-29  0:08               ` Jes Sorensen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.