[PATCH v2 0/2] qtnfmac: misc fixes intended for 4.14

[PATCH v2 0/2] qtnfmac: misc fixes intended for 4.14

[Sergey Matyukevich]

Hello Kalle, Igor, and all

Here is the second version of the 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 related to pending scans upon device configuration changes.

Changes v1 -> v2
Introduce changes for scan patch accroding to review comments:
 - remove unneeded change of netdev_carrier_off
 - update commit message: clarify change of firmware scan timeout watchdog

Sergey Matyukevich (2):
  qtnfmac: lock access to h/w in tx path
  qtnfmac: cancel scans on wireless interface changes

 cfg80211.c            |    9 ++++++---
 cfg80211.h            |    3 +++
 event.c               |    2 --
 pearl/pcie.c          |    9 ++++++++-
 pearl/pcie_bus_priv.h |    2 ++
 5 files changed, 19 insertions(+), 6 deletions(-)

[PATCH v2 1/2] qtnfmac: lock access to h/w in tx path

[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

Re: [v2,1/2] qtnfmac: lock access to h/w in tx path

[Kalle Valo]

Sergey Matyukevich <sergey.matyukevich.os@quantenna.com> wrote:

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

2 patches applied to wireless-drivers.git, thanks.

20da2ec06bfa qtnfmac: lock access to h/w in tx path
a715b3a0efe7 qtnfmac: cancel scans on wireless interface changes

-- 
https://patchwork.kernel.org/patch/9956607/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [v2,1/2] qtnfmac: lock access to h/w in tx path

[Sergey Matyukevich]

Hello Kalle,

> > 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>
> 
> 2 patches applied to wireless-drivers.git, thanks.
> 
> 20da2ec06bfa qtnfmac: lock access to h/w in tx path
> a715b3a0efe7 qtnfmac: cancel scans on wireless interface changes

Could you please clarify a couple of points regarding wireless-drivers-next
and wireless-drivers trees. I checked development process article at
wireless.wiki.kernel.org, but it looks a bit outdated. So am I correct
assuming the following:
- these two fixes were applied to wireless-drivers because I asked to
  to queue them to 4.14
- these two fixes will show up in wireless-drivers-next at some point
  in the future after you move wireless-drivers-next forward, rebasing
  it on top of one of the upcoming "-rc"

Regards,
Sergey

Re: [v2,1/2] qtnfmac: lock access to h/w in tx path

[Kalle Valo]

Sergey Matyukevich <sergey.matyukevich.os@quantenna.com> writes:

> Hello Kalle,
>
>> > 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>
>> 
>> 2 patches applied to wireless-drivers.git, thanks.
>> 
>> 20da2ec06bfa qtnfmac: lock access to h/w in tx path
>> a715b3a0efe7 qtnfmac: cancel scans on wireless interface changes
>
> Could you please clarify a couple of points regarding wireless-drivers-next
> and wireless-drivers trees. I checked development process article at
> wireless.wiki.kernel.org, but it looks a bit outdated.

You mean this page:

https://wireless.wiki.kernel.org/en/developers/process

Yeah, that is outdated :)

> So am I correct assuming the following: - these two fixes were applied
> to wireless-drivers because I asked to to queue them to 4.14

Correct.

> - these two fixes will show up in wireless-drivers-next at some point
> in the future after you move wireless-drivers-next forward, rebasing
> it on top of one of the upcoming "-rc"

I would not use the term "rebase" here, as that means rewriting the
history which should be avoided on public trees. What I usually do is to
"merge" (git pull) or "fast forward" (git pull --ff-only).

You are correct that eventually commits from wireless-drivers trickle
down to wireless-drivers-next, I guess usually it takes something like
2-4 weeks. Most of them time they trickle down when Dave merges net to
net-next and I fast forward wireless-drivers-next to latest net-next
(after Dave has pulled from me). But occasionally I also merge
wireless-drivers to wireless-drivers-next myself, for example I did that
last cycle as there were major conflicts on iwlwifi and we wanted to fix
those early on. The earlier the conflicts are resolved the smoother it
is for everyone.

So if you have needs for getting the commits from wireless-drivers to
wireless-drivers-next please let me know and let's see what's the best
way forward.

Did this help?

-- 
Kalle Valo

Re: [v2,1/2] qtnfmac: lock access to h/w in tx path

[Sergey Matyukevich]

> > - these two fixes will show up in wireless-drivers-next at some point
> > in the future after you move wireless-drivers-next forward, rebasing
> > it on top of one of the upcoming "-rc"
> 
> I would not use the term "rebase" here, as that means rewriting the
> history which should be avoided on public trees. What I usually do is to
> "merge" (git pull) or "fast forward" (git pull --ff-only).
> 
> You are correct that eventually commits from wireless-drivers trickle
> down to wireless-drivers-next, I guess usually it takes something like
> 2-4 weeks. Most of them time they trickle down when Dave merges net to
> net-next and I fast forward wireless-drivers-next to latest net-next
> (after Dave has pulled from me). But occasionally I also merge
> wireless-drivers to wireless-drivers-next myself, for example I did that
> last cycle as there were major conflicts on iwlwifi and we wanted to fix
> those early on. The earlier the conflicts are resolved the smoother it
> is for everyone.

Understood.

> So if you have needs for getting the commits from wireless-drivers to
> wireless-drivers-next please let me know and let's see what's the best
> way forward.

Ok, good to know. But not this time, of course. We are keeping a bunch
of upcoming features and fixes in our internal tree on top of
wireless-drivers-next. So two more small patches for two more weeks
don't make any difference.

> Did this help?

Sure, thanks a lot for explanation!

Regards,
Sergey

[PATCH v2 2/2] qtnfmac: cancel scans on wireless interface changes

[Sergey Matyukevich]

Cancel active scans and deactivate firmware scan watchdog timer
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/event.c    | 2 --
 3 files changed, 9 insertions(+), 5 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/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