| Message ID | 1527e88fc4a307aa218f515811f2f2c15786caec.1557567465.git.lorenzo@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Felix Fietkau |
| Headers | show |
| Series | run mt76x02_edcca_init atomically | expand |
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
> 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
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
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
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; }
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(-)