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

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

[Romain Gantois]

Hi everyone,

This series 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>
---
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 | 92 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 92 insertions(+)
---
base-commit: 32dbca2817e3611165636113f246bed30ef21b3d
change-id: 20251103-sfp-1000basex-a05c78484a54

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

[PATCH net-next 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 | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/phy/dp83869.c b/drivers/net/phy/dp83869.c
index 1f381d7b13ff..fecacaa83b04 100644
--- a/drivers/net/phy/dp83869.c
+++ b/drivers/net/phy/dp83869.c
@@ -797,6 +797,10 @@ static int dp83869_configure_mode(struct phy_device *phydev,
 		return -EINVAL;
 	}
 
+	ret = phy_write(phydev, DP83869_CTRL, DP83869_SW_RESTART);
+
+	usleep_range(10, 20);
+
 	return ret;
 }
 

-- 
2.51.2

[PATCH net-next 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 fecacaa83b04..adcd899472f2 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
 
@@ -809,6 +810,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 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 | 78 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 78 insertions(+)

diff --git a/drivers/net/phy/dp83869.c b/drivers/net/phy/dp83869.c
index adcd899472f2..e279dfa268a4 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>
 
@@ -875,6 +876,77 @@ 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)
+{
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(phy_support);
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(sfp_support);
+	struct phy_device *phydev = upstream;
+	const struct sfp_module_caps *caps;
+	struct dp83869_private *dp83869;
+	phy_interface_t interface;
+	int ret;
+
+	linkmode_zero(phy_support);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT, phy_support);
+
+	caps = sfp_get_module_caps(phydev->sfp_bus);
+
+	linkmode_and(sfp_support, phy_support, caps->link_modes);
+
+	if (linkmode_empty(sfp_support)) {
+		phydev_err(phydev, "incompatible SFP module inserted\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;
+
+	switch (interface) {
+	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!\n",
+			   phy_modes(interface));
+		return -EINVAL;
+	}
+
+	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;
@@ -891,6 +963,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 1/3] net: phy: dp83869: Restart PHY when configuring mode

[Russell King (Oracle)]

On Tue, Nov 04, 2025 at 09:50:34AM +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>
> ---
>  drivers/net/phy/dp83869.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/phy/dp83869.c b/drivers/net/phy/dp83869.c
> index 1f381d7b13ff..fecacaa83b04 100644
> --- a/drivers/net/phy/dp83869.c
> +++ b/drivers/net/phy/dp83869.c
> @@ -797,6 +797,10 @@ static int dp83869_configure_mode(struct phy_device *phydev,
>  		return -EINVAL;
>  	}
>  
> +	ret = phy_write(phydev, DP83869_CTRL, DP83869_SW_RESTART);

So if dp83869_configure_fiber() returns an error, that doesn't matter?
(This overwrites its error.)

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

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

[Russell King (Oracle)]

On Tue, Nov 04, 2025 at 09:50:36AM +0100, Romain Gantois wrote:
> +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;

Should this be done by core phylib code?

> +}
> +
> +static int dp83869_module_insert(void *upstream, const struct sfp_eeprom_id *id)
> +{
> +	__ETHTOOL_DECLARE_LINK_MODE_MASK(phy_support);
> +	__ETHTOOL_DECLARE_LINK_MODE_MASK(sfp_support);
> +	struct phy_device *phydev = upstream;
> +	const struct sfp_module_caps *caps;
> +	struct dp83869_private *dp83869;
> +	phy_interface_t interface;
> +	int ret;
> +
> +	linkmode_zero(phy_support);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT, phy_support);
> +
> +	caps = sfp_get_module_caps(phydev->sfp_bus);
> +
> +	linkmode_and(sfp_support, phy_support, caps->link_modes);
> +
> +	if (linkmode_empty(sfp_support)) {
> +		phydev_err(phydev, "incompatible SFP module inserted\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;
> +
> +	switch (interface) {
> +	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!\n",
> +			   phy_modes(interface));
> +		return -EINVAL;
> +	}

If you only support 1000BASE-X, please test that in the interface mode
mask (caps->interfaces) rather than going round this long winded
method.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

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

[Romain Gantois]

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

Hi Russell,

On Tuesday, 4 November 2025 09:57:59 CET Russell King (Oracle) wrote:
> On Tue, Nov 04, 2025 at 09:50:34AM +0100, Romain Gantois wrote:
> > The DP83869 PHY requires a software restart when the OP_MODE is changed.
...
> > @@ -797,6 +797,10 @@ static int dp83869_configure_mode(struct phy_device
> > *phydev,> 
> >  		return -EINVAL;
> >  	
> >  	}
> > 
> > +	ret = phy_write(phydev, DP83869_CTRL, DP83869_SW_RESTART);
> 
> So if dp83869_configure_fiber() returns an error, that doesn't matter?
> (This overwrites its error.)

That's a blunder on my part, thanks for pointing it out.

-- 
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 3/3] net: phy: dp83869: Support 1000Base-X SFP

[Romain Gantois]

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

On Tuesday, 4 November 2025 10:01:36 CET Russell King (Oracle) wrote:
> On Tue, Nov 04, 2025 at 09:50:36AM +0100, Romain Gantois wrote:
> > +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;
> 
> Should this be done by core phylib code?

I guess that enough PHY drivers do this by hand that a new phylib helper could 
be warranted. Maybe something like phy_clear_aneg_results(), which would set 
speed, duplex, pause and asym_pause to default values.

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 3/3] net: phy: dp83869: Support 1000Base-X SFP

[Maxime Chevallier]

Hi,

On 04/11/2025 10:10, Romain Gantois wrote:
> On Tuesday, 4 November 2025 10:01:36 CET Russell King (Oracle) wrote:
>> On Tue, Nov 04, 2025 at 09:50:36AM +0100, Romain Gantois wrote:
>>> +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;
>>
>> Should this be done by core phylib code?
> 
> I guess that enough PHY drivers do this by hand that a new phylib helper could 
> be warranted. Maybe something like phy_clear_aneg_results(), which would set 
> speed, duplex, pause and asym_pause to default values.

Note that when phy_port eventually gets merged, we'll have a 
common .module_remove() for PHY SFP :

https://lore.kernel.org/netdev/20251013143146.364919-9-maxime.chevallier@bootlin.com/

We could definitely do that here :)

Maxime

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

[Romain Gantois]

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

On Tuesday, 4 November 2025 10:15:45 CET Maxime Chevallier wrote:
> Hi,
> 
> On 04/11/2025 10:10, Romain Gantois wrote:
> > On Tuesday, 4 November 2025 10:01:36 CET Russell King (Oracle) wrote:
> >> On Tue, Nov 04, 2025 at 09:50:36AM +0100, Romain Gantois wrote:
> >>> +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;
> >> 
> >> Should this be done by core phylib code?
> > 
> > I guess that enough PHY drivers do this by hand that a new phylib helper
> > could be warranted. Maybe something like phy_clear_aneg_results(), which
> > would set speed, duplex, pause and asym_pause to default values.
> 
> Note that when phy_port eventually gets merged, we'll have a
> common .module_remove() for PHY SFP :
> 
> https://lore.kernel.org/netdev/20251013143146.364919-9-maxime.chevallier@boo
> tlin.com/
> 
> We could definitely do that here :)

That would be even better!

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 --]