[PATCH 2.6.9-rc2 1/1] r8169: default on disabling PCIDAC

[PATCH 2.6.9-rc2 1/1] r8169: default on disabling PCIDAC

[Francois Romieu]

Default to disabling PCI DAC as this option appears unsafe on amd64
(original suggestion by Hans-Frieder Vogt <hfvogt@arcor.de>).

The driver will typically report PCI System error when something goes
wrong. The relevant interrupt is not masked any more and the driver
can thus be disabled.

Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>

diff -puN linux-2.6.9-rc2.orig/drivers/net/r8169.c linux-2.6.9-rc2/drivers/net/r8169.c
--- linux-2.6.9-rc2.orig/drivers/net/r8169.c	2004-09-19 00:17:04.000000000 +0200
+++ linux-2.6.9-rc2/drivers/net/r8169.c	2004-09-20 00:16:02.000000000 +0200
@@ -156,6 +156,7 @@ static struct pci_device_id rtl8169_pci_
 MODULE_DEVICE_TABLE(pci, rtl8169_pci_tbl);
 
 static int rx_copybreak = 200;
+static int use_dac;
 
 enum RTL8169_registers {
 	MAC0 = 0,		/* Ethernet hardware address. */
@@ -358,6 +359,8 @@ MODULE_AUTHOR("Realtek");
 MODULE_DESCRIPTION("RealTek RTL-8169 Gigabit Ethernet driver");
 MODULE_PARM(media, "1-" __MODULE_STRING(MAX_UNITS) "i");
 MODULE_PARM(rx_copybreak, "i");
+MODULE_PARM(use_dac, "i");
+MODULE_PARM_DESC(use_dac, "Enable PCI DAC. Unsafe on 32 bit PCI slot.");
 MODULE_LICENSE("GPL");
 
 static int rtl8169_open(struct net_device *dev);
@@ -375,7 +378,7 @@ static int rtl8169_poll(struct net_devic
 #endif
 
 static const u16 rtl8169_intr_mask =
-	LinkChg | RxOverflow | RxFIFOOver | TxErr | TxOK | RxErr | RxOK;
+	SYSErr | LinkChg | RxOverflow | RxFIFOOver | TxErr | TxOK | RxErr | RxOK;
 static const u16 rtl8169_napi_event =
 	RxOK | RxOverflow | RxFIFOOver | TxOK | TxErr;
 static const unsigned int rtl8169_rx_config =
@@ -984,7 +987,7 @@ rtl8169_init_board(struct pci_dev *pdev,
 	tp->cp_cmd = PCIMulRW | RxChkSum;
 
 	if ((sizeof(dma_addr_t) > 4) &&
-	    !pci_set_dma_mask(pdev, DMA_64BIT_MASK))
+	    !pci_set_dma_mask(pdev, DMA_64BIT_MASK) && use_dac)
 		tp->cp_cmd |= PCIDAC;
 	else {
 		rc = pci_set_dma_mask(pdev, DMA_32BIT_MASK);
@@ -1761,6 +1764,15 @@ rtl8169_interrupt(int irq, void *dev_ins
 		if (!(status & rtl8169_intr_mask))
 			break;
 
+		if (unlikely(status & SYSErr)) {
+			printk(KERN_ERR PFX "%s: PCI error (status: 0x%04x)."
+			       " Device disabled.\n", dev->name, status);
+			RTL_W8(ChipCmd, 0x00);
+			RTL_W16(IntrMask, 0x0000);
+			RTL_R16(IntrMask);
+			break;
+		}
+
 		if (status & LinkChg)
 			rtl8169_check_link_status(dev, tp, ioaddr);
 

Re: [PATCH 2.6.9-rc2 1/1] r8169: default on disabling PCIDAC

[Jeff Garzik]

Francois Romieu wrote:
> Default to disabling PCI DAC as this option appears unsafe on amd64
> (original suggestion by Hans-Frieder Vogt <hfvogt@arcor.de>).
> 
> The driver will typically report PCI System error when something goes
> wrong. The relevant interrupt is not masked any more and the driver
> can thus be disabled.


Can this interrupt be used at runtime to detect that DAC isn't working?

	Jeff

Re: [PATCH 2.6.9-rc2 1/1] r8169: default on disabling PCIDAC

[Francois Romieu]

Jeff Garzik <jgarzik@pobox.com> :
[...]
> Can this interrupt be used at runtime to detect that DAC isn't working?

Experiment suggests that. 

Errr... Do you really want to hot-reinit the device ?

--
Ueimor

Re: [PATCH 2.6.9-rc2 1/1] r8169: default on disabling PCIDAC

[Jeff Garzik]

Francois Romieu wrote:
> Jeff Garzik <jgarzik@pobox.com> :
> [...]
> 
>>Can this interrupt be used at runtime to detect that DAC isn't working?
> 
> 
> Experiment suggests that. 
> 
> Errr... Do you really want to hot-reinit the device ?


It's only a one-time operation that occurs on weird 64-bit boxes :)

I applied the using_dac stuff, but do think there is _some_ better way.

	Jeff

Re: [PATCH 2.6.9-rc2 1/1] r8169: default on disabling PCIDAC

[Francois Romieu]

Jeff Garzik <jgarzik@pobox.com> :
[...]
> It's only a one-time operation that occurs on weird 64-bit boxes :)
> 
> I applied the using_dac stuff, but do think there is _some_ better way.

Chainsaw coded patch below. It seems to be able to recover on my 32 bit
system (the 'dtc' thing is not really needed). On real 64 bit host,
people will have to disable NETIF_F_HIGHDMA, set an adequate dma mask
and probably reallocate Tx and Rx rings (from irq context: joy and
happyness). May be it would make sense to prevent allocating the rings
on high memory from the start.

Btw, the adapter apparently quiesce if the following sequence is not
used:
+                       tp->cp_cmd &= ~PCIDAC;
+                       RTL_W16(CPlusCmd, tp->cp_cmd);
+                       RTL_W8(ChipCmd, CmdReset | CmdTxEnb | CmdRxEnb);


diff -puN drivers/net/r8169.c~r8169-145b-6 drivers/net/r8169.c
--- linux-2.6.9-rc2/drivers/net/r8169.c~r8169-145b-6	2004-09-21 01:00:17.000000000 +0200
+++ linux-2.6.9-rc2-fr/drivers/net/r8169.c	2004-09-21 01:04:04.000000000 +0200
@@ -1483,6 +1483,11 @@ rtl8169_hw_start(struct net_device *dev)
 	void *ioaddr = tp->mmio_addr;
 	u32 i;
 
+	RTL_W32(TxDescStartAddrLow, ((u64) tp->TxPhyAddr & DMA_32BIT_MASK));
+	RTL_W32(TxDescStartAddrHigh, ((u64) tp->TxPhyAddr >> 32));
+	RTL_W32(RxDescAddrLow, ((u64) tp->RxPhyAddr & DMA_32BIT_MASK));
+	RTL_W32(RxDescAddrHigh, ((u64) tp->RxPhyAddr >> 32));
+
 	/* Soft reset the chip. */
 	RTL_W8(ChipCmd, CmdReset);
 
@@ -1494,7 +1499,6 @@ rtl8169_hw_start(struct net_device *dev)
 	}
 
 	RTL_W8(Cfg9346, Cfg9346_Unlock);
-	RTL_W8(ChipCmd, CmdTxEnb | CmdRxEnb);
 	RTL_W8(EarlyTxThres, EarlyTxThld);
 
 	// For gigabit rtl8169
@@ -1509,24 +1513,9 @@ rtl8169_hw_start(struct net_device *dev)
 	RTL_W32(TxConfig,
 		(TX_DMA_BURST << TxDMAShift) | (InterFrameGap <<
 						TxInterFrameGapShift));
-	tp->cp_cmd |= RTL_R16(CPlusCmd);
-	RTL_W16(CPlusCmd, tp->cp_cmd);
-
-	if (tp->mac_version == RTL_GIGA_MAC_VER_D) {
-		dprintk(KERN_INFO PFX "Set MAC Reg C+CR Offset 0xE0. "
-			"Bit-3 and bit-14 MUST be 1\n");
-		tp->cp_cmd |= (1 << 14) | PCIMulRW;
-		RTL_W16(CPlusCmd, tp->cp_cmd);
-	}
-
 	tp->cur_rx = 0;
 
-	RTL_W32(TxDescStartAddrLow, ((u64) tp->TxPhyAddr & DMA_32BIT_MASK));
-	RTL_W32(TxDescStartAddrHigh, ((u64) tp->TxPhyAddr >> 32));
-	RTL_W32(RxDescAddrLow, ((u64) tp->RxPhyAddr & DMA_32BIT_MASK));
-	RTL_W32(RxDescAddrHigh, ((u64) tp->RxPhyAddr >> 32));
 	RTL_W8(Cfg9346, Cfg9346_Lock);
-	udelay(10);
 
 	RTL_W32(RxMissed, 0);
 
@@ -1538,6 +1527,17 @@ rtl8169_hw_start(struct net_device *dev)
 	/* Enable all known interrupts by setting the interrupt mask. */
 	RTL_W16(IntrMask, rtl8169_intr_mask);
 
+	tp->cp_cmd |= RTL_R16(CPlusCmd);
+
+	if (tp->mac_version == RTL_GIGA_MAC_VER_D) {
+		dprintk(KERN_INFO PFX "Set MAC Reg C+CR Offset 0xE0. "
+			"Bit-3 and bit-14 MUST be 1\n");
+		tp->cp_cmd |= (1 << 14) | PCIMulRW;
+	}
+
+	RTL_W16(CPlusCmd, tp->cp_cmd);
+	RTL_W8(ChipCmd, CmdTxEnb | CmdRxEnb);
+
 	netif_start_queue(dev);
 }
 
@@ -2056,11 +2056,31 @@ rtl8169_interrupt(int irq, void *dev_ins
 			break;
 
 		if (unlikely(status & SYSErr)) {
-			printk(KERN_ERR PFX "%s: PCI error (status: 0x%04x)."
-			       " Device disabled.\n", dev->name, status);
-			RTL_W8(ChipCmd, 0x00);
-			RTL_W16(IntrMask, 0x0000);
-			RTL_R16(IntrMask);
+			struct pci_dev *pdev = tp->pci_dev;
+			static int dtc = 0;
+			u16 pci_status, pci_cmd;
+
+			pci_read_config_word(pdev, PCI_COMMAND, &pci_cmd);
+			pci_write_config_word(pdev, PCI_COMMAND, 0x0157);
+			pci_read_config_word(pdev, PCI_STATUS, &pci_status);
+			pci_write_config_word(pdev, PCI_STATUS,
+					      pci_status & 0xf800);
+			printk(KERN_ERR PFX
+			       "%s: PCI error (cmd:status=0x%04x:%04x)\n",
+			       dev->name, pci_cmd, pci_status);
+
+			tp->cp_cmd &= ~PCIDAC;
+			RTL_W16(CPlusCmd, tp->cp_cmd);
+			RTL_W8(ChipCmd, CmdReset | CmdTxEnb | CmdRxEnb);
+
+			if (dtc++ > 128) {
+				printk(KERN_ERR PFX
+				       "%s: disabled (status: 0x%04x)\n",
+				       dev->name, status);
+				RTL_W8(ChipCmd, 0x00);
+				RTL_W16(IntrMask, 0x0000);
+				RTL_R16(IntrMask);
+			}
 			break;
 		}
 

_

[PATCH] r8169 tx_timeout recovery and automatic DAC step-down

[Francois Romieu]

Untested patches of the day (it's late and I need to sleep):

http://www.fr.zoreil.com/linux/kernel/2.6.x/2.6.9-rc2-mm2/r8169-200.patch
http://www.fr.zoreil.com/linux/kernel/2.6.x/2.6.9-rc2-mm2/r8169-210.patch

The first one should perform a clean reset of the device under TX_TIMEOUT.
The second one should be able to detect a broken setup wrt pci dac
transaction and downgrade the advertised features of the driver accordingly.

The quality of my untested patches past 00 AM is not guaranteed.

--
Ueimor

[PATCH] r8169 tx_timeout recovery and automatic DAC step-down

[Francois Romieu]

Francois Romieu <romieu@fr.zoreil.com> :
[...]

Errr... Oops. :o*

Ok, try #2. Now I can recover from the infamous PCI DAC error on my 32
bit box. Any taker on amd64 ?

http://www.fr.zoreil.com/linux/kernel/2.6.x/2.6.9-rc2-bk15/r8169-200.patch
http://www.fr.zoreil.com/linux/kernel/2.6.x/2.6.9-rc2-bk15/r8169-210.patch

These patches apply against 2.6.9-rc2-mm2 or -netdev. If people want to use
a vanilla post 2.6.9-rc2-bk15, the patches below must be applied first:

http://www.fr.zoreil.com/linux/kernel/2.6.x/2.6.9-rc2-bk15/r8169-2.6.9-rc2-bk15-dac-revert.patch
http://www.fr.zoreil.com/linux/kernel/2.6.x/2.6.9-rc2-bk15/r8169-rc2-mm2.patch

--
Ueimor