Re: [PATCH] wifi: mt76: mt7921: correctly handle removal in the absence of firmware

From: Helmut Grohne <helmut@freexian.com>
To: Lorenzo Bianconi <lorenzo@kernel.org>
Cc: Felix Fietkau <nbd@nbd.name>, Ryder Lee <ryder.lee@mediatek.com>,
	linux-wireless@vger.kernel.org,
	Stuart Hayhurst <stuart.a.hayhurst@gmail.com>,
	Shayne Chen <shayne.chen@mediatek.com>,
	Sean Wang <sean.wang@mediatek.com>,
	Deren Wu <deren.wu@mediatek.com>,
	netdev@vger.kernel.org, 1029116@bugs.debian.org
Subject: Re: [PATCH] wifi: mt76: mt7921: correctly handle removal in the absence of firmware
Date: Wed, 22 Feb 2023 15:42:03 +0100	[thread overview]
Message-ID: <Y/YpuyMWwAiCkcbv@alf.mars> (raw)
In-Reply-To: <Y/X6DPaxF0TMaUBh@lore-desk>

On Wed, Feb 22, 2023 at 12:18:36PM +0100, Lorenzo Bianconi wrote:
> Thx for working on this. I would say it is a more general issue since we have
> dbdc support in mt7915/mt7996/mt7915 devices. I think we can move the check in
> mac80211.c, what do you think? Something like (please note this patch is not
> tested):

Thanks for looking into this and improving the solution. Your patch
didn't apply cleanly to torvald's master, but it was easy to rebase.

> diff --git a/drivers/net/wireless/mediatek/mt76/mac80211.c b/drivers/net/wireless/mediatek/mt76/mac80211.c
> index b117e4467c87..34abf70f44af 100644
> --- a/drivers/net/wireless/mediatek/mt76/mac80211.c
> +++ b/drivers/net/wireless/mediatek/mt76/mac80211.c
> @@ -539,6 +539,7 @@ int mt76_register_phy(struct mt76_phy *phy, bool vht,
>  	if (ret)
>  		return ret;
>  
> +	set_bit(MT76_STATE_REGISTERED, &phy->state);
>  	phy->dev->phys[phy->band_idx] = phy;
>  
>  	return 0;
> @@ -549,6 +550,9 @@ void mt76_unregister_phy(struct mt76_phy *phy)
>  {
>  	struct mt76_dev *dev = phy->dev;
>  
> +	if (!test_bit(MT76_STATE_REGISTERED, &phy->state))
> +		return;
> +
>  	if (IS_ENABLED(CONFIG_MT76_LEDS))
>  		mt76_led_cleanup(phy);
>  	mt76_tx_status_check(dev, true);
> @@ -719,6 +723,7 @@ int mt76_register_device(struct mt76_dev *dev, bool vht,
>  		return ret;
>  
>  	WARN_ON(mt76_worker_setup(hw, &dev->tx_worker, NULL, "tx"));
> +	set_bit(MT76_STATE_REGISTERED, &phy->state);
>  	sched_set_fifo_low(dev->tx_worker.task);
>  
>  	return 0;
> @@ -729,6 +734,9 @@ void mt76_unregister_device(struct mt76_dev *dev)
>  {
>  	struct ieee80211_hw *hw = dev->hw;
>  
> +	if (!test_bit(MT76_STATE_REGISTERED, &dev->phy.state))
> +		return;
> +
>  	if (IS_ENABLED(CONFIG_MT76_LEDS))
>  		mt76_led_cleanup(&dev->phy);
>  	mt76_tx_status_check(dev, true);
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
> index 4ffb6be63571..38c311760d15 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76.h
> +++ b/drivers/net/wireless/mediatek/mt76/mt76.h
> @@ -402,6 +402,7 @@ struct mt76_tx_cb {
>  
>  enum {
>  	MT76_STATE_INITIALIZED,
> +	MT76_STATE_REGISTERED,
>  	MT76_STATE_RUNNING,
>  	MT76_STATE_MCU_RUNNING,
>  	MT76_SCANNING,
> 

Tested-by: Helmut Grohne <helmut@freexian.com>

I confirm that it really solves the problem on MT7921 PCIe hardware.

Helmut