Re: [PATCH net-next v8 09/15] net: lan966x: clear FDMA interrupt stickies after switch reset

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit message and the new comment in lan966x_reset_switch()
  (drivers/net/ethernet/microchip/lan966x/lan966x_main.c) describe a…

--

net: lan966x: clear FDMA interrupt stickies after switch reset

This commit clears the FDMA_ERRORS, FDMA_INTR_ERR and FDMA_INTR_DB sticky
registers in lan966x_reset_switch() right after the switch reset. The aim
is to stop spurious bits that the GCB soft reset latches in PCI mode from
triggering the FDMA IRQ handler, and its WARN, during probe.

> When in PCI mode, the GCB soft reset issued by the reset controller
> can latch spurious bits in the FDMA error stickies. The latched bits
> sit in FDMA_INTR_ERR until the FDMA IRQ is requested later in probe,
> at which point the handler fires immediately and WARNs.

> diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> index 6e6c08bb8eeab..259d81e759076 100644
> --- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> @@ -1067,6 +1067,15 @@ static int lan966x_reset_switch(struct lan966x *lan966x)
>  
>  	reset_control_reset(switch_reset);
>  
> +	/* When in PCI mode, the GCB soft reset issued by the reset
> +	 * controller can latch spurious bits in the FDMA error and
> +	 * data-block stickies. Clear them before request_irq hooks up the
> +	 * FDMA IRQ line, otherwise the handler fires immediately on probe.
> +	 */

[Severity: Low]
This isn't a bug, but can the WARN described in the commit message and in
this comment actually be hit in PCI mode at this commit?

The PCI overlay describes only two interrupts for the switch node:

drivers/misc/lan966x_pci.dtso:
	interrupt-names = "xtr", "ana";

lan966x_probe() requests the FDMA IRQ only when an "fdma" interrupt is
described:

	lan966x->fdma_irq = platform_get_irq_byname(pdev, "fdma");
	if (lan966x->fdma_irq > 0) {
		err = devm_request_irq(&pdev->dev, lan966x->fdma_irq,
				       lan966x_fdma_irq_handler, 0,
	...

So in PCI mode lan966x_fdma_irq_handler() is never registered, and its
WARN() cannot fire. That changes only with later patches in the series,
"net: lan966x: add PCIe FDMA support" and "misc: lan966x-pci: dts: add fdma
interrupt to overlay", which add "fdma" to the overlay.

The series ordering itself looks fine. Would it be clearer to say that the
clear is done in preparation for enabling the FDMA IRQ in PCI mode? As
written, it reads as a fix for a WARN that can be hit today.

[ ... ]

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