| Message ID | 20100603112428.GC24909@host-a-55.ustcsz.edu.cn |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
Junchang Wang <junchangwang@gmail.com> : > Some clean up work. Please correct me if any of this is wrong. > > Writting "cmd" back without modification is redundant. > Secondly, because rtl_rw_cpluscmd is just encapsulation of > RTL_R16, remove rtl_rw_cpluscmd. I'll figure that there may be some value in the patch if you test the change on revision X, Y and Z. Some cosmetic isolated in a sequence of changes can be fine too. Otherwise I consider such cleanups as a (useless and) conceivably harmful distraction.
> I'll figure that there may be some value in the patch if you > test the change on revision X, Y and Z. Frankly, I just tested the patch on my NIC. It works fine. > > Some cosmetic isolated in a sequence of changes can be fine too. > > Otherwise I consider such cleanups as a (useless and) conceivably > harmful distraction. As I said, those points really confused me while I was going over the driver. I searched google and mailing list archive but failed to get any traces. It seems they exist for a long time without any touch (care). So I decided to post the patches to see whether they are harmful or not. OK. I'll bypass legacy hardware-related codes since they are really very tricky. :) Thanks Francois.
diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c index 217e709..6a37813 100644 --- a/drivers/net/r8169.c +++ b/drivers/net/r8169.c @@ -3404,15 +3404,6 @@ static void rtl_set_rx_tx_desc_registers(struct rtl8169_private *tp, RTL_W32(RxDescAddrLow, ((u64) tp->RxPhyAddr) & DMA_BIT_MASK(32)); } -static u16 rtl_rw_cpluscmd(void __iomem *ioaddr) -{ - u16 cmd; - - cmd = RTL_R16(CPlusCmd); - RTL_W16(CPlusCmd, cmd); - return cmd; -} - static void rtl_set_rx_max_size(void __iomem *ioaddr, unsigned int rx_buf_sz) { /* Low hurts. Let's disable the filtering. */ @@ -3471,7 +3462,7 @@ static void rtl_hw_start_8169(struct net_device *dev) (tp->mac_version == RTL_GIGA_MAC_VER_04)) rtl_set_rx_tx_config_registers(tp); - tp->cp_cmd |= rtl_rw_cpluscmd(ioaddr) | PCIMulRW; + tp->cp_cmd |= RTL_R16(CPlusCmd) | PCIMulRW; if ((tp->mac_version == RTL_GIGA_MAC_VER_02) || (tp->mac_version == RTL_GIGA_MAC_VER_03)) { @@ -3906,7 +3897,7 @@ static void rtl_hw_start_8101(struct net_device *dev) rtl_set_rx_max_size(ioaddr, tp->rx_buf_sz); - tp->cp_cmd |= rtl_rw_cpluscmd(ioaddr) | PCIMulRW; + tp->cp_cmd |= RTL_R16(CPlusCmd) | PCIMulRW; RTL_W16(CPlusCmd, tp->cp_cmd); -- -- To unsubscribe from this list: send the line "unsubscribe netdev" in
Some clean up work. Please correct me if any of this is wrong. Writting "cmd" back without modification is redundant. Secondly, because rtl_rw_cpluscmd is just encapsulation of RTL_R16, remove rtl_rw_cpluscmd. Signed-off-by: Junchang Wang <junchangwang@gmail.com> --- drivers/net/r8169.c | 13 ++----------- 1 files changed, 2 insertions(+), 11 deletions(-) the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html