Re: [PATCH net-next v8 10/15] net: lan966x: add shutdown callback to stop the FDMA on reboot

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] net: lan966x: missing stats_work cancellation in shutdown causes reboot hang
--

commit 2e6f47e0410ffe79c3ff982d925010362cdb3723
Author: Daniel Machon <daniel.machon@microchip.com>
net: lan966x: add shutdown callback to stop the FDMA on reboot

This commit adds a shutdown callback to properly quiesce the lan966x PCIe
endpoint during a host reboot. It stops FDMA channels, disables interrupts,
and unmaps ATU windows to prevent the hardware from continuing to assert
INTx or access host memory across restarts.

> diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> index 259d81e759076..024ce9f9916c7 100644
> --- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
[ ... ]
> @@ -1324,9 +1324,65 @@ static void lan966x_remove(struct platform_device *pdev)
[ ... ]
> +static void lan966x_shutdown(struct platform_device *pdev)
> +{
> +	struct lan966x *lan966x = platform_get_drvdata(pdev);
> +
> +	/* As a PCIe endpoint the switch is not reset by the host reboot, so it
> +	 * has to be quiesced here:
> +	 *
> +	 *   Free the irqs and mask the sources: no source can assert INTx.
> +	 *   Disable NAPI: the teardown must not race a poll.
> +	 *   Stop and detach the netdevs: drains xmit, closes ndo_open and MTU.
> +	 *   Stop the FDMA channels: waits for the engine to go idle.
> +	 *   Unmap the ATU windows: revokes the engine's access to host memory.
> +	 */
> +	if (!lan966x_is_pci(lan966x))
> +		return;
> +

[Severity: High]
Does this code leave the stats_work delayed work running after the PCIe
endpoint is disabled?

When the system reboots, the new lan966x_shutdown() callback is executed to
quiesce the hardware. The PCI core subsequently disables the endpoint, but if
the stats_work background task fires and calls lan966x_stats_update(), it
will read from the disabled hardware via MMIO.

Accessing memory-mapped PCI registers after the PCI link or memory space has
been disabled typically results in a completion timeout or Unsupported
Request. This escalates to a PCIe AER fault or SError (NMI), panicking the
kernel and hanging the reboot process.

Unlike lan966x_remove(), which explicitly calls
cancel_delayed_work_sync(&lan966x->stats_work), lan966x_shutdown() appears
to omit this cancellation. Could the delayed work fire during reboot after
this shutdown path executes?

> +	if (lan966x->xtr_irq > 0)
> +		devm_free_irq(lan966x->dev, lan966x->xtr_irq, lan966x);
> +	if (lan966x->ana_irq > 0)
> +		devm_free_irq(lan966x->dev, lan966x->ana_irq, lan966x);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924-lan966x-pci-fdma-v8-0-201c7b707d8b@microchip.com?part=10