Don Fry wrote:
Reorganize code to facilitate NAPI changes.
Tested ia32 and ppc64.

Signed-off-by: Don Fry <[EMAIL PROTECTED]>

NAK.  This patch should have NO CODE CHANGES, just moving code around.

Mathematically, it should be an equivalent transform. Thus, we can trivially verify that it breaks nothing.


--- linux-2.6.18-rc6/drivers/net/pcnet32.c.orig Fri Sep  8 14:02:12 2006
+++ linux-2.6.18-rc6/drivers/net/pcnet32.c      Mon Sep 11 09:07:13 2006
@@ -299,7 +299,6 @@ static int pcnet32_probe1(unsigned long,
 static int pcnet32_open(struct net_device *);
 static int pcnet32_init_ring(struct net_device *);
 static int pcnet32_start_xmit(struct sk_buff *, struct net_device *);
-static int pcnet32_rx(struct net_device *);
 static void pcnet32_tx_timeout(struct net_device *dev);
 static irqreturn_t pcnet32_interrupt(int, void *, struct pt_regs *);
 static int pcnet32_close(struct net_device *);
@@ -1125,6 +1124,235 @@ static int pcnet32_suspend(struct net_de
        return 1;
 }
+static int pcnet32_rx_entry(struct net_device *dev,
+                           struct pcnet32_private *lp,
+                           struct pcnet32_rx_head *rxp,
+                           int entry)
+{
+       int status = (short)le16_to_cpu(rxp->status) >> 8;
+       int rx_in_place = 0;
+       struct sk_buff *skb;
+       short pkt_len;
+
+       if (status != 0x03) {   /* There was an error. */
+               /*
+                * There is a tricky error noted by John Murphy,
+                * <[EMAIL PROTECTED]> to Russ Nelson: Even with full-sized
+                * buffers it's possible for a jabber packet to use two
+                * buffers, with only the last correctly noting the error.
+                */
+               if (status & 0x01)  /* Only count a general error at the */
+                       lp->stats.rx_errors++;       /* end of a packet. */
+               if (status & 0x20)
+                       lp->stats.rx_frame_errors++;
+               if (status & 0x10)
+                       lp->stats.rx_over_errors++;
+               if (status & 0x08)
+                       lp->stats.rx_crc_errors++;
+               if (status & 0x04)
+                       lp->stats.rx_fifo_errors++;
+               return 1;
+       }
+
+       pkt_len = (le32_to_cpu(rxp->msg_length) & 0xfff) - 4;
+
+       /* Discard oversize frames. */
+       if (unlikely(pkt_len > PKT_BUF_SZ - 2)) {
+               if (netif_msg_drv(lp))
+                       printk(KERN_ERR "%s: Impossible packet size %d!\n",
+                              dev->name, pkt_len);
+               lp->stats.rx_errors++;
+               return 1;
+       }
+       if (pkt_len < 60) {
+               if (netif_msg_rx_err(lp))
+                       printk(KERN_ERR "%s: Runt packet!\n", dev->name);
+               lp->stats.rx_errors++;
+               return 1;
+       }
+
+       if (pkt_len > rx_copybreak) {
+               struct sk_buff *newskb;
+
+               if ((newskb = dev_alloc_skb(PKT_BUF_SZ))) {
+                       skb_reserve(newskb, 2);
+                       skb = lp->rx_skbuff[entry];
+                       pci_unmap_single(lp->pci_dev,
+                                        lp->rx_dma_addr[entry],
+                                        PKT_BUF_SZ - 2,
+                                        PCI_DMA_FROMDEVICE);
+                       skb_put(skb, pkt_len);
+                       lp->rx_skbuff[entry] = newskb;
+                       newskb->dev = dev;
+                       lp->rx_dma_addr[entry] =
+                           pci_map_single(lp->pci_dev,
+                                          newskb->data,
+                                          PKT_BUF_SZ - 2,
+                                          PCI_DMA_FROMDEVICE);
+                       rxp->base = le32_to_cpu(lp->rx_dma_addr[entry]);
+                       rx_in_place = 1;
+               } else
+                       skb = NULL;
+       } else {
+               skb = dev_alloc_skb(pkt_len + 2);
+       }
+
+       if (skb == NULL) {
+               if (netif_msg_drv(lp))
+                       printk(KERN_ERR
+                              "%s: Memory squeeze, dropping packet.\n",
+                              dev->name);
+               lp->stats.rx_dropped++;
+               return 1;
+       }
+       skb->dev = dev;
+       if (!rx_in_place) {
+               skb_reserve(skb, 2);    /* 16 byte align */
+               skb_put(skb, pkt_len);  /* Make room */
+               pci_dma_sync_single_for_cpu(lp->pci_dev,
+                                           lp->rx_dma_addr[entry],
+                                           PKT_BUF_SZ - 2,
+                                           PCI_DMA_FROMDEVICE);
+               eth_copy_and_sum(skb,
+                                (unsigned char *)(lp->rx_skbuff[entry]->data),
+                                pkt_len, 0);
+               pci_dma_sync_single_for_device(lp->pci_dev,
+                                              lp->rx_dma_addr[entry],
+                                              PKT_BUF_SZ - 2,
+                                              PCI_DMA_FROMDEVICE);
+       }
+       lp->stats.rx_bytes += skb->len;
+       lp->stats.rx_packets++;
+       skb->protocol = eth_type_trans(skb, dev);
+       netif_rx(skb);
+       dev->last_rx = jiffies;
+       return 1;

This function returns 1, incrementing npackets, regardless of success or failure.


+static int pcnet32_rx(struct net_device *dev, int quota)

'quota' is an obvious change that doesn't exist in the unpatched driver


+{
+       struct pcnet32_private *lp = dev->priv;
+       int entry = lp->cur_rx & lp->rx_mod_mask;
+       struct pcnet32_rx_head *rxp = &lp->rx_ring[entry];
+       int npackets = 0;
+
+       /* If we own the next entry, it's a new packet. Send it up. */
+       while (quota > npackets && (short)le16_to_cpu(rxp->status) >= 0) {

ditto


+               npackets += pcnet32_rx_entry(dev, lp, rxp, entry);

as noted above, this function always returns 1.


+               /*
+                * The docs say that the buffer length isn't touched, but Andrew
+                * Boyd of QNX reports that some revs of the 79C965 clear it.
+                */
+               rxp->buf_length = le16_to_cpu(2 - PKT_BUF_SZ);
+               wmb();  /* Make sure owner changes after others are visible */
+               rxp->status = le16_to_cpu(0x8000);
+               entry = (++lp->cur_rx) & lp->rx_mod_mask;
+               rxp = &lp->rx_ring[entry];
+       }
+
+       return npackets;
+}
+
@@ -1661,6 +1889,7 @@ pcnet32_probe1(unsigned long ioaddr, int
        dev->ethtool_ops = &pcnet32_ethtool_ops;
        dev->tx_timeout = pcnet32_tx_timeout;
        dev->watchdog_timeo = (5 * HZ);
+       dev->weight = lp->rx_ring_size / 2;

Another change that belongs in the NAPI patch


@@ -2262,9 +2491,9 @@ pcnet32_interrupt(int irq, void *dev_id,
        struct net_device *dev = dev_id;
        struct pcnet32_private *lp;
        unsigned long ioaddr;
-       u16 csr0, rap;
        int boguscnt = max_interrupt_work;
-       int must_restart;
+       u16 csr0;
+       irqreturn_t rc = IRQ_HANDLED;
if (!dev) {
                if (pcnet32_debug & NETIF_MSG_INTR)
@@ -2422,183 +2543,27 @@ pcnet32_interrupt(int irq, void *dev_id,
                                       dev->name, csr0);
                        /* unlike for the lance, there is no restart needed */
                }
-
-               if (must_restart) {
+               pcnet32_rx(dev, dev->weight);

dev->weight is a NAPI thing


+               if (pcnet32_tx(dev)) {
                        /* reset the chip to clear the error condition, then 
restart */
                        lp->a.reset(ioaddr);
-                       lp->a.write_csr(ioaddr, 4, 0x0915);
-                       pcnet32_restart(dev, 0x0002);
+                       lp->a.write_csr(ioaddr, CSR4, 0x0915);
+                       pcnet32_restart(dev, CSR0_START);
                        netif_wake_queue(dev);
                }
+               csr0 = lp->a.read_csr(ioaddr, CSR0);
        }
- /* Set interrupt enable. */
-       lp->a.write_csr(ioaddr, 0, 0x0040);
-       lp->a.write_rap(ioaddr, rap);
+       /*Set interrupt enable. */
+       lp->a.write_csr(ioaddr, CSR0, CSR0_INTEN);
if (netif_msg_intr(lp))
                printk(KERN_DEBUG "%s: exiting interrupt, csr0=%#4.4x.\n",
-                      dev->name, lp->a.read_csr(ioaddr, 0));
+                      dev->name, lp->a.read_csr(ioaddr, CSR0));

seems like this stuff belongs in patch #3

-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to