[PATCH net-next 0/2] net: phy: dp83867: Always program R/SGMII enable bits

public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed

* [PATCH net-next 0/2] net: phy: dp83867: Always program R/SGMII enable bits
@ 2026-01-29 17:12 Sean Anderson
  2026-01-29 17:12 ` [PATCH net-next 1/2] net: phy: dp83867: Program TX FIFO for all interfaces Sean Anderson
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Sean Anderson @ 2026-01-29 17:12 UTC (permalink / raw)
  To: Andrew Lunn, Heiner Kallweit, Russell King, netdev
  Cc: linux-kernel, Jakub Kicinski, Paolo Abeni, David S . Miller,
	Eric Dumazet, Sean Anderson

The hardware designers at my company neglected to read the datasheet for
this PHY and did not add appropriate resistors to configure it for
SGMII. Add support for configuring the it based on phy-mode instead of
relying on the resistors for a suitable default.


Sean Anderson (2):
  net: phy: dp83867: Program TX FIFO for all interfaces
  net: phy: dp83867: Always program R/SGMII enable bits

 drivers/net/phy/dp83867.c | 63 ++++++++++++++-------------------------
 1 file changed, 23 insertions(+), 40 deletions(-)

-- 
2.35.1.1320.gc452695387.dirty


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

* [PATCH net-next 1/2] net: phy: dp83867: Program TX FIFO for all interfaces
  2026-01-29 17:12 [PATCH net-next 0/2] net: phy: dp83867: Always program R/SGMII enable bits Sean Anderson
@ 2026-01-29 17:12 ` Sean Anderson
  2026-01-29 17:12 ` [PATCH net-next 2/2] net: phy: dp83867: Always program R/SGMII enable bits Sean Anderson
  2026-02-03  1:30 ` [PATCH net-next 0/2] " patchwork-bot+netdevbpf
  2 siblings, 0 replies; 6+ messages in thread
From: Sean Anderson @ 2026-01-29 17:12 UTC (permalink / raw)
  To: Andrew Lunn, Heiner Kallweit, Russell King, netdev
  Cc: linux-kernel, Jakub Kicinski, Paolo Abeni, David S . Miller,
	Eric Dumazet, Sean Anderson

All supported interfaces use the TX FIFO register at least some of the
time, so there's no point in checking the interface. Retain the check
for the RX FIFO level since it is only used by SGMII.

Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---

 drivers/net/phy/dp83867.c | 31 ++++++++++++++-----------------
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
index 5f5de01c41e1..7e16e9299457 100644
--- a/drivers/net/phy/dp83867.c
+++ b/drivers/net/phy/dp83867.c
@@ -744,27 +744,24 @@ static int dp83867_config_init(struct phy_device *phydev)
 	 */
 	phy_disable_eee(phydev);
 
-	if (phy_interface_is_rgmii(phydev) ||
-	    phydev->interface == PHY_INTERFACE_MODE_SGMII) {
-		val = phy_read(phydev, MII_DP83867_PHYCTRL);
-		if (val < 0)
-			return val;
+	val = phy_read(phydev, MII_DP83867_PHYCTRL);
+	if (val < 0)
+		return val;
 
-		val &= ~DP83867_PHYCR_TX_FIFO_DEPTH_MASK;
-		val |= (dp83867->tx_fifo_depth <<
-			DP83867_PHYCR_TX_FIFO_DEPTH_SHIFT);
+	val &= ~DP83867_PHYCR_TX_FIFO_DEPTH_MASK;
+	val |= (dp83867->tx_fifo_depth <<
+		DP83867_PHYCR_TX_FIFO_DEPTH_SHIFT);
 
-		if (phydev->interface == PHY_INTERFACE_MODE_SGMII) {
-			val &= ~DP83867_PHYCR_RX_FIFO_DEPTH_MASK;
-			val |= (dp83867->rx_fifo_depth <<
-				DP83867_PHYCR_RX_FIFO_DEPTH_SHIFT);
-		}
-
-		ret = phy_write(phydev, MII_DP83867_PHYCTRL, val);
-		if (ret)
-			return ret;
+	if (phydev->interface == PHY_INTERFACE_MODE_SGMII) {
+		val &= ~DP83867_PHYCR_RX_FIFO_DEPTH_MASK;
+		val |= (dp83867->rx_fifo_depth <<
+			DP83867_PHYCR_RX_FIFO_DEPTH_SHIFT);
 	}
 
+	ret = phy_write(phydev, MII_DP83867_PHYCTRL, val);
+	if (ret)
+		return ret;
+
 	if (phy_interface_is_rgmii(phydev)) {
 		val = phy_read(phydev, MII_DP83867_PHYCTRL);
 		if (val < 0)
-- 
2.35.1.1320.gc452695387.dirty


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

* [PATCH net-next 2/2] net: phy: dp83867: Always program R/SGMII enable bits
  2026-01-29 17:12 [PATCH net-next 0/2] net: phy: dp83867: Always program R/SGMII enable bits Sean Anderson
  2026-01-29 17:12 ` [PATCH net-next 1/2] net: phy: dp83867: Program TX FIFO for all interfaces Sean Anderson
@ 2026-01-29 17:12 ` Sean Anderson
  2026-01-29 17:22   ` Russell King (Oracle)
  2026-02-03  1:30 ` [PATCH net-next 0/2] " patchwork-bot+netdevbpf
  2 siblings, 1 reply; 6+ messages in thread
From: Sean Anderson @ 2026-01-29 17:12 UTC (permalink / raw)
  To: Andrew Lunn, Heiner Kallweit, Russell King, netdev
  Cc: linux-kernel, Jakub Kicinski, Paolo Abeni, David S . Miller,
	Eric Dumazet, Sean Anderson

If the board designers have neglected to populate the appropriate
resistors on the strapping pins then the phy may default to the wrong
interface mode. Enable/disable the RGMII/SGMII enable bits as necessary
to select the correct interface.

The dp83867 strapping pins have four levels and typically configure two
features at once. LED_0 controls both port mirroring and whether SGMII
is enabled. If it is pulled to VDDIO, both port mirroring and SGMII
will be enabled. For variants of the dp83867 that do not support SGMII,
this will prevent data from being transferred. As we now explicitly set
the SGMII and RGMII enable bits, we do not need to detect whether SGMII
has been inadvertently enabled.

Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---

 drivers/net/phy/dp83867.c | 34 ++++++++++------------------------
 1 file changed, 10 insertions(+), 24 deletions(-)

diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
index 7e16e9299457..3fb2293f568f 100644
--- a/drivers/net/phy/dp83867.c
+++ b/drivers/net/phy/dp83867.c
@@ -75,6 +75,7 @@
 #define MII_DP83867_MICR_JABBER_INT_EN		BIT(0)
 
 /* RGMIICTL bits */
+#define DP83867_RGMII_EN			BIT(7)
 #define DP83867_RGMII_TX_CLK_DELAY_EN		BIT(1)
 #define DP83867_RGMII_RX_CLK_DELAY_EN		BIT(0)
 
@@ -100,7 +101,7 @@
 #define DP83867_PHYCR_FIFO_DEPTH_MAX		0x03
 #define DP83867_PHYCR_TX_FIFO_DEPTH_MASK	GENMASK(15, 14)
 #define DP83867_PHYCR_RX_FIFO_DEPTH_MASK	GENMASK(13, 12)
-#define DP83867_PHYCR_RESERVED_MASK		BIT(11)
+#define DP83867_PHYCR_SGMII_EN			BIT(11)
 #define DP83867_PHYCR_FORCE_LINK_GOOD		BIT(10)
 
 /* RGMIIDCTL bits */
@@ -752,10 +753,12 @@ static int dp83867_config_init(struct phy_device *phydev)
 	val |= (dp83867->tx_fifo_depth <<
 		DP83867_PHYCR_TX_FIFO_DEPTH_SHIFT);
 
+	val &= ~DP83867_PHYCR_SGMII_EN;
 	if (phydev->interface == PHY_INTERFACE_MODE_SGMII) {
 		val &= ~DP83867_PHYCR_RX_FIFO_DEPTH_MASK;
 		val |= (dp83867->rx_fifo_depth <<
-			DP83867_PHYCR_RX_FIFO_DEPTH_SHIFT);
+			DP83867_PHYCR_RX_FIFO_DEPTH_SHIFT) |
+		       DP83867_PHYCR_SGMII_EN;
 	}
 
 	ret = phy_write(phydev, MII_DP83867_PHYCTRL, val);
@@ -763,31 +766,10 @@ static int dp83867_config_init(struct phy_device *phydev)
 		return ret;
 
 	if (phy_interface_is_rgmii(phydev)) {
-		val = phy_read(phydev, MII_DP83867_PHYCTRL);
-		if (val < 0)
-			return val;
-
-		/* The code below checks if "port mirroring" N/A MODE4 has been
-		 * enabled during power on bootstrap.
-		 *
-		 * Such N/A mode enabled by mistake can put PHY IC in some
-		 * internal testing mode and disable RGMII transmission.
-		 *
-		 * In this particular case one needs to check STRAP_STS1
-		 * register's bit 11 (marked as RESERVED).
-		 */
-
-		bs = phy_read_mmd(phydev, DP83867_DEVADDR, DP83867_STRAP_STS1);
-		if (bs & DP83867_STRAP_STS1_RESERVED)
-			val &= ~DP83867_PHYCR_RESERVED_MASK;
-
-		ret = phy_write(phydev, MII_DP83867_PHYCTRL, val);
-		if (ret)
-			return ret;
-
 		/* Set up RGMII delays */
 		val = phy_read_mmd(phydev, DP83867_DEVADDR, DP83867_RGMIICTL);
 
+		val |= DP83867_RGMII_EN;
 		val &= ~(DP83867_RGMII_TX_CLK_DELAY_EN | DP83867_RGMII_RX_CLK_DELAY_EN);
 		if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
 			val |= (DP83867_RGMII_TX_CLK_DELAY_EN | DP83867_RGMII_RX_CLK_DELAY_EN);
@@ -803,6 +785,10 @@ static int dp83867_config_init(struct phy_device *phydev)
 		phy_write_mmd(phydev, DP83867_DEVADDR, DP83867_RGMIIDCTL,
 			      dp83867->rx_id_delay |
 			      (dp83867->tx_id_delay << DP83867_RGMII_TX_CLK_DELAY_SHIFT));
+	} else {
+		val = phy_read_mmd(phydev, DP83867_DEVADDR, DP83867_RGMIICTL);
+		val &= ~DP83867_RGMII_EN;
+		phy_write_mmd(phydev, DP83867_DEVADDR, DP83867_RGMIICTL, val);
 	}
 
 	/* If specified, set io impedance */
-- 
2.35.1.1320.gc452695387.dirty


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

* Re: [PATCH net-next 2/2] net: phy: dp83867: Always program R/SGMII enable bits
  2026-01-29 17:12 ` [PATCH net-next 2/2] net: phy: dp83867: Always program R/SGMII enable bits Sean Anderson
@ 2026-01-29 17:22   ` Russell King (Oracle)
  2026-01-29 17:32     ` Sean Anderson
  0 siblings, 1 reply; 6+ messages in thread
From: Russell King (Oracle) @ 2026-01-29 17:22 UTC (permalink / raw)
  To: Sean Anderson
  Cc: Andrew Lunn, Heiner Kallweit, netdev, linux-kernel,
	Jakub Kicinski, Paolo Abeni, David S . Miller, Eric Dumazet

On Thu, Jan 29, 2026 at 12:12:05PM -0500, Sean Anderson wrote:
> If the board designers have neglected to populate the appropriate
> resistors on the strapping pins then the phy may default to the wrong
> interface mode. Enable/disable the RGMII/SGMII enable bits as necessary
> to select the correct interface.
> 
> The dp83867 strapping pins have four levels and typically configure two
> features at once. LED_0 controls both port mirroring and whether SGMII
> is enabled. If it is pulled to VDDIO, both port mirroring and SGMII
> will be enabled. For variants of the dp83867 that do not support SGMII,
> this will prevent data from being transferred. As we now explicitly set
> the SGMII and RGMII enable bits, we do not need to detect whether SGMII
> has been inadvertently enabled.
> 
> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>

Something to consider:

You have separate enable bits for SGMII and RGMII. The code you're
submitting sets the SGMII enable before clearing the RGMII enable.
Is it permitted to have both set?

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

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

* Re: [PATCH net-next 2/2] net: phy: dp83867: Always program R/SGMII enable bits
  2026-01-29 17:22   ` Russell King (Oracle)
@ 2026-01-29 17:32     ` Sean Anderson
  0 siblings, 0 replies; 6+ messages in thread
From: Sean Anderson @ 2026-01-29 17:32 UTC (permalink / raw)
  To: Russell King (Oracle)
  Cc: Andrew Lunn, Heiner Kallweit, netdev, linux-kernel,
	Jakub Kicinski, Paolo Abeni, David S . Miller, Eric Dumazet

Hi Russell,

On 1/29/26 12:22, Russell King (Oracle) wrote:
> On Thu, Jan 29, 2026 at 12:12:05PM -0500, Sean Anderson wrote:
>> If the board designers have neglected to populate the appropriate
>> resistors on the strapping pins then the phy may default to the wrong
>> interface mode. Enable/disable the RGMII/SGMII enable bits as necessary
>> to select the correct interface.
>> 
>> The dp83867 strapping pins have four levels and typically configure two
>> features at once. LED_0 controls both port mirroring and whether SGMII
>> is enabled. If it is pulled to VDDIO, both port mirroring and SGMII
>> will be enabled. For variants of the dp83867 that do not support SGMII,
>> this will prevent data from being transferred. As we now explicitly set
>> the SGMII and RGMII enable bits, we do not need to detect whether SGMII
>> has been inadvertently enabled.
>> 
>> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
> 
> Something to consider:
> 
> You have separate enable bits for SGMII and RGMII. The code you're
> submitting sets the SGMII enable before clearing the RGMII enable.
> Is it permitted to have both set?

Section 7.4.1 of SNLS504F says:

| The SGMII enable has higher priority than the RGMII enable. Table 7-1 is
| the configuration table for the MAC interfaces:
| 
| Table 7-1. Configuration Table for the MAC Interfaces
| SGMII ENABLE              RGMII ENABLE             DEVICE FUNCTIONAL MODE
| (REGISTER 0x0010, BIT 11) (REGISTER 0x0032, BIT 7)
| ========================= ======================== ======================
|                       0x1                      0x1 SGMII
|                       0x1                      0x0 SGMII
|                       0x0                      0x1 RGMII

So I don't think we will have any problems.

--Sean

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

* Re: [PATCH net-next 0/2] net: phy: dp83867: Always program R/SGMII enable bits
  2026-01-29 17:12 [PATCH net-next 0/2] net: phy: dp83867: Always program R/SGMII enable bits Sean Anderson
  2026-01-29 17:12 ` [PATCH net-next 1/2] net: phy: dp83867: Program TX FIFO for all interfaces Sean Anderson
  2026-01-29 17:12 ` [PATCH net-next 2/2] net: phy: dp83867: Always program R/SGMII enable bits Sean Anderson
@ 2026-02-03  1:30 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-02-03  1:30 UTC (permalink / raw)
  To: Sean Anderson
  Cc: andrew, hkallweit1, linux, netdev, linux-kernel, kuba, pabeni,
	davem, edumazet

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Thu, 29 Jan 2026 12:12:03 -0500 you wrote:
> The hardware designers at my company neglected to read the datasheet for
> this PHY and did not add appropriate resistors to configure it for
> SGMII. Add support for configuring the it based on phy-mode instead of
> relying on the resistors for a suitable default.
> 
> 
> Sean Anderson (2):
>   net: phy: dp83867: Program TX FIFO for all interfaces
>   net: phy: dp83867: Always program R/SGMII enable bits
> 
> [...]

Here is the summary with links:
  - [net-next,1/2] net: phy: dp83867: Program TX FIFO for all interfaces
    https://git.kernel.org/netdev/net-next/c/d700b2a44051
  - [net-next,2/2] net: phy: dp83867: Always program R/SGMII enable bits
    https://git.kernel.org/netdev/net-next/c/3d0721cfcf57

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

end of thread, other threads:[~2026-02-03  1:30 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-29 17:12 [PATCH net-next 0/2] net: phy: dp83867: Always program R/SGMII enable bits Sean Anderson
2026-01-29 17:12 ` [PATCH net-next 1/2] net: phy: dp83867: Program TX FIFO for all interfaces Sean Anderson
2026-01-29 17:12 ` [PATCH net-next 2/2] net: phy: dp83867: Always program R/SGMII enable bits Sean Anderson
2026-01-29 17:22   ` Russell King (Oracle)
2026-01-29 17:32     ` Sean Anderson
2026-02-03  1:30 ` [PATCH net-next 0/2] " patchwork-bot+netdevbpf

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