[PATCH 1/1] r8169.c correct MSIEnable register offset

correct MSIEnable (bit 5) register to Config1 (offset 0x52) instead of
Config2 (offset 0x53)

Signed-off-by: Su Kang Yin <redacted>
---
 drivers/net/ethernet/realtek/r8169.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c

b/drivers/net/ethernet/realtek/r8169.c
index 67bf078..451835c 100644

--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c

@@ -3430,18 +3430,18 @@ static unsigned rtl_try_msi(struct pci_dev

*pdev, void __iomem *ioaddr,
 			    const struct rtl_cfg_info *cfg)
 {
 	unsigned msi = 0;
-	u8 cfg2;
+	u8 cfg1;

-	cfg2 = RTL_R8(Config2) & ~MSIEnable;
+	cfg1 = RTL_R8(Config1) & ~MSIEnable;
 	if (cfg->features & RTL_FEATURE_MSI) {
 		if (pci_enable_msi(pdev)) {
 			dev_info(&pdev->dev, "no MSI. Back to INTx.\n");
 		} else {
-			cfg2 |= MSIEnable;
+			cfg1 |= MSIEnable;
 			msi = RTL_FEATURE_MSI;
 		}
 	}
-	RTL_W8(Config2, cfg2);
+	RTL_W8(Config1, cfg1);
 	return msi;
 }

-- 
1.7.0.4

Su Kang Yin [off-list ref] :

correct MSIEnable (bit 5) register to Config1 (offset 0x52) instead of
Config2 (offset 0x53)

I wonder where the inspiration for the MSIEnable bit came from.
It looks like something was confused with the Message Control word
in PCI space.

Imho you can simply remove it altogether.

-- 
Ueimor

From: Francois Romieu <romieu@fr.zoreil.com>
Date: Wed, 14 Dec 2011 22:37:13 +0100

Su Kang Yin [off-list ref] :

quoted

correct MSIEnable (bit 5) register to Config1 (offset 0x52) instead of
Config2 (offset 0x53)

I wonder where the inspiration for the MSIEnable bit came from.
It looks like something was confused with the Message Control word
in PCI space.

Imho you can simply remove it altogether.

Someone should find out what the real situation is with this.

Maybe it mirrors the PCI config space setting and is read-only, maybe
not.  But it should be determined for sure before changing this. :-)

-----Original Message-----
From: David Miller [mailto:davem@davemloft.net] 
Sent: Thursday, December 15, 2011 2:44 PM
To: romieu@fr.zoreil.com
Cc: cantona@cantona.no-ip.org; Hayeswang; 
linux-kernel@vger.kernel.org; nic_swsd; netdev@vger.kernel.org
Subject: Re: [PATCH 1/1] r8169.c correct MSIEnable register offset

From: Francois Romieu <romieu@fr.zoreil.com>
Date: Wed, 14 Dec 2011 22:37:13 +0100

quoted

Su Kang Yin [off-list ref] :

quoted

correct MSIEnable (bit 5) register to Config1 (offset 

0x52) instead of

quoted

quoted

Config2 (offset 0x53)

The bit 5 of config1 (0x52) is reserved. And the bit 5 of Config2 (0x53) is
MSIEnable only for 8169 controler series.

quoted

I wonder where the inspiration for the MSIEnable bit came from.
It looks like something was confused with the Message Control word
in PCI space.

Imho you can simply remove it altogether.

Someone should find out what the real situation is with this.

Maybe it mirrors the PCI config space setting and is read-only, maybe
not.  But it should be determined for sure before changing this. :-)

The MSIEnable bit is only available for the 8169.

Avoid Config2 writes for the post-8169 8168 and 810x.

Reported-by: Su Kang Yin <redacted>
Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
Cc: Hayes Wang <redacted>
---
 RTL_FEATURE_MSI has never been enabled for the 8169 itself. It could
 be -next material.

 drivers/net/ethernet/realtek/r8169.c |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 67bf078..c8f47f1 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c

@@ -477,7 +477,6 @@ enum rtl_register_content {
 	/* Config1 register p.24 */
 	LEDS1		= (1 << 7),
 	LEDS0		= (1 << 6),
-	MSIEnable	= (1 << 5),	/* Enable Message Signaled Interrupt */
 	Speed_down	= (1 << 4),
 	MEMMAP		= (1 << 3),
 	IOMAP		= (1 << 2),

@@ -485,6 +484,7 @@ enum rtl_register_content {
 	PMEnable	= (1 << 0),	/* Power Management Enable */
 
 	/* Config2 register p. 25 */
+	MSIEnable	= (1 << 5),	/* 8169 only. Reserved in the 8168. */
 	PCI_Clock_66MHz = 0x01,
 	PCI_Clock_33MHz = 0x00,
 

@@ -3426,22 +3426,24 @@ static const struct rtl_cfg_info {
 };
 
 /* Cfg9346_Unlock assumed. */
-static unsigned rtl_try_msi(struct pci_dev *pdev, void __iomem *ioaddr,
+static unsigned rtl_try_msi(struct rtl8169_private *tp,
 			    const struct rtl_cfg_info *cfg)
 {
+	void __iomem *ioaddr = tp->mmio_addr;
 	unsigned msi = 0;
 	u8 cfg2;
 
 	cfg2 = RTL_R8(Config2) & ~MSIEnable;
 	if (cfg->features & RTL_FEATURE_MSI) {
-		if (pci_enable_msi(pdev)) {
-			dev_info(&pdev->dev, "no MSI. Back to INTx.\n");
+		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;
 		}
 	}
-	RTL_W8(Config2, cfg2);
+	if (tp->mac_version <= RTL_GIGA_MAC_VER_06)
+		RTL_W8(Config2, cfg2);
 	return msi;
 }
 

@@ -4077,7 +4079,7 @@ rtl8169_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 		tp->features |= RTL_FEATURE_WOL;
 	if ((RTL_R8(Config5) & (UWF | BWF | MWF)) != 0)
 		tp->features |= RTL_FEATURE_WOL;
-	tp->features |= rtl_try_msi(pdev, ioaddr, cfg);
+	tp->features |= rtl_try_msi(tp, cfg);
 	RTL_W8(Cfg9346, Cfg9346_Lock);
 
 	if (rtl_tbi_enabled(tp)) {

-- 
1.7.6.4