Re: [PATCH net-next v8 09/10] net: airoha: Introduce WAN device flag

[Lorenzo Bianconi <lorenzo@kernel.org>]

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

[...]
> 
> [ ... ]
> > +static int airoha_dev_init(struct net_device *netdev)
> > +{
> > +	struct airoha_gdm_dev *dev = netdev_priv(netdev);
> > +	struct airoha_gdm_port *port = dev->port;
> >  
> >  	switch (port->id) {
> >  	case AIROHA_GDM3_IDX:
> > +	case AIROHA_GDM4_IDX: {
> > +		struct airoha_eth *eth = dev->eth;
> >  
> > +		/* GDM2 supports a single net_device */
> > +		if (eth->ports[1] && eth->ports[1]->devs[0])
> > +			break;
> 
> Is there a strict probe-order dependency for these interfaces?
> 
> If GDM3 or GDM4 is probed before GDM2, eth->ports[1]->devs[0] will be NULL.
> This leads GDM3/GDM4 to assume GDM2 is inactive, take the WAN role, and force
> GDM2 into loopback mode.
> 
> Later, when GDM2 initializes:
> 
> > +	case AIROHA_GDM2_IDX:
> > +		/* GDM2 is always used as wan */
> > +		dev->flags |= AIROHA_PRIV_F_WAN;
> >  		break;
> 
> It unconditionally assigns itself the WAN role.
> 
> Does this probe ordering create multiple WAN devices and leave GDM2's
> hardware silently misconfigured since its routing capabilities were disabled
> by GDM3's loopback configuration?

This is not a real issue since GDM2 is always registered before GDM3/GDM4.

> 
> >  	default:
> >  		break;
> >  	}
> >  
> > +	airoha_dev_set_qdma(dev);
> > +	airoha_set_macaddr(dev, netdev->dev_addr);
> 
> When setting the MAC address, airoha_set_macaddr() does the following:
> 
> static void airoha_set_macaddr(struct airoha_gdm_dev *dev, const u8 *addr)
> {
> 	...
> 	reg = airoha_is_lan_gdm_dev(dev) ? REG_FE_LAN_MAC_H : REG_FE_WAN_MAC_H;
> 	...
> 	airoha_fe_wr(eth, reg, val);
> }
> 
> Since REG_FE_LAN_MAC_H is a global forwarding engine register rather than
> per-port, will initializing subsequent LAN devices overwrite the hardware
> MAC filter configured by the first LAN device?
> 
> Might this cause the hardware to drop packets for LAN interfaces that do not
> match the single overwritten MAC address?
> 
> (Note: This was observed to be fixed later in the series by commit "net:
> airoha: Support multiple LAN/WAN interfaces for hw MAC address configuration",
> but is mentioned here as it appears to introduce a regression in this commit.)

As pointed out here, this is resolved in a subsequent patch in the series.

Regards,
Lorenzo

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260519-airoha-eth-multi-serdes-v8-0-6bd70e329df6@kernel.org?part=9

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