| Message ID | 20121130232152.GA10960@udknight |
|---|---|
| State | Superseded, archived |
| Delegated to: | David Miller |
| Headers | show |
Wang YanQing <udknight@gmail.com> : [...] > @@ -6903,6 +6903,14 @@ rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) > dev->dev_addr[i] = RTL_R8(MAC0 + i); > memcpy(dev->perm_addr, dev->dev_addr, dev->addr_len); > > + /* > + *This is a fix for BIOS forget to set > + *extend GigaMAC registers > + *Wang YanQing 12/1/2012 > + */ This part will go into the changelog. > + if (tp->mac_version == RTL_GIGA_MAC_VER_34) { > + rtl_rar_set(tp, dev->dev_addr); > + } rtl_rar_set already includes a RTL_GIGA_MAC_VER_34 test and non-8168evl devices are already able to stand an extra MAC{0, 4} write. I'll check it does not hurt on different 81xx devices and submit an update. Thanks.
On Sat, Dec 01, 2012 at 12:44:01PM +0100, Francois Romieu wrote: > Wang YanQing <udknight@gmail.com> : > > + /* > > + *This is a fix for BIOS forget to set > > + *extend GigaMAC registers > > + *Wang YanQing 12/1/2012 > > + */ > > This part will go into the changelog. I think brevity comment in code is good for code's readableness. We read out the MAC{0,4}, and write them back in next line to call rtl_rar_set, it don't have obvious sense for new readers, so I think the brevity comment is good. Could you consider remaining the comment except the no sense line "Wang YanQing 12/1/2012"? > > > + if (tp->mac_version == RTL_GIGA_MAC_VER_34) { > > + rtl_rar_set(tp, dev->dev_addr); > > + } > > rtl_rar_set already includes a RTL_GIGA_MAC_VER_34 test and non-8168evl > devices are already able to stand an extra MAC{0, 4} write. I'll check > it does not hurt on different 81xx devices and submit an update. I add the test code to ignore the an extra MAC{0,4} write for non-8168evl devices, and if you think it is not a issue, then I agree with you to remove the test code. Thanks. -- 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
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c index 927aa33..5d98296 100644 --- a/drivers/net/ethernet/realtek/r8169.c +++ b/drivers/net/ethernet/realtek/r8169.c @@ -6903,6 +6903,14 @@ rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) dev->dev_addr[i] = RTL_R8(MAC0 + i); memcpy(dev->perm_addr, dev->dev_addr, dev->addr_len); + /* + *This is a fix for BIOS forget to set + *extend GigaMAC registers + *Wang YanQing 12/1/2012 + */ + if (tp->mac_version == RTL_GIGA_MAC_VER_34) { + rtl_rar_set(tp, dev->dev_addr); + } SET_ETHTOOL_OPS(dev, &rtl8169_ethtool_ops); dev->watchdog_timeo = RTL8169_TX_TIMEOUT;
I get a board with 8168e-vl(10ec:8168 with RTL_GIGA_MAC_VER_34), everything looks well first, I can use ifconfig to set ip, netmask, etc. And the rx/tx statistics show by ifconfig looks good when I ping another host or ping it from another host. But it don't work, I can't get ICMP REPLAY from both sides, although the RX/TX statistics seem good. After add some debug code, I found this NIC only accept ethernet broadcast package, it can't filter out the package send to its MAC address, but it works good for sending.So ifconfig show the RX/TX status means it can receive ARP package.(It don't know its MAC address, so below) I have try the driver provided by realtek's website, it have the same problem at the first time. BUT IT WORK AFTER I REBOOT with CRTL-ALT-DEL, the reason is that realtek's driver call rtl8168_rar_set in the .shutdown function register with pci_register_driver. Yes, the really reason to make it work is rtl8689_rar_set, this function set extended GigaMAC registers, so after reboot without lost the power, NIC keep the status before reboot. I haven't see any code to set GigaMAC registers in kernel when boot, so I guess BIOS or NIC's circuit make it, but of course one miss the extended GigaMAC registers in this problem. The probe code can get MAC address right, so MAC{0,4} must had been setted, but some guys forget the extended GigaMAC registers. This patch fix it. [ I don't known whether others' realtek's NIC with extended GigaMAC reigisters have the same problem, I meet it in 8168e-vl with RTL_GIGA_MAC_VER_34, so I make this patch just for it.] Changes: V1-V2: I follow Francois Romieu 's below opinion to make this patch oneline: I'd rather see the GigaMAC registers written through a call to rtl_rar_set when the mac address is read in rtl_init_one instead of duplicating most of rtl_rar_set in a quite different place. V2-V3: 1:Add conditon code to around this fix, because it make no sense for most NIC 2:Add comment in code Signed-off-by: Wang YanQing <udknight@gmail.com> --- drivers/net/ethernet/realtek/r8169.c | 8 ++++++++ 1 file changed, 8 insertions(+)