diff mbox

3c589_cs: re-initialize the multicast in the tc589_reset

Message ID 20090720080812.49633622.ken_kawasaki@spring.nifty.jp
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Ken Kawasaki July 19, 2009, 11:08 p.m. UTC 
3c589_cs: 
re-initialize the multicast in the tc589_reset,
and spin_lock the set_multicast_list function.  

Signed-off-by: Ken Kawasaki <ken_kawasaki@spring.nifty.jp>

---

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

Comments

David Miller July 20, 2009, 3:29 p.m. UTC | #1 
From: Ken Kawasaki <ken_kawasaki@spring.nifty.jp>
Date: Mon, 20 Jul 2009 08:08:12 +0900

> 
> 3c589_cs: 
> re-initialize the multicast in the tc589_reset,
> and spin_lock the set_multicast_list function.  
> 
> Signed-off-by: Ken Kawasaki <ken_kawasaki@spring.nifty.jp>

Applied, but you really need to fix your coding style.

I know perhaps you were trying to be consistent with the
rest of this driver, but when adding completely new code
the conventions of the rest of the kernel ought to be
followed, for example:

> +static void set_multicast_list(struct net_device *dev)
> +{
> +    struct el3_private *priv = netdev_priv(dev);
> +    unsigned long flags;
> +
> +    spin_lock_irqsave(&priv->lock, flags);    
> +    set_rx_mode(dev);
> +    spin_unlock_irqrestore(&priv->lock, flags);    
> +}

One tab should begin each statement in this function, and
spin_lock and spin_unlock lines had unnecessary trailing
whitespace.

I fixed all of this up when applying your patch, but I
absolutely will not do so next time.

Thank you.
--
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
Ken Kawasaki July 22, 2009, 12:23 p.m. UTC | #2 
On Mon, 20 Jul 2009 08:29:06 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:


> > 3c589_cs: 
> > re-initialize the multicast in the tc589_reset,
> > and spin_lock the set_multicast_list function.  
> > 
> > Signed-off-by: Ken Kawasaki <ken_kawasaki@spring.nifty.jp>
> 
> Applied, but you really need to fix your coding style.
> 
> I know perhaps you were trying to be consistent with the
> rest of this driver, but when adding completely new code
> the conventions of the rest of the kernel ought to be
> followed, for example:
> 
> > +static void set_multicast_list(struct net_device *dev)
> > +{
> > +    struct el3_private *priv = netdev_priv(dev);
> > +    unsigned long flags;
> > +
> > +    spin_lock_irqsave(&priv->lock, flags);    
> > +    set_rx_mode(dev);
> > +    spin_unlock_irqrestore(&priv->lock, flags);    
> > +}
> 
> One tab should begin each statement in this function, and
> spin_lock and spin_unlock lines had unnecessary trailing
> whitespace.
> 
> I fixed all of this up when applying your patch, but I
> absolutely will not do so next time.

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

Patch

--- linux-2.6.31-rc3/drivers/net/pcmcia/3c589_cs.c.orig	2009-07-19 19:13:10.000000000 +0900
+++ linux-2.6.31-rc3/drivers/net/pcmcia/3c589_cs.c	2009-07-19 19:26:40.000000000 +0900
@@ -156,6 +156,7 @@  static struct net_device_stats *el3_get_
 static int el3_rx(struct net_device *dev);
 static int el3_close(struct net_device *dev);
 static void el3_tx_timeout(struct net_device *dev);
+static void set_rx_mode(struct net_device *dev);
 static void set_multicast_list(struct net_device *dev);
 static const struct ethtool_ops netdev_ethtool_ops;
 
@@ -488,8 +489,7 @@  static void tc589_reset(struct net_devic
     /* Switch to register set 1 for normal use. */
     EL3WINDOW(1);
 
-    /* Accept b-cast and phys addr only. */
-    outw(SetRxFilter | RxStation | RxBroadcast, ioaddr + EL3_CMD);
+    set_rx_mode(dev);
     outw(StatsEnable, ioaddr + EL3_CMD); /* Turn on statistics. */
     outw(RxEnable, ioaddr + EL3_CMD); /* Enable the receiver. */
     outw(TxEnable, ioaddr + EL3_CMD); /* Enable transmitter. */
@@ -700,7 +700,7 @@  static irqreturn_t el3_interrupt(int irq
 		if (fifo_diag & 0x2000) {
 		    /* Rx underrun */
 		    tc589_wait_for_completion(dev, RxReset);
-		    set_multicast_list(dev);
+    		    set_rx_mode(dev);
 		    outw(RxEnable, ioaddr + EL3_CMD);
 		}
 		outw(AckIntr | AdapterFailure, ioaddr + EL3_CMD);
@@ -905,14 +905,11 @@  static int el3_rx(struct net_device *dev
     return 0;
 }
 
-static void set_multicast_list(struct net_device *dev)
+static void set_rx_mode(struct net_device *dev)
 {
-    struct el3_private *lp = netdev_priv(dev);
-    struct pcmcia_device *link = lp->p_dev;
     unsigned int ioaddr = dev->base_addr;
     u16 opts = SetRxFilter | RxStation | RxBroadcast;
 
-    if (!pcmcia_dev_present(link)) return;
     if (dev->flags & IFF_PROMISC)
 	opts |= RxMulticast | RxProm;
     else if (dev->mc_count || (dev->flags & IFF_ALLMULTI))
@@ -920,6 +917,16 @@  static void set_multicast_list(struct ne
     outw(opts, ioaddr + EL3_CMD);
 }
 
+static void set_multicast_list(struct net_device *dev)
+{
+    struct el3_private *priv = netdev_priv(dev);
+    unsigned long flags;
+
+    spin_lock_irqsave(&priv->lock, flags);    
+    set_rx_mode(dev);
+    spin_unlock_irqrestore(&priv->lock, flags);    
+}
+
 static int el3_close(struct net_device *dev)
 {
     struct el3_private *lp = netdev_priv(dev);