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);
+}
+
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.0Hi 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
>
Hi Vladimir, On Mon, 2026-01-05 at 21:30 +0200, Vladimir Oltean wrote: > > + 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? thanks for the hint! This is definitely something I need to discuss with HW colleagues and get back to you! -- Alexander Sverdlin Siemens AG www.siemens.com
Hello Vladimir, Rob! On Mon, 2026-01-05 at 22:00 +0100, Alexander Sverdlin wrote: > > > + 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? > > thanks for the hint! This is definitely something I need to discuss with HW > colleagues and get back to you! Vladimir, according to the responsible HW colleague, it's OK and is desired to have TXD in "slow" as long as Setup-/Hold-Timing is in spec. I do understand, that this is board-specific. Do you propose to introduce two separate properties? Rob, in such case just "slew-rate" probably wouldn't fit any longer and I'd need to go back to "maxlinear,slew-rate-txd" and "maxlinear,slew-rate-txc" probably? -- Alexander Sverdlin Siemens AG www.siemens.com
On Tue, Jan 06, 2026 at 09:41:13AM +0000, Sverdlin, Alexander wrote: > Hello Vladimir, Rob! > > On Mon, 2026-01-05 at 22:00 +0100, Alexander Sverdlin wrote: > > > > + 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? > > > > thanks for the hint! This is definitely something I need to discuss with HW > > colleagues and get back to you! > > Vladimir, according to the responsible HW colleague, it's OK and is desired > to have TXD in "slow" as long as Setup-/Hold-Timing is in spec. > > I do understand, that this is board-specific. Do you propose to introduce > two separate properties? > > Rob, in such case just "slew-rate" probably wouldn't fit any longer and > I'd need to go back to "maxlinear,slew-rate-txd" and "maxlinear,slew-rate-txc" > probably? I see Rob has reviewed the binding in this form already, but I think the rule of thumb that we could apply in this case is to still describe the clock and data slew rates separately. Like Russell points out in a separate thread, it's simpler to do this from the beginning rather than end up with 3 properties you'd have to maintain, if you later need individual control. https://lore.kernel.org/netdev/aTB0x6JGcGUM04UX@shell.armlinux.org.uk/ Sadly I don't have the expertise to give any advice on how that would translate into dt-bindings. Does it make sense to implement a full pin controller device driver for the registers GPIO_DRIVE0_CFG -> RGMII_SLEW_CFG?
© 2016 - 2026 Red Hat, Inc.