[0/4][via-rhine] Improvements

[0/4][via-rhine] Improvements

[Roger Luethi]

Here comes a batch of patches for the via-rhine driver. Please apply.

via-rhine is still hardly usable on the most common Rhine hardware; it
can't sustain 100Mbps traffic. The changes presented here improve the
situation considerably; they fix a number of real problems and have been
tested for regression (alas, by few people).

Roger

[1/4][via-rhine][PATCH] Trivial changes; not affecting functionality

[Roger Luethi]

[-- Attachment #1: Type: text/plain, Size: 164 bytes --]

- nuke mii_status_bits
- move clear_tally_counters() up so it can get inlined (pointed out by
  Luca Barbieri)
- rename via_restart_tx() -> via_rhine_restart_tx()


[-- Attachment #2: via-rhine-1.16rc-01_trivial.diff --]
[-- Type: text/plain, Size: 4388 bytes --]

--- 00_2.5.61/drivers/net/via-rhine.c	Sat Feb 15 10:42:53 2003
+++ 01_trivial/drivers/net/via-rhine.c	Fri Feb 14 18:09:48 2003
@@ -425,20 +425,6 @@ enum intr_status_bits {
 	IntrNormalSummary=0x0003, IntrAbnormalSummary=0xC260,
 };
 
-/* MII interface, status flags.
-   Not to be confused with the MIIStatus register ... */
-enum mii_status_bits {
-	MIICap100T4			= 0x8000,
-	MIICap10100HdFd		= 0x7800,
-	MIIPreambleSupr		= 0x0040,
-	MIIAutoNegCompleted	= 0x0020,
-	MIIRemoteFault		= 0x0010,
-	MIICapAutoNeg		= 0x0008,
-	MIILink				= 0x0004,
-	MIIJabber			= 0x0002,
-	MIIExtended			= 0x0001
-};
-
 /* The Rx and Tx buffer descriptors. */
 struct rx_desc {
 	s32 rx_status;
@@ -533,8 +519,6 @@ static void via_rhine_set_rx_mode(struct
 static struct net_device_stats *via_rhine_get_stats(struct net_device *dev);
 static int netdev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd);
 static int  via_rhine_close(struct net_device *dev);
-static inline void clear_tally_counters(long ioaddr);
-static inline void via_restart_tx(struct net_device *dev);
 
 static void wait_for_reset(struct net_device *dev, int chip_id, char *name)
 {
@@ -797,7 +781,7 @@ static int __devinit via_rhine_init_one 
 					   mdio_read(dev, phy, 5));
 
 				/* set IFF_RUNNING */
-				if (mii_status & MIILink)
+				if (mii_status & BMSR_LSTATUS)
 					netif_carrier_on(dev);
 				else
 					netif_carrier_off(dev);
@@ -1175,8 +1159,8 @@ static void via_rhine_timer(unsigned lon
 
 	/* make IFF_RUNNING follow the MII status bit "Link established" */
 	mii_status = mdio_read(dev, np->phys[0], MII_BMSR);
-	if ( (mii_status & MIILink) != (np->mii_status & MIILink) ) {
-		if (mii_status & MIILink)
+	if ( (mii_status & BMSR_LSTATUS) != (np->mii_status & BMSR_LSTATUS) ) {
+		if (mii_status & BMSR_LSTATUS)
 			netif_carrier_on(dev);
 		else
 			netif_carrier_off(dev);
@@ -1414,8 +1398,8 @@ static void via_rhine_rx(struct net_devi
 	int boguscnt = np->dirty_rx + RX_RING_SIZE - np->cur_rx;
 
 	if (debug > 4) {
-		printk(KERN_DEBUG " In via_rhine_rx(), entry %d status %8.8x.\n",
-			   entry, le32_to_cpu(np->rx_head_desc->rx_status));
+		printk(KERN_DEBUG "%s: via_rhine_rx(), entry %d status %8.8x.\n",
+			   dev->name, entry, le32_to_cpu(np->rx_head_desc->rx_status));
 	}
 
 	/* If EOP is set on the next entry, it's a new packet. Send it up. */
@@ -1521,7 +1505,18 @@ static void via_rhine_rx(struct net_devi
 	writew(CmdRxDemand | np->chip_cmd, dev->base_addr + ChipCmd);
 }
 
-static inline void via_restart_tx(struct net_device *dev) {
+/* Clears the "tally counters" for CRC errors and missed frames(?).
+   It has been reported that some chips need a write of 0 to clear
+   these, for others the counters are set to 1 when written to and
+   instead cleared when read. So we clear them both ways ... */
+static inline void clear_tally_counters(const long ioaddr)
+{
+	writel(0, ioaddr + RxMissed);
+	readw(ioaddr + RxCRCErrs);
+	readw(ioaddr + RxMissed);
+}
+
+static inline void via_rhine_restart_tx(struct net_device *dev) {
 	struct netdev_private *np = dev->priv;
 	int entry = np->dirty_tx % TX_RING_SIZE;
 
@@ -1561,7 +1556,7 @@ static void via_rhine_error(struct net_d
 		if (debug > 1)
 			printk(KERN_INFO "%s: Abort %4.4x, frame dropped.\n",
 				   dev->name, intr_status);
-		via_restart_tx(dev);
+		via_rhine_restart_tx(dev);
 	}
 	if (intr_status & IntrTxUnderrun) {
 		if (np->tx_thresh < 0xE0)
@@ -1570,7 +1565,7 @@ static void via_rhine_error(struct net_d
 			printk(KERN_INFO "%s: Transmitter underrun, Tx "
 				   "threshold now %2.2x.\n",
 				   dev->name, np->tx_thresh);
-		via_restart_tx(dev);
+		via_rhine_restart_tx(dev);
 	}
 	if (intr_status & ~( IntrLinkChange | IntrStatsMax |
  						 IntrTxError | IntrTxAborted | IntrNormalSummary)) {
@@ -1599,17 +1594,6 @@ static struct net_device_stats *via_rhin
 	return &np->stats;
 }
 
-/* Clears the "tally counters" for CRC errors and missed frames(?).
-   It has been reported that some chips need a write of 0 to clear
-   these, for others the counters are set to 1 when written to and
-   instead cleared when read. So we clear them both ways ... */
-static inline void clear_tally_counters(const long ioaddr)
-{
-	writel(0, ioaddr + RxMissed);
-	readw(ioaddr + RxCRCErrs);
-	readw(ioaddr + RxMissed);
-}
-
 static void via_rhine_set_rx_mode(struct net_device *dev)
 {
 	struct netdev_private *np = dev->priv;

[2/4][via-rhine][PATCH] Fix broken Tx underrun handling

[Roger Luethi]

[-- Attachment #1: Type: text/plain, Size: 179 bytes --]

The Tx underrun handling in the current driver is broken. The details have
been explained in a posting "VIA Rhine 1.15exp1, patch for
testing/discussion" a couple of months ago.


[-- Attachment #2: via-rhine-1.16rc-02_underrun.diff --]
[-- Type: text/plain, Size: 2583 bytes --]

--- 01_trivial/drivers/net/via-rhine.c	Fri Feb 14 18:09:48 2003
+++ 02_underrun/drivers/net/via-rhine.c	Fri Feb 14 19:05:54 2003
@@ -416,9 +416,9 @@ int mmio_verify_registers[] = {
 /* Bits in the interrupt status/mask registers. */
 enum intr_status_bits {
 	IntrRxDone=0x0001, IntrRxErr=0x0004, IntrRxEmpty=0x0020,
-	IntrTxDone=0x0002, IntrTxError=0x0008, IntrTxUnderrun=0x0010,
+	IntrTxDone=0x0002, IntrTxError=0x0008, IntrTxUnderrun=0x0210,
 	IntrPCIErr=0x0040,
-	IntrStatsMax=0x0080, IntrRxEarly=0x0100, IntrMIIChange=0x0200,
+	IntrStatsMax=0x0080, IntrRxEarly=0x0100,
 	IntrRxOverflow=0x0400, IntrRxDropped=0x0800, IntrRxNoBuf=0x1000,
 	IntrTxAborted=0x2000, IntrLinkChange=0x4000,
 	IntrRxWakeUp=0x8000,
@@ -1005,7 +1005,7 @@ static void init_registers(struct net_de
 	writew(IntrRxDone | IntrRxErr | IntrRxEmpty| IntrRxOverflow |
 		   IntrRxDropped | IntrRxNoBuf | IntrTxAborted |
 		   IntrTxDone | IntrTxError | IntrTxUnderrun |
-		   IntrPCIErr | IntrStatsMax | IntrLinkChange | IntrMIIChange,
+		   IntrPCIErr | IntrStatsMax | IntrLinkChange,
 		   ioaddr + IntrEnable);
 
 	np->chip_cmd = CmdStart|CmdTxOn|CmdRxOn|CmdNoTxPoll;
@@ -1310,7 +1310,7 @@ static void via_rhine_interrupt(int irq,
 			via_rhine_tx(dev);
 
 		/* Abnormal error summary/uncommon events handlers. */
-		if (intr_status & (IntrPCIErr | IntrLinkChange | IntrMIIChange |
+		if (intr_status & (IntrPCIErr | IntrLinkChange |
 				   IntrStatsMax | IntrTxError | IntrTxAborted |
 				   IntrTxUnderrun))
 			via_rhine_error(dev, intr_status);
@@ -1534,7 +1534,7 @@ static void via_rhine_error(struct net_d
 
 	spin_lock (&np->lock);
 
-	if (intr_status & (IntrMIIChange | IntrLinkChange)) {
+	if (intr_status & (IntrLinkChange)) {
 		if (readb(ioaddr + MIIStatus) & 0x02) {
 			/* Link failed, restart autonegotiation. */
 			if (np->drv_flags & HasDavicomPhy)
@@ -1552,7 +1552,7 @@ static void via_rhine_error(struct net_d
 		np->stats.rx_missed_errors	+= readw(ioaddr + RxMissed);
 		clear_tally_counters(ioaddr);
 	}
-	if (intr_status & IntrTxError) {
+	if (intr_status & IntrTxAborted) {
 		if (debug > 1)
 			printk(KERN_INFO "%s: Abort %4.4x, frame dropped.\n",
 				   dev->name, intr_status);
@@ -1567,7 +1567,7 @@ static void via_rhine_error(struct net_d
 				   dev->name, np->tx_thresh);
 		via_rhine_restart_tx(dev);
 	}
-	if (intr_status & ~( IntrLinkChange | IntrStatsMax |
+	if (intr_status & ~( IntrLinkChange | IntrStatsMax | IntrTxUnderrun |
  						 IntrTxError | IntrTxAborted | IntrNormalSummary)) {
 		if (debug > 1)
 			printk(KERN_ERR "%s: Something Wicked happened! %4.4x.\n",

[3/4][via-rhine][PATCH] Various duplex related fixes

[Roger Luethi]

[-- Attachment #1: Type: text/plain, Size: 747 bytes --]

Fix the following bugs:
- after a watchdog timeout (still a common thing with the Rhine), the
  driver will fall back to half-duplex, ignoring the auto-negotiation
  results, killing performance completely until the driver is reloaded;
  fixed by clearing full_duplex in init_registers()
- driver considers only 0x200 for full-duplex option; now 0x220
- duplex code used dev->name before it's initialized; code section moved

The full_duplex and force_media related code is quite a mess, but this
minimal fix will make the most annoying problems go away. Looking at some
other code (e.g. mii.c) it seems to me the interaction between user force,
autoneg results and full_duplex/force_media is amazingly complex, so I'll
leave it at this for now.


[-- Attachment #2: via-rhine-1.16rc-03_duplex.diff --]
[-- Type: text/plain, Size: 1960 bytes --]

--- 02_underrun/drivers/net/via-rhine.c	Fri Feb 14 19:05:54 2003
+++ 03_duplex/drivers/net/via-rhine.c	Fri Feb 14 19:07:17 2003
@@ -726,21 +726,6 @@ static int __devinit via_rhine_init_one 
 	if (dev->mem_start)
 		option = dev->mem_start;
 
-	/* The lower four bits are the media type. */
-	if (option > 0) {
-		if (option & 0x200)
-			np->mii_if.full_duplex = 1;
-		np->default_port = option & 15;
-	}
-	if (card_idx < MAX_UNITS  &&  full_duplex[card_idx] > 0)
-		np->mii_if.full_duplex = 1;
-
-	if (np->mii_if.full_duplex) {
-		printk(KERN_INFO "%s: Set to forced full duplex, autonegotiation"
-			   " disabled.\n", dev->name);
-		np->mii_if.force_media = 1;
-	}
-
 	/* The chip-specific entries in the device structure. */
 	dev->open = via_rhine_open;
 	dev->hard_start_xmit = via_rhine_start_tx;
@@ -752,11 +737,27 @@ static int __devinit via_rhine_init_one 
 	dev->watchdog_timeo = TX_TIMEOUT;
 	if (np->drv_flags & ReqTxAlign)
 		dev->features |= NETIF_F_SG|NETIF_F_HW_CSUM;
-	
+
+	/* dev->name not defined before register_netdev()! */
 	i = register_netdev(dev);
 	if (i)
 		goto err_out_unmap;
 
+	/* The lower four bits are the media type. */
+	if (option > 0) {
+		if (option & 0x220)
+			np->mii_if.full_duplex = 1;
+		np->default_port = option & 15;
+	}
+	if (card_idx < MAX_UNITS  &&  full_duplex[card_idx] > 0)
+		np->mii_if.full_duplex = 1;
+
+	if (np->mii_if.full_duplex) {
+		printk(KERN_INFO "%s: Set to forced full duplex, autonegotiation"
+			   " disabled.\n", dev->name);
+		np->mii_if.force_media = 1;
+	}
+
 	printk(KERN_INFO "%s: %s at 0x%lx, ",
 		   dev->name, via_rhine_chip_info[chip_id].name,
 		   (pci_flags & PCI_USES_IO) ? ioaddr : memaddr);
@@ -992,6 +993,7 @@ static void init_registers(struct net_de
 	writeb(0x20, ioaddr + TxConfig);
 	np->tx_thresh = 0x20;
 	np->rx_thresh = 0x60;			/* Written in via_rhine_set_rx_mode(). */
+	np->mii_if.full_duplex = 0;
 
 	if (dev->if_port == 0)
 		dev->if_port = np->default_port;

[4/4][via-rhine][PATCH] Reset function rewrite

[Roger Luethi]

[-- Attachment #1: Type: text/plain, Size: 299 bytes --]

The new reset function only waits up to 0.1 ms for the reset to complete,
instead of 10 ms. However, it introduces the use of a forced reset flag if
a chip doesn't seem cooperative. This fixes cases where until now only
reloading the driver or even rebooting the machine would bring the chip
back.


[-- Attachment #2: via-rhine-1.16rc-04_reset.diff --]
[-- Type: text/plain, Size: 1201 bytes --]

--- 03_duplex/drivers/net/via-rhine.c	Fri Feb 14 19:07:17 2003
+++ 04_reset/drivers/net/via-rhine.c	Fri Feb 14 19:13:04 2003
@@ -523,24 +523,25 @@ static int  via_rhine_close(struct net_d
 static void wait_for_reset(struct net_device *dev, int chip_id, char *name)
 {
 	long ioaddr = dev->base_addr;
-	int i;
 
-	/* VT86C100A may need long delay after reset (dlink) */
-	if (chip_id == VT86C100A)
+	udelay(5);
+
+	if (readw(ioaddr + ChipCmd) & CmdReset) {
+		printk(KERN_INFO "%s: Reset did not complete in 5 us. "
+			"Trying harder.\n", name);
+
+		/* Rhine-II needs to be forced sometimes */
+		if (chip_id == VT6102)
+			writeb(0x40, ioaddr + 0x81);
+
+		/* VT86C100A may need long delay after reset (dlink) */
+		/* Seen on Rhine-II as well (rl) */
 		udelay(100);
+	}
 
-	i = 0;
-	do {
-		udelay(5);
-		i++;
-		if(i > 2000) {
-			printk(KERN_ERR "%s: reset did not complete in 10 ms.\n", name);
-			break;
-		}
-	} while(readw(ioaddr + ChipCmd) & CmdReset);
 	if (debug > 1)
-		printk(KERN_INFO "%s: reset finished after %d microseconds.\n",
-			   name, 5*i);
+		printk(KERN_INFO "%s: Reset %s.\n", name,
+			(readw(ioaddr + ChipCmd) & CmdReset) ? "failed" : "succeeded");
 }
 
 #ifdef USE_MEM

Re: [0/4][via-rhine] Improvements

[Jeff Garzik]

Roger Luethi wrote:
> Here comes a batch of patches for the via-rhine driver. Please apply.
> 
> via-rhine is still hardly usable on the most common Rhine hardware; it
> can't sustain 100Mbps traffic. The changes presented here improve the
> situation considerably; they fix a number of real problems and have been
> tested for regression (alas, by few people).


Looks good, all patches applied to 2.5.

Should these apply to 2.4, too?

Just a general comment, the reset logic seems a bit too much like voodoo 
magic ;-)  It would be nice long-term to get an official answer from Via 
about the proper reset sequence and time limits.  [regardless, like I 
said, patch applied...]

	Jeff

Re: [0/4][via-rhine] Improvements

[Roger Luethi]

[-- Attachment #1: Type: text/plain, Size: 1738 bytes --]

On Sat, 15 Feb 2003 14:08:24 -0500, Jeff Garzik wrote:

> Looks good, all patches applied to 2.5.

Thx. Patch to bump the version number up attached.

> Should these apply to 2.4, too?

Yes. I'm trying to keep the drivers in both trees in sync.

> Just a general comment, the reset logic seems a bit too much like voodoo 
> magic ;-)

Look at the rest of the driver and you will find the reset voodoo magic is
a perfect fit. We'd be waving dead chickens if only tar could handle them.
Actually, the reset logic is slightly better because for every line I have
some reasoning and actual tests conducted. The underlying problem is that
the Rhine is only loosely documented and various revisions differ in
amazing ways from each other and from the documentation that supposedly
describes them.

I've named the magic register in the patch below, does that ease your
voodoo pain?

I've been considering for a while to document the driver somewhat better.
Are documentation patches welcome, or do you just want to have official
word from VIA that they agree with the reset code? And if I need a few
lines to explain some voodoo magic, is the prefered way to put it into the
source or would a freshly created Documentation/network/via-rhine.txt be
(my first choice but the directory typically contains user rather than
developer documentation) the better place?

> It would be nice long-term to get an official answer from Via 
> about the proper reset sequence and time limits.  [regardless, like I 

Heh. If I had a contact at VIA I'd have many and more important questions
than the reset sequence that actually works now, unlike lots of other
stuff. Yes, reliable information from within VIA -- official or not --
would be a big help.

Roger

[-- Attachment #2: via-rhine-1.16rc-05_changelog.diff --]
[-- Type: text/plain, Size: 1298 bytes --]

--- 04_reset/drivers/net/via-rhine.c	Fri Feb 14 19:13:04 2003
+++ 05_changelog/drivers/net/via-rhine.c	Sat Feb 15 21:33:55 2003
@@ -101,11 +101,18 @@
 	LK1.1.15 (jgarzik):
 	- Use new MII lib helper generic_mii_ioctl
 
+	LK1.1.16 (Roger Luethi)
+	- Etherleak fix
+	- Handle Tx buffer underrun
+	- Fix bugs in full duplex handling
+	- New reset code uses "force reset" cmd on Rhine-II
+	- Various clean ups
+
 */
 
 #define DRV_NAME	"via-rhine"
-#define DRV_VERSION	"1.1.15"
-#define DRV_RELDATE	"November-22-2002"
+#define DRV_VERSION	"1.1.16"
+#define DRV_RELDATE	"February-15-2003"
 
 
 /* A few user-configurable values.
@@ -395,7 +402,7 @@ enum register_offsets {
 	MIIPhyAddr=0x6C, MIIStatus=0x6D, PCIBusConfig=0x6E,
 	MIICmd=0x70, MIIRegAddr=0x71, MIIData=0x72, MACRegEEcsr=0x74,
 	ConfigA=0x78, ConfigB=0x79, ConfigC=0x7A, ConfigD=0x7B,
-	RxMissed=0x7C, RxCRCErrs=0x7E,
+	RxMissed=0x7C, RxCRCErrs=0x7E, MiscCmd=0x81,
 	StickyHW=0x83, WOLcrClr=0xA4, WOLcgClr=0xA7, PwrcsrClr=0xAC,
 };
 
@@ -532,7 +539,7 @@ static void wait_for_reset(struct net_de
 
 		/* Rhine-II needs to be forced sometimes */
 		if (chip_id == VT6102)
-			writeb(0x40, ioaddr + 0x81);
+			writeb(0x40, ioaddr + MiscCmd);
 
 		/* VT86C100A may need long delay after reset (dlink) */
 		/* Seen on Rhine-II as well (rl) */

Re: [0/4][via-rhine] Improvements

[Jeff Garzik]

Roger Luethi wrote:
> On Sat, 15 Feb 2003 14:08:24 -0500, Jeff Garzik wrote:
> 
> 
>>Looks good, all patches applied to 2.5.
> 
> 
> Thx. Patch to bump the version number up attached.

applied to 2.4 and 2.5


>>Should these apply to 2.4, too?
> 
> 
> Yes. I'm trying to keep the drivers in both trees in sync.

noted


>>Just a general comment, the reset logic seems a bit too much like voodoo 
>>magic ;-)
> 
> 
> Look at the rest of the driver and you will find the reset voodoo magic is
> a perfect fit. We'd be waving dead chickens if only tar could handle them.
> Actually, the reset logic is slightly better because for every line I have
> some reasoning and actual tests conducted. The underlying problem is that
> the Rhine is only loosely documented and various revisions differ in
> amazing ways from each other and from the documentation that supposedly
> describes them.
> 
> I've named the magic register in the patch below, does that ease your
> voodoo pain?

I applied the patch, but I meant more that wait_for_reset seems 
questionable.  There is generally a PIO or MMIO write preceding 
wait_for_reset function call, and then the function delays.  If the PCI 
write is posted, for example, which at least my own Via EPIA does, then 
you cannot be guaranteed the timing of
	write[bwl]()
	udelay(5)

PCI writes must be flushed, by doing a read[bwl]().

So, obvious PCI posting bugs in the wait_for_reset may be the cause of 
some of the randomness that appears in the field.  (Note this applies to 
all PCI writes in the driver...)

So, I said "voodoo magic" because it doesn't really seem like we know 
what the exact handling is... and randomly placed udelay() calls are, 
unfortunately, sometimes a sign of driver bugs instead of necessary 
hardware delays.


> I've been considering for a while to document the driver somewhat better.
> Are documentation patches welcome, or do you just want to have official
> word from VIA that they agree with the reset code? And if I need a few
> lines to explain some voodoo magic, is the prefered way to put it into the
> source or would a freshly created Documentation/network/via-rhine.txt be
> (my first choice but the directory typically contains user rather than
> developer documentation) the better place?

I would prefer both user and developer docs go in 
Documentation/networking/via-rhine.txt.  It is easy enough to note 
separate sections of the document...


>>It would be nice long-term to get an official answer from Via 
>>about the proper reset sequence and time limits.  [regardless, like I 
> 
> 
> Heh. If I had a contact at VIA I'd have many and more important questions
> than the reset sequence that actually works now, unlike lots of other
> stuff. Yes, reliable information from within VIA -- official or not --
> would be a big help.

Let me wave some dead chickens of my own, and see what happens...

	Jeff

Re: [0/4][via-rhine] Improvements

[Roger Luethi]

On Sat, 15 Feb 2003 16:49:24 -0500, Jeff Garzik wrote:
> I applied the patch, but I meant more that wait_for_reset seems 
> questionable.  There is generally a PIO or MMIO write preceding 
> wait_for_reset function call, and then the function delays.  If the PCI 
> write is posted, for example, which at least my own Via EPIA does, then 
> you cannot be guaranteed the timing of
> 	write[bwl]()
> 	udelay(5)
> 
> PCI writes must be flushed, by doing a read[bwl]().

Thanks for raising that issue. It is my understanding that PIO ops are
synchronous (on IA-32). If that is correct, problems should only occur if
the driver is built with MMIO support, no?

I have been building the driver without MMIO for quite a while to eliminate
one source of problems for now.

> what the exact handling is... and randomly placed udelay() calls are, 
> unfortunately, sometimes a sign of driver bugs instead of necessary 
> hardware delays.

You won't see me rule out driver bugs as a likely explanation for anything
anytime soon ;-).

> I would prefer both user and developer docs go in 
> Documentation/networking/via-rhine.txt.  It is easy enough to note 
> separate sections of the document...

ACK.

Roger

Re: [0/4][via-rhine] Improvements

[Linus Torvalds]

On Sat, 15 Feb 2003, Roger Luethi wrote:
> 
> Thanks for raising that issue. It is my understanding that PIO ops are
> synchronous (on IA-32). If that is correct, problems should only occur if
> the driver is built with MMIO support, no?

No, even PIO ops are asynchronous. They are _more_ synchronous than the
MMIO ones (I think the CPU waits until they hit the bus, and most bridges
just pass them through), but the CPU does not wait for them to hit the
device.

So in practice, this _tends_ to mean that a PIO write will usually hit the 
device within a microsecond or less of being issued by the CPU, and you 
don't need a IO read to force it out. But considering the wide variety of 
PCI bridges out there I bet there are some that will post even PIO writes 
and might hold on to them for some time, especially if other activity like 
DMA keeps the bus busy.

In other words: I suspect the code will work. But it's probably _safer_ to 
do the normal "read to synchronize" unless there are major performance 
issues (which is clearly not the case in this particular instance, but 
might be somewhere else).

		Linus

Re: [0/4][via-rhine] Improvements

[Roger Luethi]

> On Sat, 15 Feb 2003, Roger Luethi wrote:
> > 
> > Thanks for raising that issue. It is my understanding that PIO ops are
> > synchronous (on IA-32). If that is correct, problems should only occur if
> > the driver is built with MMIO support, no?
> 
> No, even PIO ops are asynchronous. They are _more_ synchronous than the
> MMIO ones (I think the CPU waits until they hit the bus, and most bridges

Hmmm... A recent thread on PCI write posting seemed to confirm my view [1].
What am I missing here?

------------------------------ cut here -----------------------------------
From: Alan Cox <alan@lxorguk.ukuu.org.uk>
Subject: Re: Linux 2.4.21-pre3-ac4
Date: 12 Jan 2003 20:40:54 +0000

On Sun, 2003-01-12 at 19:51, Benjamin Herrenschmidt wrote:
> What about PCI write posting ? How can we enforce the 400ns delay here ?

For i/o space it is ok as in*/out* are synchronous. For mmio right now I
don't know. I need to talk to Andre about that for SATA. I guess for the
PPC its going to be fun

[...]
------------------------------ cut here -----------------------------------

> don't need a IO read to force it out. But considering the wide variety of 
> PCI bridges out there I bet there are some that will post even PIO writes 
> and might hold on to them for some time, especially if other activity like 
> DMA keeps the bus busy.

There was some talking about hwif->IOSYNC() (for IDE). That might be
interesting for other devices, too. It could resolve to a nop for
synchronous operations, and say a read* for MMIO. IMHO it shouldn't be up
to a driver maintainer to figure out what sync op some arch the driver may
run on needs. What a maintainer typically _can_ provide is type of
operation (MMIO/PIO) and a register that is considered safe for a sync
read.

Roger

[1] http://marc.theaimsgroup.com/?l=linux-kernel&m=104240180906935&w=4

Re: [0/4][via-rhine] Improvements

[Jeff Garzik]

Roger Luethi wrote:
>>On Sat, 15 Feb 2003, Roger Luethi wrote:
>>
>>>Thanks for raising that issue. It is my understanding that PIO ops are
>>>synchronous (on IA-32). If that is correct, problems should only occur if
>>>the driver is built with MMIO support, no?
>>
>>No, even PIO ops are asynchronous. They are _more_ synchronous than the
>>MMIO ones (I think the CPU waits until they hit the bus, and most bridges
> 
> 
> Hmmm... A recent thread on PCI write posting seemed to confirm my view [1].
> What am I missing here?


Basically, in a wait_for_reset, performance doesn't matter, so just 
flush with a read, and it will work in all cases :)

A PIO write may be posted through multiple levels of PCI bridges, 
perhaps... in any case, even if PIO is completely synchronous in test 
cases, an additional read will not hurt here.

	Jeff

Re: [0/4][via-rhine] Improvements

[Jeff Garzik]

Roger Luethi wrote:
> Here comes a batch of patches for the via-rhine driver. Please apply.
> 
> via-rhine is still hardly usable on the most common Rhine hardware; it
> can't sustain 100Mbps traffic. The changes presented here improve the
> situation considerably; they fix a number of real problems and have been
> tested for regression (alas, by few people).


applied to 2.4, as well.