Tulip RaQ2 64 Bit Fix

PATCH Jim Gifford 6 participants 10 messages Dec 5, 2005 – Jan 17, 2006

Subsystems (2)

Network drivers MIPS

lore

Share & tools

lore

Jim Gifford

author

2005-12-05 05:19 UTC #0 lore raw

The attached patch allows the tulip driver to work with the RaQ2's network adapter. Without the patch under a 64 bit build, it will never negotiate and will drop packets. This driver is part of Linux Parisc, by Grant Grundler. It's currently in -mm, but Jeff Garzick will not apply it to the main tree.

When Grant modified this driver, he used the manufactures specs on the tulip chip.

Signature · 3 lines

----
Jim Gifford
[email protected]

drivers/net/tulip/21142.c +1 -1

diff -Naur linux-mips-2.6.14.orig/drivers/net/tulip/21142.c linux-mips-2.6.14/drivers/net/tulip/21142.c
@@ -172,7 +172,7 @@
 			int i;
 			for (i = 0; i < tp->mtable->leafcount; i++)
 				if (tp->mtable->mleaf[i].media == dev->if_port) {
-					int startup = ! ((tp->chip_id == DC21143 && (tp->revision == 48 || tp->revision == 65)));
+					int startup = ! ((tp->chip_id == DC21143 && tp->revision == 65));
 					tp->cur_index = i;
 					tulip_select_media(dev, startup);
 					setup_done = 1;

drivers/net/tulip/media.c +37 -3

diff -Naur linux-mips-2.6.14.orig/drivers/net/tulip/media.c linux-mips-2.6.14/drivers/net/tulip/media.c
@@ -44,8 +44,10 @@
 
 /* MII transceiver control section.
    Read and write the MII registers using software-generated serial
-   MDIO protocol.  See the MII specifications or DP83840A data sheet
-   for details. */
+   MDIO protocol.
+   See IEEE 802.3-2002.pdf (Section 2, Chapter "22.2.4 Management functions")
+   or DP83840A data sheet for more details.
+   */
 
 int tulip_mdio_read(struct net_device *dev, int phy_id, int location)
 {

@@ -261,24 +263,56 @@
 				u16 *reset_sequence = &((u16*)(p+3))[init_length];
 				int reset_length = p[2 + init_length*2];
 				misc_info = reset_sequence + reset_length;
-				if (startup)
+				if (startup) {
+					int timeout = 10;	/* max 1 ms */
 					for (i = 0; i < reset_length; i++)
 						iowrite32(get_u16(&reset_sequence[i]) << 16, ioaddr + CSR15);
+				
+					/* flush posted writes */
+					ioread32(ioaddr + CSR15);
+
+					/* Sect 3.10.3 in DP83840A.pdf (p39) */
+					udelay(500);
+
+					/* Section 4.2 in DP83840A.pdf (p43) */
+					/* and IEEE 802.3 "22.2.4.1.1 Reset" */
+					while (timeout-- &&
+						(tulip_mdio_read (dev, phy_num, MII_BMCR) & BMCR_RESET))
+						udelay(100);
+				}
 				for (i = 0; i < init_length; i++)
 					iowrite32(get_u16(&init_sequence[i]) << 16, ioaddr + CSR15);
+
+				ioread32(ioaddr + CSR15);	/* flush posted writes */
 			} else {
 				u8 *init_sequence = p + 2;
 				u8 *reset_sequence = p + 3 + init_length;
 				int reset_length = p[2 + init_length];
 				misc_info = (u16*)(reset_sequence + reset_length);
 				if (startup) {
+					int timeout = 10;	/* max 1 ms */
 					iowrite32(mtable->csr12dir | 0x100, ioaddr + CSR12);
 					for (i = 0; i < reset_length; i++)
 						iowrite32(reset_sequence[i], ioaddr + CSR12);
+
+					/* flush posted writes */
+					ioread32(ioaddr + CSR12);
+
+					/* Sect 3.10.3 in DP83840A.pdf (p39) */
+					udelay(500);
+
+					/* Section 4.2 in DP83840A.pdf (p43) */
+					/* and IEEE 802.3 "22.2.4.1.1 Reset" */
+					while (timeout-- &&
+						(tulip_mdio_read (dev, phy_num, MII_BMCR) & BMCR_RESET))
+						udelay(100);
 				}
 				for (i = 0; i < init_length; i++)
 					iowrite32(init_sequence[i], ioaddr + CSR12);
+
+				ioread32(ioaddr + CSR12);	/* flush posted writes */
 			}
+
 			tmp_info = get_u16(&misc_info[1]);
 			if (tmp_info)
 				tp->advertising[phy_num] = tmp_info | 1;

drivers/net/tulip/tulip_core.c +2 -2

diff -Naur linux-mips-2.6.14.orig/drivers/net/tulip/tulip_core.c linux-mips-2.6.14/drivers/net/tulip/tulip_core.c
@@ -22,7 +22,7 @@
 #else
 #define DRV_VERSION	"1.1.13"
 #endif
-#define DRV_RELDATE	"May 11, 2002"
+#define DRV_RELDATE	"December 15, 2004"
 
 
 #include <linux/module.h>

@@ -148,7 +148,7 @@
 	HAS_MII | HAS_MEDIA_TABLE | CSR12_IN_SROM | HAS_PCI_MWI, tulip_timer },
 
   /* DC21142, DC21143 */
-  { "Digital DS21143 Tulip", 128, 0x0801fbff,
+  { "Digital DS21142/DS21143 Tulip", 128, 0x0801fbff,
 	HAS_MII | HAS_MEDIA_TABLE | ALWAYS_CHECK_MII | HAS_ACPI | HAS_NWAY
 	| HAS_INTR_MITIGATION | HAS_PCI_MWI, t21142_timer },

drivers/net/tulip/tulip.h +5 -2

diff -Naur linux-mips-2.6.14.orig/drivers/net/tulip/tulip.h linux-mips-2.6.14/drivers/net/tulip/tulip.h
@@ -474,8 +474,11 @@
 			udelay(10);
 
 		if (!i)
-			printk(KERN_DEBUG "%s: tulip_stop_rxtx() failed\n",
-					pci_name(tp->pdev));
+			printk(KERN_DEBUG "%s: tulip_stop_rxtx() failed"
+					" (CSR5 0x%x CSR6 0x%x)\n",
+					pci_name(tp->pdev),
+					ioread32(ioaddr + CSR5),
+					ioread32(ioaddr + CSR6));
 	}
 }

Ralf Baechle

2005-12-05 11:46 UTC #1 lore raw

Quoted 5 lines · Jim Gifford

The attached patch allows the tulip driver to work with the RaQ2's network adapter. Without the patch under a 64 bit build, it will never negotiate and will drop packets. This driver is part of Linux Parisc, by Grant Grundler. It's currently in -mm, but Jeff Garzick will not apply it to the main tree.

Why?

Ralf

Martin Michlmayr

2006-01-16 16:01 UTC #2 lore raw

Quoted 7 lines · Ralf Baechle

Quoted 5 lines
The attached patch allows the tulip driver to work with the RaQ2's network adapter. Without the patch under a 64 bit build, it will never negotiate and will drop packets. This driver is part of Linux Parisc, by Grant Grundler. It's currently in -mm, but Jeff Garzick will not apply it to the main tree.
Why?

Jim, I don't think you ever responded to this.

Do you know the status of this patch?

Signature · 2 lines

Martin Michlmayr
http://www.cyrius.com/

Jim Gifford

author

2006-01-16 16:24 UTC #3 lore raw

Quoted 8 lines · Martin Michlmayr

Quoted 6 lines · Ralf Baechle
Quoted 6 lines
The attached patch allows the tulip driver to work with the RaQ2's network adapter. Without the patch under a 64 bit build, it will never negotiate and will drop packets. This driver is part of Linux Parisc, by Grant Grundler. It's currently in -mm, but Jeff Garzick will not apply it to the main tree.

Quoted 7 lines

Quoted 2 lines
Why?
Jim, I don't think you ever responded to this.
Do you know the status of this patch?

Jeff Garzick refuses to apply it do to spinlocks. Andrew Morton is including in his tree because it fixes issue with Parisc and with MIPS based builds. So it's kinda of what is the right thing to do. I also use this driver on my x86 builds, and it actually performs better. Here is a little history of how Grant made the driver.

Grant Grundler is the network maintainer for Parisc Linux.
He discovered that the tulip driver didn't perform that well. He researched the manufactures documentation and found out how to fix the driver to work to its optimum performance. He did this back in 2003, has submitted it to Jeff Garzick several times with no response. Around late 2004, I started to do test builds on 64 bit on my RaQ2 and discovered that the driver would not auto-negotiate transfer speeds. Talked to numerous people, then someone put me in touch with Grant. I tested the driver for about 2 weeks, ask Grant why it wasn't sent upstream, he told me about the spinlock issue. I then contacted Andrew Morton, explained everything as I am here, and he agreed it was needed and tried to get Jeff to add it. Jeff sends back a one liner say doing to it's use of spinlocks it's not accepted.

That's the gory history.

Signature · 3 lines

----
Jim Gifford
[email protected]

Martin Michlmayr

2006-01-16 16:55 UTC #4 lore raw

Quoted 13 lines · Jim Gifford

Quoted 7 lines
Quoted 7 lines
Quoted 6 lines
The attached patch allows the tulip driver to work with the RaQ2's network adapter. Without the patch under a 64 bit build, it will never negotiate and will drop packets. This driver is part of Linux Parisc, by Grant Grundler. It's currently in -mm, but Jeff Garzick will not apply it to the main tree.
Why?
Jeff Garzick refuses to apply it do to spinlocks. Andrew Morton is including in his tree because it fixes issue with Parisc and with MIPS based builds. So it's kinda of what is the right thing to do. I also use this driver on my x86 builds, and it actually performs better. Here is a little history of how Grant made the driver.

Quoted 14 lines

Grant Grundler is the network maintainer for Parisc Linux. He discovered that the tulip driver didn't perform that well. He researched the manufactures documentation and found out how to fix the driver to work to its optimum performance. He did this back in 2003, has submitted it to Jeff Garzick several times with no response. Around late 2004, I started to do test builds on 64 bit on my RaQ2 and discovered that the driver would not auto-negotiate transfer speeds. Talked to numerous people, then someone put me in touch with Grant. I tested the driver for about 2 weeks, ask Grant why it wasn't sent upstream, he told me about the spinlock issue. I then contacted Andrew Morton, explained everything as I am here, and he agreed it was needed and tried to get Jeff to add it. Jeff sends back a one liner say doing to it's use of spinlocks it's not accepted.

Andrew, do you think that issue will be resolved in some way at some point?

Signature · 2 lines

Martin Michlmayr
http://www.cyrius.com/

Andrew Morton

2006-01-17 01:20 UTC #5 lore raw

Quoted 30 lines · Martin Michlmayr

Quoted 13 lines · Jim Gifford
Quoted 7 lines
Quoted 7 lines
Quoted 6 lines
The attached patch allows the tulip driver to work with the RaQ2's network adapter. Without the patch under a 64 bit build, it will never negotiate and will drop packets. This driver is part of Linux Parisc, by Grant Grundler. It's currently in -mm, but Jeff Garzick will not apply it to the main tree.
Why?
Jeff Garzick refuses to apply it do to spinlocks. Andrew Morton is including in his tree because it fixes issue with Parisc and with MIPS based builds. So it's kinda of what is the right thing to do. I also use this driver on my x86 builds, and it actually performs better. Here is a little history of how Grant made the driver.
Quoted 14 lines
Grant Grundler is the network maintainer for Parisc Linux. He discovered that the tulip driver didn't perform that well. He researched the manufactures documentation and found out how to fix the driver to work to its optimum performance. He did this back in 2003, has submitted it to Jeff Garzick several times with no response. Around late 2004, I started to do test builds on 64 bit on my RaQ2 and discovered that the driver would not auto-negotiate transfer speeds. Talked to numerous people, then someone put me in touch with Grant. I tested the driver for about 2 weeks, ask Grant why it wasn't sent upstream, he told me about the spinlock issue. I then contacted Andrew Morton, explained everything as I am here, and he agreed it was needed and tried to get Jeff to add it. Jeff sends back a one liner say doing to it's use of spinlocks it's not accepted.

Quoted 3 lines

Andrew, do you think that issue will be resolved in some way at some point?

This has been hanging around for too long. We need to get it over the hump.

Jeff, can you please suggest how this patch should be altered to make it acceptable?

Thanks.

From: Jim Gifford <[email protected]>,
Grant Grundler <[email protected]>,
Peter Horton <[email protected]>

With Grant's help I was able to get the tulip driver to work with 64 bit MIPS.

Signed-off-by Andrew Morton <[email protected]>

---

 drivers/net/tulip/media.c      |   22 ++++++++++++++++++++--
 drivers/net/tulip/tulip.h      |    7 +++++--
 drivers/net/tulip/tulip_core.c |    2 +-
 net/tulip/eeprom.c             |    0 
 net/tulip/interrupt.c          |    0 
 5 files changed, 26 insertions(+), 5 deletions(-)

diff -puN drivers/net/tulip/eeprom.c~tulip-fix-for-64-bit-mips drivers/net/tulip/eeprom.c diff -puN drivers/net/tulip/interrupt.c~tulip-fix-for-64-bit-mips drivers/net/tulip/interrupt.c

drivers/net/tulip/media.c renamed from drivers/net/tulip/media.c~tulip-fix-for-64-bit-mips +20 -2

diff -puN drivers/net/tulip/media.c~tulip-fix-for-64-bit-mips drivers/net/tulip/media.c
@@ -44,8 +44,10 @@ static const unsigned char comet_miireg2
 
 /* MII transceiver control section.
    Read and write the MII registers using software-generated serial
-   MDIO protocol.  See the MII specifications or DP83840A data sheet
-   for details. */
+   MDIO protocol.
+   See IEEE 802.3-2002.pdf (Section 2, Chapter "22.2.4 Management functions")
+   or DP83840A data sheet for more details.
+   */
 
 int tulip_mdio_read(struct net_device *dev, int phy_id, int location)
 {

@@ -272,13 +274,29 @@ void tulip_select_media(struct net_devic
 				int reset_length = p[2 + init_length];
 				misc_info = (u16*)(reset_sequence + reset_length);
 				if (startup) {
+					int timeout = 10;	/* max 1 ms */
 					iowrite32(mtable->csr12dir | 0x100, ioaddr + CSR12);
 					for (i = 0; i < reset_length; i++)
 						iowrite32(reset_sequence[i], ioaddr + CSR12);
+
+					/* flush posted writes */
+					ioread32(ioaddr + CSR12);
+
+					/* Sect 3.10.3 in DP83840A.pdf (p39) */
+					udelay(500);
+
+					/* Section 4.2 in DP83840A.pdf (p43) */
+					/* and IEEE 802.3 "22.2.4.1.1 Reset" */
+					while (timeout-- &&
+						(tulip_mdio_read (dev, phy_num, MII_BMCR) & BMCR_RESET))
+						udelay(100);
 				}
 				for (i = 0; i < init_length; i++)
 					iowrite32(init_sequence[i], ioaddr + CSR12);
+
+				ioread32(ioaddr + CSR12);	/* flush posted writes */
 			}
+
 			tmp_info = get_u16(&misc_info[1]);
 			if (tmp_info)
 				tp->advertising[phy_num] = tmp_info | 1;

drivers/net/tulip/tulip_core.c renamed from drivers/net/tulip/tulip_core.c~tulip-fix-for-64-bit-mips +1 -1

diff -puN drivers/net/tulip/tulip_core.c~tulip-fix-for-64-bit-mips drivers/net/tulip/tulip_core.c
@@ -22,7 +22,7 @@
 #else
 #define DRV_VERSION	"1.1.13"
 #endif
-#define DRV_RELDATE	"May 11, 2002"
+#define DRV_RELDATE	"December 15, 2004"
 
 
 #include <linux/module.h>

drivers/net/tulip/tulip.h renamed from drivers/net/tulip/tulip.h~tulip-fix-for-64-bit-mips +5 -2

diff -puN drivers/net/tulip/tulip.h~tulip-fix-for-64-bit-mips drivers/net/tulip/tulip.h
@@ -474,8 +474,11 @@ static inline void tulip_stop_rxtx(struc
 			udelay(10);
 
 		if (!i)
-			printk(KERN_DEBUG "%s: tulip_stop_rxtx() failed\n",
-					pci_name(tp->pdev));
+			printk(KERN_DEBUG "%s: tulip_stop_rxtx() failed"
+					" (CSR5 0x%x CSR6 0x%x)\n",
+					pci_name(tp->pdev),
+					ioread32(ioaddr + CSR5),
+					ioread32(ioaddr + CSR6));
 	}
 }

Jeff Garzik

2006-01-17 01:33 UTC #6 lore raw

Quoted 39 lines · Andrew Morton

Quoted 32 lines · Martin Michlmayr
Quoted 15 lines · Jim Gifford
Quoted 8 lines
Quoted 8 lines
Quoted 6 lines
The attached patch allows the tulip driver to work with the RaQ2's network adapter. Without the patch under a 64 bit build, it will never negotiate and will drop packets. This driver is part of Linux Parisc, by Grant Grundler. It's currently in -mm, but Jeff Garzick will not apply it to the main tree.
Why?
Jeff Garzick refuses to apply it do to spinlocks. Andrew Morton is including in his tree because it fixes issue with Parisc and with MIPS based builds. So it's kinda of what is the right thing to do. I also use this driver on my x86 builds, and it actually performs better. Here is a little history of how Grant made the driver.
Quoted 14 lines
Grant Grundler is the network maintainer for Parisc Linux. He discovered that the tulip driver didn't perform that well. He researched the manufactures documentation and found out how to fix the driver to work to its optimum performance. He did this back in 2003, has submitted it to Jeff Garzick several times with no response. Around late 2004, I started to do test builds on 64 bit on my RaQ2 and discovered that the driver would not auto-negotiate transfer speeds. Talked to numerous people, then someone put me in touch with Grant. I tested the driver for about 2 weeks, ask Grant why it wasn't sent upstream, he told me about the spinlock issue. I then contacted Andrew Morton, explained everything as I am here, and he agreed it was needed and tried to get Jeff to add it. Jeff sends back a one liner say doing to it's use of spinlocks it's not accepted.
Quoted 3 lines
Andrew, do you think that issue will be resolved in some way at some point?

Quoted 4 lines

This has been hanging around for too long. We need to get it over the hump.
Jeff, can you please suggest how this patch should be altered to make it acceptable?

Answer hasn't changed since this was last discussed: sleep, rather than delay for an extra-long time. That's the only hurdle for the tulip patches you keep resending.

Francois Romieu even had an untested patch that attempted this, somewhere.

Jeff

Grant Grundler

2006-01-17 02:22 UTC #7 lore raw

Quoted 8 lines · Jeff Garzik

Quoted 2 lines
Jeff, can you please suggest how this patch should be altered to make it acceptable?
Answer hasn't changed since this was last discussed: sleep, rather than delay for an extra-long time. That's the only hurdle for the tulip patches you keep resending.
Francois Romieu even had an untested patch that attempted this, somewhere.

Yes, he implemented a workqueue to invoke tulip_select_media().

	http://lkml.org/lkml/2005/5/21/69

His patch didn't deal with the same issue in tulip_restart_rxtx() as noted here:

	http://lkml.org/lkml/2005/5/22/6

Otherwise, it was mostly ok - just some other nits.
Last reply on that thread was Oct 2005: "an updated version is cooking".

hth,
grant

Andrew Morton

2006-01-17 02:26 UTC #8 lore raw

Quoted 12 lines · Grant Grundler

Quoted 8 lines · Jeff Garzik
Quoted 2 lines
Jeff, can you please suggest how this patch should be altered to make it acceptable?
Answer hasn't changed since this was last discussed: sleep, rather than delay for an extra-long time. That's the only hurdle for the tulip patches you keep resending.
Francois Romieu even had an untested patch that attempted this, somewhere.
Yes, he implemented a workqueue to invoke tulip_select_media().

Quoted 9 lines

	http://lkml.org/lkml/2005/5/21/69
His patch didn't deal with the same issue in tulip_restart_rxtx() as noted here:
	http://lkml.org/lkml/2005/5/22/6
Otherwise, it was mostly ok - just some other nits.
Last reply on that thread was Oct 2005: "an updated version is cooking".

Might help to cc him.

Look, we really don't care who writes the patch.

Grant Grundler

2006-01-16 20:23 UTC #9 lore raw

In reply to Jim Gifford

Quoted 1 line · Jim Gifford

Jeff Garzick refuses to apply it do to spinlocks.

Jeff refuses to apply the tulip phy init patch because it could hold off interrupts for up to 2.5ms. I agree this is not a good "side effect" of this patch. However, rewriting tulip initialization sequence to avoid this "side effect" is non-trivial.
And in practice, the interrupts are held off only for 600us or so.

Quoted 3 lines

Andrew Morton is
including in his tree because it fixes issue with Parisc and with MIPS based builds.

and ia64-linux.

Quoted 6 lines

So it's kinda of what is the right thing to do. I also use
this driver on my x86 builds, and it actually performs better. Here is a little history of how Grant made the driver.
Grant Grundler is the network maintainer for Parisc Linux. He discovered that the tulip driver didn't perform that well.

No, with faster CPUs, tulip just didn't work on parisc-linux or ia64-linux. Exact same symptom you had on the mips platform.

Quoted 3 lines

He
researched the manufactures documentation and found out how to fix the driver to work to its optimum performance. He did this back in 2003, has

Oct 2002 actually.
http://lists.parisc-linux.org/pipermail/parisc-linux-cvs/2002-October/031613.html

That was a first mostly correct version.
Here's the "final" patch (at the time):
http://lists.parisc-linux.org/pipermail/parisc-linux-cvs/2002-December/032081.html

Quoted 1 line

submitted it to Jeff Garzick several times with no response.

That's not fair to Jeff - as much as I think he's being juvenile in this case. Jeff and I did exchange email. He's just trying "encourage" me to rewrite the driver initialization sequence. Not interested. I prefer to maintain this patch in parisc-linux source tree myself.

Quoted 8 lines

Around late
2004, I started to do test builds on 64 bit on my RaQ2 and discovered that the driver would not auto-negotiate transfer speeds. Talked to numerous people, then someone put me in touch with Grant. I tested the driver for about 2 weeks, ask Grant why it wasn't sent upstream, he told me about the spinlock issue. I then contacted Andrew Morton, explained everything as I am here, and he agreed it was needed and tried to get Jeff to add it.

I happened to talk to Andrew about this at OLS2004 - just before you showed up with the nice failure case on Mips:

	http://www.spinics.net/lists/linux-net/msg11267.html

And a second, similar patch that I had outstanding at the same time:

	http://www.spinics.net/lists/linux-net/msg11268.html

Quoted 2 lines

Jeff sends back a one liner say doing to it's use of
spinlocks it's not accepted.

I didn't need a longer explanation - I understood his concern.

Quoted 1 line

That's the gory history.

Sorry - it's more gory than you thought. :)

cheers,
grant