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

[netdev-bot+sashiko@kernel.org]

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

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

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).

- [Medium] When adin1110_net_open() fails after it has written IMASK1,
  the port's own RX-ready interrupt stays enabled even though the port…

--

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

This patch changes adin1110_net_stop() so that it masks the RX-ready
interrupt bit of the port being stopped instead of the peer's bit. It
also changes adin1110_net_open() so that, on ADIN2111, the peer's
RX-ready bit is unmasked in IMASK1 only when the peer is running.

> diff --git a/drivers/net/ethernet/adi/adin1110.c b/drivers/net/ethernet/adi/adin1110.c
> index 53fac3dc8be62..c41fad7835d07 100644
> --- a/drivers/net/ethernet/adi/adin1110.c
> +++ b/drivers/net/ethernet/adi/adin1110.c

[ ... ]

> @@ -895,12 +896,24 @@ static int adin1110_net_open(struct net_device *net_dev)
>  	if (ret < 0)
>  		goto out;
>  
> -	val = ADIN1110_TX_RDY_IRQ | ADIN1110_RX_RDY_IRQ | ADIN1110_SPI_ERR_IRQ;
> -	if (priv->cfg->id == ADIN2111_MAC)
> -		val |= ADIN2111_RX_RDY_IRQ;
> +	common = ADIN1110_TX_RDY_IRQ | ADIN1110_SPI_ERR_IRQ;
> +	own = !port_priv->nr ? ADIN1110_RX_RDY_IRQ : ADIN2111_RX_RDY_IRQ;
> +	peer = !port_priv->nr ? ADIN2111_RX_RDY_IRQ : ADIN1110_RX_RDY_IRQ;
>  
> -	priv->irq_mask = val;
> -	ret = adin1110_write_reg(priv, ADIN1110_IMASK1, ~val);
> +	/*
> +	 * The write below is absolute, so only keep the peer port's
> +	 * RX-ready IRQ enabled while that port is running; a stopped peer
> +	 * must stay masked until it is opened again.
> +	 */
> +	irq_mask = common | own;
> +	priv->irq_mask = common | ADIN1110_RX_RDY_IRQ;
> +	if (priv->cfg->id == ADIN2111_MAC) {
> +		priv->irq_mask |= ADIN2111_RX_RDY_IRQ;
> +		if (netif_running(priv->ports[!port_priv->nr]->netdev))
> +			irq_mask |= peer;
> +	}
> +
> +	ret = adin1110_write_reg(priv, ADIN1110_IMASK1, ~irq_mask);
>  	if (ret < 0) {
>  		netdev_err(net_dev, "Failed to enable chip IRQs: %d\n", ret);
>  		goto out;

[Severity: Medium]
The commit message says "A port's RX-ready IRQ is now enabled only while
that port is open". Is that still true if adin1110_net_open() fails after
this IMASK1 write?

This write unmasks the port's own RX-ready bit. Three later steps in
adin1110_net_open() can still fail, and each one jumps to out without
masking that bit again:

	ret = adin1110_read_reg(priv, ADIN1110_TX_SPACE, &val);
	if (ret < 0) {
		...
		goto out;
	}
	...
	ret = adin1110_set_mac_address(net_dev, net_dev->dev_addr);
	if (ret < 0) {
		...
		goto out;
	}

	ret = adin1110_set_bits(priv, ADIN1110_CONFIG1, ADIN1110_CONFIG1_SYNC,
				ADIN1110_CONFIG1_SYNC);

out:
	mutex_unlock(&priv->lock);

	if (ret < 0)
		return ret;

When ndo_open fails, __dev_open() only clears __LINK_STATE_START. It does
not call ndo_stop, so adin1110_net_stop() never masks the bit either.

The port then ends up in the state the commit message describes as the
bug. Its RX-ready interrupt is unmasked, but the handler skips the port
because netif_oper_up() is false:

adin1110_irq()
    adin1110_port_rx_ready()
        if (!netif_oper_up(port_priv->netdev))
            return false;

Any frames left in that port's FIFO keep the level-triggered interrupt
asserted, so the threaded handler keeps running and drains nothing.

On ADIN1110 this lasts until the next successful open. On ADIN2111 it
lasts until the peer port is opened. The peer's open sees netif_running()
as false for this port and masks the bit again.

Could the open error path mask the own RX-ready bit in IMASK1 again while
priv->lock is still held?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261111-adin1110-net-stop-v5-1-yuehongwu%40cmss.chinamobile.com