[RFC 1/2] wifi: mt76: disable NAPI before deleting in dma_cleanup

[RFC 1/2] wifi: mt76: disable NAPI before deleting in dma_cleanup

[greearb]

From: Ben Greear <greearb@candelatech.com>

NAPI should be disabled before deleted, this stops
a warning seen when trying to rmmod mt7996e.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---

NOTE:  Seems to be regression between 6.18 and 7.2, haven't looked
closer yet.

 drivers/net/wireless/mediatek/mt76/dma.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/mediatek/mt76/dma.c b/drivers/net/wireless/mediatek/mt76/dma.c
index a67880a9281c..6de3f28219bc 100644
--- a/drivers/net/wireless/mediatek/mt76/dma.c
+++ b/drivers/net/wireless/mediatek/mt76/dma.c
@@ -1217,6 +1217,7 @@ void mt76_dma_cleanup(struct mt76_dev *dev)
 	mt76_for_each_q_rx(dev, i) {
 		struct mt76_queue *q = &dev->q_rx[i];
 
+		napi_disable(&dev->napi[i]);
 		netif_napi_del(&dev->napi[i]);
 		mt76_dma_rx_cleanup(dev, q);
 
-- 
2.42.0

[RFC 2/2] wifi: mt76: mt7996: Clean up IRQ on module unload.

[greearb]

From: Ben Greear <greearb@candelatech.com>

When removing mt7996e module, softirqd would go to 100%
cpu usage.  It appears the problem is lack of cleanup of
the irq objects and proper kill of the irq_tasklet.

Clean these up, similar to how the error cleanup path works
in mt7996_pci_probe.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---

NOTE:  Tested in 7.2 + modifications kernel, but applies and compiles
against 7.3 linus tree.  Not tested there at this point.

 drivers/net/wireless/mediatek/mt76/mt7996/init.c | 16 +++++++++++++++-
 .../net/wireless/mediatek/mt76/mt7996/mt7996.h   |  1 +
 drivers/net/wireless/mediatek/mt76/mt7996/pci.c  | 11 +++++++++--
 3 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/init.c b/drivers/net/wireless/mediatek/mt76/mt7996/init.c
index fb635a092584..45fc76cecd04 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7996/init.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7996/init.c
@@ -1807,6 +1807,18 @@ int mt7996_register_device(struct mt7996_dev *dev)
 	return ret;
 }
 
+static void mt7996_cleanup_irqs(struct mt7996_dev *dev)
+{
+	struct mt76_dev *mdev = &dev->mt76;
+	struct pci_dev *pdev = to_pci_dev(mdev->dev);
+
+	mt7996_put_hif2(dev, dev->hif2); /* frees hif2 irq */
+
+	devm_free_irq(mdev->dev, pdev->irq, dev);
+	if (!mtk_wed_device_active(&dev->mt76.mmio.wed))
+		pci_free_irq_vectors(pdev);
+}
+
 void mt7996_unregister_device(struct mt7996_dev *dev)
 {
 	cancel_work_sync(&dev->dump_work);
@@ -1825,7 +1837,9 @@ void mt7996_unregister_device(struct mt7996_dev *dev)
 	if (mt7996_has_hwrro(dev) &&
 	    !mtk_wed_device_active(&dev->mt76.mmio.wed))
 		mt7996_rro_msdu_page_map_free(dev);
-	tasklet_disable(&dev->mt76.irq_tasklet);
+	tasklet_kill(&dev->mt76.irq_tasklet);
+
+	mt7996_cleanup_irqs(dev);
 
 	mt76_free_device(&dev->mt76);
 }
diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h b/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h
index 2a0cdb56f822..79c57e88e107 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h
+++ b/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h
@@ -678,6 +678,7 @@ irqreturn_t mt7996_irq_handler(int irq, void *dev_instance);
 u64 __mt7996_get_tsf(struct ieee80211_hw *hw, struct mt7996_vif_link *link);
 int mt7996_register_device(struct mt7996_dev *dev);
 void mt7996_unregister_device(struct mt7996_dev *dev);
+void mt7996_put_hif2(struct mt7996_dev *dev, struct mt7996_hif *hif);
 int mt7996_vif_link_add(struct mt76_phy *mphy, struct ieee80211_vif *vif,
 			struct ieee80211_bss_conf *link_conf,
 			struct mt76_vif_link *mlink);
diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/pci.c b/drivers/net/wireless/mediatek/mt76/mt7996/pci.c
index b7d9193e042f..5c686cf66d0a 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7996/pci.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7996/pci.c
@@ -53,11 +53,19 @@ static struct mt7996_hif *mt7996_pci_get_hif2(u32 idx)
 	return hif;
 }
 
-static void mt7996_put_hif2(struct mt7996_hif *hif)
+void mt7996_put_hif2(struct mt7996_dev *dev, struct mt7996_hif *hif)
 {
+	struct pci_dev *hif2_dev;
+
 	if (!hif)
 		return;
 
+	hif2_dev = container_of(hif->dev, struct pci_dev, dev);
+
+	devm_free_irq(dev->mt76.dev, hif->irq, dev);
+	if (!mtk_wed_device_active(&dev->mt76.mmio.wed_hif2))
+		pci_free_irq_vectors(hif2_dev);
+
 	put_device(hif->dev);
 }
 
@@ -237,7 +245,6 @@ static void mt7996_pci_remove(struct pci_dev *pdev)
 
 	mdev = pci_get_drvdata(pdev);
 	dev = container_of(mdev, struct mt7996_dev, mt76);
-	mt7996_put_hif2(dev->hif2);
 	mt7996_unregister_device(dev);
 }
 
-- 
2.42.0

Re: [RFC 1/2] wifi: mt76: disable NAPI before deleting in dma_cleanup

[Shiji Yang]

Hi,
This change will result in another regression. See:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3aa1dcaa4f6f5ae08936491e08bd456f331f2d40

Regards,
Shiji Yang

Re: [RFC 1/2] wifi: mt76: disable NAPI before deleting in dma_cleanup

[Ben Greear]

On 9/18/26 04:04, Shiji Yang wrote:
> Hi,
> This change will result in another regression. See:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3aa1dcaa4f6f5ae08936491e08bd456f331f2d40
> 
> Regards,
> Shiji Yang
> 

Thanks for the pointer.  Maybe we have to make the common code pay attention to the radio
type since different radio driver logic is acting differently.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [RFC 1/2] wifi: mt76: disable NAPI before deleting in dma_cleanup

[Devin Wittmayer]

On 9/16/26 13:47, Ben Greear wrote:
> +		napi_disable(&dev->napi[i]);
>  		netif_napi_del(&dev->napi[i]);

Ben,

The warning and the softirqd spin are both real, and your 2/2 looks like
the right place for them.

On 1/2 there is a snag. mt7921e and mt7925e already stop RX themselves,
and they do it early:

	mt76_unregister_device()
	napi_disable() on each rx queue
	tx_token_put()
	mt792x_dma_cleanup()

It has to be early. A poll still in flight can come back with a transmit
status and take an entry out of the table tx_token_put() is tearing
down. By the time the shared cleanup runs RX is already stopped, so a
second disable there just sits:

	task in D state, refcount -1
	napi_disable_locked <- mt76_dma_cleanup <- mt7925_pci_remove

That was on MT7927. The USB parts tear down a different way and never
arrive, which is probably why this is easy to miss.

Would putting the disable next to the IRQ and tasklet work in your 2/2
cover mt7996? That keeps it where the driver already knows whether it
has stopped RX.

Devin

Re: [RFC 1/2] wifi: mt76: disable NAPI before deleting in dma_cleanup

[Ben Greear]

On 9/19/26 20:49, Devin Wittmayer wrote:
> On 9/16/26 13:47, Ben Greear wrote:
>> +		napi_disable(&dev->napi[i]);
>>   		netif_napi_del(&dev->napi[i]);
> 
> Ben,
> 
> The warning and the softirqd spin are both real, and your 2/2 looks like
> the right place for them.
> 
> On 1/2 there is a snag. mt7921e and mt7925e already stop RX themselves,
> and they do it early:
> 
> 	mt76_unregister_device()
> 	napi_disable() on each rx queue
> 	tx_token_put()
> 	mt792x_dma_cleanup()
> 
> It has to be early. A poll still in flight can come back with a transmit
> status and take an entry out of the table tx_token_put() is tearing
> down. By the time the shared cleanup runs RX is already stopped, so a
> second disable there just sits:
> 
> 	task in D state, refcount -1
> 	napi_disable_locked <- mt76_dma_cleanup <- mt7925_pci_remove
> 
> That was on MT7927. The USB parts tear down a different way and never
> arrive, which is probably why this is easy to miss.
> 
> Would putting the disable next to the IRQ and tasklet work in your 2/2
> cover mt7996? That keeps it where the driver already knows whether it
> has stopped RX.

I have a hard time figuring out exactly how to safely tear down
the mt7996 driver, especially without breaking other mt76 drivers.

But instead of just playing whack-a-mole with this,
we should figure out if sub-driver or mt76 core should be handling this,
and add some comments of expected behaviour so we don't keep fixing one chipset
and breaking another.

Maybe Felix has an opinion on correct path forward?

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com