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(-)
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 <[email protected]>
---
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
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 <[email protected]>
---
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
> > > > > > - 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
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