[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20260105193016.jlnsvgavlilhync7@skbuf>
Date: Mon, 5 Jan 2026 21:30:16 +0200
From: Vladimir Oltean <olteanv@...il.com>
To: "A. Sverdlin" <alexander.sverdlin@...mens.com>
Cc: netdev@...r.kernel.org, Hauke Mehrtens <hauke@...ke-m.de>,
Andrew Lunn <andrew@...n.ch>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, Daniel Golle <daniel@...rotopia.org>
Subject: Re: [PATCH v3 2/2] net: dsa: mxl-gsw1xx: Support R(G)MII slew rate
configuration
Hi Alexander,
On Mon, Jan 05, 2026 at 06:53:11PM +0100, A. Sverdlin wrote:
> From: Alexander Sverdlin <alexander.sverdlin@...mens.com>
>
> Support newly introduced slew-rate device tree property to configure
> R(G)MII interface pins slew rate. It might be used to reduce the radiated
> emissions.
>
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@...mens.com>
> ---
> Changelog:
> v3:
> - use [pinctrl] standard "slew-rate" property as suggested by Rob
> https://lore.kernel.org/all/20251219204324.GA3881969-robh@kernel.org/
> - better sorted struct gswip_hw_info initialisers as suggested by Daniel
> v2:
> - do not hijack gsw1xx_phylink_mac_select_pcs() for configuring the port,
> introduce struct gswip_hw_info::port_setup callback
> - actively configure "normal" slew rate (if the new DT property is missing)
> - properly use regmap_set_bits() (v1 had reg and value mixed up)
>
> drivers/net/dsa/lantiq/lantiq_gswip.h | 1 +
> drivers/net/dsa/lantiq/lantiq_gswip_common.c | 6 ++++
> drivers/net/dsa/lantiq/mxl-gsw1xx.c | 31 ++++++++++++++++++++
> drivers/net/dsa/lantiq/mxl-gsw1xx.h | 2 ++
> 4 files changed, 40 insertions(+)
>
> diff --git a/drivers/net/dsa/lantiq/lantiq_gswip.h b/drivers/net/dsa/lantiq/lantiq_gswip.h
> index 2e0f2afbadbbc..8fc4c7cc5283a 100644
> --- a/drivers/net/dsa/lantiq/lantiq_gswip.h
> +++ b/drivers/net/dsa/lantiq/lantiq_gswip.h
> @@ -263,6 +263,7 @@ struct gswip_hw_info {
> struct phylink_config *config);
> struct phylink_pcs *(*mac_select_pcs)(struct phylink_config *config,
> phy_interface_t interface);
> + int (*port_setup)(struct dsa_switch *ds, int port);
> };
>
> struct gswip_gphy_fw {
> diff --git a/drivers/net/dsa/lantiq/lantiq_gswip_common.c b/drivers/net/dsa/lantiq/lantiq_gswip_common.c
> index e790f2ef75884..17a61e445f00f 100644
> --- a/drivers/net/dsa/lantiq/lantiq_gswip_common.c
> +++ b/drivers/net/dsa/lantiq/lantiq_gswip_common.c
> @@ -425,6 +425,12 @@ static int gswip_port_setup(struct dsa_switch *ds, int port)
> struct gswip_priv *priv = ds->priv;
> int err;
>
> + if (priv->hw_info->port_setup) {
> + err = priv->hw_info->port_setup(ds, port);
> + if (err)
> + return err;
> + }
> +
> if (!dsa_is_cpu_port(ds, port)) {
> err = gswip_add_single_port_br(priv, port, true);
> if (err)
> diff --git a/drivers/net/dsa/lantiq/mxl-gsw1xx.c b/drivers/net/dsa/lantiq/mxl-gsw1xx.c
> index f8ff8a604bf53..6c290bac537ad 100644
> --- a/drivers/net/dsa/lantiq/mxl-gsw1xx.c
> +++ b/drivers/net/dsa/lantiq/mxl-gsw1xx.c
> @@ -559,6 +559,34 @@ static struct phylink_pcs *gsw1xx_phylink_mac_select_pcs(struct phylink_config *
> }
> }
>
> +static int gsw1xx_port_setup(struct dsa_switch *ds, int port)
> +{
> + struct dsa_port *dp = dsa_to_port(ds, port);
> + struct gsw1xx_priv *gsw1xx_priv;
> + struct gswip_priv *gswip_priv;
> + u32 rate;
> + int ret;
> +
> + if (dp->index != GSW1XX_MII_PORT)
> + return 0;
> +
> + gswip_priv = ds->priv;
> + gsw1xx_priv = container_of(gswip_priv, struct gsw1xx_priv, gswip);
> +
> + ret = of_property_read_u32(dp->dn, "slew-rate", &rate);
> + /* Optional property */
> + if (ret == -EINVAL)
> + return 0;
> + if (ret < 0 || rate > 1) {
> + dev_err(&gsw1xx_priv->mdio_dev->dev, "Invalid slew-rate\n");
> + return (ret < 0) ? ret : -EINVAL;
> + }
> +
> + return regmap_update_bits(gsw1xx_priv->shell, GSW1XX_SHELL_RGMII_SLEW_CFG,
> + RGMII_SLEW_CFG_DRV_TXD | RGMII_SLEW_CFG_DRV_TXC,
> + (RGMII_SLEW_CFG_DRV_TXD | RGMII_SLEW_CFG_DRV_TXC) * rate);
I don't have a particularly strong EE background, but my understanding
is this:
RGMII MACs provide individual slew rate configuration for TXD[3:0] and
for TX_CLK because normally, you'd want to focus on the TX_CLK slew rate
(in the sense of reducing EMI) more than on the TXD[3:0] slew rate.
This is for 2 reasons:
(1) the EMI noise produced by TX_CLK is in a much narrower spectrum
(runs at fixed 125/25/2.5 MHz) than TXD[3:0] (pseudo-random data).
(2) reducing the slew rate for TXD[3:0] risks introducing inter-symbol
interference, risk which does not exist for TX_CLK
Your dt-binding does not permit configuring the slew rates separately,
even though the hardware permits that. Was it intentional?
> +}
> +
> static struct regmap *gsw1xx_regmap_init(struct gsw1xx_priv *priv,
> const char *name,
> unsigned int reg_base,
> @@ -707,6 +735,7 @@ static const struct gswip_hw_info gsw12x_data = {
> .mac_select_pcs = gsw1xx_phylink_mac_select_pcs,
> .phylink_get_caps = &gsw1xx_phylink_get_caps,
> .supports_2500m = true,
> + .port_setup = gsw1xx_port_setup,
> .pce_microcode = &gsw1xx_pce_microcode,
> .pce_microcode_size = ARRAY_SIZE(gsw1xx_pce_microcode),
> .tag_protocol = DSA_TAG_PROTO_MXL_GSW1XX,
> @@ -720,6 +749,7 @@ static const struct gswip_hw_info gsw140_data = {
> .mac_select_pcs = gsw1xx_phylink_mac_select_pcs,
> .phylink_get_caps = &gsw1xx_phylink_get_caps,
> .supports_2500m = true,
> + .port_setup = gsw1xx_port_setup,
> .pce_microcode = &gsw1xx_pce_microcode,
> .pce_microcode_size = ARRAY_SIZE(gsw1xx_pce_microcode),
> .tag_protocol = DSA_TAG_PROTO_MXL_GSW1XX,
> @@ -732,6 +762,7 @@ static const struct gswip_hw_info gsw141_data = {
> .mii_port_reg_offset = -GSW1XX_MII_PORT,
> .mac_select_pcs = gsw1xx_phylink_mac_select_pcs,
> .phylink_get_caps = gsw1xx_phylink_get_caps,
> + .port_setup = gsw1xx_port_setup,
> .pce_microcode = &gsw1xx_pce_microcode,
> .pce_microcode_size = ARRAY_SIZE(gsw1xx_pce_microcode),
> .tag_protocol = DSA_TAG_PROTO_MXL_GSW1XX,
> diff --git a/drivers/net/dsa/lantiq/mxl-gsw1xx.h b/drivers/net/dsa/lantiq/mxl-gsw1xx.h
> index 38e03c048a26c..8c0298b2b7663 100644
> --- a/drivers/net/dsa/lantiq/mxl-gsw1xx.h
> +++ b/drivers/net/dsa/lantiq/mxl-gsw1xx.h
> @@ -110,6 +110,8 @@
> #define GSW1XX_RST_REQ_SGMII_SHELL BIT(5)
> /* RGMII PAD Slew Control Register */
> #define GSW1XX_SHELL_RGMII_SLEW_CFG 0x78
> +#define RGMII_SLEW_CFG_DRV_TXC BIT(2)
> +#define RGMII_SLEW_CFG_DRV_TXD BIT(3)
> #define RGMII_SLEW_CFG_RX_2_5_V BIT(4)
> #define RGMII_SLEW_CFG_TX_2_5_V BIT(5)
>
> --
> 2.52.0
>
Powered by blists - more mailing lists