Re: [PATCH v3 2/2] net: dsa: mxl-gsw1xx: Support R(G)MII slew rate configuration

[Vladimir Oltean <olteanv@gmail.com>]

Hi Alexander,

On Mon, Jan 05, 2026 at 06:53:11PM +0100, A. Sverdlin wrote:
> From: Alexander Sverdlin <alexander.sverdlin@siemens.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@siemens.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
>