[PATCH 0/4] run mt76x02_edcca_init atomically

[PATCH 0/4] run mt76x02_edcca_init atomically

[Lorenzo Bianconi]

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

-- 
2.20.1

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

[Lorenzo Bianconi]

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

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 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(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76x0/main.c b/drivers/net/wireless/mediatek/mt76/mt76x0/main.c
index 691984037f98..800ebbfc3055 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x0/main.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x0/main.c
@@ -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);
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_debugfs.c b/drivers/net/wireless/mediatek/mt76/mt76x02_debugfs.c
index b1d6fd4861e3..7853078e8ca4 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_debugfs.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_debugfs.c
@@ -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;
 }
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_dfs.c b/drivers/net/wireless/mediatek/mt76/mt76x02_dfs.c
index 17d12d212d1b..84b845647881 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_dfs.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_dfs.c
@@ -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);
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
index 56510a1a843a..ee4a86971be7 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
@@ -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;
 
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.h b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.h
index e4a9e0d0924b..cb39da79527a 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.h
@@ -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
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x2/pci_phy.c b/drivers/net/wireless/mediatek/mt76/mt76x2/pci_phy.c
index cc1aebcb0696..7a39a390a7ac 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x2/pci_phy.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x2/pci_phy.c
@@ -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;
 }
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x2/usb_main.c b/drivers/net/wireless/mediatek/mt76/mt76x2/usb_main.c
index 97bcf6494ec1..6657693edc3e 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x2/usb_main.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x2/usb_main.c
@@ -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);
 
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x2/usb_phy.c b/drivers/net/wireless/mediatek/mt76/mt76x2/usb_phy.c
index 07f67cb6854c..c7208c5375ac 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x2/usb_phy.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x2/usb_phy.c
@@ -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;
 }
-- 
2.20.1

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

[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 <lorenzo@kernel.org>
---
 drivers/net/wireless/mediatek/mt76/mt76x2/usb_main.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76x2/usb_main.c b/drivers/net/wireless/mediatek/mt76/mt76x2/usb_main.c
index 6657693edc3e..3351b736603d 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x2/usb_main.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x2/usb_main.c
@@ -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);
 
-- 
2.20.1

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

[Lorenzo Bianconi]

This is a preliminary patch to run mt76x02_edcca_init atomically

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 .../wireless/mediatek/mt76/mt76x2/pci_main.c  | 16 ++++++++------
 .../wireless/mediatek/mt76/mt76x2/usb_main.c  | 22 ++++++++++---------
 2 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76x2/pci_main.c b/drivers/net/wireless/mediatek/mt76/mt76x2/pci_main.c
index e416eee6a306..3a1467326f4d 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x2/pci_main.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x2/pci_main.c
@@ -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;
 }
 
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x2/usb_main.c b/drivers/net/wireless/mediatek/mt76/mt76x2/usb_main.c
index 3351b736603d..e4dfc3bea3c5 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x2/usb_main.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x2/usb_main.c
@@ -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;
 }
 
-- 
2.20.1

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

[Stanislaw Gruszka]

On Sat, May 11, 2019 at 12:17:53PM +0200, Lorenzo Bianconi wrote:
> This is a preliminary patch to run mt76x02_edcca_init atomically
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
>  .../wireless/mediatek/mt76/mt76x2/pci_main.c  | 16 ++++++++------
>  .../wireless/mediatek/mt76/mt76x2/usb_main.c  | 22 ++++++++++---------
>  2 files changed, 21 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76x2/pci_main.c b/drivers/net/wireless/mediatek/mt76/mt76x2/pci_main.c
> index e416eee6a306..3a1467326f4d 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76x2/pci_main.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt76x2/pci_main.c
> @@ -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

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

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 1380 bytes --]

> On Sat, May 11, 2019 at 12:17:53PM +0200, Lorenzo Bianconi wrote:
> > This is a preliminary patch to run mt76x02_edcca_init atomically
> > 
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > ---
> >  .../wireless/mediatek/mt76/mt76x2/pci_main.c  | 16 ++++++++------
> >  .../wireless/mediatek/mt76/mt76x2/usb_main.c  | 22 ++++++++++---------
> >  2 files changed, 21 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt76x2/pci_main.c b/drivers/net/wireless/mediatek/mt76/mt76x2/pci_main.c
> > index e416eee6a306..3a1467326f4d 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt76x2/pci_main.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt76x2/pci_main.c
> > @@ -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.

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

> 
> Stanislaw

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Felix Fietkau]

On 2019-05-13 11:19, Lorenzo Bianconi wrote:
>> On Sat, May 11, 2019 at 12:17:53PM +0200, Lorenzo Bianconi wrote:
>> > This is a preliminary patch to run mt76x02_edcca_init atomically
>> > 
>> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
>> > ---
>> >  .../wireless/mediatek/mt76/mt76x2/pci_main.c  | 16 ++++++++------
>> >  .../wireless/mediatek/mt76/mt76x2/usb_main.c  | 22 ++++++++++---------
>> >  2 files changed, 21 insertions(+), 17 deletions(-)
>> > 
>> > diff --git a/drivers/net/wireless/mediatek/mt76/mt76x2/pci_main.c b/drivers/net/wireless/mediatek/mt76/mt76x2/pci_main.c
>> > index e416eee6a306..3a1467326f4d 100644
>> > --- a/drivers/net/wireless/mediatek/mt76/mt76x2/pci_main.c
>> > +++ b/drivers/net/wireless/mediatek/mt76/mt76x2/pci_main.c
>> > @@ -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.
> 
> 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

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

[Stanislaw Gruszka]

On Mon, May 13, 2019 at 11:19:06AM +0200, Lorenzo Bianconi wrote:
> > On Sat, May 11, 2019 at 12:17:53PM +0200, Lorenzo Bianconi wrote:
> > > This is a preliminary patch to run mt76x02_edcca_init atomically
> > > 
> > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > > ---
> > >  .../wireless/mediatek/mt76/mt76x2/pci_main.c  | 16 ++++++++------
> > >  .../wireless/mediatek/mt76/mt76x2/usb_main.c  | 22 ++++++++++---------
> > >  2 files changed, 21 insertions(+), 17 deletions(-)
> > > 
> > > diff --git a/drivers/net/wireless/mediatek/mt76/mt76x2/pci_main.c b/drivers/net/wireless/mediatek/mt76/mt76x2/pci_main.c
> > > index e416eee6a306..3a1467326f4d 100644
> > > --- a/drivers/net/wireless/mediatek/mt76/mt76x2/pci_main.c
> > > +++ b/drivers/net/wireless/mediatek/mt76/mt76x2/pci_main.c
> > > @@ -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.
> 
> 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

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

[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 <lorenzo@kernel.org>
---
 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(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76x0/main.c b/drivers/net/wireless/mediatek/mt76/mt76x0/main.c
index 800ebbfc3055..115961a054e3 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x0/main.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x0/main.c
@@ -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);
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02.h b/drivers/net/wireless/mediatek/mt76/mt76x02.h
index f7fd53a1738a..e028c1a4cf88 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02.h
@@ -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 */
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_debugfs.c b/drivers/net/wireless/mediatek/mt76/mt76x02_debugfs.c
index 7853078e8ca4..501794a6076b 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_debugfs.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_debugfs.c
@@ -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;
 }
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_dfs.c b/drivers/net/wireless/mediatek/mt76/mt76x02_dfs.c
index 84b845647881..e372621c3798 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_dfs.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_dfs.c
@@ -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);
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
index ee4a86971be7..ac29422b5335 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
@@ -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
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.h b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.h
index cb39da79527a..ce73c60c579a 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.h
@@ -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
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x2/pci_phy.c b/drivers/net/wireless/mediatek/mt76/mt76x2/pci_phy.c
index 7a39a390a7ac..818c4f051df3 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x2/pci_phy.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x2/pci_phy.c
@@ -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);
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x2/usb_phy.c b/drivers/net/wireless/mediatek/mt76/mt76x2/usb_phy.c
index c7208c5375ac..2576654f2920 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x2/usb_phy.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x2/usb_phy.c
@@ -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)) {
-- 
2.20.1

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

[Felix Fietkau]

On 2019-05-11 12:17, Lorenzo Bianconi wrote:
> 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 <lorenzo@kernel.org>
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

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

[Lorenzo Bianconi]

>
> On 2019-05-11 12:17, Lorenzo Bianconi wrote:
> > 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 <lorenzo@kernel.org>
> 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

>
> - Felix

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

[Felix Fietkau]

On 2019-05-11 16:29, Lorenzo Bianconi wrote:
>>
>> On 2019-05-11 12:17, Lorenzo Bianconi wrote:
>> > 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 <lorenzo@kernel.org>
>> 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?
Right.

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

Thanks,

- Felix