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

login
register
mail settings
Submitter Su Kang Yin
Date Dec. 14, 2011, 7:13 a.m.
Message ID <CABJLtPHWfRLcbzDFR0ZXe4da3jv04eDZL2CabF1FTcaadoy0xQ@mail.gmail.com>
Download mbox | patch
Permalink /patch/131319/
State Superseded
Delegated to: David Miller
Headers show

Comments

Su Kang Yin - Dec. 14, 2011, 7:13 a.m.
correct MSIEnable (bit 5) register to Config1 (offset 0x52) instead of
Config2 (offset 0x53)

Signed-off-by: Su Kang Yin <cantona@cantona.net>
---
 drivers/net/ethernet/realtek/r8169.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)
fran├žois romieu - Dec. 14, 2011, 9:37 p.m.
Su Kang Yin <cantona@cantona.no-ip.org> :
> 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.
David Miller - Dec. 15, 2011, 6:43 a.m.
From: Francois Romieu <romieu@fr.zoreil.com>
Date: Wed, 14 Dec 2011 22:37:13 +0100

> Su Kang Yin <cantona@cantona.no-ip.org> :
>> 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. :-)

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
hayeswang - Dec. 15, 2011, 8:34 a.m.
> -----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
> 
> > Su Kang Yin <cantona@cantona.no-ip.org> :
> >> correct MSIEnable (bit 5) register to Config1 (offset 
> 0x52) instead of
> >> 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.

> > 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. :-)
> 

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

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;
 }