From: Jakub Kicinski <kuba@kernel.org>
To: Vineeth Karumanchi <vineeth.karumanchi@amd.com>
Cc: <theo.lebrun@bootlin.com>, <conor.dooley@microchip.com>,
<andrew+netdev@lunn.ch>, <davem@davemloft.net>,
<edumazet@google.com>, <pabeni@redhat.com>, <git@amd.com>,
<netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net-next] net: macb: detach netdev before closing on shutdown
Date: Fri, 4 Sep 2026 18:44:32 -0700 [thread overview]
Message-ID: <20260904184432.02fd59a1@kernel.org> (raw)
In-Reply-To: <20260901142541.1469544-1-vineeth.karumanchi@amd.com>
On Tue, 1 Sep 2026 19:55:40 +0530 Vineeth Karumanchi wrote:
> macb_shutdown() calls netif_device_detach() after dev_close().
> netif_device_detach() only stops the TX queues when the device is
> still running:
>
> if (test_and_clear_bit(__LINK_STATE_PRESENT, &dev->state) &&
> netif_running(dev))
> netif_tx_stop_all_queues(dev);
>
> By that point dev_close() has already cleared the running state, so the
> detach only clears __LINK_STATE_PRESENT and the netif_tx_stop_all_queues()
> step is skipped.
>
> Reorder so the device is detached while still running, letting
> netif_device_detach() mark it absent and stop the TX queues as intended,
> before dev_close() tears the interface down.
>
> This is a cleanup with no functional change: dev_close() stops the queues
> and quiesces the DMA/HW via macb_close() regardless of ordering, so the
> reboot/kexec safety path is unaffected.
I'd guess the driver is trying to clear PRESENT so that the device
can't be opened again. It's not about stopping queues.
The patches doesn't make sense to me as is. You say yourself that
the move is a nop.
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 76ee4f506033..21ebaee00f9e 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -6246,12 +6246,11 @@ static void macb_shutdown(struct platform_device *pdev)
> struct net_device *netdev = platform_get_drvdata(pdev);
>
> rtnl_lock();
> + netif_device_detach(netdev);
>
> if (netif_running(netdev))
> dev_close(netdev);
>
> - netif_device_detach(netdev);
> -
> rtnl_unlock();
> }
>
prev parent reply other threads:[~2026-09-05 1:44 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-01 14:25 Vineeth Karumanchi
2026-09-05 1:44 ` Jakub Kicinski [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260904184432.02fd59a1@kernel.org \
--to=kuba@kernel.org \
--cc=andrew+netdev@lunn.ch \
--cc=conor.dooley@microchip.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=git@amd.com \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=theo.lebrun@bootlin.com \
--cc=vineeth.karumanchi@amd.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®