r8169: fix invalid register dump

For some reason, my PCIe RTL8111E onboard NIC on a GA-Z68X-UD3H-B3 motherboard reads as FFs when reading from MMIO with a block size larger than 7. Therefore change to reading blocks of four bytes.

Ben Hutchings noted that the buffer is large enough to hold all registers, so now all registers are read. Since regs->len is not used anymore, drop the superfluous range check as well. (ethtool would already ensure that regs->len <= R8169_REGS_SIZE).

---
Hi,

This partly obsoletes "r8169,sis190: remove unnecessary length check"[1]. I do not have sis190 hardware, but since that is based on this r8169 driver, would it make sense to apply this patch to sis190 too?

Regards,
Peter

[1]: http://www.spinics.net/lists/netdev/msg246824.html

---
 drivers/net/ethernet/realtek/r8169.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

index b5eb419..c0c9e14 100644
@@ -1897,12 +1897,13 @@ static void rtl8169_get_regs(struct net_device *dev, struct ethtool_regs *regs,
 			     void *p)
 {
 	struct rtl8169_private *tp = netdev_priv(dev);
-
-	if (regs->len > R8169_REGS_SIZE)
-		regs->len = R8169_REGS_SIZE;
+	u32 __iomem *data = tp->mmio_addr;
+	u32 *dw = p;
+	int i;
 
 	rtl_lock_work(tp);
-	memcpy_fromio(p, tp->mmio_addr, regs->len);
+	for (i = 0; i < R8169_REGS_SIZE; i += 4)
+		memcpy_fromio(dw++, data++, 4);
 	rtl_unlock_work(tp);
 }
 

1.8.3.4

Peter Wu <[email protected]> :
[...]

Yes for the _REGS_SIZE part. Nobody proved that the memcpy_fromio split is needed or even useful with a sis19x.

Ueimor

From: Peter Wu <[email protected]>
Date: Sun, 18 Aug 2013 15:41:38 +0200

You're going to have to respin this since I applied the length check removal patch already.

Thanks.

Please find the updated patch below. There may be differences in lines because net-next has the r8169,sis190 patch and net the WOL fix.

Can I consider Francois reply[1] as Acked-by?

Thanks,
Peter

[1]: http://lkml.kernel.org/r/[email protected]

--

From: Peter Wu <[email protected]>
Date: Tue, 23 Jul 2013 12:27:27 +0200
Subject: [PATCH] r8169: fix invalid register dump

For some reason, my PCIe RTL8111E onboard NIC on a GA-Z68X-UD3H-B3 motherboard reads as FFs when reading from MMIO with a block size larger than 7. Therefore change to reading blocks of four bytes.

Ben Hutchings noted that the buffer is large enough to hold all registers, so now all registers are read.

---
 drivers/net/ethernet/realtek/r8169.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

index 93ee49d..f724d9c 100644
@@ -1897,9 +1897,12 @@ static void rtl8169_get_regs(struct net_device *dev, struct ethtool_regs *regs,
 			     void *p)
 {
 	struct rtl8169_private *tp = netdev_priv(dev);
-
+	u32 __iomem *data = tp->mmio_addr;
+	u32 *dw = p;
+	int i;
 	rtl_lock_work(tp);
-	memcpy_fromio(p, tp->mmio_addr, regs->len);
+	for (i = 0; i < R8169_REGS_SIZE; i += 4)
+		memcpy_fromio(dw++, data++, 4);
 	rtl_unlock_work(tp);
 }
 

1.8.3.4

Peter Wu <[email protected]> :
[...]

Yes, provided you insert an empty line between declarations and code.

You may / should send patches as normal submissions. It will probably be easier for davem to use and you can always raise questions or add minor remarks in the "--- ... diff" section.

Ueimor

Done, new patch will arrive in a minute.

I noticed that patchwork also recognized submissions that are appended to a mail, so I tried that out ;-) Well, apparantly it does not work well with git, so I will just send a new patch with a useful In-Reply-To header.

Peter

For some reason, my PCIe RTL8111E onboard NIC on a GA-Z68X-UD3H-B3 motherboard reads as FFs when reading from MMIO with a block size larger than 7. Therefore change to reading blocks of four bytes.

Ben Hutchings noted that the buffer is large enough to hold all registers, so now all registers are read.

---
 drivers/net/ethernet/realtek/r8169.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

index 93ee49d..c0c9e14 100644
@@ -1897,9 +1897,13 @@ static void rtl8169_get_regs(struct net_device *dev, struct ethtool_regs *regs,
 			     void *p)
 {
 	struct rtl8169_private *tp = netdev_priv(dev);
+	u32 __iomem *data = tp->mmio_addr;
+	u32 *dw = p;
+	int i;
 
 	rtl_lock_work(tp);
-	memcpy_fromio(p, tp->mmio_addr, regs->len);
+	for (i = 0; i < R8169_REGS_SIZE; i += 4)
+		memcpy_fromio(dw++, data++, 4);
 	rtl_unlock_work(tp);
 }
 

1.8.3.4

From: Peter Wu <[email protected]>
Date: Wed, 21 Aug 2013 23:17:11 +0200

Applied, thanks.