mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: netdev-bot+sashiko@kernel.org
To: daniel.machon@microchip.com
Cc: andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
	kuba@kernel.org, pabeni@redhat.com, horatiu.vultur@microchip.com,
	steen.hegelund@microchip.com, UNGLinuxDriver@microchip.com,
	ast@kernel.org, daniel@iogearbox.net, hawk@kernel.org,
	john.fastabend@gmail.com, sdf@fomichev.me,
	herve.codina@bootlin.com, arnd@arndb.de,
	gregkh@linuxfoundation.org, mohsin.bashr@gmail.com,
	richardcochran@gmail.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, bpf@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH net-next v6 08/14] net: lan966x: clear FDMA interrupt stickies after switch reset
Date: Thu, 10 Sep 2026 13:05:43 +0000	[thread overview]
Message-ID: <178904554399.219967.11177166628090236711@kernel.org> (raw)
In-Reply-To: <20260909-lan966x-pci-fdma-v6-8-6f48dab9d671@microchip.com>

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

Critical: 0 · High: 0 · Medium: 0 · Low: 1

- [Low] Commit-message/comment vs. reachability at this revision: the
  added FDMA sticky clear in lan966x_reset_switch()…

--

net: lan966x: clear FDMA interrupt stickies after switch reset

The patch acks FDMA_ERRORS, FDMA_INTR_ERR and FDMA_INTR_DB right after
reset_control_reset() in lan966x_reset_switch(), so that spurious sticky
bits latched by the GCB soft reset in PCI mode do not make the FDMA IRQ
handler fire as soon as the IRQ is requested. The clear is unconditional and
is described as a no-op on the platform path.

> 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 in the register writes themselves, but is the failure
described in the changelog reachable at this point in the series?

The commit message says 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". In lan966x_probe() the FDMA handler is only installed when a named
"fdma" interrupt exists:

	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,
				       "fdma irq", lan966x);
		...
		lan966x->fdma = true;
	}

At this commit the PCI overlay in drivers/misc/lan966x_pci.dtso does not
describe that interrupt:

	interrupts = <12 IRQ_TYPE_LEVEL_HIGH>,
		     <9 IRQ_TYPE_LEVEL_HIGH>;
	interrupt-names = "xtr", "ana";

so platform_get_irq_byname(pdev, "fdma") fails and lan966x_fdma_irq_handler()
is never requested on the PCI path.

The overlay also disagrees with the positional list in
Documentation/devicetree/bindings/net/microchip,lan966x-switch.yaml, where the
second entry is reserved for "fdma":

	interrupt-names:
	  minItems: 1
	  items:
	    - const: xtr
	    - const: fdma
	    - const: ana

Both points are resolved later in the same series by "misc: lan966x-pci: dts:
add fdma interrupt to overlay", which switches the overlay to
interrupts <12>,<14>,<9> and interrupt-names = "xtr", "fdma", "ana", so this is
only an observation about the ordering of the changelog against the code state
here rather than an actionable defect.

Would it help to mention in the commit message that the WARN only becomes
reachable once the overlay gains the "fdma" interrupt?

>  	/* 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/20260909-lan966x-pci-fdma-v6-0-6f48dab9d671%40microchip.com

  reply	other threads:[~2026-09-10 13:05 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-09 13:00 [PATCH net-next v6 00/14] net: lan966x: add support for PCIe FDMA Daniel Machon
2026-09-09 13:00 ` [PATCH net-next v6 01/14] MAINTAINERS: add FDMA library to Sparx5 SoC entry Daniel Machon
2026-09-09 13:00 ` [PATCH net-next v6 02/14] net: microchip: fdma: rename contiguous dataptr helpers Daniel Machon
2026-09-09 13:00 ` [PATCH net-next v6 03/14] net: microchip: fdma: add PCIe ATU support Daniel Machon
2026-09-10 13:05   ` netdev-bot+sashiko
2026-09-09 13:00 ` [PATCH net-next v6 04/14] net: lan966x: add FDMA LLP register write helper Daniel Machon
2026-09-09 13:00 ` [PATCH net-next v6 05/14] net: lan966x: export FDMA helpers for reuse Daniel Machon
2026-09-09 13:00 ` [PATCH net-next v6 06/14] net: lan966x: use a dedicated device for DMA operations Daniel Machon
2026-09-09 13:00 ` [PATCH net-next v6 07/14] net: lan966x: add FDMA ops dispatch for PCIe support Daniel Machon
2026-09-09 13:00 ` [PATCH net-next v6 08/14] net: lan966x: clear FDMA interrupt stickies after switch reset Daniel Machon
2026-09-10 13:05   ` netdev-bot+sashiko [this message]
2026-09-09 13:00 ` [PATCH net-next v6 09/14] net: lan966x: add shutdown callback to stop FDMA on reboot Daniel Machon
2026-09-10 13:05   ` netdev-bot+sashiko
2026-09-09 13:00 ` [PATCH net-next v6 10/14] net: lan966x: add PCIe FDMA support Daniel Machon
2026-09-10 13:05   ` netdev-bot+sashiko
2026-09-09 13:00 ` [PATCH net-next v6 11/14] net: lan966x: add PCIe FDMA MTU change support Daniel Machon
2026-09-10 13:05   ` netdev-bot+sashiko
2026-09-09 13:00 ` [PATCH net-next v6 12/14] net: lan966x: add PCIe FDMA XDP support Daniel Machon
2026-09-10 13:05   ` netdev-bot+sashiko
2026-09-09 13:00 ` [PATCH net-next v6 13/14] misc: lan966x-pci: dts: extend cpu reg to cover PCIE DBI space Daniel Machon
2026-09-10 13:05   ` netdev-bot+sashiko
2026-09-09 13:00 ` [PATCH net-next v6 14/14] misc: lan966x-pci: dts: add fdma interrupt to overlay Daniel Machon
2026-09-10 13:05   ` netdev-bot+sashiko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=178904554399.219967.11177166628090236711@kernel.org \
    --to=netdev-bot+sashiko@kernel.org \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=arnd@arndb.de \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel.machon@microchip.com \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hawk@kernel.org \
    --cc=herve.codina@bootlin.com \
    --cc=horatiu.vultur@microchip.com \
    --cc=john.fastabend@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mohsin.bashr@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=richardcochran@gmail.com \
    --cc=sdf@fomichev.me \
    --cc=steen.hegelund@microchip.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®