Skip to contentSkip to conversation

Do not force master mode on unaffected PHY's

PATCH New Olliver Schinagl 6 participants 18 messages Nov 8, 2016 – Dec 8, 2016

Subsystems (2)
Network drivers Ethernet PHY
lore
Share & tools
lore

Olliver Schinagl

author
#0 lore raw

PATCH 1/1 Do not force master mode on unaffected PHY's

The current implementation to force the PHY into master mode is to have a define which affects all realtek PHY's. This is not needed as the problem only exists in the RTL8211C chips. Let us thus turn this into a quirk flag instead.

author
#1 lore raw

PATCH 1/3 net: phy: realtek: Use the BIT() macro

The BIT macro is the preferred method to set bits. This patch adds the bit macro and converts bit invocations.

Signed-off-by Olliver Schinagl <[email protected]>
---
 drivers/net/phy/realtek.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/net/phy/realtek.c +3 -2
index 7a99cb0..35b934b 100644
@@ -9,13 +9,14 @@
  */
 #include <config.h>
 #include <common.h>
+#include <linux/bitops.h>
 #include <phy.h>
 
 #define PHY_AUTONEGOTIATE_TIMEOUT 5000
 
 /* RTL8211x 1000BASE-T Control Register */
-#define MIIM_RTL8211x_CTRL1000T_MSCE (1 << 12);
-#define MIIM_RTL8211X_CTRL1000T_MASTER (1 << 11);
+#define MIIM_RTL8211x_CTRL1000T_MSCE BIT(12);
+#define MIIM_RTL8211X_CTRL1000T_MASTER BIT(11);
 
 /* RTL8211x PHY Status Register */
 #define MIIM_RTL8211x_PHY_STATUS       0x11
Signature · 1 line
2.10.2
#2 lore raw

PATCH 1/3 net: phy: realtek: Use the BIT() macro

Quoted 22 lines · Olliver Schinagl

The BIT macro is the preferred method to set bits. This patch adds the bit macro and converts bit invocations.

Signed-off-by Olliver Schinagl <[email protected]>

---
?drivers/net/phy/realtek.c | 5 +++--
?1 file changed, 3 insertions(+), 2 deletions(-)

drivers/net/phy/realtek.c +0 -0
index 7a99cb0..35b934b 100644
@@ -9,13 +9,14 @@

? */
?#include <config.h>
?#include <common.h>
+#include <linux/bitops.h>
?#include <phy.h>
?
?#define PHY_AUTONEGOTIATE_TIMEOUT 5000
?
?/* RTL8211x 1000BASE-T Control Register */

drivers/net/phy/realtek.c line 19

Quoted 4 lines
drivers/net/phy/realtek.c +2 -2
-#define MIIM_RTL8211x_CTRL1000T_MSCE (1 << 12);
-#define MIIM_RTL8211X_CTRL1000T_MASTER (1 << 11);
+#define MIIM_RTL8211x_CTRL1000T_MSCE BIT(12);
+#define MIIM_RTL8211X_CTRL1000T_MASTER BIT(11);

You should also get rid of semicolons.

Quoted 6 lines

?
?/* RTL8211x PHY Status Register */
?#define MIIM_RTL8211x_PHY_STATUS???????0x11
--?
2.10.2

author
#3 lore raw

PATCH 1/3 net: phy: realtek: Use the BIT() macro

Hi,

Quoted 20 lines · Priit Laes
Quoted 19 lines · Olliver Schinagl

The BIT macro is the preferred method to set bits. This patch adds the bit macro and converts bit invocations.

Signed-off-by Olliver Schinagl <[email protected]>
---
  drivers/net/phy/realtek.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)
drivers/net/phy/realtek.c +1 -0
index 7a99cb0..35b934b 100644
@@ -9,13 +9,14 @@
   */
  #include <config.h>
  #include <common.h>
+#include <linux/bitops.h>
  #include <phy.h>
Quoted 7 lines

drivers/net/phy/realtek.c line 19

Quoted 7 lines
drivers/net/phy/realtek.c +2 -2
  #define PHY_AUTONEGOTIATE_TIMEOUT 5000
 
  /* RTL8211x 1000BASE-T Control Register */
-#define MIIM_RTL8211x_CTRL1000T_MSCE (1 << 12);
-#define MIIM_RTL8211X_CTRL1000T_MASTER (1 << 11);
+#define MIIM_RTL8211x_CTRL1000T_MSCE BIT(12);
+#define MIIM_RTL8211X_CTRL1000T_MASTER BIT(11);

ah yeah, didn't even notice it, it's actually wrong. Also here, if that's all I don't mind if it gets fixed in the merge, if it needs a v2 I'll gladly do it however.

Olliver

Quoted 9 lines

You should also get rid of semicolons.

Quoted 6 lines
drivers/net/phy/realtek.c +0 -0
  /* RTL8211x PHY Status Register */
  #define MIIM_RTL8211x_PHY_STATUS       0x11
Signature · 1 line
2.10.2
author
#6 lore raw

PATCH 2/3 net: phy: realtek: make define more concistent

In reply to Olliver Schinagl

All internal defines in the realtek phy are with a small X, except MIIM_RTL8211X_CTRL1000T_MASTER. Make this more concistent

Signed-off-by Olliver Schinagl <[email protected]>
---
 drivers/net/phy/realtek.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/net/phy/realtek.c +2 -2
index 35b934b..62b8c1e 100644
@@ -16,7 +16,7 @@
 
 /* RTL8211x 1000BASE-T Control Register */
 #define MIIM_RTL8211x_CTRL1000T_MSCE BIT(12);
-#define MIIM_RTL8211X_CTRL1000T_MASTER BIT(11);
+#define MIIM_RTL8211x_CTRL1000T_MASTER BIT(11);
 
 /* RTL8211x PHY Status Register */
 #define MIIM_RTL8211x_PHY_STATUS       0x11

@@ -64,7 +64,7 @@ static int rtl8211x_config(struct phy_device *phydev)
 	/* force manual master/slave configuration */
 	reg |= MIIM_RTL8211x_CTRL1000T_MSCE;
 	/* force master mode */
-	reg |= MIIM_RTL8211X_CTRL1000T_MASTER;
+	reg |= MIIM_RTL8211x_CTRL1000T_MASTER;
 	phy_write(phydev, MDIO_DEVAD_NONE, MII_CTRL1000, reg);
 #endif
 	/* read interrupt status just to clear it */
Signature · 1 line
2.10.2
#7 lore raw

PATCH 2/3 net: phy: realtek: make define more concistent

Small nitpick:

El 08/11/16 a las 13:38, Olliver Schinagl escribi?:

Quoted 2 lines

All internal defines in the realtek phy are with a small X, except MIIM_RTL8211X_CTRL1000T_MASTER. Make this more concistent

s/concistent/consistent/ both here and on the subject :)

Cheers!
Emilio

author
#8 lore raw

PATCH 2/3 net: phy: realtek: make define more concistent

Crap! I thought I spotted it but wasn't sure :)

If there needs to be a v2 i'll fix it; if the v1 is accepted, Joe could hopefully fix that in the message *wink* :)

Olliver

Quoted 9 lines · Emilio L?pez

Small nitpick:

El 08/11/16 a las 13:38, Olliver Schinagl escribi?:

Quoted 2 lines

All internal defines in the realtek phy are with a small X, except MIIM_RTL8211X_CTRL1000T_MASTER. Make this more concistent

s/concistent/consistent/ both here and on the subject :)

Cheers!
Emilio

author
#11 lore raw

PATCH 3/3 net: phy: realtek: Only force master mode on rtl8211b/c

In reply to Olliver Schinagl

Commit 525d187af ("net: phy: Optionally force master mode for RTL PHY") added the define to force the PHY into master mode. Unfortunatly this is an all or nothing switch. So it applies to either all PHY's or no PHY's.

The bug that define tried to solve was a buggy PLL in the RTL8211C only.

The Olimex OLinuXino Lime2 has gotten an upgrade where the PHY was replaced with an RTL8211E. With this define however, both lime2 boards are either forced to master mode or not. We could of course have a binary for each board, but the following patch fixes this by adding a 'quirk' to the flags to the rtl8211b and rtl8211c only. It is now possible to force master mode, but only have it apply to the rtl8211b and rtl8211c.

Signed-off-by Olliver Schinagl <[email protected]>
---
 drivers/net/phy/realtek.c | 31 +++++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 8 deletions(-)
drivers/net/phy/realtek.c +23 -8
index 62b8c1e..635acf5 100644
@@ -12,6 +12,8 @@
 #include <linux/bitops.h>
 #include <phy.h>
 
+#define PHY_RTL8211x_FORCE_MASTER BIT(1)
+
 #define PHY_AUTONEGOTIATE_TIMEOUT 5000
 
 /* RTL8211x 1000BASE-T Control Register */

@@ -49,6 +51,15 @@
 #define MIIM_RTL8211F_TX_DELAY		0x100
 #define MIIM_RTL8211F_LCR		0x10
 
+static int rtl8211b_probe(struct phy_device *phydev)
+{
+#ifdef CONFIG_RTL8211X_PHY_FORCE_MASTER
+	phydev->flags |= PHY_RTL8211x_FORCE_MASTER;
+#endif
+
+	return 0;
+}
+
 /* RealTek RTL8211x */
 static int rtl8211x_config(struct phy_device *phydev)
 {

@@ -59,14 +70,17 @@ static int rtl8211x_config(struct phy_device *phydev)
 	 */
 	phy_write(phydev, MDIO_DEVAD_NONE, MIIM_RTL8211x_PHY_INER,
 		  MIIM_RTL8211x_PHY_INTR_DIS);
-#ifdef CONFIG_RTL8211X_PHY_FORCE_MASTER
-	unsigned int reg = phy_read(phydev, MDIO_DEVAD_NONE, MII_CTRL1000);
-	/* force manual master/slave configuration */
-	reg |= MIIM_RTL8211x_CTRL1000T_MSCE;
-	/* force master mode */
-	reg |= MIIM_RTL8211x_CTRL1000T_MASTER;
-	phy_write(phydev, MDIO_DEVAD_NONE, MII_CTRL1000, reg);
-#endif
+
+	if (phydev->flags & PHY_RTL8211x_FORCE_MASTER) {
+		unsigned int reg;
+
+		reg = phy_read(phydev, MDIO_DEVAD_NONE, MII_CTRL1000);
+		/* force manual master/slave configuration */
+		reg |= MIIM_RTL8211x_CTRL1000T_MSCE;
+		/* force master mode */
+		reg |= MIIM_RTL8211x_CTRL1000T_MASTER;
+		phy_write(phydev, MDIO_DEVAD_NONE, MII_CTRL1000, reg);
+	}
 	/* read interrupt status just to clear it */
 	phy_read(phydev, MDIO_DEVAD_NONE, MIIM_RTL8211x_PHY_INER);
 

@@ -249,6 +263,7 @@ static struct phy_driver RTL8211B_driver = {
 	.uid = 0x1cc912,
 	.mask = 0xffffff,
 	.features = PHY_GBIT_FEATURES,
+	.probe = &rtl8211b_probe,
 	.config = &rtl8211x_config,
 	.startup = &rtl8211x_startup,
 	.shutdown = &genphy_shutdown,
Signature · 1 line
2.10.2
#12 lore raw

PATCH 3/3 net: phy: realtek: Only force master mode on rtl8211b/c

Quoted 14 lines · Olliver Schinagl

Commit 525d187af ("net: phy: Optionally force master mode for RTL PHY") added the define to force the PHY into master mode. Unfortunatly this is an all or nothing switch. So it applies to either all PHY's or no PHY's.

The bug that define tried to solve was a buggy PLL in the RTL8211C only.

The Olimex OLinuXino Lime2 has gotten an upgrade where the PHY was replaced with an RTL8211E. With this define however, both lime2 boards are either forced to master mode or not. We could of course have a binary for each board, but the following patch fixes this by adding a 'quirk' to the flags to the rtl8211b and rtl8211c only. It is now possible to force master mode, but only have it apply to the rtl8211b and rtl8211c.

Quoted 1 line
Signed-off-by Olliver Schinagl <[email protected]>
Acked-by Joe Hershberger <[email protected]>
#14 lore raw

PATCH 1/1 Do not force master mode on unaffected PHY's

In reply to Olliver Schinagl

Hi,

Quoted 4 lines · Olliver Schinagl

The current implementation to force the PHY into master mode is to have a define which affects all realtek PHY's. This is not needed as the problem only exists in the RTL8211C chips. Let us thus turn this into a quirk flag instead.

Series looks good to me:

Reviewed-by Hans de Goede <[email protected]>

Regards,

Hans

author
#15 lore raw

PATCH 1/1 Do not force master mode on unaffected PHY's

Hey Hans,

Quoted 11 lines · Hans de Goede

Hi,

Quoted 7 lines · Olliver Schinagl

The current implementation to force the PHY into master mode is to have a
define which affects all realtek PHY's. This is not needed as the problem
only exists in the RTL8211C chips. Let us thus turn this into a quirk flag
instead.

Quoted 3 lines

Series looks good to me:

Reviewed-by Hans de Goede <[email protected]>

Thanks, but keep your eye on the thread. I'm working on a v3 where i'm pulling the eeprom stuff out of the sunxi stuff, and in the net-uclass layer!

Olliver

Quoted 4 lines

Regards,

Hans

#16 lore raw

PATCH 1/1 Do not force master mode on unaffected PHY's

Hi,

Quoted 12 lines · Olliver Schinagl

Hey Hans,

Quoted 8 lines · Hans de Goede

Hi,

Quoted 4 lines · Olliver Schinagl

The current implementation to force the PHY into master mode is to have a define which affects all realtek PHY's. This is not needed as the problem only exists in the RTL8211C chips. Let us thus turn this into a quirk flag instead.

Quoted 4 lines
Quoted 3 lines

Series looks good to me:

Reviewed-by Hans de Goede <[email protected]>

Thanks, but keep your eye on the thread. I'm working on a v3 where i'm pulling the eeprom stuff out of the sunxi stuff, and in the net-uclass layer!

Erm, this is another series :)

Regards,

Hans

author
#17 lore raw

PATCH 1/1 Do not force master mode on unaffected PHY's

Hans,

Quoted 18 lines · Hans de Goede

Hi,

Quoted 15 lines · Olliver Schinagl

Hey Hans,

Quoted 11 lines · Hans de Goede

Hi,

Quoted 7 lines · Olliver Schinagl

The current implementation to force the PHY into master mode is to have a
define which affects all realtek PHY's. This is not needed as the problem
only exists in the RTL8211C chips. Let us thus turn this into a quirk flag
instead.

Quoted 8 lines
Quoted 6 lines
Quoted 3 lines

Series looks good to me:

Reviewed-by Hans de Goede <[email protected]>

Thanks, but keep your eye on the thread. I'm working on a v3 where i'm pulling the eeprom stuff out of the sunxi stuff, and in the net-uclass layer!

Erm, this is another series :)

Ah poop. You are right! Sorry :)

Quoted 4 lines

Regards,

Hans