Re: [net-next PATCH v16 06/10] net: dsa: Add Airoha AN8855 5-Port Gigabit DSA Switch driver

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Tue, Sep 09, 2025 at 01:21:18PM +0200, Christian Marangi wrote:
> On Tue, Sep 09, 2025 at 10:08:57AM +0100, Russell King (Oracle) wrote:
> > On Tue, Sep 09, 2025 at 02:43:37AM +0200, Christian Marangi wrote:
> > > +static void an8855_phylink_get_caps(struct dsa_switch *ds, int port,
> > > +				    struct phylink_config *config)
> > > +{
> > > +	struct an8855_priv *priv = ds->priv;
> > > +	u32 reg;
> > > +	int ret;
> > > +
> > > +	switch (port) {
> > > +	case 0:
> > > +	case 1:
> > > +	case 2:
> > > +	case 3:
> > > +	case 4:
> > > +		__set_bit(PHY_INTERFACE_MODE_GMII,
> > > +			  config->supported_interfaces);
> > > +		__set_bit(PHY_INTERFACE_MODE_INTERNAL,
> > > +			  config->supported_interfaces);
> > > +		break;
> > > +	case 5:
> > > +		phy_interface_set_rgmii(config->supported_interfaces);
> > > +		__set_bit(PHY_INTERFACE_MODE_SGMII,
> > > +			  config->supported_interfaces);
> > > +		__set_bit(PHY_INTERFACE_MODE_2500BASEX,
> > > +			  config->supported_interfaces);
> > > +		break;
> > > +	}
> > > +
> > > +	config->mac_capabilities = MAC_ASYM_PAUSE | MAC_SYM_PAUSE |
> > > +				   MAC_10 | MAC_100 | MAC_1000FD | MAC_2500FD;
> > > +
> > > +	ret = regmap_read(priv->regmap, AN8855_CKGCR, &reg);
> > > +	if (ret)
> > > +		dev_err(ds->dev, "failed to read EEE LPI timer\n");
> > > +
> > > +	config->lpi_capabilities = MAC_100FD | MAC_1000FD;
> > > +	/* Global LPI TXIDLE Threshold, default 60ms (unit 2us) */
> > > +	config->lpi_timer_default = FIELD_GET(AN8855_LPI_TXIDLE_THD_MASK, reg) *
> > > +				    AN8855_TX_LPI_UNIT;
> > 
> > You're not filling in config->lpi_interfaces, which means phylink won't
> > LPI won't be functional.
> > 
> 
> Thanks for pointing this out, I notice lpi_interfaces is also not set on
> other DSA driver that were converted to the new EEE handling, for
> example mt7530.
> 
> I assume EEE is also half broken there and the required change wasn't
> notice at times?

Without checking (sorry, I'm busy, so I'm not going to), I have no
idea. What I can say is that phylink won't call the enable/disable
tx_lpi methods unless it is managing the EEE state, and to do that
it needs _all_ the LPI properties to be correctly populated.

A lot of other switches (e.g. Marvell DSA) doesn't need that level
of management.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!