> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > - [High] Hardware state corruption for shared GDM ports > -- > > commit 68e8fa8fe02248409d0048f81217cd5cf7eda775 > Author: Lorenzo Bianconi > > net: airoha: Support multiple net_devices for a single FE GDM port > > This commit introduces support for multiple net_devices connected to the same > Frame Engine GDM port via an external hardware arbiter. Note that issues with > shared hardware state introduced here appear to be addressed in later commits > such as 28e02ac461157 ("net: airoha: Do not stop GDM port if it is shared") > and 01081daf7ce24 ("net: airoha: Introduce WAN device flag"), which add port > user reference counting, aggregate the maximum MTU, and restrict WAN loopback > configuration. > > > diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c > > --- a/drivers/net/ethernet/airoha/airoha_eth.c > > +++ b/drivers/net/ethernet/airoha/airoha_eth.c > > @@ -3035,11 +3043,25 @@ static int airoha_alloc_gdm_device(struct airoha_eth *eth, > > netdev->dev_addr); > > } > > > > + /* Allowed nbq for EN7581 on GDM3 port are 4 and 5 for PCIE0 > > + * and PCIE1 respectively. > > + */ > > + index = nbq; > > + if (index && airoha_is_7581(eth) && port->id == AIROHA_GDM3_IDX) > > + index -= 4; > > + > > + if (index >= ARRAY_SIZE(port->devs) || port->devs[index]) { > > + dev_err(eth->dev, "invalid nbq id: %d\n", nbq); > > + return -EINVAL; > > + } > > + > > + netdev->dev.of_node = of_node_get(np); > > dev = netdev_priv(netdev); > > dev->dev = netdev; > > dev->port = port; > > - port->dev = dev; > > dev->eth = eth; > > + dev->nbq = nbq; > > + port->devs[index] = dev; > > With multiple net_devices now sharing a single GDM port, could this cause > hardware state corruption when configuring shared hardware registers? > > For example, in airoha_dev_init(), airoha_set_gdm2_loopback() overwrites > the global REG_FE_WAN_PORT register entirely, which might wipe out the > configuration of previously initialized devices on the same port: > > airoha_fe_rmw(eth, REG_FE_WAN_PORT, > WAN1_EN_MASK | WAN1_MASK | WAN0_MASK, > FIELD_PREP(WAN0_MASK, src_port)); A subsequent patch enforces this constraints, we can have at most one port configured as WAN. > > Additionally, in airoha_dev_stop(), unconditionally writing FE_PSE_PORT_DROP > to REG_GDM_FWD_CFG(port->id) appears to drop all traffic for the entire > GDM port, breaking other active devices sharing it. This is fixed by a subsequent patch. > > Similarly, in airoha_dev_change_mtu(), overwriting REG_GDM_LEN_CFG(port->id) > with a lower MTU could restrict the hardware limit for the entire port, causing > packet drops for other devices that require a higher MTU. This is fixed by a subsequent patch. Regards, Lorenzo > > -- > Sashiko AI review · https://sashiko.dev/#/patchset/20260516-airoha-eth-multi-serdes-v7-0-99e0093303e2@kernel.org?part=7