Re: [net-next PATCH RFC 2/5] net: ethernet: stmicro: stmmac: first disable all queues in release

From: Jakub Kicinski <kuba@kernel.org>
To: Christian Marangi <ansuelsmth@gmail.com>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>,
	Alexandre Torgue <alexandre.torgue@foss.st.com>,
	Jose Abreu <joabreu@synopsys.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Paolo Abeni <pabeni@redhat.com>,
	Maxime Coquelin <mcoquelin.stm32@gmail.com>,
	Russell King <linux@armlinux.org.uk>,
	netdev@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [net-next PATCH RFC 2/5] net: ethernet: stmicro: stmmac: first disable all queues in release
Date: Tue, 28 Jun 2022 20:54:35 -0700	[thread overview]
Message-ID: <20220628205435.44b0c78c@kernel.org> (raw)
In-Reply-To: <20220628013342.13581-3-ansuelsmth@gmail.com>

On Tue, 28 Jun 2022 03:33:39 +0200 Christian Marangi wrote:
> +	stmmac_disable_all_queues(priv);
> +
> +	for (chan = 0; chan < priv->plat->tx_queues_to_use; chan++)
> +		hrtimer_cancel(&priv->tx_queue[chan].txtimer);

IIRC this hrtimer is to check for completions. Canceling it before
netif_tx_disable() looks odd, presumably until the queues are stopped
the timer can get scheduled again, no?

>  	netif_tx_disable(dev);
>  
>  	if (device_may_wakeup(priv->device))
> @@ -3764,11 +3769,6 @@ static int stmmac_release(struct net_device *dev)
>  	phylink_stop(priv->phylink);
>  	phylink_disconnect_phy(priv->phylink);
>  
> -	stmmac_disable_all_queues(priv);
> -
> -	for (chan = 0; chan < priv->plat->tx_queues_to_use; chan++)
> -		hrtimer_cancel(&priv->tx_queue[chan].txtimer);