[PATCH] r8169: set TxConfig register after TX / RX is enabled, just like RxConfig

[PATCH] r8169: set TxConfig register after TX / RX is enabled, just like RxConfig

[Maciej S. Szmigiero]

Commit 3559d81e76bf ("r8169: simplify rtl_hw_start_8169") changed order of
two register writes:
1) Caused RxConfig to be written before TX / RX is enabled,
2) Caused TxConfig to be written before TX / RX is enabled.

At least on XIDs 10000000 ("RTL8169sb/8110sb") and
18000000 ("RTL8169sc/8110sc") such writes are ignored by the chip, leaving
values in these registers intact.

Change 1) was reverted by
commit 05212ba8132b42 ("r8169: set RxConfig after tx/rx is enabled for RTL8169sb/8110sb devices"),
however change 2) wasn't.

In practice, this caused TxConfig's "InterFrameGap time" and "Max DMA Burst
Size per Tx DMA Burst" bits to be zero dramatically reducing TX performance
(in my tests it dropped from around 500Mbps to around 50Mbps).

This patch fixes the issue by moving TxConfig register write a bit later in
the code so it happens after TX / RX is already enabled.

Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
Fixes: 05212ba8132b42 ("r8169: set RxConfig after tx/rx is enabled for RTL8169sb/8110sb devices")
---
"Fixes" tag points to the RxConfig fix instead of the actual commit that
introduced this regression to maintain patch ordering since the RxConfig fix
partially affects the same code lines as this fix.

 drivers/net/ethernet/realtek/r8169.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index b935a18358cb..2ade3a27d7f1 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -4634,13 +4634,13 @@ static void rtl_hw_start(struct  rtl8169_private *tp)
 
 	rtl_set_rx_max_size(tp);
 	rtl_set_rx_tx_desc_registers(tp);
-	rtl_set_tx_config_registers(tp);
 	RTL_W8(tp, Cfg9346, Cfg9346_Lock);
 
 	/* Initially a 10 us delay. Turned it into a PCI commit. - FR */
 	RTL_R8(tp, IntrMask);
 	RTL_W8(tp, ChipCmd, CmdTxEnb | CmdRxEnb);
 	rtl_init_rxcfg(tp);
+	rtl_set_tx_config_registers(tp);
 
 	rtl_set_rx_mode(tp->dev);
 	/* no early-rx interrupts */
-- 
2.17.0

Re: [PATCH] r8169: set TxConfig register after TX / RX is enabled, just like RxConfig

[David Miller]

From: "Maciej S. Szmigiero" <mail@maciej.szmigiero.name>
Date: Fri, 7 Sep 2018 20:15:22 +0200

> Commit 3559d81e76bf ("r8169: simplify rtl_hw_start_8169") changed order of
> two register writes:
> 1) Caused RxConfig to be written before TX / RX is enabled,
> 2) Caused TxConfig to be written before TX / RX is enabled.
> 
> At least on XIDs 10000000 ("RTL8169sb/8110sb") and
> 18000000 ("RTL8169sc/8110sc") such writes are ignored by the chip, leaving
> values in these registers intact.
> 
> Change 1) was reverted by
> commit 05212ba8132b42 ("r8169: set RxConfig after tx/rx is enabled for RTL8169sb/8110sb devices"),
> however change 2) wasn't.
> 
> In practice, this caused TxConfig's "InterFrameGap time" and "Max DMA Burst
> Size per Tx DMA Burst" bits to be zero dramatically reducing TX performance
> (in my tests it dropped from around 500Mbps to around 50Mbps).
> 
> This patch fixes the issue by moving TxConfig register write a bit later in
> the code so it happens after TX / RX is already enabled.
> 
> Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
> Fixes: 05212ba8132b42 ("r8169: set RxConfig after tx/rx is enabled for RTL8169sb/8110sb devices")
> ---
> "Fixes" tag points to the RxConfig fix instead of the actual commit that
> introduced this regression to maintain patch ordering since the RxConfig fix
> partially affects the same code lines as this fix.

Applied, thanks.

Please, in the future, always put your Fixes: tag first in the set of tags.

Thank you.