mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net-next v3 0/3] net: dsa: realtek: EEE support for RTL8365MB-VC
@ 2026-09-16  9:00 Oleksij Rempel
  2026-09-16  9:01 ` [PATCH net-next v3 1/3] net: phy: realtek: use C45 for RTL8365MB-VC internal PHY MMD access Oleksij Rempel
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Oleksij Rempel @ 2026-09-16  9:00 UTC (permalink / raw)
  To: Linus Walleij, Luiz Angelo Daros de Luca, Andrew Lunn,
	Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni
  Cc: Oleksij Rempel, kernel, linux-kernel, Ahmad Fatoum,
	Alvin Šipraga, netdev

On these switches EEE is enabled by default, but so far it is neither
visible nor controllable from user space. This series wires EEE up to
phylib/phylink on the RTL8365MB-VC integrated-PHY ports so it can be
observed and controlled per port via ethtool

Oleksij Rempel (3):
  net: phy: realtek: use C45 for RTL8365MB-VC internal PHY MMD access
  net: dsa: realtek: rtl8365mb: extract PHY OCP address halves with
    FIELD_GET
  net: dsa: realtek: rtl8365mb: add EEE support

 drivers/net/dsa/realtek/realtek.h        |   4 +
 drivers/net/dsa/realtek/rtl8365mb_main.c | 137 ++++++++++++++++++++++-
 drivers/net/dsa/realtek/rtl83xx.c        |  20 ++++
 drivers/net/phy/realtek/realtek_main.c   |   2 +
 4 files changed, 161 insertions(+), 2 deletions(-)

--
2.47.3


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH net-next v3 1/3] net: phy: realtek: use C45 for RTL8365MB-VC internal PHY MMD access
  2026-09-16  9:00 [PATCH net-next v3 0/3] net: dsa: realtek: EEE support for RTL8365MB-VC Oleksij Rempel
@ 2026-09-16  9:01 ` Oleksij Rempel
  2026-09-18  3:03   ` netdev-bot+sashiko
  2026-09-16  9:01 ` [PATCH net-next v3 2/3] net: dsa: realtek: rtl8365mb: extract PHY OCP address halves with FIELD_GET Oleksij Rempel
  2026-09-16  9:01 ` [PATCH net-next v3 3/3] net: dsa: realtek: rtl8365mb: add EEE support Oleksij Rempel
  2 siblings, 1 reply; 8+ messages in thread
From: Oleksij Rempel @ 2026-09-16  9:01 UTC (permalink / raw)
  To: Linus Walleij, Luiz Angelo Daros de Luca, Andrew Lunn,
	Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni
  Cc: Oleksij Rempel, kernel, linux-kernel, Ahmad Fatoum,
	Alvin Šipraga, netdev

The RTL8365MB-VC PHYs are integrated into the switch and have no MMD
registers of their own. What phylib accesses as MMD registers (EEE)
actually lives in the switch's OCP registers; the realtek DSA driver maps
those MMD requests onto OCP and exposes them over C45.

For a C22 PHY, phy_read_mmd()/phy_write_mmd() would use C22 indirect-MMD,
which this hardware does not implement. Point .read_mmd/.write_mmd at
genphy_{read,write}_mmd_c45() so phylib's MMD accesses go over C45 to the
DSA driver's mapping instead.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Linus Walleij <linusw@kernel.org>
Reviewed-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
---
Anticipated reviewer questions:

Q: .read_mmd/.write_mmd now use the C45 helpers, but the DSA user MDIO bus
   only gains .read_c45/.write_c45 in the next patch -- is this bisect-broken?
A: No. Until then an MMD access returns -EOPNOTSUPP, but the only one that runs
   is the EEE-ability read in phy_probe(), which phylib ignores; probe, link and
   traffic are unaffected. ethtool --show-eee reports EEE unsupported here -- the
   same as before this series, since these ports gain EEE only in the next patch
   -- so it is not a regression. Verified on an RTL8365MB-VC.
---
 drivers/net/phy/realtek/realtek_main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/phy/realtek/realtek_main.c b/drivers/net/phy/realtek/realtek_main.c
index 97b0b67b9900..b55691681df6 100644
--- a/drivers/net/phy/realtek/realtek_main.c
+++ b/drivers/net/phy/realtek/realtek_main.c
@@ -3338,6 +3338,8 @@ static struct phy_driver realtek_drvs[] = {
 	}, {
 		PHY_ID_MATCH_EXACT(0x001cc942),
 		.name		= "RTL8365MB-VC Gigabit Ethernet",
+		.read_mmd	= genphy_read_mmd_c45,
+		.write_mmd	= genphy_write_mmd_c45,
 		/* Interrupt handling analogous to RTL8366RB */
 		.config_intr	= genphy_no_config_intr,
 		.handle_interrupt = genphy_handle_interrupt_no_ack,
-- 
2.47.3


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH net-next v3 2/3] net: dsa: realtek: rtl8365mb: extract PHY OCP address halves with FIELD_GET
  2026-09-16  9:00 [PATCH net-next v3 0/3] net: dsa: realtek: EEE support for RTL8365MB-VC Oleksij Rempel
  2026-09-16  9:01 ` [PATCH net-next v3 1/3] net: phy: realtek: use C45 for RTL8365MB-VC internal PHY MMD access Oleksij Rempel
@ 2026-09-16  9:01 ` Oleksij Rempel
  2026-09-16 12:36   ` Andrew Lunn
  2026-09-16 12:49   ` Linus Walleij
  2026-09-16  9:01 ` [PATCH net-next v3 3/3] net: dsa: realtek: rtl8365mb: add EEE support Oleksij Rempel
  2 siblings, 2 replies; 8+ messages in thread
From: Oleksij Rempel @ 2026-09-16  9:01 UTC (permalink / raw)
  To: Linus Walleij, Luiz Angelo Daros de Luca, Andrew Lunn,
	Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni
  Cc: Oleksij Rempel, kernel, linux-kernel, Ahmad Fatoum,
	Alvin Šipraga, netdev

Extract the two OCP address halves with FIELD_GET() before handing them
to FIELD_PREP() to build the ADDRESS register value.

rtl8365mb_phy_ocp_prepare() previously fed FIELD_PREP() the raw
ocp_addr >> 1 and >> 6. FIELD_PREP() masks the value to the field at run
time, so that was correct for the current callers, which all pass a
run-time register address.

FIELD_PREP()'s compile-time width check only fires for a compile-time
constant value. None reached it here: the address is computed as
PHYREG_BASE + regnum * 2 from a run-time regnum. The EEE support that
follows adds callers with constant OCP addresses (0xa5c4 etc.); clang
folds those into the inlined ocp_prepare(), so the shifted constant now
reaches the check, overflows the 5-/4-bit field and breaks the build
(gcc does not fold it here). FIELD_GET() narrows each half to the field
width first, so the value already fits.

No functional change.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
New in v3, fixing the clang FIELD_PREP build failure Jakub Kicinski
reported against v2.
---
 drivers/net/dsa/realtek/rtl8365mb_main.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/realtek/rtl8365mb_main.c b/drivers/net/dsa/realtek/rtl8365mb_main.c
index f8220d832e46..efde1f3b604e 100644
--- a/drivers/net/dsa/realtek/rtl8365mb_main.c
+++ b/drivers/net/dsa/realtek/rtl8365mb_main.c
@@ -199,6 +199,14 @@
 #define   RTL8365MB_GPHY_OCP_MSB_0_CFG_CPU_OCPADR_MASK	0x0FC0
 #define RTL8365MB_PHY_OCP_ADDR_PREFIX_MASK		0xFC00
 
+/* The full 16-bit OCP address is split across two registers: bits [15:10] are
+ * the prefix (RTL8365MB_PHY_OCP_ADDR_PREFIX_MASK above), and bits [9:1] go into
+ * the ADDRESS register as two fields, [5:1] and [9:6]. Bit 0 is always 0 - PHY
+ * OCP registers are 2-byte aligned.
+ */
+#define RTL8365MB_PHY_OCP_ADDR_5_1_MASK			GENMASK(5, 1)
+#define RTL8365MB_PHY_OCP_ADDR_9_6_MASK			GENMASK(9, 6)
+
 /* The PHY OCP addresses of PHY registers 0~31 start here */
 #define RTL8365MB_PHY_OCP_ADDR_PHYREG_BASE		0xA400
 
@@ -871,6 +879,8 @@ static int rtl8365mb_phy_poll_busy(struct realtek_priv *priv)
 static int rtl8365mb_phy_ocp_prepare(struct realtek_priv *priv, int phy,
 				     u32 ocp_addr)
 {
+	u16 ocp_addr_lo = FIELD_GET(RTL8365MB_PHY_OCP_ADDR_5_1_MASK, ocp_addr);
+	u16 ocp_addr_hi = FIELD_GET(RTL8365MB_PHY_OCP_ADDR_9_6_MASK, ocp_addr);
 	u32 val;
 	int ret;
 
@@ -887,9 +897,9 @@ static int rtl8365mb_phy_ocp_prepare(struct realtek_priv *priv, int phy,
 	val = RTL8365MB_PHY_BASE;
 	val |= FIELD_PREP(RTL8365MB_INDIRECT_ACCESS_ADDRESS_PHYNUM_MASK, phy);
 	val |= FIELD_PREP(RTL8365MB_INDIRECT_ACCESS_ADDRESS_OCPADR_5_1_MASK,
-			  ocp_addr >> 1);
+			  ocp_addr_lo);
 	val |= FIELD_PREP(RTL8365MB_INDIRECT_ACCESS_ADDRESS_OCPADR_9_6_MASK,
-			  ocp_addr >> 6);
+			  ocp_addr_hi);
 	ret = regmap_write(priv->map_nolock,
 			   RTL8365MB_INDIRECT_ACCESS_ADDRESS_REG, val);
 	if (ret)
-- 
2.47.3


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH net-next v3 3/3] net: dsa: realtek: rtl8365mb: add EEE support
  2026-09-16  9:00 [PATCH net-next v3 0/3] net: dsa: realtek: EEE support for RTL8365MB-VC Oleksij Rempel
  2026-09-16  9:01 ` [PATCH net-next v3 1/3] net: phy: realtek: use C45 for RTL8365MB-VC internal PHY MMD access Oleksij Rempel
  2026-09-16  9:01 ` [PATCH net-next v3 2/3] net: dsa: realtek: rtl8365mb: extract PHY OCP address halves with FIELD_GET Oleksij Rempel
@ 2026-09-16  9:01 ` Oleksij Rempel
  2026-09-18  3:03   ` netdev-bot+sashiko
  2 siblings, 1 reply; 8+ messages in thread
From: Oleksij Rempel @ 2026-09-16  9:01 UTC (permalink / raw)
  To: Linus Walleij, Luiz Angelo Daros de Luca, Andrew Lunn,
	Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni
  Cc: Oleksij Rempel, kernel, linux-kernel, Ahmad Fatoum,
	Alvin Šipraga, netdev

Add EEE support on the integrated-PHY ports, controllable per port via
ethtool.

The integrated PHYs have no MMD registers; their EEE registers live in the
switch's OCP space. phylib accesses EEE as MMD registers, so add
phy_{read,write}_c45 realtek_ops that map those MMD accesses onto their OCP
addresses, wired as the user MII bus C45 handlers when the chip provides
them (rtl8366rb etc. unaffected).

The hardware manages LPI on its own, so advertise EEE to phylink with
no-op mac_{en,dis}able_tx_lpi callbacks. The LPI timer is a single global
per-speed register, so .set_mac_eee rejects the per-port TX LPI knobs
rather than silently ignoring them.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Linus Walleij <linusw@kernel.org>
Reviewed-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
---
Changes since v2:
- The clang FIELD_PREP build error reported on v2 (Jakub Kicinski) is now
  fixed by the preceding patch ("net: dsa: realtek: rtl8365mb: extract PHY
  OCP address halves with FIELD_GET"); this patch itself is unchanged.

Changes since v1:
- .set_mac_eee: return -EOPNOTSUPP instead of -EINVAL for the rejected
  TX LPI cases -- the hardware cannot support them, it is not an invalid
  configuration (Andrew Lunn).
---
 drivers/net/dsa/realtek/realtek.h        |   4 +
 drivers/net/dsa/realtek/rtl8365mb_main.c | 123 +++++++++++++++++++++++
 drivers/net/dsa/realtek/rtl83xx.c        |  20 ++++
 3 files changed, 147 insertions(+)

diff --git a/drivers/net/dsa/realtek/realtek.h b/drivers/net/dsa/realtek/realtek.h
index 75a127545e5d..1c0ca6daf545 100644
--- a/drivers/net/dsa/realtek/realtek.h
+++ b/drivers/net/dsa/realtek/realtek.h
@@ -156,6 +156,10 @@ struct realtek_ops {
 	int	(*phy_read)(struct realtek_priv *priv, int phy, int regnum);
 	int	(*phy_write)(struct realtek_priv *priv, int phy, int regnum,
 			     u16 val);
+	int	(*phy_read_c45)(struct realtek_priv *priv, int phy, int devad,
+				int regnum);
+	int	(*phy_write_c45)(struct realtek_priv *priv, int phy, int devad,
+				 int regnum, u16 val);
 };
 
 struct realtek_variant {
diff --git a/drivers/net/dsa/realtek/rtl8365mb_main.c b/drivers/net/dsa/realtek/rtl8365mb_main.c
index efde1f3b604e..87ec924a0611 100644
--- a/drivers/net/dsa/realtek/rtl8365mb_main.c
+++ b/drivers/net/dsa/realtek/rtl8365mb_main.c
@@ -101,6 +101,7 @@
 #include <linux/regmap.h>
 #include <linux/if_bridge.h>
 #include <linux/if_vlan.h>
+#include <linux/mdio.h>
 #include <linux/phylink.h>
 
 #include "realtek.h"
@@ -210,6 +211,10 @@
 /* The PHY OCP addresses of PHY registers 0~31 start here */
 #define RTL8365MB_PHY_OCP_ADDR_PHYREG_BASE		0xA400
 
+#define RTL8365MB_PHY_OCP_ADDR_EEE_ABLE			0xA5C4
+#define RTL8365MB_PHY_OCP_ADDR_EEE_ADV			0xA5D0
+#define RTL8365MB_PHY_OCP_ADDR_EEE_LPABLE		0xA5D2
+
 /* External interface port mode values - used in DIGITAL_INTERFACE_SELECT */
 #define RTL8365MB_EXT_PORT_MODE_DISABLE		0
 #define RTL8365MB_EXT_PORT_MODE_RGMII		1
@@ -1050,6 +1055,66 @@ static int rtl8365mb_phy_write(struct realtek_priv *priv, int phy, int regnum,
 	return 0;
 }
 
+static int rtl8365mb_phy_read_c45(struct realtek_priv *priv, int phy, int devad,
+				  int regnum)
+{
+	u32 ocp_addr;
+	u16 val;
+	int ret;
+
+	if (phy > RTL8365MB_PHYADDRMAX)
+		return -EINVAL;
+
+	if (devad == MDIO_MMD_PCS && regnum == MDIO_PCS_EEE_ABLE)
+		ocp_addr = RTL8365MB_PHY_OCP_ADDR_EEE_ABLE;
+	else if (devad == MDIO_MMD_AN && regnum == MDIO_AN_EEE_ADV)
+		ocp_addr = RTL8365MB_PHY_OCP_ADDR_EEE_ADV;
+	else if (devad == MDIO_MMD_AN && regnum == MDIO_AN_EEE_LPABLE)
+		ocp_addr = RTL8365MB_PHY_OCP_ADDR_EEE_LPABLE;
+	else
+		/* Only the EEE registers are mapped; others read as 0, as the
+		 * hardware does, so the generic MMD code is not tripped up by
+		 * an error.
+		 */
+		return 0;
+
+	ret = rtl8365mb_phy_ocp_read(priv, phy, ocp_addr, &val);
+	if (ret) {
+		dev_err(priv->dev,
+			"failed to read PHY%d OCP %04x, ret %d\n", phy, ocp_addr,
+			ret);
+		return ret;
+	}
+
+	return val;
+}
+
+static int rtl8365mb_phy_write_c45(struct realtek_priv *priv, int phy,
+				   int devad, int regnum, u16 val)
+{
+	int ret;
+
+	if (phy > RTL8365MB_PHYADDRMAX)
+		return -EINVAL;
+
+	/* Only the EEE advertisement register is writable; writes to other
+	 * registers are ignored, as the hardware does.
+	 */
+	if (devad != MDIO_MMD_AN || regnum != MDIO_AN_EEE_ADV)
+		return 0;
+
+	ret = rtl8365mb_phy_ocp_write(priv, phy, RTL8365MB_PHY_OCP_ADDR_EEE_ADV,
+				      val);
+	if (ret) {
+		dev_err(priv->dev,
+			"failed to write PHY%d OCP %04x, ret %d\n", phy,
+			RTL8365MB_PHY_OCP_ADDR_EEE_ADV, ret);
+		return ret;
+	}
+
+	return 0;
+}
+
 static const struct rtl8365mb_extint *
 rtl8365mb_get_port_extint(struct realtek_priv *priv, int port)
 {
@@ -1649,6 +1714,14 @@ static void rtl8365mb_phylink_get_caps(struct dsa_switch *ds, int port,
 		 */
 		__set_bit(PHY_INTERFACE_MODE_GMII,
 			  config->supported_interfaces);
+
+		/* Integrated PHYs support EEE at 100M/1G; the hardware manages
+		 * LPI on its own, so just advertise LPI awareness to phylink.
+		 */
+		memcpy(config->lpi_interfaces, config->supported_interfaces,
+		       sizeof(config->lpi_interfaces));
+		config->lpi_capabilities = MAC_100FD | MAC_1000FD;
+		config->eee_enabled_default = true;
 		return;
 	}
 
@@ -3285,18 +3358,66 @@ static int rtl8365mb_detect(struct realtek_priv *priv)
 	return 0;
 }
 
+static int rtl8365mb_phylink_mac_enable_tx_lpi(struct phylink_config *config,
+					       u32 timer, bool tx_clock_stop)
+{
+	/* The hardware manages LPI itself; there is no MAC-level LPI control.
+	 * This callback only signals LPI awareness to phylink.
+	 */
+	return 0;
+}
+
+static void rtl8365mb_phylink_mac_disable_tx_lpi(struct phylink_config *config)
+{
+}
+
 static const struct phylink_mac_ops rtl8365mb_phylink_mac_ops = {
 	.mac_select_pcs = rtl8365mb_phylink_mac_select_pcs,
 	.mac_config = rtl8365mb_phylink_mac_config,
 	.mac_link_down = rtl8365mb_phylink_mac_link_down,
 	.mac_link_up = rtl8365mb_phylink_mac_link_up,
+	.mac_enable_tx_lpi = rtl8365mb_phylink_mac_enable_tx_lpi,
+	.mac_disable_tx_lpi = rtl8365mb_phylink_mac_disable_tx_lpi,
 };
 
+static bool rtl8365mb_support_eee(struct dsa_switch *ds, int port)
+{
+	/* Only integrated-PHY ports support EEE, not the external RGMII ports. */
+	return !rtl8365mb_get_port_extint(ds->priv, port);
+}
+
+static int rtl8365mb_set_mac_eee(struct dsa_switch *ds, int port,
+				 struct ethtool_keee *e)
+{
+	struct realtek_priv *priv = ds->priv;
+
+	/* The only LPI timing control (tx_lpi_timer) is a single global
+	 * per-speed register shared by all ports, so it cannot be set from this
+	 * per-port callback; leave it at its reset default. Per-port EEE is
+	 * driven through the PHY advertisement. Reject the per-port TX LPI knobs
+	 * rather than silently ignoring them.
+	 */
+	if (!e->tx_lpi_enabled) {
+		dev_err(priv->dev, "disabling EEE TX LPI is not supported\n");
+		return -EOPNOTSUPP;
+	}
+
+	if (e->tx_lpi_timer) {
+		dev_err(priv->dev,
+			"setting the EEE TX LPI timer is not supported\n");
+		return -EOPNOTSUPP;
+	}
+
+	return 0;
+}
+
 static const struct dsa_switch_ops rtl8365mb_switch_ops = {
 	.get_tag_protocol = rtl8365mb_get_tag_protocol,
 	.change_tag_protocol = rtl8365mb_change_tag_protocol,
 	.setup = rtl8365mb_setup,
 	.teardown = rtl8365mb_teardown,
+	.support_eee = rtl8365mb_support_eee,
+	.set_mac_eee = rtl8365mb_set_mac_eee,
 	.phylink_get_caps = rtl8365mb_phylink_get_caps,
 	.port_bridge_join = rtl83xx_port_bridge_join,
 	.port_bridge_leave = rtl83xx_port_bridge_leave,
@@ -3342,6 +3463,8 @@ static const struct realtek_ops rtl8365mb_ops = {
 	.l2_flush = rtl8365mb_l2_flush,
 	.phy_read = rtl8365mb_phy_read,
 	.phy_write = rtl8365mb_phy_write,
+	.phy_read_c45 = rtl8365mb_phy_read_c45,
+	.phy_write_c45 = rtl8365mb_phy_write_c45,
 };
 
 static const char *const rtl8365mb_supplies[] = {
diff --git a/drivers/net/dsa/realtek/rtl83xx.c b/drivers/net/dsa/realtek/rtl83xx.c
index 8121ec12fd8b..ad42c05d0a88 100644
--- a/drivers/net/dsa/realtek/rtl83xx.c
+++ b/drivers/net/dsa/realtek/rtl83xx.c
@@ -62,6 +62,22 @@ static int rtl83xx_user_mdio_write(struct mii_bus *bus, int addr, int regnum,
 	return priv->ops->phy_write(priv, addr, regnum, val);
 }
 
+static int rtl83xx_user_mdio_read_c45(struct mii_bus *bus, int addr, int devad,
+				      int regnum)
+{
+	struct realtek_priv *priv = bus->priv;
+
+	return priv->ops->phy_read_c45(priv, addr, devad, regnum);
+}
+
+static int rtl83xx_user_mdio_write_c45(struct mii_bus *bus, int addr, int devad,
+				       int regnum, u16 val)
+{
+	struct realtek_priv *priv = bus->priv;
+
+	return priv->ops->phy_write_c45(priv, addr, devad, regnum, val);
+}
+
 /**
  * rtl83xx_setup_user_mdio() - register the user mii bus driver
  * @ds: DSA switch associated with this user_mii_bus
@@ -95,6 +111,10 @@ int rtl83xx_setup_user_mdio(struct dsa_switch *ds)
 	bus->name = "Realtek user MII";
 	bus->read = rtl83xx_user_mdio_read;
 	bus->write = rtl83xx_user_mdio_write;
+	if (priv->ops->phy_read_c45)
+		bus->read_c45 = rtl83xx_user_mdio_read_c45;
+	if (priv->ops->phy_write_c45)
+		bus->write_c45 = rtl83xx_user_mdio_write_c45;
 	snprintf(bus->id, MII_BUS_ID_SIZE, "%s:user_mii", dev_name(priv->dev));
 	bus->parent = priv->dev;
 
-- 
2.47.3


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next v3 2/3] net: dsa: realtek: rtl8365mb: extract PHY OCP address halves with FIELD_GET
  2026-09-16  9:01 ` [PATCH net-next v3 2/3] net: dsa: realtek: rtl8365mb: extract PHY OCP address halves with FIELD_GET Oleksij Rempel
@ 2026-09-16 12:36   ` Andrew Lunn
  2026-09-16 12:49   ` Linus Walleij
  1 sibling, 0 replies; 8+ messages in thread
From: Andrew Lunn @ 2026-09-16 12:36 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: Linus Walleij, Luiz Angelo Daros de Luca, Vladimir Oltean,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	kernel, linux-kernel, Ahmad Fatoum, Alvin Šipraga, netdev

On Wed, Sep 16, 2026 at 11:01:01AM +0200, Oleksij Rempel wrote:
> Extract the two OCP address halves with FIELD_GET() before handing them
> to FIELD_PREP() to build the ADDRESS register value.
> 
> rtl8365mb_phy_ocp_prepare() previously fed FIELD_PREP() the raw
> ocp_addr >> 1 and >> 6. FIELD_PREP() masks the value to the field at run
> time, so that was correct for the current callers, which all pass a
> run-time register address.
> 
> FIELD_PREP()'s compile-time width check only fires for a compile-time
> constant value. None reached it here: the address is computed as
> PHYREG_BASE + regnum * 2 from a run-time regnum. The EEE support that
> follows adds callers with constant OCP addresses (0xa5c4 etc.); clang
> folds those into the inlined ocp_prepare(), so the shifted constant now
> reaches the check, overflows the 5-/4-bit field and breaks the build
> (gcc does not fold it here). FIELD_GET() narrows each half to the field
> width first, so the value already fits.
> 
> No functional change.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next v3 2/3] net: dsa: realtek: rtl8365mb: extract PHY OCP address halves with FIELD_GET
  2026-09-16  9:01 ` [PATCH net-next v3 2/3] net: dsa: realtek: rtl8365mb: extract PHY OCP address halves with FIELD_GET Oleksij Rempel
  2026-09-16 12:36   ` Andrew Lunn
@ 2026-09-16 12:49   ` Linus Walleij
  1 sibling, 0 replies; 8+ messages in thread
From: Linus Walleij @ 2026-09-16 12:49 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: Luiz Angelo Daros de Luca, Andrew Lunn, Vladimir Oltean,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	kernel, linux-kernel, Ahmad Fatoum, Alvin Šipraga, netdev

On Wed, Sep 16, 2026 at 11:01 AM Oleksij Rempel <o.rempel@pengutronix.de> wrote:

> Extract the two OCP address halves with FIELD_GET() before handing them
> to FIELD_PREP() to build the ADDRESS register value.
>
> rtl8365mb_phy_ocp_prepare() previously fed FIELD_PREP() the raw
> ocp_addr >> 1 and >> 6. FIELD_PREP() masks the value to the field at run
> time, so that was correct for the current callers, which all pass a
> run-time register address.
>
> FIELD_PREP()'s compile-time width check only fires for a compile-time
> constant value. None reached it here: the address is computed as
> PHYREG_BASE + regnum * 2 from a run-time regnum. The EEE support that
> follows adds callers with constant OCP addresses (0xa5c4 etc.); clang
> folds those into the inlined ocp_prepare(), so the shifted constant now
> reaches the check, overflows the 5-/4-bit field and breaks the build
> (gcc does not fold it here). FIELD_GET() narrows each half to the field
> width first, so the value already fits.
>
> No functional change.
>
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>

Reviewed-by: Linus Walleij <linusw@kernel.org>

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next v3 1/3] net: phy: realtek: use C45 for RTL8365MB-VC internal PHY MMD access
  2026-09-16  9:01 ` [PATCH net-next v3 1/3] net: phy: realtek: use C45 for RTL8365MB-VC internal PHY MMD access Oleksij Rempel
@ 2026-09-18  3:03   ` netdev-bot+sashiko
  0 siblings, 0 replies; 8+ messages in thread
From: netdev-bot+sashiko @ 2026-09-18  3:03 UTC (permalink / raw)
  To: o.rempel
  Cc: linusw, luizluca, andrew, olteanv, davem, edumazet, kuba, pabeni,
	kernel, linux-kernel, a.fatoum, alvin.sipraga, netdev

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 0 · Low: 1

- [Low] The RTL8365MB-VC phy_driver entry (PHY ID 0x001cc942) is pointed
  at genphy_read_mmd_c45()/genphy_write_mmd_c45(), which go straight to…

--

net: phy: realtek: use C45 for RTL8365MB-VC internal PHY MMD access

This points the RTL8365MB-VC phy_driver entry (PHY ID 0x001cc942) at
genphy_read_mmd_c45()/genphy_write_mmd_c45() so that phylib MMD accesses
are issued as C45 transfers to the Realtek DSA driver, which maps them
onto the switch OCP registers.

> diff --git a/drivers/net/phy/realtek/realtek_main.c b/drivers/net/phy/realtek/realtek_main.c
> index 97b0b67b9900c..b55691681df6e 100644
> --- a/drivers/net/phy/realtek/realtek_main.c
> +++ b/drivers/net/phy/realtek/realtek_main.c
> @@ -3338,6 +3338,8 @@ static struct phy_driver realtek_drvs[] = {
>  	}, {
>  		PHY_ID_MATCH_EXACT(0x001cc942),
>  		.name		= "RTL8365MB-VC Gigabit Ethernet",
> +		.read_mmd	= genphy_read_mmd_c45,
> +		.write_mmd	= genphy_write_mmd_c45,

[Severity: Low]
This isn't a bug, but the commit message describes the DSA-side mapping in
the present tense:

  "the realtek DSA driver maps those MMD requests onto OCP and exposes them
   over C45"

At this commit, rtl83xx_setup_user_mdio() in
drivers/net/dsa/realtek/rtl83xx.c only installs the C22 accessors:

	bus->read = rtl83xx_user_mdio_read;
	bus->write = rtl83xx_user_mdio_write;

The read_c45/write_c45 callbacks and the MMD to OCP mapping arrive later in
the series, in "net: dsa: realtek: rtl8365mb: add EEE support".

genphy_read_mmd_c45()/genphy_write_mmd_c45() go straight to
__mdiobus_c45_read()/__mdiobus_c45_write() with no C22 fallback, and those
return -EOPNOTSUPP when bus->read_c45 is NULL:

	if (bus->read_c45)
		retval = bus->read_c45(bus, addr, devad, regnum);
	else
		retval = -EOPNOTSUPP;

so every MMD access on these ports fails until the later patch lands.

The effect at this intermediate commit looks benign: the only MMD access
that actually runs is genphy_c45_read_eee_abilities() from
genphy_read_abilities(), whose error is explicitly discarded:

	/* This is optional functionality. If not supported, we may get an error
	 * which should be ignored.
	 */
	genphy_c45_read_eee_abilities(phydev);

and phy_check_link_status() only treats genphy_c45_eee_is_active() as a
boolean (err > 0), so a failure just leaves eee_active/enable_tx_lpi false.

Would it be worth wording that sentence as a forward reference to the later
patch, so the commit does not claim an in-tree mapping that only exists
after the following patch in the series?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260916090102.349157-1-o.rempel%40pengutronix.de

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next v3 3/3] net: dsa: realtek: rtl8365mb: add EEE support
  2026-09-16  9:01 ` [PATCH net-next v3 3/3] net: dsa: realtek: rtl8365mb: add EEE support Oleksij Rempel
@ 2026-09-18  3:03   ` netdev-bot+sashiko
  0 siblings, 0 replies; 8+ messages in thread
From: netdev-bot+sashiko @ 2026-09-18  3:03 UTC (permalink / raw)
  To: o.rempel
  Cc: linusw, luizluca, andrew, olteanv, davem, edumazet, kuba, pabeni,
	kernel, linux-kernel, a.fatoum, alvin.sipraga, netdev

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 0

- [Medium] config->eee_enabled_default = true (rtl8365mb_main.c:1724)
  turns EEE on out of the box for every integrated-PHY port; no user…

--

net: dsa: realtek: rtl8365mb: add EEE support

This adds EEE support for the integrated-PHY ports of rtl8365mb by mapping
the EEE MMD registers onto the switch OCP address space through new
phy_{read,write}_c45 realtek_ops. It also advertises LPI awareness to
phylink with no-op mac_{en,dis}able_tx_lpi callbacks and rejects the
per-port TX LPI knobs in .set_mac_eee.

> diff --git a/drivers/net/dsa/realtek/rtl8365mb_main.c b/drivers/net/dsa/realtek/rtl8365mb_main.c
> index efde1f3b604e1..87ec924a06118 100644
> --- a/drivers/net/dsa/realtek/rtl8365mb_main.c
> +++ b/drivers/net/dsa/realtek/rtl8365mb_main.c

[ ... ]

> @@ -1649,6 +1714,14 @@ static void rtl8365mb_phylink_get_caps(struct dsa_switch *ds, int port,
>  		 */
>  		__set_bit(PHY_INTERFACE_MODE_GMII,
>  			  config->supported_interfaces);
> +
> +		/* Integrated PHYs support EEE at 100M/1G; the hardware manages
> +		 * LPI on its own, so just advertise LPI awareness to phylink.
> +		 */
> +		memcpy(config->lpi_interfaces, config->supported_interfaces,
> +		       sizeof(config->lpi_interfaces));
> +		config->lpi_capabilities = MAC_100FD | MAC_1000FD;
> +		config->eee_enabled_default = true;
                ^^^^

[Severity: Medium]
Should the commit message mention that this line changes the default
behaviour of already-supported boards?

The commit message says:

    Add EEE support on the integrated-PHY ports, controllable per port via
    ethtool.

and:

    The hardware manages LPI on its own, so advertise EEE to phylink with
    no-op mac_{en,dis}able_tx_lpi callbacks.

That reads as opt-in, but setting config->eee_enabled_default in
rtl8365mb_phylink_get_caps() makes EEE active without any ethtool command.
phylink_create() copies the flag into its own configuration:

drivers/net/phy/phylink.c:phylink_create() {
	...
	pl->eee_cfg.eee_enabled = pl->config->eee_enabled_default;
	pl->eee_cfg.tx_lpi_enabled = pl->eee_cfg.eee_enabled;
	...
}

and the advertisement is then written at the first link bring-up:

drivers/net/phy/phylink.c:phylink_bringup_phy() {
	...
	if (pl->eee_cfg.eee_enabled)
		phy_support_eee(phy);
	...
}

So after a kernel upgrade every integrated-PHY port on every supported
rtl8365mb board starts advertising EEE and entering LPI, where previously it
did not. EEE enabled by default is known to cause link instability with some
link partners.

Could the commit message state this default change explicitly, so that
maintainers and backporters see it?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260916090102.349157-1-o.rempel%40pengutronix.de

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-09-18  3:03 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-16  9:00 [PATCH net-next v3 0/3] net: dsa: realtek: EEE support for RTL8365MB-VC Oleksij Rempel
2026-09-16  9:01 ` [PATCH net-next v3 1/3] net: phy: realtek: use C45 for RTL8365MB-VC internal PHY MMD access Oleksij Rempel
2026-09-18  3:03   ` netdev-bot+sashiko
2026-09-16  9:01 ` [PATCH net-next v3 2/3] net: dsa: realtek: rtl8365mb: extract PHY OCP address halves with FIELD_GET Oleksij Rempel
2026-09-16 12:36   ` Andrew Lunn
2026-09-16 12:49   ` Linus Walleij
2026-09-16  9:01 ` [PATCH net-next v3 3/3] net: dsa: realtek: rtl8365mb: add EEE support Oleksij Rempel
2026-09-18  3:03   ` netdev-bot+sashiko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®