Re: [PATCH net-next v7 10/10] net: airoha: Support multiple LAN/WAN interfaces for hw MAC address configuration

[Lorenzo Bianconi <lorenzo@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3166 bytes --]

On May 17, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Device probe fails when multiple LAN/WAN interfaces are assigned randomly generated MAC addresses.
> --
> 
> commit 7861c44e7e1da4203e80f67d02418deac7e491b4
> Author: Lorenzo Bianconi <lorenzo@kernel.org>
> 
> net: airoha: Support multiple LAN/WAN interfaces for hw MAC address configuration
> 
> This commit extends the hardware MAC address configuration logic in the airoha
> driver to support multiple LAN or WAN interfaces by deriving minimum and
> maximum values for the lower MAC bytes, while expecting the upper MAC bytes to
> be shared.
> 
> > 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
> > @@ -71,20 +71,69 @@ 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)
> >  {
> > +	u8 ref_addr[ETH_ALEN] __aligned(2);
> >  	struct airoha_eth *eth = dev->eth;
> 
> [ ... ]
> 
> > +			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)) {
> 
> If the device tree lacks a MAC address for an interface and the driver assigns
> a random one via eth_hw_addr_random(), will this comparison fail during device
> probe?
> 
> Since eth_hw_addr_random() generates independent random bytes for each
> interface, the upper 3 bytes will likely mismatch. This returns -EINVAL and
> propagates up to register_netdev(), which causes the driver probe to abort.
> 
> Prior to this patch, the hardware MAC register was overwritten with the newest
> interface's MAC, but the probe continued successfully. Can this be adjusted to
> handle randomly generated MAC addresses without aborting the probe?

As already pointed out this is a requirement imposed by the hw design. Please
note we are not introducing any regression since this is required just when we
have multiple net_devices configured as LAN and this feature is introduced by
this patch.

Regards,
Lorenzo

> 
> > +		/* According to the HW design, hw mac address MS bits
> > +		 * must be the same for each net_device with the same
> > +		 * LAN/WAN configuration.
> > +		 */
> > +		netdev_err(dev->dev, "wrong mac addr for shared GDM port\n");
> > +		return -EINVAL;
> > +	}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260516-airoha-eth-multi-serdes-v7-0-99e0093303e2@kernel.org?part=10

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]