* [PATCH 1/5] r8169: more magic during initialization of the hardware
2006-12-03 23:52 [PATCH 0/5] Pull request for 'r8169-upstream-20061204-00' tag Francois Romieu
@ 2006-12-03 23:54 ` Francois Romieu
2006-12-06 5:28 ` Philip Craig
2006-12-03 23:58 ` [PATCH 2/5] r8169: tweak the PCI data parity error recovery Francois Romieu
` (3 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: Francois Romieu @ 2006-12-03 23:54 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev, Darren Salt
Mostly taken from Realtek's driver.
It's a bit yucky but the original is even worse.
Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
Signed-off-by: Darren Salt <linux@youmustbejoking.demon.co.uk>
---
drivers/net/r8169.c | 58 +++++++++++++++++++++++++++++++++++++++------------
1 files changed, 44 insertions(+), 14 deletions(-)
diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index 45d3ca4..c8fa9b1 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -1815,12 +1815,25 @@ static void rtl8169_hw_reset(void __iome
RTL_R8(ChipCmd);
}
-static void
-rtl8169_hw_start(struct net_device *dev)
+static void rtl8169_set_rx_tx_config_registers(struct rtl8169_private *tp)
+{
+ void __iomem *ioaddr = tp->mmio_addr;
+ u32 cfg = rtl8169_rx_config;
+
+ cfg |= (RTL_R32(RxConfig) & rtl_chip_info[tp->chipset].RxConfigMask);
+ RTL_W32(RxConfig, cfg);
+
+ /* Set DMA burst size and Interframe Gap Time */
+ RTL_W32(TxConfig, (TX_DMA_BURST << TxDMAShift) |
+ (InterFrameGap << TxInterFrameGapShift));
+}
+
+static void rtl8169_hw_start(struct net_device *dev)
{
struct rtl8169_private *tp = netdev_priv(dev);
void __iomem *ioaddr = tp->mmio_addr;
struct pci_dev *pdev = tp->pci_dev;
+ u16 cmd;
u32 i;
/* Soft reset the chip. */
@@ -1833,6 +1846,11 @@ rtl8169_hw_start(struct net_device *dev)
msleep_interruptible(1);
}
+ if (tp->mac_version == RTL_GIGA_MAC_VER_05) {
+ RTL_W16(CPlusCmd, RTL_R16(CPlusCmd) | PCIMulRW);
+ pci_write_config_byte(pdev, PCI_CACHE_LINE_SIZE, 0x08);
+ }
+
if (tp->mac_version == RTL_GIGA_MAC_VER_13) {
pci_write_config_word(pdev, 0x68, 0x00);
pci_write_config_word(pdev, 0x69, 0x08);
@@ -1840,8 +1858,6 @@ rtl8169_hw_start(struct net_device *dev)
/* Undocumented stuff. */
if (tp->mac_version == RTL_GIGA_MAC_VER_05) {
- u16 cmd;
-
/* Realtek's r1000_n.c driver uses '&& 0x01' here. Well... */
if ((RTL_R8(Config2) & 0x07) & 0x01)
RTL_W32(0x7c, 0x0007ffff);
@@ -1853,23 +1869,29 @@ rtl8169_hw_start(struct net_device *dev)
pci_write_config_word(pdev, PCI_COMMAND, cmd);
}
-
RTL_W8(Cfg9346, Cfg9346_Unlock);
+ if ((tp->mac_version == RTL_GIGA_MAC_VER_01) ||
+ (tp->mac_version == RTL_GIGA_MAC_VER_02) ||
+ (tp->mac_version == RTL_GIGA_MAC_VER_03) ||
+ (tp->mac_version == RTL_GIGA_MAC_VER_04))
+ RTL_W8(ChipCmd, CmdTxEnb | CmdRxEnb);
+
RTL_W8(EarlyTxThres, EarlyTxThld);
/* Low hurts. Let's disable the filtering. */
RTL_W16(RxMaxSize, 16383);
- /* Set Rx Config register */
- i = rtl8169_rx_config |
- (RTL_R32(RxConfig) & rtl_chip_info[tp->chipset].RxConfigMask);
- RTL_W32(RxConfig, i);
+ if ((tp->mac_version == RTL_GIGA_MAC_VER_01) ||
+ (tp->mac_version == RTL_GIGA_MAC_VER_02) ||
+ (tp->mac_version == RTL_GIGA_MAC_VER_03) ||
+ (tp->mac_version == RTL_GIGA_MAC_VER_04))
+ RTL_W8(ChipCmd, CmdTxEnb | CmdRxEnb);
+ rtl8169_set_rx_tx_config_registers(tp);
- /* Set DMA burst size and Interframe Gap Time */
- RTL_W32(TxConfig, (TX_DMA_BURST << TxDMAShift) |
- (InterFrameGap << TxInterFrameGapShift));
+ cmd = RTL_R16(CPlusCmd);
+ RTL_W16(CPlusCmd, cmd);
- tp->cp_cmd |= RTL_R16(CPlusCmd) | PCIMulRW;
+ tp->cp_cmd |= cmd | PCIMulRW;
if ((tp->mac_version == RTL_GIGA_MAC_VER_02) ||
(tp->mac_version == RTL_GIGA_MAC_VER_03)) {
@@ -1895,7 +1917,15 @@ rtl8169_hw_start(struct net_device *dev)
RTL_W32(TxDescStartAddrLow, ((u64) tp->TxPhyAddr & DMA_32BIT_MASK));
RTL_W32(RxDescAddrHigh, ((u64) tp->RxPhyAddr >> 32));
RTL_W32(RxDescAddrLow, ((u64) tp->RxPhyAddr & DMA_32BIT_MASK));
- RTL_W8(ChipCmd, CmdTxEnb | CmdRxEnb);
+
+ if ((tp->mac_version != RTL_GIGA_MAC_VER_01) &&
+ (tp->mac_version != RTL_GIGA_MAC_VER_02) &&
+ (tp->mac_version != RTL_GIGA_MAC_VER_03) &&
+ (tp->mac_version != RTL_GIGA_MAC_VER_04)) {
+ RTL_W8(ChipCmd, CmdTxEnb | CmdRxEnb);
+ rtl8169_set_rx_tx_config_registers(tp);
+ }
+
RTL_W8(Cfg9346, Cfg9346_Lock);
/* Initially a 10 us delay. Turned it into a PCI commit. - FR */
--
1.4.2.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 1/5] r8169: more magic during initialization of the hardware
2006-12-03 23:54 ` [PATCH 1/5] r8169: more magic during initialization of the hardware Francois Romieu
@ 2006-12-06 5:28 ` Philip Craig
0 siblings, 0 replies; 17+ messages in thread
From: Philip Craig @ 2006-12-06 5:28 UTC (permalink / raw)
To: Francois Romieu; +Cc: Jeff Garzik, netdev, Darren Salt
Francois Romieu wrote:
> Mostly taken from Realtek's driver.
>
> It's a bit yucky but the original is even worse.
This patch fixes a performance regression for my 8169s.
But it appears to have a typo, see below.
>
> Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
> Signed-off-by: Darren Salt <linux@youmustbejoking.demon.co.uk>
> ---
> drivers/net/r8169.c | 58 +++++++++++++++++++++++++++++++++++++++------------
> 1 files changed, 44 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
> index 45d3ca4..c8fa9b1 100644
> --- a/drivers/net/r8169.c
> +++ b/drivers/net/r8169.c
> @@ -1815,12 +1815,25 @@ static void rtl8169_hw_reset(void __iome
> RTL_R8(ChipCmd);
> }
>
> -static void
> -rtl8169_hw_start(struct net_device *dev)
> +static void rtl8169_set_rx_tx_config_registers(struct rtl8169_private *tp)
> +{
> + void __iomem *ioaddr = tp->mmio_addr;
> + u32 cfg = rtl8169_rx_config;
> +
> + cfg |= (RTL_R32(RxConfig) & rtl_chip_info[tp->chipset].RxConfigMask);
> + RTL_W32(RxConfig, cfg);
> +
> + /* Set DMA burst size and Interframe Gap Time */
> + RTL_W32(TxConfig, (TX_DMA_BURST << TxDMAShift) |
> + (InterFrameGap << TxInterFrameGapShift));
> +}
> +
> +static void rtl8169_hw_start(struct net_device *dev)
> {
> struct rtl8169_private *tp = netdev_priv(dev);
> void __iomem *ioaddr = tp->mmio_addr;
> struct pci_dev *pdev = tp->pci_dev;
> + u16 cmd;
> u32 i;
>
> /* Soft reset the chip. */
> @@ -1833,6 +1846,11 @@ rtl8169_hw_start(struct net_device *dev)
> msleep_interruptible(1);
> }
>
> + if (tp->mac_version == RTL_GIGA_MAC_VER_05) {
> + RTL_W16(CPlusCmd, RTL_R16(CPlusCmd) | PCIMulRW);
> + pci_write_config_byte(pdev, PCI_CACHE_LINE_SIZE, 0x08);
> + }
> +
> if (tp->mac_version == RTL_GIGA_MAC_VER_13) {
> pci_write_config_word(pdev, 0x68, 0x00);
> pci_write_config_word(pdev, 0x69, 0x08);
> @@ -1840,8 +1858,6 @@ rtl8169_hw_start(struct net_device *dev)
>
> /* Undocumented stuff. */
> if (tp->mac_version == RTL_GIGA_MAC_VER_05) {
> - u16 cmd;
> -
> /* Realtek's r1000_n.c driver uses '&& 0x01' here. Well... */
> if ((RTL_R8(Config2) & 0x07) & 0x01)
> RTL_W32(0x7c, 0x0007ffff);
> @@ -1853,23 +1869,29 @@ rtl8169_hw_start(struct net_device *dev)
> pci_write_config_word(pdev, PCI_COMMAND, cmd);
> }
>
> -
> RTL_W8(Cfg9346, Cfg9346_Unlock);
> + if ((tp->mac_version == RTL_GIGA_MAC_VER_01) ||
> + (tp->mac_version == RTL_GIGA_MAC_VER_02) ||
> + (tp->mac_version == RTL_GIGA_MAC_VER_03) ||
> + (tp->mac_version == RTL_GIGA_MAC_VER_04))
> + RTL_W8(ChipCmd, CmdTxEnb | CmdRxEnb);
> +
> RTL_W8(EarlyTxThres, EarlyTxThld);
>
> /* Low hurts. Let's disable the filtering. */
> RTL_W16(RxMaxSize, 16383);
>
> - /* Set Rx Config register */
> - i = rtl8169_rx_config |
> - (RTL_R32(RxConfig) & rtl_chip_info[tp->chipset].RxConfigMask);
> - RTL_W32(RxConfig, i);
> + if ((tp->mac_version == RTL_GIGA_MAC_VER_01) ||
> + (tp->mac_version == RTL_GIGA_MAC_VER_02) ||
> + (tp->mac_version == RTL_GIGA_MAC_VER_03) ||
> + (tp->mac_version == RTL_GIGA_MAC_VER_04))
> + RTL_W8(ChipCmd, CmdTxEnb | CmdRxEnb);
> + rtl8169_set_rx_tx_config_registers(tp);
Should this RTL_W8() be deleted?
Otherwise there is an indentation/braces mismatch.
>
> - /* Set DMA burst size and Interframe Gap Time */
> - RTL_W32(TxConfig, (TX_DMA_BURST << TxDMAShift) |
> - (InterFrameGap << TxInterFrameGapShift));
> + cmd = RTL_R16(CPlusCmd);
> + RTL_W16(CPlusCmd, cmd);
>
> - tp->cp_cmd |= RTL_R16(CPlusCmd) | PCIMulRW;
> + tp->cp_cmd |= cmd | PCIMulRW;
>
> if ((tp->mac_version == RTL_GIGA_MAC_VER_02) ||
> (tp->mac_version == RTL_GIGA_MAC_VER_03)) {
> @@ -1895,7 +1917,15 @@ rtl8169_hw_start(struct net_device *dev)
> RTL_W32(TxDescStartAddrLow, ((u64) tp->TxPhyAddr & DMA_32BIT_MASK));
> RTL_W32(RxDescAddrHigh, ((u64) tp->RxPhyAddr >> 32));
> RTL_W32(RxDescAddrLow, ((u64) tp->RxPhyAddr & DMA_32BIT_MASK));
> - RTL_W8(ChipCmd, CmdTxEnb | CmdRxEnb);
> +
> + if ((tp->mac_version != RTL_GIGA_MAC_VER_01) &&
> + (tp->mac_version != RTL_GIGA_MAC_VER_02) &&
> + (tp->mac_version != RTL_GIGA_MAC_VER_03) &&
> + (tp->mac_version != RTL_GIGA_MAC_VER_04)) {
> + RTL_W8(ChipCmd, CmdTxEnb | CmdRxEnb);
> + rtl8169_set_rx_tx_config_registers(tp);
> + }
> +
> RTL_W8(Cfg9346, Cfg9346_Lock);
>
> /* Initially a 10 us delay. Turned it into a PCI commit. - FR */
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/5] r8169: tweak the PCI data parity error recovery
2006-12-03 23:52 [PATCH 0/5] Pull request for 'r8169-upstream-20061204-00' tag Francois Romieu
2006-12-03 23:54 ` [PATCH 1/5] r8169: more magic during initialization of the hardware Francois Romieu
@ 2006-12-03 23:58 ` Francois Romieu
2006-12-03 23:59 ` [PATCH 3/5] r8169: phy program update Francois Romieu
2006-12-04 0:03 ` [PATCH 4/5] r8169: more alignment for the 0x8168 Francois Romieu
` (2 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: Francois Romieu @ 2006-12-03 23:58 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev, Martin Michlmayr, Lennert Buytenhek, Riku Voipio
The 8110SB based n2100 board signals a lot of what ought to be
PCI data parity errors durint operation of the 8169 as target.
Experiment proved that the driver can ignore the error and
process the packet as if nothing had happened.
Let's add an ad-hoc knob to enable users to fix their system while
avoiding the risks of a wholesale change.
Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
---
drivers/net/r8169.c | 17 +++++++++++++----
1 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index c8fa9b1..7438049 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -225,6 +225,7 @@ MODULE_DEVICE_TABLE(pci, rtl8169_pci_tbl
static int rx_copybreak = 200;
static int use_dac;
+static int ignore_parity_err;
static struct {
u32 msg_enable;
} debug = { -1 };
@@ -469,6 +470,8 @@ module_param(use_dac, int, 0);
MODULE_PARM_DESC(use_dac, "Enable PCI DAC. Unsafe on 32 bit PCI slot.");
module_param_named(debug, debug.msg_enable, int, 0);
MODULE_PARM_DESC(debug, "Debug verbosity level (0=none, ..., 16=all)");
+module_param_named(ignore_parity_err, ignore_parity_err, bool, 0);
+MODULE_PARM_DESC(ignore_parity_err, "Ignore PCI parity error as target. Default: false");
MODULE_LICENSE("GPL");
MODULE_VERSION(RTL8169_VERSION);
@@ -2380,12 +2383,17 @@ static void rtl8169_pcierr_interrupt(str
/*
* The recovery sequence below admits a very elaborated explanation:
* - it seems to work;
- * - I did not see what else could be done.
+ * - I did not see what else could be done;
+ * - it makes iop3xx happy.
*
* Feel free to adjust to your needs.
*/
- pci_write_config_word(pdev, PCI_COMMAND,
- pci_cmd | PCI_COMMAND_SERR | PCI_COMMAND_PARITY);
+ if (ignore_parity_err)
+ pci_cmd &= ~PCI_COMMAND_PARITY;
+ else
+ pci_cmd |= PCI_COMMAND_SERR | PCI_COMMAND_PARITY;
+
+ pci_write_config_word(pdev, PCI_COMMAND, pci_cmd);
pci_write_config_word(pdev, PCI_STATUS,
pci_status & (PCI_STATUS_DETECTED_PARITY |
@@ -2399,10 +2407,11 @@ static void rtl8169_pcierr_interrupt(str
tp->cp_cmd &= ~PCIDAC;
RTL_W16(CPlusCmd, tp->cp_cmd);
dev->features &= ~NETIF_F_HIGHDMA;
- rtl8169_schedule_work(dev, rtl8169_reinit_task);
}
rtl8169_hw_reset(ioaddr);
+
+ rtl8169_schedule_work(dev, rtl8169_reinit_task);
}
static void
--
1.4.2.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 3/5] r8169: phy program update
2006-12-03 23:58 ` [PATCH 2/5] r8169: tweak the PCI data parity error recovery Francois Romieu
@ 2006-12-03 23:59 ` Francois Romieu
0 siblings, 0 replies; 17+ messages in thread
From: Francois Romieu @ 2006-12-03 23:59 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev
This is commented out in Realtek's driver as well.
Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
---
drivers/net/r8169.c | 5 -----
1 files changed, 0 insertions(+), 5 deletions(-)
diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index 7438049..0b57050 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -1286,11 +1286,6 @@ static void rtl8169_hw_phy_config(struct
/* Shazam ! */
if (tp->mac_version == RTL_GIGA_MAC_VER_04) {
- mdio_write(ioaddr, 31, 0x0001);
- mdio_write(ioaddr, 9, 0x273a);
- mdio_write(ioaddr, 14, 0x7bfb);
- mdio_write(ioaddr, 27, 0x841e);
-
mdio_write(ioaddr, 31, 0x0002);
mdio_write(ioaddr, 1, 0x90d0);
mdio_write(ioaddr, 31, 0x0000);
--
1.4.2.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 4/5] r8169: more alignment for the 0x8168
2006-12-03 23:52 [PATCH 0/5] Pull request for 'r8169-upstream-20061204-00' tag Francois Romieu
2006-12-03 23:54 ` [PATCH 1/5] r8169: more magic during initialization of the hardware Francois Romieu
2006-12-03 23:58 ` [PATCH 2/5] r8169: tweak the PCI data parity error recovery Francois Romieu
@ 2006-12-04 0:03 ` Francois Romieu
2007-02-13 3:50 ` Philip Craig
2006-12-04 0:14 ` [PATCH 5/5] r8169: teach yourself arithmetic in 21 days Francois Romieu
2006-12-07 10:07 ` [PATCH 0/5] Pull request for 'r8169-upstream-20061204-00' tag Jeff Garzik
4 siblings, 1 reply; 17+ messages in thread
From: Francois Romieu @ 2006-12-04 0:03 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev, isely
Two thirds of packets are lost because of misalignment. Users of
Asus laptop did apparently not notice it.
Reported on Gigabyte GA-945GM-S2.
Fix for http://bugzilla.kernel.org/show_bug.cgi?id=7517
Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
---
drivers/net/r8169.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index 0b57050..2379d83 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -2018,7 +2018,7 @@ static int rtl8169_alloc_rx_skb(struct p
if (!skb)
goto err_out;
- skb_reserve(skb, align);
+ skb_reserve(skb, (align - 1) & (u32)skb->data);
*sk_buff = skb;
mapping = pci_map_single(pdev, skb->data, rx_buf_sz,
@@ -2486,7 +2486,7 @@ static inline int rtl8169_try_rx_copy(st
skb = dev_alloc_skb(pkt_size + align);
if (skb) {
- skb_reserve(skb, align);
+ skb_reserve(skb, (align - 1) & (u32)skb->data);
eth_copy_and_sum(skb, sk_buff[0]->data, pkt_size, 0);
*sk_buff = skb;
rtl8169_mark_to_asic(desc, rx_buf_sz);
--
1.4.2.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 4/5] r8169: more alignment for the 0x8168
2006-12-04 0:03 ` [PATCH 4/5] r8169: more alignment for the 0x8168 Francois Romieu
@ 2007-02-13 3:50 ` Philip Craig
2007-02-13 8:14 ` Francois Romieu
0 siblings, 1 reply; 17+ messages in thread
From: Philip Craig @ 2007-02-13 3:50 UTC (permalink / raw)
To: Francois Romieu; +Cc: Jeff Garzik, netdev, isely
Francois Romieu wrote:
> Two thirds of packets are lost because of misalignment. Users of
> Asus laptop did apparently not notice it.
>
> Reported on Gigabyte GA-945GM-S2.
>
> Fix for http://bugzilla.kernel.org/show_bug.cgi?id=7517
>
> Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
> ---
> drivers/net/r8169.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
> index 0b57050..2379d83 100644
> --- a/drivers/net/r8169.c
> +++ b/drivers/net/r8169.c
> @@ -2018,7 +2018,7 @@ static int rtl8169_alloc_rx_skb(struct p
> if (!skb)
> goto err_out;
>
> - skb_reserve(skb, align);
> + skb_reserve(skb, (align - 1) & (u32)skb->data);
> *sk_buff = skb;
>
> mapping = pci_map_single(pdev, skb->data, rx_buf_sz,
> @@ -2486,7 +2486,7 @@ static inline int rtl8169_try_rx_copy(st
>
> skb = dev_alloc_skb(pkt_size + align);
> if (skb) {
> - skb_reserve(skb, align);
> + skb_reserve(skb, (align - 1) & (u32)skb->data);
> eth_copy_and_sum(skb, sk_buff[0]->data, pkt_size, 0);
> *sk_buff = skb;
> rtl8169_mark_to_asic(desc, rx_buf_sz);
This patch caused a drop in throughput from 178 Mbits/sec to 135 Mbits/sec
on an Intel XScale IXP465.
It seems like there is some confusion about what the align parameter
here means. It was originally an offset from an aligned address so that
the IP header aligned, and this patch changes it to the alignment of the
ethernet header. But align is still set to NET_IP_ALIGN for some chips.
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 4/5] r8169: more alignment for the 0x8168
2007-02-13 3:50 ` Philip Craig
@ 2007-02-13 8:14 ` Francois Romieu
2007-02-13 23:32 ` Mike Isely
2007-02-13 23:56 ` Philip Craig
0 siblings, 2 replies; 17+ messages in thread
From: Francois Romieu @ 2007-02-13 8:14 UTC (permalink / raw)
To: Philip Craig; +Cc: Jeff Garzik, netdev, isely
Philip Craig <philipc@snapgear.com> :
[...]
> This patch caused a drop in throughput from 178 Mbits/sec to 135 Mbits/sec
> on an Intel XScale IXP465.
Which distribution of packet sizes ?
> It seems like there is some confusion about what the align parameter
> here means. It was originally an offset from an aligned address so that
> the IP header aligned, and this patch changes it to the alignment of the
> ethernet header. But align is still set to NET_IP_ALIGN for some chips.
Yes, I should have distinguished both in the first place.
Can you describe which chipset from realtek the board is using (lspci -vxx) ?
--
Ueimor
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] r8169: more alignment for the 0x8168
2007-02-13 8:14 ` Francois Romieu
@ 2007-02-13 23:32 ` Mike Isely
2007-02-23 23:13 ` Francois Romieu
2007-02-13 23:56 ` Philip Craig
1 sibling, 1 reply; 17+ messages in thread
From: Mike Isely @ 2007-02-13 23:32 UTC (permalink / raw)
To: Francois Romieu; +Cc: Philip Craig, Jeff Garzik, netdev, Mike Isely at pobox
On Tue, 13 Feb 2007, Francois Romieu wrote:
> Philip Craig <philipc@snapgear.com> :
> [...]
> > This patch caused a drop in throughput from 178 Mbits/sec to 135 Mbits/sec
> > on an Intel XScale IXP465.
>
> Which distribution of packet sizes ?
>
> > It seems like there is some confusion about what the align parameter
> > here means. It was originally an offset from an aligned address so that
> > the IP header aligned, and this patch changes it to the alignment of the
> > ethernet header. But align is still set to NET_IP_ALIGN for some chips.
>
> Yes, I should have distinguished both in the first place.
>
> Can you describe which chipset from realtek the board is using (lspci -vxx) ?
>
Francois:
Obviously I have an interest in any change here not breaking the NIC on
my system. So please let me know if/when you'd like me to test drive a
candidate fix that keeps everyone happy...
-Mike
--
| Mike Isely | PGP fingerprint
Spammers Die!! | | 03 54 43 4D 75 E5 CC 92
| isely @ pobox (dot) com | 71 16 01 E2 B5 F5 C1 E8
| |
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 4/5] r8169: more alignment for the 0x8168
2007-02-13 23:32 ` Mike Isely
@ 2007-02-23 23:13 ` Francois Romieu
2007-02-26 7:42 ` Philip Craig
0 siblings, 1 reply; 17+ messages in thread
From: Francois Romieu @ 2007-02-23 23:13 UTC (permalink / raw)
To: Mike Isely; +Cc: Philip Craig, Jeff Garzik, netdev
Mike Isely <isely@isely.net> :
[...]
> Obviously I have an interest in any change here not breaking the NIC on
> my system. So please let me know if/when you'd like me to test drive a
> candidate fix that keeps everyone happy...
The experimental r8169 patch of the day against 2.6.21-rc1 is available at:
http://www.fr.zoreil.com/linux/kernel/2.6.x/2.6.21-rc1/
--
Ueimor
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] r8169: more alignment for the 0x8168
2007-02-23 23:13 ` Francois Romieu
@ 2007-02-26 7:42 ` Philip Craig
2007-02-26 22:25 ` Stephen Hemminger
2007-02-28 21:48 ` Francois Romieu
0 siblings, 2 replies; 17+ messages in thread
From: Philip Craig @ 2007-02-26 7:42 UTC (permalink / raw)
To: Francois Romieu; +Cc: Mike Isely, Jeff Garzik, netdev
Francois Romieu wrote:
> The experimental r8169 patch of the day against 2.6.21-rc1 is available at:
> http://www.fr.zoreil.com/linux/kernel/2.6.x/2.6.21-rc1/
Is 0006-r8169-confusion-between-hardware-and-IP-header-alignment.txt
the only relevant patch?
This only partially helps. Many of the packets are greater than 200
bytes so copybreak doesn't apply to them.
Can we assume anything about the alignment of skb->data? I think it
should be 4 byte aligned, otherwise the whole NET_IP_ALIGN thing
won't work. All the drivers I looked at just reserve NET_IP_ALIGN
without checking the alignment first.
So can you do something like set align to 0 for RTL_CFG_0 and change
rtl8169_alloc_rx_skb() to:
skb_reserve(skb, align ? (align - 1) & (u32)skb->data : NET_IP_ALIGN);
BTW, should the alignment expression be:
(((u32)skb->data + (align - 1)) & ~(align - 1)) - (u32)skb->data
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] r8169: more alignment for the 0x8168
2007-02-26 7:42 ` Philip Craig
@ 2007-02-26 22:25 ` Stephen Hemminger
2007-02-28 21:48 ` Francois Romieu
1 sibling, 0 replies; 17+ messages in thread
From: Stephen Hemminger @ 2007-02-26 22:25 UTC (permalink / raw)
To: Philip Craig; +Cc: Francois Romieu, Mike Isely, Jeff Garzik, netdev
On Mon, 26 Feb 2007 17:42:27 +1000
Philip Craig <philipc@snapgear.com> wrote:
> Francois Romieu wrote:
> > The experimental r8169 patch of the day against 2.6.21-rc1 is available at:
> > http://www.fr.zoreil.com/linux/kernel/2.6.x/2.6.21-rc1/
>
> Is 0006-r8169-confusion-between-hardware-and-IP-header-alignment.txt
> the only relevant patch?
>
> This only partially helps. Many of the packets are greater than 200
> bytes so copybreak doesn't apply to them.
>
> Can we assume anything about the alignment of skb->data? I think it
> should be 4 byte aligned, otherwise the whole NET_IP_ALIGN thing
> won't work. All the drivers I looked at just reserve NET_IP_ALIGN
> without checking the alignment first.
>
This is where the alignment comes from in slab.c
#ifndef ARCH_KMALLOC_MINALIGN
/*
* Enforce a minimum alignment for the kmalloc caches.
* Usually, the kmalloc caches are cache_line_size() aligned, except when
* DEBUG and FORCED_DEBUG are enabled, then they are BYTES_PER_WORD aligned.
* Some archs want to perform DMA into kmalloc caches and need a guaranteed
* alignment larger than BYTES_PER_WORD. ARCH_KMALLOC_MINALIGN allows that.
* Note that this flag disables some debug features.
*/
#define ARCH_KMALLOC_MINALIGN 0
#endif
Since BYTES_PER_WORD is sizeof(void *) you are safe since Linux doesn't
run on 16 bit platforms.
--
Stephen Hemminger <shemminger@linux-foundation.org>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] r8169: more alignment for the 0x8168
2007-02-26 7:42 ` Philip Craig
2007-02-26 22:25 ` Stephen Hemminger
@ 2007-02-28 21:48 ` Francois Romieu
2007-02-28 23:18 ` Francois Romieu
1 sibling, 1 reply; 17+ messages in thread
From: Francois Romieu @ 2007-02-28 21:48 UTC (permalink / raw)
To: Philip Craig; +Cc: Mike Isely, Jeff Garzik, netdev
Sorry for the delay, I took some time to check the history of the
r8169 alignment issues.
Philip Craig <philipc@snapgear.com> :
[...]
> This only partially helps. Many of the packets are greater than 200
> bytes so copybreak doesn't apply to them.
Yes.
> Can we assume anything about the alignment of skb->data? I think it
> should be 4 byte aligned, otherwise the whole NET_IP_ALIGN thing
> won't work. All the drivers I looked at just reserve NET_IP_ALIGN
> without checking the alignment first.
>
> So can you do something like set align to 0 for RTL_CFG_0 and change
> rtl8169_alloc_rx_skb() to:
> skb_reserve(skb, align ? (align - 1) & (u32)skb->data : NET_IP_ALIGN);
The "So" part assumes that the 0x8169 can DMA at any address.
/me ponders...
It's easy to debug if it misbehaves now or in 6 months on some obscure
system. It's consistent with the preprevious code. Ok, good idea, I like it.
[...]
> BTW, should the alignment expression be:
> (((u32)skb->data + (align - 1)) & ~(align - 1)) - (u32)skb->data
I'll see if something can be hacked with a zero or power of two alignment.
--
Ueimor
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] r8169: more alignment for the 0x8168
2007-02-28 21:48 ` Francois Romieu
@ 2007-02-28 23:18 ` Francois Romieu
0 siblings, 0 replies; 17+ messages in thread
From: Francois Romieu @ 2007-02-28 23:18 UTC (permalink / raw)
To: Mike Isely; +Cc: Philip Craig, Jeff Garzik, netdev, Edward Hsu
Francois Romieu <romieu@fr.zoreil.com> :
[...]
The experimental r8169 patch of the day against 2.6.21-rc2 is available at:
http://www.fr.zoreil.com/linux/www.fr.zoreil.com/people/francois/misc/20070228-2.6.21-rc2-r8169-test.patch
(single patch)
or:
http://www.fr.zoreil.com/linux/kernel/2.6.x/2.6.21-rc2
(series)
Log below:
commit 6686d80d6972cd5ff3ca81b72c46f4ffcc40eb4c
Author: Francois Romieu <romieu@fr.zoreil.com>
Date: Wed Feb 28 23:16:57 2007 +0100
r8169: align the IP header when there is no DMA constraint
Align the IP header when the chipset can DMA at any location (plain 0x8169).
Otherwise (0x8136/0x8168) obey the constraint imposed by the hardware.
This patch complements the previous alignment rework done for copybreak.
Original idea from Philip Craig <philipc@snapgear.com>
Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
Cc: Philip Craig <philipc@snapgear.com>
Cc: Mike Isely <isely@pobox.com>
commit d20a6ba195172f7fb9fd30832a054effb9773bc3
Author: Francois Romieu <romieu@fr.zoreil.com>
Date: Fri Feb 23 23:50:28 2007 +0100
r8169.c: add bit description for the TxPoll register
Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
Cc: Edward Hsu <edward_hsu@realtek.com>
commit 37dc1270eba2874a00564abe0d857429af5370f2
Author: Francois Romieu <romieu@fr.zoreil.com>
Date: Fri Feb 23 23:24:55 2007 +0100
r8169: MSI support
It is currently limited to 0x8136 and 0x8168. 8169sb/8110sb ought to
handle it as well where they support MSI.
Includes unregister_netdev() fix from Bernhard Walle <bwalle@suse.de>
against BUG_ON(irq_has_action(dev->first_msi_irq)) (2007/02/24).
Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
Fixed-by: Bernhard Walle <bernhard.walle@gmx.de>
Cc: Edward Hsu <edward_hsu@realtek.com>
commit b388fb659dc5803cdb2293649e25807f88ba94ec
Author: Francois Romieu <romieu@fr.zoreil.com>
Date: Wed Feb 21 22:40:46 2007 +0100
r8169: cleanup
No functionnal change:
- trim the old history log
- whitespace/indent/case police
- unsigned int where signedness does not matte
- removal of obsolete assert
- needless cast from void * (dev_instance)
Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
Cc: Edward Hsu <edward_hsu@realtek.com>
commit 090121a1d8b9452fd454fa44ba67d9761a6e8f1e
Author: Francois Romieu <romieu@fr.zoreil.com>
Date: Wed Feb 21 00:10:20 2007 +0100
r8169: remove the media option
It has been documented as deprecated:
- in MODULE_PARM_DESC since may 2005 ;
- at the top of the source file and in printk since june 2004.
Good bye.
Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
Cc: Edward Hsu <edward_hsu@realtek.com>
commit 5231dd72b4d9551c6cd8baa9b7026a1f21b12052
Author: Francois Romieu <romieu@fr.zoreil.com>
Date: Tue Feb 20 22:58:51 2007 +0100
r8169: small 8101 comment
Extracted from version 1.001.00 of Realtek's r8101.
Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
Cc: Edward Hsu <edward_hsu@realtek.com>
commit 20be52f668774727ba1d1a3606cc2888f66d40bf
Author: Francois Romieu <romieu@fr.zoreil.com>
Date: Tue Feb 20 22:20:51 2007 +0100
r8169: confusion between hardware and IP header alignment
The rx copybreak part is straightforward.
The align field in struct rtl_cfg_info is related to the alignment
requirements of the DMA operation. Its value is set at 2 to limit the
scale of possible regression but my old v1.21 8169 datasheet claims a
8 bytes requirements (that was never followed by the driver of course)
and the 8101/8168 go with a plain 8 bytes alignments. Yuck...
/me waits for the attack of ballistic vegetables...
Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
Cc: Edward Hsu <edward_hsu@realtek.com>
commit b0ee36861173a3ac57017c8a3850ad21a4c1acf6
Author: Francois Romieu <romieu@fr.zoreil.com>
Date: Tue Feb 20 00:00:26 2007 +0100
r8169: merge with version 8.001.00 of Realtek's r8168 driver
This one includes:
- more tweaks to rtl_hw_start_8168
- a work around for a Rx FiFO overflow issue on the 8168Bb
+ rtl8169_{intr_mask/napi_event} are replaced with per-device fields
+ rtl_cfg_info is converted to C99 for readability but the values are
not changed for the 8169/8110 and the 8101
Includes ChipCmd fix from Bernhard Walle <bwalle@suse.de> (2007/02/24).
Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
Cc: Edward Hsu <edward_hsu@realtek.com>
commit 9d4139624a1c2ae138ea043083263c84d14bbd3a
Author: Francois Romieu <romieu@fr.zoreil.com>
Date: Tue Feb 13 23:38:05 2007 +0100
r8169: merge with version 6.001.00 of Realtek's r8169 driver
- new identifier for the 8110SCe ;
- the PCI latency timer is set unconditionally. This part is identical
in Realtek's r8168 (8.001.00) and r8101 (1.001.00) ;
- initialization of the cache line size register is for the 8169s only ;
- more magic in rtl_hw_start_8169 ;
- it is not possible to factor out the setting of the the irq event mask
with the 8168 and the 8101 any more. Pushed it into the hw_start handler ;
- rtl_set_rx_tx_config_registers() and write to the ChipCmd register are
issued identically for the whole 8169/8110 family: the 8110SCd/8110SCe
make no more difference ;
- work around for AMD platform.
I let aside some registers definitions for later.
Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
Cc: Edward Hsu <edward_hsu@realtek.com>
commit 7b6de0565fb48bb2a7a998c5f1e80d1301a9c14a
Author: Francois Romieu <romieu@fr.zoreil.com>
Date: Tue Feb 13 22:30:50 2007 +0100
r8169: add per-device hw_start handler (2/2)
Part 2: populate the hw_start handlers for the 8168 and the 8101.
- annotate mac_version
- factor out the initialization of the receive and transmit ring
descriptor registers (rtl_set_rx_tx_desc_registers)
- access to the CPlusCmd and setting of the maximum receive size are
idiomatic too (rtl_rw_cpluscmd and rtl_set_rx_max_size)
Each chipset should run through the same code as before in its
dedicated hw_start handler.
Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
Cc: Edward Hsu <edward_hsu@realtek.com>
commit f33de543281a3fbe2e15ad482b746b3a21f1d865
Author: Francois Romieu <romieu@fr.zoreil.com>
Date: Fri Feb 23 23:36:39 2007 +0100
r8169: add per-device hw_start handler (1/2)
rtl8169_hw_start will not help maintaining an unified driver for
different chipsets but people at Realtek are probably too polite
to say it distinctly.
Part 1: add the hook and keep hw_start handler unchanged.
As can be seen from the content of rtl8169_pci_tbl, the RTL_CFG_1
entry in rtl_cfg_info was unused. I recycled it for the 0x8168.
Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
Cc: Edward Hsu <edward_hsu@realtek.com>
commit bf52169682e73b43ddd58608c981e39932db7174
Author: Jeff Garzik <jeff@garzik.org>
Date: Sun Feb 11 22:50:31 2007 +0100
r8169: fix a race between PCI probe and dev_open
Initialize the timer with the rest of the private-struct.
Signed-off-by: Jeff Garzik <jeff@garzik.org>
Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
Cc: Edward Hsu <edward_hsu@realtek.com>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] r8169: more alignment for the 0x8168
2007-02-13 8:14 ` Francois Romieu
2007-02-13 23:32 ` Mike Isely
@ 2007-02-13 23:56 ` Philip Craig
1 sibling, 0 replies; 17+ messages in thread
From: Philip Craig @ 2007-02-13 23:56 UTC (permalink / raw)
To: Francois Romieu; +Cc: Jeff Garzik, netdev, isely
Francois Romieu wrote:
> Philip Craig <philipc@snapgear.com> :
> [...]
>> This patch caused a drop in throughput from 178 Mbits/sec to 135 Mbits/sec
>> on an Intel XScale IXP465.
>
> Which distribution of packet sizes ?
Just using iperf with the default options, MTU 1500,
forwarding between 2 PCs with the board in the middle.
>> It seems like there is some confusion about what the align parameter
>> here means. It was originally an offset from an aligned address so that
>> the IP header aligned, and this patch changes it to the alignment of the
>> ethernet header. But align is still set to NET_IP_ALIGN for some chips.
>
> Yes, I should have distinguished both in the first place.
>
> Can you describe which chipset from realtek the board is using (lspci -vxx) ?
# lspci -vxx
00:0c.0 Ethernet controller: Realtek Semiconductor Co., Ltd.: Unknown device 8169 (rev 10)
Subsystem: Realtek Semiconductor Co., Ltd.: Unknown device 8169
Flags: bus master, 66Mhz, medium devsel, latency 0, IRQ 26
I/O ports at 1000 [size=256]
Memory at 48061000 (32-bit, non-prefetchable) [size=256]
Expansion ROM at 48000000 [disabled] [size=128K]
Capabilities: [dc] Power Management version 2
00: ec 10 69 81 57 01 b0 02 10 00 00 02 08 00 00 00
10: 01 10 00 00 00 10 06 48 00 00 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 ec 10 69 81
30: 00 00 00 00 dc 00 00 00 00 00 00 00 1a 01 20 40
00:0d.0 Ethernet controller: Realtek Semiconductor Co., Ltd.: Unknown device 8169 (rev 10)
Subsystem: Realtek Semiconductor Co., Ltd.: Unknown device 8169
Flags: bus master, 66Mhz, medium devsel, latency 0, IRQ 26
I/O ports at 1400 [size=256]
Memory at 48061100 (32-bit, non-prefetchable) [size=256]
Expansion ROM at 48020000 [disabled] [size=128K]
Capabilities: [dc] Power Management version 2
00: ec 10 69 81 57 01 b0 02 10 00 00 02 08 00 00 00
10: 01 14 00 00 00 11 06 48 00 00 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 ec 10 69 81
30: 00 00 00 00 dc 00 00 00 00 00 00 00 1a 01 20 40
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 5/5] r8169: teach yourself arithmetic in 21 days
2006-12-03 23:52 [PATCH 0/5] Pull request for 'r8169-upstream-20061204-00' tag Francois Romieu
` (2 preceding siblings ...)
2006-12-04 0:03 ` [PATCH 4/5] r8169: more alignment for the 0x8168 Francois Romieu
@ 2006-12-04 0:14 ` Francois Romieu
2006-12-07 10:07 ` [PATCH 0/5] Pull request for 'r8169-upstream-20061204-00' tag Jeff Garzik
4 siblings, 0 replies; 17+ messages in thread
From: Francois Romieu @ 2006-12-04 0:14 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev
Ok, I can not count up to 5 and the serie really contains 4 patches.
Btw the patch a0e21b002db5bce80aac82c21cd996423487a44c (top of the
pile) contained a reference to bug 7617 instead of 7517. It has been
replaced and tag r8169-upstream-20061204-00 now points to
cc9f022d97d08e4e36d38661857991fe91447d68
--
Ueimor
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 0/5] Pull request for 'r8169-upstream-20061204-00' tag
2006-12-03 23:52 [PATCH 0/5] Pull request for 'r8169-upstream-20061204-00' tag Francois Romieu
` (3 preceding siblings ...)
2006-12-04 0:14 ` [PATCH 5/5] r8169: teach yourself arithmetic in 21 days Francois Romieu
@ 2006-12-07 10:07 ` Jeff Garzik
4 siblings, 0 replies; 17+ messages in thread
From: Jeff Garzik @ 2006-12-07 10:07 UTC (permalink / raw)
To: Francois Romieu; +Cc: netdev
Francois Romieu wrote:
> Please pull from tag 'r8169-upstream-20061204-00' in repository
>
> git://electric-eye.fr.zoreil.com/home/romieu/linux-2.6.git#r8169-upstream-20061204-00
pulled. please change your "please pull" email message to reflect
something I can cut-n-paste directly to the command line. For tags,
this would be "$url tag $tag".
Jeff
^ permalink raw reply [flat|nested] 17+ messages in thread