mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net-next v1 1/1] net: phy: realtek: add MDI-X support for RTL8365MB-VC
@ 2026-09-29  9:20 Oleksij Rempel
  2026-09-29 10:21 ` Alvin Šipraga
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Oleksij Rempel @ 2026-09-29  9:20 UTC (permalink / raw)
  To: Linus Walleij, Luiz Angelo Daros de Luca, Andrew Lunn,
	Heiner Kallweit, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni
  Cc: Oleksij Rempel, kernel, linux-kernel, Russell King, netdev,
	Alvin Šipraga

Add MDI-X configuration and status reporting for the RTL8365MB-VC internal
PHYs. Without a driver hook genphy cannot reach the chip's crossover
control register, so forcing MDI/MDI-X with "ethtool -s <dev> mdix on|off"
is a no-op and "ethtool <dev>" cannot report the resolved MDI-X state on
this PHY.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/phy/realtek/realtek_main.c | 80 ++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)

diff --git a/drivers/net/phy/realtek/realtek_main.c b/drivers/net/phy/realtek/realtek_main.c
index 3b3352e2cdc6..e14960192e79 100644
--- a/drivers/net/phy/realtek/realtek_main.c
+++ b/drivers/net/phy/realtek/realtek_main.c
@@ -239,6 +239,16 @@
 #define RTL_PHYSR_MASTER			BIT(11)
 #define RTL_PHYSR_SPEED_MASK			(RTL_PHYSR_SPEEDL | RTL_PHYSR_SPEEDH)
 
+/* MDI-X control and status of the RTL8365MB-VC internal PHY. The resolved
+ * MDI-X state is reported in RTL_PHYSR; the configuration lives in a
+ * chip-specific control register. Both are only known to apply to this model.
+ * On this chip a set select/resolved bit means MDI, not MDI-X (measured).
+ */
+#define RTL8365MB_VC_PHYCR1			0x18
+#define RTL8365MB_VC_PHYCR1_MDIX_FORCE		BIT(9)
+#define RTL8365MB_VC_PHYCR1_MDI			BIT(8)
+#define RTL8365MB_VC_PHYSR_MDI			BIT(1)
+
 #define	RTL_MDIO_PCS_EEE_ABLE			0xa5c4
 #define	RTL_MDIO_AN_EEE_ADV			0xa5d0
 #define	RTL_MDIO_AN_EEE_LPABLE			0xa5d2
@@ -3058,6 +3068,74 @@ static irqreturn_t rtl8221b_handle_interrupt(struct phy_device *phydev)
 	return IRQ_HANDLED;
 }
 
+static int rtl8365mb_config_mdix(struct phy_device *phydev)
+{
+	u16 val;
+
+	switch (phydev->mdix_ctrl) {
+	case ETH_TP_MDI:
+		val = RTL8365MB_VC_PHYCR1_MDIX_FORCE |
+		      RTL8365MB_VC_PHYCR1_MDI;
+		break;
+	case ETH_TP_MDI_X:
+		val = RTL8365MB_VC_PHYCR1_MDIX_FORCE;
+		break;
+	case ETH_TP_MDI_AUTO:
+		val = 0;
+		break;
+	default:
+		/* Leave the hardware configuration alone until user space
+		 * asks for a specific mode.
+		 */
+		return 0;
+	}
+
+	return phy_modify_changed(phydev, RTL8365MB_VC_PHYCR1,
+				  RTL8365MB_VC_PHYCR1_MDIX_FORCE |
+				  RTL8365MB_VC_PHYCR1_MDI, val);
+}
+
+static int rtl8365mb_config_aneg(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = rtl8365mb_config_mdix(phydev);
+	if (ret < 0)
+		return ret;
+
+	/* The pair assignment is only evaluated while the link is brought up,
+	 * so renegotiate if the crossover configuration changed.
+	 */
+	return __genphy_config_aneg(phydev, ret);
+}
+
+static int rtl8365mb_read_status(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = phy_read(phydev, RTL8365MB_VC_PHYCR1);
+	if (ret < 0)
+		return ret;
+
+	if (ret & RTL8365MB_VC_PHYCR1_MDIX_FORCE) {
+		if (ret & RTL8365MB_VC_PHYCR1_MDI)
+			phydev->mdix_ctrl = ETH_TP_MDI;
+		else
+			phydev->mdix_ctrl = ETH_TP_MDI_X;
+	} else {
+		phydev->mdix_ctrl = ETH_TP_MDI_AUTO;
+	}
+
+	ret = phy_read(phydev, RTL_PHYSR);
+	if (ret < 0)
+		return ret;
+
+	phydev->mdix = (ret & RTL8365MB_VC_PHYSR_MDI) ? ETH_TP_MDI :
+							ETH_TP_MDI_X;
+
+	return genphy_read_status(phydev);
+}
+
 static struct phy_driver realtek_drvs[] = {
 	{
 		PHY_ID_MATCH_EXACT(0x00008201),
@@ -3354,6 +3432,8 @@ static struct phy_driver realtek_drvs[] = {
 		/* Interrupt handling analogous to RTL8366RB */
 		.config_intr	= genphy_no_config_intr,
 		.handle_interrupt = genphy_handle_interrupt_no_ack,
+		.config_aneg	= rtl8365mb_config_aneg,
+		.read_status	= rtl8365mb_read_status,
 		.suspend	= genphy_suspend,
 		.resume		= genphy_resume,
 	}, {
-- 
2.47.3


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

* Re: [PATCH net-next v1 1/1] net: phy: realtek: add MDI-X support for RTL8365MB-VC
  2026-09-29  9:20 [PATCH net-next v1 1/1] net: phy: realtek: add MDI-X support for RTL8365MB-VC Oleksij Rempel
@ 2026-09-29 10:21 ` Alvin Šipraga
  2026-09-30 10:59 ` Linus Walleij
  2026-10-01 12:21 ` netdev-bot+sashiko
  2 siblings, 0 replies; 4+ messages in thread
From: Alvin Šipraga @ 2026-09-29 10:21 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: Linus Walleij, Luiz Angelo Daros de Luca, Andrew Lunn,
	Heiner Kallweit, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, kernel, linux-kernel, Russell King, netdev

On Tue, Sep 29, 2026 at 11:20:07AM +0200, Oleksij Rempel wrote:
> Add MDI-X configuration and status reporting for the RTL8365MB-VC internal
> PHYs. Without a driver hook genphy cannot reach the chip's crossover
> control register, so forcing MDI/MDI-X with "ethtool -s <dev> mdix on|off"
> is a no-op and "ethtool <dev>" cannot report the resolved MDI-X state on
> this PHY.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>

Reviewed-by: Alvin Šipraga <alvin.sipraga@analog.com>

> ---
>  drivers/net/phy/realtek/realtek_main.c | 80 ++++++++++++++++++++++++++
>  1 file changed, 80 insertions(+)
> 
> diff --git a/drivers/net/phy/realtek/realtek_main.c b/drivers/net/phy/realtek/realtek_main.c
> index 3b3352e2cdc6..e14960192e79 100644
> --- a/drivers/net/phy/realtek/realtek_main.c
> +++ b/drivers/net/phy/realtek/realtek_main.c
> @@ -239,6 +239,16 @@
>  #define RTL_PHYSR_MASTER			BIT(11)
>  #define RTL_PHYSR_SPEED_MASK			(RTL_PHYSR_SPEEDL | RTL_PHYSR_SPEEDH)
>  
> +/* MDI-X control and status of the RTL8365MB-VC internal PHY. The resolved
> + * MDI-X state is reported in RTL_PHYSR; the configuration lives in a
> + * chip-specific control register. Both are only known to apply to this model.
> + * On this chip a set select/resolved bit means MDI, not MDI-X (measured).
> + */
> +#define RTL8365MB_VC_PHYCR1			0x18
> +#define RTL8365MB_VC_PHYCR1_MDIX_FORCE		BIT(9)
> +#define RTL8365MB_VC_PHYCR1_MDI			BIT(8)
> +#define RTL8365MB_VC_PHYSR_MDI			BIT(1)
> +
>  #define	RTL_MDIO_PCS_EEE_ABLE			0xa5c4
>  #define	RTL_MDIO_AN_EEE_ADV			0xa5d0
>  #define	RTL_MDIO_AN_EEE_LPABLE			0xa5d2
> @@ -3058,6 +3068,74 @@ static irqreturn_t rtl8221b_handle_interrupt(struct phy_device *phydev)
>  	return IRQ_HANDLED;
>  }
>  
> +static int rtl8365mb_config_mdix(struct phy_device *phydev)
> +{
> +	u16 val;
> +
> +	switch (phydev->mdix_ctrl) {
> +	case ETH_TP_MDI:
> +		val = RTL8365MB_VC_PHYCR1_MDIX_FORCE |
> +		      RTL8365MB_VC_PHYCR1_MDI;
> +		break;
> +	case ETH_TP_MDI_X:
> +		val = RTL8365MB_VC_PHYCR1_MDIX_FORCE;
> +		break;
> +	case ETH_TP_MDI_AUTO:
> +		val = 0;
> +		break;
> +	default:
> +		/* Leave the hardware configuration alone until user space
> +		 * asks for a specific mode.
> +		 */
> +		return 0;
> +	}
> +
> +	return phy_modify_changed(phydev, RTL8365MB_VC_PHYCR1,
> +				  RTL8365MB_VC_PHYCR1_MDIX_FORCE |
> +				  RTL8365MB_VC_PHYCR1_MDI, val);
> +}
> +
> +static int rtl8365mb_config_aneg(struct phy_device *phydev)
> +{
> +	int ret;
> +
> +	ret = rtl8365mb_config_mdix(phydev);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* The pair assignment is only evaluated while the link is brought up,
> +	 * so renegotiate if the crossover configuration changed.
> +	 */
> +	return __genphy_config_aneg(phydev, ret);
> +}
> +
> +static int rtl8365mb_read_status(struct phy_device *phydev)
> +{
> +	int ret;
> +
> +	ret = phy_read(phydev, RTL8365MB_VC_PHYCR1);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (ret & RTL8365MB_VC_PHYCR1_MDIX_FORCE) {
> +		if (ret & RTL8365MB_VC_PHYCR1_MDI)
> +			phydev->mdix_ctrl = ETH_TP_MDI;
> +		else
> +			phydev->mdix_ctrl = ETH_TP_MDI_X;
> +	} else {
> +		phydev->mdix_ctrl = ETH_TP_MDI_AUTO;
> +	}
> +
> +	ret = phy_read(phydev, RTL_PHYSR);
> +	if (ret < 0)
> +		return ret;
> +
> +	phydev->mdix = (ret & RTL8365MB_VC_PHYSR_MDI) ? ETH_TP_MDI :
> +							ETH_TP_MDI_X;
> +
> +	return genphy_read_status(phydev);
> +}
> +
>  static struct phy_driver realtek_drvs[] = {
>  	{
>  		PHY_ID_MATCH_EXACT(0x00008201),
> @@ -3354,6 +3432,8 @@ static struct phy_driver realtek_drvs[] = {
>  		/* Interrupt handling analogous to RTL8366RB */
>  		.config_intr	= genphy_no_config_intr,
>  		.handle_interrupt = genphy_handle_interrupt_no_ack,
> +		.config_aneg	= rtl8365mb_config_aneg,
> +		.read_status	= rtl8365mb_read_status,
>  		.suspend	= genphy_suspend,
>  		.resume		= genphy_resume,
>  	}, {
> -- 
> 2.47.3
> 

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

* Re: [PATCH net-next v1 1/1] net: phy: realtek: add MDI-X support for RTL8365MB-VC
  2026-09-29  9:20 [PATCH net-next v1 1/1] net: phy: realtek: add MDI-X support for RTL8365MB-VC Oleksij Rempel
  2026-09-29 10:21 ` Alvin Šipraga
@ 2026-09-30 10:59 ` Linus Walleij
  2026-10-01 12:21 ` netdev-bot+sashiko
  2 siblings, 0 replies; 4+ messages in thread
From: Linus Walleij @ 2026-09-30 10:59 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: Luiz Angelo Daros de Luca, Andrew Lunn, Heiner Kallweit,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	kernel, linux-kernel, Russell King, netdev, Alvin Šipraga

On Tue, Sep 29, 2026 at 11:20 AM Oleksij Rempel <o.rempel@pengutronix.de> wrote:

> Add MDI-X configuration and status reporting for the RTL8365MB-VC internal
> PHYs. Without a driver hook genphy cannot reach the chip's crossover
> control register, so forcing MDI/MDI-X with "ethtool -s <dev> mdix on|off"
> is a no-op and "ethtool <dev>" cannot report the resolved MDI-X state on
> this PHY.
>
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>

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

Yours,
Linus Walleij

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

* Re: [PATCH net-next v1 1/1] net: phy: realtek: add MDI-X support for RTL8365MB-VC
  2026-09-29  9:20 [PATCH net-next v1 1/1] net: phy: realtek: add MDI-X support for RTL8365MB-VC Oleksij Rempel
  2026-09-29 10:21 ` Alvin Šipraga
  2026-09-30 10:59 ` Linus Walleij
@ 2026-10-01 12:21 ` netdev-bot+sashiko
  2 siblings, 0 replies; 4+ messages in thread
From: netdev-bot+sashiko @ 2026-10-01 12:21 UTC (permalink / raw)
  To: o.rempel
  Cc: linusw, luizluca, andrew, hkallweit1, davem, edumazet, kuba,
	pabeni, kernel, linux-kernel, linux, netdev, alvin.sipraga

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

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

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] Changing MDI/MDI-X on a port that is forced to 10 or 100 Mb/s
  does not restart the link, so the new setting is not applied.

--

net: phy: realtek: add MDI-X support for RTL8365MB-VC

This commit adds config_aneg and read_status hooks for the RTL8365MB-VC
internal PHY. They let the MDI/MDI-X crossover mode be forced through a
chip-specific control register and report the resolved MDI-X state to
ethtool.

> diff --git a/drivers/net/phy/realtek/realtek_main.c b/drivers/net/phy/realtek/realtek_main.c
> index 3b3352e2cdc6a..e14960192e791 100644
> --- a/drivers/net/phy/realtek/realtek_main.c
> +++ b/drivers/net/phy/realtek/realtek_main.c

[ ... ]

> @@ -3058,6 +3068,74 @@ static irqreturn_t rtl8221b_handle_interrupt(struct phy_device *phydev)
>  	return IRQ_HANDLED;
>  }
>  
> +static int rtl8365mb_config_mdix(struct phy_device *phydev)
> +{

[ ... ]

> +	return phy_modify_changed(phydev, RTL8365MB_VC_PHYCR1,
> +				  RTL8365MB_VC_PHYCR1_MDIX_FORCE |
> +				  RTL8365MB_VC_PHYCR1_MDI, val);
> +}
> +
> +static int rtl8365mb_config_aneg(struct phy_device *phydev)
> +{
> +	int ret;
> +
> +	ret = rtl8365mb_config_mdix(phydev);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* The pair assignment is only evaluated while the link is brought up,
> +	 * so renegotiate if the crossover configuration changed.
> +	 */
> +	return __genphy_config_aneg(phydev, ret);

[Severity: Medium]
Does this renegotiate when the port is forced to 10 or 100 Mb/s?

When autoneg is disabled and the speed is below SPEED_1000,
__genphy_config_aneg() returns early and drops the changed argument:

drivers/net/phy/phy_device.c:__genphy_config_aneg() {
    ...
	} else if (phydev->speed < SPEED_1000) {
		return genphy_setup_forced(phydev);
	} else {
    ...
	return genphy_check_and_restart_aneg(phydev, changed);
}

genphy_setup_forced() only calls phy_modify() on MII_BMCR. If speed and
duplex are unchanged, __mdiobus_modify_changed() skips the write. Nothing
then restarts the link: there is no ANRESTART, no soft reset and no power
cycle.

For example, take a link that is up at a forced 100/full:

  ethtool -s <dev> autoneg off speed 100 duplex full
  ethtool -s <dev> mdix on

This goes through:

phy_ethtool_ksettings_set()->phy_start_aneg()->rtl8365mb_config_aneg()->
    __genphy_config_aneg()->genphy_setup_forced()

PHYCR1 bits 9:8 get updated. According to the comment above, though, the
pair assignment is not re-evaluated until the link drops for some other
reason. This part depends on the hardware behaviour that comment describes.

In the meantime, rtl8365mb_read_status() reads the new mode back from
PHYCR1 and reports it as mdix_ctrl. That no longer matches the pair
assignment actually in use.

The eth_tp_mdix_ctrl description in include/uapi/linux/ethtool.h says:
"When written successfully, the link should be renegotiated if necessary."

Would it help to handle the forced case the way marvell.c does? That
driver calls genphy_soft_reset() when "phydev->autoneg != AUTONEG_ENABLE ||
changed".

> +}
> +
> +static int rtl8365mb_read_status(struct phy_device *phydev)
> +{

[ ... ]

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

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

end of thread, other threads:[~2026-10-01 12:21 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-29  9:20 [PATCH net-next v1 1/1] net: phy: realtek: add MDI-X support for RTL8365MB-VC Oleksij Rempel
2026-09-29 10:21 ` Alvin Šipraga
2026-09-30 10:59 ` Linus Walleij
2026-10-01 12:21 ` 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®