Re: [PATCH net-next] net: macb: detach netdev before closing on shutdown

["Karumanchi, Vineeth" <vineeth@amd.com>]

Hi Jakub,

On 9/5/2026 7:14 AM, Jakub Kicinski wrote:
> 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.
> 

You are right, I will drop this patch.
Thanks for the clarification.

Thanks,
>> 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();
>>  }
>>  
> 

-- 
🙏 Vineeth