[PATCH net-next 0/8] r8169: smaller improvements again

[PATCH net-next 0/8] r8169: smaller improvements again

[Heiner Kallweit]

Series includes a number of different smaller improvements.

Heiner Kallweit (8):
  r8169: add info for DASH being enabled
  r8169: remove unused constant RsvdMask
  r8169: improve setting WoL on runtime-resume
  r8169: replace synchronize_rcu with synchronize_net
  r8169: move napi_disable call and rename rtl8169_hw_reset
  r8169: move updating counters to rtl8169_down
  r8169: move switching optional clock on/off to pll power functions
  r8169: allow setting irq coalescing if link is down

 drivers/net/ethernet/realtek/r8169_main.c | 51 +++++++++++------------
 1 file changed, 25 insertions(+), 26 deletions(-)

-- 
2.27.0

[PATCH net-next 1/8] r8169: add info for DASH being enabled

[Heiner Kallweit]

In case of problems it facilitates the bug analysis if we know whether
DASH is active. Therefore emit a message in probe if this is the case.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/realtek/r8169_main.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index dad84ecf5..7bb26fb07 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -5432,8 +5432,10 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 			    jumbo_max, tp->mac_version <= RTL_GIGA_MAC_VER_06 ?
 			    "ok" : "ko");
 
-	if (r8168_check_dash(tp))
+	if (r8168_check_dash(tp)) {
+		netdev_info(dev, "DASH enabled\n");
 		rtl8168_driver_start(tp);
+	}
 
 	if (pci_dev_run_wake(pdev))
 		pm_runtime_put_sync(&pdev->dev);
-- 
2.27.0

[PATCH net-next 2/8] r8169: remove unused constant RsvdMask

[Heiner Kallweit]

Since 9d3679fe0f30 ("r8169: inline rtl8169_make_unusable_by_asic")
this constant isn't used any longer, so remove it.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/realtek/r8169_main.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 7bb26fb07..4bc6c5529 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -529,8 +529,6 @@ enum rtl_rx_desc_bit {
 	RxVlanTag	= (1 << 16), /* VLAN tag available */
 };
 
-#define RsvdMask	0x3fffc000
-
 #define RTL_GSO_MAX_SIZE_V1	32000
 #define RTL_GSO_MAX_SEGS_V1	24
 #define RTL_GSO_MAX_SIZE_V2	64000
-- 
2.27.0

[PATCH net-next 3/8] r8169: improve setting WoL on runtime-resume

[Heiner Kallweit]

In the following scenario WoL isn't configured properly:
- Driver is loaded, interface isn't brought up within 10s, so driver
  runtime-suspends.
- WoL is set.
- Interface is brought up, stored WoL setting isn't applied.

It has always been like that, but the scenario seems to be quite
theoretical as I haven't seen any bug report yet. Therefore treat
the change as an improvement.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/realtek/r8169_main.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 4bc6c5529..bd95c0ae6 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -4887,14 +4887,12 @@ static int rtl8169_runtime_resume(struct device *device)
 
 	rtl_rar_set(tp, tp->dev->dev_addr);
 
-	if (!tp->TxDescArray)
-		return 0;
-
 	rtl_lock_work(tp);
 	__rtl8169_set_wol(tp, tp->saved_wolopts);
 	rtl_unlock_work(tp);
 
-	__rtl8169_resume(tp);
+	if (tp->TxDescArray)
+		__rtl8169_resume(tp);
 
 	return 0;
 }
-- 
2.27.0

[PATCH net-next 4/8] r8169: replace synchronize_rcu with synchronize_net

[Heiner Kallweit]

rtl8169_hw_reset() may be called under RTNL lock, therefore switch to
synchronize_net().

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/realtek/r8169_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index bd95c0ae6..0d3e58ae1 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -3929,7 +3929,7 @@ static void rtl8169_tx_clear(struct rtl8169_private *tp)
 static void rtl8169_hw_reset(struct rtl8169_private *tp, bool going_down)
 {
 	/* Give a racing hard_start_xmit a few cycles to complete. */
-	synchronize_rcu();
+	synchronize_net();
 
 	/* Disable interrupts */
 	rtl8169_irq_mask_and_ack(tp);
-- 
2.27.0

[PATCH net-next 5/8] r8169: move napi_disable call and rename rtl8169_hw_reset

[Heiner Kallweit]

rtl8169_hw_reset() meanwhile does more than a hw reset, therefore rename
it to rtl8169_cleanup(). In addition move calling napi_disable() to this
function.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/realtek/r8169_main.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 0d3e58ae1..afcdaace2 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -3926,8 +3926,10 @@ static void rtl8169_tx_clear(struct rtl8169_private *tp)
 	netdev_reset_queue(tp->dev);
 }
 
-static void rtl8169_hw_reset(struct rtl8169_private *tp, bool going_down)
+static void rtl8169_cleanup(struct rtl8169_private *tp, bool going_down)
 {
+	napi_disable(&tp->napi);
+
 	/* Give a racing hard_start_xmit a few cycles to complete. */
 	synchronize_net();
 
@@ -3970,10 +3972,9 @@ static void rtl_reset_work(struct rtl8169_private *tp)
 	struct net_device *dev = tp->dev;
 	int i;
 
-	napi_disable(&tp->napi);
 	netif_stop_queue(dev);
 
-	rtl8169_hw_reset(tp, false);
+	rtl8169_cleanup(tp, false);
 
 	for (i = 0; i < NUM_RX_DESC; i++)
 		rtl8169_mark_to_asic(tp->RxDescArray + i);
@@ -4636,9 +4637,8 @@ static void rtl8169_down(struct rtl8169_private *tp)
 	bitmap_zero(tp->wk.flags, RTL_FLAG_MAX);
 
 	phy_stop(tp->phydev);
-	napi_disable(&tp->napi);
 
-	rtl8169_hw_reset(tp, true);
+	rtl8169_cleanup(tp, true);
 
 	rtl_pll_power_down(tp);
 
-- 
2.27.0

[PATCH net-next 6/8] r8169: move updating counters to rtl8169_down

[Heiner Kallweit]

Counters are updated whenever we go down, therefore move the call to
rtl8169_down().

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/realtek/r8169_main.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index afcdaace2..9f99b3f07 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -4638,6 +4638,8 @@ static void rtl8169_down(struct rtl8169_private *tp)
 
 	phy_stop(tp->phydev);
 
+	rtl8169_update_counters(tp);
+
 	rtl8169_cleanup(tp, true);
 
 	rtl_pll_power_down(tp);
@@ -4652,9 +4654,6 @@ static int rtl8169_close(struct net_device *dev)
 
 	pm_runtime_get_sync(&pdev->dev);
 
-	/* Update counters before going down */
-	rtl8169_update_counters(tp);
-
 	netif_stop_queue(dev);
 	rtl8169_down(tp);
 	rtl8169_rx_clear(tp);
@@ -4875,9 +4874,6 @@ static int rtl8169_runtime_suspend(struct device *device)
 
 	rtl8169_net_suspend(tp);
 
-	/* Update counters before going runtime suspend */
-	rtl8169_update_counters(tp);
-
 	return 0;
 }
 
-- 
2.27.0

[PATCH net-next 7/8] r8169: move switching optional clock on/off to pll power functions

[Heiner Kallweit]

Relevant chip clocks are disabled in rtl_pll_power_down(), therefore
move calling clk_disable_unprepare() there. Similar for enabling the
clock.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/realtek/r8169_main.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 9f99b3f07..d55bf2cd2 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -2292,10 +2292,14 @@ static void rtl_pll_power_down(struct rtl8169_private *tp)
 	default:
 		break;
 	}
+
+	clk_disable_unprepare(tp->clk);
 }
 
 static void rtl_pll_power_up(struct rtl8169_private *tp)
 {
+	clk_prepare_enable(tp->clk);
+
 	switch (tp->mac_version) {
 	case RTL_GIGA_MAC_VER_25 ... RTL_GIGA_MAC_VER_33:
 	case RTL_GIGA_MAC_VER_37:
@@ -4826,7 +4830,6 @@ static int __maybe_unused rtl8169_suspend(struct device *device)
 	struct rtl8169_private *tp = dev_get_drvdata(device);
 
 	rtl8169_net_suspend(tp);
-	clk_disable_unprepare(tp->clk);
 
 	return 0;
 }
@@ -4853,8 +4856,6 @@ static int __maybe_unused rtl8169_resume(struct device *device)
 
 	rtl_rar_set(tp, tp->dev->dev_addr);
 
-	clk_prepare_enable(tp->clk);
-
 	if (netif_running(tp->dev))
 		__rtl8169_resume(tp);
 
-- 
2.27.0

[PATCH net-next 8/8] r8169: allow setting irq coalescing if link is down

[Heiner Kallweit]

So far we can not configure irq coalescing when link is down. Allow the
user to do this, and assume that he wants to configure irq coalescing
for highest speed. Otherwise the irq rate is low enough anyway.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/realtek/r8169_main.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index d55bf2cd2..a3c4187d9 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -1731,16 +1731,16 @@ struct rtl_coalesce_info {
 #define COALESCE_DELAY(d) { (d), 8 * (d), 16 * (d), 32 * (d) }
 
 static const struct rtl_coalesce_info rtl_coalesce_info_8169[] = {
-	{ SPEED_10,	COALESCE_DELAY(40960) },
-	{ SPEED_100,	COALESCE_DELAY(2560) },
 	{ SPEED_1000,	COALESCE_DELAY(320) },
+	{ SPEED_100,	COALESCE_DELAY(2560) },
+	{ SPEED_10,	COALESCE_DELAY(40960) },
 	{ 0 },
 };
 
 static const struct rtl_coalesce_info rtl_coalesce_info_8168_8136[] = {
-	{ SPEED_10,	COALESCE_DELAY(40960) },
-	{ SPEED_100,	COALESCE_DELAY(2560) },
 	{ SPEED_1000,	COALESCE_DELAY(5000) },
+	{ SPEED_100,	COALESCE_DELAY(2560) },
+	{ SPEED_10,	COALESCE_DELAY(40960) },
 	{ 0 },
 };
 #undef COALESCE_DELAY
@@ -1756,6 +1756,10 @@ rtl_coalesce_info(struct rtl8169_private *tp)
 	else
 		ci = rtl_coalesce_info_8168_8136;
 
+	/* if speed is unknown assume highest one */
+	if (tp->phydev->speed == SPEED_UNKNOWN)
+		return ci;
+
 	for (; ci->speed; ci++) {
 		if (tp->phydev->speed == ci->speed)
 			return ci;
-- 
2.27.0

Re: [PATCH net-next 0/8] r8169: smaller improvements again

[David Miller]

From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Wed, 17 Jun 2020 22:50:11 +0200

> Series includes a number of different smaller improvements.

Series applied, thanks Heiner.