[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a4048989adc1724a8aff80f954b9dfeac2bfa9b4.camel@siemens.com>
Date: Thu, 21 Aug 2025 20:13:24 +0000
From: "Sverdlin, Alexander" <alexander.sverdlin@...mens.com>
To: "daniel@...rotopia.org" <daniel@...rotopia.org>
CC: "olteanv@...il.com" <olteanv@...il.com>, "andrew@...n.ch"
<andrew@...n.ch>, "Christen, Peter" <peter.christen@...mens.com>,
"lxu@...linear.com" <lxu@...linear.com>, "john@...ozen.org"
<john@...ozen.org>, "davem@...emloft.net" <davem@...emloft.net>,
"yweng@...linear.com" <yweng@...linear.com>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "pabeni@...hat.com" <pabeni@...hat.com>,
"edumazet@...gle.com" <edumazet@...gle.com>, "bxu@...linear.com"
<bxu@...linear.com>, "linux@...linux.org.uk" <linux@...linux.org.uk>,
"fchan@...linear.com" <fchan@...linear.com>, "ajayaraman@...linear.com"
<ajayaraman@...linear.com>, "hauke@...ke-m.de" <hauke@...ke-m.de>,
"arkadis@...lanox.com" <arkadis@...lanox.com>, "kuba@...nel.org"
<kuba@...nel.org>, "horms@...nel.org" <horms@...nel.org>,
"jpovazanec@...linear.com" <jpovazanec@...linear.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>, "Stockmann, Lukas"
<lukas.stockmann@...mens.com>, "lrosu@...linear.com" <lrosu@...linear.com>,
"f.fainelli@...il.com" <f.fainelli@...il.com>, "Schirm, Andreas"
<andreas.schirm@...mens.com>
Subject: Re: [PATCH RFC net-next 22/23] net: dsa: add driver for MaxLinear
GSW1xx switch family
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
Powered by blists - more mailing lists