linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 475] HP300 LANCE
@ 2004-10-31 10:03 Geert Uytterhoeven
  2004-10-31 10:14 ` Jeff Garzik
  2004-11-01 14:22 ` Christoph Hellwig
  0 siblings, 2 replies; 20+ messages in thread
From: Geert Uytterhoeven @ 2004-10-31 10:03 UTC (permalink / raw)
  To: Linus Torvalds, Andrew Morton, Jeff Garzik
  Cc: Linux Kernel Development, linux-net, Geert Uytterhoeven

HP300 LANCE updates from Kars de Jong:
  - Updated HP LANCE driver to use the new DIO semantics
  - If only HP LANCE or MVME147 LANCE is selected, enable compile-time
    choice of LANCE register access. If both are defined, go through the
    function pointer
  - Added support for CONFIG_NET_POLL_CONTROLLER
  - Fixed problem with disabling board interrupts in hplance_close() which
    caused the driver to lock up

Signed-off-by: Kars de Jong <jongk@linux-m68k.org>
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>

--- linux-2.6.10-rc1/drivers/net/7990.c	2004-04-04 12:14:30.000000000 +0200
+++ linux-m68k-2.6.10-rc1/drivers/net/7990.c	2004-07-14 13:18:54.000000000 +0200
@@ -14,7 +14,6 @@
  */
 #include <linux/crc32.h>
 #include <linux/delay.h>
-#include <linux/dio.h>
 #include <linux/errno.h>
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
@@ -30,6 +29,7 @@
 #include <linux/slab.h>
 #include <linux/string.h>
 #include <linux/skbuff.h>
+#include <linux/irq.h>
 /* Used for the temporal inet entries and routing */
 #include <linux/socket.h>
 
@@ -38,18 +38,58 @@
 #include <asm/io.h>
 #include <asm/dma.h>
 #include <asm/pgtable.h>
+#ifdef CONFIG_HP300
+#include <asm/blinken.h>
+#endif
 
 #include "7990.h"
 
+#define WRITERAP(lp,x) out_be16(lp->base + LANCE_RAP, (x))
+#define WRITERDP(lp,x) out_be16(lp->base + LANCE_RDP, (x))
+#define READRDP(lp) in_be16(lp->base + LANCE_RDP)
+
+#if defined(CONFIG_HPLANCE) || defined(CONFIG_HPLANCE_MODULE)
+#include "hplance.h"
+
+#undef WRITERAP
+#undef WRITERDP
+#undef READRDP
+
+#if defined(CONFIG_MVME147_NET) || defined(CONFIG_MVME147_NET_MODULE)
+
 /* Lossage Factor Nine, Mr Sulu. */
-#define WRITERAP(x) (lp->writerap(lp,x))
-#define WRITERDP(x) (lp->writerdp(lp,x))
-#define READRDP() (lp->readrdp(lp))
-/* These used to be ll->rap = x, ll->rdp = x, and (ll->rdp). Sigh. 
- * If you want to switch them back then 
- * #define DECLARE_LL volatile struct lance_regs *ll = lp->ll
- */
-#define DECLARE_LL /* nothing to declare */
+#define WRITERAP(lp,x) (lp->writerap(lp,x))
+#define WRITERDP(lp,x) (lp->writerdp(lp,x))
+#define READRDP(lp) (lp->readrdp(lp))
+
+#else
+
+/* These inlines can be used if only CONFIG_HPLANCE is defined */
+static inline void WRITERAP(struct lance_private *lp, __u16 value)
+{
+	do {
+		out_be16(lp->base + HPLANCE_REGOFF + LANCE_RAP, value);
+	} while ((in_8(lp->base + HPLANCE_STATUS) & LE_ACK) == 0);
+}
+
+static inline void WRITERDP(struct lance_private *lp, __u16 value)
+{
+	do {
+		out_be16(lp->base + HPLANCE_REGOFF + LANCE_RDP, value);
+	} while ((in_8(lp->base + HPLANCE_STATUS) & LE_ACK) == 0);
+}
+
+static inline __u16 READRDP(struct lance_private *lp)
+{
+	__u16 value;
+	do {
+		value = in_be16(lp->base + HPLANCE_REGOFF + LANCE_RDP);
+	} while ((in_8(lp->base + HPLANCE_STATUS) & LE_ACK) == 0);
+	return value;
+}
+
+#endif
+#endif /* CONFIG_HPLANCE || CONFIG_HPLANCE_MODULE */
 
 /* debugging output macros, various flavours */
 /* #define TEST_HITS */
@@ -79,19 +119,18 @@
 {
         volatile struct lance_init_block *aib = lp->lance_init_block;
         int leptr;
-        DECLARE_LL;
 
         leptr = LANCE_ADDR (aib);
 
-        WRITERAP(LE_CSR1);                        /* load address of init block */
-        WRITERDP(leptr & 0xFFFF);
-        WRITERAP(LE_CSR2);
-        WRITERDP(leptr >> 16);
-        WRITERAP(LE_CSR3);
-        WRITERDP(lp->busmaster_regval);           /* set byteswap/ALEctrl/byte ctrl */
+        WRITERAP(lp, LE_CSR1);                    /* load address of init block */
+        WRITERDP(lp, leptr & 0xFFFF);
+        WRITERAP(lp, LE_CSR2);
+        WRITERDP(lp, leptr >> 16);
+        WRITERAP(lp, LE_CSR3);
+        WRITERDP(lp, lp->busmaster_regval);       /* set byteswap/ALEctrl/byte ctrl */
 
         /* Point back to csr0 */
-        WRITERAP(LE_CSR0);
+        WRITERAP(lp, LE_CSR0);
 }
 
 /* #define to 0 or 1 appropriately */
@@ -192,24 +231,23 @@
 static int init_restart_lance (struct lance_private *lp)
 {
         int i;
-        DECLARE_LL;
 
-        WRITERAP(LE_CSR0);
-        WRITERDP(LE_C0_INIT);
+        WRITERAP(lp, LE_CSR0);
+        WRITERDP(lp, LE_C0_INIT);
 
         /* Need a hook here for sunlance ledma stuff */
 
         /* Wait for the lance to complete initialization */
-        for (i = 0; (i < 100) && !(READRDP() & (LE_C0_ERR | LE_C0_IDON)); i++)
+        for (i = 0; (i < 100) && !(READRDP(lp) & (LE_C0_ERR | LE_C0_IDON)); i++)
                 barrier();
-        if ((i == 100) || (READRDP() & LE_C0_ERR)) {
-                printk ("LANCE unopened after %d ticks, csr0=%4.4x.\n", i, READRDP());
+        if ((i == 100) || (READRDP(lp) & LE_C0_ERR)) {
+                printk ("LANCE unopened after %d ticks, csr0=%4.4x.\n", i, READRDP(lp));
                 return -1;
         }
 
         /* Clear IDON by writing a "1", enable interrupts and start lance */
-        WRITERDP(LE_C0_IDON);
-        WRITERDP(LE_C0_INEA | LE_C0_STRT);
+        WRITERDP(lp, LE_C0_IDON);
+        WRITERDP(lp, LE_C0_INEA | LE_C0_STRT);
 
         return 0;
 }
@@ -218,11 +256,10 @@
 {
         struct lance_private *lp = netdev_priv(dev);
         int status;
-        DECLARE_LL;
     
         /* Stop the lance */
-        WRITERAP(LE_CSR0);
-        WRITERDP(LE_C0_STOP);
+        WRITERAP(lp, LE_CSR0);
+        WRITERDP(lp, LE_C0_STOP);
 
         load_csrs (lp);
         lance_init_ring (dev);
@@ -245,7 +282,6 @@
 #ifdef TEST_HITS
         int i;
 #endif
-        DECLARE_LL;
 
 #ifdef TEST_HITS
         printk ("[");
@@ -259,8 +295,10 @@
         }
         printk ("]");
 #endif
-    
-        WRITERDP(LE_C0_RINT | LE_C0_INEA);     /* ack Rx int, reenable ints */
+#ifdef CONFIG_HP300
+	blinken_leds(0x40, 0);
+#endif    
+        WRITERDP(lp, LE_C0_RINT | LE_C0_INEA);     /* ack Rx int, reenable ints */
         for (rd = &ib->brx_ring [lp->rx_new];     /* For each Rx ring we own... */
              !((bits = rd->rmd1_bits) & LE_R1_OWN);
              rd = &ib->brx_ring [lp->rx_new]) {
@@ -321,10 +359,12 @@
         volatile struct lance_tx_desc *td;
         int i, j;
         int status;
-        DECLARE_LL;
 
+#ifdef CONFIG_HP300
+	blinken_leds(0x80, 0);
+#endif
         /* csr0 is 2f3 */
-        WRITERDP(LE_C0_TINT | LE_C0_INEA);
+        WRITERDP(lp, LE_C0_TINT | LE_C0_INEA);
         /* csr0 is 73 */
 
         j = lp->tx_old;
@@ -349,8 +389,8 @@
                                         printk("%s: Carrier Lost, trying %s\n",
                                                dev->name, lp->tpe?"TPE":"AUI");
                                         /* Stop the lance */
-                                        WRITERAP(LE_CSR0);
-                                        WRITERDP(LE_C0_STOP);
+                                        WRITERAP(lp, LE_CSR0);
+                                        WRITERDP(lp, LE_C0_STOP);
                                         lance_init_ring (dev);
                                         load_csrs (lp);
                                         init_restart_lance (lp);
@@ -366,8 +406,8 @@
                                 printk ("%s: Tx: ERR_BUF|ERR_UFL, restarting\n",
                                         dev->name);
                                 /* Stop the lance */
-                                WRITERAP(LE_CSR0);
-                                WRITERDP(LE_C0_STOP);
+                                WRITERAP(lp, LE_CSR0);
+                                WRITERDP(lp, LE_C0_STOP);
                                 lance_init_ring (dev);
                                 load_csrs (lp);
                                 init_restart_lance (lp);
@@ -393,7 +433,7 @@
                 j = (j + 1) & lp->tx_ring_mod_mask;
         }
         lp->tx_old = j;
-        WRITERDP(LE_C0_TINT | LE_C0_INEA);
+        WRITERDP(lp, LE_C0_TINT | LE_C0_INEA);
         return 0;
 }
 
@@ -403,26 +443,25 @@
         struct net_device *dev = (struct net_device *)dev_id;
         struct lance_private *lp = netdev_priv(dev);
         int csr0;
-        DECLARE_LL;
 
 	spin_lock (&lp->devlock);
 
-        WRITERAP(LE_CSR0);              /* LANCE Controller Status */
-        csr0 = READRDP();
+        WRITERAP(lp, LE_CSR0);              /* LANCE Controller Status */
+        csr0 = READRDP(lp);
 
         PRINT_RINGS();
         
         if (!(csr0 & LE_C0_INTR)) {     /* Check if any interrupt has */
-		spin_lock (&lp->devlock);
+		spin_unlock (&lp->devlock);
                 return IRQ_NONE;        /* been generated by the Lance. */
 	}
 
         /* Acknowledge all the interrupt sources ASAP */
-        WRITERDP(csr0 & ~(LE_C0_INEA|LE_C0_TDMD|LE_C0_STOP|LE_C0_STRT|LE_C0_INIT));
+        WRITERDP(lp, csr0 & ~(LE_C0_INEA|LE_C0_TDMD|LE_C0_STOP|LE_C0_STRT|LE_C0_INIT));
 
         if ((csr0 & LE_C0_ERR)) {
                 /* Clear the error condition */
-                WRITERDP(LE_C0_BABL|LE_C0_ERR|LE_C0_MISS|LE_C0_INEA);
+                WRITERDP(lp, LE_C0_BABL|LE_C0_ERR|LE_C0_MISS|LE_C0_INEA);
         }
 
         if (csr0 & LE_C0_RINT)
@@ -440,7 +479,7 @@
                 printk("%s: Bus master arbitration failure, status %4.4x.\n", 
                        dev->name, csr0);
                 /* Restart the chip. */
-                WRITERDP(LE_C0_STRT);
+                WRITERDP(lp, LE_C0_STRT);
         }
 
         if (lp->tx_full && netif_queue_stopped(dev) && (TX_BUFFS_AVAIL >= 0)) {
@@ -448,8 +487,8 @@
 		netif_wake_queue (dev);
         }
         
-        WRITERAP(LE_CSR0);
-        WRITERDP(LE_C0_BABL|LE_C0_CERR|LE_C0_MISS|LE_C0_MERR|LE_C0_IDON|LE_C0_INEA);
+        WRITERAP(lp, LE_CSR0);
+        WRITERDP(lp, LE_C0_BABL|LE_C0_CERR|LE_C0_MISS|LE_C0_MERR|LE_C0_IDON|LE_C0_INEA);
 
 	spin_unlock (&lp->devlock);
 	return IRQ_HANDLED;
@@ -459,7 +498,6 @@
 {
         struct lance_private *lp = netdev_priv(dev);
 	int res;
-        DECLARE_LL;
         
         /* Install the Interrupt handler. Or we could shunt this out to specific drivers? */
         if (request_irq(lp->irq, lance_interrupt, 0, lp->name, dev))
@@ -475,13 +513,12 @@
 int lance_close (struct net_device *dev)
 {
         struct lance_private *lp = netdev_priv(dev);
-        DECLARE_LL;
         
 	netif_stop_queue (dev);
 
         /* Stop the LANCE */
-        WRITERAP(LE_CSR0);
-        WRITERDP(LE_C0_STOP);
+        WRITERAP(lp, LE_CSR0);
+        WRITERDP(lp, LE_C0_STOP);
 
         free_irq(lp->irq, dev);
 
@@ -504,7 +541,6 @@
         int entry, skblen, len;
         static int outs;
 	unsigned long flags;
-        DECLARE_LL;
 
         if (!TX_BUFFS_AVAIL)
                 return -1;
@@ -540,7 +576,7 @@
 
         outs++;
         /* Kick the lance: transmit now */
-        WRITERDP(LE_C0_INEA | LE_C0_TDMD);
+        WRITERDP(lp, LE_C0_INEA | LE_C0_TDMD);
         dev->trans_start = jiffies;
         dev_kfree_skb (skb);
     
@@ -604,7 +640,6 @@
         struct lance_private *lp = netdev_priv(dev);
         volatile struct lance_init_block *ib = lp->init_block;
 	int stopped;
-        DECLARE_LL;
 
 	stopped = netif_queue_stopped(dev);
 	if (!stopped)
@@ -613,8 +648,8 @@
         while (lp->tx_old != lp->tx_new)
                 schedule();
 
-        WRITERAP(LE_CSR0);
-        WRITERDP(LE_C0_STOP);
+        WRITERAP(lp, LE_CSR0);
+        WRITERDP(lp, LE_C0_STOP);
         lance_init_ring (dev);
 
         if (dev->flags & IFF_PROMISC) {
@@ -630,4 +665,17 @@
 		netif_start_queue (dev);
 }
 
+#ifdef CONFIG_NET_POLL_CONTROLLER
+void lance_poll(struct net_device *dev)
+{
+	struct lance_private *lp = netdev_priv(dev);
+
+	spin_lock (&lp->devlock);
+	WRITERAP(lp, LE_CSR0);
+	WRITERDP(lp, LE_C0_STRT);
+	spin_unlock (&lp->devlock);
+	lance_interrupt(dev->irq, dev, NULL);
+}
+#endif
+
 MODULE_LICENSE("GPL");
--- linux-2.6.10-rc1/drivers/net/7990.h	2002-01-15 19:24:07.000000000 +0100
+++ linux-m68k-2.6.10-rc1/drivers/net/7990.h	2004-07-14 13:18:54.000000000 +0200
@@ -14,11 +14,8 @@
 #define _7990_H
 
 /* The lance only has two register locations. We communicate mostly via memory. */
-struct lance_regs 
-{
-        unsigned short rdp;                       /* Register Data Port */
-        unsigned short rap;                       /* Register Address Port */
-};
+#define LANCE_RDP	0	/* Register Data Port */
+#define LANCE_RAP	2	/* Register Address Port */
 
 /* Transmit/receive ring definitions.
  * We allow the specific drivers to override these defaults if they want to.
@@ -104,7 +101,7 @@
 struct lance_private
 {
         char *name;
-        volatile struct lance_regs *ll;
+	unsigned long base;
         volatile struct lance_init_block *init_block; /* CPU address of RAM */
         volatile struct lance_init_block *lance_init_block; /* LANCE address of RAM */
         
@@ -252,5 +249,8 @@
 extern struct net_device_stats *lance_get_stats (struct net_device *dev);
 extern void lance_set_multicast (struct net_device *dev);
 extern void lance_tx_timeout(struct net_device *dev);
+#ifdef CONFIG_NET_POLL_CONTROLLER
+extern void lance_poll(struct net_device *dev);
+#endif
 
 #endif /* ndef _7990_H */
--- linux-2.6.10-rc1/drivers/net/hplance.c	2004-04-04 12:14:46.000000000 +0200
+++ linux-m68k-2.6.10-rc1/drivers/net/hplance.c	2004-07-14 13:18:56.000000000 +0200
@@ -42,7 +42,6 @@
 struct hplance_private {
   struct lance_private lance;
   unsigned int scode;
-  void *base;
 };
 
 /* function prototypes... This is easy because all the grot is in the
@@ -91,15 +90,17 @@
         {
                 int scode = dio_find(DIO_ID_LAN);
                                 
-                if (!scode)
+                if (scode < 0)
                         break;
                 
 		dio_config_board(scode);
                 hplance_init(dev, scode);
 		if (!register_netdev(dev)) {
+#ifdef MODULE
 			struct hplance_private *lp = netdev_priv(dev);
 			lp->next_module = root_hplance_dev;
 			root_hplance_dev = lp;
+#endif
 			return dev;
 		}
 		cleanup_card(dev);
@@ -112,11 +113,12 @@
 static void __init hplance_init(struct net_device *dev, int scode)
 {
         const char *name = dio_scodetoname(scode);
-        void *va = dio_scodetoviraddr(scode);
+	unsigned long pa = dio_scodetophysaddr(scode);
+        unsigned long va = (pa + DIO_VIRADDRBASE);
         struct hplance_private *lp;
         int i;
         
-        printk("%s: %s; select code %d, addr", dev->name, name, scode);
+        printk(KERN_INFO "%s: %s; select code %d, addr", dev->name, name, scode);
 
         /* reset the board */
         out_8(va+DIO_IDOFF, 0xff);
@@ -123,9 +125,12 @@
         udelay(100);                              /* ariba! ariba! udelay! udelay! */
 
         /* Fill the dev fields */
-        dev->base_addr = (unsigned long)va;
+        dev->base_addr = va;
         dev->open = &hplance_open;
         dev->stop = &hplance_close;
+#ifdef CONFIG_NET_POLL_CONTROLLER
+        dev->poll_controller = lance_poll;
+#endif
         dev->hard_start_xmit = &lance_start_xmit;
         dev->get_stats = &lance_get_stats;
         dev->set_multicast_list = &lance_set_multicast;
@@ -143,7 +148,7 @@
         
         lp = netdev_priv(dev);
         lp->lance.name = (char*)name;                   /* discards const, shut up gcc */
-        lp->lance.ll = (struct lance_regs *)(va + HPLANCE_REGOFF);
+        lp->lance.base = va;
         lp->lance.init_block = (struct lance_init_block *)(va + HPLANCE_MEMOFF); /* CPU addr */
         lp->lance.lance_init_block = 0;                 /* LANCE addr of same RAM */
         lp->lance.busmaster_regval = LE_C3_BSWP;        /* we're bigendian */
@@ -156,7 +161,6 @@
         lp->lance.rx_ring_mod_mask = RX_RING_MOD_MASK;
         lp->lance.tx_ring_mod_mask = TX_RING_MOD_MASK;
         lp->scode = scode;
-	lp->base = va;
 	printk(", irq %d\n", lp->lance.irq);
 }
 
@@ -165,53 +169,49 @@
  */
 static void hplance_writerap(void *priv, unsigned short value)
 {
-	struct hplance_private *lp = (struct hplance_private *)priv;
-        struct hplance_reg *hpregs = (struct hplance_reg *)lp->base;
-        do {
-                lp->lance.ll->rap = value;
-        } while ((hpregs->status & LE_ACK) == 0);
+	struct lance_private *lp = (struct lance_private *)priv;
+	do {
+		out_be16(lp->base + HPLANCE_REGOFF + LANCE_RAP, value);
+	} while ((in_8(lp->base + HPLANCE_STATUS) & LE_ACK) == 0);
 }
 
 static void hplance_writerdp(void *priv, unsigned short value)
 {
-	struct hplance_private *lp = (struct hplance_private *)priv;
-        struct hplance_reg *hpregs = (struct hplance_reg *)lp->base;
-        do {
-                lp->lance.ll->rdp = value;
-        } while ((hpregs->status & LE_ACK) == 0);
+	struct lance_private *lp = (struct lance_private *)priv;
+	do {
+		out_be16(lp->base + HPLANCE_REGOFF + LANCE_RDP, value);
+	} while ((in_8(lp->base + HPLANCE_STATUS) & LE_ACK) == 0);
 }
 
 static unsigned short hplance_readrdp(void *priv)
 {
-        unsigned short val;
-	struct hplance_private *lp = (struct hplance_private *)priv;
-        struct hplance_reg *hpregs = (struct hplance_reg *)lp->base;
-        do {
-                val = lp->lance.ll->rdp;
-        } while ((hpregs->status & LE_ACK) == 0);
-        return val;
+	struct lance_private *lp = (struct lance_private *)priv;
+	__u16 value;
+	do {
+		value = in_be16(lp->base + HPLANCE_REGOFF + LANCE_RDP);
+	} while ((in_8(lp->base + HPLANCE_STATUS) & LE_ACK) == 0);
+	return value;
 }
 
 static int hplance_open(struct net_device *dev)
 {
         int status;
-        struct hplance_private *lp = netdev_priv(dev);
-        struct hplance_reg *hpregs = (struct hplance_reg *)lp->base;
+        struct lance_private *lp = netdev_priv(dev);
         
         status = lance_open(dev);                 /* call generic lance open code */
         if (status)
                 return status;
         /* enable interrupts at board level. */
-        out_8(&(hpregs->status), LE_IE);
+        out_8(lp->base + HPLANCE_STATUS, LE_IE);
 
         return 0;
 }
 
 static int hplance_close(struct net_device *dev)
 {
-        struct hplance_private *lp = netdev_priv(dev);
-        struct hplance_reg *hpregs = (struct hplance_reg *)lp->base;
-        out_8(&(hpregs->status), 8);              /* disable interrupts at boardlevel */
+        struct lance_private *lp = netdev_priv(dev);
+
+        out_8(lp->base + HPLANCE_STATUS, 0);	/* disable interrupts at boardlevel */
         lance_close(dev);
         return 0;
 }
--- linux-2.6.10-rc1/drivers/net/hplance.h	2001-10-22 01:51:53.000000000 +0200
+++ linux-m68k-2.6.10-rc1/drivers/net/hplance.h	2004-07-14 13:18:56.000000000 +0200
@@ -4,15 +4,10 @@
  */
 
 /* Registers */
-struct hplance_reg
-{
-        u_char pad0;
-        volatile u_char id;                       /* DIO register: ID byte */
-        u_char pad1;
-        volatile u_char status;                   /* DIO register: interrupt enable */
-};
+#define HPLANCE_ID		0x01		/* DIO register: ID byte */
+#define HPLANCE_STATUS		0x03		/* DIO register: interrupt enable/status */
 
-/* Control and status bits for the hplance->status register */
+/* Control and status bits for the status register */
 #define LE_IE 0x80                                /* interrupt enable */
 #define LE_IR 0x40                                /* interrupt requested */
 #define LE_LOCK 0x08                              /* lock status register */
@@ -25,7 +20,7 @@
 /* These are the offsets for the DIO regs (hplance_reg), lance_ioreg,
  * memory and NVRAM:
  */
-#define HPLANCE_IDOFF 0                           /* board baseaddr, struct hplance_reg */
-#define HPLANCE_REGOFF 0x4000                     /* struct lance_regs */
+#define HPLANCE_IDOFF 0                           /* board baseaddr */
+#define HPLANCE_REGOFF 0x4000                     /* lance registers */
 #define HPLANCE_MEMOFF 0x8000                     /* struct lance_init_block */
 #define HPLANCE_NVRAMOFF 0xC008                   /* etheraddress as one *nibble* per byte */
--- linux-2.6.10-rc1/drivers/net/mvme147.c	2004-02-18 20:36:08.000000000 +0100
+++ linux-m68k-2.6.10-rc1/drivers/net/mvme147.c	2004-07-14 13:18:56.000000000 +0200
@@ -18,7 +18,6 @@
 /* Used for the temporal inet entries and routing */
 #include <linux/socket.h>
 #include <linux/route.h>
-#include <linux/dio.h>
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
 #include <linux/skbuff.h>
@@ -40,7 +39,6 @@
 /* Our private data structure */
 struct m147lance_private {
 	struct lance_private lance;
-	void *base;
 	unsigned long ram;
 };
 
@@ -51,9 +49,9 @@
  */
 static int m147lance_open(struct net_device *dev);
 static int m147lance_close(struct net_device *dev);
-static void m147lance_writerap(struct m147lance_private *lp, unsigned short value);
-static void m147lance_writerdp(struct m147lance_private *lp, unsigned short value);
-static unsigned short m147lance_readrdp(struct m147lance_private *lp);
+static void m147lance_writerap(struct lance_private *lp, unsigned short value);
+static void m147lance_writerdp(struct lance_private *lp, unsigned short value);
+static unsigned short m147lance_readrdp(struct lance_private *lp);
 
 typedef void (*writerap_t)(void *, unsigned short);
 typedef void (*writerdp_t)(void *, unsigned short);
@@ -122,7 +120,7 @@
 	}
 
 	lp->lance.name = (char*)name;                   /* discards const, shut up gcc */
-	lp->lance.ll = (struct lance_regs *)(dev->base_addr);
+	lp->lance.base = dev->base_addr;
 	lp->lance.init_block = (struct lance_init_block *)(lp->ram); /* CPU addr */
 	lp->lance.lance_init_block = (struct lance_init_block *)(lp->ram);                 /* LANCE addr of same RAM */
 	lp->lance.busmaster_regval = LE_C3_BSWP;        /* we're bigendian */
@@ -145,19 +143,19 @@
 	return dev;
 }
 
-static void m147lance_writerap(struct m147lance_private *lp, unsigned short value)
+static void m147lance_writerap(struct lance_private *lp, unsigned short value)
 {
-	lp->lance.ll->rap = value;
+	out_be16(lp->base + LANCE_RAP, value);
 }
 
-static void m147lance_writerdp(struct m147lance_private *lp, unsigned short value)
+static void m147lance_writerdp(struct lance_private *lp, unsigned short value)
 {
-	lp->lance.ll->rdp = value;
+	out_be16(lp->base + LANCE_RDP, value);
 }
 
-static unsigned short m147lance_readrdp(struct m147lance_private *lp)
+static unsigned short m147lance_readrdp(struct lance_private *lp)
 {
-	return lp->lance.ll->rdp;
+	return in_be16(lp->base + LANCE_RDP);
 }
 
 static int m147lance_open(struct net_device *dev)

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 475] HP300 LANCE
  2004-10-31 10:03 [PATCH 475] HP300 LANCE Geert Uytterhoeven
@ 2004-10-31 10:14 ` Jeff Garzik
  2004-10-31 10:19   ` Andrew Morton
  2004-11-01 14:22 ` Christoph Hellwig
  1 sibling, 1 reply; 20+ messages in thread
From: Jeff Garzik @ 2004-10-31 10:14 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Linus Torvalds, Andrew Morton, Linux Kernel Development, linux-net

content looks OK, but patch appears to be whitespace-challenged...

	Jeff




^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 475] HP300 LANCE
  2004-10-31 10:14 ` Jeff Garzik
@ 2004-10-31 10:19   ` Andrew Morton
  2004-10-31 10:41     ` Jeff Garzik
  0 siblings, 1 reply; 20+ messages in thread
From: Andrew Morton @ 2004-10-31 10:19 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: geert, torvalds, linux-kernel, linux-net

Jeff Garzik <jgarzik@pobox.com> wrote:
>
> content looks OK, but patch appears to be whitespace-challenged...
> 

It applies successfully.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 475] HP300 LANCE
  2004-10-31 10:19   ` Andrew Morton
@ 2004-10-31 10:41     ` Jeff Garzik
  2004-10-31 10:48       ` Andrew Morton
  0 siblings, 1 reply; 20+ messages in thread
From: Jeff Garzik @ 2004-10-31 10:41 UTC (permalink / raw)
  To: Andrew Morton; +Cc: geert, torvalds, linux-kernel, linux-net

Andrew Morton wrote:
> Jeff Garzik <jgarzik@pobox.com> wrote:
> 
>>content looks OK, but patch appears to be whitespace-challenged...
>>
> 
> 
> It applies successfully.

I'm talking about the _other_ type of "whitespace challenged", such as,

-        volatile struct lance_regs *ll;
+	unsigned long base;

	and

-        void *va = dio_scodetoviraddr(scode);
+	unsigned long pa = dio_scodetophysaddr(scode);
+        unsigned long va = (pa + DIO_VIRADDRBASE);

Reading through the patch you can see other one-space-off spots.

	Jeff



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 475] HP300 LANCE
  2004-10-31 10:41     ` Jeff Garzik
@ 2004-10-31 10:48       ` Andrew Morton
  2004-10-31 10:57         ` Jeff Garzik
  0 siblings, 1 reply; 20+ messages in thread
From: Andrew Morton @ 2004-10-31 10:48 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: geert, torvalds, linux-kernel, linux-net

Jeff Garzik <jgarzik@pobox.com> wrote:
>
> Andrew Morton wrote:
> > Jeff Garzik <jgarzik@pobox.com> wrote:
> > 
> >>content looks OK, but patch appears to be whitespace-challenged...
> >>
> > 
> > 
> > It applies successfully.
> 
> I'm talking about the _other_ type of "whitespace challenged", such as,
> 
> -        volatile struct lance_regs *ll;
> +	unsigned long base;
> 
> 	and
> 
> -        void *va = dio_scodetoviraddr(scode);
> +	unsigned long pa = dio_scodetophysaddr(scode);
> +        unsigned long va = (pa + DIO_VIRADDRBASE);
> 
> Reading through the patch you can see other one-space-off spots.
> 

That's because the stoopid driver is using spaces instead of tabs all over
the place.  It comes out visually OK once the patch is applied.  But it's a
useful reminder of how much dreck we have in the tree.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 475] HP300 LANCE
  2004-10-31 10:48       ` Andrew Morton
@ 2004-10-31 10:57         ` Jeff Garzik
  2004-10-31 11:09           ` Andrew Morton
  0 siblings, 1 reply; 20+ messages in thread
From: Jeff Garzik @ 2004-10-31 10:57 UTC (permalink / raw)
  To: Andrew Morton; +Cc: geert, torvalds, linux-kernel, linux-net

On Sun, Oct 31, 2004 at 02:48:40AM -0800, Andrew Morton wrote:
> > -        void *va = dio_scodetoviraddr(scode);
> > +	unsigned long pa = dio_scodetophysaddr(scode);
> > +        unsigned long va = (pa + DIO_VIRADDRBASE);


> That's because the stoopid driver is using spaces instead of tabs all over
> the place.  It comes out visually OK once the patch is applied.  But it's a
> useful reminder of how much dreck we have in the tree.

Did you see the above quoted patch chunk?  The patch is inconsistent
with _itself_, adding 'pa' and 'va' with different idents (but when they
should be at the same identation level).

	Jeff




^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 475] HP300 LANCE
  2004-10-31 10:57         ` Jeff Garzik
@ 2004-10-31 11:09           ` Andrew Morton
  2004-10-31 11:44             ` Geert Uytterhoeven
  2004-10-31 11:50             ` Herbert Xu
  0 siblings, 2 replies; 20+ messages in thread
From: Andrew Morton @ 2004-10-31 11:09 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: geert, torvalds, linux-kernel, linux-net

Jeff Garzik <jgarzik@pobox.com> wrote:
>
> On Sun, Oct 31, 2004 at 02:48:40AM -0800, Andrew Morton wrote:
>  > > -        void *va = dio_scodetoviraddr(scode);
>  > > +	unsigned long pa = dio_scodetophysaddr(scode);
>  > > +        unsigned long va = (pa + DIO_VIRADDRBASE);
> 
> 
>  > That's because the stoopid driver is using spaces instead of tabs all over
>  > the place.  It comes out visually OK once the patch is applied.  But it's a
>  > useful reminder of how much dreck we have in the tree.
> 
>  Did you see the above quoted patch chunk?  The patch is inconsistent
>  with _itself_, adding 'pa' and 'va' with different idents (but when they
>  should be at the same identation level).

Trust me ;)

http://www.zip.com.au/~akpm/linux/patches/stuff/x.jpg

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 475] HP300 LANCE
  2004-10-31 11:09           ` Andrew Morton
@ 2004-10-31 11:44             ` Geert Uytterhoeven
  2004-10-31 11:50             ` Herbert Xu
  1 sibling, 0 replies; 20+ messages in thread
From: Geert Uytterhoeven @ 2004-10-31 11:44 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Jeff Garzik, Linus Torvalds, Linux Kernel Development, linux-net

On Sun, 31 Oct 2004, Andrew Morton wrote:
> Jeff Garzik <jgarzik@pobox.com> wrote:
> >
> > On Sun, Oct 31, 2004 at 02:48:40AM -0800, Andrew Morton wrote:
> >  > > -        void *va = dio_scodetoviraddr(scode);
> >  > > +	unsigned long pa = dio_scodetophysaddr(scode);
> >  > > +        unsigned long va = (pa + DIO_VIRADDRBASE);
> > 
> > 
> >  > That's because the stoopid driver is using spaces instead of tabs all over
> >  > the place.  It comes out visually OK once the patch is applied.  But it's a
> >  > useful reminder of how much dreck we have in the tree.

I did a big whitespace cleanup in arch/m68k a while ago, but I didn't dare to
touch drivers/...

> >  Did you see the above quoted patch chunk?  The patch is inconsistent
> >  with _itself_, adding 'pa' and 'va' with different idents (but when they
> >  should be at the same identation level).
> 
> Trust me ;)
> 
> http://www.zip.com.au/~akpm/linux/patches/stuff/x.jpg

Yep, tabstops should be 1+n*8 (sometimes 2+n*8) instead of n*8 when looking at
patches :-)

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 475] HP300 LANCE
  2004-10-31 11:09           ` Andrew Morton
  2004-10-31 11:44             ` Geert Uytterhoeven
@ 2004-10-31 11:50             ` Herbert Xu
  2004-10-31 11:58               ` Jeff Garzik
  1 sibling, 1 reply; 20+ messages in thread
From: Herbert Xu @ 2004-10-31 11:50 UTC (permalink / raw)
  To: Andrew Morton; +Cc: jgarzik, geert, torvalds, linux-kernel, linux-net

Andrew Morton <akpm@osdl.org> wrote:
> Jeff Garzik <jgarzik@pobox.com> wrote:
>>
>> On Sun, Oct 31, 2004 at 02:48:40AM -0800, Andrew Morton wrote:
>>  > > -        void *va = dio_scodetoviraddr(scode);
>>  > > +        unsigned long pa = dio_scodetophysaddr(scode);
>>  > > +        unsigned long va = (pa + DIO_VIRADDRBASE);
>> 
>>  Did you see the above quoted patch chunk?  The patch is inconsistent
>>  with _itself_, adding 'pa' and 'va' with different idents (but when they
>>  should be at the same identation level).
> 
> Trust me ;)

What Jeff means is that the patch is using a tab for pa and 8 spaces
for va.
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 475] HP300 LANCE
  2004-10-31 11:50             ` Herbert Xu
@ 2004-10-31 11:58               ` Jeff Garzik
  0 siblings, 0 replies; 20+ messages in thread
From: Jeff Garzik @ 2004-10-31 11:58 UTC (permalink / raw)
  To: Herbert Xu; +Cc: Andrew Morton, geert, torvalds, linux-kernel, linux-net

Herbert Xu wrote:
> Andrew Morton <akpm@osdl.org> wrote:
> 
>>Jeff Garzik <jgarzik@pobox.com> wrote:
>>
>>>On Sun, Oct 31, 2004 at 02:48:40AM -0800, Andrew Morton wrote:
>>> > > -        void *va = dio_scodetoviraddr(scode);
>>> > > +        unsigned long pa = dio_scodetophysaddr(scode);
>>> > > +        unsigned long va = (pa + DIO_VIRADDRBASE);
>>>
>>> Did you see the above quoted patch chunk?  The patch is inconsistent
>>> with _itself_, adding 'pa' and 'va' with different idents (but when they
>>> should be at the same identation level).
>>
>>Trust me ;)
> 
> 
> What Jeff means is that the patch is using a tab for pa and 8 spaces
> for va.


Precisely.  That's what my entire "whitespace challenged" comment meant.

When the patch adds two lines at the same visual indentation level in 
the source, but appear different in the patch, that is a signal that 
some tabs got accidentally converted to spaces somewhere.

	Jeff



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 475] HP300 LANCE
  2004-10-31 10:03 [PATCH 475] HP300 LANCE Geert Uytterhoeven
  2004-10-31 10:14 ` Jeff Garzik
@ 2004-11-01 14:22 ` Christoph Hellwig
  2004-11-16  8:43   ` Christoph Hellwig
  1 sibling, 1 reply; 20+ messages in thread
From: Christoph Hellwig @ 2004-11-01 14:22 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Linus Torvalds, Andrew Morton, Jeff Garzik,
	Linux Kernel Development, linux-net

On Sun, Oct 31, 2004 at 11:03:30AM +0100, Geert Uytterhoeven wrote:
> HP300 LANCE updates from Kars de Jong:
>   - Updated HP LANCE driver to use the new DIO semantics
>   - If only HP LANCE or MVME147 LANCE is selected, enable compile-time
>     choice of LANCE register access. If both are defined, go through the
>     function pointer
>   - Added support for CONFIG_NET_POLL_CONTROLLER
>   - Fixed problem with disabling board interrupts in hplance_close() which
>     caused the driver to lock up

There's tons of leaks in the hplcance probing code, and it doesn't release
he memory region on removal either.

Untested patch to fix those issues below:

--- 1.12/drivers/net/hplance.c	2004-10-06 22:44:40 +02:00
+++ edited/drivers/net/hplance.c	2004-11-01 10:20:17 +01:00
@@ -71,30 +71,42 @@
 	.remove    = __devexit_p(hplance_remove_one),
 };
 
+/* XXX(hch): should probably move to a better place */
+#define dio_resource_start(d) \
+	((d)->resource.start)
+#define dio_resource_len(d) \
+	((d)->resource.end - (d)->resource.start)
+
 /* Find all the HP Lance boards and initialise them... */
 static int __devinit hplance_init_one(struct dio_dev *d,
 				const struct dio_device_id *ent)
 {
 	struct net_device *dev;
-	int err;
+	int err = -ENOMEM;
 
 	dev = alloc_etherdev(sizeof(struct hplance_private));
 	if (!dev)
-		return -ENOMEM;
+		goto out;
 
-	if (!request_mem_region(d->resource.start, d->resource.end-d->resource.start, d->name))
-		return -EBUSY;
+	err = -EBUSY;
+	if (!request_mem_region(dio_resource_start(d),
+				dio_resource_len(d), d->name))
+		goto out_free_netdev;
 
-	SET_MODULE_OWNER(dev);
-        
 	hplance_init(dev, d);
 	err = register_netdev(dev);
-	if (err) {
-		free_netdev(dev);
-		return err;
-	}
+	if (err)
+		goto out_free_netdev;
+
 	dio_set_drvdata(d, dev);
 	return 0;
+
+ out_release_mem_region:
+	release_mem_region(dio_resource_start(d), dio_resource_len(d));
+ out_free_netdev:
+	free_netdev(dev);
+ out:
+	return err;
 }
 
 static void __devexit hplance_remove_one(struct dio_dev *d)
@@ -102,6 +114,7 @@
 	struct net_device *dev = dio_get_drvdata(d);
 
 	unregister_netdev(dev);
+	release_mem_region(dio_resource_start(d), dio_resource_len(d));
 	free_netdev(dev);
 }
 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 475] HP300 LANCE
  2004-11-01 14:22 ` Christoph Hellwig
@ 2004-11-16  8:43   ` Christoph Hellwig
  2004-11-17  7:12     ` Andrew Morton
  0 siblings, 1 reply; 20+ messages in thread
From: Christoph Hellwig @ 2004-11-16  8:43 UTC (permalink / raw)
  To: Christoph Hellwig, Geert Uytterhoeven, Linus Torvalds,
	Andrew Morton, Jeff Garzik, Linux Kernel Development, linux-net

On Mon, Nov 01, 2004 at 02:22:45PM +0000, Christoph Hellwig wrote:
> On Sun, Oct 31, 2004 at 11:03:30AM +0100, Geert Uytterhoeven wrote:
> > HP300 LANCE updates from Kars de Jong:
> >   - Updated HP LANCE driver to use the new DIO semantics
> >   - If only HP LANCE or MVME147 LANCE is selected, enable compile-time
> >     choice of LANCE register access. If both are defined, go through the
> >     function pointer
> >   - Added support for CONFIG_NET_POLL_CONTROLLER
> >   - Fixed problem with disabling board interrupts in hplance_close() which
> >     caused the driver to lock up
> 
> There's tons of leaks in the hplcance probing code, and it doesn't release
> he memory region on removal either.
> 
> Untested patch to fix those issues below:

ping.

--- 1.12/drivers/net/hplance.c	2004-10-06 22:44:40 +02:00
+++ edited/drivers/net/hplance.c	2004-11-01 10:20:17 +01:00
@@ -71,30 +71,42 @@
 	.remove    = __devexit_p(hplance_remove_one),
 };
 
+/* XXX(hch): should probably move to a better place */
+#define dio_resource_start(d) \
+	((d)->resource.start)
+#define dio_resource_len(d) \
+	((d)->resource.end - (d)->resource.start)
+
 /* Find all the HP Lance boards and initialise them... */
 static int __devinit hplance_init_one(struct dio_dev *d,
 				const struct dio_device_id *ent)
 {
 	struct net_device *dev;
-	int err;
+	int err = -ENOMEM;
 
 	dev = alloc_etherdev(sizeof(struct hplance_private));
 	if (!dev)
-		return -ENOMEM;
+		goto out;
 
-	if (!request_mem_region(d->resource.start, d->resource.end-d->resource.start, d->name))
-		return -EBUSY;
+	err = -EBUSY;
+	if (!request_mem_region(dio_resource_start(d),
+				dio_resource_len(d), d->name))
+		goto out_free_netdev;
 
-	SET_MODULE_OWNER(dev);
-        
 	hplance_init(dev, d);
 	err = register_netdev(dev);
-	if (err) {
-		free_netdev(dev);
-		return err;
-	}
+	if (err)
+		goto out_free_netdev;
+
 	dio_set_drvdata(d, dev);
 	return 0;
+
+ out_release_mem_region:
+	release_mem_region(dio_resource_start(d), dio_resource_len(d));
+ out_free_netdev:
+	free_netdev(dev);
+ out:
+	return err;
 }
 
 static void __devexit hplance_remove_one(struct dio_dev *d)
@@ -102,6 +114,7 @@
 	struct net_device *dev = dio_get_drvdata(d);
 
 	unregister_netdev(dev);
+	release_mem_region(dio_resource_start(d), dio_resource_len(d));
 	free_netdev(dev);
 }
 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 475] HP300 LANCE
  2004-11-16  8:43   ` Christoph Hellwig
@ 2004-11-17  7:12     ` Andrew Morton
  2004-11-21 10:01       ` Geert Uytterhoeven
  0 siblings, 1 reply; 20+ messages in thread
From: Andrew Morton @ 2004-11-17  7:12 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: hch, geert, torvalds, jgarzik, linux-kernel, linux-net

Christoph Hellwig <hch@infradead.org> wrote:
>
> > There's tons of leaks in the hplcance probing code, and it doesn't release
>  > he memory region on removal either.
>  > 
>  > Untested patch to fix those issues below:
> 
>  ping.

The fix needs a fix:


diff -puN drivers/net/hplance.c~hp300-lance-leak-fixes-fix drivers/net/hplance.c
--- 25/drivers/net/hplance.c~hp300-lance-leak-fixes-fix	2004-11-16 23:11:46.546476832 -0800
+++ 25-akpm/drivers/net/hplance.c	2004-11-16 23:12:00.027427416 -0800
@@ -96,7 +96,7 @@ static int __devinit hplance_init_one(st
 	hplance_init(dev, d);
 	err = register_netdev(dev);
 	if (err)
-		goto out_free_netdev;
+		goto out_release_mem_region;
 
 	dio_set_drvdata(d, dev);
 	return 0;
_


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 475] HP300 LANCE
  2004-11-17  7:12     ` Andrew Morton
@ 2004-11-21 10:01       ` Geert Uytterhoeven
  2004-11-21 11:19         ` Kars de Jong
  2004-11-22  0:12         ` Andrew Morton
  0 siblings, 2 replies; 20+ messages in thread
From: Geert Uytterhoeven @ 2004-11-21 10:01 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Christoph Hellwig, Linus Torvalds, Kars de Jong, Jeff Garzik,
	Linux Kernel Development, linux-net

On Tue, 16 Nov 2004, Andrew Morton wrote:
> Christoph Hellwig <hch@infradead.org> wrote:
> > > There's tons of leaks in the hplcance probing code, and it doesn't release
> >  > he memory region on removal either.
> >  > 
> >  > Untested patch to fix those issues below:
> > 
> >  ping.
> 
> The fix needs a fix:

Indeed.

And you should remove the definitions of dio_resource_{start,len}(), as they're
already defined in linux/dio.h.

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 475] HP300 LANCE
  2004-11-21 10:01       ` Geert Uytterhoeven
@ 2004-11-21 11:19         ` Kars de Jong
  2004-11-22  0:12         ` Andrew Morton
  1 sibling, 0 replies; 20+ messages in thread
From: Kars de Jong @ 2004-11-21 11:19 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Andrew Morton, Christoph Hellwig, Linus Torvalds, Jeff Garzik,
	Linux Kernel Development, linux-net

On Sun, 2004-11-21 at 11:01 +0100, Geert Uytterhoeven wrote:
> On Tue, 16 Nov 2004, Andrew Morton wrote:
> > Christoph Hellwig <hch@infradead.org> wrote:
> > > > There's tons of leaks in the hplcance probing code, and it doesn't release
> > >  > he memory region on removal either.
> > >  > 
> > >  > Untested patch to fix those issues below:
> > > 
> > >  ping.
> > 
> > The fix needs a fix:
> 
> Indeed.
> 
> And you should remove the definitions of dio_resource_{start,len}(), as they're
> already defined in linux/dio.h.

Yes, that's what I wanted to write. Sorry I didn't respond sooner, my
harddisk died last week and I had to replace it first, which always
takes more time than you like...


Kind regards,

Kars.



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 475] HP300 LANCE
  2004-11-21 10:01       ` Geert Uytterhoeven
  2004-11-21 11:19         ` Kars de Jong
@ 2004-11-22  0:12         ` Andrew Morton
  2004-11-22  0:25           ` Sohail Somani
  2004-11-22  8:26           ` Geert Uytterhoeven
  1 sibling, 2 replies; 20+ messages in thread
From: Andrew Morton @ 2004-11-22  0:12 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: hch, torvalds, jongk, jgarzik, linux-kernel, linux-net

Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> On Tue, 16 Nov 2004, Andrew Morton wrote:
> > Christoph Hellwig <hch@infradead.org> wrote:
> > > > There's tons of leaks in the hplcance probing code, and it doesn't release
> > >  > he memory region on removal either.
> > >  > 
> > >  > Untested patch to fix those issues below:
> > > 
> > >  ping.
> > 
> > The fix needs a fix:
> 
> Indeed.
> 
> And you should remove the definitions of dio_resource_{start,len}(), as they're
> already defined in linux/dio.h.
> 

But differently.   Christoph had:

+#define dio_resource_len(d) \
+       ((d)->resource.end - (d)->resource.start)

but dio.h has:

#define dio_resource_len(d)   ((d)->resource.end-(z)->resource.start+1)


Which is correct?

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 475] HP300 LANCE
  2004-11-22  0:12         ` Andrew Morton
@ 2004-11-22  0:25           ` Sohail Somani
  2004-11-22 10:54             ` Geert Uytterhoeven
  2004-11-22  8:26           ` Geert Uytterhoeven
  1 sibling, 1 reply; 20+ messages in thread
From: Sohail Somani @ 2004-11-22  0:25 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel, linux-net

On Sun, 2004-11-21 at 16:12 -0800, Andrew Morton wrote:

> +#define dio_resource_len(d) \
> +       ((d)->resource.end - (d)->resource.start)
> 
> but dio.h has:
> 
> #define dio_resource_len(d)   ((d)->resource.end-(z)->resource.start+1)
> 
> 
> Which is correct?

Where did z come from in the second definition anyway?


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 475] HP300 LANCE
  2004-11-22  0:12         ` Andrew Morton
  2004-11-22  0:25           ` Sohail Somani
@ 2004-11-22  8:26           ` Geert Uytterhoeven
  1 sibling, 0 replies; 20+ messages in thread
From: Geert Uytterhoeven @ 2004-11-22  8:26 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Christoph Hellwig, Linus Torvalds, Kars de Jong, Jeff Garzik,
	Linux Kernel Development, linux-net

On Sun, 21 Nov 2004, Andrew Morton wrote:
> Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Tue, 16 Nov 2004, Andrew Morton wrote:
> > > Christoph Hellwig <hch@infradead.org> wrote:
> > > > > There's tons of leaks in the hplcance probing code, and it doesn't release
> > > >  > he memory region on removal either.
> > > >  > 
> > > >  > Untested patch to fix those issues below:
> > > > 
> > > >  ping.
> > > 
> > > The fix needs a fix:
> > 
> > Indeed.
> > 
> > And you should remove the definitions of dio_resource_{start,len}(), as they're
> > already defined in linux/dio.h.
> > 
> 
> But differently.   Christoph had:
> 
> +#define dio_resource_len(d) \
> +       ((d)->resource.end - (d)->resource.start)
> 
> but dio.h has:
> 
> #define dio_resource_len(d)   ((d)->resource.end-(z)->resource.start+1)
> 
> 
> Which is correct?

The one in linux/dio.h (i.e. incl. +1, cfr. the definitions in
linux/{pci,zorro}.h, too).

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 475] HP300 LANCE
  2004-11-22  0:25           ` Sohail Somani
@ 2004-11-22 10:54             ` Geert Uytterhoeven
  0 siblings, 0 replies; 20+ messages in thread
From: Geert Uytterhoeven @ 2004-11-22 10:54 UTC (permalink / raw)
  To: Sohail Somani; +Cc: Andrew Morton, Linux Kernel Development, linux-net

On Sun, 21 Nov 2004, Sohail Somani wrote:
> On Sun, 2004-11-21 at 16:12 -0800, Andrew Morton wrote:
> > +#define dio_resource_len(d) \
> > +       ((d)->resource.end - (d)->resource.start)
> > 
> > but dio.h has:
> > 
> > #define dio_resource_len(d)   ((d)->resource.end-(z)->resource.start+1)
> > 
> > 
> > Which is correct?
> 
> Where did z come from in the second definition anyway?

I guess it got copied from linux/zorro.h...
It's a bug which went unnoticed, and got fixed in the mean time.

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 475] HP300 LANCE
@ 2004-11-01  3:46 Chuck Ebbert
  0 siblings, 0 replies; 20+ messages in thread
From: Chuck Ebbert @ 2004-11-01  3:46 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel, Jeff Garzik

On Sun, 31 Oct 2004 at 02:48:40 -0800 Andrew Morton <akpm@osdl.org> wrote:

> It comes out visually OK once the patch is applied.  But it's a
> useful reminder of how much dreck we have in the tree.

That's for sure:

[me@nb linux-2.6.9]$ find . -name "*.[ch]" | xargs egrep -H -n '^[[:space:]]+if\(' | \
> tee if_should_be_followed_by_space | wc -l
18991


And my favorite pet peeve:

[me@nb linux-2.6.9]$ find . -name "*.[ch]" | \
> xargs egrep -H -n '^[[:space:]]+return[[:space:]]*\([^)]+\);' | \
> tee return_is_not_a_function | wc -l
9867

There are a lot more than that but I could find no obvious way of including
"return(parenthesized expr)" while excluding "return (typecast)(expr)".


--Chuck Ebbert  31-Oct-04  23:46:37

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2004-11-22 10:59 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-10-31 10:03 [PATCH 475] HP300 LANCE Geert Uytterhoeven
2004-10-31 10:14 ` Jeff Garzik
2004-10-31 10:19   ` Andrew Morton
2004-10-31 10:41     ` Jeff Garzik
2004-10-31 10:48       ` Andrew Morton
2004-10-31 10:57         ` Jeff Garzik
2004-10-31 11:09           ` Andrew Morton
2004-10-31 11:44             ` Geert Uytterhoeven
2004-10-31 11:50             ` Herbert Xu
2004-10-31 11:58               ` Jeff Garzik
2004-11-01 14:22 ` Christoph Hellwig
2004-11-16  8:43   ` Christoph Hellwig
2004-11-17  7:12     ` Andrew Morton
2004-11-21 10:01       ` Geert Uytterhoeven
2004-11-21 11:19         ` Kars de Jong
2004-11-22  0:12         ` Andrew Morton
2004-11-22  0:25           ` Sohail Somani
2004-11-22 10:54             ` Geert Uytterhoeven
2004-11-22  8:26           ` Geert Uytterhoeven
2004-11-01  3:46 Chuck Ebbert

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).