* [PATCH 0/2] qtnfmac: misc fixes intended for 4.14
@ 2017-09-15 11:52 Sergey Matyukevich
2017-09-15 11:52 ` [PATCH 1/2] qtnfmac: lock access to h/w in tx path Sergey Matyukevich
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Sergey Matyukevich @ 2017-09-15 11:52 UTC (permalink / raw)
To: linux-wireless; +Cc: Igor Mitsyanko, Avinash Patil
Hello Kalle, Igor, and all
Here are two patches intended for 4.14. The first patch fixes tx path
regression. Lock should be held when queuing packets to h/w fifos in order to
properly handle configurations with two enabled interfaces or multiple enabled
mbss. The second patch fixes scan issues addressing a review comment
from K.Valo regarding pending active scans upon device removal.
Regards,
Sergey
Sergey Matyukevich (2):
qtnfmac: lock access to h/w in tx path
qtnfmac: abort scans on wireless interface changes
cfg80211.c | 9 ++++++---
cfg80211.h | 3 +++
core.c | 2 +-
event.c | 2 --
pearl/pcie.c | 9 ++++++++-
pearl/pcie_bus_priv.h | 2 ++
6 files changed, 20 insertions(+), 7 deletions(-)
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 1/2] qtnfmac: lock access to h/w in tx path 2017-09-15 11:52 [PATCH 0/2] qtnfmac: misc fixes intended for 4.14 Sergey Matyukevich @ 2017-09-15 11:52 ` Sergey Matyukevich 2017-09-15 11:53 ` [PATCH 2/2] qtnfmac: abort scans on wireless interface changes Sergey Matyukevich 2017-09-18 4:52 ` [PATCH 0/2] qtnfmac: misc fixes intended for 4.14 Kalle Valo 2 siblings, 0 replies; 12+ messages in thread From: Sergey Matyukevich @ 2017-09-15 11:52 UTC (permalink / raw) To: linux-wireless; +Cc: Igor Mitsyanko, Avinash Patil, Sergey Matyukevich Fix tx path regression. Lock should be held when queuing packets to h/w fifos in order to properly handle configurations with multiple enabled interfaces. Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com> --- drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c | 9 ++++++++- drivers/net/wireless/quantenna/qtnfmac/pearl/pcie_bus_priv.h | 2 ++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c index 502e72b7cdcc..69131965a298 100644 --- a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c +++ b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c @@ -661,14 +661,18 @@ static int qtnf_pcie_data_tx(struct qtnf_bus *bus, struct sk_buff *skb) struct qtnf_pcie_bus_priv *priv = (void *)get_bus_priv(bus); dma_addr_t txbd_paddr, skb_paddr; struct qtnf_tx_bd *txbd; + unsigned long flags; int len, i; u32 info; int ret = 0; + spin_lock_irqsave(&priv->tx0_lock, flags); + if (!qtnf_tx_queue_ready(priv)) { if (skb->dev) netif_stop_queue(skb->dev); + spin_unlock_irqrestore(&priv->tx0_lock, flags); return NETDEV_TX_BUSY; } @@ -717,8 +721,10 @@ static int qtnf_pcie_data_tx(struct qtnf_bus *bus, struct sk_buff *skb) dev_kfree_skb_any(skb); } - qtnf_pcie_data_tx_reclaim(priv); priv->tx_done_count++; + spin_unlock_irqrestore(&priv->tx0_lock, flags); + + qtnf_pcie_data_tx_reclaim(priv); return NETDEV_TX_OK; } @@ -1247,6 +1253,7 @@ static int qtnf_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id) strcpy(bus->fwname, QTN_PCI_PEARL_FW_NAME); init_completion(&bus->request_firmware_complete); mutex_init(&bus->bus_lock); + spin_lock_init(&pcie_priv->tx0_lock); spin_lock_init(&pcie_priv->irq_lock); spin_lock_init(&pcie_priv->tx_reclaim_lock); diff --git a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie_bus_priv.h b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie_bus_priv.h index e76a23716ee0..86ac1ccedb52 100644 --- a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie_bus_priv.h +++ b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie_bus_priv.h @@ -34,6 +34,8 @@ struct qtnf_pcie_bus_priv { /* lock for tx reclaim operations */ spinlock_t tx_reclaim_lock; + /* lock for tx0 operations */ + spinlock_t tx0_lock; u8 msi_enabled; int mps; -- 2.11.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/2] qtnfmac: abort scans on wireless interface changes 2017-09-15 11:52 [PATCH 0/2] qtnfmac: misc fixes intended for 4.14 Sergey Matyukevich 2017-09-15 11:52 ` [PATCH 1/2] qtnfmac: lock access to h/w in tx path Sergey Matyukevich @ 2017-09-15 11:53 ` Sergey Matyukevich 2017-09-18 4:52 ` [PATCH 0/2] qtnfmac: misc fixes intended for 4.14 Kalle Valo 2 siblings, 0 replies; 12+ messages in thread From: Sergey Matyukevich @ 2017-09-15 11:53 UTC (permalink / raw) To: linux-wireless; +Cc: Igor Mitsyanko, Avinash Patil, Sergey Matyukevich Abort active scans when wireless interface configuration is changed. The usecases include wireless interface mode change, interface down, AP stop, virtual interface removal. Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com> --- drivers/net/wireless/quantenna/qtnfmac/cfg80211.c | 9 ++++++--- drivers/net/wireless/quantenna/qtnfmac/cfg80211.h | 3 +++ drivers/net/wireless/quantenna/qtnfmac/core.c | 2 +- drivers/net/wireless/quantenna/qtnfmac/event.c | 2 -- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c index 856fa6e8327e..a450bc6bc774 100644 --- a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c +++ b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c @@ -115,6 +115,8 @@ int qtnf_del_virtual_intf(struct wiphy *wiphy, struct wireless_dev *wdev) vif = qtnf_netdev_get_priv(wdev->netdev); + qtnf_scan_done(vif->mac, true); + if (qtnf_cmd_send_del_intf(vif)) pr_err("VIF%u.%u: failed to delete VIF\n", vif->mac->macid, vif->vifid); @@ -335,6 +337,8 @@ static int qtnf_stop_ap(struct wiphy *wiphy, struct net_device *dev) struct qtnf_vif *vif = qtnf_netdev_get_priv(dev); int ret; + qtnf_scan_done(vif->mac, true); + ret = qtnf_cmd_send_stop_ap(vif); if (ret) { pr_err("VIF%u.%u: failed to stop AP operation in FW\n", @@ -570,8 +574,6 @@ qtnf_del_station(struct wiphy *wiphy, struct net_device *dev, !qtnf_sta_list_lookup(&vif->sta_list, params->mac)) return 0; - qtnf_scan_done(vif->mac, true); - ret = qtnf_cmd_send_del_sta(vif, params); if (ret) pr_err("VIF%u.%u: failed to delete STA %pM\n", @@ -1134,8 +1136,9 @@ void qtnf_virtual_intf_cleanup(struct net_device *ndev) } vif->sta_state = QTNF_STA_DISCONNECTED; - qtnf_scan_done(mac, true); } + + qtnf_scan_done(mac, true); } void qtnf_cfg80211_vif_reset(struct qtnf_vif *vif) diff --git a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.h b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.h index 6a4af52522b8..66db26613b1f 100644 --- a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.h +++ b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.h @@ -34,6 +34,9 @@ static inline void qtnf_scan_done(struct qtnf_wmac *mac, bool aborted) .aborted = aborted, }; + if (timer_pending(&mac->scan_timeout)) + del_timer_sync(&mac->scan_timeout); + mutex_lock(&mac->mac_lock); if (mac->scan_req) { diff --git a/drivers/net/wireless/quantenna/qtnfmac/core.c b/drivers/net/wireless/quantenna/qtnfmac/core.c index 5e60180482d1..7f83bedc0b88 100644 --- a/drivers/net/wireless/quantenna/qtnfmac/core.c +++ b/drivers/net/wireless/quantenna/qtnfmac/core.c @@ -68,9 +68,9 @@ static int qtnf_netdev_open(struct net_device *ndev) */ static int qtnf_netdev_close(struct net_device *ndev) { - netif_carrier_off(ndev); qtnf_virtual_intf_cleanup(ndev); qtnf_netdev_updown(ndev, 0); + netif_carrier_off(ndev); return 0; } diff --git a/drivers/net/wireless/quantenna/qtnfmac/event.c b/drivers/net/wireless/quantenna/qtnfmac/event.c index 0fc2814eafad..43d2e7fd6e02 100644 --- a/drivers/net/wireless/quantenna/qtnfmac/event.c +++ b/drivers/net/wireless/quantenna/qtnfmac/event.c @@ -345,8 +345,6 @@ qtnf_event_handle_scan_complete(struct qtnf_wmac *mac, return -EINVAL; } - if (timer_pending(&mac->scan_timeout)) - del_timer_sync(&mac->scan_timeout); qtnf_scan_done(mac, le32_to_cpu(status->flags) & QLINK_SCAN_ABORTED); return 0; -- 2.11.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 0/2] qtnfmac: misc fixes intended for 4.14 2017-09-15 11:52 [PATCH 0/2] qtnfmac: misc fixes intended for 4.14 Sergey Matyukevich 2017-09-15 11:52 ` [PATCH 1/2] qtnfmac: lock access to h/w in tx path Sergey Matyukevich 2017-09-15 11:53 ` [PATCH 2/2] qtnfmac: abort scans on wireless interface changes Sergey Matyukevich @ 2017-09-18 4:52 ` Kalle Valo 2 siblings, 0 replies; 12+ messages in thread From: Kalle Valo @ 2017-09-18 4:52 UTC (permalink / raw) To: Sergey Matyukevich; +Cc: linux-wireless, Igor Mitsyanko, Avinash Patil Sergey Matyukevich <sergey.matyukevich.os@quantenna.com> writes: > Here are two patches intended for 4.14. The first patch fixes tx path > regression. Lock should be held when queuing packets to h/w fifos in order to > properly handle configurations with two enabled interfaces or multiple enabled > mbss. The second patch fixes scan issues addressing a review comment > from K.Valo regarding pending active scans upon device removal. > > Regards, > Sergey > > Sergey Matyukevich (2): > > qtnfmac: lock access to h/w in tx path > qtnfmac: abort scans on wireless interface changes There was a problem patchwork.kernel.org over the weekend, but it should be fixed now. Unfortunately your patches didn't make it to patchwork, so please resend. https://patchwork.kernel.org/project/linux-wireless/list/?state=* -- Kalle Valo ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH RESEND 0/2] qtnfmac: misc fixes intended for 4.14 @ 2017-09-18 8:04 Sergey Matyukevich 2017-09-18 8:04 ` [PATCH 2/2] qtnfmac: abort scans on wireless interface changes Sergey Matyukevich 0 siblings, 1 reply; 12+ messages in thread From: Sergey Matyukevich @ 2017-09-18 8:04 UTC (permalink / raw) To: linux-wireless; +Cc: Igor Mitsyanko, Avinash Patil Hello Kalle, Igor, and all Here are two patches intended for 4.14. The first patch fixes tx path regression. Lock should be held when queuing packets to h/w fifos in order to properly handle configurations with two enabled interfaces or multiple enabled mbss. The second patch fixes scan issues addressing a review comment from K.Valo regarding pending active scans upon device removal. Regards, Sergey Sergey Matyukevich (2): qtnfmac: lock access to h/w in tx path qtnfmac: abort scans on wireless interface changes cfg80211.c | 9 ++++++--- cfg80211.h | 3 +++ core.c | 2 +- event.c | 2 -- pearl/pcie.c | 9 ++++++++- pearl/pcie_bus_priv.h | 2 ++ 6 files changed, 20 insertions(+), 7 deletions(-) ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/2] qtnfmac: abort scans on wireless interface changes 2017-09-18 8:04 [PATCH RESEND " Sergey Matyukevich @ 2017-09-18 8:04 ` Sergey Matyukevich 2017-09-18 8:41 ` Johannes Berg 0 siblings, 1 reply; 12+ messages in thread From: Sergey Matyukevich @ 2017-09-18 8:04 UTC (permalink / raw) To: linux-wireless; +Cc: Igor Mitsyanko, Avinash Patil, Sergey Matyukevich Abort active scans when wireless interface configuration is changed. The usecases include wireless interface mode change, interface down, AP stop, virtual interface removal. Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com> --- drivers/net/wireless/quantenna/qtnfmac/cfg80211.c | 9 ++++++--- drivers/net/wireless/quantenna/qtnfmac/cfg80211.h | 3 +++ drivers/net/wireless/quantenna/qtnfmac/core.c | 2 +- drivers/net/wireless/quantenna/qtnfmac/event.c | 2 -- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c index 856fa6e8327e..a450bc6bc774 100644 --- a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c +++ b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c @@ -115,6 +115,8 @@ int qtnf_del_virtual_intf(struct wiphy *wiphy, struct wireless_dev *wdev) vif = qtnf_netdev_get_priv(wdev->netdev); + qtnf_scan_done(vif->mac, true); + if (qtnf_cmd_send_del_intf(vif)) pr_err("VIF%u.%u: failed to delete VIF\n", vif->mac->macid, vif->vifid); @@ -335,6 +337,8 @@ static int qtnf_stop_ap(struct wiphy *wiphy, struct net_device *dev) struct qtnf_vif *vif = qtnf_netdev_get_priv(dev); int ret; + qtnf_scan_done(vif->mac, true); + ret = qtnf_cmd_send_stop_ap(vif); if (ret) { pr_err("VIF%u.%u: failed to stop AP operation in FW\n", @@ -570,8 +574,6 @@ qtnf_del_station(struct wiphy *wiphy, struct net_device *dev, !qtnf_sta_list_lookup(&vif->sta_list, params->mac)) return 0; - qtnf_scan_done(vif->mac, true); - ret = qtnf_cmd_send_del_sta(vif, params); if (ret) pr_err("VIF%u.%u: failed to delete STA %pM\n", @@ -1134,8 +1136,9 @@ void qtnf_virtual_intf_cleanup(struct net_device *ndev) } vif->sta_state = QTNF_STA_DISCONNECTED; - qtnf_scan_done(mac, true); } + + qtnf_scan_done(mac, true); } void qtnf_cfg80211_vif_reset(struct qtnf_vif *vif) diff --git a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.h b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.h index 6a4af52522b8..66db26613b1f 100644 --- a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.h +++ b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.h @@ -34,6 +34,9 @@ static inline void qtnf_scan_done(struct qtnf_wmac *mac, bool aborted) .aborted = aborted, }; + if (timer_pending(&mac->scan_timeout)) + del_timer_sync(&mac->scan_timeout); + mutex_lock(&mac->mac_lock); if (mac->scan_req) { diff --git a/drivers/net/wireless/quantenna/qtnfmac/core.c b/drivers/net/wireless/quantenna/qtnfmac/core.c index 5e60180482d1..7f83bedc0b88 100644 --- a/drivers/net/wireless/quantenna/qtnfmac/core.c +++ b/drivers/net/wireless/quantenna/qtnfmac/core.c @@ -68,9 +68,9 @@ static int qtnf_netdev_open(struct net_device *ndev) */ static int qtnf_netdev_close(struct net_device *ndev) { - netif_carrier_off(ndev); qtnf_virtual_intf_cleanup(ndev); qtnf_netdev_updown(ndev, 0); + netif_carrier_off(ndev); return 0; } diff --git a/drivers/net/wireless/quantenna/qtnfmac/event.c b/drivers/net/wireless/quantenna/qtnfmac/event.c index 0fc2814eafad..43d2e7fd6e02 100644 --- a/drivers/net/wireless/quantenna/qtnfmac/event.c +++ b/drivers/net/wireless/quantenna/qtnfmac/event.c @@ -345,8 +345,6 @@ qtnf_event_handle_scan_complete(struct qtnf_wmac *mac, return -EINVAL; } - if (timer_pending(&mac->scan_timeout)) - del_timer_sync(&mac->scan_timeout); qtnf_scan_done(mac, le32_to_cpu(status->flags) & QLINK_SCAN_ABORTED); return 0; -- 2.11.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] qtnfmac: abort scans on wireless interface changes 2017-09-18 8:04 ` [PATCH 2/2] qtnfmac: abort scans on wireless interface changes Sergey Matyukevich @ 2017-09-18 8:41 ` Johannes Berg 2017-09-18 9:57 ` Sergey Matyukevich 0 siblings, 1 reply; 12+ messages in thread From: Johannes Berg @ 2017-09-18 8:41 UTC (permalink / raw) To: Sergey Matyukevich, linux-wireless; +Cc: Igor Mitsyanko, Avinash Patil On Mon, 2017-09-18 at 11:04 +0300, Sergey Matyukevich wrote: > > */ > static int qtnf_netdev_close(struct net_device *ndev) > { > - netif_carrier_off(ndev); > qtnf_virtual_intf_cleanup(ndev); > qtnf_netdev_updown(ndev, 0); > + netif_carrier_off(ndev); > return 0; > } This seems unrelated? > - if (timer_pending(&mac->scan_timeout)) > - del_timer_sync(&mac->scan_timeout); > qtnf_scan_done(mac, le32_to_cpu(status->flags) & > QLINK_SCAN_ABORTED); and that's related perhaps but not really explained in the changelog, not sure? johannes ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] qtnfmac: abort scans on wireless interface changes 2017-09-18 8:41 ` Johannes Berg @ 2017-09-18 9:57 ` Sergey Matyukevich 2017-09-18 10:03 ` Johannes Berg 2017-09-18 12:51 ` Kalle Valo 0 siblings, 2 replies; 12+ messages in thread From: Sergey Matyukevich @ 2017-09-18 9:57 UTC (permalink / raw) To: Johannes Berg; +Cc: linux-wireless, Igor Mitsyanko, Avinash Patil On Mon, Sep 18, 2017 at 10:41:08AM +0200, Johannes Berg wrote: Hello Johannes, > On Mon, 2017-09-18 at 11:04 +0300, Sergey Matyukevich wrote: > > > > */ > > static int qtnf_netdev_close(struct net_device *ndev) > > { > > - netif_carrier_off(ndev); > > qtnf_virtual_intf_cleanup(ndev); > > qtnf_netdev_updown(ndev, 0); > > + netif_carrier_off(ndev); > > return 0; > > } > > This seems unrelated? Hmm... The idea was to make sure that scan is canceled before cfg80211_netdev_notifier_call throws WARN when state is changed to NETDEV_DOWN. However this is not needed if scans are properly canceled in cfg80211_ops handlers. Thanks for catching, will remove. > > - if (timer_pending(&mac->scan_timeout)) > > - del_timer_sync(&mac->scan_timeout); > > qtnf_scan_done(mac, le32_to_cpu(status->flags) & > > QLINK_SCAN_ABORTED); > > and that's related perhaps but not really explained in the changelog, > not sure? That was minor optimization: to remove pedning timer whenever scan is canceled. Sure, it worth mentioning in changelog, will do. By the way, is it ok to send corrected single patch in reply to this discussion ? Or the appropriate way is to resend the whole patch set ? Regards, Sergey ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] qtnfmac: abort scans on wireless interface changes 2017-09-18 9:57 ` Sergey Matyukevich @ 2017-09-18 10:03 ` Johannes Berg 2017-09-18 10:16 ` Sergey Matyukevich 2017-09-18 12:51 ` Kalle Valo 1 sibling, 1 reply; 12+ messages in thread From: Johannes Berg @ 2017-09-18 10:03 UTC (permalink / raw) To: Sergey Matyukevich; +Cc: linux-wireless, Igor Mitsyanko, Avinash Patil Hi, > > > static int qtnf_netdev_close(struct net_device *ndev) > > > { > > > - netif_carrier_off(ndev); > > > qtnf_virtual_intf_cleanup(ndev); > > > qtnf_netdev_updown(ndev, 0); > > > + netif_carrier_off(ndev); > > > return 0; > > > } > > > > This seems unrelated? > > Hmm... The idea was to make sure that scan is canceled before > cfg80211_netdev_notifier_call throws WARN when state is changed > to NETDEV_DOWN. However this is not needed if scans are properly > canceled in cfg80211_ops handlers. Thanks for catching, will remove. Not sure how the carrier change is relevant though - that doesn't even notify cfg80211? Anyway, I don't really know, it just didn't seem related :) > > > - if (timer_pending(&mac->scan_timeout)) > > > - del_timer_sync(&mac->scan_timeout); > > > qtnf_scan_done(mac, le32_to_cpu(status->flags) & > > > QLINK_SCAN_ABORTED); > > > > and that's related perhaps but not really explained in the > > changelog, > > not sure? > > That was minor optimization: to remove pedning timer whenever scan > is canceled. Sure, it worth mentioning in changelog, will do. I realized (after sending my email) that it actually made a lot of sense, so yeah - and in fact I'm not sure it's just an optimisation, depends on where else you cancel the timer and if that could leave the timer running until after the interface is removed or something. Btw, I don't really see why you check pending first? > By the way, is it ok to send corrected single patch in reply to this > discussion ? Or the appropriate way is to resend the whole patch set > ? Good question. I think it's up to the specific maintainer, I know davem wants a full resend, but Kalle might have his own preference. I for one am willing to deal with both, though a full resend is somewhat easier. johannes ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] qtnfmac: abort scans on wireless interface changes 2017-09-18 10:03 ` Johannes Berg @ 2017-09-18 10:16 ` Sergey Matyukevich 2017-09-18 10:20 ` Johannes Berg 0 siblings, 1 reply; 12+ messages in thread From: Sergey Matyukevich @ 2017-09-18 10:16 UTC (permalink / raw) To: Johannes Berg; +Cc: linux-wireless, Igor Mitsyanko, Avinash Patil On Mon, Sep 18, 2017 at 12:03:07PM +0200, Johannes Berg wrote: > > > > - if (timer_pending(&mac->scan_timeout)) > > > > - del_timer_sync(&mac->scan_timeout); > > > > qtnf_scan_done(mac, le32_to_cpu(status->flags) & > > > > QLINK_SCAN_ABORTED); > > > > > > and that's related perhaps but not really explained in the > > > changelog, > > > not sure? > > > > That was minor optimization: to remove pedning timer whenever scan > > is canceled. Sure, it worth mentioning in changelog, will do. > > I realized (after sending my email) that it actually made a lot of > sense, so yeah - and in fact I'm not sure it's just an optimisation, > depends on where else you cancel the timer and if that could leave the > timer running until after the interface is removed or something. > > Btw, I don't really see why you check pending first? I don't quite understand the question. Do you mean it makes sense to call cfg80211_scan_done first and then cancel timer ? > > By the way, is it ok to send corrected single patch in reply to this > > discussion ? Or the appropriate way is to resend the whole patch set > > ? > > Good question. I think it's up to the specific maintainer, I know davem > wants a full resend, but Kalle might have his own preference. I for one > am willing to deal with both, though a full resend is somewhat easier. Ok, understood. Regards, Sergey ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] qtnfmac: abort scans on wireless interface changes 2017-09-18 10:16 ` Sergey Matyukevich @ 2017-09-18 10:20 ` Johannes Berg 2017-09-18 10:31 ` Sergey Matyukevich 0 siblings, 1 reply; 12+ messages in thread From: Johannes Berg @ 2017-09-18 10:20 UTC (permalink / raw) To: Sergey Matyukevich; +Cc: linux-wireless, Igor Mitsyanko, Avinash Patil On Mon, 2017-09-18 at 13:16 +0300, Sergey Matyukevich wrote: > > > > > > - if (timer_pending(&mac->scan_timeout)) > > > > > - del_timer_sync(&mac->scan_timeout); > I don't quite understand the question. Do you mean it makes sense to > call cfg80211_scan_done first and then cancel timer ? No, I'm just not sure why you call timer_pending() first. johannes ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] qtnfmac: abort scans on wireless interface changes 2017-09-18 10:20 ` Johannes Berg @ 2017-09-18 10:31 ` Sergey Matyukevich 0 siblings, 0 replies; 12+ messages in thread From: Sergey Matyukevich @ 2017-09-18 10:31 UTC (permalink / raw) To: Johannes Berg; +Cc: linux-wireless, Igor Mitsyanko, Avinash Patil > > > > > > - if (timer_pending(&mac->scan_timeout)) > > > > > > - del_timer_sync(&mac->scan_timeout); > > > I don't quite understand the question. Do you mean it makes sense to > > call cfg80211_scan_done first and then cancel timer ? > > No, I'm just not sure why you call timer_pending() first. Well, it looks like before this change it was redundant. But now it makes sense to avoid deadlock when this function is called from the timer itself. Regards, Sergey ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] qtnfmac: abort scans on wireless interface changes 2017-09-18 9:57 ` Sergey Matyukevich 2017-09-18 10:03 ` Johannes Berg @ 2017-09-18 12:51 ` Kalle Valo 1 sibling, 0 replies; 12+ messages in thread From: Kalle Valo @ 2017-09-18 12:51 UTC (permalink / raw) To: Johannes Berg; +Cc: linux-wireless, Igor Mitsyanko, Avinash Patil Sergey Matyukevich <sergey.matyukevich.os@quantenna.com> writes: > By the way, is it ok to send corrected single patch in reply to this > discussion ? Or the appropriate way is to resend the whole patch set ? This has been a common question so I documented it: https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#resubmit_the_whole_patchset I leave it as an exercise to the reader to find the answer, even though the URL even spoils it ;) -- Kalle Valo ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-09-18 12:52 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-09-15 11:52 [PATCH 0/2] qtnfmac: misc fixes intended for 4.14 Sergey Matyukevich 2017-09-15 11:52 ` [PATCH 1/2] qtnfmac: lock access to h/w in tx path Sergey Matyukevich 2017-09-15 11:53 ` [PATCH 2/2] qtnfmac: abort scans on wireless interface changes Sergey Matyukevich 2017-09-18 4:52 ` [PATCH 0/2] qtnfmac: misc fixes intended for 4.14 Kalle Valo 2017-09-18 8:04 [PATCH RESEND " Sergey Matyukevich 2017-09-18 8:04 ` [PATCH 2/2] qtnfmac: abort scans on wireless interface changes Sergey Matyukevich 2017-09-18 8:41 ` Johannes Berg 2017-09-18 9:57 ` Sergey Matyukevich 2017-09-18 10:03 ` Johannes Berg 2017-09-18 10:16 ` Sergey Matyukevich 2017-09-18 10:20 ` Johannes Berg 2017-09-18 10:31 ` Sergey Matyukevich 2017-09-18 12:51 ` Kalle Valo
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).