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

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.

Critical: 0 · High: 2 · Medium: 0 · Low: 2

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] Baseline decision: false positive; not reachable at this commit,
  and closed later in the series.
- [High] Baseline decision: false positive; not reachable at this commit,
  and closed later in the series.
- [Low] The part of lan966x_shutdown() that actually runs on PCIe at this
  commit fixes behaviour that has existed since PCIe support for lan966x…
- [Low] Baseline decision: false positive.

--

net: lan966x: add shutdown callback to stop the FDMA on reboot

This adds a .shutdown callback to the lan966x switch driver. The callback
only acts on the PCIe instantiation, where it frees the xtr, ana and FDMA
irqs, masks the analyzer interrupt, detaches the netdevs, disables both
FDMA channels and unmaps the outbound ATU windows.

It also restores the full FDMA_INTR_ENA enable in lan966x_fdma_rx_start(),
because that register persists across a warm reboot.

> 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)
>  	debugfs_remove_recursive(lan966x->debugfs_root);
>  }
>
> +static void lan966x_shutdown(struct platform_device *pdev)
> +{
> +	struct lan966x *lan966x = platform_get_drvdata(pdev);
> +

[ ... ]

> +	if (!lan966x_is_pci(lan966x))
> +		return;
> +
> +	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);
> +	if (lan966x->fdma_irq > 0)
> +		devm_free_irq(lan966x->dev, lan966x->fdma_irq, lan966x);
> +
> +	lan_wr(0, lan966x, ANA_ANAINTR);

[Severity: Low]
The code here looks correct, but should this change carry a Fixes: tag?

At this point in the series, the only part of lan966x_shutdown() that runs
on PCIe is the part that frees the xtr and ana irqs and clears ANA_ANAINTR.

The xtr and ana interrupts were wired up by commit 185686beb464 ("misc: Add
support for LAN966x PCI device"), in drivers/misc/lan966x_pci.dtso:

    interrupt-names = "xtr", "ana";

Since that commit, a warm host reboot leaves these OIC sources enabled on an
endpoint that is not reset. The same is true of the analyzer interrupt
armed by lan966x_init(). This matches what the commit message describes:

  "As a PCIe endpoint, lan966x is not reset by a host reboot: its FDMA
  channels and interrupt sources stay armed, and the OIC ORs every
  source into the shared PCIe INTx, asserted before the driver has
  re-probed."

Could the xtr/ana part be split out as a standalone fix with:

  Fixes: 185686beb464 ("misc: Add support for LAN966x PCI device")

As written, it depends on lan966x_is_pci() and lan966x->dma_dev, which were
added earlier in this series. That means it can't be identified for stable
or backported on its own.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260924-lan966x-pci-fdma-v8-0-201c7b707d8b%40microchip.com