[PATCH 0/7] Fix an issue that makes r8169 fails to work after

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/7] Fix an issue that makes r8169 fails to work after

Kai-Heng Feng
BugLink: https://bugs.launchpad.net/bugs/1752772

[Impact]
Ethernet r8169 stops working after system resumed from suspend.

[Test]
User confirmed these patches fix the issue. r8169 continues to work
after resume from suspend.

[Fix]
Patch 6/7 is the commit that fixes the issue.
Patch 7/7 fixes 6/7, also includes it.
Patches [1-5]/7 are dependencies to make 6/7 a clean cherry-pick.
Disabling WOL as default should be okay, here's the commit log:
"
Vast majority of users doesn't use WOL even if the BIOS enables it in
the chip. And having WOL being active keeps the PHY(s) from powering
down if being idle.
If somebody needs WOL, he can enable it during boot, e.g. by
configuring systemd.link/WakeOnLan.
"

[Regression Potential]
Medium. The fix is limited to one device, all patches are in mainline.
The WOL default change might cause regression for users that depend on
BIOS settings. We can advice them to use userspace tool (systemd,
ethtool, etc.) instead.

Heiner Kallweit (7):
  PCI: Add pcim_set_mwi(), a device-managed pci_set_mwi()
  r8169: switch to device-managed functions in probe
  r8169: remove netif_napi_del in probe error path
  r8169: remove some WOL-related dead code
  r8169: disable WOL per default
  r8169: improve interrupt handling
  r8169: fix interrupt number after adding support for MSI-X interrupts

 drivers/net/ethernet/realtek/r8169.c | 178 +++++++--------------------
 drivers/pci/pci.c                    |  25 ++++
 include/linux/pci.h                  |   1 +
 3 files changed, 72 insertions(+), 132 deletions(-)

--
2.17.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/7] PCI: Add pcim_set_mwi(), a device-managed pci_set_mwi()

Kai-Heng Feng
From: Heiner Kallweit <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1752772

Add pcim_set_mwi(), a device-managed version of pci_set_mwi().
First user is the Realtek r8169 driver.

Signed-off-by: Heiner Kallweit <[hidden email]>
Acked-by: Bjorn Helgaas <[hidden email]>
Signed-off-by: David S. Miller <[hidden email]>
(cherry picked from commit fc0f9f4d2f26b12fd2eda239bb8f18ceaf192c91)
Signed-off-by: Kai-Heng Feng <[hidden email]>
---
 drivers/pci/pci.c   | 25 +++++++++++++++++++++++++
 include/linux/pci.h |  1 +
 2 files changed, 26 insertions(+)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 2174f8f9da8e..f953042c7cdf 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1459,6 +1459,7 @@ struct pci_devres {
  unsigned int pinned:1;
  unsigned int orig_intx:1;
  unsigned int restore_intx:1;
+ unsigned int mwi:1;
  u32 region_mask;
 };
 
@@ -1477,6 +1478,9 @@ static void pcim_release(struct device *gendev, void *res)
  if (this->region_mask & (1 << i))
  pci_release_region(dev, i);
 
+ if (this->mwi)
+ pci_clear_mwi(dev);
+
  if (this->restore_intx)
  pci_intx(dev, this->orig_intx);
 
@@ -3720,6 +3724,27 @@ int pci_set_mwi(struct pci_dev *dev)
 }
 EXPORT_SYMBOL(pci_set_mwi);
 
+/**
+ * pcim_set_mwi - a device-managed pci_set_mwi()
+ * @dev: the PCI device for which MWI is enabled
+ *
+ * Managed pci_set_mwi().
+ *
+ * RETURNS: An appropriate -ERRNO error value on error, or zero for success.
+ */
+int pcim_set_mwi(struct pci_dev *dev)
+{
+ struct pci_devres *dr;
+
+ dr = find_pci_dr(dev);
+ if (!dr)
+ return -ENOMEM;
+
+ dr->mwi = 1;
+ return pci_set_mwi(dev);
+}
+EXPORT_SYMBOL(pcim_set_mwi);
+
 /**
  * pci_try_set_mwi - enables memory-write-invalidate PCI transaction
  * @dev: the PCI device for which MWI is enabled
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 0962a3a36d62..db81cad70970 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1074,6 +1074,7 @@ int pci_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state);
 int pci_set_cacheline_size(struct pci_dev *dev);
 #define HAVE_PCI_SET_MWI
 int __must_check pci_set_mwi(struct pci_dev *dev);
+int __must_check pcim_set_mwi(struct pci_dev *dev);
 int pci_try_set_mwi(struct pci_dev *dev);
 void pci_clear_mwi(struct pci_dev *dev);
 void pci_intx(struct pci_dev *dev, int enable);
--
2.17.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 2/7] r8169: switch to device-managed functions in probe

Kai-Heng Feng
In reply to this post by Kai-Heng Feng
From: Heiner Kallweit <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1752772

Simplify probe error path and remove callback by using device-managed
functions.

rtl_disable_msi isn't needed any longer because the release callback
of pcim_enable_device does this implicitely.

Signed-off-by: Heiner Kallweit <[hidden email]>
Signed-off-by: David S. Miller <[hidden email]>
(cherry picked from commit 4c45d24a759d8e7257347ef7200f1319db6ab5f8)
Signed-off-by: Kai-Heng Feng <[hidden email]>
---
 drivers/net/ethernet/realtek/r8169.c | 80 +++++++---------------------
 1 file changed, 20 insertions(+), 60 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 3a0c450552d6..958dae66540f 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -4638,16 +4638,6 @@ static void rtl8169_phy_timer(struct timer_list *t)
  rtl_schedule_task(tp, RTL_FLAG_TASK_PHY_PENDING);
 }
 
-static void rtl8169_release_board(struct pci_dev *pdev, struct net_device *dev,
-  void __iomem *ioaddr)
-{
- iounmap(ioaddr);
- pci_release_regions(pdev);
- pci_clear_mwi(pdev);
- pci_disable_device(pdev);
- free_netdev(dev);
-}
-
 DECLARE_RTL_COND(rtl_phy_reset_cond)
 {
  return tp->phy_reset_pending(tp);
@@ -4779,14 +4769,6 @@ static int rtl_tbi_ioctl(struct rtl8169_private *tp, struct mii_ioctl_data *data
  return -EOPNOTSUPP;
 }
 
-static void rtl_disable_msi(struct pci_dev *pdev, struct rtl8169_private *tp)
-{
- if (tp->features & RTL_FEATURE_MSI) {
- pci_disable_msi(pdev);
- tp->features &= ~RTL_FEATURE_MSI;
- }
-}
-
 static void rtl_init_mdio_ops(struct rtl8169_private *tp)
 {
  struct mdio_ops *ops = &tp->mdio_ops;
@@ -8251,9 +8233,6 @@ static void rtl_remove_one(struct pci_dev *pdev)
 
  unregister_netdev(dev);
 
- dma_free_coherent(&tp->pci_dev->dev, sizeof(*tp->counters),
-  tp->counters, tp->counters_phys_addr);
-
  rtl_release_firmware(tp);
 
  if (pci_dev_run_wake(pdev))
@@ -8261,9 +8240,6 @@ static void rtl_remove_one(struct pci_dev *pdev)
 
  /* restore original MAC address */
  rtl_rar_set(tp, dev->perm_addr);
-
- rtl_disable_msi(pdev, tp);
- rtl8169_release_board(pdev, dev, tp->mmio_addr);
 }
 
 static const struct net_device_ops rtl_netdev_ops = {
@@ -8440,11 +8416,9 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
        MODULENAME, RTL8169_VERSION);
  }
 
- dev = alloc_etherdev(sizeof (*tp));
- if (!dev) {
- rc = -ENOMEM;
- goto out;
- }
+ dev = devm_alloc_etherdev(&pdev->dev, sizeof (*tp));
+ if (!dev)
+ return -ENOMEM;
 
  SET_NETDEV_DEV(dev, &pdev->dev);
  dev->netdev_ops = &rtl_netdev_ops;
@@ -8467,13 +8441,13 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
      PCIE_LINK_STATE_CLKPM);
 
  /* enable device (incl. PCI PM wakeup and hotplug setup) */
- rc = pci_enable_device(pdev);
+ rc = pcim_enable_device(pdev);
  if (rc < 0) {
  netif_err(tp, probe, dev, "enable failure\n");
- goto err_out_free_dev_1;
+ return rc;
  }
 
- if (pci_set_mwi(pdev) < 0)
+ if (pcim_set_mwi(pdev) < 0)
  netif_info(tp, probe, dev, "Mem-Wr-Inval unavailable\n");
 
  /* make sure PCI base addr 1 is MMIO */
@@ -8481,30 +8455,28 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
  netif_err(tp, probe, dev,
   "region #%d not an MMIO resource, aborting\n",
   region);
- rc = -ENODEV;
- goto err_out_mwi_2;
+ return -ENODEV;
  }
 
  /* check for weird/broken PCI region reporting */
  if (pci_resource_len(pdev, region) < R8169_REGS_SIZE) {
  netif_err(tp, probe, dev,
   "Invalid PCI region size(s), aborting\n");
- rc = -ENODEV;
- goto err_out_mwi_2;
+ return -ENODEV;
  }
 
  rc = pci_request_regions(pdev, MODULENAME);
  if (rc < 0) {
  netif_err(tp, probe, dev, "could not request regions\n");
- goto err_out_mwi_2;
+ return rc;
  }
 
  /* ioremap MMIO region */
- ioaddr = ioremap(pci_resource_start(pdev, region), R8169_REGS_SIZE);
+ ioaddr = devm_ioremap(&pdev->dev, pci_resource_start(pdev, region),
+      R8169_REGS_SIZE);
  if (!ioaddr) {
  netif_err(tp, probe, dev, "cannot remap MMIO, aborting\n");
- rc = -EIO;
- goto err_out_free_res_3;
+ return -EIO;
  }
  tp->mmio_addr = ioaddr;
 
@@ -8530,7 +8502,7 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
  rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
  if (rc < 0) {
  netif_err(tp, probe, dev, "DMA configuration failed\n");
- goto err_out_unmap_4;
+ return rc;
  }
  }
 
@@ -8692,8 +8664,9 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
  tp->rtl_fw = RTL_FIRMWARE_UNKNOWN;
 
- tp->counters = dma_alloc_coherent (&pdev->dev, sizeof(*tp->counters),
-   &tp->counters_phys_addr, GFP_KERNEL);
+ tp->counters = dmam_alloc_coherent (&pdev->dev, sizeof(*tp->counters),
+    &tp->counters_phys_addr,
+    GFP_KERNEL);
  if (!tp->counters) {
  rc = -ENOMEM;
  goto err_out_msi_5;
@@ -8703,7 +8676,7 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
  rc = register_netdev(dev);
  if (rc < 0)
- goto err_out_cnt_6;
+ goto err_out_msi_5;
 
  netif_info(tp, probe, dev, "%s at 0x%p, %pM, XID %08x IRQ %d\n",
    rtl_chip_infos[chipset].name, ioaddr, dev->dev_addr,
@@ -8730,25 +8703,12 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
  netif_carrier_off(dev);
 
-out:
- return rc;
+ return 0;
 
-err_out_cnt_6:
- dma_free_coherent(&pdev->dev, sizeof(*tp->counters), tp->counters,
-  tp->counters_phys_addr);
 err_out_msi_5:
  netif_napi_del(&tp->napi);
- rtl_disable_msi(pdev, tp);
-err_out_unmap_4:
- iounmap(ioaddr);
-err_out_free_res_3:
- pci_release_regions(pdev);
-err_out_mwi_2:
- pci_clear_mwi(pdev);
- pci_disable_device(pdev);
-err_out_free_dev_1:
- free_netdev(dev);
- goto out;
+
+ return rc;
 }
 
 static struct pci_driver rtl8169_pci_driver = {
--
2.17.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 3/7] r8169: remove netif_napi_del in probe error path

Kai-Heng Feng
In reply to this post by Kai-Heng Feng
From: Heiner Kallweit <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1752772

netif_napi_del is called implicitely by free_netdev, therefore we
don't have to do it explicitely.

When the probe error path is reached, the net_device isn't
registered yet. Therefore reordering the call to netif_napi_del
shouldn't cause any issues.

Signed-off-by: Heiner Kallweit <[hidden email]>
Signed-off-by: David S. Miller <[hidden email]>
(cherry picked from commit 4cf964af0a7238f9d9dbebf4ec70a415ff5164e5)
Signed-off-by: Kai-Heng Feng <[hidden email]>
---
 drivers/net/ethernet/realtek/r8169.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 958dae66540f..123cc4f58c56 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -8667,16 +8667,14 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
  tp->counters = dmam_alloc_coherent (&pdev->dev, sizeof(*tp->counters),
     &tp->counters_phys_addr,
     GFP_KERNEL);
- if (!tp->counters) {
- rc = -ENOMEM;
- goto err_out_msi_5;
- }
+ if (!tp->counters)
+ return -ENOMEM;
 
  pci_set_drvdata(pdev, dev);
 
  rc = register_netdev(dev);
  if (rc < 0)
- goto err_out_msi_5;
+ return rc;
 
  netif_info(tp, probe, dev, "%s at 0x%p, %pM, XID %08x IRQ %d\n",
    rtl_chip_infos[chipset].name, ioaddr, dev->dev_addr,
@@ -8704,11 +8702,6 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
  netif_carrier_off(dev);
 
  return 0;
-
-err_out_msi_5:
- netif_napi_del(&tp->napi);
-
- return rc;
 }
 
 static struct pci_driver rtl8169_pci_driver = {
--
2.17.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 4/7] r8169: remove some WOL-related dead code

Kai-Heng Feng
In reply to this post by Kai-Heng Feng
From: Heiner Kallweit <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1752772

Commit bde135a672bf "r8169: only enable PCI wakeups when WOL is active"
removed the only user of flag RTL_FEATURE_WOL. So let's remove some
now dead code.

Signed-off-by: Heiner Kallweit <[hidden email]>
Signed-off-by: David S. Miller <[hidden email]>
(cherry picked from commit 022ddbca86ce692518bc1809e2dfe27add669608)
Signed-off-by: Kai-Heng Feng <[hidden email]>
---
 drivers/net/ethernet/realtek/r8169.c | 39 ++--------------------------
 1 file changed, 2 insertions(+), 37 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 123cc4f58c56..c89802135b61 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -736,9 +736,8 @@ struct ring_info {
 };
 
 enum features {
- RTL_FEATURE_WOL = (1 << 0),
- RTL_FEATURE_MSI = (1 << 1),
- RTL_FEATURE_GMII = (1 << 2),
+ RTL_FEATURE_MSI = (1 << 0),
+ RTL_FEATURE_GMII = (1 << 1),
 };
 
 struct rtl8169_counters {
@@ -1868,10 +1867,6 @@ static int rtl8169_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
 
  rtl_lock_work(tp);
 
- if (wol->wolopts)
- tp->features |= RTL_FEATURE_WOL;
- else
- tp->features &= ~RTL_FEATURE_WOL;
  if (pm_runtime_active(d))
  __rtl8169_set_wol(tp, wol->wolopts);
  else
@@ -8531,36 +8526,6 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
  RTL_W8(Cfg9346, Cfg9346_Unlock);
  RTL_W8(Config1, RTL_R8(Config1) | PMEnable);
  RTL_W8(Config5, RTL_R8(Config5) & (BWF | MWF | UWF | LanWake | PMEStatus));
- switch (tp->mac_version) {
- case RTL_GIGA_MAC_VER_34:
- case RTL_GIGA_MAC_VER_35:
- case RTL_GIGA_MAC_VER_36:
- case RTL_GIGA_MAC_VER_37:
- case RTL_GIGA_MAC_VER_38:
- case RTL_GIGA_MAC_VER_40:
- case RTL_GIGA_MAC_VER_41:
- case RTL_GIGA_MAC_VER_42:
- case RTL_GIGA_MAC_VER_43:
- case RTL_GIGA_MAC_VER_44:
- case RTL_GIGA_MAC_VER_45:
- case RTL_GIGA_MAC_VER_46:
- case RTL_GIGA_MAC_VER_47:
- case RTL_GIGA_MAC_VER_48:
- case RTL_GIGA_MAC_VER_49:
- case RTL_GIGA_MAC_VER_50:
- case RTL_GIGA_MAC_VER_51:
- if (rtl_eri_read(tp, 0xdc, ERIAR_EXGMAC) & MagicPacket_v2)
- tp->features |= RTL_FEATURE_WOL;
- if ((RTL_R8(Config3) & LinkUp) != 0)
- tp->features |= RTL_FEATURE_WOL;
- break;
- default:
- if ((RTL_R8(Config3) & (LinkUp | MagicPacket)) != 0)
- tp->features |= RTL_FEATURE_WOL;
- break;
- }
- if ((RTL_R8(Config5) & (UWF | BWF | MWF)) != 0)
- tp->features |= RTL_FEATURE_WOL;
  tp->features |= rtl_try_msi(tp, cfg);
  RTL_W8(Cfg9346, Cfg9346_Lock);
 
--
2.17.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 5/7] r8169: disable WOL per default

Kai-Heng Feng
In reply to this post by Kai-Heng Feng
From: Heiner Kallweit <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1752772

Currently, if BIOS enables WOL in the chip, settings are inconsistent
because the device isn't marked as wakeup-enabled (if not done
explicitly via userspace tools). This causes issues with suspend/
resume because mdio_bus_phy_may_suspend() checks whether device is
wakeup-enabled. In detail MDIO bus access in phy_suspend() can fail
because the MDIO bus is disabled.

In the history of the driver we find two competing approaches:
8f9d5138035d "r8169: remember WOL preferences on driver load" prefers
to preserve what the BIOS may have set, whilst bde135a672bf
"r8169: only enable PCI wakeups when WOL is active" disabled PCI
wakeup per default to work around a bug on one platform.

Seems like nobody complained after the latter patch about non-working
WOL, what makes me think that nobody uses WOL w/o configuring it
explicitly.

My opinion:
Vast majority of users doesn't use WOL even if the BIOS enables it in
the chip. And having WOL being active keeps the PHY(s) from powering
down if being idle.
If somebody needs WOL, he can enable it during boot, e.g. by
configuring systemd.link/WakeOnLan.

Therefore, to make WOL consistent again, disable it per default.

Signed-off-by: Heiner Kallweit <[hidden email]>
Signed-off-by: David S. Miller <[hidden email]>
(cherry picked from commit 7edf6d314cd061e1d0a1b7bc0b511d64322c3f72)
Signed-off-by: Kai-Heng Feng <[hidden email]>
---
 drivers/net/ethernet/realtek/r8169.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index c89802135b61..05c0f9e641e9 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -8524,11 +8524,12 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
  tp->txd_version = rtl_chip_infos[chipset].txd_version;
 
  RTL_W8(Cfg9346, Cfg9346_Unlock);
- RTL_W8(Config1, RTL_R8(Config1) | PMEnable);
- RTL_W8(Config5, RTL_R8(Config5) & (BWF | MWF | UWF | LanWake | PMEStatus));
  tp->features |= rtl_try_msi(tp, cfg);
  RTL_W8(Cfg9346, Cfg9346_Lock);
 
+ /* override BIOS settings, use userspace tools to enable WOL */
+ __rtl8169_set_wol(tp, 0);
+
  if (rtl_tbi_enabled(tp)) {
  tp->set_speed = rtl8169_set_speed_tbi;
  tp->get_link_ksettings = rtl8169_get_link_ksettings_tbi;
--
2.17.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 6/7] r8169: improve interrupt handling

Kai-Heng Feng
In reply to this post by Kai-Heng Feng
From: Heiner Kallweit <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1752772

This patch improves few aspects of interrupt handling:
- update to current interrupt allocation API
  (use pci_alloc_irq_vectors() instead of deprecated pci_enable_msi())
- this implicitly will allocate a MSI-X interrupt if available
- get rid of flag RTL_FEATURE_MSI
- remove some dead code, intentionally disabling (unreliable) MSI
  being partially available on old PCI chips.

The patch works fine on a RTL8168evl (chip version 34) and on a
RTL8169SB (chip version 04).

Signed-off-by: Heiner Kallweit <[hidden email]>
Signed-off-by: David S. Miller <[hidden email]>
(cherry picked from commit 6c6aa15fdea52aa4a19cc0b5478884af591c9351)
Signed-off-by: Kai-Heng Feng <[hidden email]>
---
 drivers/net/ethernet/realtek/r8169.c | 48 ++++++++++++----------------
 1 file changed, 21 insertions(+), 27 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 05c0f9e641e9..2bcc3f05ce33 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -736,8 +736,7 @@ struct ring_info {
 };
 
 enum features {
- RTL_FEATURE_MSI = (1 << 0),
- RTL_FEATURE_GMII = (1 << 1),
+ RTL_FEATURE_GMII = (1 << 0),
 };
 
 struct rtl8169_counters {
@@ -7865,7 +7864,7 @@ static int rtl8169_close(struct net_device *dev)
 
  cancel_work_sync(&tp->wk.work);
 
- free_irq(pdev->irq, dev);
+ pci_free_irq(pdev, 0, dev);
 
  dma_free_coherent(&pdev->dev, R8169_RX_RING_BYTES, tp->RxDescArray,
   tp->RxPhyAddr);
@@ -7921,9 +7920,8 @@ static int rtl_open(struct net_device *dev)
 
  rtl_request_firmware(tp);
 
- retval = request_irq(pdev->irq, rtl8169_interrupt,
-     (tp->features & RTL_FEATURE_MSI) ? 0 : IRQF_SHARED,
-     dev->name, dev);
+ retval = pci_request_irq(pdev, 0, rtl8169_interrupt, NULL, dev,
+ dev->name);
  if (retval < 0)
  goto err_release_fw_2;
 
@@ -8279,7 +8277,7 @@ static const struct rtl_cfg_info {
  .region = 2,
  .align = 8,
  .event_slow = SYSErr | LinkChg | RxOverflow,
- .features = RTL_FEATURE_GMII | RTL_FEATURE_MSI,
+ .features = RTL_FEATURE_GMII,
  .coalesce_info = rtl_coalesce_info_8168_8136,
  .default_ver = RTL_GIGA_MAC_VER_11,
  },
@@ -8289,32 +8287,26 @@ static const struct rtl_cfg_info {
  .align = 8,
  .event_slow = SYSErr | LinkChg | RxOverflow | RxFIFOOver |
   PCSTimeout,
- .features = RTL_FEATURE_MSI,
  .coalesce_info = rtl_coalesce_info_8168_8136,
  .default_ver = RTL_GIGA_MAC_VER_13,
  }
 };
 
-/* Cfg9346_Unlock assumed. */
-static unsigned rtl_try_msi(struct rtl8169_private *tp,
-    const struct rtl_cfg_info *cfg)
+static int rtl_alloc_irq(struct rtl8169_private *tp)
 {
  void __iomem *ioaddr = tp->mmio_addr;
- unsigned msi = 0;
- u8 cfg2;
+ unsigned int flags;
 
- cfg2 = RTL_R8(Config2) & ~MSIEnable;
- if (cfg->features & RTL_FEATURE_MSI) {
- if (pci_enable_msi(tp->pci_dev)) {
- netif_info(tp, hw, tp->dev, "no MSI. Back to INTx.\n");
- } else {
- cfg2 |= MSIEnable;
- msi = RTL_FEATURE_MSI;
- }
+ if (tp->mac_version <= RTL_GIGA_MAC_VER_06) {
+ RTL_W8(Cfg9346, Cfg9346_Unlock);
+ RTL_W8(Config2, RTL_R8(Config2) & ~MSIEnable);
+ RTL_W8(Cfg9346, Cfg9346_Lock);
+ flags = PCI_IRQ_LEGACY;
+ } else {
+ flags = PCI_IRQ_ALL_TYPES;
  }
- if (tp->mac_version <= RTL_GIGA_MAC_VER_06)
- RTL_W8(Config2, cfg2);
- return msi;
+
+ return pci_alloc_irq_vectors(tp->pci_dev, 1, 1, flags);
 }
 
 DECLARE_RTL_COND(rtl_link_list_ready_cond)
@@ -8523,9 +8515,11 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
  chipset = tp->mac_version;
  tp->txd_version = rtl_chip_infos[chipset].txd_version;
 
- RTL_W8(Cfg9346, Cfg9346_Unlock);
- tp->features |= rtl_try_msi(tp, cfg);
- RTL_W8(Cfg9346, Cfg9346_Lock);
+ rc = rtl_alloc_irq(tp);
+ if (rc < 0) {
+ netif_err(tp, probe, dev, "Can't allocate interrupt\n");
+ return rc;
+ }
 
  /* override BIOS settings, use userspace tools to enable WOL */
  __rtl8169_set_wol(tp, 0);
--
2.17.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 7/7] r8169: fix interrupt number after adding support for MSI-X interrupts

Kai-Heng Feng
In reply to this post by Kai-Heng Feng
From: Heiner Kallweit <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1752772

In case of MSI-X the interrupt number may differ from pcidev->irq.
Fix this by using pci_irq_vector().

Fixes: 6c6aa15fdea5 ("r8169: improve interrupt handling")
Signed-off-by: Heiner Kallweit <[hidden email]>
Signed-off-by: David S. Miller <[hidden email]>
(cherry picked from commit 292749915743758013e290c994f41de196d47498)
Signed-off-by: Kai-Heng Feng <[hidden email]>
---
 drivers/net/ethernet/realtek/r8169.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 2bcc3f05ce33..395aaa6fb940 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -7883,7 +7883,7 @@ static void rtl8169_netpoll(struct net_device *dev)
 {
  struct rtl8169_private *tp = netdev_priv(dev);
 
- rtl8169_interrupt(tp->pci_dev->irq, dev);
+ rtl8169_interrupt(pci_irq_vector(tp->pci_dev, 0), dev);
 }
 #endif
 
@@ -8638,7 +8638,8 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
  netif_info(tp, probe, dev, "%s at 0x%p, %pM, XID %08x IRQ %d\n",
    rtl_chip_infos[chipset].name, ioaddr, dev->dev_addr,
-   (u32)(RTL_R32(TxConfig) & 0x9cf0f8ff), pdev->irq);
+   (u32)(RTL_R32(TxConfig) & 0x9cf0f8ff),
+   pci_irq_vector(pdev, 0));
  if (rtl_chip_infos[chipset].jumbo_max != JUMBO_1K) {
  netif_info(tp, probe, dev, "jumbo features [frames: %d bytes, "
    "tx checksumming: %s]\n",
--
2.17.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/7] Fix an issue that makes r8169 fails to work after

Stefan Bader-2
In reply to this post by Kai-Heng Feng
On 23.05.2018 21:35, Kai-Heng Feng wrote:

> BugLink: https://bugs.launchpad.net/bugs/1752772
>
> [Impact]
> Ethernet r8169 stops working after system resumed from suspend.
>
> [Test]
> User confirmed these patches fix the issue. r8169 continues to work
> after resume from suspend.
>
> [Fix]
> Patch 6/7 is the commit that fixes the issue.
> Patch 7/7 fixes 6/7, also includes it.
> Patches [1-5]/7 are dependencies to make 6/7 a clean cherry-pick.
> Disabling WOL as default should be okay, here's the commit log:
> "
> Vast majority of users doesn't use WOL even if the BIOS enables it in
> the chip. And having WOL being active keeps the PHY(s) from powering
> down if being idle.
> If somebody needs WOL, he can enable it during boot, e.g. by
> configuring systemd.link/WakeOnLan.
> "
>
> [Regression Potential]
> Medium. The fix is limited to one device, all patches are in mainline.
> The WOL default change might cause regression for users that depend on
> BIOS settings. We can advice them to use userspace tool (systemd,
> ethtool, etc.) instead.
>
> Heiner Kallweit (7):
>   PCI: Add pcim_set_mwi(), a device-managed pci_set_mwi()
>   r8169: switch to device-managed functions in probe
>   r8169: remove netif_napi_del in probe error path
>   r8169: remove some WOL-related dead code
>   r8169: disable WOL per default
>   r8169: improve interrupt handling
>   r8169: fix interrupt number after adding support for MSI-X interrupts
>
>  drivers/net/ethernet/realtek/r8169.c | 178 +++++++--------------------
>  drivers/pci/pci.c                    |  25 ++++
>  include/linux/pci.h                  |   1 +
>  3 files changed, 72 insertions(+), 132 deletions(-)
>
Which kernel/series does this set target? Neither the submission nor the bug
report clearly say this.

-Stefan


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/7] Fix an issue that makes r8169 fails to work after

Kai-Heng Feng

> On Jun 5, 2018, at 6:03 AM, Stefan Bader <[hidden email]>  
> wrote:
>
> On 23.05.2018 21:35, Kai-Heng Feng wrote:
>> BugLink: https://bugs.launchpad.net/bugs/1752772
>>
>> [Impact]
>> Ethernet r8169 stops working after system resumed from suspend.
>>
>> [Test]
>> User confirmed these patches fix the issue. r8169 continues to work
>> after resume from suspend.
>>
>> [Fix]
>> Patch 6/7 is the commit that fixes the issue.
>> Patch 7/7 fixes 6/7, also includes it.
>> Patches [1-5]/7 are dependencies to make 6/7 a clean cherry-pick.
>> Disabling WOL as default should be okay, here's the commit log:
>> "
>> Vast majority of users doesn't use WOL even if the BIOS enables it in
>> the chip. And having WOL being active keeps the PHY(s) from powering
>> down if being idle.
>> If somebody needs WOL, he can enable it during boot, e.g. by
>> configuring systemd.link/WakeOnLan.
>> "
>>
>> [Regression Potential]
>> Medium. The fix is limited to one device, all patches are in mainline.
>> The WOL default change might cause regression for users that depend on
>> BIOS settings. We can advice them to use userspace tool (systemd,
>> ethtool, etc.) instead.
>>
>> Heiner Kallweit (7):
>>   PCI: Add pcim_set_mwi(), a device-managed pci_set_mwi()
>>   r8169: switch to device-managed functions in probe
>>   r8169: remove netif_napi_del in probe error path
>>   r8169: remove some WOL-related dead code
>>   r8169: disable WOL per default
>>   r8169: improve interrupt handling
>>   r8169: fix interrupt number after adding support for MSI-X interrupts
>>
>>  drivers/net/ethernet/realtek/r8169.c | 178 +++++++--------------------
>>  drivers/pci/pci.c                    |  25 ++++
>>  include/linux/pci.h                  |   1 +
>>  3 files changed, 72 insertions(+), 132 deletions(-)
> Which kernel/series does this set target? Neither the submission nor the  
> bug
> report clearly say this.

Sorry I missed that.
This is for Bionic.
Should I send a V2 for this?

Kai-Heng

>
> -Stefan

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK[Bionic]: [PATCH 0/7] Fix an issue that makes r8169 fails to work after

Kleber Sacilotto de Souza
In reply to this post by Kai-Heng Feng
On 05/23/18 21:35, Kai-Heng Feng wrote:

> BugLink: https://bugs.launchpad.net/bugs/1752772
>
> [Impact]
> Ethernet r8169 stops working after system resumed from suspend.
>
> [Test]
> User confirmed these patches fix the issue. r8169 continues to work
> after resume from suspend.
>
> [Fix]
> Patch 6/7 is the commit that fixes the issue.
> Patch 7/7 fixes 6/7, also includes it.
> Patches [1-5]/7 are dependencies to make 6/7 a clean cherry-pick.
> Disabling WOL as default should be okay, here's the commit log:
> "
> Vast majority of users doesn't use WOL even if the BIOS enables it in
> the chip. And having WOL being active keeps the PHY(s) from powering
> down if being idle.
> If somebody needs WOL, he can enable it during boot, e.g. by
> configuring systemd.link/WakeOnLan.
> "
>
> [Regression Potential]
> Medium. The fix is limited to one device, all patches are in mainline.
> The WOL default change might cause regression for users that depend on
> BIOS settings. We can advice them to use userspace tool (systemd,
> ethtool, etc.) instead.
>
> Heiner Kallweit (7):
>   PCI: Add pcim_set_mwi(), a device-managed pci_set_mwi()
>   r8169: switch to device-managed functions in probe
>   r8169: remove netif_napi_del in probe error path
>   r8169: remove some WOL-related dead code
>   r8169: disable WOL per default
>   r8169: improve interrupt handling
>   r8169: fix interrupt number after adding support for MSI-X interrupts
>
>  drivers/net/ethernet/realtek/r8169.c | 178 +++++++--------------------
>  drivers/pci/pci.c                    |  25 ++++
>  include/linux/pci.h                  |   1 +
>  3 files changed, 72 insertions(+), 132 deletions(-)
>

It's a relatively large number of patches as prereq, but they seem
reasonable.

I have added the nomination for Bionic on the Launchpad bug and no
changes are needed on the patches themselves, so no need for a
re-submission.

So for Bionic:

Acked-by: Kleber Sacilotto de Souza <[hidden email]>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [PATCH 0/7] Fix an issue that makes r8169 fails to work after

Stefan Bader-2
In reply to this post by Kai-Heng Feng
On 23.05.2018 21:35, Kai-Heng Feng wrote:

> BugLink: https://bugs.launchpad.net/bugs/1752772
>
> [Impact]
> Ethernet r8169 stops working after system resumed from suspend.
>
> [Test]
> User confirmed these patches fix the issue. r8169 continues to work
> after resume from suspend.
>
> [Fix]
> Patch 6/7 is the commit that fixes the issue.
> Patch 7/7 fixes 6/7, also includes it.
> Patches [1-5]/7 are dependencies to make 6/7 a clean cherry-pick.
> Disabling WOL as default should be okay, here's the commit log:
> "
> Vast majority of users doesn't use WOL even if the BIOS enables it in
> the chip. And having WOL being active keeps the PHY(s) from powering
> down if being idle.
> If somebody needs WOL, he can enable it during boot, e.g. by
> configuring systemd.link/WakeOnLan.
> "
>
> [Regression Potential]
> Medium. The fix is limited to one device, all patches are in mainline.
> The WOL default change might cause regression for users that depend on
> BIOS settings. We can advice them to use userspace tool (systemd,
> ethtool, etc.) instead.
>
> Heiner Kallweit (7):
>   PCI: Add pcim_set_mwi(), a device-managed pci_set_mwi()
>   r8169: switch to device-managed functions in probe
>   r8169: remove netif_napi_del in probe error path
>   r8169: remove some WOL-related dead code
>   r8169: disable WOL per default
>   r8169: improve interrupt handling
>   r8169: fix interrupt number after adding support for MSI-X interrupts
>
>  drivers/net/ethernet/realtek/r8169.c | 178 +++++++--------------------
>  drivers/pci/pci.c                    |  25 ++++
>  include/linux/pci.h                  |   1 +
>  3 files changed, 72 insertions(+), 132 deletions(-)
>
Acked-by: Stefan Bader <[hidden email]>



--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

APPLIED: [PATCH 0/7] Fix an issue that makes r8169 fails to work after

Khaled Elmously
In reply to this post by Kai-Heng Feng
Applied to Bionic


On 2018-05-24 12:35:13 , Kai-Heng Feng wrote:

> BugLink: https://bugs.launchpad.net/bugs/1752772
>
> [Impact]
> Ethernet r8169 stops working after system resumed from suspend.
>
> [Test]
> User confirmed these patches fix the issue. r8169 continues to work
> after resume from suspend.
>
> [Fix]
> Patch 6/7 is the commit that fixes the issue.
> Patch 7/7 fixes 6/7, also includes it.
> Patches [1-5]/7 are dependencies to make 6/7 a clean cherry-pick.
> Disabling WOL as default should be okay, here's the commit log:
> "
> Vast majority of users doesn't use WOL even if the BIOS enables it in
> the chip. And having WOL being active keeps the PHY(s) from powering
> down if being idle.
> If somebody needs WOL, he can enable it during boot, e.g. by
> configuring systemd.link/WakeOnLan.
> "
>
> [Regression Potential]
> Medium. The fix is limited to one device, all patches are in mainline.
> The WOL default change might cause regression for users that depend on
> BIOS settings. We can advice them to use userspace tool (systemd,
> ethtool, etc.) instead.
>
> Heiner Kallweit (7):
>   PCI: Add pcim_set_mwi(), a device-managed pci_set_mwi()
>   r8169: switch to device-managed functions in probe
>   r8169: remove netif_napi_del in probe error path
>   r8169: remove some WOL-related dead code
>   r8169: disable WOL per default
>   r8169: improve interrupt handling
>   r8169: fix interrupt number after adding support for MSI-X interrupts
>
>  drivers/net/ethernet/realtek/r8169.c | 178 +++++++--------------------
>  drivers/pci/pci.c                    |  25 ++++
>  include/linux/pci.h                  |   1 +
>  3 files changed, 72 insertions(+), 132 deletions(-)
>
> --
> 2.17.0
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team