[PATCH net] net: ethernet: adi: adin1110: mask the RX IRQ of the port being stopped

[PATCH net] net: ethernet: adi: adin1110: mask the RX IRQ of the port being stopped

[Yuehong Wu]

adin1110_net_stop() programs the RX-ready interrupt mask of the wrong
port: it uses ADIN2111_RX_RDY_IRQ (the second port's bit) when stopping
port 0 and ADIN1110_RX_RDY_IRQ (the first port's bit) when stopping
port 1, while adin1110_port_rx_ready() uses the opposite mapping, port 0
-> ADIN1110_RX_RDY and port 1 -> ADIN2111_P2_RX_RDY.

The port being stopped therefore keeps its own RX interrupt enabled
while the peer's is masked.  On ADIN2111 that stalls RX on the peer port;
on ADIN1110 the RX interrupt is never masked at all.  As the interrupt is
level triggered and adin1110_read_frames() skips the FIFO while
netif_oper_up() is false, the stopped port also keeps re-raising its
interrupt with nobody draining the FIFO.

Swap the two bits so the port being stopped masks its own interrupt.

Fixes: bc93e19d088b ("net: ethernet: adi: Add ADIN1110 support")
Signed-off-by: Yuehong Wu <wuyuehong@cmss.chinamobile.com>
---
 drivers/net/ethernet/adi/adin1110.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/adi/adin1110.c b/drivers/net/ethernet/adi/adin1110.c
index 53fac3d..abdc160 100644
--- a/drivers/net/ethernet/adi/adin1110.c
+++ b/drivers/net/ethernet/adi/adin1110.c
@@ -945,7 +945,7 @@ static int adin1110_net_stop(struct net_device *net_dev)
 	u32 mask;
 	int ret;
 
-	mask = !port_priv->nr ? ADIN2111_RX_RDY_IRQ : ADIN1110_RX_RDY_IRQ;
+	mask = !port_priv->nr ? ADIN1110_RX_RDY_IRQ : ADIN2111_RX_RDY_IRQ;
 
 	/* Disable RX RDY IRQs */
 	mutex_lock(&priv->lock);
-- 
2.43.0

Re: [PATCH net] net: ethernet: adi: adin1110: mask the RX IRQ of the port being stopped

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

 - What hardware the change was tested on. For driver fixes please
   mention the device (and if relevant firmware version) used for
   testing, or say that the change was not tested on real hardware.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net] net: ethernet: adi: adin1110: mask the RX IRQ of the port being stopped

[Yuehong Wu]

Hi,

Here is the information missing from the changelog:

- How the issue was found: during a code review of the driver that was
  assisted by an AI coding assistant (an LLM).  The assistant compared
  adin1110_net_stop() against the bit definitions and the mapping used by
  adin1110_port_rx_ready(); I then checked the finding against the register
  semantics used by adin1110_net_open() (IMASK1: 1 = masked).

- Triggered or theoretical: theoretical.  I have not triggered it and there
  is no reproducer.  Expected symptoms if it is hit: on ADIN2111, bringing
  one port down masks the peer port's RX-ready interrupt so the peer stops
  receiving, while the port that was brought down keeps its own RX interrupt
  enabled; since the interrupt is level triggered and
  adin1110_read_frames() skips the FIFO while netif_oper_up() is false,
  pending frames keep re-asserting it.  On the single-port ADIN1110 the RX
  interrupt is never masked at all.

- Hardware: none.  The change was not tested on real hardware; it was
  compile-tested only (CONFIG_ADIN1110=m, x86_64) and passes
  scripts/checkpatch.pl --strict.

Per Documentation/process/generated-content.rst, the tool assistance is
noted above.  I have reviewed the analysis and the change and take
responsibility for them.

If another revision is needed, I will fold this information into the
commit message.

Thanks,
Yuehong Wu