On Thu, Jan 01, 2004 at 09:01:57PM +0100, Geert Uytterhoeven wrote:
> Macintosh CS89x0 Ethernet: Netif updates (from Matthias Urlichs)
>
> --- linux-2.6.0/drivers/net/Kconfig Sun Oct 19 10:45:04 2003
> +++ linux-m68k-2.6.0/drivers/net/Kconfig Mon Oct 20 21:39:36 2003
> @@ -318,7 +318,7 @@
>
> config MAC89x0
> tristate "Macintosh CS89x0 based ethernet cards"
> - depends on NETDEVICES && MAC && BROKEN
> + depends on NETDEVICES && MAC
> ---help---
> Support for CS89x0 chipset based Ethernet cards. If you have a
> Nubus or LC-PDS network (Ethernet) card of this type, say Y and
> --- linux-2.6.0/drivers/net/mac89x0.c Mon May 5 10:31:32 2003
> +++ linux-m68k-2.6.0/drivers/net/mac89x0.c Mon Oct 20 21:34:24 2003
> @@ -128,7 +128,7 @@
> extern void reset_chip(struct net_device *dev);
> #endif
> static int net_open(struct net_device *dev);
> -static int net_send_packet(struct sk_buff *skb, struct net_device *dev);
> +static int net_send_packet(struct sk_buff *skb, struct net_device *dev);
> static irqreturn_t net_interrupt(int irq, void *dev_id, struct pt_regs *regs);
> static void set_multicast_list(struct net_device *dev);
> static void net_rx(struct net_device *dev);
> @@ -367,56 +367,37 @@
> static int
> net_send_packet(struct sk_buff *skb, struct net_device *dev)
> {
> - if (dev->tbusy) {
> - /* If we get here, some higher level has decided we are broken.
> - There should really be a "kick me" function call instead. */
> - int tickssofar = jiffies - dev->trans_start;
> - if (tickssofar < 5)
> - return 1;
> - if (net_debug > 0) printk("%s: transmit timed out, %s?\n", dev->name,
> - tx_done(dev) ? "IRQ conflict" : "network cable problem");
> - /* Try to restart the adaptor. */
> - dev->tbusy=0;
> - dev->trans_start = jiffies;
> - }
> -
> - /* Block a timer-based transmit from overlapping. This could better be
> - done with atomic_swap(1, dev->tbusy), but set_bit() works as well. */
> - if (test_and_set_bit(0, (void*)&dev->tbusy) != 0)
> - printk("%s: Transmitter access conflict.\n", dev->name);
> - else {
> - struct net_local *lp = (struct net_local *)dev->priv;
> - unsigned long flags;
> -
> - if (net_debug > 3)
> - printk("%s: sent %d byte packet of type %x\n",
> - dev->name, skb->len,
> - (skb->data[ETH_ALEN+ETH_ALEN] << 8)
> - | skb->data[ETH_ALEN+ETH_ALEN+1]);
> -
> - /* keep the upload from being interrupted, since we
> - ask the chip to start transmitting before the
> - whole packet has been completely uploaded. */
> - local_irq_save(flags);
> -
> - /* initiate a transmit sequence */
> - writereg(dev, PP_TxCMD, lp->send_cmd);
> - writereg(dev, PP_TxLength, skb->len);
> -
> - /* Test to see if the chip has allocated memory for the packet */
> - if ((readreg(dev, PP_BusST) & READY_FOR_TX_NOW) == 0) {
> - /* Gasp! It hasn't. But that shouldn't happen since
> - we're waiting for TxOk, so return 1 and requeue this packet. */
> - local_irq_restore(flags);
> - return 1;
> - }
> + struct net_local *lp = (struct net_local *)dev->priv;
> + unsigned long flags;
>
> - /* Write the contents of the packet */
> - memcpy_toio(dev->mem_start + PP_TxFrame, skb->data, skb->len+1);
> + if (net_debug > 3)
> + printk("%s: sent %d byte packet of type %x\n",
> + dev->name, skb->len,
> + (skb->data[ETH_ALEN+ETH_ALEN] << 8)
> + | skb->data[ETH_ALEN+ETH_ALEN+1]);
> +
> + /* keep the upload from being interrupted, since we
> + ask the chip to start transmitting before the
> + whole packet has been completely uploaded. */
> + local_irq_save(flags);
>
> + /* initiate a transmit sequence */
> + writereg(dev, PP_TxCMD, lp->send_cmd);
> + writereg(dev, PP_TxLength, skb->len);
> +
> + /* Test to see if the chip has allocated memory for the packet */
> + if ((readreg(dev, PP_BusST) & READY_FOR_TX_NOW) == 0) {
> + /* Gasp! It hasn't. But that shouldn't happen since
> + we're waiting for TxOk, so return 1 and requeue this packet. */
> local_irq_restore(flags);
> - dev->trans_start = jiffies;
> + return 1;
> }
I know the code was present before your change... but... this is very
wrong. Returning 1 only requeues the packet depending on the packet
scheduler. Other times the packet is simply dropped... So the above
assumption is incorrect.
If the NIC only has enough room for N maximally-sized packets (possibly
N==1, even), then the driver should netif_stop_queue() after that
capacity is reached, and netif_wake_queue() after the interrupt handler
signals completion.
> + /* Write the contents of the packet */
> + memcpy((void *)(dev->mem_start + PP_TxFrame), skb->data, skb->len+1);
Is dev->mem_start DMA memory?
The rest of the patch looks OK.
Jeff
-
: send the line "unsubscribe linux-net" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
