[PATCH net-next v2 0/3] net: phy: dp83869: Support 1000Base-X SFP

[PATCH net-next v2 0/3] net: phy: dp83869: Support 1000Base-X SFP

[Romain Gantois]

Hi everyone,

This is version two of my series which adds support for using the DP83869
PHY as a transceiver between an RGMII upper MAC and a downstream 1000Base-X
SFP module.

Patch 1 and 2 of the series are necessary to get the PHY to properly switch its
operating mode to RGMII<->1000Base-X when an SFP module is inserted.

Patch 3 adds the actual SFP support, with only 1000Base-X modules supported for
now.

Side note: A wider-scoped series adding general SFP support to this PHY was sent
some time ago, but was not pursued, mainly due to complications with SGMII
support:

https://lore.kernel.org/netdev/20250514-dp83869-1000basex-v1-0-1bdb3c9c3d63@bootlin.com/

Best Regards,

Romain

Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
---
Changes in v2:
- Simplified module capability check.
- Fixed unchecked return value in configure_mode().
- Link to v1: https://lore.kernel.org/r/20251104-sfp-1000basex-v1-0-f461f170c74e@bootlin.com

---
Romain Gantois (3):
      net: phy: dp83869: Restart PHY when configuring mode
      net: phy: dp83869: ensure FORCE_LINK_GOOD is cleared
      net: phy: dp83869: Support 1000Base-X SFP

 drivers/net/phy/dp83869.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)
---
base-commit: dd43cb8c0f1de74d7fa47913acbc2bc54672c6e0
change-id: 20251103-sfp-1000basex-a05c78484a54

Best regards,
-- 
Romain Gantois <romain.gantois@bootlin.com>

[PATCH net-next v2 1/3] net: phy: dp83869: Restart PHY when configuring mode

[Romain Gantois]

The DP83869 PHY requires a software restart when the OP_MODE is changed.

Add this restart in dp83869_configure_mode().

Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
---
 drivers/net/phy/dp83869.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/phy/dp83869.c b/drivers/net/phy/dp83869.c
index 1f381d7b13ff..309bf608d630 100644
--- a/drivers/net/phy/dp83869.c
+++ b/drivers/net/phy/dp83869.c
@@ -792,11 +792,17 @@ static int dp83869_configure_mode(struct phy_device *phydev,
 	case DP83869_RGMII_1000_BASE:
 	case DP83869_RGMII_100_BASE:
 		ret = dp83869_configure_fiber(phydev, dp83869);
+		if (ret)
+			return ret;
 		break;
 	default:
 		return -EINVAL;
 	}
 
+	ret = phy_write(phydev, DP83869_CTRL, DP83869_SW_RESTART);
+
+	usleep_range(10, 20);
+
 	return ret;
 }
 

-- 
2.51.2

[PATCH net-next v2 2/3] net: phy: dp83869: ensure FORCE_LINK_GOOD is cleared

[Romain Gantois]

The FORCE_LINK_GOOD bit in the PHY_CONTROL register forces the reported
link status to 1 if the selected speed is 1Gbps.

According to the DP83869 PHY datasheet, this bit should default to 0 after
a hardware reset. However, the opposite has been observed on some DP83869
components.

As a consequence, a valid link will be reported in 1000Base-X operational
modes, even if the autonegotiation process failed.

Make sure that the FORCE_LINK_GOOD bit is cleared during initial
configuration.

Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
---
 drivers/net/phy/dp83869.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/net/phy/dp83869.c b/drivers/net/phy/dp83869.c
index 309bf608d630..143b75842fc7 100644
--- a/drivers/net/phy/dp83869.c
+++ b/drivers/net/phy/dp83869.c
@@ -89,6 +89,7 @@
 #define DP83869_STRAP_MIRROR_ENABLED           BIT(12)
 
 /* PHYCTRL bits */
+#define DP83869_FORCE_LINK_GOOD	BIT(10)
 #define DP83869_RX_FIFO_SHIFT	12
 #define DP83869_TX_FIFO_SHIFT	14
 
@@ -811,6 +812,15 @@ static int dp83869_config_init(struct phy_device *phydev)
 	struct dp83869_private *dp83869 = phydev->priv;
 	int ret, val;
 
+	/* The FORCE_LINK_GOOD bit in the PHYCTRL register should be
+	 * unset after a hardware reset but it is not. make sure it is
+	 * cleared so that the PHY can function properly.
+	 */
+	ret = phy_clear_bits(phydev, MII_DP83869_PHYCTRL,
+			     DP83869_FORCE_LINK_GOOD);
+	if (ret)
+		return ret;
+
 	/* Force speed optimization for the PHY even if it strapped */
 	ret = phy_modify(phydev, DP83869_CFG2, DP83869_DOWNSHIFT_EN,
 			 DP83869_DOWNSHIFT_EN);

-- 
2.51.2

[PATCH net-next v2 3/3] net: phy: dp83869: Support 1000Base-X SFP

[Romain Gantois]

Associate with an SFP cage described in the device tree and provide the
module_insert() callback that will set the appropriate DP83869 operation
mode when an SFP module is inserted.

Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
---
 drivers/net/phy/dp83869.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/drivers/net/phy/dp83869.c b/drivers/net/phy/dp83869.c
index 143b75842fc7..407518e6077f 100644
--- a/drivers/net/phy/dp83869.c
+++ b/drivers/net/phy/dp83869.c
@@ -10,6 +10,7 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/phy.h>
+#include <linux/sfp.h>
 #include <linux/delay.h>
 #include <linux/bitfield.h>
 
@@ -877,6 +878,57 @@ static int dp83869_config_init(struct phy_device *phydev)
 	return ret;
 }
 
+static void dp83869_module_remove(void *upstream)
+{
+	struct phy_device *phydev = upstream;
+
+	phydev_info(phydev, "SFP module removed\n");
+
+	/* Set speed and duplex to unknown to avoid downshifting warning. */
+	phydev->speed = SPEED_UNKNOWN;
+	phydev->duplex = DUPLEX_UNKNOWN;
+}
+
+static int dp83869_module_insert(void *upstream, const struct sfp_eeprom_id *id)
+{
+	struct phy_device *phydev = upstream;
+	const struct sfp_module_caps *caps;
+	struct dp83869_private *dp83869;
+	int ret;
+
+	caps = sfp_get_module_caps(phydev->sfp_bus);
+
+	if (!linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT,
+			       caps->link_modes)) {
+		phydev_err(phydev, "incompatible SFP module inserted\n");
+		return -EINVAL;
+	}
+
+	dp83869 = phydev->priv;
+
+	dp83869->mode = DP83869_RGMII_1000_BASE;
+	phydev->port = PORT_FIBRE;
+
+	ret = dp83869_configure_mode(phydev, dp83869);
+	if (ret)
+		return ret;
+
+	/* Reconfigure advertisement */
+	if (mutex_trylock(&phydev->lock)) {
+		ret = dp83869_config_aneg(phydev);
+		mutex_unlock(&phydev->lock);
+	}
+
+	return ret;
+}
+
+static const struct sfp_upstream_ops dp83869_sfp_ops = {
+	.attach = phy_sfp_attach,
+	.detach = phy_sfp_detach,
+	.module_insert = dp83869_module_insert,
+	.module_remove = dp83869_module_remove,
+};
+
 static int dp83869_probe(struct phy_device *phydev)
 {
 	struct dp83869_private *dp83869;
@@ -893,6 +945,12 @@ static int dp83869_probe(struct phy_device *phydev)
 	if (ret)
 		return ret;
 
+	if (of_property_read_bool(phydev->mdio.dev.of_node, "sfp")) {
+		ret = phy_sfp_probe(phydev, &dp83869_sfp_ops);
+		if (ret)
+			return ret;
+	}
+
 	if (dp83869->mode == DP83869_RGMII_100_BASE ||
 	    dp83869->mode == DP83869_RGMII_1000_BASE)
 		phydev->port = PORT_FIBRE;

-- 
2.51.2

Re: [PATCH net-next v2 1/3] net: phy: dp83869: Restart PHY when configuring mode

[Andrew Lunn]

On Mon, Nov 10, 2025 at 10:24:53AM +0100, Romain Gantois wrote:
> The DP83869 PHY requires a software restart when the OP_MODE is changed.
> 
> Add this restart in dp83869_configure_mode().
> 
> Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>

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

    Andrew

Re: [PATCH net-next v2 2/3] net: phy: dp83869: ensure FORCE_LINK_GOOD is cleared

[Andrew Lunn]

On Mon, Nov 10, 2025 at 10:24:54AM +0100, Romain Gantois wrote:
> The FORCE_LINK_GOOD bit in the PHY_CONTROL register forces the reported
> link status to 1 if the selected speed is 1Gbps.
> 
> According to the DP83869 PHY datasheet, this bit should default to 0 after
> a hardware reset. However, the opposite has been observed on some DP83869
> components.
> 
> As a consequence, a valid link will be reported in 1000Base-X operational
> modes, even if the autonegotiation process failed.
> 
> Make sure that the FORCE_LINK_GOOD bit is cleared during initial
> configuration.
> 
> Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>

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

    Andrew

Re: [PATCH net-next v2 3/3] net: phy: dp83869: Support 1000Base-X SFP

[Andrew Lunn]

> +static void dp83869_module_remove(void *upstream)
> +{
> +	struct phy_device *phydev = upstream;
> +
> +	phydev_info(phydev, "SFP module removed\n");

I think this should probably be downgraded to phydev_dbg().

> +static int dp83869_module_insert(void *upstream, const struct sfp_eeprom_id *id)
> +{
> +	struct phy_device *phydev = upstream;
> +	const struct sfp_module_caps *caps;
> +	struct dp83869_private *dp83869;
> +	int ret;
> +
> +	caps = sfp_get_module_caps(phydev->sfp_bus);
> +
> +	if (!linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT,
> +			       caps->link_modes)) {
> +		phydev_err(phydev, "incompatible SFP module inserted\n");
> +		return -EINVAL;
> +	}
> +
> +	dp83869 = phydev->priv;
> +
> +	dp83869->mode = DP83869_RGMII_1000_BASE;
> +	phydev->port = PORT_FIBRE;
> +
> +	ret = dp83869_configure_mode(phydev, dp83869);
> +	if (ret)
> +		return ret;
> +
> +	/* Reconfigure advertisement */
> +	if (mutex_trylock(&phydev->lock)) {
> +		ret = dp83869_config_aneg(phydev);
> +		mutex_unlock(&phydev->lock);

Why is it safe to call dp83869_configure_mode() without the lock, but
dp83869_config_aneg() does need the lock? And what are the
consequences of not being able to get the lock and so aneg is not
configured?

Some comments would be good here.

    Andrew

---
pw-bot: cr

Re: [PATCH net-next v2 3/3] net: phy: dp83869: Support 1000Base-X SFP

[Romain Gantois]

[-- Attachment #1: Type: text/plain, Size: 1089 bytes --]

Hello Andrew,

On Tuesday, 11 November 2025 02:08:05 CET Andrew Lunn wrote:
...
> > +	ret = dp83869_configure_mode(phydev, dp83869);
> > +	if (ret)
> > +		return ret;
> > +
> > +	/* Reconfigure advertisement */
> > +	if (mutex_trylock(&phydev->lock)) {
> > +		ret = dp83869_config_aneg(phydev);
> > +		mutex_unlock(&phydev->lock);
> 
> Why is it safe to call dp83869_configure_mode() without the lock, but
> dp83869_config_aneg() does need the lock? And what are the
> consequences of not being able to get the lock and so aneg is not
> configured?

Yeah the idea here is that if the LP changes, then the autoneg should be 
restarted afterwards. But doing it in the module_insert() callback of the PHY 
driver is quite complex as far as locking goes...

Maxime's phy_port series will make quite a bit of SFP code generic and will 
conflict with my series, so maybe it's better that I wait for Maxime's series 
to be merged before sending v3. Then I can try tackling this in a generic 
manner.

Thanks,

-- 
Romain Gantois, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH net-next v2 0/3] net: phy: dp83869: Support 1000Base-X SFP

[Álvaro G. M.]

On Mon, 10 Nov 2025 10:24:52 +0100
Romain Gantois <romain.gantois@bootlin.com> wrote:
> Hi everyone,
> 
> This is version two of my series which adds support for using the
> DP83869 PHY as a transceiver between an RGMII upper MAC and a
> downstream 1000Base-X SFP module.

Hi Romain,

I am quite interested in this too, as we have a new version of a PCB in
which we replaced a dp83848 (which only works with 100base-tx) with
dp83869 with the intent of using 1000base-sx or similar SFPs.

The are two different SFPs that I was able to use with dp83848 and
would like to use them with the dp83869 as well:

- RJ45 based:
    - Robofiber SFP-5000-RJ45A: 10/100Base-Tx SFP RJ45
    - H!Fiber ASF-GE-T1 1000Base-T SFP RJ-25

Both of these already work with dp83869 with kernel 6.12, provided I
set mode to DP83869_RGMII_100_BASE. If I try to make the 1000Base-T
work at 1Gbps by setting op mode to DP83869_RGMII_1000_BASE it will
still set speed to 100Mbps and will work just fine, but if I try to set
speed to 1000, all data transfer stops.

Personally, I could live with this, but I assume it should be possible
for this SFP to work at 1Gbps.

- Fiber based:
    - CISCO-FINISAR G155B3520KCD which I believe reports bit
      ETHTOOL_LINK_MODE_10000baseSR_Full_BIT
    - AVAGO AFBR-709SMZ which reports as 10gbase-r
    - ATGBICS AFBR-5710ALZ-C 1000BASE-SX

The first two, even if they report as 10Gbase, did work with dp83848 at
100Mbps out of the box, but do not seem to work in any case with
dp83869. The last one, 1000base-sx, is the one that I expected to
work either out of the box or by using this set of patches of yours.
However, none of them do work.

What I did next, as the 10gbase was simply refused, was to apply the
following patch above your series, which is more or less based on one
of your previous patches from 2025. The key here is that I try hard to
fallback to 100Mbps on every instance of non working SFP. So, I need
that if linkmode_empty(sfp_support) because the 10/100 SFPs simply do
not have any bit active to signal that capability, and I also set mode
to RGMII_100_BASE in every case. So, with this patch:

sfp amba_pl:sfp: module AVAGO            AFBR-709SMZ      rev G4.1 sn
AA17053045F      dc 170204  
TI DP83869 axienet-40c00000:01: dp83869_module_insert SFP compatible link mode:
10gbase-r
TI DP83869 axienet-40c00000:01: incompatible PHY-to-SFP module link mode
10gbase-r! Try simply 10/100
Configuring network: xilinx_axienet 40c00000.ethernet eth0: PHY [axienet-
40c00000:01] driver [TI DP83869] (irq=POLL)
xilinx_axienet 40c00000.ethernet eth0: configuring for phy/1000base-x link mode
xilinx_axienet 40c00000.ethernet eth0: Link is Up - 100Mbps/Full - flow control
off

And finally I have my other device answer to ping! So it seems that the
DP83869, as is, won't be able to run at 1Gbps no matter what.

From the datasheet... wouldn't it be necessary to update OP_MODE_DECODE
Register (Offset = 1DFh) [Reset = 0040h] CFG_OPMODE bits 2-0 to set value 1
RGMII to 1000Base-X?

diff --git c/dp83869.c w/dp83869.c
index 31a2d4f..3857577 100644
--- a/drivers/net/phy/dp83869.c
+++ b/drivers/net/phy/dp83869.c
@@ -10,6 +10,7 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/phy.h>
+#include <linux/phylink.h>
 #include <linux/sfp.h>
 #include <linux/delay.h>
 #include <linux/bitfield.h>
@@ -894,22 +895,63 @@ static void dp83869_module_remove(void *upstream)
 static int dp83869_module_insert(void *upstream, const struct sfp_eeprom_id
*id)
 {
 	struct phy_device *phydev = upstream;
-	const struct sfp_module_caps *caps;
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(phy_support);
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(sfp_support);
+	DECLARE_PHY_INTERFACE_MASK(interfaces);
 	struct dp83869_private *dp83869;
+	phy_interface_t interface;
 	int ret;
 
-	caps = sfp_get_module_caps(phydev->sfp_bus);
+	linkmode_zero(phy_support);
+	phylink_set(phy_support, 10000baseSR_Full);
+	phylink_set(phy_support, 1000baseT_Full);
+	phylink_set(phy_support, 1000baseX_Full);
+	phylink_set(phy_support, 100baseFX_Full);
+	phylink_set(phy_support, 100baseFX_Half);
 
-	if (!linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT,
-			       caps->link_modes)) {
-		phydev_err(phydev, "incompatible SFP module inserted\n");
-		return -EINVAL;
+	linkmode_zero(sfp_support);
+	sfp_parse_support(phydev->sfp_bus, id, sfp_support, interfaces);
+
+	sfp_parse_support(phydev->sfp_bus, id, sfp_support, interfaces);
+	linkmode_and(sfp_support, phy_support, sfp_support);
+
+	if (linkmode_empty(sfp_support)) {
+		phydev_err(phydev, "incompatible SFP module inserted, let's
assume 10/100\n");
+
+		linkmode_zero(sfp_support);
+		sfp_parse_support(phydev->sfp_bus, id, sfp_support,
interfaces);
+		phylink_set(sfp_support, 100baseFX_Full);
+		linkmode_and(sfp_support, phy_support, sfp_support);
+
+		if (linkmode_empty(sfp_support)) {
+			phydev_err(phydev, "incompatible SFP module inserted,
10/100 didn't work either\n");
+			return -EINVAL;
+		}
 	}
 
+	interface = sfp_select_interface(phydev->sfp_bus, sfp_support);
+	phydev_info(phydev, "%s SFP compatible link mode: %s\n", __func__,
+		phy_modes(interface));
+
 	dp83869 = phydev->priv;
 
-	dp83869->mode = DP83869_RGMII_1000_BASE;
-	phydev->port = PORT_FIBRE;
+	switch (interface) {
+	case PHY_INTERFACE_MODE_SGMII:
+	case PHY_INTERFACE_MODE_100BASEX:
+		dp83869->mode = DP83869_RGMII_100_BASE;
+		phydev->port = PORT_FIBRE;
+		break;
+	case PHY_INTERFACE_MODE_1000BASEX:
+		dp83869->mode = DP83869_RGMII_1000_BASE;
+		phydev->port = PORT_FIBRE;
+		break;
+	default:
+		phydev_err(phydev, "incompatible PHY-to-SFP module link mode
%s! Try simply 10/100\n",
+			   phy_modes(interface));
+		dp83869->mode = DP83869_RGMII_100_BASE;
+		phydev->port = PORT_FIBRE;
+		break;
+	}
 
 	ret = dp83869_configure_mode(phydev, dp83869);
 	if (ret)


It is absolutely possible that I misconfigured something, so here are
the relevant parts of my DTS. Sadly, as you can see, we don't have GPIO
access to the SFP pins, but at least they are properly pulled to the
value that enables the SFP and the transmitter at all times.

sfp0: sfp {
	compatible = "sff,sfp";
	i2c-bus = <&axi_iic_0>;
};

axi_ethernet_eth: ethernet@40c00000 {
	compatible = "xlnx,axi-ethernet-1.00.a";
	reg = <0x40c00000 0x40000>;
	phy-handle = <&phy0>;
	interrupt-parent = <&microblaze_0_axi_intc>;
	interrupts = <3>;
	xlnx,rxmem = <0x1000>;
	phy-mode = "1000base-x";
	max-speed = <1000>;
	xlnx,txcsum = <0x2>;
	xlnx,rxcsum = <0x2>;
	clock-names = "s_axi_lite_clk", "axis_clk", "ref_clk", "mgt_clk";
	clocks = <&clk_bus_0>, <&clk_bus_0>, <&clk_bus_0>, <&clk_bus_0>;
	axistream-connected = <&axi_ethernet_0_dma>;
	dma-names = "tx_chan0", "rx_chan0";
	mdio {
		#address-cells = <1>;
		#size-cells = <0>;

		phy0: ethernet-phy@0 {
			sfp = <&sfp0>;
			reg = <1>;
		};
	};
};

Thanks

Best regards,

-- 
Alvaro G. M.