linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC/RFT 0/4] restore some old mt76x0u behaviour
@ 2018-11-08 14:47 Stanislaw Gruszka
  2018-11-08 14:47 ` [RFC/RFT 1/4] mt76x02: configure basic rates and fallback on STA mode Stanislaw Gruszka
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: Stanislaw Gruszka @ 2018-11-08 14:47 UTC (permalink / raw)
  To: Felix Fietkau; +Cc: linux-wireless, Lorenzo Bianconi

I can still observe random performance drops on my setup.
Those drops are very rare after applying those:

mt76x02: assure we update gain after scan
mt76x02: run calibration after scanning
mt76x0: do not overwrite other MT_BBP(AGC, 8) fields

but sill happened in Felix tree up to:

79864387b0a0 mt76x0: pci: add DFS support

I did not notice them on linux-4.20 with 3 above fixes
and following patches:

1737618972d6 mt76x0: phy: unify calibration between mt76x0u and mt76x0e
c9540a53b257 mt76x2u: introduce mac workqueue support
9ed42cfca329 mt76x0: use mt76x02_mac_work as stats handler
f6f36ffc6da8 mt76x0: use shared debugfs implementation
1ef5b4893baa mt76: move mt76x02_debugfs in mt76x02-lib module
210ec41d92a0 mt76: move mt76x02_mac_work routine in mt76x02-lib module
9e72f73dc692 mt76x0: pci: add get_survey support
70ef7506c741 mt76x0: phy: improve code readability in initvals_phy.h
30ec3a3558c8 mt76x0: phy: simplify rf configuration routines
37385b8f96bc mt76x0: phy: use proper name convention
7936959c4544 mt76x2u: align channel gain logic to mt76x2 one
0129c6cf5577 mt76x2: align mt76x2 and mt76x2u firmware

Since is hard to reproduce the problem, I was not able to
identify offended commit. However looking at the history there are
planty of commits like:

mt76: move XXX in mt76x02-lib module

which does not only move the code, but change the behavior of
the mt76x0u driver in various ways.

Following patches restore some of old mt76x0u behaviour and
should do correct thing and do not cause any harm on different
setup (AP mode , mt76x2u, mt76x0e), but I did not test them 
in those setups.

Stanislaw Gruszka (4):
  mt76x02: configure basic rates and fallback on STA mode
  mt76x02: reserve wcid 0 for global
  mt76x02: do not set protection on set_rts_threshold callback
  mt76x02: set protection according to ht capabilities

 drivers/net/wireless/mediatek/mt76/mt76x02_mac.c  | 74 ++++++++++++++++++-----
 drivers/net/wireless/mediatek/mt76/mt76x02_mac.h  |  4 +-
 drivers/net/wireless/mediatek/mt76/mt76x02_util.c | 24 +++++++-
 3 files changed, 84 insertions(+), 18 deletions(-)

-- 
1.9.3


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

* [RFC/RFT 1/4] mt76x02: configure basic rates and fallback on STA mode
  2018-11-08 14:47 [RFC/RFT 0/4] restore some old mt76x0u behaviour Stanislaw Gruszka
@ 2018-11-08 14:47 ` Stanislaw Gruszka
  2018-11-08 14:58   ` Lorenzo Bianconi
  2018-11-08 14:47 ` [RFC/RFT 2/4] mt76x02: reserve wcid 0 for global traffic Stanislaw Gruszka
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 16+ messages in thread
From: Stanislaw Gruszka @ 2018-11-08 14:47 UTC (permalink / raw)
  To: Felix Fietkau; +Cc: linux-wireless, Lorenzo Bianconi

For STA mode configure legacy basic rates according to info
mac80211 provides to us, as well as follback registers, which
are setup in vendor driver under CONFIG_STA_SUPPORT .
For LB_FBK_CFG1 register use values from vendor driver, which
are different for mt76x0 and mt76x2 .

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/mediatek/mt76/mt76x02_util.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
index 87ce6a51fb05..2be4b527477f 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
@@ -678,6 +678,18 @@ void mt76x02_bss_info_changed(struct ieee80211_hw *hw,
 		tasklet_enable(&dev->pre_tbtt_tasklet);
 	}
 
+	if (changed & BSS_CHANGED_BASIC_RATES &&
+	    vif->type == NL80211_IFTYPE_STATION) {
+		mt76_wr(dev, MT_LEGACY_BASIC_RATE, info->basic_rates);
+		mt76_wr(dev, MT_VHT_HT_FBK_CFG0, 0x65432100);
+		mt76_wr(dev, MT_VHT_HT_FBK_CFG1, 0xedcba980);
+		mt76_wr(dev, MT_LG_FBK_CFG0, 0xedcba988);
+		if (is_mt76x2(dev))
+			mt76_wr(dev, MT_LG_FBK_CFG1, 0x87872100);
+		else
+			mt76_wr(dev, MT_LG_FBK_CFG1, 0x00872100);
+	}
+
 	if (changed & BSS_CHANGED_BEACON_INT) {
 		mt76_rmw_field(dev, MT_BEACON_TIME_CFG,
 			       MT_BEACON_TIME_CFG_INTVAL,
-- 
1.9.3


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

* [RFC/RFT 2/4] mt76x02: reserve wcid 0 for global traffic
  2018-11-08 14:47 [RFC/RFT 0/4] restore some old mt76x0u behaviour Stanislaw Gruszka
  2018-11-08 14:47 ` [RFC/RFT 1/4] mt76x02: configure basic rates and fallback on STA mode Stanislaw Gruszka
@ 2018-11-08 14:47 ` Stanislaw Gruszka
  2018-11-08 15:01   ` Lorenzo Bianconi
  2018-11-08 14:47 ` [RFC/RFT 3/4] mt76x02: do not set protection on set_rts_threshold callback Stanislaw Gruszka
  2018-11-08 14:47 ` [RFC/RFT 4/4] mt76x02: set protection according to ht capabilities Stanislaw Gruszka
  3 siblings, 1 reply; 16+ messages in thread
From: Stanislaw Gruszka @ 2018-11-08 14:47 UTC (permalink / raw)
  To: Felix Fietkau; +Cc: linux-wireless, Lorenzo Bianconi

Restore behaviour on mt76x0 before commit  1bb04bb4b838 ("mt76: move
mt76x02_init_device in mt76x02-lib module"). This will allow to use
wcid 1 for AP when we work in station mode. It's not clear if this
is needed, but this is how vendor driver assign wcid's in STA mode.
This should be harmless for mt76x2.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/mediatek/mt76/mt76x02_util.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
index 2be4b527477f..e624397b3d8b 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
@@ -113,7 +113,11 @@ void mt76x02_init_device(struct mt76x02_dev *dev)
 	ieee80211_hw_set(hw, SUPPORTS_HT_CCK_RATES);
 	ieee80211_hw_set(hw, SUPPORTS_REORDERING_BUFFER);
 
-	dev->mt76.global_wcid.idx = 255;
+	/* Reserve WCID 0 for mcast - thanks to this APs WCID will go to
+	 * entry no. 1 like it does in the vendor driver.
+	 */
+	dev->mt76.wcid_mask[0] |= 1;
+	dev->mt76.global_wcid.idx = 0;
 	dev->mt76.global_wcid.hw_key_idx = -1;
 	dev->slottime = 9;
 
-- 
1.9.3


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

* [RFC/RFT 3/4] mt76x02: do not set protection on set_rts_threshold callback
  2018-11-08 14:47 [RFC/RFT 0/4] restore some old mt76x0u behaviour Stanislaw Gruszka
  2018-11-08 14:47 ` [RFC/RFT 1/4] mt76x02: configure basic rates and fallback on STA mode Stanislaw Gruszka
  2018-11-08 14:47 ` [RFC/RFT 2/4] mt76x02: reserve wcid 0 for global traffic Stanislaw Gruszka
@ 2018-11-08 14:47 ` Stanislaw Gruszka
  2018-11-09  9:23   ` Lorenzo Bianconi
  2018-11-08 14:47 ` [RFC/RFT 4/4] mt76x02: set protection according to ht capabilities Stanislaw Gruszka
  3 siblings, 1 reply; 16+ messages in thread
From: Stanislaw Gruszka @ 2018-11-08 14:47 UTC (permalink / raw)
  To: Felix Fietkau; +Cc: linux-wireless, Lorenzo Bianconi

Use set_rts_threshold calback to enable/disable threshold only for
legacy traffic. RTS/CTS threshold for HT TXOP make make no sense
to me since used protection (RTS/CTS , CTS-to-self or none)
should be determined by HT capabilities and applied to any HT
frames.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/mediatek/mt76/mt76x02_mac.c  | 16 +---------------
 drivers/net/wireless/mediatek/mt76/mt76x02_mac.h  |  2 +-
 drivers/net/wireless/mediatek/mt76/mt76x02_util.c |  2 +-
 3 files changed, 3 insertions(+), 17 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
index 59b336e34cb5..567a7ab47fab 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
@@ -737,7 +737,7 @@ void mt76x02_tx_complete_skb(struct mt76_dev *mdev, struct mt76_queue *q,
 }
 EXPORT_SYMBOL_GPL(mt76x02_tx_complete_skb);
 
-void mt76x02_mac_set_tx_protection(struct mt76x02_dev *dev, u32 val)
+void mt76x02_mac_set_rts_thresh(struct mt76x02_dev *dev, u32 val)
 {
 	u32 data = 0;
 
@@ -751,20 +751,6 @@ void mt76x02_mac_set_tx_protection(struct mt76x02_dev *dev, u32 val)
 		 MT_PROT_CFG_CTRL | MT_PROT_CFG_RTS_THRESH, data);
 	mt76_rmw(dev, MT_OFDM_PROT_CFG,
 		 MT_PROT_CFG_CTRL | MT_PROT_CFG_RTS_THRESH, data);
-	mt76_rmw(dev, MT_MM20_PROT_CFG,
-		 MT_PROT_CFG_CTRL | MT_PROT_CFG_RTS_THRESH, data);
-	mt76_rmw(dev, MT_MM40_PROT_CFG,
-		 MT_PROT_CFG_CTRL | MT_PROT_CFG_RTS_THRESH, data);
-	mt76_rmw(dev, MT_GF20_PROT_CFG,
-		 MT_PROT_CFG_CTRL | MT_PROT_CFG_RTS_THRESH, data);
-	mt76_rmw(dev, MT_GF40_PROT_CFG,
-		 MT_PROT_CFG_CTRL | MT_PROT_CFG_RTS_THRESH, data);
-	mt76_rmw(dev, MT_TX_PROT_CFG6,
-		 MT_PROT_CFG_CTRL | MT_PROT_CFG_RTS_THRESH, data);
-	mt76_rmw(dev, MT_TX_PROT_CFG7,
-		 MT_PROT_CFG_CTRL | MT_PROT_CFG_RTS_THRESH, data);
-	mt76_rmw(dev, MT_TX_PROT_CFG8,
-		 MT_PROT_CFG_CTRL | MT_PROT_CFG_RTS_THRESH, data);
 }
 
 void mt76x02_update_channel(struct mt76_dev *mdev)
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.h b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.h
index b076c4305585..34da8c600db8 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.h
@@ -197,7 +197,7 @@ void mt76x02_send_tx_status(struct mt76x02_dev *dev,
 			    struct mt76x02_tx_status *stat, u8 *update);
 int mt76x02_mac_process_rx(struct mt76x02_dev *dev, struct sk_buff *skb,
 			   void *rxi);
-void mt76x02_mac_set_tx_protection(struct mt76x02_dev *dev, u32 val);
+void mt76x02_mac_set_rts_thresh(struct mt76x02_dev *dev, u32 val);
 void mt76x02_mac_setaddr(struct mt76x02_dev *dev, u8 *addr);
 void mt76x02_mac_write_txwi(struct mt76x02_dev *dev, struct mt76x02_txwi *txwi,
 			    struct sk_buff *skb, struct mt76_wcid *wcid,
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
index e624397b3d8b..fda67b0923a6 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
@@ -484,7 +484,7 @@ int mt76x02_set_rts_threshold(struct ieee80211_hw *hw, u32 val)
 		return -EINVAL;
 
 	mutex_lock(&dev->mutex);
-	mt76x02_mac_set_tx_protection(dev, val);
+	mt76x02_mac_set_rts_thresh(dev, val);
 	mutex_unlock(&dev->mutex);
 
 	return 0;
-- 
1.9.3


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

* [RFC/RFT 4/4] mt76x02: set protection according to ht capabilities
  2018-11-08 14:47 [RFC/RFT 0/4] restore some old mt76x0u behaviour Stanislaw Gruszka
                   ` (2 preceding siblings ...)
  2018-11-08 14:47 ` [RFC/RFT 3/4] mt76x02: do not set protection on set_rts_threshold callback Stanislaw Gruszka
@ 2018-11-08 14:47 ` Stanislaw Gruszka
  2018-11-09  9:29   ` Lorenzo Bianconi
  3 siblings, 1 reply; 16+ messages in thread
From: Stanislaw Gruszka @ 2018-11-08 14:47 UTC (permalink / raw)
  To: Felix Fietkau; +Cc: linux-wireless, Lorenzo Bianconi

Use information about protection that mac80211 provide to us.
Used protection should be part of ht capabilites that either
remote AP provde to us in STA mode or is set in hostapd.conf
in ht_capab option.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/mediatek/mt76/mt76x02_mac.c  | 58 +++++++++++++++++++++++
 drivers/net/wireless/mediatek/mt76/mt76x02_mac.h  |  2 +
 drivers/net/wireless/mediatek/mt76/mt76x02_util.c |  4 ++
 3 files changed, 64 insertions(+)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
index 567a7ab47fab..6096efc4119b 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
@@ -753,6 +753,64 @@ void mt76x02_mac_set_rts_thresh(struct mt76x02_dev *dev, u32 val)
 		 MT_PROT_CFG_CTRL | MT_PROT_CFG_RTS_THRESH, data);
 }
 
+void mt76x02_mac_set_tx_protection(struct mt76x02_dev *dev, bool legacy_prot,
+				   int ht_mode)
+{
+	int mode = ht_mode & IEEE80211_HT_OP_MODE_PROTECTION;
+	bool non_gf = !!(ht_mode & IEEE80211_HT_OP_MODE_NON_GF_STA_PRSNT);
+	u32 prot[6];
+	bool ht_rts[4] = {};
+	int i;
+
+	for (i = 0; i < 6; i++) {
+		prot[i] = mt76_rr(dev, MT_CCK_PROT_CFG + i * 4);
+		if (i >= 2)
+			prot[i] &= ~(MT_PROT_CFG_CTRL | MT_PROT_CFG_RATE);
+	}
+
+	if (legacy_prot) {
+		prot[1] &= ~MT_PROT_CFG_CTRL;
+		prot[1] |= MT_PROT_CTRL_CTS2SELF;
+
+		prot[2] |= MT_PROT_RATE_CCK_11;
+		prot[3] |= MT_PROT_RATE_CCK_11;
+		prot[4] |= MT_PROT_RATE_CCK_11;
+		prot[5] |= MT_PROT_RATE_CCK_11;
+	} else {
+		prot[2] |= MT_PROT_RATE_OFDM_24;
+		prot[3] |= MT_PROT_RATE_DUP_OFDM_24;
+		prot[4] |= MT_PROT_RATE_OFDM_24;
+		prot[5] |= MT_PROT_RATE_DUP_OFDM_24;
+	}
+
+	switch (mode) {
+	case IEEE80211_HT_OP_MODE_PROTECTION_NONE:
+		break;
+
+	case IEEE80211_HT_OP_MODE_PROTECTION_NONMEMBER:
+		ht_rts[0] = ht_rts[1] = ht_rts[2] = ht_rts[3] = true;
+		break;
+
+	case IEEE80211_HT_OP_MODE_PROTECTION_20MHZ:
+		ht_rts[1] = ht_rts[3] = true;
+		break;
+
+	case IEEE80211_HT_OP_MODE_PROTECTION_NONHT_MIXED:
+		ht_rts[0] = ht_rts[1] = ht_rts[2] = ht_rts[3] = true;
+		break;
+	}
+
+	if (non_gf)
+		ht_rts[2] = ht_rts[3] = true;
+
+	for (i = 0; i < 4; i++)
+		if (ht_rts[i])
+			prot[i + 2] |= MT_PROT_CTRL_RTS_CTS;
+
+	for (i = 0; i < 6; i++)
+		mt76_wr(dev, MT_CCK_PROT_CFG + i * 4, prot[i]);
+}
+
 void mt76x02_update_channel(struct mt76_dev *mdev)
 {
 	struct mt76x02_dev *dev = container_of(mdev, struct mt76x02_dev, mt76);
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.h b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.h
index 34da8c600db8..9035397ae081 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.h
@@ -197,6 +197,8 @@ void mt76x02_send_tx_status(struct mt76x02_dev *dev,
 			    struct mt76x02_tx_status *stat, u8 *update);
 int mt76x02_mac_process_rx(struct mt76x02_dev *dev, struct sk_buff *skb,
 			   void *rxi);
+void mt76x02_mac_set_tx_protection(struct mt76x02_dev *dev, bool legacy_prot,
+				   int ht_mode);
 void mt76x02_mac_set_rts_thresh(struct mt76x02_dev *dev, u32 val);
 void mt76x02_mac_setaddr(struct mt76x02_dev *dev, u8 *addr);
 void mt76x02_mac_write_txwi(struct mt76x02_dev *dev, struct mt76x02_txwi *txwi,
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
index fda67b0923a6..51db4d3dcc13 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
@@ -682,6 +682,10 @@ void mt76x02_bss_info_changed(struct ieee80211_hw *hw,
 		tasklet_enable(&dev->pre_tbtt_tasklet);
 	}
 
+	if (changed & BSS_CHANGED_HT || changed & BSS_CHANGED_ERP_CTS_PROT)
+		mt76x02_mac_set_tx_protection(dev, info->use_cts_prot,
+					      info->ht_operation_mode);
+
 	if (changed & BSS_CHANGED_BASIC_RATES &&
 	    vif->type == NL80211_IFTYPE_STATION) {
 		mt76_wr(dev, MT_LEGACY_BASIC_RATE, info->basic_rates);
-- 
1.9.3


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

* Re: [RFC/RFT 1/4] mt76x02: configure basic rates and fallback on STA mode
  2018-11-08 14:47 ` [RFC/RFT 1/4] mt76x02: configure basic rates and fallback on STA mode Stanislaw Gruszka
@ 2018-11-08 14:58   ` Lorenzo Bianconi
  2018-11-08 15:52     ` Stanislaw Gruszka
  0 siblings, 1 reply; 16+ messages in thread
From: Lorenzo Bianconi @ 2018-11-08 14:58 UTC (permalink / raw)
  To: Stanislaw Gruszka; +Cc: Felix Fietkau, linux-wireless

> For STA mode configure legacy basic rates according to info
> mac80211 provides to us, as well as follback registers, which
> are setup in vendor driver under CONFIG_STA_SUPPORT .
> For LB_FBK_CFG1 register use values from vendor driver, which
> are different for mt76x0 and mt76x2 .
>
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
>  drivers/net/wireless/mediatek/mt76/mt76x02_util.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
> index 87ce6a51fb05..2be4b527477f 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
> @@ -678,6 +678,18 @@ void mt76x02_bss_info_changed(struct ieee80211_hw *hw,
>                 tasklet_enable(&dev->pre_tbtt_tasklet);
>         }
>
> +       if (changed & BSS_CHANGED_BASIC_RATES &&
> +           vif->type == NL80211_IFTYPE_STATION) {
> +               mt76_wr(dev, MT_LEGACY_BASIC_RATE, info->basic_rates);
> +               mt76_wr(dev, MT_VHT_HT_FBK_CFG0, 0x65432100);
> +               mt76_wr(dev, MT_VHT_HT_FBK_CFG1, 0xedcba980);
> +               mt76_wr(dev, MT_LG_FBK_CFG0, 0xedcba988);
> +               if (is_mt76x2(dev))
> +                       mt76_wr(dev, MT_LG_FBK_CFG1, 0x87872100);
> +               else
> +                       mt76_wr(dev, MT_LG_FBK_CFG1, 0x00872100);
> +       }
> +

since these values are constant, why not put them init_vals?

Regards,
Lorenzo

>         if (changed & BSS_CHANGED_BEACON_INT) {
>                 mt76_rmw_field(dev, MT_BEACON_TIME_CFG,
>                                MT_BEACON_TIME_CFG_INTVAL,
> --
> 1.9.3
>

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

* Re: [RFC/RFT 2/4] mt76x02: reserve wcid 0 for global traffic
  2018-11-08 14:47 ` [RFC/RFT 2/4] mt76x02: reserve wcid 0 for global traffic Stanislaw Gruszka
@ 2018-11-08 15:01   ` Lorenzo Bianconi
  2018-11-08 15:54     ` Stanislaw Gruszka
  0 siblings, 1 reply; 16+ messages in thread
From: Lorenzo Bianconi @ 2018-11-08 15:01 UTC (permalink / raw)
  To: Stanislaw Gruszka; +Cc: Felix Fietkau, linux-wireless

> Restore behaviour on mt76x0 before commit  1bb04bb4b838 ("mt76: move
> mt76x02_init_device in mt76x02-lib module"). This will allow to use
> wcid 1 for AP when we work in station mode. It's not clear if this
> is needed, but this is how vendor driver assign wcid's in STA mode.
> This should be harmless for mt76x2.
>
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
>  drivers/net/wireless/mediatek/mt76/mt76x02_util.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
> index 2be4b527477f..e624397b3d8b 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
> @@ -113,7 +113,11 @@ void mt76x02_init_device(struct mt76x02_dev *dev)
>         ieee80211_hw_set(hw, SUPPORTS_HT_CCK_RATES);
>         ieee80211_hw_set(hw, SUPPORTS_REORDERING_BUFFER);
>
> -       dev->mt76.global_wcid.idx = 255;
> +       /* Reserve WCID 0 for mcast - thanks to this APs WCID will go to
> +        * entry no. 1 like it does in the vendor driver.
> +        */
> +       dev->mt76.wcid_mask[0] |= 1;
> +       dev->mt76.global_wcid.idx = 0;
>         dev->mt76.global_wcid.hw_key_idx = -1;
>         dev->slottime = 9;
>

Does it make any difference in AP mode? What about using 0 instead of
255 for global_wcid.idx?

Regards,
Lorenzo

> --
> 1.9.3
>

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

* Re: [RFC/RFT 1/4] mt76x02: configure basic rates and fallback on STA mode
  2018-11-08 14:58   ` Lorenzo Bianconi
@ 2018-11-08 15:52     ` Stanislaw Gruszka
  2018-11-08 16:02       ` Felix Fietkau
  0 siblings, 1 reply; 16+ messages in thread
From: Stanislaw Gruszka @ 2018-11-08 15:52 UTC (permalink / raw)
  To: Lorenzo Bianconi; +Cc: Felix Fietkau, linux-wireless

On Thu, Nov 08, 2018 at 03:58:29PM +0100, Lorenzo Bianconi wrote:
> > For STA mode configure legacy basic rates according to info
> > mac80211 provides to us, as well as follback registers, which
> > are setup in vendor driver under CONFIG_STA_SUPPORT .
> > For LB_FBK_CFG1 register use values from vendor driver, which
> > are different for mt76x0 and mt76x2 .
> >
> > Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> > ---
> >  drivers/net/wireless/mediatek/mt76/mt76x02_util.c | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
> > index 87ce6a51fb05..2be4b527477f 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
> > @@ -678,6 +678,18 @@ void mt76x02_bss_info_changed(struct ieee80211_hw *hw,
> >                 tasklet_enable(&dev->pre_tbtt_tasklet);
> >         }
> >
> > +       if (changed & BSS_CHANGED_BASIC_RATES &&
> > +           vif->type == NL80211_IFTYPE_STATION) {
> > +               mt76_wr(dev, MT_LEGACY_BASIC_RATE, info->basic_rates);
> > +               mt76_wr(dev, MT_VHT_HT_FBK_CFG0, 0x65432100);
> > +               mt76_wr(dev, MT_VHT_HT_FBK_CFG1, 0xedcba980);
> > +               mt76_wr(dev, MT_LG_FBK_CFG0, 0xedcba988);
> > +               if (is_mt76x2(dev))
> > +                       mt76_wr(dev, MT_LG_FBK_CFG1, 0x87872100);
> > +               else
> > +                       mt76_wr(dev, MT_LG_FBK_CFG1, 0x00872100);
> > +       }
> > +
> 
> since these values are constant, why not put them init_vals?

I wanted them to be used only in STA mode like vendor driver does.
However if we will have AP+STA , we still will configure them,
so better to configure them anyway and put in init_vals .

Thanks
Stanislaw


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

* Re: [RFC/RFT 2/4] mt76x02: reserve wcid 0 for global traffic
  2018-11-08 15:01   ` Lorenzo Bianconi
@ 2018-11-08 15:54     ` Stanislaw Gruszka
  2018-11-08 16:05       ` Felix Fietkau
  0 siblings, 1 reply; 16+ messages in thread
From: Stanislaw Gruszka @ 2018-11-08 15:54 UTC (permalink / raw)
  To: Lorenzo Bianconi; +Cc: Felix Fietkau, linux-wireless

On Thu, Nov 08, 2018 at 04:01:54PM +0100, Lorenzo Bianconi wrote:
> > Restore behaviour on mt76x0 before commit  1bb04bb4b838 ("mt76: move
> > mt76x02_init_device in mt76x02-lib module"). This will allow to use
> > wcid 1 for AP when we work in station mode. It's not clear if this
> > is needed, but this is how vendor driver assign wcid's in STA mode.
> > This should be harmless for mt76x2.
> >
> > Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> > ---
> >  drivers/net/wireless/mediatek/mt76/mt76x02_util.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
> > index 2be4b527477f..e624397b3d8b 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
> > @@ -113,7 +113,11 @@ void mt76x02_init_device(struct mt76x02_dev *dev)
> >         ieee80211_hw_set(hw, SUPPORTS_HT_CCK_RATES);
> >         ieee80211_hw_set(hw, SUPPORTS_REORDERING_BUFFER);
> >
> > -       dev->mt76.global_wcid.idx = 255;
> > +       /* Reserve WCID 0 for mcast - thanks to this APs WCID will go to
> > +        * entry no. 1 like it does in the vendor driver.
> > +        */
> > +       dev->mt76.wcid_mask[0] |= 1;
> > +       dev->mt76.global_wcid.idx = 0;
> >         dev->mt76.global_wcid.hw_key_idx = -1;
> >         dev->slottime = 9;
> >
> 
> Does it make any difference in AP mode?

First sta will get wcid = 1 instead of 0.

> What about using 0 instead of
> 255 for global_wcid.idx?

Patch do exactly that , it assigns:

	dev->mt76.global_wcid.idx = 0;

Thanks
Stanislaw

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

* Re: [RFC/RFT 1/4] mt76x02: configure basic rates and fallback on STA mode
  2018-11-08 15:52     ` Stanislaw Gruszka
@ 2018-11-08 16:02       ` Felix Fietkau
  2018-12-07 13:25         ` Stanislaw Gruszka
  0 siblings, 1 reply; 16+ messages in thread
From: Felix Fietkau @ 2018-11-08 16:02 UTC (permalink / raw)
  To: Stanislaw Gruszka, Lorenzo Bianconi; +Cc: linux-wireless

On 2018-11-08 16:52, Stanislaw Gruszka wrote:
> On Thu, Nov 08, 2018 at 03:58:29PM +0100, Lorenzo Bianconi wrote:
>> > For STA mode configure legacy basic rates according to info
>> > mac80211 provides to us, as well as follback registers, which
>> > are setup in vendor driver under CONFIG_STA_SUPPORT .
>> > For LB_FBK_CFG1 register use values from vendor driver, which
>> > are different for mt76x0 and mt76x2 .
>> >
>> > Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
>> > ---
>> >  drivers/net/wireless/mediatek/mt76/mt76x02_util.c | 12 ++++++++++++
>> >  1 file changed, 12 insertions(+)
>> >
>> > diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
>> > index 87ce6a51fb05..2be4b527477f 100644
>> > --- a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
>> > +++ b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
>> > @@ -678,6 +678,18 @@ void mt76x02_bss_info_changed(struct ieee80211_hw *hw,
>> >                 tasklet_enable(&dev->pre_tbtt_tasklet);
>> >         }
>> >
>> > +       if (changed & BSS_CHANGED_BASIC_RATES &&
>> > +           vif->type == NL80211_IFTYPE_STATION) {
>> > +               mt76_wr(dev, MT_LEGACY_BASIC_RATE, info->basic_rates);
>> > +               mt76_wr(dev, MT_VHT_HT_FBK_CFG0, 0x65432100);
>> > +               mt76_wr(dev, MT_VHT_HT_FBK_CFG1, 0xedcba980);
>> > +               mt76_wr(dev, MT_LG_FBK_CFG0, 0xedcba988);
>> > +               if (is_mt76x2(dev))
>> > +                       mt76_wr(dev, MT_LG_FBK_CFG1, 0x87872100);
>> > +               else
>> > +                       mt76_wr(dev, MT_LG_FBK_CFG1, 0x00872100);
>> > +       }
>> > +
>> 
>> since these values are constant, why not put them init_vals?
> 
> I wanted them to be used only in STA mode like vendor driver does.
> However if we will have AP+STA , we still will configure them,
> so better to configure them anyway and put in init_vals .
Yes, I think we should put them in initvals. Also, I think we should
keep the values the shared between mt76x0 and mt76x2.

For MT_LG_FBK_CFG1 there is no need to make a distinction between 76x2
and 76x0, because 76x0 will simply ignore the upper values (they apply
to 2SS transmission only).
Also, I don't think MT_VHT_HT_FBK_CFG1 even gets used on mt76x0, because
it is for 2SS rates only. However, the value in there seems more useful
than the one we currently use on mt76x2, because it allows fallback from
2SS MCS0 to 1SS MCS0.
The contents of MT_LG_FBK_CFG0 also match the default initialization
values documented in the 7612E datasheet.

I don't think there is any meaningful difference between the 76x2 and
the 76x0 MAC except for the missing 2SS support.

- Felix

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

* Re: [RFC/RFT 2/4] mt76x02: reserve wcid 0 for global traffic
  2018-11-08 15:54     ` Stanislaw Gruszka
@ 2018-11-08 16:05       ` Felix Fietkau
  0 siblings, 0 replies; 16+ messages in thread
From: Felix Fietkau @ 2018-11-08 16:05 UTC (permalink / raw)
  To: Stanislaw Gruszka, Lorenzo Bianconi; +Cc: linux-wireless

On 2018-11-08 16:54, Stanislaw Gruszka wrote:
> On Thu, Nov 08, 2018 at 04:01:54PM +0100, Lorenzo Bianconi wrote:
>> > Restore behaviour on mt76x0 before commit  1bb04bb4b838 ("mt76: move
>> > mt76x02_init_device in mt76x02-lib module"). This will allow to use
>> > wcid 1 for AP when we work in station mode. It's not clear if this
>> > is needed, but this is how vendor driver assign wcid's in STA mode.
>> > This should be harmless for mt76x2.
>> >
>> > Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
>> > ---
>> >  drivers/net/wireless/mediatek/mt76/mt76x02_util.c | 6 +++++-
>> >  1 file changed, 5 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
>> > index 2be4b527477f..e624397b3d8b 100644
>> > --- a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
>> > +++ b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
>> > @@ -113,7 +113,11 @@ void mt76x02_init_device(struct mt76x02_dev *dev)
>> >         ieee80211_hw_set(hw, SUPPORTS_HT_CCK_RATES);
>> >         ieee80211_hw_set(hw, SUPPORTS_REORDERING_BUFFER);
>> >
>> > -       dev->mt76.global_wcid.idx = 255;
>> > +       /* Reserve WCID 0 for mcast - thanks to this APs WCID will go to
>> > +        * entry no. 1 like it does in the vendor driver.
>> > +        */
>> > +       dev->mt76.wcid_mask[0] |= 1;
>> > +       dev->mt76.global_wcid.idx = 0;
>> >         dev->mt76.global_wcid.hw_key_idx = -1;
>> >         dev->slottime = 9;
>> >
>> 
>> Does it make any difference in AP mode?
> 
> First sta will get wcid = 1 instead of 0.
> 
>> What about using 0 instead of
>> 255 for global_wcid.idx?
> 
> Patch do exactly that , it assigns:
> 
> 	dev->mt76.global_wcid.idx = 0;
I don't think we should reserve WCID0 just because the vendor driver
does it, unless we can find a case where it actually makes a meaningful
difference. WCID entries >128 are useless for normal stations, so let's
use those for reserved entries instead of reducing the effective station
number limit.

- Felix

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

* Re: [RFC/RFT 3/4] mt76x02: do not set protection on set_rts_threshold callback
  2018-11-08 14:47 ` [RFC/RFT 3/4] mt76x02: do not set protection on set_rts_threshold callback Stanislaw Gruszka
@ 2018-11-09  9:23   ` Lorenzo Bianconi
  2018-12-04 10:45     ` Stanislaw Gruszka
  0 siblings, 1 reply; 16+ messages in thread
From: Lorenzo Bianconi @ 2018-11-09  9:23 UTC (permalink / raw)
  To: Stanislaw Gruszka; +Cc: Felix Fietkau, linux-wireless

> Use set_rts_threshold calback to enable/disable threshold only for
> legacy traffic. RTS/CTS threshold for HT TXOP make make no sense
> to me since used protection (RTS/CTS , CTS-to-self or none)
> should be determined by HT capabilities and applied to any HT
> frames.
> 
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
>  drivers/net/wireless/mediatek/mt76/mt76x02_mac.c  | 16 +---------------
>  drivers/net/wireless/mediatek/mt76/mt76x02_mac.h  |  2 +-
>  drivers/net/wireless/mediatek/mt76/mt76x02_util.c |  2 +-
>  3 files changed, 3 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
> index 59b336e34cb5..567a7ab47fab 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
> @@ -737,7 +737,7 @@ void mt76x02_tx_complete_skb(struct mt76_dev *mdev, struct mt76_queue *q,
>  }
>  EXPORT_SYMBOL_GPL(mt76x02_tx_complete_skb);
>  
> -void mt76x02_mac_set_tx_protection(struct mt76x02_dev *dev, u32 val)
> +void mt76x02_mac_set_rts_thresh(struct mt76x02_dev *dev, u32 val)
>  {
>  	u32 data = 0;
>  
> @@ -751,20 +751,6 @@ void mt76x02_mac_set_tx_protection(struct mt76x02_dev *dev, u32 val)
>  		 MT_PROT_CFG_CTRL | MT_PROT_CFG_RTS_THRESH, data);
>  	mt76_rmw(dev, MT_OFDM_PROT_CFG,
>  		 MT_PROT_CFG_CTRL | MT_PROT_CFG_RTS_THRESH, data);

Do we need to configure MT_OFDM_PROT_CFG and MT_CCK_PROT_CFG here? (since they
are already configured in 4/4 (mt76x02: set protection according to ht
capabilities))

> -	mt76_rmw(dev, MT_MM20_PROT_CFG,
> -		 MT_PROT_CFG_CTRL | MT_PROT_CFG_RTS_THRESH, data);
> -	mt76_rmw(dev, MT_MM40_PROT_CFG,
> -		 MT_PROT_CFG_CTRL | MT_PROT_CFG_RTS_THRESH, data);
> -	mt76_rmw(dev, MT_GF20_PROT_CFG,
> -		 MT_PROT_CFG_CTRL | MT_PROT_CFG_RTS_THRESH, data);
> -	mt76_rmw(dev, MT_GF40_PROT_CFG,
> -		 MT_PROT_CFG_CTRL | MT_PROT_CFG_RTS_THRESH, data);
> -	mt76_rmw(dev, MT_TX_PROT_CFG6,
> -		 MT_PROT_CFG_CTRL | MT_PROT_CFG_RTS_THRESH, data);
> -	mt76_rmw(dev, MT_TX_PROT_CFG7,
> -		 MT_PROT_CFG_CTRL | MT_PROT_CFG_RTS_THRESH, data);
> -	mt76_rmw(dev, MT_TX_PROT_CFG8,
> -		 MT_PROT_CFG_CTRL | MT_PROT_CFG_RTS_THRESH, data);

Removing these lines we are no longer able to configure protection for VHT
rates. Do we have an equivalent for them in vht_capab?

Regards,
Lorenzo

>  }

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

* Re: [RFC/RFT 4/4] mt76x02: set protection according to ht capabilities
  2018-11-08 14:47 ` [RFC/RFT 4/4] mt76x02: set protection according to ht capabilities Stanislaw Gruszka
@ 2018-11-09  9:29   ` Lorenzo Bianconi
  0 siblings, 0 replies; 16+ messages in thread
From: Lorenzo Bianconi @ 2018-11-09  9:29 UTC (permalink / raw)
  To: Stanislaw Gruszka; +Cc: Felix Fietkau, linux-wireless

> Use information about protection that mac80211 provide to us.
> Used protection should be part of ht capabilites that either
> remote AP provde to us in STA mode or is set in hostapd.conf
> in ht_capab option.
> 
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
>  drivers/net/wireless/mediatek/mt76/mt76x02_mac.c  | 58 +++++++++++++++++++++++
>  drivers/net/wireless/mediatek/mt76/mt76x02_mac.h  |  2 +
>  drivers/net/wireless/mediatek/mt76/mt76x02_util.c |  4 ++
>  3 files changed, 64 insertions(+)
> 
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
> index 567a7ab47fab..6096efc4119b 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
> @@ -753,6 +753,64 @@ void mt76x02_mac_set_rts_thresh(struct mt76x02_dev *dev, u32 val)
>  		 MT_PROT_CFG_CTRL | MT_PROT_CFG_RTS_THRESH, data);
>  }
>  
> +void mt76x02_mac_set_tx_protection(struct mt76x02_dev *dev, bool legacy_prot,
> +				   int ht_mode)
> +{
> +	int mode = ht_mode & IEEE80211_HT_OP_MODE_PROTECTION;
> +	bool non_gf = !!(ht_mode & IEEE80211_HT_OP_MODE_NON_GF_STA_PRSNT);
> +	u32 prot[6];
> +	bool ht_rts[4] = {};
> +	int i;
> +
> +	for (i = 0; i < 6; i++) {

maybe better ARRAY_SIZE() here?

> +		prot[i] = mt76_rr(dev, MT_CCK_PROT_CFG + i * 4);
> +		if (i >= 2)
> +			prot[i] &= ~(MT_PROT_CFG_CTRL | MT_PROT_CFG_RATE);
> +	}
> +
> +	if (legacy_prot) {
> +		prot[1] &= ~MT_PROT_CFG_CTRL;
> +		prot[1] |= MT_PROT_CTRL_CTS2SELF;
> +
> +		prot[2] |= MT_PROT_RATE_CCK_11;
> +		prot[3] |= MT_PROT_RATE_CCK_11;
> +		prot[4] |= MT_PROT_RATE_CCK_11;
> +		prot[5] |= MT_PROT_RATE_CCK_11;
> +	} else {
> +		prot[2] |= MT_PROT_RATE_OFDM_24;
> +		prot[3] |= MT_PROT_RATE_DUP_OFDM_24;
> +		prot[4] |= MT_PROT_RATE_OFDM_24;
> +		prot[5] |= MT_PROT_RATE_DUP_OFDM_24;
> +	}
> +
> +	switch (mode) {
> +	case IEEE80211_HT_OP_MODE_PROTECTION_NONE:

I think you can cover it with 'default' case

> +		break;
> +
> +	case IEEE80211_HT_OP_MODE_PROTECTION_NONMEMBER:
> +		ht_rts[0] = ht_rts[1] = ht_rts[2] = ht_rts[3] = true;
> +		break;
> +
> +	case IEEE80211_HT_OP_MODE_PROTECTION_20MHZ:
> +		ht_rts[1] = ht_rts[3] = true;
> +		break;
> +
> +	case IEEE80211_HT_OP_MODE_PROTECTION_NONHT_MIXED:
> +		ht_rts[0] = ht_rts[1] = ht_rts[2] = ht_rts[3] = true;
> +		break;

this code is duplicated of 'IEEE80211_HT_OP_MODE_PROTECTION_NONMEMBER'.
maybe better to have:

	case IEEE80211_HT_OP_MODE_PROTECTION_NONMEMBER:
	case IEEE80211_HT_OP_MODE_PROTECTION_NONHT_MIXED:
		ht_rts[0] = ht_rts[1] = ht_rts[2] = ht_rts[3] = true;
		break;

> +	}
> +
> +	if (non_gf)
> +		ht_rts[2] = ht_rts[3] = true;
> +
> +	for (i = 0; i < 4; i++)

ARRAY_SIZE() here?

> +		if (ht_rts[i])
> +			prot[i + 2] |= MT_PROT_CTRL_RTS_CTS;
> +
> +	for (i = 0; i < 6; i++)

ARRAY_SIZE() here?

> +		mt76_wr(dev, MT_CCK_PROT_CFG + i * 4, prot[i]);
> +}
> +
>  void mt76x02_update_channel(struct mt76_dev *mdev)
>  {
>  	struct mt76x02_dev *dev = container_of(mdev, struct mt76x02_dev, mt76);
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.h b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.h
> index 34da8c600db8..9035397ae081 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.h
> +++ b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.h
> @@ -197,6 +197,8 @@ void mt76x02_send_tx_status(struct mt76x02_dev *dev,
>  			    struct mt76x02_tx_status *stat, u8 *update);
>  int mt76x02_mac_process_rx(struct mt76x02_dev *dev, struct sk_buff *skb,
>  			   void *rxi);
> +void mt76x02_mac_set_tx_protection(struct mt76x02_dev *dev, bool legacy_prot,
> +				   int ht_mode);
>  void mt76x02_mac_set_rts_thresh(struct mt76x02_dev *dev, u32 val);
>  void mt76x02_mac_setaddr(struct mt76x02_dev *dev, u8 *addr);
>  void mt76x02_mac_write_txwi(struct mt76x02_dev *dev, struct mt76x02_txwi *txwi,
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
> index fda67b0923a6..51db4d3dcc13 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
> @@ -682,6 +682,10 @@ void mt76x02_bss_info_changed(struct ieee80211_hw *hw,
>  		tasklet_enable(&dev->pre_tbtt_tasklet);
>  	}
>  
> +	if (changed & BSS_CHANGED_HT || changed & BSS_CHANGED_ERP_CTS_PROT)
> +		mt76x02_mac_set_tx_protection(dev, info->use_cts_prot,
> +					      info->ht_operation_mode);
> +
>  	if (changed & BSS_CHANGED_BASIC_RATES &&
>  	    vif->type == NL80211_IFTYPE_STATION) {
>  		mt76_wr(dev, MT_LEGACY_BASIC_RATE, info->basic_rates);
> -- 
> 1.9.3
> 

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

* Re: [RFC/RFT 3/4] mt76x02: do not set protection on set_rts_threshold callback
  2018-11-09  9:23   ` Lorenzo Bianconi
@ 2018-12-04 10:45     ` Stanislaw Gruszka
  2018-12-04 11:50       ` Stanislaw Gruszka
  0 siblings, 1 reply; 16+ messages in thread
From: Stanislaw Gruszka @ 2018-12-04 10:45 UTC (permalink / raw)
  To: Lorenzo Bianconi; +Cc: Felix Fietkau, linux-wireless

On Fri, Nov 09, 2018 at 10:23:56AM +0100, Lorenzo Bianconi wrote:
> > Use set_rts_threshold calback to enable/disable threshold only for
> > legacy traffic. RTS/CTS threshold for HT TXOP make make no sense
> > to me since used protection (RTS/CTS , CTS-to-self or none)
> > should be determined by HT capabilities and applied to any HT
> > frames.
> > 
> > Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> > ---
> >  drivers/net/wireless/mediatek/mt76/mt76x02_mac.c  | 16 +---------------
> >  drivers/net/wireless/mediatek/mt76/mt76x02_mac.h  |  2 +-
> >  drivers/net/wireless/mediatek/mt76/mt76x02_util.c |  2 +-
> >  3 files changed, 3 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
> > index 59b336e34cb5..567a7ab47fab 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
> > @@ -737,7 +737,7 @@ void mt76x02_tx_complete_skb(struct mt76_dev *mdev, struct mt76_queue *q,
> >  }
> >  EXPORT_SYMBOL_GPL(mt76x02_tx_complete_skb);
> >  
> > -void mt76x02_mac_set_tx_protection(struct mt76x02_dev *dev, u32 val)
> > +void mt76x02_mac_set_rts_thresh(struct mt76x02_dev *dev, u32 val)
> >  {
> >  	u32 data = 0;
> >  
> > @@ -751,20 +751,6 @@ void mt76x02_mac_set_tx_protection(struct mt76x02_dev *dev, u32 val)
> >  		 MT_PROT_CFG_CTRL | MT_PROT_CFG_RTS_THRESH, data);
> >  	mt76_rmw(dev, MT_OFDM_PROT_CFG,
> >  		 MT_PROT_CFG_CTRL | MT_PROT_CFG_RTS_THRESH, data);
> 
> Do we need to configure MT_OFDM_PROT_CFG and MT_CCK_PROT_CFG here? (since they
> are already configured in 4/4 (mt76x02: set protection according to ht
> capabilities))

Only OFDM_PROT_CFG is configured there based on legacy proto
value. I'm not sure how handle CCK_PROT_CFG.

> > -	mt76_rmw(dev, MT_MM20_PROT_CFG,
> > -		 MT_PROT_CFG_CTRL | MT_PROT_CFG_RTS_THRESH, data);
> > -	mt76_rmw(dev, MT_MM40_PROT_CFG,
> > -		 MT_PROT_CFG_CTRL | MT_PROT_CFG_RTS_THRESH, data);
> > -	mt76_rmw(dev, MT_GF20_PROT_CFG,
> > -		 MT_PROT_CFG_CTRL | MT_PROT_CFG_RTS_THRESH, data);
> > -	mt76_rmw(dev, MT_GF40_PROT_CFG,
> > -		 MT_PROT_CFG_CTRL | MT_PROT_CFG_RTS_THRESH, data);
> > -	mt76_rmw(dev, MT_TX_PROT_CFG6,
> > -		 MT_PROT_CFG_CTRL | MT_PROT_CFG_RTS_THRESH, data);
> > -	mt76_rmw(dev, MT_TX_PROT_CFG7,
> > -		 MT_PROT_CFG_CTRL | MT_PROT_CFG_RTS_THRESH, data);
> > -	mt76_rmw(dev, MT_TX_PROT_CFG8,
> > -		 MT_PROT_CFG_CTRL | MT_PROT_CFG_RTS_THRESH, data);
> 
> Removing these lines we are no longer able to configure protection for VHT
> rates. Do we have an equivalent for them in vht_capab?

Actually it's not based on HT capabilities but by on ht operation and
it's modified dynamically by hostapd based on what stations are
associated. For STA mode it's provided by remote AP via HT operation IE.

VHT Operation IE do not define protection. Seems interoperability with
legacy STA's is not allowed for VHT, so leaving default values from
initvals where PROT bits are 0 (none protection) is right thing to do.

Regards
Stanislaw

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

* Re: [RFC/RFT 3/4] mt76x02: do not set protection on set_rts_threshold callback
  2018-12-04 10:45     ` Stanislaw Gruszka
@ 2018-12-04 11:50       ` Stanislaw Gruszka
  0 siblings, 0 replies; 16+ messages in thread
From: Stanislaw Gruszka @ 2018-12-04 11:50 UTC (permalink / raw)
  To: Lorenzo Bianconi; +Cc: Felix Fietkau, linux-wireless

On Tue, Dec 04, 2018 at 11:45:09AM +0100, Stanislaw Gruszka wrote:

> Only OFDM_PROT_CFG is configured there based on legacy proto
> value. I'm not sure how handle CCK_PROT_CFG.
> 
> > > -	mt76_rmw(dev, MT_MM20_PROT_CFG,
> > > -		 MT_PROT_CFG_CTRL | MT_PROT_CFG_RTS_THRESH, data);
> > > -	mt76_rmw(dev, MT_MM40_PROT_CFG,
> > > -		 MT_PROT_CFG_CTRL | MT_PROT_CFG_RTS_THRESH, data);
> > > -	mt76_rmw(dev, MT_GF20_PROT_CFG,
> > > -		 MT_PROT_CFG_CTRL | MT_PROT_CFG_RTS_THRESH, data);
> > > -	mt76_rmw(dev, MT_GF40_PROT_CFG,
> > > -		 MT_PROT_CFG_CTRL | MT_PROT_CFG_RTS_THRESH, data);
> > > -	mt76_rmw(dev, MT_TX_PROT_CFG6,
> > > -		 MT_PROT_CFG_CTRL | MT_PROT_CFG_RTS_THRESH, data);
> > > -	mt76_rmw(dev, MT_TX_PROT_CFG7,
> > > -		 MT_PROT_CFG_CTRL | MT_PROT_CFG_RTS_THRESH, data);
> > > -	mt76_rmw(dev, MT_TX_PROT_CFG8,
> > > -		 MT_PROT_CFG_CTRL | MT_PROT_CFG_RTS_THRESH, data);
> > 
> > Removing these lines we are no longer able to configure protection for VHT
> > rates. Do we have an equivalent for them in vht_capab?
> 
> Actually it's not based on HT capabilities but by on ht operation and
> it's modified dynamically by hostapd based on what stations are
> associated. For STA mode it's provided by remote AP via HT operation IE.
> 
> VHT Operation IE do not define protection. Seems interoperability with
> legacy STA's is not allowed for VHT, so leaving default values from
> initvals where PROT bits are 0 (none protection) is right thing to do.

But vendor driver change the VHT protection bits based on HT
operation element, with the comment:

"TODO: shiang-6590, fix me for this protection mechanism"

So I'm not sure any longer what correct behaviour should be for 
TX_PROT_CFG{6,7,8}.

Regards
Stanislaw

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

* Re: [RFC/RFT 1/4] mt76x02: configure basic rates and fallback on STA mode
  2018-11-08 16:02       ` Felix Fietkau
@ 2018-12-07 13:25         ` Stanislaw Gruszka
  0 siblings, 0 replies; 16+ messages in thread
From: Stanislaw Gruszka @ 2018-12-07 13:25 UTC (permalink / raw)
  To: Felix Fietkau; +Cc: Lorenzo Bianconi, linux-wireless

On Thu, Nov 08, 2018 at 05:02:14PM +0100, Felix Fietkau wrote:
> >> > +       if (changed & BSS_CHANGED_BASIC_RATES &&
> >> > +           vif->type == NL80211_IFTYPE_STATION) {
> >> > +               mt76_wr(dev, MT_LEGACY_BASIC_RATE, info->basic_rates);

It's a bit hard to interpret how vendor driver set LEGACY_BIT_RATE
in MlmeUpdateTxRates(). It seems to always enable bits for
OFDM6/OFDM12/OFDM24 (bitmask 0x150) or maybe just do this for 5GHz,
I'm not sure. Need more investigation on this.
 
> >> > +               mt76_wr(dev, MT_VHT_HT_FBK_CFG0, 0x65432100);
> >> > +               mt76_wr(dev, MT_VHT_HT_FBK_CFG1, 0xedcba980);
> >> > +               mt76_wr(dev, MT_LG_FBK_CFG0, 0xedcba988);
> >> > +               if (is_mt76x2(dev))
> >> > +                       mt76_wr(dev, MT_LG_FBK_CFG1, 0x87872100);
> >> > +               else
> >> > +                       mt76_wr(dev, MT_LG_FBK_CFG1, 0x00872100);
> >> > +       }
> >> > +
> >> 
> >> since these values are constant, why not put them init_vals?
> > 
> > I wanted them to be used only in STA mode like vendor driver does.
> > However if we will have AP+STA , we still will configure them,
> > so better to configure them anyway and put in init_vals .
> Yes, I think we should put them in initvals. Also, I think we should
> keep the values the shared between mt76x0 and mt76x2.
> 
> For MT_LG_FBK_CFG1 there is no need to make a distinction between 76x2
> and 76x0, because 76x0 will simply ignore the upper values (they apply
> to 2SS transmission only).
> Also, I don't think MT_VHT_HT_FBK_CFG1 even gets used on mt76x0, because
> it is for 2SS rates only. However, the value in there seems more useful
> than the one we currently use on mt76x2, because it allows fallback from
> 2SS MCS0 to 1SS MCS0.
> The contents of MT_LG_FBK_CFG0 also match the default initialization
> values documented in the 7612E datasheet.
> 
> I don't think there is any meaningful difference between the 76x2 and
> the 76x0 MAC except for the missing 2SS support.

Since there are different initvals tables for mt76x2 and mt76x0, I use
different values of MT_LG_FBK_CFG1. Will post a patches soon, please
comment there, if something is not correct.

Thanks
Stanislaw

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

end of thread, other threads:[~2018-12-07 13:25 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-08 14:47 [RFC/RFT 0/4] restore some old mt76x0u behaviour Stanislaw Gruszka
2018-11-08 14:47 ` [RFC/RFT 1/4] mt76x02: configure basic rates and fallback on STA mode Stanislaw Gruszka
2018-11-08 14:58   ` Lorenzo Bianconi
2018-11-08 15:52     ` Stanislaw Gruszka
2018-11-08 16:02       ` Felix Fietkau
2018-12-07 13:25         ` Stanislaw Gruszka
2018-11-08 14:47 ` [RFC/RFT 2/4] mt76x02: reserve wcid 0 for global traffic Stanislaw Gruszka
2018-11-08 15:01   ` Lorenzo Bianconi
2018-11-08 15:54     ` Stanislaw Gruszka
2018-11-08 16:05       ` Felix Fietkau
2018-11-08 14:47 ` [RFC/RFT 3/4] mt76x02: do not set protection on set_rts_threshold callback Stanislaw Gruszka
2018-11-09  9:23   ` Lorenzo Bianconi
2018-12-04 10:45     ` Stanislaw Gruszka
2018-12-04 11:50       ` Stanislaw Gruszka
2018-11-08 14:47 ` [RFC/RFT 4/4] mt76x02: set protection according to ht capabilities Stanislaw Gruszka
2018-11-09  9:29   ` Lorenzo Bianconi

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