On Thu, Jun 05, 2008 at 05:33:06AM -0700, Dale Farnsworth wrote:

quoted text
> > @@ -441,7 +445,7 @@ static void rxq_refill(struct rx_queue *rxq)
> >  						RX_ENABLE_INTERRUPT;
> >  		wmb();
> >  
> > -		skb_reserve(skb, ETH_HW_IP_ALIGN);
> > +		skb_reserve(skb, 2);
> 
> I think you've removed some useful documentation here.  Either keep the
> define or add a comment explaining what the 2 is for.
> 
> >  	}
> >  
> >  	if (rxq->rx_desc_count == 0) {
> 
> [snip]
> 
> > @@ -498,24 +502,26 @@ static int rxq_process(struct rx_queue *rxq, int budget)
> >  		 * Note byte count includes 4 byte CRC count
> >  		 */
> >  		stats->rx_packets++;
> > -		stats->rx_bytes += rx_desc->byte_cnt - ETH_HW_IP_ALIGN;
> > +		stats->rx_bytes += rx_desc->byte_cnt - 2;
> 
> Same comment here.
> 
> >  		/*
> > -		 * In case received a packet without first / last bits on OR
> > -		 * the error summary bit is on, the packets needs to be dropeed.
> > +		 * In case we received a packet without first / last bits
> > +		 * on, or the error summary bit is set, the packet needs
> > +		 * to be dropped.
> >  		 */
> >  		if (((cmd_sts & (RX_FIRST_DESC | RX_LAST_DESC)) !=
> >  					(RX_FIRST_DESC | RX_LAST_DESC))
> >  				|| (cmd_sts & ERROR_SUMMARY)) {
> >  			stats->rx_dropped++;
> > +
> >  			if ((cmd_sts & (RX_FIRST_DESC | RX_LAST_DESC)) !=
> >  				(RX_FIRST_DESC | RX_LAST_DESC)) {
> >  				if (net_ratelimit())
> > -					printk(KERN_ERR
> > -						"%s: Received packet spread "
> > -						"on multiple descriptors\n",
> > -						mep->dev->name);
> > +					dev_printk(KERN_ERR, &mep->dev->dev,
> > +						   "received packet spanning "
> > +						   "multiple descriptors\n");
> >  			}
> > +
> >  			if (cmd_sts & ERROR_SUMMARY)
> >  				stats->rx_errors++;
> >  
> 
> [snip]
> 
> > @@ -974,33 +990,31 @@ static int mv643xx_eth_nway_restart(struct net_device *dev)
> >  static u32 mv643xx_eth_get_link(struct net_device *dev)
> >  {
> >  	struct mv643xx_eth_private *mep = netdev_priv(dev);
> > -
> >  	return mii_link_ok(&mep->mii);
> >  }
> 
> Keep the blank line, IMHO.

I've folded the following patch into the patch.  OK?


Index: linux-2.6.26-rc5/drivers/net/mv643xx_eth.c
===================================================================
--- linux-2.6.26-rc5.orig/drivers/net/mv643xx_eth.c
+++ linux-2.6.26-rc5/drivers/net/mv643xx_eth.c
@@ -454,6 +454,11 @@ static void rxq_refill(struct rx_queue *
 						RX_ENABLE_INTERRUPT;
 		wmb();
 
+		/*
+		 * The hardware automatically prepends 2 bytes of
+		 * dummy data to each received packet, so that the
+		 * IP header ends up 16-byte aligned.
+		 */
 		skb_reserve(skb, 2);
 	}
 
@@ -508,7 +513,11 @@ static int rxq_process(struct rx_queue *
 
 		/*
 		 * Update statistics.
-		 * Note byte count includes 4 byte CRC count
+		 *
+		 * Note that the descriptor byte count includes 2 dummy
+		 * bytes automatically inserted by the hardware at the
+		 * start of the packet (which we don't count), and a 4
+		 * byte CRC at the end of the packet (which we do count).
 		 */
 		stats->rx_packets++;
 		stats->rx_bytes += rx_desc->byte_cnt - 2;
@@ -999,6 +1008,7 @@ static int mv643xx_eth_nway_reset(struct
 static u32 mv643xx_eth_get_link(struct net_device *dev)
 {
 	struct mv643xx_eth_private *mp = netdev_priv(dev);
+
 	return mii_link_ok(&mp->mii);
 }
--
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