Re: [patch 0/4] 2.6.5-rc2-mm5 - r8169 queue

Re: [patch 0/4] 2.6.5-rc2-mm5 - r8169 queue

[Andi Kleen]

On Mon, 29 Mar 2004 19:36:04 +0200
Francois Romieu <romieu@fr.zoreil.com> wrote:

> There are a few (hopefully) harmless things in the local queue that I'd
> appreciate to go mainline before the second round of r8169 changes.

Do you have any plans to convert it to use DAC?  

I feel a bit uncomfortable with using the IOMMU on x86-64 for gigabit
ethernet because it would be very easy to overflow it at these bandwidths. 
DAC would be much safer.

-Andi

[patch 0/4] 2.6.5-rc2-mm5 - r8169 queue

[Francois Romieu]

There are a few (hopefully) harmless things in the local queue that I'd
appreciate to go mainline before the second round of r8169 changes.
The patches are diffed against 2.6.5-rc2-mm5 and should apply cleanly to
the -netdev serie.

- r8169-ethtool-introduction.patch
- r8169-missing-static.patch
- r8169-maintainer.patch
- r8169-dma-api-update.patch

Despite its limited performance, #3/4 does not work too bad.

The patch I provided today on l-k includes a small change which could
improve performance. If Daniel Egger is satisfied with it, a patch for
the adequate changes will follow.

--
Ueimor

Re: [patch 1/4] 2.6.5-rc2-mm5 - r8169 queue

[Francois Romieu]

ethtool driver info (Krishnakumar R. <krishnakumar@naturesoft.net>).



 drivers/net/r8169.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+)

diff -puN drivers/net/r8169.c~r8169-ethtool-introduction drivers/net/r8169.c
--- linux-2.6.5-rc2-mm5/drivers/net/r8169.c~r8169-ethtool-introduction	2004-03-29 18:42:10.000000000 +0200
+++ linux-2.6.5-rc2-mm5-fr/drivers/net/r8169.c	2004-03-29 18:42:10.000000000 +0200
@@ -40,6 +40,7 @@ VERSION 1.2	<2002/11/30>
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
 #include <linux/delay.h>
+#include <linux/ethtool.h>
 #include <linux/crc32.h>
 #include <linux/init.h>
 
@@ -382,6 +383,20 @@ mdio_read(void *ioaddr, int RegAddr)
 	return value;
 }
 
+static void rtl8169_get_drvinfo(struct net_device *dev,
+				struct ethtool_drvinfo *info)
+{
+	struct rtl8169_private *tp = dev->priv;
+
+	strcpy(info->driver, RTL8169_DRIVER_NAME);
+	strcpy(info->version, RTL8169_VERSION);
+	strcpy(info->bus_info, pci_name(tp->pci_dev));
+}
+
+static struct ethtool_ops rtl8169_ethtool_ops = {
+	.get_drvinfo		= rtl8169_get_drvinfo,
+};
+
 static void rtl8169_write_gmii_reg_bit(void *ioaddr, int reg, int bitnum,
 				       int bitval)
 {
@@ -793,6 +808,7 @@ rtl8169_init_one(struct pci_dev *pdev, c
 	dev->open = rtl8169_open;
 	dev->hard_start_xmit = rtl8169_start_xmit;
 	dev->get_stats = rtl8169_get_stats;
+	dev->ethtool_ops = &rtl8169_ethtool_ops;
 	dev->stop = rtl8169_close;
 	dev->tx_timeout = rtl8169_tx_timeout;
 	dev->set_multicast_list = rtl8169_set_rx_mode;

_

[patch 2/4] 2.6.5-rc2-mm5 - r8169 queue

[Francois Romieu]

Missing 'static' qualifier for functions.


 drivers/net/r8169.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff -puN drivers/net/r8169.c~r8169-missing-static drivers/net/r8169.c
--- linux-2.6.5-rc2-mm5/drivers/net/r8169.c~r8169-missing-static	2004-03-29 18:42:21.000000000 +0200
+++ linux-2.6.5-rc2-mm5-fr/drivers/net/r8169.c	2004-03-29 18:42:21.000000000 +0200
@@ -346,8 +346,7 @@ static const unsigned int rtl8169_rx_con
 #define PHY_Cap_100_Half_Or_Less PHY_Cap_100_Half | PHY_Cap_10_Full_Or_Less
 #define PHY_Cap_100_Full_Or_Less PHY_Cap_100_Full | PHY_Cap_100_Half_Or_Less
 
-void
-mdio_write(void *ioaddr, int RegAddr, int value)
+static void mdio_write(void *ioaddr, int RegAddr, int value)
 {
 	int i;
 
@@ -364,8 +363,7 @@ mdio_write(void *ioaddr, int RegAddr, in
 	}
 }
 
-int
-mdio_read(void *ioaddr, int RegAddr)
+static int mdio_read(void *ioaddr, int RegAddr)
 {
 	int i, value = -1;
 
@@ -1639,8 +1637,7 @@ rtl8169_set_rx_mode(struct net_device *d
  *
  *  Get TX/RX statistics for rtl8169
  */
-struct net_device_stats *
-rtl8169_get_stats(struct net_device *dev)
+static struct net_device_stats *rtl8169_get_stats(struct net_device *dev)
 {
 	struct rtl8169_private *tp = dev->priv;
 	void *ioaddr = tp->mmio_addr;

_

[patch 3/4] 2.6.5-rc2-mm5 - r8169 queue

[Francois Romieu]

MAINTAINER entry for the r8169 driver.


 MAINTAINERS |    6 ++++++
 1 files changed, 6 insertions(+)

diff -puN MAINTAINERS~r8169-maintainer MAINTAINERS
--- linux-2.6.5-rc2-mm5/MAINTAINERS~r8169-maintainer	2004-03-29 18:58:06.000000000 +0200
+++ linux-2.6.5-rc2-mm5-fr/MAINTAINERS	2004-03-29 18:58:06.000000000 +0200
@@ -113,6 +113,12 @@ M:	jgarzik@pobox.com
 W:	http://sourceforge.net/projects/gkernel/
 S:	Maintained
 
+8169 10/100/1000 GIGABIT ETHERNET DRIVER
+P:	Francois Romieu
+M:	romieu@fr.zoreil.com
+L:	netdev@oss.sgi.com
+S:	Maintained
+
 8250/16?50 (AND CLONE UARTS) SERIAL DRIVER
 P:	Russell King
 M:	rmk+serial@arm.linux.org.uk

_

[patch 4/4] 2.6.5-rc2-mm5 - r8169 queue

[Francois Romieu]

pci_dma_sync_single_for_{cpu/device} sneaked in 2.6.5-rcX. Let's resync.


 drivers/net/r8169.c |   17 ++++++++++-------
 1 files changed, 10 insertions(+), 7 deletions(-)

diff -puN drivers/net/r8169.c~r8169-dma-api-update drivers/net/r8169.c
--- linux-2.6.5-rc2-mm5/drivers/net/r8169.c~r8169-dma-api-update	2004-03-29 19:01:07.000000000 +0200
+++ linux-2.6.5-rc2-mm5-fr/drivers/net/r8169.c	2004-03-29 19:03:19.000000000 +0200
@@ -1443,19 +1443,22 @@ rtl8169_rx_interrupt(struct net_device *
 			struct RxDesc *desc = tp->RxDescArray + entry;
 			struct sk_buff *skb = tp->Rx_skbuff[entry];
 			int pkt_size = (status & 0x00001FFF) - 4;
+			void (*pci_action)(struct pci_dev *, dma_addr_t,
+				size_t, int) = pci_dma_sync_single_for_device;
 
-			pci_dma_sync_single(tp->pci_dev,
-					    le32_to_cpu(desc->buf_addr),
-					    RX_BUF_SIZE, PCI_DMA_FROMDEVICE);
+
+			pci_dma_sync_single_for_cpu(tp->pci_dev,
+				le32_to_cpu(desc->buf_addr), RX_BUF_SIZE,
+				PCI_DMA_FROMDEVICE);
 
 			if (rtl8169_try_rx_copy(&skb, pkt_size, desc, dev)) {
-				pci_unmap_single(tp->pci_dev,
-						 le32_to_cpu(desc->buf_addr),
-						 RX_BUF_SIZE,
-						 PCI_DMA_FROMDEVICE);
+				pci_action = pci_unmap_single;
 				tp->Rx_skbuff[entry] = NULL;
 			}
 
+			pci_action(tp->pci_dev, le32_to_cpu(desc->buf_addr),
+				   RX_BUF_SIZE, PCI_DMA_FROMDEVICE);
+
 			skb_put(skb, pkt_size);
 			skb->protocol = eth_type_trans(skb, dev);
 			netif_rx(skb);

_

Re: [patch 0/4] 2.6.5-rc2-mm5 - r8169 queue

[Francois Romieu]

Jeff Garzik <jgarzik@pobox.com> :
[...]
> I am pondering taking your latest version of r8169, and moving the 
> phy-related code from there to 8139cp.c.  Then just add
> 	MODULE_ALIAS("r8169");
> and deleting r8169.c.
> 
> The rationale is, 8169 is _exactly_ the same as 8139C+, except the phy 
> code and one or two bits in the chip registers.  I would rather have 
> just one codebase, supporting one MAC (i.e. the RX/TX part) and multiple 
> phys.

It seems appealing. Do you have a specific schedule in mind ?

--
Ueimor

Re: [patch 0/4] 2.6.5-rc2-mm5 - r8169 queue

[Francois Romieu]

Andi Kleen <ak@suse.de> :
[...]
> Do you have any plans to convert it to use DAC?  

I should be able to cook up something shortly (i.e. during the week) so that
it is available in -mm or in a separate tree.

For anything else I'll go leveraging the 8139cp functions as
- it will not appear in 2.6.5 anyway so there is plenty of time ahead (TM);
- current -mm tree (+ a possible performance change) looks satisfactory
  enough so that it can be freezed in the meantime.

--
Ueimor

Re: [patch 0/4] 2.6.5-rc2-mm5 - r8169 queue

[Jeff Garzik]

Francois Romieu wrote:
> Jeff Garzik <jgarzik@pobox.com> :
> [...]
> 
>>I am pondering taking your latest version of r8169, and moving the 
>>phy-related code from there to 8139cp.c.  Then just add
>>	MODULE_ALIAS("r8169");
>>and deleting r8169.c.
>>
>>The rationale is, 8169 is _exactly_ the same as 8139C+, except the phy 
>>code and one or two bits in the chip registers.  I would rather have 
>>just one codebase, supporting one MAC (i.e. the RX/TX part) and multiple 
>>phys.
> 
> 
> It seems appealing. Do you have a specific schedule in mind ?


schedules are an anathema to me ;-)

I was thinking about getting your initial patches into 2.6.x upstream so
that r8169 users at least have something usable while the phy code gets
moved over to 8139cp.c.

	Jeff

[patch] 2.6.5-rc2 - r8169 DAC

[Francois Romieu]

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

By popular request, patch for DAC on r8169 is available at:
http://www.fr.zoreil.com/people/francois/misc/20040403-2.6.5-rc2-r8169.c-test.patch

It applies against 2.6.5-rc2 (i.e. anything 2.6.5-rcX I guess).

The incremental patches against the latest serie of r8169 patches posted
on netdev is attached.

A tester on a x86-32 and a tester on something-64 bits would be _really_
welcome.

--
Ueimor

[-- Attachment #2: r8169-dac-00.patch --]
[-- Type: text/plain, Size: 606 bytes --]


DMA_{32/64}BIT_MASK for all.


 include/linux/dma-mapping.h |    3 +++
 1 files changed, 3 insertions(+)

diff -puN include/linux/dma-mapping.h~r8169-dac-00 include/linux/dma-mapping.h
--- linux-2.6.5-rc2/include/linux/dma-mapping.h~r8169-dac-00	2004-04-03 00:29:17.000000000 +0200
+++ linux-2.6.5-rc2-fr/include/linux/dma-mapping.h	2004-04-03 00:29:17.000000000 +0200
@@ -10,6 +10,9 @@ enum dma_data_direction {
 	DMA_NONE = 3,
 };
 
+#define DMA_64BIT_MASK 0xffffffffffffffffULL
+#define DMA_32BIT_MASK 0x00000000ffffffffULL
+
 #include <asm/dma-mapping.h>
 
 /* Backwards compat, remove in 2.7.x */

_

[-- Attachment #3: r8169-dac-10.patch --]
[-- Type: text/plain, Size: 5803 bytes --]


DAC experience:
- Rx and Tx descriptors rings handled on 64 bit;
- enable DAC through use of CPlusCmd register (reserved bits apart, this is
  similar to the 8139cp driver);
- use the higher 32 bits of addresses for the Rx/Tx buffers.


 linux/dma-mapping.h |    0 
 drivers/net/r8169.c |   66 ++++++++++++++++++++++++++++++++++++----------------
 2 files changed, 46 insertions(+), 20 deletions(-)

diff -puN drivers/net/r8169.c~r8169-dac-10 drivers/net/r8169.c
--- linux-2.6.5-rc2/drivers/net/r8169.c~r8169-dac-10	2004-04-03 00:29:35.000000000 +0200
+++ linux-2.6.5-rc2-fr/drivers/net/r8169.c	2004-04-03 00:29:35.000000000 +0200
@@ -152,8 +152,10 @@ static int rx_copybreak = 200;
 enum RTL8169_registers {
 	MAC0 = 0,		/* Ethernet hardware address. */
 	MAR0 = 8,		/* Multicast filter. */
-	TxDescStartAddr = 0x20,
-	TxHDescStartAddr = 0x28,
+	TxDescStartAddrLow = 0x20,
+	TxDescStartAddrHigh = 0x24,
+	TxHDescStartAddrLow = 0x28,
+	TxHDescStartAddrHigh = 0x2c,
 	FLASH = 0x30,
 	ERSR = 0x36,
 	ChipCmd = 0x37,
@@ -178,7 +180,8 @@ enum RTL8169_registers {
 	PHYstatus = 0x6C,
 	RxMaxSize = 0xDA,
 	CPlusCmd = 0xE0,
-	RxDescStartAddr = 0xE4,
+	RxDescAddrLow = 0xE4,
+	RxDescAddrHigh = 0xE8,
 	EarlyTxThres = 0xEC,
 	FuncEvent = 0xF0,
 	FuncEventMask = 0xF4,
@@ -230,7 +233,13 @@ enum RTL8169_register_content {
 
 	/*TxConfigBits */
 	TxInterFrameGapShift = 24,
-	TxDMAShift = 8,		/* DMA burst value (0-7) is shift this many bits */
+	TxDMAShift = 8,	/* DMA burst value (0-7) is shift this many bits */
+
+	/* CPlusCmd p.31 */
+	RxVlan		= (1 << 6),
+	RxChkSum	= (1 << 5),
+	PCIDAC		= (1 << 4),
+	PCIMulRW	= (1 << 3),
 
 	/*rtl8169_PHYstatus */
 	TBI_Enable = 0x80,
@@ -289,15 +298,13 @@ enum _DescStatusBit {
 struct TxDesc {
 	u32 status;
 	u32 vlan_tag;
-	u32 buf_addr;
-	u32 buf_Haddr;
+	u64 addr;
 };
 
 struct RxDesc {
 	u32 status;
 	u32 vlan_tag;
-	u32 buf_addr;
-	u32 buf_Haddr;
+	u64 addr;
 };
 
 struct rtl8169_private {
@@ -320,6 +327,7 @@ struct rtl8169_private {
 	struct sk_buff *Tx_skbuff[NUM_TX_DESC];	/* Index of Transmit data buffer */
 	struct timer_list timer;
 	unsigned long phy_link_down_cnt;
+	u16 cp_cmd;
 };
 
 MODULE_AUTHOR("Realtek");
@@ -716,6 +724,20 @@ rtl8169_init_board(struct pci_dev *pdev,
 		goto err_out_disable;
 	}
 
+	tp->cp_cmd = PCIMulRW | RxChkSum;
+
+	if ((sizeof(dma_addr_t) > 32) &&
+	    !pci_set_dma_mask(pdev, DMA_64BIT_MASK))
+		tp->cp_cmd |= PCIDAC;
+	else {
+		rc = pci_set_dma_mask(pdev, DMA_32BIT_MASK);
+		if (rc < 0) {
+			printk(KERN_ERR PFX "DMA configuration failed.\n");
+			goto err_out_free_res;
+		}
+	}
+
+
 	// enable PCI bus-mastering
 	pci_set_master(pdev);
 
@@ -1102,17 +1124,21 @@ rtl8169_hw_start(struct net_device *dev)
 	RTL_W32(TxConfig,
 		(TX_DMA_BURST << TxDMAShift) | (InterFrameGap <<
 						TxInterFrameGapShift));
-	RTL_W16(CPlusCmd, RTL_R16(CPlusCmd));
+	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");
-		RTL_W16(CPlusCmd, RTL_R16(CPlusCmd) | (1 << 14) | (1 << 3));
+		tp->cp_cmd |= (1 << 14) | PCIMulRW;
+		RTL_W16(CPlusCmd, tp->cp_cmd);
 	}
 
 	tp->cur_rx = 0;
 
-	RTL_W32(TxDescStartAddr, tp->TxPhyAddr);
-	RTL_W32(RxDescStartAddr, tp->RxPhyAddr);
+	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);
 
@@ -1132,14 +1158,14 @@ rtl8169_hw_start(struct net_device *dev)
 
 static inline void rtl8169_make_unusable_by_asic(struct RxDesc *desc)
 {
-	desc->buf_addr = 0xdeadbeef;
+	desc->addr = 0x0badbadbadbadbad;
 	desc->status &= ~cpu_to_le32(OWNbit | RsvdMask);
 }
 
 static void rtl8169_free_rx_skb(struct pci_dev *pdev, struct sk_buff **sk_buff,
 				struct RxDesc *desc)
 {
-	pci_unmap_single(pdev, le32_to_cpu(desc->buf_addr), RX_BUF_SIZE,
+	pci_unmap_single(pdev, le64_to_cpu(desc->addr), RX_BUF_SIZE,
 			 PCI_DMA_FROMDEVICE);
 	dev_kfree_skb(*sk_buff);
 	*sk_buff = NULL;
@@ -1153,7 +1179,7 @@ static inline void rtl8169_return_to_asi
 
 static inline void rtl8169_give_to_asic(struct RxDesc *desc, dma_addr_t mapping)
 {
-	desc->buf_addr = cpu_to_le32(mapping);
+	desc->addr = cpu_to_le64(mapping);
 	desc->status |= cpu_to_le32(OWNbit + RX_BUF_SIZE);
 }
 
@@ -1251,9 +1277,9 @@ static void rtl8169_unmap_tx_skb(struct 
 {
 	u32 len = sk_buff[0]->len;
 
-	pci_unmap_single(pdev, le32_to_cpu(desc->buf_addr),
+	pci_unmap_single(pdev, le64_to_cpu(desc->addr),
 			 len < ETH_ZLEN ? ETH_ZLEN : len, PCI_DMA_TODEVICE);
-	desc->buf_addr = 0x00;
+	desc->addr = 0x00;
 	*sk_buff = NULL;
 }
 
@@ -1325,7 +1351,7 @@ rtl8169_start_xmit(struct sk_buff *skb, 
 					 PCI_DMA_TODEVICE);
 
 		tp->Tx_skbuff[entry] = skb;
-		tp->TxDescArray[entry].buf_addr = cpu_to_le32(mapping);
+		tp->TxDescArray[entry].addr = cpu_to_le64(mapping);
 
 		tp->TxDescArray[entry].status = cpu_to_le32(OWNbit | FSbit |
 			LSbit | len | (EORbit * !((entry + 1) % NUM_TX_DESC)));
@@ -1452,7 +1478,7 @@ rtl8169_rx_interrupt(struct net_device *
 
 
 			pci_dma_sync_single_for_cpu(tp->pci_dev,
-				le32_to_cpu(desc->buf_addr), RX_BUF_SIZE,
+				le64_to_cpu(desc->addr), RX_BUF_SIZE,
 				PCI_DMA_FROMDEVICE);
 
 			if (rtl8169_try_rx_copy(&skb, pkt_size, desc, dev)) {
@@ -1460,7 +1486,7 @@ rtl8169_rx_interrupt(struct net_device *
 				tp->Rx_skbuff[entry] = NULL;
 			}
 
-			pci_action(tp->pci_dev, le32_to_cpu(desc->buf_addr),
+			pci_action(tp->pci_dev, le64_to_cpu(desc->addr),
 				   RX_BUF_SIZE, PCI_DMA_FROMDEVICE);
 
 			skb_put(skb, pkt_size);
diff -puN include/linux/dma-mapping.h~r8169-dac-10 include/linux/dma-mapping.h

_

Re: [patch] 2.6.5-rc2 - r8169 DAC

[Srihari Vijayaraghavan]

Hello Francois,

On Saturday 03 April 2004 09:00, Francois Romieu wrote:
> By popular request, patch for DAC on r8169 is available at:
> http://www.fr.zoreil.com/people/francois/misc/20040403-2.6.5-rc2-r8169.c-te
>st.patch
>
> It applies against 2.6.5-rc2 (i.e. anything 2.6.5-rcX I guess).
Yes it does apply cleanly on both 2.6.5-rc2 and 2.6.5-rc3.

> A tester on a x86-32 and a tester on something-64 bits would be _really_
> welcome.

I can test your patch on x86-64, but it does not compile. Here is the error 
message (same error message on both 2.6.5-rc2 and 2.6.5-rc3):
  CC [M]  drivers/net/r8169.o
drivers/net/r8169.c: In function `rtl8169_init_board':
drivers/net/r8169.c:730: error: `DMA_64BIT_MASK' undeclared (first use in this 
function)
drivers/net/r8169.c:730: error: (Each undeclared identifier is reported only 
once
drivers/net/r8169.c:730: error: for each function it appears in.)
drivers/net/r8169.c:733: error: `DMA_32BIT_MASK' undeclared (first use in this 
function)
drivers/net/r8169.c: In function `rtl8169_hw_start':
drivers/net/r8169.c:1138: error: `DMA_32BIT_MASK' undeclared (first use in 
this function)
make[2]: *** [drivers/net/r8169.o] Error 1
make[1]: *** [drivers/net] Error 2
make: *** [drivers] Error 2
  CC      net/core/dev.o
make: *** Waiting for unfinished jobs....

Thanks
Hari

Re: [patch] 2.6.5-rc2 - r8169 DAC

[Francois Romieu]

Srihari Vijayaraghavan <harisri@bigpond.com> :
[compile error message]


DMA_{32/64}BIT_MASK for all.


 include/linux/dma-mapping.h |    3 +++
 1 files changed, 3 insertions(+)

diff -puN include/linux/dma-mapping.h~r8169-dac-00 include/linux/dma-mapping.h
--- linux-2.6.5-rc2/include/linux/dma-mapping.h~r8169-dac-00	2004-04-03 00:29:17.000000000 +0200
+++ linux-2.6.5-rc2-fr/include/linux/dma-mapping.h	2004-04-03 00:29:17.000000000 +0200
@@ -10,6 +10,9 @@ enum dma_data_direction {
 	DMA_NONE = 3,
 };
 
+#define DMA_64BIT_MASK 0xffffffffffffffffULL
+#define DMA_32BIT_MASK 0x00000000ffffffffULL
+
 #include <asm/dma-mapping.h>
 
 /* Backwards compat, remove in 2.7.x */

_