[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <59f32c924cd8ebd02483dfd19c2788cf09d9ab75.camel@siemens.com>
Date: Thu, 21 Aug 2025 18:53:22 +0000
From: "Sverdlin, Alexander" <alexander.sverdlin@...mens.com>
To: "hauke@...ke-m.de" <hauke@...ke-m.de>, "olteanv@...il.com"
<olteanv@...il.com>, "davem@...emloft.net" <davem@...emloft.net>,
"andrew@...n.ch" <andrew@...n.ch>, "linux@...linux.org.uk"
<linux@...linux.org.uk>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "arkadis@...lanox.com"
<arkadis@...lanox.com>, "daniel@...rotopia.org" <daniel@...rotopia.org>,
"kuba@...nel.org" <kuba@...nel.org>, "pabeni@...hat.com" <pabeni@...hat.com>,
"edumazet@...gle.com" <edumazet@...gle.com>, "f.fainelli@...il.com"
<f.fainelli@...il.com>, "horms@...nel.org" <horms@...nel.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC: "john@...ozen.org" <john@...ozen.org>, "Stockmann, Lukas"
<lukas.stockmann@...mens.com>, "yweng@...linear.com" <yweng@...linear.com>,
"fchan@...linear.com" <fchan@...linear.com>, "lxu@...linear.com"
<lxu@...linear.com>, "jpovazanec@...linear.com" <jpovazanec@...linear.com>,
"Schirm, Andreas" <andreas.schirm@...mens.com>, "Christen, Peter"
<peter.christen@...mens.com>, "ajayaraman@...linear.com"
<ajayaraman@...linear.com>, "bxu@...linear.com" <bxu@...linear.com>,
"lrosu@...linear.com" <lrosu@...linear.com>
Subject: Re: [PATCH RFC net-next 22/23] net: dsa: add driver for MaxLinear
GSW1xx switch family
Hi Daniel,
On Sat, 2025-08-16 at 20:57 +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.
> + if (ret < 0)
> + return ret;
> +
> + ret = regmap_clear_bits(priv->shell, GSW1XX_SHELL_RST_REQ,
> + GSW1XX_RST_REQ_SGMII_SHELL);
> + if (ret < 0)
> + return ret;
> +
> + /* Hardware Bringup FSM Enable */
> + ret = regmap_write(priv->sgmii, GSW1XX_SGMII_PHY_HWBU_CTRL,
> + GSW1XX_SGMII_PHY_HWBU_CTRL_EN_HWBU_FSM |
> + GSW1XX_SGMII_PHY_HWBU_CTRL_HW_FSM_EN);
> + if (ret < 0)
> + return ret;
> +
> + /* Configure SGMII PHY Receiver */
> + val = FIELD_PREP(GSW1XX_SGMII_PHY_RX0_CFG2_EQ,
> + GSW1XX_SGMII_PHY_RX0_CFG2_EQ_DEF) |
> + GSW1XX_SGMII_PHY_RX0_CFG2_LOS_EN |
> + GSW1XX_SGMII_PHY_RX0_CFG2_TERM_EN |
> + FIELD_PREP(GSW1XX_SGMII_PHY_RX0_CFG2_FILT_CNT,
> + GSW1XX_SGMII_PHY_RX0_CFG2_FILT_CNT_DEF);
> +
> + // if (!priv->dts.sgmii_rx_invert)
^^
There is still a room for some cleanup ;-)
> + val |= GSW1XX_SGMII_PHY_RX0_CFG2_INVERT;
> +
> + ret = regmap_write(priv->sgmii, GSW1XX_SGMII_PHY_RX0_CFG2, val);
> + if (ret < 0)
> + return ret;
> +
...
> +static int gsw1xx_probe(struct mdio_device *mdiodev)
> +{
> + struct device *dev = &mdiodev->dev;
> + struct gsw1xx_priv *priv;
> + struct dsa_switch *ds;
> + int ret;
> +
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + priv->mdio_dev = mdiodev;
> + priv->smdio_badr = GSW1XX_SMDIO_BADR_UNKNOWN;
> +
> + priv->gswip.dev = dev;
> + priv->gswip.hw_info = of_device_get_match_data(dev);
> + if (!priv->gswip.hw_info)
> + return -EINVAL;
> +
> + mutex_init(&priv->gswip.pce_table_lock);
> +
> + priv->gswip.gswip = gsw1xx_regmap_init(priv, "switch",
> + GSW1XX_SWITCH_BASE, 0xfff);
> + if (IS_ERR(priv->gswip.gswip))
> + return PTR_ERR(priv->gswip.gswip);
> +
> + priv->gswip.mdio = gsw1xx_regmap_init(priv, "mdio", GSW1XX_MMDIO_BASE,
> + 0xff);
> + if (IS_ERR(priv->gswip.mdio))
> + return PTR_ERR(priv->gswip.mdio);
> +
> + priv->gswip.mii = gsw1xx_regmap_init(priv, "mii", GSW1XX_RGMII_BASE,
> + 0xff);
> + if (IS_ERR(priv->gswip.mii))
> + return PTR_ERR(priv->gswip.mii);
> +
> + priv->sgmii = gsw1xx_regmap_init(priv, "sgmii", GSW1XX_SGMII_BASE,
> + 0xfff);
> + if (IS_ERR(priv->sgmii))
> + return PTR_ERR(priv->sgmii);
> +
> + priv->gpio = gsw1xx_regmap_init(priv, "gpio", GSW1XX_GPIO_BASE,
> + 0xff);
> + if (IS_ERR(priv->gpio))
> + return PTR_ERR(priv->gpio);
> +
> + priv->clk = gsw1xx_regmap_init(priv, "clk", GSW1XX_CLK_BASE, 0xff);
> + if (IS_ERR(priv->clk))
> + return PTR_ERR(priv->clk);
> +
> + priv->shell = gsw1xx_regmap_init(priv, "shell", GSW1XX_SHELL_BASE,
> + 0xff);
> + if (IS_ERR(priv->shell))
> + return PTR_ERR(priv->shell);
> +
> + priv->sgmii_pcs.ops = &gsw1xx_sgmii_pcs_ops;
> + priv->sgmii_pcs.poll = 1;
> + __set_bit(PHY_INTERFACE_MODE_SGMII,
> + priv->sgmii_pcs.supported_interfaces);
> + __set_bit(PHY_INTERFACE_MODE_1000BASEX,
> + priv->sgmii_pcs.supported_interfaces);
> + if (priv->gswip.hw_info->supports_2500m)
> + __set_bit(PHY_INTERFACE_MODE_2500BASEX,
> + priv->sgmii_pcs.supported_interfaces);
> +
> + ret = regmap_read(priv->gswip.gswip, GSWIP_VERSION, &priv->gswip.version);
> + if (ret < 0)
> + return ret;
> +
> + ds = devm_kzalloc(dev, sizeof(*ds), GFP_KERNEL);
> + if (!ds)
> + return -ENOMEM;
> +
> + priv->gswip.ds = ds;
> +
> + ret = gswip_allocate_vlans(&priv->gswip);
> + if (ret)
> + return ret;
> +
> + ds->dev = dev;
> + ds->num_ports = GSW1XX_PORTS;
> + ds->priv = &priv->gswip;
> + ds->ops = &gswip_switch_ops;
> + ds->phylink_mac_ops = &gswip_phylink_mac_ops;
> + ds->fdb_isolation = true;
> +
> + /* configure GPIO pin-mux for MMDIO in case of external PHY connected to
> + * SGMII or RGMII as slave interface
> + */
> + regmap_set_bits(priv->gpio, GPIO_ALTSEL0, 3);
> + regmap_set_bits(priv->gpio, GPIO_ALTSEL1, 3);
> +
> + dev_set_drvdata(dev, ds);
> +
> + ret = dsa_register_switch(ds);
> + if (ret) {
> + if (ret != -EPROBE_DEFER)
> + dev_err(dev, "%s: Error %d register DSA switch\n",
> + __func__, ret);
> + return ret;
> + }
> +
> + ret = gswip_validate_cpu_port(ds);
> + if (ret)
> + goto disable_switch;
> +
> + dev_info(dev, "probed GSWIP version %lx mod %lx\n",
> + (priv->gswip.version & GSWIP_VERSION_REV_MASK) >> GSWIP_VERSION_REV_SHIFT,
> + (priv->gswip.version & GSWIP_VERSION_MOD_MASK) >> GSWIP_VERSION_MOD_SHIFT);
> +
> + return 0;
> +
> +disable_switch:
> + gswip_disable_switch(&priv->gswip);
> + dsa_unregister_switch(ds);
> +
> + return ret;
> +}
...
> +static struct mdio_driver gsw1xx_driver = {
> + .probe = gsw1xx_probe,
> + .remove = gsw1xx_remove,
> + .shutdown = gsw1xx_shutdown,
> + .mdiodrv.driver = {
> + .name = "mxl-gsw1xx",
> + .of_match_table = gsw1xx_of_match,
> + },
> +};
--
Alexander Sverdlin
Siemens AG
www.siemens.com
Powered by blists - more mailing lists