Re: [PATCH net-next v3] net: dsa: mxl-gsw1xx: force internal PHYs into a known reset state

[Sverdlin, Alexander]

Thanks for the timely review, Daniel!

On Tue, 2026-09-15 at 12:17 +0100, Daniel Golle wrote:
> > When the switch is bootstrapped with PS_NOWAIT = 0, the internal PHYs
> > default to a held-in-reset state. The driver previously only worked with
> > PS_NOWAIT = 1 where PHYs are released from reset automatically.
> > 
> > Program the PHY reset lines via the RST_REQ register before the MDIO bus
> > is registered, so that PHY IDs can be read during bus scan. Release only
> > the PHYs of ports that have a phy-handle in the device tree and keep the
> > unused ones held in reset. Driving both directions makes the resulting
> > state identical regardless of the PS_NOWAIT bootstrap: unused PHYs no
> > longer stay powered when PS_NOWAIT = 1 released them automatically.
> > 
> > A single 300ms delay after deasserting the needed resets ensures the PHYs
> > are ready for MDIO communication (refer to Intel XWAY, Maxlinear GPY111
> > datasheets or the Lantiq counterpart of the driver). The delay is skipped
> > when no PHY is released from reset.
> > [...]
> > diff --git a/drivers/net/dsa/lantiq/mxl-gsw1xx.c b/drivers/net/dsa/lantiq/mxl-gsw1xx.c
> > index 66bf7e3319057..08c633b2caebf 100644
> > --- a/drivers/net/dsa/lantiq/mxl-gsw1xx.c
> > +++ b/drivers/net/dsa/lantiq/mxl-gsw1xx.c
> > [...]
> > @@ -589,6 +590,52 @@ static void gsw150_phylink_get_caps(struct dsa_switch *ds, int port,
> >   	gsw1xx_phylink_get_lpi_caps(config);
> >   }
> >   
> > +static int gsw1xx_setup(struct dsa_switch *ds)
> > +{
> > +	struct gsw1xx_priv *priv = container_of(ds->priv, struct gsw1xx_priv, gswip);
> > +	struct gswip_priv *gswip_priv = ds->priv;
> > +	u32 phy_mask = 0, active_mask = 0;
> > +	struct device_node *phy_np;
> > +	struct dsa_port *dp;
> > +	int ret;
> > +
> > +	dsa_switch_for_each_user_port(dp, ds) {
> 
> This loop runs only over *user ports*. It skips DSA_PORT_TYPE_UNUSED.
> 
> > +		struct phylink_config cfg = {};
> > +
> > +		/* Is there an internal PHY on this port? */
> > +		gswip_priv->hw_info->phylink_get_caps(ds, dp->index, &cfg);
> > +		if (!test_bit(PHY_INTERFACE_MODE_INTERNAL, cfg.supported_interfaces))
> > +			continue;
> > +
> > +		phy_mask |= GSW1XX_RST_REQ_PHY(dp->index);
> > +
> > +		/* Will the PHY be really used? */
> > +		phy_np = of_parse_phandle(dp->dn, "phy-handle", 0);
> > +		if (!phy_np)
> > +			continue;
> 
> Hence this check is redundant and doesn't make much sense.
> 
> > +
> > +		of_node_put(phy_np);
> > +		active_mask |= GSW1XX_RST_REQ_PHY(dp->index);
> > +	}
> > +
> > +	if (!phy_mask)
> > +		return 0;
> 
> So this means if only the MII interfaces are in use the PHYs are
> not being put into reset. Why?

Agree on both points! Will fix in v4.

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com