[PATCH net-next v4 10/10] net: airoha: Support multiple LAN/WAN interfaces for hw MAC address configuration
Lorenzo Bianconi
lorenzo at kernel.org
Sat May 9 03:07:58 PDT 2026
> 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: Support multiple LAN/WAN interfaces for hw MAC address configuration
>
> This patch extends airoha_set_macaddr() to derive REG_FE_MAC_LMIN and
> REG_FE_MAC_LMAX by iterating over all sibling LAN/WAN net_devices, and
> rejects addresses whose upper 3 bytes do not match the already-registered
> peers. airoha_dev_set_macaddr() is also changed to split eth_mac_addr()
> into eth_prepare_mac_addr_change() and eth_commit_mac_addr_change() around
> the hardware programming step.
>
> > diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> > index 8617dd4f7932..fbfefd0ec355 100644
> > --- a/drivers/net/ethernet/airoha/airoha_eth.c
> > +++ b/drivers/net/ethernet/airoha/airoha_eth.c
> > @@ -71,20 +71,67 @@ static void airoha_qdma_irq_disable(struct airoha_irq_bank *irq_bank,
> > airoha_qdma_set_irqmask(irq_bank, index, mask, 0);
> > }
> >
> > -static void airoha_set_macaddr(struct airoha_gdm_dev *dev, const u8 *addr)
> > +static int airoha_set_macaddr(struct airoha_gdm_dev *dev, const u8 *addr)
> > {
> > struct airoha_eth *eth = dev->eth;
> > - u32 val, reg;
> > + u8 ref_addr[ETH_ALEN] = {};
> > + u32 reg, val, lmin, lmax;
> > + int i;
> > +
> > + lmin = (addr[3] << 16) | (addr[4] << 8) | addr[5];
> > + lmax = lmin;
> > +
> > + for (i = 0; i < ARRAY_SIZE(eth->ports); i++) {
> > + struct airoha_gdm_port *port = eth->ports[i];
> > + int j;
> > +
> > + if (!port)
> > + continue;
> > +
> > + for (j = 0; j < ARRAY_SIZE(port->devs); j++) {
> > + struct airoha_gdm_dev *iter_dev;
> > + struct net_device *netdev;
> > +
> > + iter_dev = port->devs[j];
> > + if (!iter_dev || iter_dev == dev)
> > + continue;
> > +
> > + if (airoha_is_lan_gdm_dev(iter_dev) !=
> > + airoha_is_lan_gdm_dev(dev))
> > + continue;
> > +
> > + netdev = iter_dev->dev;
> > + if (netdev->reg_state != NETREG_REGISTERED)
> > + continue;
> > +
> > + ether_addr_copy(ref_addr, netdev->dev_addr);
> > + val = (netdev->dev_addr[3] << 16) |
> > + (netdev->dev_addr[4] << 8) | netdev->dev_addr[5];
> > + if (val < lmin)
> > + lmin = val;
> > + if (val > lmax)
> > + lmax = val;
> > + }
> > + }
> > +
> > + if (!is_zero_ether_addr(ref_addr) && memcmp(ref_addr, addr, 3)) {
> > + /* According to the HW design, hw mac address MS bits
> > + * must be the same for each net_device with the same
> > + * LAN/WAN configuration.
> > + */
> > + return -EINVAL;
> > + }
>
> [Medium]
> The NETREG_REGISTERED filter here means the check only sees peers that
> have already completed register_netdev(). airoha_dev_init() is invoked
> as ndo_init from register_netdev() before dev->reg_state transitions
> to NETREG_REGISTERED, so the constraint is only enforced across
> already-registered siblings.
>
> When the device tree does not supply a mac-address property,
> airoha_alloc_gdm_device() falls back to eth_hw_addr_random() and each
> sibling gets an independently generated random MAC. With two or more
> LAN (or two or more WAN) net_devices going through this fallback, the
> second and later siblings' upper 3 bytes will almost always differ
> from the first, so airoha_set_macaddr() returns -EINVAL:
>
> if (!is_zero_ether_addr(ref_addr) && memcmp(ref_addr, addr, 3))
> return -EINVAL;
>
> airoha_dev_init() now propagates that error, which fails
> register_netdev() and airoha_register_gdm_devices(), aborting probe.
>
> Before this patch the same configuration still came up (with the single
> REG_FE_{LAN,WAN}_MAC_H being last-writer-wins). Should the random
> fallback path derive sibling MACs from a shared base, or should the
> upper-3-bytes constraint only be enforced for user-triggered
> ndo_set_mac_address rather than for init-time programming?
Hw design requires that if two (or more) net_devices are configured as LAN
(or theoretically as WAN), they need to share the mac address upper-3-bytes.
If the user wants to use multiple devices with the same LAN/WAN configuration,
he/she will need to properly store the net_device mac address int the device
eeprom/nvme or in the dts.
Please note this series is not introducing any regression since the user was
not able to run this configuration before this series.
>
> >
> > reg = airoha_is_lan_gdm_dev(dev) ? REG_FE_LAN_MAC_H : REG_FE_WAN_MAC_H;
> > val = (addr[0] << 16) | (addr[1] << 8) | addr[2];
> > airoha_fe_wr(eth, reg, val);
> >
> > - val = (addr[3] << 16) | (addr[4] << 8) | addr[5];
> > - airoha_fe_wr(eth, REG_FE_MAC_LMIN(reg), val);
> > - airoha_fe_wr(eth, REG_FE_MAC_LMAX(reg), val);
> > + airoha_fe_wr(eth, REG_FE_MAC_LMIN(reg), lmin);
> > + airoha_fe_wr(eth, REG_FE_MAC_LMAX(reg), lmax);
> >
> > airoha_ppe_init_upd_mem(dev);
> > +
> > + return 0;
> > }
> >
> > static void airoha_set_gdm_port_fwd_cfg(struct airoha_eth *eth, u32 addr,
> > @@ -1792,13 +1839,18 @@ static int airoha_dev_stop(struct net_device *netdev)
> > static int airoha_dev_set_macaddr(struct net_device *netdev, void *p)
> > {
> > struct airoha_gdm_dev *dev = netdev_priv(netdev);
> > + struct sockaddr *addr = p;
> > int err;
> >
> > - err = eth_mac_addr(netdev, p);
> > + err = eth_prepare_mac_addr_change(netdev, p);
> > if (err)
> > return err;
> >
> > - airoha_set_macaddr(dev, netdev->dev_addr);
> > + err = airoha_set_macaddr(dev, addr->sa_data);
> > + if (err)
> > + return err;
> > +
> > + eth_commit_mac_addr_change(netdev, p);
> >
> > return 0;
> > }
>
> [High]
> After this reordering, does airoha_ppe_init_upd_mem() end up
> programming the PPE update memory with the old MAC rather than the new
> one?
>
> eth_prepare_mac_addr_change() only validates the sockaddr and does not
> touch netdev->dev_addr; the actual write to netdev->dev_addr happens in
> eth_commit_mac_addr_change() via eth_hw_addr_set():
>
> net/ethernet/eth.c:eth_prepare_mac_addr_change() {
> if (!(dev->priv_flags & IFF_LIVE_ADDR_CHANGE) && netif_running(dev))
> return -EBUSY;
> if (!is_valid_ether_addr(addr->sa_data))
> return -EADDRNOTAVAIL;
> return 0;
> }
>
> net/ethernet/eth.c:eth_commit_mac_addr_change() {
> eth_hw_addr_set(dev, addr->sa_data);
> }
>
> airoha_set_macaddr() is invoked between those two, so when it reaches
> airoha_ppe_init_upd_mem(dev) the commit has not yet run. That helper
> reads netdev->dev_addr directly rather than using the addr argument:
>
> drivers/net/ethernet/airoha/airoha_ppe.c:airoha_ppe_init_upd_mem() {
> const u8 *addr = netdev->dev_addr;
> ...
> val = (addr[2] << 24) | (addr[3] << 16) | (addr[4] << 8) | addr[5];
> airoha_fe_wr(eth, REG_UPDMEM_DATA(0), val);
> ...
> val = (addr[0] << 8) | addr[1];
> airoha_fe_wr(eth, REG_UPDMEM_DATA(0), val);
> ...
> }
>
> So the FE registers REG_FE_{LAN,WAN}_MAC_H / REG_FE_MAC_LMIN /
> REG_FE_MAC_LMAX are written from the new addr parameter, but the PPE
> REG_UPDMEM_DATA/REG_UPDMEM_CTRL entries are written from the still-old
> netdev->dev_addr.
>
> Previously eth_mac_addr() (prepare + commit combined) ran first, so
> netdev->dev_addr was the new MAC by the time airoha_set_macaddr() and
> airoha_ppe_init_upd_mem() ran, keeping the two blocks in sync.
>
> Would it be cleaner to either pass addr through to
> airoha_ppe_init_upd_mem(), or invoke eth_commit_mac_addr_change() before
> airoha_set_macaddr() (with a rollback path if the hardware programming
> fails)?
ack, I will fix it in v5.
Regards,
Lorenzo
>
> > @@ -1903,6 +1955,7 @@ static int airoha_dev_init(struct net_device *netdev)
> [ ... ]
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-mediatek/attachments/20260509/a5aac8bd/attachment.sig>
More information about the Linux-mediatek
mailing list