Messages in this thread | | | Date | Mon, 3 Dec 2012 00:34:50 +0800 | From | Wang YanQing <> | Subject | Re: [PATCH v3]realtek:r8169: Bugfix or workaround for missing extended GigaMAC registers settings |
| |
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.
| |