Re: [PATCH net-next] r8169: improve interrupt handling

Am 24.02.2018 um 02:35 schrieb Francois Romieu:

Heiner Kallweit [off-list ref] :
[...]

quoted

Last but not least it enables a feature which was (I presume accidently)
disabled before. There are members of the RTL8169 family supporting MSI
(e.g. RTL8169SB), however MSI never got enabled because RTL_CFG_0 was
missing flag RTL_FEATURE_MSI.
An indicator for "accidently" is that statement "cfg2 |= MSIEnable;"

The reality is more simple: it could had been removed.

quoted

in rtl_try_msi() is dead code. cfg2 is used for chip versions <= 06
only and for all these chip versions RTL_FEATURE_MSI isn't set.

On purpose:
1. mostly untested
2. MSI without MSI-X does not buy much
3. wrt 2., ok, it kills (yucky) plain old shared PCI irq (remember those ?)
   but I didn't end feeling that good about realtek MSI support on older
   chipsets to enable it any further

Good to know, thanks for the feedback.
Then I'll change the patch to leave MSI disabled on old PCI chips.
Plus addressing your last comment.

[...]

quoted

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 96db3283e..4730db990 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c

[...]

quoted

-static unsigned rtl_try_msi(struct rtl8169_private *tp,
-			    const struct rtl_cfg_info *cfg)
+static void rtl_alloc_irq(struct rtl8169_private *tp)
 {

[...]

quoted

+	ret = pci_alloc_irq_vectors(tp->pci_dev, 1, 1, PCI_IRQ_ALL_TYPES);
+	if (ret < 0) {
+		netif_err(tp, drv, tp->dev, "failed to allocate irq!\n");
+		return;

[...]

quoted

@@ -8497,9 +8495,7 @@ 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);
+	rtl_alloc_irq(tp);

Happily proceeding after error. :o/