phy/gianfar: Fixes for gianfar and vitesse RGMII-ID support

PATCH In review Andy Fleming 4 participants 8 messages Jul 17, 2007

Subsystems (6)

Networking Network drivers Device tree bindings Documentation +2 more subsystems

lore

Share & tools

lore

Andy Fleming

author

2007-07-17 01:27 UTC #0 lore raw

cover phy/gianfar: Fixes for gianfar and vitesse RGMII-ID support

A few bugs in the Vitesse PHY driver were found on the 8641D HPCN board. Originally, they were masked by a bug in the PHY Lib which was fixed by patch 5f708dd91d15876e26d7a57f97a255cedffca463 (Fix phy_id for Vitesse 824x PHY). That patch allowed the Vitesse PHY to bind on the 8641D HPCN board, thereby exposing a bug in the interrupt handling and a bug in the configuration of the PHY. This sequence of patches fixes the irq handling bug, then fixes the configuration bug in 3 places:

1) The Vitesse PHY driver
2) The gianfar driver (needs to pass in the mode correctly for internal delay)
3) The OF device tree for arch/powerpc boards

You can apply the following patches, or pull them directly:

The following changes since commit c5e3ae8823693b260ce1f217adca8add1bc0b3de:
Ayaz Abdulla (1):
forcedeth bug fix: realtek phy

are found in the git repository at:

http://opensource.freescale.com/pub/scm/linux-2.6-85xx.git netdev

Andy Fleming (4):
Fix Vitesse 824x PHY interrupt acking
Add phy-connection-type to gianfar nodes
Fix Vitesse RGMII-ID support
Fix RGMII-ID handling in gianfar

 Documentation/powerpc/booting-without-of.txt |    6 +++
 arch/powerpc/boot/dts/mpc8641_hpcn.dts       |    4 ++
 arch/powerpc/sysdev/fsl_soc.c                |    9 +++++
 drivers/net/gianfar.c                        |   12 ++++++-
 drivers/net/phy/vitesse.c                    |   46 +++++++++++++++++++++++---
 include/linux/fsl_devices.h                  |    1 +
 6 files changed, 72 insertions(+), 6 deletions(-)

Andy Fleming

author

2007-07-17 01:27 UTC #1 lore raw

PATCH 1/4 phy: Fix Vitesse 824x PHY interrupt acking

The Vitesse 824x PHY doesn't allow an interrupt to be cleared if the mask bit for that interrupt isn't set. This means that the PHY Lib's order of handling interrupts (disable, then clear) breaks on this PHY. However, clearing then disabling the interrupt opens up the code for a silly race condition. So rather than change the PHY Lib, we change the Vitesse driver so it always clears interrupts before disabling them. Further, the ack function only clears the interrupt if interrupts are enabled.

Signed-off-by Andy Fleming <[email protected]>

Signed-off-by York Sun <[email protected]>

Acked-by Haiying Wang <[email protected]>

---
 drivers/net/phy/vitesse.c |   23 +++++++++++++++++++++--
 1 files changed, 21 insertions(+), 2 deletions(-)

drivers/net/phy/vitesse.c +21 -2

index 596222b..f39ab76 100644
@@ -65,7 +65,15 @@ static int vsc824x_config_init(struct phy_device *phydev)
 
 static int vsc824x_ack_interrupt(struct phy_device *phydev)
 {
-	int err = phy_read(phydev, MII_VSC8244_ISTAT);
+	int err = 0;
+	
+	/*
+	 * Don't bother to ACK the interrupts if interrupts
+	 * are disabled.  The 824x cannot clear the interrupts
+	 * if they are disabled.
+	 */
+	if (phydev->interrupts == PHY_INTERRUPT_ENABLED)
+		err = phy_read(phydev, MII_VSC8244_ISTAT);
 
 	return (err < 0) ? err : 0;
 }

@@ -77,8 +85,19 @@ static int vsc824x_config_intr(struct phy_device *phydev)
 	if (phydev->interrupts == PHY_INTERRUPT_ENABLED)
 		err = phy_write(phydev, MII_VSC8244_IMASK,
 				MII_VSC8244_IMASK_MASK);
-	else
+	else {
+		/*
+		 * The Vitesse PHY cannot clear the interrupt
+		 * once it has disabled them, so we clear them first
+		 */
+		err = phy_read(phydev, MII_VSC8244_ISTAT);
+
+		if (err)
+			return err;
+
 		err = phy_write(phydev, MII_VSC8244_IMASK, 0);
+	}
+
 	return err;
 }

Signature · 1 line

1.5.0.2.230.gfbe3d-dirty

Andy Fleming

author

2007-07-17 01:27 UTC #2 lore raw

PATCH 2/4 gianfar: Add phy-connection-type to gianfar nodes

The TSEC/eTSEC automatically detect their PHY interface type, unless the type is RGMII-ID (RGMII with internal delay). In that situation, it just detects RGMII. In order to fix this, we need to pass in rgmii-id if that is the connection type.

Signed-off-by Andy Fleming <[email protected]>

---
 Documentation/powerpc/booting-without-of.txt |    6 ++++++
 arch/powerpc/boot/dts/mpc8641_hpcn.dts       |    4 ++++
 2 files changed, 10 insertions(+), 0 deletions(-)

Documentation/powerpc/booting-without-of.txt +6 -0

index d42d981..a41b6b1 100644
@@ -1241,6 +1241,12 @@ platforms are moved over to use the flattened-device-tree model.
       network device.  This is used by the bootwrapper to interpret
       MAC addresses passed by the firmware when no information other
       than indices is available to associate an address with a device.
+    - phy-connection-type : a string naming the controller/PHY interface type,
+      i.e., "mii" (default), "rmii", "gmii", "rgmii", "rgmii-id", "sgmii",
+      "tbi", or "rtbi".  This property is only really needed if the connection
+      is of type "rgmii-id", as all other connection types are detected by
+      hardware.
+
 
   Example:

arch/powerpc/boot/dts/mpc8641_hpcn.dts +4 -0

index 260b264..9849073 100644
@@ -121,6 +121,7 @@
 			interrupts = <1d 2 1e 2 22 2>;
 			interrupt-parent = <&mpic>;
 			phy-handle = <&phy0>;
+			phy-connection-type = "rgmii-id";
 		};
 
 		ethernet@25000 {

@@ -134,6 +135,7 @@
 			interrupts = <23 2 24 2 28 2>;
 			interrupt-parent = <&mpic>;
 			phy-handle = <&phy1>;
+			phy-connection-type = "rgmii-id";
 		};
 		
 		ethernet@26000 {

@@ -147,6 +149,7 @@
 			interrupts = <1F 2 20 2 21 2>;
 			interrupt-parent = <&mpic>;
 			phy-handle = <&phy2>;
+			phy-connection-type = "rgmii-id";
 		};
 
 		ethernet@27000 {

@@ -160,6 +163,7 @@
 			interrupts = <25 2 26 2 27 2>;
 			interrupt-parent = <&mpic>;
 			phy-handle = <&phy3>;
+			phy-connection-type = "rgmii-id";
 		};
 		serial@4500 {
 			device_type = "serial";

Signature · 1 line

1.5.0.2.230.gfbe3d-dirty

Andy Fleming

author

2007-07-17 01:27 UTC #3 lore raw

PATCH 3/4 phy: Fix Vitesse RGMII-ID support

The Vitesse PHY on the 8641D needs to be set up with internal delay to work in RGMII mode. So we add skew when it is set to RGMII_ID mode.

Signed-off-by Andy Fleming <[email protected]>

Signed-off-by Haruki Dai <[email protected]>

Signed-off-by Haiying Wang <[email protected]>

---
 drivers/net/phy/vitesse.c |   23 ++++++++++++++++++++---
 1 files changed, 20 insertions(+), 3 deletions(-)

drivers/net/phy/vitesse.c +20 -3

index f39ab76..6a53856 100644
@@ -21,6 +21,10 @@
 /* Vitesse Extended Control Register 1 */
 #define MII_VSC8244_EXT_CON1           0x17
 #define MII_VSC8244_EXTCON1_INIT       0x0000
+#define MII_VSC8244_EXTCON1_TX_SKEW_MASK	0x0c00
+#define MII_VSC8244_EXTCON1_RX_SKEW_MASK	0x0300
+#define MII_VSC8244_EXTCON1_TX_SKEW	0x0800
+#define MII_VSC8244_EXTCON1_RX_SKEW	0x0200
 
 /* Vitesse Interrupt Mask Register */
 #define MII_VSC8244_IMASK		0x19

@@ -39,7 +43,7 @@
 
 /* Vitesse Auxiliary Control/Status Register */
 #define MII_VSC8244_AUX_CONSTAT        	0x1c
-#define MII_VSC8244_AUXCONSTAT_INIT    	0x0004
+#define MII_VSC8244_AUXCONSTAT_INIT    	0x0000
 #define MII_VSC8244_AUXCONSTAT_DUPLEX  	0x0020
 #define MII_VSC8244_AUXCONSTAT_SPEED   	0x0018
 #define MII_VSC8244_AUXCONSTAT_GBIT    	0x0010

@@ -51,6 +55,7 @@ MODULE_LICENSE("GPL");
 
 static int vsc824x_config_init(struct phy_device *phydev)
 {
+	int extcon;
 	int err;
 
 	err = phy_write(phydev, MII_VSC8244_AUX_CONSTAT,

@@ -58,8 +63,20 @@ static int vsc824x_config_init(struct phy_device *phydev)
 	if (err < 0)
 		return err;
 
-	err = phy_write(phydev, MII_VSC8244_EXT_CON1,
-			MII_VSC8244_EXTCON1_INIT);
+	extcon = phy_read(phydev, MII_VSC8244_EXT_CON1);
+
+	if (extcon < 0)
+		return err;
+
+	extcon &= ~(MII_VSC8244_EXTCON1_TX_SKEW_MASK |
+			MII_VSC8244_EXTCON1_RX_SKEW_MASK);
+
+	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
+		extcon |= (MII_VSC8244_EXTCON1_TX_SKEW |
+				MII_VSC8244_EXTCON1_RX_SKEW);
+
+	err = phy_write(phydev, MII_VSC8244_EXT_CON1, extcon);
+
 	return err;
 }

Signature · 1 line

1.5.0.2.230.gfbe3d-dirty

Andy Fleming

author

2007-07-17 01:27 UTC #4 lore raw

PATCH 4/4 gianfar: Fix RGMII-ID handling in gianfar

The TSEC/eTSEC can detect the interface to the PHY automatically, but it isn't able to detect whether the RGMII connection needs internal delay. So we need to detect that change in the device tree, propagate it to the platform data, and then check it if we're in RGMII. This fixes a bug on the 8641D HPCN board where the Vitesse PHY doesn't use the delay for RGMII.

Signed-off-by Andy Fleming <[email protected]>

---
 arch/powerpc/sysdev/fsl_soc.c |    9 +++++++++
 drivers/net/gianfar.c         |   12 +++++++++++-
 include/linux/fsl_devices.h   |    1 +
 3 files changed, 21 insertions(+), 1 deletions(-)

arch/powerpc/sysdev/fsl_soc.c +9 -0

index cad1757..64efee6 100644
@@ -197,6 +197,7 @@ static int __init gfar_of_init(void)
 		struct gianfar_platform_data gfar_data;
 		const unsigned int *id;
 		const char *model;
+		const char *ctype;
 		const void *mac_addr;
 		const phandle *ph;
 		int n_res = 2;

@@ -254,6 +255,14 @@ static int __init gfar_of_init(void)
 			    FSL_GIANFAR_DEV_HAS_VLAN |
 			    FSL_GIANFAR_DEV_HAS_EXTENDED_HASH;
 
+		ctype = of_get_property(np, "phy-connection-type", NULL);
+
+		/* We only care about rgmii-id.  The rest are autodetected */
+		if (ctype && !strcmp(ctype, "rgmii-id"))
+			gfar_data.interface = PHY_INTERFACE_MODE_RGMII_ID;
+		else
+			gfar_data.interface = PHY_INTERFACE_MODE_MII;
+
 		ph = of_get_property(np, "phy-handle", NULL);
 		phy = of_find_node_by_phandle(*ph);

drivers/net/gianfar.c +11 -1

index d7a1a58..f926905 100644
@@ -420,8 +420,18 @@ static phy_interface_t gfar_get_interface(struct net_device *dev)
 	if (ecntrl & ECNTRL_REDUCED_MODE) {
 		if (ecntrl & ECNTRL_REDUCED_MII_MODE)
 			return PHY_INTERFACE_MODE_RMII;
-		else
+		else {
+			phy_interface_t interface = priv->einfo->interface;
+
+			/*
+			 * This isn't autodetected right now, so it must
+			 * be set by the device tree or platform code.
+			 */
+			if (interface == PHY_INTERFACE_MODE_RGMII_ID)
+				return PHY_INTERFACE_MODE_RGMII_ID;
+
 			return PHY_INTERFACE_MODE_RGMII;
+		}
 	}
 
 	if (priv->einfo->device_flags & FSL_GIANFAR_DEV_HAS_GIGABIT)

include/linux/fsl_devices.h +1 -0

index 73710d6..2de358f 100644
@@ -53,6 +53,7 @@ struct gianfar_platform_data {
 	u32	bus_id;
 	u32	phy_id;
 	u8	mac_addr[6];
+	phy_interface_t interface;
 };
 
 struct gianfar_mdio_data {

Signature · 1 line

1.5.0.2.230.gfbe3d-dirty

Jeff Garzik

2007-07-17 05:07 UTC #5 lore raw

In reply to Andy Fleming

Quoted 26 lines · Andy Fleming

A few bugs in the Vitesse PHY driver were found on the 8641D HPCN board.
Originally, they were masked by a bug in the PHY Lib which was fixed by patch 5f708dd91d15876e26d7a57f97a255cedffca463 (Fix phy_id for Vitesse 824x PHY). That patch allowed the Vitesse PHY to bind on the 8641D HPCN board, thereby
exposing a bug in the interrupt handling and a bug in the configuration of
the PHY. This sequence of patches fixes the irq handling bug, then fixes the configuration bug in 3 places:
1) The Vitesse PHY driver
2) The gianfar driver (needs to pass in the mode correctly for internal delay)
3) The OF device tree for arch/powerpc boards
You can apply the following patches, or pull them directly:
The following changes since commit c5e3ae8823693b260ce1f217adca8add1bc0b3de:
Ayaz Abdulla (1):
forcedeth bug fix: realtek phy
are found in the git repository at:
http://opensource.freescale.com/pub/scm/linux-2.6-85xx.git netdev
Andy Fleming (4):
Fix Vitesse 824x PHY interrupt acking
Add phy-connection-type to gianfar nodes
Fix Vitesse RGMII-ID support
Fix RGMII-ID handling in gianfar

Quoted 7 lines

 Documentation/powerpc/booting-without-of.txt |    6 +++
 arch/powerpc/boot/dts/mpc8641_hpcn.dts       |    4 ++
 arch/powerpc/sysdev/fsl_soc.c                |    9 +++++
 drivers/net/gianfar.c                        |   12 ++++++-
 drivers/net/phy/vitesse.c                    |   46 +++++++++++++++++++++++---
 include/linux/fsl_devices.h                  |    1 +
 6 files changed, 72 insertions(+), 6 deletions(-)

pulled

Kumar Gala

2007-07-17 05:16 UTC #6 lore raw

Greg,

Can you look at pulling these fixes for the next 2.6.22.x stable release.

- k

Quoted 31 lines · Jeff Garzik

Quoted 19 lines · Andy Fleming
A few bugs in the Vitesse PHY driver were found on the 8641D HPCN board.
Originally, they were masked by a bug in the PHY Lib which was fixed by patch 5f708dd91d15876e26d7a57f97a255cedffca463 (Fix phy_id for Vitesse 824x PHY). That patch allowed the Vitesse PHY to bind on the 8641D HPCN board, thereby
exposing a bug in the interrupt handling and a bug in the configuration of
the PHY. This sequence of patches fixes the irq handling bug, then fixes the configuration bug in 3 places:
1) The Vitesse PHY driver
2) The gianfar driver (needs to pass in the mode correctly for internal delay)
3) The OF device tree for arch/powerpc boards
You can apply the following patches, or pull them directly:
The following changes since commit c5e3ae8823693b260ce1f217adca8add1bc0b3de:
Ayaz Abdulla (1):
forcedeth bug fix: realtek phy
are found in the git repository at:
http://opensource.freescale.com/pub/scm/linux-2.6-85xx.git netdev
Andy Fleming (4):
Fix Vitesse 824x PHY interrupt acking
Add phy-connection-type to gianfar nodes
Fix Vitesse RGMII-ID support
Fix RGMII-ID handling in gianfar
Quoted 8 lines
 Documentation/powerpc/booting-without-of.txt |    6 +++
 arch/powerpc/boot/dts/mpc8641_hpcn.dts       |    4 ++
 arch/powerpc/sysdev/fsl_soc.c                |    9 +++++
 drivers/net/gianfar.c                        |   12 ++++++-
 drivers/net/phy/vitesse.c                    |   46 ++++++++++++++ 
+++++++++---
 include/linux/fsl_devices.h                  |    1 +
 6 files changed, 72 insertions(+), 6 deletions(-)
pulled

Greg KH

2007-07-17 05:27 UTC #7 lore raw

Quoted 3 lines · Kumar Gala

Greg,
Can you look at pulling these fixes for the next 2.6.22.x stable release.

Can you send them as patches, to the [email protected] address, after they go into Linus's tree?

thanks,

greg k-h