Re: [PATCH net-next v7 08/14] 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] Commit message describes the WARN-on-probe failure in the present
  tense, but at this commit the PCI path cannot request the FDMA IRQ:…
- [Low] In-code comment under-describes the code it documents.

--

net: lan966x: clear FDMA interrupt stickies after switch reset

This adds three register acknowledgements right after the switch reset in
lan966x_reset_switch(), clearing FDMA_ERRORS, FDMA_INTR_ERR and
FDMA_INTR_DB so that a spurious sticky latched by the GCB soft reset in PCI
mode does not make the FDMA handler fire as soon as the IRQ is requested.
The clear is done unconditionally on both the PCI and platform paths.

> diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> index 6e6c08bb8eea..11094a381ec2 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 stickies.
> +	 * Clear them before request_irq hooks up the FDMA IRQ line,
> +	 * otherwise the handler fires immediately on probe.
> +	 */
> +	lan_wr(lan_rd(lan966x, FDMA_ERRORS), lan966x, FDMA_ERRORS);
> +	lan_wr(lan_rd(lan966x, FDMA_INTR_ERR), lan966x, FDMA_INTR_ERR);
> +	lan_wr(lan_rd(lan966x, FDMA_INTR_DB), lan966x, FDMA_INTR_DB);

[Severity: Low]
This isn't a bug, but should the comment also mention FDMA_INTR_DB?  The
comment talks only about the "FDMA error stickies", while the code below it
also acks FDMA_INTR_DB, which is the data-block completion sticky rather
than an error sticky.

In lan966x_fdma.c, lan966x_fdma_irq_handler() consumes it in the non-error
branch:

	if (db) {
		lan_wr(0, lan966x, FDMA_INTR_DB_ENA);
		lan_wr(db, lan966x, FDMA_INTR_DB);

		napi_schedule(&lan966x->napi);
	}

So a latched DB sticky would lead to napi_schedule() rather than the WARN
described in the comment.

The commit message does list all three registers, so only the in-code
comment is out of sync.  Could it be extended to say that the data-block
sticky is cleared too, so a later cleanup doesn't drop the FDMA_INTR_DB
write as unrelated?

>  
>  	/* Don't reinitialize the switch core, if it is already initialized. In
>  	 * case it is initialized twice, some pointers inside the queue system
>  	 * in HW will get corrupted and then after a while the queue system gets

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260918-lan966x-pci-fdma-v7-0-0ecc179c8a2c%40microchip.com