run mt76x02_edcca_init atomically

PATCH In review Lorenzo Bianconi 4 participants 12 messages May 11, 2019 – May 13, 2019

Subsystems (2)

Arm MediaTek MediaTek MT76 wireless LAN driver

lore

Share & tools

lore

Lorenzo Bianconi

author maintainer

2019-05-11 10:18 UTC #0 lore raw

cover run mt76x02_edcca_init atomically

Run mt76x02_edcca_init atomically in mt76_edcca_set since it is concurrent with mt76x2_set_channel/mt76x2u_set_channel and channel calibration

Lorenzo Bianconi (4):
mt76: mt76x02: remove enable from mt76x02_edcca_init signature
mt76: mt76x2u: remove mt76x02_edcca_init in mt76x2u_set_channel
mt76: mt76x2: move mutex_lock inside mt76x2_set_channel
mt76: mt76x02: run mt76x02_edcca_init atomically in mt76_edcca_set

 .../net/wireless/mediatek/mt76/mt76x0/main.c  |  2 +-
 drivers/net/wireless/mediatek/mt76/mt76x02.h  |  7 ++++++
 .../wireless/mediatek/mt76/mt76x02_debugfs.c  |  6 ++++-
 .../net/wireless/mediatek/mt76/mt76x02_dfs.c  |  2 +-
 .../net/wireless/mediatek/mt76/mt76x02_mac.c  |  6 ++---
 .../net/wireless/mediatek/mt76/mt76x02_mac.h  |  1 -
 .../wireless/mediatek/mt76/mt76x2/pci_main.c  | 16 +++++++------
 .../wireless/mediatek/mt76/mt76x2/pci_phy.c   | 15 ++++++++----
 .../wireless/mediatek/mt76/mt76x2/usb_main.c  | 23 ++++++++++---------
 .../wireless/mediatek/mt76/mt76x2/usb_phy.c   | 15 ++++++++----
 10 files changed, 58 insertions(+), 35 deletions(-)

Signature · 1 line

2.20.1

Lorenzo Bianconi

author maintainer

2019-05-11 10:18 UTC #1 lore raw

PATCH 1/4 mt76: mt76x02: remove enable from mt76x02_edcca_init signature

Remove enable parameter from mt76x02_edcca_init routine signature since it is always true

Signed-off-by Lorenzo Bianconi <[email protected]>

---
 drivers/net/wireless/mediatek/mt76/mt76x0/main.c     | 2 +-
 drivers/net/wireless/mediatek/mt76/mt76x02_debugfs.c | 2 +-
 drivers/net/wireless/mediatek/mt76/mt76x02_dfs.c     | 2 +-
 drivers/net/wireless/mediatek/mt76/mt76x02_mac.c     | 4 ++--
 drivers/net/wireless/mediatek/mt76/mt76x02_mac.h     | 2 +-
 drivers/net/wireless/mediatek/mt76/mt76x2/pci_phy.c  | 2 +-
 drivers/net/wireless/mediatek/mt76/mt76x2/usb_main.c | 2 +-
 drivers/net/wireless/mediatek/mt76/mt76x2/usb_phy.c  | 2 +-
 8 files changed, 9 insertions(+), 9 deletions(-)

drivers/net/wireless/mediatek/mt76/mt76x0/main.c +1 -1

index 691984037f98..800ebbfc3055 100644
@@ -33,7 +33,7 @@ mt76x0_set_channel(struct mt76x02_dev *dev, struct cfg80211_chan_def *chandef)
 	mt76_rr(dev, MT_CH_IDLE);
 	mt76_rr(dev, MT_CH_BUSY);
 
-	mt76x02_edcca_init(dev, true);
+	mt76x02_edcca_init(dev);
 
 	if (mt76_is_mmio(dev)) {
 		mt76x02_dfs_init_params(dev);

drivers/net/wireless/mediatek/mt76/mt76x02_debugfs.c +1 -1

index b1d6fd4861e3..7853078e8ca4 100644
@@ -125,7 +125,7 @@ mt76_edcca_set(void *data, u64 val)
 	dev->ed_monitor_enabled = !!val;
 	dev->ed_monitor = dev->ed_monitor_enabled &&
 			  region == NL80211_DFS_ETSI;
-	mt76x02_edcca_init(dev, true);
+	mt76x02_edcca_init(dev);
 
 	return 0;
 }

drivers/net/wireless/mediatek/mt76/mt76x02_dfs.c +1 -1

index 17d12d212d1b..84b845647881 100644
@@ -887,7 +887,7 @@ mt76x02_dfs_set_domain(struct mt76x02_dev *dev,
 
 		dev->ed_monitor = dev->ed_monitor_enabled &&
 				  region == NL80211_DFS_ETSI;
-		mt76x02_edcca_init(dev, true);
+		mt76x02_edcca_init(dev);
 
 		dfs_pd->region = region;
 		mt76x02_dfs_init_params(dev);

drivers/net/wireless/mediatek/mt76/mt76x02_mac.c +2 -2

index 56510a1a843a..ee4a86971be7 100644
@@ -945,12 +945,12 @@ mt76x02_edcca_tx_enable(struct mt76x02_dev *dev, bool enable)
 	dev->ed_tx_blocked = !enable;
 }
 
-void mt76x02_edcca_init(struct mt76x02_dev *dev, bool enable)
+void mt76x02_edcca_init(struct mt76x02_dev *dev)
 {
 	dev->ed_trigger = 0;
 	dev->ed_silent = 0;
 
-	if (dev->ed_monitor && enable) {
+	if (dev->ed_monitor) {
 		struct ieee80211_channel *chan = dev->mt76.chandef.chan;
 		u8 ed_th = chan->band == NL80211_BAND_5GHZ ? 0x0e : 0x20;

drivers/net/wireless/mediatek/mt76/mt76x02_mac.h +1 -1

index e4a9e0d0924b..cb39da79527a 100644
@@ -209,5 +209,5 @@ int mt76x02_mac_set_beacon(struct mt76x02_dev *dev, u8 vif_idx,
 void mt76x02_mac_set_beacon_enable(struct mt76x02_dev *dev,
 				   struct ieee80211_vif *vif, bool val);
 
-void mt76x02_edcca_init(struct mt76x02_dev *dev, bool enable);
+void mt76x02_edcca_init(struct mt76x02_dev *dev);
 #endif

drivers/net/wireless/mediatek/mt76/mt76x2/pci_phy.c +1 -1

index cc1aebcb0696..7a39a390a7ac 100644
@@ -74,7 +74,7 @@ mt76x2_phy_channel_calibrate(struct mt76x02_dev *dev, bool mac_stopped)
 		mt76x2_mac_resume(dev);
 
 	mt76x2_apply_gain_adj(dev);
-	mt76x02_edcca_init(dev, true);
+	mt76x02_edcca_init(dev);
 
 	dev->cal.channel_cal_done = true;
 }

drivers/net/wireless/mediatek/mt76/mt76x2/usb_main.c +1 -1

index 97bcf6494ec1..6657693edc3e 100644
@@ -59,7 +59,7 @@ mt76x2u_set_channel(struct mt76x02_dev *dev,
 	err = mt76x2u_phy_set_channel(dev, chandef);
 
 	mt76x2_mac_resume(dev);
-	mt76x02_edcca_init(dev, true);
+	mt76x02_edcca_init(dev);
 
 	dev->beacon_ops->pre_tbtt_enable(dev, true);

drivers/net/wireless/mediatek/mt76/mt76x2/usb_phy.c +1 -1

index 07f67cb6854c..c7208c5375ac 100644
@@ -45,7 +45,7 @@ mt76x2u_phy_channel_calibrate(struct mt76x02_dev *dev, bool mac_stopped)
 	if (!mac_stopped)
 		mt76x2_mac_resume(dev);
 	mt76x2_apply_gain_adj(dev);
-	mt76x02_edcca_init(dev, true);
+	mt76x02_edcca_init(dev);
 
 	dev->cal.channel_cal_done = true;
 }

Signature · 1 line

2.20.1

Lorenzo Bianconi

author maintainer

2019-05-11 10:18 UTC #2 lore raw

PATCH 2/4 mt76: mt76x2u: remove mt76x02_edcca_init in mt76x2u_set_channel

In reply to Lorenzo Bianconi

Remove mt76x02_edcca_init in mt76x2u_set_channel since it is already run by mt76x2u_phy_channel_calibrate performing channel calibration

Signed-off-by Lorenzo Bianconi <[email protected]>

---
 drivers/net/wireless/mediatek/mt76/mt76x2/usb_main.c | 1 -
 1 file changed, 1 deletion(-)

drivers/net/wireless/mediatek/mt76/mt76x2/usb_main.c +0 -1

index 6657693edc3e..3351b736603d 100644
@@ -59,7 +59,6 @@ mt76x2u_set_channel(struct mt76x02_dev *dev,
 	err = mt76x2u_phy_set_channel(dev, chandef);
 
 	mt76x2_mac_resume(dev);
-	mt76x02_edcca_init(dev);
 
 	dev->beacon_ops->pre_tbtt_enable(dev, true);

Signature · 1 line

2.20.1

Lorenzo Bianconi

author maintainer

2019-05-11 10:18 UTC #3 lore raw

PATCH 3/4 mt76: mt76x2: move mutex_lock inside mt76x2_set_channel

In reply to Lorenzo Bianconi

This is a preliminary patch to run mt76x02_edcca_init atomically

Signed-off-by Lorenzo Bianconi <[email protected]>

---
 .../wireless/mediatek/mt76/mt76x2/pci_main.c  | 16 ++++++++------
 .../wireless/mediatek/mt76/mt76x2/usb_main.c  | 22 ++++++++++---------
 2 files changed, 21 insertions(+), 17 deletions(-)

drivers/net/wireless/mediatek/mt76/mt76x2/pci_main.c +9 -7

index e416eee6a306..3a1467326f4d 100644
@@ -54,14 +54,14 @@ mt76x2_set_channel(struct mt76x02_dev *dev, struct cfg80211_chan_def *chandef)
 	int ret;
 
 	cancel_delayed_work_sync(&dev->cal_work);
+	tasklet_disable(&dev->mt76.pre_tbtt_tasklet);
+	tasklet_disable(&dev->dfs_pd.dfs_tasklet);
 
+	mutex_lock(&dev->mt76.mutex);
 	set_bit(MT76_RESET, &dev->mt76.state);
 
 	mt76_set_channel(&dev->mt76);
 
-	tasklet_disable(&dev->mt76.pre_tbtt_tasklet);
-	tasklet_disable(&dev->dfs_pd.dfs_tasklet);
-
 	mt76x2_mac_stop(dev, true);
 	ret = mt76x2_phy_set_channel(dev, chandef);

@@ -72,10 +72,12 @@ mt76x2_set_channel(struct mt76x02_dev *dev, struct cfg80211_chan_def *chandef)
 	mt76x02_dfs_init_params(dev);
 
 	mt76x2_mac_resume(dev);
-	tasklet_enable(&dev->dfs_pd.dfs_tasklet);
-	tasklet_enable(&dev->mt76.pre_tbtt_tasklet);
 
 	clear_bit(MT76_RESET, &dev->mt76.state);
+	mutex_unlock(&dev->mt76.mutex);
+
+	tasklet_enable(&dev->dfs_pd.dfs_tasklet);
+	tasklet_enable(&dev->mt76.pre_tbtt_tasklet);
 
 	mt76_txq_schedule_all(&dev->mt76);

@@ -111,14 +113,14 @@ mt76x2_config(struct ieee80211_hw *hw, u32 changed)
 		}
 	}
 
+	mutex_unlock(&dev->mt76.mutex);
+
 	if (changed & IEEE80211_CONF_CHANGE_CHANNEL) {
 		ieee80211_stop_queues(hw);
 		ret = mt76x2_set_channel(dev, &hw->conf.chandef);
 		ieee80211_wake_queues(hw);
 	}
 
-	mutex_unlock(&dev->mt76.mutex);
-
 	return ret;
 }

drivers/net/wireless/mediatek/mt76/mt76x2/usb_main.c +12 ~~-10~~

index 3351b736603d..e4dfc3bea3c5 100644
@@ -48,21 +48,23 @@ mt76x2u_set_channel(struct mt76x02_dev *dev,
 	int err;
 
 	cancel_delayed_work_sync(&dev->cal_work);
+	dev->beacon_ops->pre_tbtt_enable(dev, false);
+
+	mutex_lock(&dev->mt76.mutex);
 	set_bit(MT76_RESET, &dev->mt76.state);
 
 	mt76_set_channel(&dev->mt76);
 
-	dev->beacon_ops->pre_tbtt_enable(dev, false);
-
 	mt76x2_mac_stop(dev, false);
 
 	err = mt76x2u_phy_set_channel(dev, chandef);
 
 	mt76x2_mac_resume(dev);
 
-	dev->beacon_ops->pre_tbtt_enable(dev, true);
-
 	clear_bit(MT76_RESET, &dev->mt76.state);
+	mutex_unlock(&dev->mt76.mutex);
+
+	dev->beacon_ops->pre_tbtt_enable(dev, true);
 	mt76_txq_schedule_all(&dev->mt76);
 
 	return err;

@@ -84,12 +86,6 @@ mt76x2u_config(struct ieee80211_hw *hw, u32 changed)
 		mt76_wr(dev, MT_RX_FILTR_CFG, dev->mt76.rxfilter);
 	}
 
-	if (changed & IEEE80211_CONF_CHANGE_CHANNEL) {
-		ieee80211_stop_queues(hw);
-		err = mt76x2u_set_channel(dev, &hw->conf.chandef);
-		ieee80211_wake_queues(hw);
-	}
-
 	if (changed & IEEE80211_CONF_CHANGE_POWER) {
 		dev->mt76.txpower_conf = hw->conf.power_level * 2;

@@ -102,6 +98,12 @@ mt76x2u_config(struct ieee80211_hw *hw, u32 changed)
 
 	mutex_unlock(&dev->mt76.mutex);
 
+	if (changed & IEEE80211_CONF_CHANGE_CHANNEL) {
+		ieee80211_stop_queues(hw);
+		err = mt76x2u_set_channel(dev, &hw->conf.chandef);
+		ieee80211_wake_queues(hw);
+	}
+
 	return err;
 }

Signature · 1 line

2.20.1

Stanislaw Gruszka

2019-05-13 08:38 UTC #4 lore raw

Re: [PATCH 3/4] mt76: mt76x2: move mutex_lock inside mt76x2_set_channel

Quoted 13 lines · Lorenzo Bianconi

This is a preliminary patch to run mt76x02_edcca_init atomically
Signed-off-by Lorenzo Bianconi <[email protected]>
---
 .../wireless/mediatek/mt76/mt76x2/pci_main.c  | 16 ++++++++------
 .../wireless/mediatek/mt76/mt76x2/usb_main.c  | 22 ++++++++++---------
 2 files changed, 21 insertions(+), 17 deletions(-)
drivers/net/wireless/mediatek/mt76/mt76x2/pci_main.c +0 -0
index e416eee6a306..3a1467326f4d 100644

drivers/net/wireless/mediatek/mt76/mt76x2/pci_main.c mt76x2_set_channel(struct mt76x02_dev *dev, struct cfg80211_chan_def *chandef) · line 56

Quoted 3 lines

drivers/net/wireless/mediatek/mt76/mt76x2/pci_main.c +0 -0

@@ -54,14 +54,14 @@ mt76x2_set_channel(struct mt76x02_dev *dev, struct cfg80211_chan_def *chandef)
 	int ret;
 
 	cancel_delayed_work_sync(&dev->cal_work);

Since now you use mutex in mt76x2_phy_calibrate() you can remove cancel_delayed_work_sync() and drop other changes from this patch as releasing mutex just to acquire it in almost next step make no sense.

Stanislaw

Lorenzo Bianconi

author maintainer

2019-05-13 09:19 UTC #5 lore raw

Re: [PATCH 3/4] mt76: mt76x2: move mutex_lock inside mt76x2_set_channel

Quoted 16 lines · Lorenzo Bianconi

Quoted 15 lines
This is a preliminary patch to run mt76x02_edcca_init atomically
Signed-off-by Lorenzo Bianconi <[email protected]>
---
 .../wireless/mediatek/mt76/mt76x2/pci_main.c  | 16 ++++++++------
 .../wireless/mediatek/mt76/mt76x2/usb_main.c  | 22 ++++++++++---------
 2 files changed, 21 insertions(+), 17 deletions(-)
drivers/net/wireless/mediatek/mt76/mt76x2/pci_main.c +0 -0
index e416eee6a306..3a1467326f4d 100644
@@ -54,14 +54,14 @@ mt76x2_set_channel(struct mt76x02_dev *dev, struct cfg80211_chan_def *chandef)
 	int ret;
 

Quoted 6 lines

drivers/net/wireless/mediatek/mt76/mt76x2/pci_main.c mt76x2_set_channel(struct mt76x02_dev *dev, struct cfg80211_chan_def *chandef) · line 56
Quoted 1 line
drivers/net/wireless/mediatek/mt76/mt76x2/pci_main.c +0 -0
 	cancel_delayed_work_sync(&dev->cal_work);
Since now you use mutex in mt76x2_phy_calibrate() you can remove cancel_delayed_work_sync() and drop other changes from this patch as releasing mutex just to acquire it in almost next step make no sense.

I agree with you, the only difference is in that way we will perform phy calibration even during scanning. If the there are no objections I will post a v3 removing cancel_delayed_work_sync and reworking patch 3/4

Regards,
Lorenzo

Quoted 2 lines

Stanislaw

Felix Fietkau

maintainer

2019-05-13 09:30 UTC #6 lore raw

Re: [PATCH 3/4] mt76: mt76x2: move mutex_lock inside mt76x2_set_channel

Quoted 23 lines · Lorenzo Bianconi

Quoted 16 lines · Lorenzo Bianconi
Quoted 15 lines
This is a preliminary patch to run mt76x02_edcca_init atomically
Signed-off-by Lorenzo Bianconi <[email protected]>
---
 .../wireless/mediatek/mt76/mt76x2/pci_main.c  | 16 ++++++++------
 .../wireless/mediatek/mt76/mt76x2/usb_main.c  | 22 ++++++++++---------
 2 files changed, 21 insertions(+), 17 deletions(-)
drivers/net/wireless/mediatek/mt76/mt76x2/pci_main.c +0 -0
index e416eee6a306..3a1467326f4d 100644
@@ -54,14 +54,14 @@ mt76x2_set_channel(struct mt76x02_dev *dev, struct cfg80211_chan_def *chandef)
 	int ret;
 
Quoted 6 lines
drivers/net/wireless/mediatek/mt76/mt76x2/pci_main.c mt76x2_set_channel(struct mt76x02_dev *dev, struct cfg80211_chan_def *chandef) · line 56
Quoted 1 line
drivers/net/wireless/mediatek/mt76/mt76x2/pci_main.c +0 -0
 	cancel_delayed_work_sync(&dev->cal_work);
Since now you use mutex in mt76x2_phy_calibrate() you can remove cancel_delayed_work_sync() and drop other changes from this patch as releasing mutex just to acquire it in almost next step make no sense.

Quoted 4 lines

I agree with you, the only difference is in that way we will perform phy calibration even during scanning. If the there are no objections I will post a v3 removing cancel_delayed_work_sync and reworking patch 3/4

I don't agree for two reasons:

1. If we only rely on the mutex, we're blocking the workqueue. That might have some unwanted side effects.
2. We really should avoid having the calibration work during scanning, otherwise this creates extra latency on channel changes, making the whole scan slower.

- Felix

Stanislaw Gruszka

2019-05-13 09:41 UTC #7 lore raw

Re: [PATCH 3/4] mt76: mt76x2: move mutex_lock inside mt76x2_set_channel

In reply to Lorenzo Bianconi

Quoted 23 lines · Lorenzo Bianconi

Quoted 16 lines · Lorenzo Bianconi
Quoted 15 lines
This is a preliminary patch to run mt76x02_edcca_init atomically
Signed-off-by Lorenzo Bianconi <[email protected]>
---
 .../wireless/mediatek/mt76/mt76x2/pci_main.c  | 16 ++++++++------
 .../wireless/mediatek/mt76/mt76x2/usb_main.c  | 22 ++++++++++---------
 2 files changed, 21 insertions(+), 17 deletions(-)
drivers/net/wireless/mediatek/mt76/mt76x2/pci_main.c +0 -0
index e416eee6a306..3a1467326f4d 100644
@@ -54,14 +54,14 @@ mt76x2_set_channel(struct mt76x02_dev *dev, struct cfg80211_chan_def *chandef)
 	int ret;
 
Quoted 6 lines
drivers/net/wireless/mediatek/mt76/mt76x2/pci_main.c mt76x2_set_channel(struct mt76x02_dev *dev, struct cfg80211_chan_def *chandef) · line 56
Quoted 1 line
drivers/net/wireless/mediatek/mt76/mt76x2/pci_main.c +0 -0
 	cancel_delayed_work_sync(&dev->cal_work);
Since now you use mutex in mt76x2_phy_calibrate() you can remove cancel_delayed_work_sync() and drop other changes from this patch as releasing mutex just to acquire it in almost next step make no sense.

Quoted 4 lines

I agree with you, the only difference is in that way we will perform phy calibration even during scanning. If the there are no objections I will post a v3 removing cancel_delayed_work_sync and reworking patch 3/4

Oh, calibration work should not be done during scanning, so cancel cal_work should be added to .sw_scan_start() callback or this patch should stay unchanged.

Stanislaw

Lorenzo Bianconi

author maintainer

2019-05-11 10:18 UTC #8 lore raw

PATCH 4/4 mt76: mt76x02: run mt76x02_edcca_init atomically in mt76_edcca_set

In reply to Lorenzo Bianconi

Run mt76x02_edcca_init atomically in mt76_edcca_set since it runs concurrently with calibration work and mt76x2_set_channel. Introduce __mt76x02_edcca_init helper routine

Signed-off-by Lorenzo Bianconi <[email protected]>

---
 drivers/net/wireless/mediatek/mt76/mt76x0/main.c  |  2 +-
 drivers/net/wireless/mediatek/mt76/mt76x02.h      |  7 +++++++
 .../net/wireless/mediatek/mt76/mt76x02_debugfs.c  |  6 +++++-
 drivers/net/wireless/mediatek/mt76/mt76x02_dfs.c  |  2 +-
 drivers/net/wireless/mediatek/mt76/mt76x02_mac.c  |  4 ++--
 drivers/net/wireless/mediatek/mt76/mt76x02_mac.h  |  1 -
 .../net/wireless/mediatek/mt76/mt76x2/pci_phy.c   | 15 ++++++++++-----
 .../net/wireless/mediatek/mt76/mt76x2/usb_phy.c   | 15 ++++++++++-----
 8 files changed, 36 insertions(+), 16 deletions(-)

drivers/net/wireless/mediatek/mt76/mt76x0/main.c +1 -1

index 800ebbfc3055..115961a054e3 100644
@@ -33,7 +33,7 @@ mt76x0_set_channel(struct mt76x02_dev *dev, struct cfg80211_chan_def *chandef)
 	mt76_rr(dev, MT_CH_IDLE);
 	mt76_rr(dev, MT_CH_BUSY);
 
-	mt76x02_edcca_init(dev);
+	__mt76x02_edcca_init(dev);
 
 	if (mt76_is_mmio(dev)) {
 		mt76x02_dfs_init_params(dev);

drivers/net/wireless/mediatek/mt76/mt76x02.h +7 -0

index f7fd53a1738a..e028c1a4cf88 100644
@@ -268,4 +268,11 @@ mt76x02_rx_get_sta_wcid(struct mt76x02_sta *sta, bool unicast)
 		return &sta->vif->group_wcid;
 }
 
+void __mt76x02_edcca_init(struct mt76x02_dev *dev);
+static inline void mt76x02_edcca_init(struct mt76x02_dev *dev)
+{
+	mutex_lock(&dev->mt76.mutex);
+	__mt76x02_edcca_init(dev);
+	mutex_unlock(&dev->mt76.mutex);
+}
 #endif /* __MT76x02_H */

drivers/net/wireless/mediatek/mt76/mt76x02_debugfs.c +5 -1

index 7853078e8ca4..501794a6076b 100644
@@ -122,10 +122,14 @@ mt76_edcca_set(void *data, u64 val)
 	struct mt76x02_dev *dev = data;
 	enum nl80211_dfs_regions region = dev->dfs_pd.region;
 
+	mutex_lock(&dev->mt76.mutex);
+
 	dev->ed_monitor_enabled = !!val;
 	dev->ed_monitor = dev->ed_monitor_enabled &&
 			  region == NL80211_DFS_ETSI;
-	mt76x02_edcca_init(dev);
+	__mt76x02_edcca_init(dev);
+
+	mutex_unlock(&dev->mt76.mutex);
 
 	return 0;
 }

drivers/net/wireless/mediatek/mt76/mt76x02_dfs.c +1 -1

index 84b845647881..e372621c3798 100644
@@ -887,7 +887,7 @@ mt76x02_dfs_set_domain(struct mt76x02_dev *dev,
 
 		dev->ed_monitor = dev->ed_monitor_enabled &&
 				  region == NL80211_DFS_ETSI;
-		mt76x02_edcca_init(dev);
+		__mt76x02_edcca_init(dev);
 
 		dfs_pd->region = region;
 		mt76x02_dfs_init_params(dev);

drivers/net/wireless/mediatek/mt76/mt76x02_mac.c +2 -2

index ee4a86971be7..ac29422b5335 100644
@@ -945,7 +945,7 @@ mt76x02_edcca_tx_enable(struct mt76x02_dev *dev, bool enable)
 	dev->ed_tx_blocked = !enable;
 }
 
-void mt76x02_edcca_init(struct mt76x02_dev *dev)
+void __mt76x02_edcca_init(struct mt76x02_dev *dev)
 {
 	dev->ed_trigger = 0;
 	dev->ed_silent = 0;

@@ -979,7 +979,7 @@ void mt76x02_edcca_init(struct mt76x02_dev *dev)
 	mt76_rr(dev, MT_ED_CCA_TIMER);
 	dev->ed_time = ktime_get_boottime();
 }
-EXPORT_SYMBOL_GPL(mt76x02_edcca_init);
+EXPORT_SYMBOL_GPL(__mt76x02_edcca_init);
 
 #define MT_EDCCA_TH		92
 #define MT_EDCCA_BLOCK_TH	2

drivers/net/wireless/mediatek/mt76/mt76x02_mac.h +0 -1

index cb39da79527a..ce73c60c579a 100644
@@ -209,5 +209,4 @@ int mt76x02_mac_set_beacon(struct mt76x02_dev *dev, u8 vif_idx,
 void mt76x02_mac_set_beacon_enable(struct mt76x02_dev *dev,
 				   struct ieee80211_vif *vif, bool val);
 
-void mt76x02_edcca_init(struct mt76x02_dev *dev);
 #endif

drivers/net/wireless/mediatek/mt76/mt76x2/pci_phy.c +10 -5

index 7a39a390a7ac..818c4f051df3 100644
@@ -43,17 +43,17 @@ mt76x2_phy_tssi_init_cal(struct mt76x02_dev *dev)
 	return true;
 }
 
-static void
+static bool
 mt76x2_phy_channel_calibrate(struct mt76x02_dev *dev, bool mac_stopped)
 {
 	struct ieee80211_channel *chan = dev->mt76.chandef.chan;
 	bool is_5ghz = chan->band == NL80211_BAND_5GHZ;
 
 	if (dev->cal.channel_cal_done)
-		return;
+		return false;
 
 	if (mt76x2_channel_silent(dev))
-		return;
+		return false;
 
 	if (!dev->cal.tssi_cal_done)
 		mt76x2_phy_tssi_init_cal(dev);

@@ -74,9 +74,10 @@ mt76x2_phy_channel_calibrate(struct mt76x02_dev *dev, bool mac_stopped)
 		mt76x2_mac_resume(dev);
 
 	mt76x2_apply_gain_adj(dev);
-	mt76x02_edcca_init(dev);
 
 	dev->cal.channel_cal_done = true;
+
+	return true;
 }
 
 void mt76x2_phy_set_antenna(struct mt76x02_dev *dev)

@@ -245,6 +246,7 @@ int mt76x2_phy_set_channel(struct mt76x02_dev *dev,
 		return 0;
 
 	mt76x2_phy_channel_calibrate(dev, true);
+	__mt76x02_edcca_init(dev);
 	mt76x02_init_agc_gain(dev);
 
 	/* init default values for temp compensation */

@@ -292,9 +294,12 @@ mt76x2_phy_temp_compensate(struct mt76x02_dev *dev)
 void mt76x2_phy_calibrate(struct work_struct *work)
 {
 	struct mt76x02_dev *dev;
+	bool ret;
 
 	dev = container_of(work, struct mt76x02_dev, cal_work.work);
-	mt76x2_phy_channel_calibrate(dev, false);
+	ret = mt76x2_phy_channel_calibrate(dev, false);
+	if (ret)
+		mt76x02_edcca_init(dev);
 	mt76x2_phy_tssi_compensate(dev);
 	mt76x2_phy_temp_compensate(dev);
 	mt76x2_phy_update_channel_gain(dev);

drivers/net/wireless/mediatek/mt76/mt76x2/usb_phy.c +10 -5

index c7208c5375ac..2576654f2920 100644
@@ -18,17 +18,17 @@
 #include "eeprom.h"
 #include "../mt76x02_phy.h"
 
-static void
+static bool
 mt76x2u_phy_channel_calibrate(struct mt76x02_dev *dev, bool mac_stopped)
 {
 	struct ieee80211_channel *chan = dev->mt76.chandef.chan;
 	bool is_5ghz = chan->band == NL80211_BAND_5GHZ;
 
 	if (dev->cal.channel_cal_done)
-		return;
+		return false;
 
 	if (mt76x2_channel_silent(dev))
-		return;
+		return false;
 
 	if (!mac_stopped)
 		mt76x2u_mac_stop(dev);

@@ -45,17 +45,21 @@ mt76x2u_phy_channel_calibrate(struct mt76x02_dev *dev, bool mac_stopped)
 	if (!mac_stopped)
 		mt76x2_mac_resume(dev);
 	mt76x2_apply_gain_adj(dev);
-	mt76x02_edcca_init(dev);
 
 	dev->cal.channel_cal_done = true;
+
+	return true;
 }
 
 void mt76x2u_phy_calibrate(struct work_struct *work)
 {
 	struct mt76x02_dev *dev;
+	bool ret;
 
 	dev = container_of(work, struct mt76x02_dev, cal_work.work);
-	mt76x2u_phy_channel_calibrate(dev, false);
+	ret = mt76x2u_phy_channel_calibrate(dev, false);
+	if (ret)
+		mt76x02_edcca_init(dev);
 	mt76x2_phy_tssi_compensate(dev);
 	mt76x2_phy_update_channel_gain(dev);

@@ -177,6 +181,7 @@ int mt76x2u_phy_set_channel(struct mt76x02_dev *dev,
 		return 0;
 
 	mt76x2u_phy_channel_calibrate(dev, true);
+	__mt76x02_edcca_init(dev);
 	mt76x02_init_agc_gain(dev);
 
 	if (mt76x2_tssi_enabled(dev)) {

Signature · 1 line

2.20.1

Felix Fietkau

maintainer

2019-05-11 14:23 UTC #9 lore raw

Re: [PATCH 4/4] mt76: mt76x02: run mt76x02_edcca_init atomically in mt76_edcca_set

Quoted 5 lines · Lorenzo Bianconi

Run mt76x02_edcca_init atomically in mt76_edcca_set since it runs concurrently with calibration work and mt76x2_set_channel. Introduce __mt76x02_edcca_init helper routine
Signed-off-by Lorenzo Bianconi <[email protected]>

I don't think this is enough. To prevent issues with calibration, we probably need to hold the mutex for the duration of the calibration anyway. Otherwise it might get enabled right in the middle of it and screw things up.
Also, it probably simplifies the patch if you don't add the wrapper function that takes the mutex, and instead just explicitly take the mutex where needed.

- Felix

Lorenzo Bianconi

2019-05-11 14:29 UTC #10 lore raw

Re: [PATCH 4/4] mt76: mt76x02: run mt76x02_edcca_init atomically in mt76_edcca_set

Quoted 6 lines · Lorenzo Bianconi

Quoted 4 lines
Run mt76x02_edcca_init atomically in mt76_edcca_set since it runs concurrently with calibration work and mt76x2_set_channel. Introduce __mt76x02_edcca_init helper routine

Quoted 8 lines

Quoted 1 line
Signed-off-by Lorenzo Bianconi <[email protected]>
I don't think this is enough. To prevent issues with calibration, we probably need to hold the mutex for the duration of the calibration anyway. Otherwise it might get enabled right in the middle of it and screw things up.
Also, it probably simplifies the patch if you don't add the wrapper function that takes the mutex, and instead just explicitly take the mutex where needed.

So IIUC it would be better to hold the mutex during mt76x2_phy_calibrate processing, right?
If so, do I need to repost all the series or just this patch?

Regards,
Lorenzo

Quoted 2 lines

- Felix

Felix Fietkau

maintainer

2019-05-11 14:30 UTC #11 lore raw

Re: [PATCH 4/4] mt76: mt76x02: run mt76x02_edcca_init atomically in mt76_edcca_set

Quoted 15 lines · Lorenzo Bianconi

Quoted 6 lines · Lorenzo Bianconi
Quoted 4 lines
Run mt76x02_edcca_init atomically in mt76_edcca_set since it runs concurrently with calibration work and mt76x2_set_channel. Introduce __mt76x02_edcca_init helper routine
Quoted 8 lines
Quoted 1 line
Signed-off-by Lorenzo Bianconi <[email protected]>
I don't think this is enough. To prevent issues with calibration, we probably need to hold the mutex for the duration of the calibration anyway. Otherwise it might get enabled right in the middle of it and screw things up.
Also, it probably simplifies the patch if you don't add the wrapper function that takes the mutex, and instead just explicitly take the mutex where needed.

Quoted 2 lines

So IIUC it would be better to hold the mutex during mt76x2_phy_calibrate processing, right?

Right.

Quoted 1 line

If so, do I need to repost all the series or just this patch?

Feel free to repost just this patch.

Thanks,

- Felix