[PATCH net-next v2 0/8] r8169: series with smaller improvements

[PATCH net-next v2 0/8] r8169: series with smaller improvements

[Heiner Kallweit]

Series with smaller improvements.

v2:
- fixed a small copy & paste error in patch 4

Heiner Kallweit (8):
  r8169: remove rtl_get_events
  r8169: remove unneeded call in pcierr
  r8169: initialize task workqueue only once
  r8169: improve rtl_pcie_state_l2l3_enable
  r8169: add helpers for locking / unlocking the config registers
  r8169: reset chip synchronously in __rtl8169_resume
  r8169: replace mii_bus member with phy_device member in struct
    rtl8169_private
  r8169: factor out getting ether_clk

 drivers/net/ethernet/realtek/r8169.c | 188 +++++++++++++--------------
 1 file changed, 89 insertions(+), 99 deletions(-)

-- 
2.20.1

[PATCH net-next v2 1/8] r8169: remove rtl_get_events

[Heiner Kallweit]

This helper is used only once, so remove it.

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

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index abb94c543..b2e30e302 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -1278,11 +1278,6 @@ static u8 rtl8168d_efuse_read(struct rtl8169_private *tp, int reg_addr)
 		RTL_R32(tp, EFUSEAR) & EFUSEAR_DATA_MASK : ~0;
 }
 
-static u16 rtl_get_events(struct rtl8169_private *tp)
-{
-	return RTL_R16(tp, IntrStatus);
-}
-
 static void rtl_ack_events(struct rtl8169_private *tp, u16 bits)
 {
 	RTL_W16(tp, IntrStatus, bits);
@@ -6409,7 +6404,7 @@ static int rtl_rx(struct net_device *dev, struct rtl8169_private *tp, u32 budget
 static irqreturn_t rtl8169_interrupt(int irq, void *dev_instance)
 {
 	struct rtl8169_private *tp = dev_instance;
-	u16 status = rtl_get_events(tp);
+	u16 status = RTL_R16(tp, IntrStatus);
 	u16 irq_mask = RTL_R16(tp, IntrMask);
 
 	if (status == 0xffff || !(status & irq_mask))
-- 
2.20.1

[PATCH net-next v2 2/8] r8169: remove unneeded call in pcierr

[Heiner Kallweit]

rtl8169_hw_reset() is called as part of the reset routine which is
scheduled in the line after. So we can remove the call to
rtl8169_hw_reset() here.

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

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index b2e30e302..fd3c6a548 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -6196,8 +6196,6 @@ static void rtl8169_pcierr_interrupt(struct net_device *dev)
 		dev->features &= ~NETIF_F_HIGHDMA;
 	}
 
-	rtl8169_hw_reset(tp);
-
 	rtl_schedule_task(tp, RTL_FLAG_TASK_RESET_PENDING);
 }
 
-- 
2.20.1

[PATCH net-next v2 3/8] r8169: initialize task workqueue only once

[Heiner Kallweit]

It's sufficient to initialize the workqueue once, therefore remove the
additional initialization whenever rtl_open() is called.

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

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index fd3c6a548..c4d53bc6a 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -6630,10 +6630,6 @@ static int rtl_open(struct net_device *dev)
 	if (retval < 0)
 		goto err_free_rx_1;
 
-	INIT_WORK(&tp->wk.work, rtl_task);
-
-	smp_mb();
-
 	rtl_request_firmware(tp);
 
 	retval = pci_request_irq(pdev, 0, rtl8169_interrupt, NULL, tp,
@@ -7293,6 +7289,7 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	tp->saved_wolopts = __rtl8169_get_wol(tp);
 
 	mutex_init(&tp->wk.mutex);
+	INIT_WORK(&tp->wk.work, rtl_task);
 	u64_stats_init(&tp->rx_stats.syncp);
 	u64_stats_init(&tp->tx_stats.syncp);
 
-- 
2.20.1

[PATCH net-next v2 4/8] r8169: improve rtl_pcie_state_l2l3_enable

[Heiner Kallweit]

All calls to this function have the enable parameter set to false.
So we can replace the function with a disable-only version.

v2:
- fixed copy & paste error

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

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index c4d53bc6a..df3d2633f 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -4691,18 +4691,10 @@ static void rtl_enable_clock_request(struct rtl8169_private *tp)
 				 PCI_EXP_LNKCTL_CLKREQ_EN);
 }
 
-static void rtl_pcie_state_l2l3_enable(struct rtl8169_private *tp, bool enable)
+static void rtl_pcie_state_l2l3_disable(struct rtl8169_private *tp)
 {
-	u8 data;
-
-	data = RTL_R8(tp, Config3);
-
-	if (enable)
-		data |= Rdy_to_L23;
-	else
-		data &= ~Rdy_to_L23;
-
-	RTL_W8(tp, Config3, data);
+	/* work around an issue when PCI reset occurs during L2/L3 state */
+	RTL_W8(tp, Config3, RTL_R8(tp, Config3) & ~Rdy_to_L23);
 }
 
 static void rtl_hw_aspm_clkreq_enable(struct rtl8169_private *tp, bool enable)
@@ -5023,7 +5015,7 @@ static void rtl_hw_start_8411(struct rtl8169_private *tp)
 	};
 
 	rtl_hw_start_8168f(tp);
-	rtl_pcie_state_l2l3_enable(tp, false);
+	rtl_pcie_state_l2l3_disable(tp);
 
 	rtl_ephy_init(tp, e_info_8168f_1, ARRAY_SIZE(e_info_8168f_1));
 
@@ -5057,7 +5049,7 @@ static void rtl_hw_start_8168g(struct rtl8169_private *tp)
 	rtl_w0w1_eri(tp, 0x2fc, ERIAR_MASK_0001, 0x01, 0x06, ERIAR_EXGMAC);
 	rtl_w0w1_eri(tp, 0x1b0, ERIAR_MASK_0011, 0x0000, 0x1000, ERIAR_EXGMAC);
 
-	rtl_pcie_state_l2l3_enable(tp, false);
+	rtl_pcie_state_l2l3_disable(tp);
 }
 
 static void rtl_hw_start_8168g_1(struct rtl8169_private *tp)
@@ -5163,7 +5155,7 @@ static void rtl_hw_start_8168h_1(struct rtl8169_private *tp)
 
 	rtl_w0w1_eri(tp, 0x1b0, ERIAR_MASK_0011, 0x0000, 0x1000, ERIAR_EXGMAC);
 
-	rtl_pcie_state_l2l3_enable(tp, false);
+	rtl_pcie_state_l2l3_disable(tp);
 
 	rtl_writephy(tp, 0x1f, 0x0c42);
 	rg_saw_cnt = (rtl_readphy(tp, 0x13) & 0x3fff);
@@ -5240,7 +5232,7 @@ static void rtl_hw_start_8168ep(struct rtl8169_private *tp)
 
 	RTL_W8(tp, DLLPR, RTL_R8(tp, DLLPR) & ~TX_10M_PS_EN);
 
-	rtl_pcie_state_l2l3_enable(tp, false);
+	rtl_pcie_state_l2l3_disable(tp);
 }
 
 static void rtl_hw_start_8168ep_1(struct rtl8169_private *tp)
@@ -5511,7 +5503,7 @@ static void rtl_hw_start_8105e_1(struct rtl8169_private *tp)
 
 	rtl_ephy_init(tp, e_info_8105e_1, ARRAY_SIZE(e_info_8105e_1));
 
-	rtl_pcie_state_l2l3_enable(tp, false);
+	rtl_pcie_state_l2l3_disable(tp);
 }
 
 static void rtl_hw_start_8105e_2(struct rtl8169_private *tp)
@@ -5546,7 +5538,7 @@ static void rtl_hw_start_8402(struct rtl8169_private *tp)
 	rtl_eri_write(tp, 0xb8, ERIAR_MASK_0011, 0x0000, ERIAR_EXGMAC);
 	rtl_w0w1_eri(tp, 0x0d4, ERIAR_MASK_0011, 0x0e00, 0xff00, ERIAR_EXGMAC);
 
-	rtl_pcie_state_l2l3_enable(tp, false);
+	rtl_pcie_state_l2l3_disable(tp);
 }
 
 static void rtl_hw_start_8106(struct rtl8169_private *tp)
@@ -5560,7 +5552,7 @@ static void rtl_hw_start_8106(struct rtl8169_private *tp)
 	RTL_W8(tp, MCU, RTL_R8(tp, MCU) | EN_NDP | EN_OOB_RESET);
 	RTL_W8(tp, DLLPR, RTL_R8(tp, DLLPR) & ~PFM_EN);
 
-	rtl_pcie_state_l2l3_enable(tp, false);
+	rtl_pcie_state_l2l3_disable(tp);
 	rtl_hw_aspm_clkreq_enable(tp, true);
 }
 
-- 
2.20.1

[PATCH net-next v2 5/8] r8169: add helpers for locking / unlocking the config registers

[Heiner Kallweit]

Add helpers for locking / unlocking the config registers.

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

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index df3d2633f..a3df61d81 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -745,6 +745,16 @@ static void rtl_unlock_work(struct rtl8169_private *tp)
 	mutex_unlock(&tp->wk.mutex);
 }
 
+static void rtl_lock_config_regs(struct rtl8169_private *tp)
+{
+	RTL_W8(tp, Cfg9346, Cfg9346_Lock);
+}
+
+static void rtl_unlock_config_regs(struct rtl8169_private *tp)
+{
+	RTL_W8(tp, Cfg9346, Cfg9346_Unlock);
+}
+
 static void rtl_tx_performance_tweak(struct rtl8169_private *tp, u16 force)
 {
 	pcie_capability_clear_and_set_word(tp->pci_dev, PCI_EXP_DEVCTL,
@@ -1426,7 +1436,7 @@ static void __rtl8169_set_wol(struct rtl8169_private *tp, u32 wolopts)
 	};
 	u8 options;
 
-	RTL_W8(tp, Cfg9346, Cfg9346_Unlock);
+	rtl_unlock_config_regs(tp);
 
 	switch (tp->mac_version) {
 	case RTL_GIGA_MAC_VER_34 ... RTL_GIGA_MAC_VER_38:
@@ -1474,7 +1484,7 @@ static void __rtl8169_set_wol(struct rtl8169_private *tp, u32 wolopts)
 		break;
 	}
 
-	RTL_W8(tp, Cfg9346, Cfg9346_Lock);
+	rtl_lock_config_regs(tp);
 
 	device_set_wakeup_enable(tp_to_dev(tp), wolopts);
 }
@@ -4006,7 +4016,7 @@ static void rtl_rar_set(struct rtl8169_private *tp, u8 *addr)
 {
 	rtl_lock_work(tp);
 
-	RTL_W8(tp, Cfg9346, Cfg9346_Unlock);
+	rtl_unlock_config_regs(tp);
 
 	RTL_W32(tp, MAC4, addr[4] | addr[5] << 8);
 	RTL_R32(tp, MAC4);
@@ -4017,7 +4027,7 @@ static void rtl_rar_set(struct rtl8169_private *tp, u8 *addr)
 	if (tp->mac_version == RTL_GIGA_MAC_VER_34)
 		rtl_rar_exgmac_set(tp, addr);
 
-	RTL_W8(tp, Cfg9346, Cfg9346_Lock);
+	rtl_lock_config_regs(tp);
 
 	rtl_unlock_work(tp);
 }
@@ -4229,18 +4239,18 @@ static void rtl8169_init_ring_indexes(struct rtl8169_private *tp)
 static void rtl_hw_jumbo_enable(struct rtl8169_private *tp)
 {
 	if (tp->jumbo_ops.enable) {
-		RTL_W8(tp, Cfg9346, Cfg9346_Unlock);
+		rtl_unlock_config_regs(tp);
 		tp->jumbo_ops.enable(tp);
-		RTL_W8(tp, Cfg9346, Cfg9346_Lock);
+		rtl_lock_config_regs(tp);
 	}
 }
 
 static void rtl_hw_jumbo_disable(struct rtl8169_private *tp)
 {
 	if (tp->jumbo_ops.disable) {
-		RTL_W8(tp, Cfg9346, Cfg9346_Unlock);
+		rtl_unlock_config_regs(tp);
 		tp->jumbo_ops.disable(tp);
-		RTL_W8(tp, Cfg9346, Cfg9346_Lock);
+		rtl_lock_config_regs(tp);
 	}
 }
 
@@ -4561,13 +4571,13 @@ static void rtl_set_rx_mode(struct net_device *dev)
 
 static void rtl_hw_start(struct  rtl8169_private *tp)
 {
-	RTL_W8(tp, Cfg9346, Cfg9346_Unlock);
+	rtl_unlock_config_regs(tp);
 
 	tp->hw_start(tp);
 
 	rtl_set_rx_max_size(tp);
 	rtl_set_rx_tx_desc_registers(tp);
-	RTL_W8(tp, Cfg9346, Cfg9346_Lock);
+	rtl_lock_config_regs(tp);
 
 	/* Initially a 10 us delay. Turned it into a PCI commit. - FR */
 	RTL_R8(tp, IntrMask);
@@ -6976,9 +6986,9 @@ static int rtl_alloc_irq(struct rtl8169_private *tp)
 	unsigned int flags;
 
 	if (tp->mac_version <= RTL_GIGA_MAC_VER_06) {
-		RTL_W8(tp, Cfg9346, Cfg9346_Unlock);
+		rtl_unlock_config_regs(tp);
 		RTL_W8(tp, Config2, RTL_R8(tp, Config2) & ~MSIEnable);
-		RTL_W8(tp, Cfg9346, Cfg9346_Lock);
+		rtl_lock_config_regs(tp);
 		flags = PCI_IRQ_LEGACY;
 	} else {
 		flags = PCI_IRQ_ALL_TYPES;
-- 
2.20.1

[PATCH net-next v2 6/8] r8169: reset chip synchronously in __rtl8169_resume

[Heiner Kallweit]

Triggering an asynchronous reset is problematic for the following
reasons, therefore reset the chip synchronously.

- The reset routine resets registers and parameters behind our back
  what may collide with code executed after triggering the reset.

- __rtl8169_resume() is called as part of pm_runtime_get_sync() and
  callers expect that the chip is fully resumed afterwards.

In context of this driver triggering an asynchonous reset should be
considered an emergency procedure.

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

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index a3df61d81..c7a89d2a8 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -6787,9 +6787,8 @@ static void __rtl8169_resume(struct net_device *dev)
 	rtl_lock_work(tp);
 	napi_enable(&tp->napi);
 	set_bit(RTL_FLAG_TASK_ENABLED, tp->wk.flags);
+	rtl_reset_work(tp);
 	rtl_unlock_work(tp);
-
-	rtl_schedule_task(tp, RTL_FLAG_TASK_RESET_PENDING);
 }
 
 static int rtl8169_resume(struct device *device)
-- 
2.20.1

[PATCH net-next v2 7/8] r8169: replace mii_bus member with phy_device member in struct rtl8169_private

[Heiner Kallweit]

Accessing the phy_device indirectly via the netdevice causes few issues:
- Accessing the phy_device when it's not attached may cause a NPE.
- If we have to access the phy_device when it's not attached we have
  to use mdiobus_get_phy() to get a reference to the phy_device.

Therefore store a phy_device reference in struct rtl8169_private directly.

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

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index c7a89d2a8..07984f7e3 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -639,6 +639,7 @@ struct rtl8169_private {
 	void __iomem *mmio_addr;	/* memory map physical address */
 	struct pci_dev *pci_dev;
 	struct net_device *dev;
+	struct phy_device *phydev;
 	struct napi_struct napi;
 	u32 msg_enable;
 	u16 mac_version;
@@ -679,7 +680,6 @@ struct rtl8169_private {
 	} wk;
 
 	unsigned supports_gmii:1;
-	struct mii_bus *mii_bus;
 	dma_addr_t counters_phys_addr;
 	struct rtl8169_counters *counters;
 	struct rtl8169_tc_offsets tc_offset;
@@ -1318,7 +1318,7 @@ static void rtl8169_irq_mask_and_ack(struct rtl8169_private *tp)
 static void rtl_link_chg_patch(struct rtl8169_private *tp)
 {
 	struct net_device *dev = tp->dev;
-	struct phy_device *phydev = dev->phydev;
+	struct phy_device *phydev = tp->phydev;
 
 	if (!netif_running(dev))
 		return;
@@ -3999,17 +3999,17 @@ static void rtl8169_init_phy(struct net_device *dev, struct rtl8169_private *tp)
 	}
 
 	/* We may have called phy_speed_down before */
-	phy_speed_up(dev->phydev);
+	phy_speed_up(tp->phydev);
 
-	genphy_soft_reset(dev->phydev);
+	genphy_soft_reset(tp->phydev);
 
 	/* It was reported that several chips end up with 10MBit/Half on a
 	 * 1GBit link after resuming from S3. For whatever reason the PHY on
 	 * these chips doesn't properly start a renegotiation when soft-reset.
 	 * Explicitly requesting a renegotiation fixes this.
 	 */
-	if (dev->phydev->autoneg == AUTONEG_ENABLE)
-		phy_restart_aneg(dev->phydev);
+	if (tp->phydev->autoneg == AUTONEG_ENABLE)
+		phy_restart_aneg(tp->phydev);
 }
 
 static void rtl_rar_set(struct rtl8169_private *tp, u8 *addr)
@@ -4054,10 +4054,12 @@ static int rtl_set_mac_address(struct net_device *dev, void *p)
 
 static int rtl8169_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 {
+	struct rtl8169_private *tp = netdev_priv(dev);
+
 	if (!netif_running(dev))
 		return -ENODEV;
 
-	return phy_mii_ioctl(dev->phydev, ifr, cmd);
+	return phy_mii_ioctl(tp->phydev, ifr, cmd);
 }
 
 static void rtl_init_mdio_ops(struct rtl8169_private *tp)
@@ -4106,15 +4108,10 @@ static void rtl_wol_suspend_quirk(struct rtl8169_private *tp)
 
 static bool rtl_wol_pll_power_down(struct rtl8169_private *tp)
 {
-	struct phy_device *phydev;
-
 	if (!__rtl8169_get_wol(tp))
 		return false;
 
-	/* phydev may not be attached to netdevice */
-	phydev = mdiobus_get_phy(tp->mii_bus, 0);
-
-	phy_speed_down(phydev, false);
+	phy_speed_down(tp->phydev, false);
 	rtl_wol_suspend_quirk(tp);
 
 	return true;
@@ -4183,7 +4180,7 @@ static void r8168_pll_power_up(struct rtl8169_private *tp)
 		break;
 	}
 
-	phy_resume(tp->dev->phydev);
+	phy_resume(tp->phydev);
 	/* give MAC/PHY some time to resume */
 	msleep(20);
 }
@@ -6415,8 +6412,8 @@ static irqreturn_t rtl8169_interrupt(int irq, void *dev_instance)
 		goto out;
 	}
 
-	if (status & LinkChg && tp->dev->phydev)
-		phy_mac_interrupt(tp->dev->phydev);
+	if (status & LinkChg)
+		phy_mac_interrupt(tp->phydev);
 
 	if (unlikely(status & RxFIFOOver &&
 	    tp->mac_version == RTL_GIGA_MAC_VER_11)) {
@@ -6507,12 +6504,12 @@ static void r8169_phylink_handler(struct net_device *ndev)
 	}
 
 	if (net_ratelimit())
-		phy_print_status(ndev->phydev);
+		phy_print_status(tp->phydev);
 }
 
 static int r8169_phy_connect(struct rtl8169_private *tp)
 {
-	struct phy_device *phydev = mdiobus_get_phy(tp->mii_bus, 0);
+	struct phy_device *phydev = tp->phydev;
 	phy_interface_t phy_mode;
 	int ret;
 
@@ -6539,7 +6536,7 @@ static void rtl8169_down(struct net_device *dev)
 {
 	struct rtl8169_private *tp = netdev_priv(dev);
 
-	phy_stop(dev->phydev);
+	phy_stop(tp->phydev);
 
 	napi_disable(&tp->napi);
 	netif_stop_queue(dev);
@@ -6581,7 +6578,7 @@ static int rtl8169_close(struct net_device *dev)
 
 	cancel_work_sync(&tp->wk.work);
 
-	phy_disconnect(dev->phydev);
+	phy_disconnect(tp->phydev);
 
 	pci_free_irq(pdev, 0, tp);
 
@@ -6658,7 +6655,7 @@ static int rtl_open(struct net_device *dev)
 	if (!rtl8169_init_counter_offsets(tp))
 		netif_warn(tp, hw, dev, "counter reset/update failed\n");
 
-	phy_start(dev->phydev);
+	phy_start(tp->phydev);
 	netif_start_queue(dev);
 
 	rtl_unlock_work(tp);
@@ -6747,7 +6744,7 @@ static void rtl8169_net_suspend(struct net_device *dev)
 	if (!netif_running(dev))
 		return;
 
-	phy_stop(dev->phydev);
+	phy_stop(tp->phydev);
 	netif_device_detach(dev);
 
 	rtl_lock_work(tp);
@@ -6782,7 +6779,7 @@ static void __rtl8169_resume(struct net_device *dev)
 	rtl_pll_power_up(tp);
 	rtl8169_init_phy(dev, tp);
 
-	phy_start(tp->dev->phydev);
+	phy_start(tp->phydev);
 
 	rtl_lock_work(tp);
 	napi_enable(&tp->napi);
@@ -6925,7 +6922,7 @@ static void rtl_remove_one(struct pci_dev *pdev)
 	netif_napi_del(&tp->napi);
 
 	unregister_netdev(dev);
-	mdiobus_unregister(tp->mii_bus);
+	mdiobus_unregister(tp->phydev->mdio.bus);
 
 	rtl_release_firmware(tp);
 
@@ -7032,7 +7029,6 @@ static int r8169_mdio_write_reg(struct mii_bus *mii_bus, int phyaddr,
 static int r8169_mdio_register(struct rtl8169_private *tp)
 {
 	struct pci_dev *pdev = tp->pci_dev;
-	struct phy_device *phydev;
 	struct mii_bus *new_bus;
 	int ret;
 
@@ -7054,16 +7050,14 @@ static int r8169_mdio_register(struct rtl8169_private *tp)
 	if (ret)
 		return ret;
 
-	phydev = mdiobus_get_phy(new_bus, 0);
-	if (!phydev) {
+	tp->phydev = mdiobus_get_phy(new_bus, 0);
+	if (!tp->phydev) {
 		mdiobus_unregister(new_bus);
 		return -ENODEV;
 	}
 
 	/* PHY will be woken up in rtl_open() */
-	phy_suspend(phydev);
-
-	tp->mii_bus = new_bus;
+	phy_suspend(tp->phydev);
 
 	return 0;
 }
@@ -7397,7 +7391,7 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	return 0;
 
 err_mdio_unregister:
-	mdiobus_unregister(tp->mii_bus);
+	mdiobus_unregister(tp->phydev->mdio.bus);
 	return rc;
 }
 
-- 
2.20.1

[PATCH net-next v2 8/8] r8169: factor out getting ether_clk

[Heiner Kallweit]

rtl_init_one() is complex enough, so we better factor out getting the
ether_clk.

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

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 07984f7e3..e790a4116 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -7155,6 +7155,32 @@ static void rtl_disable_clk(void *data)
 	clk_disable_unprepare(data);
 }
 
+static int rtl_get_ether_clk(struct rtl8169_private *tp)
+{
+	struct device *d = tp_to_dev(tp);
+	struct clk *clk;
+	int rc;
+
+	clk = devm_clk_get(d, "ether_clk");
+	if (IS_ERR(clk)) {
+		rc = PTR_ERR(clk);
+		if (rc == -ENOENT)
+			/* clk-core allows NULL (for suspend / resume) */
+			rc = 0;
+		else if (rc != -EPROBE_DEFER)
+			dev_err(d, "failed to get clk: %d\n", rc);
+	} else {
+		tp->clk = clk;
+		rc = clk_prepare_enable(clk);
+		if (rc)
+			dev_err(d, "failed to enable clk: %d\n", rc);
+		else
+			rc = devm_add_action_or_reset(d, rtl_disable_clk, clk);
+	}
+
+	return rc;
+}
+
 static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	const struct rtl_cfg_info *cfg = rtl_cfg_infos + ent->driver_data;
@@ -7176,30 +7202,9 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	tp->supports_gmii = cfg->has_gmii;
 
 	/* Get the *optional* external "ether_clk" used on some boards */
-	tp->clk = devm_clk_get(&pdev->dev, "ether_clk");
-	if (IS_ERR(tp->clk)) {
-		rc = PTR_ERR(tp->clk);
-		if (rc == -ENOENT) {
-			/* clk-core allows NULL (for suspend / resume) */
-			tp->clk = NULL;
-		} else if (rc == -EPROBE_DEFER) {
-			return rc;
-		} else {
-			dev_err(&pdev->dev, "failed to get clk: %d\n", rc);
-			return rc;
-		}
-	} else {
-		rc = clk_prepare_enable(tp->clk);
-		if (rc) {
-			dev_err(&pdev->dev, "failed to enable clk: %d\n", rc);
-			return rc;
-		}
-
-		rc = devm_add_action_or_reset(&pdev->dev, rtl_disable_clk,
-					      tp->clk);
-		if (rc)
-			return rc;
-	}
+	rc = rtl_get_ether_clk(tp);
+	if (rc)
+		return rc;
 
 	/* enable device (incl. PCI PM wakeup and hotplug setup) */
 	rc = pcim_enable_device(pdev);
-- 
2.20.1

Re: [PATCH net-next v2 0/8] r8169: series with smaller improvements

[David Miller]

From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Sat, 19 Jan 2019 22:01:57 +0100

> Series with smaller improvements.
> 
> v2:
> - fixed a small copy & paste error in patch 4

Series applied, thanks Heiner.