Skip to contentSkip to conversation

Fix RGMII delays for 88E1118

PATCH Applied Russell King (Oracle) 5 participants 7 messages Jan 4, 2022 – Jan 5, 2022

Subsystems (3)
lore
Share & tools
lore

Russell King (Oracle)

author reviewer
#0 lore raw

cover Fix RGMII delays for 88E1118

Hi,

This series fixes the RGMII delays for 88E1118 Marvell PHYs, after a report by Corentin Labbe that the Marvell driver fails to work.

Patch 1 cleans up the paged register accesses in m88e1118_config_init() and patch 2 adds the RGMII delay configuration.

This comes with an element of risk as existing DT may need to be fixed for this in a similar way as we have done in the recent past for other PHY drivers that have misinterpreted the RGMII interface modes.

 drivers/net/phy/marvell.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)
Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
#1 lore raw

PATCH 1/2 net: phy: marvell: use phy_write_paged() to set MSCR

Use phy_write_paged() in m88e1118_config_init() to set the MSCR value. We leave the other paged write for the LEDs in case the DT register parsing is relying on this page.

---
 drivers/net/phy/marvell.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)
drivers/net/phy/marvell.c +8 -14
index 4fcfca4e1702..64e7874c95f4 100644
@@ -1225,28 +1225,22 @@ static int m88e1118_config_aneg(struct phy_device *phydev)
 
 static int m88e1118_config_init(struct phy_device *phydev)
 {
+	u16 leds;
 	int err;
 
-	/* Change address */
-	err = marvell_set_page(phydev, MII_MARVELL_MSCR_PAGE);
-	if (err < 0)
-		return err;
-
 	/* Enable 1000 Mbit */
-	err = phy_write(phydev, 0x15, 0x1070);
-	if (err < 0)
-		return err;
-
-	/* Change address */
-	err = marvell_set_page(phydev, MII_MARVELL_LED_PAGE);
+	err = phy_write_paged(phydev, MII_MARVELL_MSCR_PAGE,
+			      MII_88E1121_PHY_MSCR_REG, 0x1070);
 	if (err < 0)
 		return err;
 
 	/* Adjust LED Control */
 	if (phydev->dev_flags & MARVELL_PHY_M1118_DNS323_LEDS)
-		err = phy_write(phydev, 0x10, 0x1100);
+		leds = 0x1100;
 	else
-		err = phy_write(phydev, 0x10, 0x021e);
+		leds = 0x021e;
+
+	err = phy_write_paged(phydev, MII_MARVELL_LED_PAGE, 0x10, leds);
 	if (err < 0)
 		return err;
 
@@ -1254,7 +1248,7 @@ static int m88e1118_config_init(struct phy_device *phydev)
 	if (err < 0)
 		return err;
 
-	/* Reset address */
+	/* Reset page register */
 	err = marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
 	if (err < 0)
 		return err;
Signature · 1 line
2.30.2
#4 lore raw

PATCH 2/2 net: phy: marvell: configure RGMII delays for 88E1118

In reply to Russell King (Oracle)

Corentin Labbe reports that the SSI 1328 does not work when allowing the PHY to operate at gigabit speeds, but does work with the generic PHY driver.

This appears to be because m88e1118_config_init() writes a fixed value to the MSCR register, claiming that this is to enable 1G speeds. However, this always sets bits 4 and 5, enabling RGMII transmit and receive delays. The suspicion is that the original board this was added for required the delays to make 1G speeds work.

Add the necessary configuration for RGMII delays for the 88E1118 to bring this into line with the requirements for RGMII support, and thus make the SSI 1328 work.

Corentin Labbe has tested this on gemini-ssi1328 and gemini-ns2502.

---
 drivers/net/phy/marvell.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/net/phy/marvell.c +6 -0
index 64e7874c95f4..739859c0dfb1 100644
@@ -1234,6 +1234,12 @@ static int m88e1118_config_init(struct phy_device *phydev)
 	if (err < 0)
 		return err;
 
+	if (phy_interface_is_rgmii(phydev)) {
+		err = m88e1121_config_aneg_rgmii_delays(phydev);
+		if (err < 0)
+			return err;
+	}
+
 	/* Adjust LED Control */
 	if (phydev->dev_flags & MARVELL_PHY_M1118_DNS323_LEDS)
 		leds = 0x1100;
Signature · 1 line
2.30.2
maintainer Reviewed-by on v1 1/2
#2 lore raw

Re: [PATCH net-next 1/2] net: phy: marvell: use phy_write_paged() to set MSCR

In reply to Russell King (Oracle)
Quoted 5 lines · Russell King (Oracle)

Use phy_write_paged() in m88e1118_config_init() to set the MSCR value. We leave the other paged write for the LEDs in case the DT register parsing is relying on this page.

Andrew

maintainer Reviewed-by on v1 2/2
#5 lore raw

Re: [PATCH net-next 2/2] net: phy: marvell: configure RGMII delays for 88E1118

In reply to Russell King (Oracle)
Quoted 14 lines · Russell King (Oracle)

Corentin Labbe reports that the SSI 1328 does not work when allowing the PHY to operate at gigabit speeds, but does work with the generic PHY driver.

This appears to be because m88e1118_config_init() writes a fixed value to the MSCR register, claiming that this is to enable 1G speeds. However, this always sets bits 4 and 5, enabling RGMII transmit and receive delays. The suspicion is that the original board this was added for required the delays to make 1G speeds work.

Add the necessary configuration for RGMII delays for the 88E1118 to bring this into line with the requirements for RGMII support, and thus make the SSI 1328 work.

Quoted 5 lines

Corentin Labbe has tested this on gemini-ssi1328 and gemini-ns2502.

Andrew

Tested-by on v1 1/2
#3 lore raw

Re: [PATCH net-next 1/2] net: phy: marvell: use phy_write_paged() to set MSCR

In reply to Russell King (Oracle)
Quoted 9 lines · Russell King (Oracle)

Use phy_write_paged() in m88e1118_config_init() to set the MSCR value. We leave the other paged write for the LEDs in case the DT register parsing is relying on this page.

---
 drivers/net/phy/marvell.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

Thanks!

#6 lore raw
In reply to Russell King (Oracle)
This series was applied to netdev/net-next.git (master) by Jakub Kicinski <[email protected]>:

Hello:

This series was applied to netdev/net-next.git (master) by Jakub Kicinski <[email protected]>:

Quoted 9 lines · On Tue, 4 Jan 2022 16:37:21 +0000 you wrote:

Hi,

This series fixes the RGMII delays for 88E1118 Marvell PHYs, after a report by Corentin Labbe that the Marvell driver fails to work.

Patch 1 cleans up the paged register accesses in m88e1118_config_init() and patch 2 adds the RGMII delay configuration.

[...]

Here is the summary with links:
- [net-next,1/2] net: phy: marvell: use phy_write_paged() to set MSCR

    https://git.kernel.org/netdev/net-next/c/5b8f970309dd
  - [net-next,2/2] net: phy: marvell: configure RGMII delays for 88E1118
    https://git.kernel.org/netdev/net-next/c/f22725c95ece

You are awesome, thank you!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html