netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 3/3] r8169: clean up
@ 2004-11-02 18:03 Jon Mason
  2004-11-02 19:27 ` Francois Romieu
  0 siblings, 1 reply; 5+ messages in thread
From: Jon Mason @ 2004-11-02 18:03 UTC (permalink / raw)
  To: Francois Romieu; +Cc: netdev

[-- Attachment #1: Type: text/plain, Size: 1849 bytes --]

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

Signed-off-by: Jon Mason <jdmason@us.ibm.com>

--- r8169.c     2004-11-02 10:37:31.035203120 -0600
+++ r8169.c.latest      2004-11-02 10:44:01.367863536 -0600
@@ -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);


[-- Attachment #2: r8169-cleanup-3.patch --]
[-- Type: text/x-diff, Size: 1381 bytes --]

--- r8169.c	2004-11-02 10:37:31.035203120 -0600
+++ r8169.c.latest	2004-11-02 10:44:01.367863536 -0600
@@ -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);

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 3/3] r8169: clean up
  2004-11-02 18:03 [PATCH 3/3] r8169: clean up Jon Mason
@ 2004-11-02 19:27 ` Francois Romieu
  2004-11-02 19:47   ` Jon Mason
  0 siblings, 1 reply; 5+ messages in thread
From: Francois Romieu @ 2004-11-02 19:27 UTC (permalink / raw)
  To: Jon Mason; +Cc: netdev, jgarzik

Jon Mason <jdmason@us.ibm.com> :
> Removal of some magic numbers, unnecessary double not, and addition of "link 
> down" notification.
> 
> Signed-off-by: Jon Mason <jdmason@us.ibm.com>
> 
> --- r8169.c     2004-11-02 10:37:31.035203120 -0600
> +++ r8169.c.latest      2004-11-02 10:44:01.367863536 -0600
[...]
> @@ -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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 3/3] r8169: clean up
  2004-11-02 19:27 ` Francois Romieu
@ 2004-11-02 19:47   ` Jon Mason
  2004-11-02 19:55     ` Thomas Graf
  2004-11-02 21:56     ` Francois Romieu
  0 siblings, 2 replies; 5+ messages in thread
From: Jon Mason @ 2004-11-02 19:47 UTC (permalink / raw)
  To: Francois Romieu; +Cc: netdev, jgarzik

On Tuesday 02 November 2004 01:27 pm, Francois Romieu wrote:
> Jon Mason <jdmason@us.ibm.com> :
> > Removal of some magic numbers, unnecessary double not, and addition of
> > "link down" notification.
> >
> > Signed-off-by: Jon Mason <jdmason@us.ibm.com>
> >
> > --- r8169.c     2004-11-02 10:37:31.035203120 -0600
> > +++ r8169.c.latest      2004-11-02 10:44:01.367863536 -0600
>
> [...]
>
> > @@ -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.

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;

-- 
Jon Mason
jdmason@us.ibm.com

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 3/3] r8169: clean up
  2004-11-02 19:47   ` Jon Mason
@ 2004-11-02 19:55     ` Thomas Graf
  2004-11-02 21:56     ` Francois Romieu
  1 sibling, 0 replies; 5+ messages in thread
From: Thomas Graf @ 2004-11-02 19:55 UTC (permalink / raw)
  To: Jon Mason; +Cc: Francois Romieu, netdev, jgarzik

> > > 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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 3/3] r8169: clean up
  2004-11-02 19:47   ` Jon Mason
  2004-11-02 19:55     ` Thomas Graf
@ 2004-11-02 21:56     ` Francois Romieu
  1 sibling, 0 replies; 5+ messages in thread
From: Francois Romieu @ 2004-11-02 21:56 UTC (permalink / raw)
  To: Jon Mason; +Cc: netdev, jgarzik

Jon Mason <jdmason@us.ibm.com> :
[...]
> Silly question, does "!!" not execute a bitwise not twice?  If not, then 
> please forgive my ignorance.

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

> 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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2004-11-02 21:56 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-11-02 18:03 [PATCH 3/3] r8169: clean up Jon Mason
2004-11-02 19:27 ` Francois Romieu
2004-11-02 19:47   ` Jon Mason
2004-11-02 19:55     ` Thomas Graf
2004-11-02 21:56     ` Francois Romieu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).