Skip to contentSkip to conversation

r8169: clean up

PATCH In review Jon Mason 3 participants 5 messages Nov 2, 2004

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

Jon Mason

author
#0 lore raw

PATCH 3/3 r8169: clean up

Removal of some magic numbers, unnecessary double not, and addition of "link down" notification.

Signed-off-by Jon Mason <[email protected]>
r8169.c.latest 2004-11-02 10:44:01.367863536 -0600 renamed from r8169.c 2004-11-02 10:37:31.035203120 -0600 +7 -5
@@ -519,8 +519,10 @@ static void rtl8169_check_link_status(st
        if (tp->link_ok(ioaddr)) {
                netif_carrier_on(dev);
                printk(KERN_INFO PFX "%s: link up\n", dev->name);
-       } else
+       } else {
                netif_carrier_off(dev);
+               printk(KERN_INFO PFX "%s: link down\n", dev->name);
+       }
        spin_unlock_irqrestore(&tp->lock, flags);
 }
 

@@ -773,7 +775,7 @@ static void rtl8169_gset_tbi(struct net_
 
        status = RTL_R32(TBICSR);
        cmd->advertising = (status & TBINwEnable) ?  ADVERTISED_Autoneg : 0;
-       cmd->autoneg = !!(status & TBINwEnable);
+       cmd->autoneg = status & TBINwEnable;
 
        cmd->speed = SPEED_1000;
        cmd->duplex = DUPLEX_FULL; /* Always set */

@@ -1639,7 +1641,7 @@ static int rtl8169_alloc_rx_skb(struct p
        if (!skb)
                goto err_out;
 
-       skb_reserve(skb, 2);
+       skb_reserve(skb, NET_IP_ALIGN);
        *sk_buff = skb;
 
        mapping = pci_map_single(pdev, skb->tail, rx_buf_sz,

@@ -2071,9 +2073,9 @@ static inline int rtl8169_try_rx_copy(st
        if (pkt_size < rx_copybreak) {
                struct sk_buff *skb;
 
-               skb = dev_alloc_skb(pkt_size + 2);
+               skb = dev_alloc_skb(pkt_size + NET_IP_ALIGN);
                if (skb) {
-                       skb_reserve(skb, 2);
+                       skb_reserve(skb, NET_IP_ALIGN);
                        eth_copy_and_sum(skb, sk_buff[0]->tail, pkt_size, 0);
                        *sk_buff = skb;
                        rtl8169_return_to_asic(desc, rx_buf_sz);
r8169.c.latest renamed from r8169.c +7 -5
@@ -519,8 +519,10 @@ static void rtl8169_check_link_status(st
 	if (tp->link_ok(ioaddr)) {
 		netif_carrier_on(dev);
 		printk(KERN_INFO PFX "%s: link up\n", dev->name);
-	} else
+	} else {
 		netif_carrier_off(dev);
+		printk(KERN_INFO PFX "%s: link down\n", dev->name);
+	}	
 	spin_unlock_irqrestore(&tp->lock, flags);
 }
 

@@ -773,7 +775,7 @@ static void rtl8169_gset_tbi(struct net_
 
 	status = RTL_R32(TBICSR);
 	cmd->advertising = (status & TBINwEnable) ?  ADVERTISED_Autoneg : 0;
-	cmd->autoneg = !!(status & TBINwEnable);
+	cmd->autoneg = status & TBINwEnable;
 
 	cmd->speed = SPEED_1000;
 	cmd->duplex = DUPLEX_FULL; /* Always set */

@@ -1639,7 +1641,7 @@ static int rtl8169_alloc_rx_skb(struct p
 	if (!skb)
 		goto err_out;
 
-	skb_reserve(skb, 2);
+	skb_reserve(skb, NET_IP_ALIGN);
 	*sk_buff = skb;
 
 	mapping = pci_map_single(pdev, skb->tail, rx_buf_sz,

@@ -2071,9 +2073,9 @@ static inline int rtl8169_try_rx_copy(st
 	if (pkt_size < rx_copybreak) {
 		struct sk_buff *skb;
 
-		skb = dev_alloc_skb(pkt_size + 2);
+		skb = dev_alloc_skb(pkt_size + NET_IP_ALIGN);
 		if (skb) {
-			skb_reserve(skb, 2);
+			skb_reserve(skb, NET_IP_ALIGN);
 			eth_copy_and_sum(skb, sk_buff[0]->tail, pkt_size, 0);
 			*sk_buff = skb;
 			rtl8169_return_to_asic(desc, rx_buf_sz);
#1 lore raw

Jon Mason <[email protected]> :

Quoted 7 lines

Removal of some magic numbers, unnecessary double not, and addition of "link down" notification.

Signed-off-by Jon Mason <[email protected]>
r8169.c.latest 2004-11-02 10:44:01.367863536 -0600 renamed from r8169.c 2004-11-02 10:37:31.035203120 -0600 +0 -0

[...]

r8169.c.latest 2004-11-02 10:44:01.367863536 -0600 static void rtl8169_gset_tbi(struct net_ · line 781

Quoted 9 lines
r8169.c.latest 2004-11-02 10:44:01.367863536 -0600 renamed from r8169.c 2004-11-02 10:37:31.035203120 -0600 +1 -1
@@ -773,7 +775,7 @@ static void rtl8169_gset_tbi(struct net_
 
        status = RTL_R32(TBICSR);
        cmd->advertising = (status & TBINwEnable) ?  ADVERTISED_Autoneg : 0;
-       cmd->autoneg = !!(status & TBINwEnable);
+       cmd->autoneg = status & TBINwEnable;
 
        cmd->speed = SPEED_1000;
        cmd->duplex = DUPLEX_FULL; /* Always set */

drivers/net/*.c use 0/1 or AUTONEG_{EN/DIS}ABLE.

--
Ueimor

author
#2 lore raw
Quoted 18 lines · Francois Romieu

Jon Mason <[email protected]> :

Quoted 7 lines

Removal of some magic numbers, unnecessary double not, and addition of "link down" notification.

Signed-off-by Jon Mason <[email protected]>
r8169.c.latest 2004-11-02 10:44:01.367863536 -0600 renamed from r8169.c 2004-11-02 10:37:31.035203120 -0600 +0 -0

[...]

Quoted 7 lines
r8169.c.latest 2004-11-02 10:44:01.367863536 -0600 renamed from r8169.c 2004-11-02 10:37:31.035203120 -0600 +1 -0
@@ -773,7 +775,7 @@ static void rtl8169_gset_tbi(struct net_
 
        status = RTL_R32(TBICSR);
        cmd->advertising = (status & TBINwEnable) ?  ADVERTISED_Autoneg :
 0; -       cmd->autoneg = !!(status & TBINwEnable);
+       cmd->autoneg = status & TBINwEnable;
Quoted 4 lines

r8169.c.latest 2004-11-02 10:44:01.367863536 -0600 static void rtl8169_gset_tbi(struct net_

Quoted 2 lines
r8169.c.latest 2004-11-02 10:44:01.367863536 -0600 renamed from r8169.c 2004-11-02 10:37:31.035203120 -0600 +0 -0
        cmd->speed = SPEED_1000;
        cmd->duplex = DUPLEX_FULL; /* Always set */

drivers/net/*.c use 0/1 or AUTONEG_{EN/DIS}ABLE.

Silly question, does "!!" not execute a bitwise not twice? If not, then please forgive my ignorance.

If so, then it is unnecessary. But if you want something prettier, how about:

cmd->autoneg = (status & TBINwEnable) ? AUTONEG_ENABLE : AUTONEG_DISABLE;

Signature · 2 lines
Jon Mason
[email protected]
#3 lore raw
Quoted 4 lines
Quoted 2 lines
Quoted 2 lines

0; - cmd->autoneg = !!(status & TBINwEnable);
+ cmd->autoneg = status & TBINwEnable;

Silly question, does "!!" not execute a bitwise not twice? If not, then please forgive my ignorance.

Yes but the result is not the same, the result will be either 1 or 0 if done with !! and yours will result in TBINwEnable or 0.

#4 lore raw
In reply to Jon Mason

Jon Mason <[email protected]> :
[...]

Quoted 2 lines

Silly question, does "!!" not execute a bitwise not twice? If not, then please forgive my ignorance.

<coffee alert>
logical not: !
bitwise not: ~
</coffee>

Quoted 3 lines

If so, then it is unnecessary. But if you want something prettier, how about:

cmd->autoneg = (status & TBINwEnable) ? AUTONEG_ENABLE : AUTONEG_DISABLE;

Ok, I take it.

--
Ueimor.