Re: [PATCH net-next v2 10/11] net: airoha: Do not stop GDM port if it is shared

Jakub Kicinski
2026-04-03T01:12:35Z

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: airoha: Do not stop GDM port if it is shared

This patch introduces a port->users reference counter to prevent setting
the GDM port forward configuration to FE_PSE_PORT_DROP if another net_device
is still using the shared GDM port.

> diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> index 0780fb8aacfb..3f9bce8c5370 100644
> --- a/drivers/net/ethernet/airoha/airoha_eth.c
> +++ b/drivers/net/ethernet/airoha/airoha_eth.c
[ ... ]
> @@ -1681,9 +1682,6 @@ static int airoha_dev_stop(struct net_device *netdev)
> for (i = 0; i < ARRAY_SIZE(qdma->q_tx); i++)
> netdev_tx_reset_subqueue(netdev, i);
>
> - airoha_set_gdm_port_fwd_cfg(qdma->eth, REG_GDM_FWD_CFG(port->id),
> - FE_PSE_PORT_DROP);
> -
> if (atomic_dec_and_test(&qdma->users)) {
> airoha_qdma_clear(qdma, REG_QDMA_GLOBAL_CFG,
> GLOBAL_CFG_TX_DMA_EN_MASK |

While this patch aims to support shared GDM ports, does the remaining
unconditional call to airoha_set_vip_for_gdm_port(dev, false) at the start
of airoha_dev_stop() break other active devices sharing the port?

static int airoha_dev_stop(struct net_device *netdev)
{
...
err = airoha_set_vip_for_gdm_port(dev, false);
...
}

This appears to clear the REG_FE_VIP_PORT_EN and REG_FE_IFC_PORT_EN hardware
bits for the physical port, which might instantly kill traffic for any other
active interface using the shared port.

Similarly, in airoha_dev_open() and airoha_dev_change_mtu(), the
REG_GDM_LEN_CFG (MTU) is overwritten unconditionally. If a device with a
smaller MTU is brought up, will it overwrite the shared port's MTU limit
and cause the hardware to silently drop larger packets for the other device?

[ ... ]
> @@ -1697,6 +1695,11 @@ static int airoha_dev_stop(struct net_device *netdev)
> }
> }
>
> + if (atomic_dec_and_test(&port->users))
> + airoha_set_gdm_port_fwd_cfg(qdma->eth,
> + REG_GDM_FWD_CFG(port->id),
> + FE_PSE_PORT_DROP);
> +
> return 0;
> }

Does moving this FE_PSE_PORT_DROP configuration to the end of airoha_dev_stop()
cause a race condition when stopping the device?

In airoha_dev_open(), the driver correctly enables the QDMA engine before
configuring the GDM port to forward packets. A safe teardown typically
mirrors this by first stopping the packet source (setting FE_PSE_PORT_DROP)
and then disabling the destination DMA engine.

By moving airoha_set_gdm_port_fwd_cfg() after the airoha_qdma_clear() block,
when both qdma->users and port->users reach zero, the QDMA engine and its
queues are shut down while the GDM port is still actively forwarding packets.

Could incoming packets be forwarded to a disabled DMA engine during this
window, risking hardware lockups or buffer leaks?