6 Mar 21:23
Re: PATCH dm9ks interrupt handling
Michael Gao <mgao <at> neuros.us>
2007-03-06 20:23:17 GMT
2007-03-06 20:23:17 GMT
Chris, thanks for the patch, please see comment below.
Terry, please share your input on this.
On Tue, 2007-03-06 at 07:51 -0800, chrispe wrote:
> I was having problems with the OSD not receiving packets on a busy
> network. Typically I would notice this when a telnet session hung for
> 5-15 seconds when I pressed a key, but it also happened in many other
> circumstances.
>
> I discovered that the driver had some code that disabled the receive
> interrupt after 10 packets were received. The interrupt would be re-
> enabled by completing a transmit or by a 5 second timer. This somewhat
> crazy code was presumably designed to reduce the load on the processor
> (or maybe the IRQ system) when the device is not actively
> communicating. However, on a network with a moderate amount of
> broadcast traffic it was very common for the device to stop receiving
> for several seconds and then overrun its receive buffer when the
> interrupt was re-enabled.
5 second timer seems crazy to me too. However if we remove it
completely, this may bring us a situation where the interrupt is held
too long?
Would an approach like the following work better?
"if the number of receive packet > LIMIT, exit the interrupt handler
with Ethernet interrupt enabled.", please see blow code portion comment.
>
> I have removed this mechanism and also removed the bogus ioctl handler
> that just returned success to all calls, which makes things like the
> mii interface look like they work, but always return junk!
>
MG: Not sure about this, do we need this stub for driver compatibility
purpose? thus not getting NULL pointer if somewhere upper layer will
ever call the ioctl?
> Patch below.
>
> Chris.
>
> --- dm9ks.c.orig 2007-03-06 15:10:23.000000000 +0000
> +++ dm9ks.c 2007-03-06 15:15:16.000000000 +0000
> @@ -37,6 +37,8 @@
> V2.03 11/22/2005 Power-off and Power-on PHY in dmfe_init()
> support IOL
> 06/17/2006 Butchered for NTR3 OSD
> + 06/Mar/2007 Removed masking of RX interrupt when not
> transmitting
> + Removed ioctl handler which just said OK!
> */
>
> #if defined(MODVERSIONS)
> @@ -121,11 +123,6 @@
> #define DATAOFST1 2 /* OSD:use A0 as CMD line */
> #define DATAOFST2 0x10000 /* OSD:use A15 as CMD line */
>
> -/* Number of continuous Rx packets */
> -#define CONT_RX_PKT_CNT 10
> -
> -#define DMFE_TIMER_WUT jiffies+(HZ*5) /* timer wakeup time : 5
> second */
> -
> #define CARDNAME "dm9ks"
>
> #define MEM_MAPPED_IO 1
> @@ -198,8 +195,6 @@
> u8 device_wait_reset; /* device state */
> u8 Speed; /* current speed */
>
> - int cont_rx_pkt_cnt;/* current number of continuos rx packets */
> - struct timer_list timer;
> struct net_device_stats stats;
> unsigned char srom[128];
> spinlock_t lock;
> @@ -222,13 +217,11 @@
> static void dmfe_packet_receive(struct net_device *);
> static int dmfe_stop(struct net_device *);
> static struct net_device_stats * dmfe_get_stats(struct net_device
> *);
> -static int dmfe_do_ioctl(struct net_device *, struct ifreq *, int);
> #if LINUX_VERSION_CODE < KERNEL_VERSION(2,5,0)
> static void dmfe_interrupt(int , void *, struct pt_regs *);
> #else
> static irqreturn_t dmfe_interrupt(int , void *, struct pt_regs *);
> #endif
> -static void dmfe_timer(unsigned long);
> static void dmfe_init_dm9000(struct net_device *);
> static unsigned long cal_CRC(unsigned char *, unsigned int, u8);
> static u8 ior(board_info_t *, int);
> @@ -311,7 +304,6 @@
> dev->stop = &dmfe_stop;
> dev->get_stats = &dmfe_get_stats;
> dev->set_multicast_list = &dm9000_hash_table;
> - dev->do_ioctl = &dmfe_do_ioctl;
>
> #if defined(CHECKSUM)
> dev->features = dev->features | NETIF_F_NO_CSUM;
> @@ -382,7 +374,6 @@
> /* Init driver variable */
> db->reset_counter = 0;
> db->reset_tx_timeout = 0;
> - db->cont_rx_pkt_cnt = 0;
>
> /* check link state and media speed */
> db->Speed =10;
> @@ -404,12 +395,6 @@
> mdelay(1);
> }while(i<3000); /* wait 3 second */
> //printk("i=%d Speed=%d\n",i,db->Speed);
> - /* set and active a timer process */
> - init_timer(&db->timer);
> - db->timer.expires = DMFE_TIMER_WUT * 2;
> - db->timer.data = (unsigned long)dev;
> - db->timer.function = &dmfe_timer;
> - add_timer(&db->timer); //Move to DM9000 initiallization was
> finished.
>
> netif_start_queue(dev);
>
> @@ -588,7 +573,6 @@
>
> /* Saved the time stamp */
> dev->trans_start = jiffies;
> - db->cont_rx_pkt_cnt =0;
>
> /* Free this SKB */
> dev_kfree_skb(skb);
> @@ -608,9 +592,6 @@
> board_info_t *db = (board_info_t *)dev->priv;
> DMFE_DBUG(0, "dmfe_stop", 0);
>
> - /* deleted timer */
> - del_timer(&db->timer);
> -
> netif_stop_queue(dev);
>
> /* free interrupt */
> @@ -717,15 +698,8 @@
> if (int_status & DM9KS_TX_INTR)
> dmfe_tx_done(0);
>
> - if (db->cont_rx_pkt_cnt>=CONT_RX_PKT_CNT)
> - {
> - iow(db, DM9KS_IMR, 0xa2);
> - }
> - else
> - {
> - /* Re-enable interrupt mask */
> - iow(db, DM9KS_IMR, DM9KS_REGFF);
> - }
> + /* Re-enable interrupt mask */
> + iow(db, DM9KS_IMR, DM9KS_REGFF);
>
> /* Restore previous register address */
> PUTB(reg_save, db->io_addr);
> @@ -746,15 +720,6 @@
> return &db->stats;
> }
>
> -/*
> - Process the upper socket ioctl command
> -*/
> -static int dmfe_do_ioctl(struct net_device *dev, struct ifreq *ifr,
> int cmd)
> -{
> - DMFE_DBUG(0, "dmfe_do_ioctl()", 0);
> - return 0;
> -}
> -
> /* Our watchdog timed out. Called by the networking layer */
> static void
> dmfe_timeout(struct net_device *dev)
> @@ -805,26 +770,6 @@
> PUTB(reg_save, db->io_addr);
>
> }
> -/*
> - A periodic timer routine
> -*/
> -static void dmfe_timer(unsigned long data)
> -{
> - struct net_device * dev = (struct net_device *)data;
> - board_info_t *db = (board_info_t *)dev->priv;
> - DMFE_DBUG(0, "dmfe_timer()", 0);
> -
> - if (db->cont_rx_pkt_cnt>=CONT_RX_PKT_CNT)
> - {
> - db->cont_rx_pkt_cnt=0;
> - iow(db, DM9KS_IMR, DM9KS_REGFF);
> - }
> - /* Set timer again */
> - db->timer.expires = DMFE_TIMER_WUT;
> - add_timer(&db->timer);
> -
> - return;
> -}
>
> #if !defined(CHECKSUM)
> #define check_rx_ready(a) ((a) == 0x01)
> @@ -975,13 +920,6 @@
> dev->last_rx=jiffies;
> db->stats.rx_packets++;
> db->stats.rx_bytes += rx.desc.length;
> - db->cont_rx_pkt_cnt++;
> -
> - if (db->cont_rx_pkt_cnt>=CONT_RX_PKT_CNT)
> - {
> - dmfe_tx_done(0);
> - break;
> - }
MG: keep this code to exit the loop to prevent holding the interrupt for
too long? Without the interrupt being disabled, we'll probably return to
this routine immediately, but this will give other interrupt source a
chance to be responded?
> }
>
> }while((rxbyte & 0x01) == DM9KS_PKT_RDY);
>
>
RSS Feed