diff mbox series

[RFC/RFT,3/4] mt76x02: do not set protection on set_rts_threshold callback

Message ID 1541688430-31855-4-git-send-email-sgruszka@redhat.com (mailing list archive)
State RFC
Delegated to: Kalle Valo
Headers show
Series restore some old mt76x0u behaviour | expand

Commit Message

Stanislaw Gruszka Nov. 8, 2018, 2:47 p.m. UTC
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(-)

Comments

Lorenzo Bianconi Nov. 9, 2018, 9:23 a.m. UTC | #1
> 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

>  }
Stanislaw Gruszka Dec. 4, 2018, 10:45 a.m. UTC | #2
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
Stanislaw Gruszka Dec. 4, 2018, 11:50 a.m. UTC | #3
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
diff mbox series

Patch

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;