Re: [PATCH] forcedeth: reconfigure multicast packet filter only when needed

[Jindřich Makovička]

2010/10/9 Stephen Hemminger <shemminger@xxxxxxxxxx>:
> On Sat, 9 Oct 2010 13:26:05 +0200
> Jindřich Makovička <makovick@xxxxxxxxx> wrote:
>
>> +
>> +     /* current packet filter state */
>> +     u32 cur_pff;
>> +     u32 cur_addr[2];
>> +     u32 cur_mask[2];
>>  };
>
> No big deal, but couldn't you just put those temporary variables
> on the stack. and reread the current value before stopping.

Sure, this version should do the same (still untested, I don't have a
machine with nForce here at the moment). I just wanted to avoid more
NIC register accesses, but it's probably a premature optimization.

Regards,
-- 
Jindrich Makovicka
--- forcedeth.c.orig	2010-10-07 08:56:55.564511153 +0200
+++ forcedeth.c	2010-10-09 22:34:53.151523511 +0200
@@ -3027,9 +3027,15 @@
 {
 	struct fe_priv *np = netdev_priv(dev);
 	u8 __iomem *base = get_hwbase(dev);
-	u32 addr[2];
-	u32 mask[2];
-	u32 pff = readl(base + NvRegPacketFilterFlags) & NVREG_PFF_PAUSE_RX;
+	u32 addr[2], prev_addr[2];
+	u32 mask[2], prev_mask[2];
+	u32 prev_pff = readl(base + NvRegPacketFilterFlags);
+	u32 pff = prev_pff & NVREG_PFF_PAUSE_RX;
+
+	prev_addr[0] = readl(base + NvRegMulticastAddrA);
+	prev_addr[1] = readl(base + NvRegMulticastAddrB);
+	prev_mask[0] = readl(base + NvRegMulticastMaskA);
+	prev_mask[1] = readl(base + NvRegMulticastMaskB);
 
 	memset(addr, 0, sizeof(addr));
 	memset(mask, 0, sizeof(mask));
@@ -3072,17 +3078,25 @@
 	}
 	addr[0] |= NVREG_MCASTADDRA_FORCE;
 	pff |= NVREG_PFF_ALWAYS;
-	spin_lock_irq(&np->lock);
-	nv_stop_rx(dev);
-	writel(addr[0], base + NvRegMulticastAddrA);
-	writel(addr[1], base + NvRegMulticastAddrB);
-	writel(mask[0], base + NvRegMulticastMaskA);
-	writel(mask[1], base + NvRegMulticastMaskB);
-	writel(pff, base + NvRegPacketFilterFlags);
-	dprintk(KERN_INFO "%s: reconfiguration for multicast lists.\n",
-		dev->name);
-	nv_start_rx(dev);
-	spin_unlock_irq(&np->lock);
+	if (prev_pff != pff
+	    || memcmp(prev_addr, addr, sizeof(prev_addr)) != 0
+	    || memcmp(prev_mask, mask, sizeof(prev_mask)) != 0)
+	{
+		dprintk(KERN_INFO "%s: reconfiguration for multicast lists.\n",
+			dev->name);
+		spin_lock_irq(&np->lock);
+		nv_stop_rx(dev);
+		writel(addr[0], base + NvRegMulticastAddrA);
+		writel(addr[1], base + NvRegMulticastAddrB);
+		writel(mask[0], base + NvRegMulticastMaskA);
+		writel(mask[1], base + NvRegMulticastMaskB);
+		writel(pff, base + NvRegPacketFilterFlags);
+		nv_start_rx(dev);
+		spin_unlock_irq(&np->lock);
+	} else {
+		dprintk(KERN_INFO "%s: pff state unchanged - skipping reconfiguration.\n",
+			dev->name);
+	}
 }
 
 static void nv_update_pause(struct net_device *dev, u32 pause_flags)