Re: [PATCH RFC net-next 22/23] net: dsa: add driver for MaxLinear GSW1xx switch family

["Sverdlin, Alexander" <alexander.sverdlin@siemens.com>]

Hello Daniel,

On Thu, 2025-08-21 at 20:18 +0100, Daniel Golle wrote:
> > > Add driver for the MaxLinear GSW1xx family of Ethernet switch ICs which
> > > are based on the same IP as the Lantiq/Intel GSWIP found in the Lantiq VR9
> > > and Intel GRX MIPS router SoCs. The main difference is that instead of
> > > using memory-mapped I/O to communicate with the host CPU these ICs are
> > > connected via MDIO (or SPI, which isn't supported by this driver).
> > > Implement the regmap API to access the switch registers over MDIO to allow
> > > reusing lantiq_gswip_common for all core functionality.
> > > 
> > > The GSW1xx also comes with a SerDes port capable of 1000Base-X, SGMII and
> > > 2500Base-X, which can either be used to connect an external PHY or SFP
> > > cage, or as the CPU port. Support for the SerDes interface is implemented
> > > in this driver using the phylink_pcs interface.
> > 
> > ...
> > 
> > > --- /dev/null
> > > +++ b/drivers/net/dsa/mxl-gsw1xx.c
> > 
> > ...
> > 
> > > static int gsw1xx_sgmii_pcs_config(struct phylink_pcs *pcs,
> > > +				   unsigned int neg_mode,
> > > +				   phy_interface_t interface,
> > > +				   const unsigned long *advertising,
> > > +				   bool permit_pause_to_mac)
> > > +{
> > > +	struct gsw1xx_priv *priv = sgmii_pcs_to_gsw1xx(pcs);
> > > +	bool sgmii_mac_mode = dsa_is_user_port(priv->gswip.ds, GSW1XX_SGMII_PORT);
> > > +	u16 txaneg, anegctl, val, nco_ctrl;
> > > +	int ret;
> > > +
> > > +	/* Assert and deassert SGMII shell reset */
> > > +	ret = regmap_set_bits(priv->shell, GSW1XX_SHELL_RST_REQ,
> > > +			      GSW1XX_RST_REQ_SGMII_SHELL);
> > 
> > Can this be moved into gsw1xx_probe() maybe?
> > 
> > The thing is, if the switch is bootstrapped in
> > "Self-start Mode: Managed Switch Sub-Mode", SGMII will be already
> > brought out of reset (by bootloader?) (GSWIP_CFG register), refer
> > to "Table 12 Registers Configuration for Self-start Mode: Managed Switch Sub-Mode"
> > in datasheet. And nobody would disable SGMII if it's unused otherwise.
> 
> What you say is true if the SGMII interface is used as the CPU port or
> to connect a (1000M/100M/10M) PHY. However, it can also be used to connect
> SFP modules, which can be hot-plugged. Or a 2500M/1000M/100M/10M PHY which
> requires switching to 2500Base-X mode in case of a 2500M link on the UTP
> interface comes up, but uses SGMII for all lower speeds.

I'm actually concerned about use-cases where SGMII is unused.
In "Self-start Mode" SGMII block is being brought up and driver will never disable it.
I'm not proposing to move the de-assertion of the reset, but either
the assertion can be done unconditionally somewhere around probe
or struct dsa_switch_ops::setup callback or the assertion can remain
here and be duplicated somewhere around init.

> We can probably do this similar to drivers/net/pcs/pcs-mtk-lynxi.c and
> only do a full reconf including reset if there are major changes which
> actually require that, but as the impact is minimal and the vendor
> implementation also carries out a reset as the first thing when
> configuring the SGMII interface, I'd just keep it like that for now.
> Optimization can come later if actually required.

Sure, it goes a bit beyond basic support as it's a power consumption
optimization, but I thought I'll bring this up now as the re-spin will happen
anyway and if you agree on moving the reset assertion, then later patching
will not be required.

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com