[PATCH][ATM] assorted he driver cleanup

[PATCH][ATM] assorted he driver cleanup

[chas williams]

the three following changesets attempt to bring the he atm
driver into conformance with the accepted linux coding style,
fix some chattiness the irq handler, and address the stack
usage issue in he_init_cs_block_rcm().


# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.1158  -> 1.1159 
#	    drivers/atm/he.c	1.7     -> 1.8    
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 03/05/23	chas@relax.cmf.nrl.navy.mil	1.1159
# he coding style conformance
# --------------------------------------------
#
diff -Nru a/drivers/atm/he.c b/drivers/atm/he.c
--- a/drivers/atm/he.c	Thu May 29 11:48:40 2003
+++ b/drivers/atm/he.c	Thu May 29 11:48:40 2003
@@ -132,9 +132,9 @@
 
 #undef DEBUG
 #ifdef DEBUG
-#define HPRINTK(fmt,args...)	hprintk(fmt,args)
+#define HPRINTK(fmt,args...)	printk(KERN_DEBUG DEV_LABEL "%d: " fmt, he_dev->number , ##args)
 #else
-#define HPRINTK(fmt,args...)	do { } while(0)
+#define HPRINTK(fmt,args...)	do { } while (0)
 #endif /* DEBUG */
 
 
@@ -179,9 +179,7 @@
    phy_put:	he_phy_put,
    phy_get:	he_phy_get,
    proc_read:	he_proc_read,
-#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,4,1)
    owner:	THIS_MODULE
-#endif
 };
 
 /* see the comments in he.h about global_lock */
@@ -189,7 +187,7 @@
 #define HE_SPIN_LOCK(dev, flags)	spin_lock_irqsave(&(dev)->global_lock, flags)
 #define HE_SPIN_UNLOCK(dev, flags)	spin_unlock_irqrestore(&(dev)->global_lock, flags)
 
-#define he_writel(dev, val, reg)	do { writel(val, (dev)->membase + (reg)); wmb(); } while(0)
+#define he_writel(dev, val, reg)	do { writel(val, (dev)->membase + (reg)); wmb(); } while (0)
 #define he_readl(dev, reg)		readl((dev)->membase + (reg))
 
 /* section 2.12 connection memory access */
@@ -203,7 +201,7 @@
 	(void) he_readl(he_dev, CON_DAT);
 #endif
 	he_writel(he_dev, flags | CON_CTL_WRITE | CON_CTL_ADDR(addr), CON_CTL);
-	while(he_readl(he_dev, CON_CTL) & CON_CTL_BUSY);
+	while (he_readl(he_dev, CON_CTL) & CON_CTL_BUSY);
 }
 
 #define he_writel_rcm(dev, val, reg) 				\
@@ -219,7 +217,7 @@
 he_readl_internal(struct he_dev *he_dev, unsigned addr, unsigned flags)
 {
 	he_writel(he_dev, flags | CON_CTL_READ | CON_CTL_ADDR(addr), CON_CTL);
-	while(he_readl(he_dev, CON_CTL) & CON_CTL_BUSY);
+	while (he_readl(he_dev, CON_CTL) & CON_CTL_BUSY);
 	return he_readl(he_dev, CON_DAT);
 }
 
@@ -374,7 +372,8 @@
 
 	printk(KERN_INFO "he: %s\n", version);
 
-	if (pci_enable_device(pci_dev)) return -EIO;
+	if (pci_enable_device(pci_dev))
+		return -EIO;
 	if (pci_set_dma_mask(pci_dev, HE_DMA_MASK) != 0) {
 		printk(KERN_WARNING "he: no suitable dma available\n");
 		err = -EIO;
@@ -407,7 +406,8 @@
 		goto init_one_failure;
 	}
 	he_dev->next = NULL;
-	if (he_devs) he_dev->next = he_devs;
+	if (he_devs)
+		he_dev->next = he_devs;
 	he_devs = he_dev;
 	return 0;
 
@@ -447,11 +447,11 @@
 
         unsigned exp = 0;
 
-        if (rate == 0) return(0);
+        if (rate == 0)
+		return(0);
 
         rate <<= 9;
-        while (rate > 0x3ff)
-        {
+        while (rate > 0x3ff) {
                 ++exp;
                 rate >>= 1;
         }
@@ -472,16 +472,14 @@
 
 	he_writel(he_dev, lbufd_index, RLBF0_H);
 
-        for (i = 0, lbuf_count = 0; i < he_dev->r0_numbuffs; ++i)
-        {
+        for (i = 0, lbuf_count = 0; i < he_dev->r0_numbuffs; ++i) {
 		lbufd_index += 2;
                 lbuf_addr = (row_offset + (lbuf_count * lbuf_bufsize)) / 32;
 
 		he_writel_rcm(he_dev, lbuf_addr, lbm_offset);
 		he_writel_rcm(he_dev, lbufd_index, lbm_offset + 1);
 
-                if (++lbuf_count == lbufs_per_row)
-                {
+                if (++lbuf_count == lbufs_per_row) {
                         lbuf_count = 0;
                         row_offset += he_dev->bytes_per_row;
                 }
@@ -505,16 +503,14 @@
 
 	he_writel(he_dev, lbufd_index, RLBF1_H);
 
-        for (i = 0, lbuf_count = 0; i < he_dev->r1_numbuffs; ++i)
-        {
+        for (i = 0, lbuf_count = 0; i < he_dev->r1_numbuffs; ++i) {
 		lbufd_index += 2;
                 lbuf_addr = (row_offset + (lbuf_count * lbuf_bufsize)) / 32;
 
 		he_writel_rcm(he_dev, lbuf_addr, lbm_offset);
 		he_writel_rcm(he_dev, lbufd_index, lbm_offset + 1);
 
-                if (++lbuf_count == lbufs_per_row)
-                {
+                if (++lbuf_count == lbufs_per_row) {
                         lbuf_count = 0;
                         row_offset += he_dev->bytes_per_row;
                 }
@@ -538,16 +534,14 @@
 
 	he_writel(he_dev, lbufd_index, TLBF_H);
 
-        for (i = 0, lbuf_count = 0; i < he_dev->tx_numbuffs; ++i)
-        {
+        for (i = 0, lbuf_count = 0; i < he_dev->tx_numbuffs; ++i) {
 		lbufd_index += 1;
                 lbuf_addr = (row_offset + (lbuf_count * lbuf_bufsize)) / 32;
 
 		he_writel_rcm(he_dev, lbuf_addr, lbm_offset);
 		he_writel_rcm(he_dev, lbufd_index, lbm_offset + 1);
 
-                if (++lbuf_count == lbufs_per_row)
-                {
+                if (++lbuf_count == lbufs_per_row) {
                         lbuf_count = 0;
                         row_offset += he_dev->bytes_per_row;
                 }
@@ -562,8 +556,7 @@
 {
 	he_dev->tpdrq_base = pci_alloc_consistent(he_dev->pci_dev,
 		CONFIG_TPDRQ_SIZE * sizeof(struct he_tpdrq), &he_dev->tpdrq_phys);
-	if (he_dev->tpdrq_base == NULL) 
-	{
+	if (he_dev->tpdrq_base == NULL) {
 		hprintk("failed to alloc tpdrq\n");
 		return -ENOMEM;
 	}
@@ -588,7 +581,7 @@
 
 	/* 5.1.7 cs block initialization */
 
-	for(reg = 0; reg < 0x20; ++reg)
+	for (reg = 0; reg < 0x20; ++reg)
 		he_writel_mbox(he_dev, 0x0, CS_STTIM0 + reg);
 
 	/* rate grid timer reload values */
@@ -597,8 +590,7 @@
 	rate = he_dev->atm_dev->link_rate;
 	delta = rate / 16 / 2;
 
-	for(reg = 0; reg < 0x10; ++reg)
-	{
+	for (reg = 0; reg < 0x10; ++reg) {
 		/* 2.4 internal transmit function
 		 *
 	 	 * we initialize the first row in the rate grid.
@@ -610,8 +602,7 @@
 		rate -= delta;
 	}
 
-	if (he_is622(he_dev))
-	{
+	if (he_is622(he_dev)) {
 		/* table 5.2 (4 cells per lbuf) */
 		he_writel_mbox(he_dev, 0x000800fa, CS_ERTHR0);
 		he_writel_mbox(he_dev, 0x000c33cb, CS_ERTHR1);
@@ -640,9 +631,7 @@
 		/* table 5.9 */
 		he_writel_mbox(he_dev, 0x5, CS_OTPPER);
 		he_writel_mbox(he_dev, 0x14, CS_OTWPER);
-	}
-	else
-	{
+	} else {
 		/* table 5.1 (4 cells per lbuf) */
 		he_writel_mbox(he_dev, 0x000400ea, CS_ERTHR0);
 		he_writel_mbox(he_dev, 0x00063388, CS_ERTHR1);
@@ -671,12 +660,11 @@
 		/* table 5.9 */
 		he_writel_mbox(he_dev, 0x6, CS_OTPPER);
 		he_writel_mbox(he_dev, 0x1e, CS_OTWPER);
-
 	}
 
 	he_writel_mbox(he_dev, 0x8, CS_OTTLIM);
 
-	for(reg = 0; reg < 0x8; ++reg)
+	for (reg = 0; reg < 0x8; ++reg)
 		he_writel_mbox(he_dev, 0x0, CS_HGRRT0 + reg);
 
 }
@@ -720,8 +708,7 @@
 	 * in order to construct the rate to group table below
 	 */
 
-	for (j = 0; j < 16; j++)
-	{
+	for (j = 0; j < 16; j++) {
 		rategrid[0][j] = rate;
 		rate -= delta;
 	}
@@ -742,8 +729,7 @@
 	 */
 
 	rate_atmf = 0;
-	while (rate_atmf < 0x400)
-	{
+	while (rate_atmf < 0x400) {
 		man = (rate_atmf & 0x1f) << 4;
 		exp = rate_atmf >> 5;
 
@@ -753,12 +739,12 @@
 		*/
 		rate_cps = (unsigned long long) (1 << exp) * (man + 512) >> 9;
 
-		if (rate_cps < 10) rate_cps = 10;
-				/* 2.2.1 minimum payload rate is 10 cps */
+		if (rate_cps < 10)
+			rate_cps = 10;	/* 2.2.1 minimum payload rate is 10 cps */
 
 		for (i = 255; i > 0; i--)
-			if (rategrid[i/16][i%16] >= rate_cps) break;
-				/* pick nearest rate instead? */
+			if (rategrid[i/16][i%16] >= rate_cps)
+				break;	 /* pick nearest rate instead? */
 
 		/*
 		 * each table entry is 16 bits: (rate grid index (8 bits)
@@ -773,12 +759,17 @@
 		/* this is pretty, but avoids _divdu3 and is mostly correct */
                 buf = 0;
                 mult = he_dev->atm_dev->link_rate / ATM_OC3_PCR;
-                if (rate_cps > (68 * mult)) buf = 1;
-                if (rate_cps > (136 * mult)) buf = 2;
-                if (rate_cps > (204 * mult)) buf = 3;
-                if (rate_cps > (272 * mult)) buf = 4;
+                if (rate_cps > (68 * mult))
+			buf = 1;
+                if (rate_cps > (136 * mult))
+			buf = 2;
+                if (rate_cps > (204 * mult))
+			buf = 3;
+                if (rate_cps > (272 * mult))
+			buf = 4;
 #endif
-                if (buf > buf_limit) buf = buf_limit;
+                if (buf > buf_limit)
+			buf = buf_limit;
 		reg = (reg<<16) | ((i<<8) | buf);
 
 #define RTGTBL_OFFSET 0x400
@@ -801,8 +792,7 @@
 #ifdef USE_RBPS_POOL
 	he_dev->rbps_pool = pci_pool_create("rbps", he_dev->pci_dev,
 			CONFIG_RBPS_BUFSIZE, 8, 0);
-	if (he_dev->rbps_pool == NULL)
-	{
+	if (he_dev->rbps_pool == NULL) {
 		hprintk("unable to create rbps pages\n");
 		return -ENOMEM;
 	}
@@ -817,16 +807,14 @@
 
 	he_dev->rbps_base = pci_alloc_consistent(he_dev->pci_dev,
 		CONFIG_RBPS_SIZE * sizeof(struct he_rbp), &he_dev->rbps_phys);
-	if (he_dev->rbps_base == NULL)
-	{
+	if (he_dev->rbps_base == NULL) {
 		hprintk("failed to alloc rbps\n");
 		return -ENOMEM;
 	}
 	memset(he_dev->rbps_base, 0, CONFIG_RBPS_SIZE * sizeof(struct he_rbp));
 	he_dev->rbps_virt = kmalloc(CONFIG_RBPS_SIZE * sizeof(struct he_virt), GFP_KERNEL);
 
-	for (i = 0; i < CONFIG_RBPS_SIZE; ++i)
-	{
+	for (i = 0; i < CONFIG_RBPS_SIZE; ++i) {
 		dma_addr_t dma_handle;
 		void *cpuaddr;
 
@@ -868,16 +856,14 @@
 #ifdef USE_RBPL_POOL
 	he_dev->rbpl_pool = pci_pool_create("rbpl", he_dev->pci_dev,
 			CONFIG_RBPL_BUFSIZE, 8, 0);
-	if (he_dev->rbpl_pool == NULL)
-	{
+	if (he_dev->rbpl_pool == NULL) {
 		hprintk("unable to create rbpl pool\n");
 		return -ENOMEM;
 	}
 #else /* !USE_RBPL_POOL */
 	he_dev->rbpl_pages = (void *) pci_alloc_consistent(he_dev->pci_dev,
 		CONFIG_RBPL_SIZE * CONFIG_RBPL_BUFSIZE, &he_dev->rbpl_pages_phys);
-	if (he_dev->rbpl_pages == NULL)
-	{
+	if (he_dev->rbpl_pages == NULL) {
 		hprintk("unable to create rbpl pages\n");
 		return -ENOMEM;
 	}
@@ -885,16 +871,14 @@
 
 	he_dev->rbpl_base = pci_alloc_consistent(he_dev->pci_dev,
 		CONFIG_RBPL_SIZE * sizeof(struct he_rbp), &he_dev->rbpl_phys);
-	if (he_dev->rbpl_base == NULL)
-	{
+	if (he_dev->rbpl_base == NULL) {
 		hprintk("failed to alloc rbpl\n");
 		return -ENOMEM;
 	}
 	memset(he_dev->rbpl_base, 0, CONFIG_RBPL_SIZE * sizeof(struct he_rbp));
 	he_dev->rbpl_virt = kmalloc(CONFIG_RBPL_SIZE * sizeof(struct he_virt), GFP_KERNEL);
 
-	for (i = 0; i < CONFIG_RBPL_SIZE; ++i)
-	{
+	for (i = 0; i < CONFIG_RBPL_SIZE; ++i) {
 		dma_addr_t dma_handle;
 		void *cpuaddr;
 
@@ -910,7 +894,6 @@
 		he_dev->rbpl_virt[i].virt = cpuaddr;
 		he_dev->rbpl_base[i].status = RBP_LOANED | (i << RBP_INDEX_OFF);
 		he_dev->rbpl_base[i].phys = dma_handle;
-
 	}
 	he_dev->rbpl_tail = &he_dev->rbpl_base[CONFIG_RBPL_SIZE-1];
 
@@ -929,8 +912,7 @@
 
 	he_dev->rbrq_base = pci_alloc_consistent(he_dev->pci_dev,
 		CONFIG_RBRQ_SIZE * sizeof(struct he_rbrq), &he_dev->rbrq_phys);
-	if (he_dev->rbrq_base == NULL)
-	{
+	if (he_dev->rbrq_base == NULL) {
 		hprintk("failed to allocate rbrq\n");
 		return -ENOMEM;
 	}
@@ -942,13 +924,11 @@
 	he_writel(he_dev,
 		RBRQ_THRESH(CONFIG_RBRQ_THRESH) | RBRQ_SIZE(CONFIG_RBRQ_SIZE-1),
 						G0_RBRQ_Q + (group * 16));
-	if (irq_coalesce)
-	{
+	if (irq_coalesce) {
 		hprintk("coalescing interrupts\n");
 		he_writel(he_dev, RBRQ_TIME(768) | RBRQ_COUNT(7),
 						G0_RBRQ_I + (group * 16));
-	}
-	else
+	} else
 		he_writel(he_dev, RBRQ_TIME(0) | RBRQ_COUNT(1),
 						G0_RBRQ_I + (group * 16));
 
@@ -956,8 +936,7 @@
 
 	he_dev->tbrq_base = pci_alloc_consistent(he_dev->pci_dev,
 		CONFIG_TBRQ_SIZE * sizeof(struct he_tbrq), &he_dev->tbrq_phys);
-	if (he_dev->tbrq_base == NULL)
-	{
+	if (he_dev->tbrq_base == NULL) {
 		hprintk("failed to allocate tbrq\n");
 		return -ENOMEM;
 	}
@@ -983,8 +962,7 @@
 
         he_dev->irq_base = pci_alloc_consistent(he_dev->pci_dev,
 			(CONFIG_IRQ_SIZE+1) * sizeof(struct he_irq), &he_dev->irq_phys);
-	if (he_dev->irq_base == NULL)
-	{
+	if (he_dev->irq_base == NULL) {
 		hprintk("failed to allocate irq\n");
 		return -ENOMEM;
 	}
@@ -994,7 +972,7 @@
 	he_dev->irq_head = he_dev->irq_base;
 	he_dev->irq_tail = he_dev->irq_base;
 
-	for(i=0; i < CONFIG_IRQ_SIZE; ++i)
+	for (i=0; i < CONFIG_IRQ_SIZE; ++i)
 		he_dev->irq_base[i].isw = ITYPE_INVALID;
 
 	he_writel(he_dev, he_dev->irq_phys, IRQ0_BASE);
@@ -1026,8 +1004,7 @@
 	he_writel(he_dev, 0x0, GRP_54_MAP);
 	he_writel(he_dev, 0x0, GRP_76_MAP);
 
-	if (request_irq(he_dev->pci_dev->irq, he_irq_handler, SA_INTERRUPT|SA_SHIRQ, DEV_LABEL, he_dev))
-	{
+	if (request_irq(he_dev->pci_dev->irq, he_irq_handler, SA_INTERRUPT|SA_SHIRQ, DEV_LABEL, he_dev)) {
 		hprintk("irq %d already in use\n", he_dev->pci_dev->irq);
 		return -EINVAL;
         }   
@@ -1067,46 +1044,39 @@
 	 */
 
 	/* 4.3 pci bus controller-specific initialization */
-	if (pci_read_config_dword(pci_dev, GEN_CNTL_0, &gen_cntl_0) != 0)
-	{
+	if (pci_read_config_dword(pci_dev, GEN_CNTL_0, &gen_cntl_0) != 0) {
 		hprintk("can't read GEN_CNTL_0\n");
 		return -EINVAL;
 	}
 	gen_cntl_0 |= (MRL_ENB | MRM_ENB | IGNORE_TIMEOUT);
-	if (pci_write_config_dword(pci_dev, GEN_CNTL_0, gen_cntl_0) != 0)
-	{
+	if (pci_write_config_dword(pci_dev, GEN_CNTL_0, gen_cntl_0) != 0) {
 		hprintk("can't write GEN_CNTL_0.\n");
 		return -EINVAL;
 	}
 
-	if (pci_read_config_word(pci_dev, PCI_COMMAND, &command) != 0)
-	{
+	if (pci_read_config_word(pci_dev, PCI_COMMAND, &command) != 0) {
 		hprintk("can't read PCI_COMMAND.\n");
 		return -EINVAL;
 	}
 
 	command |= (PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER | PCI_COMMAND_INVALIDATE);
-	if (pci_write_config_word(pci_dev, PCI_COMMAND, command) != 0)
-	{
+	if (pci_write_config_word(pci_dev, PCI_COMMAND, command) != 0) {
 		hprintk("can't enable memory.\n");
 		return -EINVAL;
 	}
 
-	if (pci_read_config_byte(pci_dev, PCI_CACHE_LINE_SIZE, &cache_size))
-	{
+	if (pci_read_config_byte(pci_dev, PCI_CACHE_LINE_SIZE, &cache_size)) {
 		hprintk("can't read cache line size?\n");
 		return -EINVAL;
 	}
 
-	if (cache_size < 16)
-	{
+	if (cache_size < 16) {
 		cache_size = 16;
 		if (pci_write_config_byte(pci_dev, PCI_CACHE_LINE_SIZE, cache_size))
 			hprintk("can't set cache line size to %d\n", cache_size);
 	}
 
-	if (pci_read_config_byte(pci_dev, PCI_LATENCY_TIMER, &timer))
-	{
+	if (pci_read_config_byte(pci_dev, PCI_LATENCY_TIMER, &timer)) {
 		hprintk("can't read latency timer?\n");
 		return -EINVAL;
 	}
@@ -1120,8 +1090,7 @@
 	 *
 	 */ 
 #define LAT_TIMER 209
-	if (timer < LAT_TIMER)
-	{
+	if (timer < LAT_TIMER) {
 		HPRINTK("latency timer was %d, setting to %d\n", timer, LAT_TIMER);
 		timer = LAT_TIMER;
 		if (pci_write_config_byte(pci_dev, PCI_LATENCY_TIMER, timer))
@@ -1139,8 +1108,7 @@
 
 	udelay(16*1000);	/* 16 ms */
 	status = he_readl(he_dev, RESET_CNTL);
-	if ((status & BOARD_RST_STATUS) == 0)
-	{
+	if ((status & BOARD_RST_STATUS) == 0) {
 		hprintk("reset failed\n");
 		return -EINVAL;
 	}
@@ -1152,23 +1120,23 @@
 	else
 		gen_cntl_0 &= ~ENBL_64;
 
-	if (disable64 == 1)
-	{
+	if (disable64 == 1) {
 		hprintk("disabling 64-bit pci bus transfers\n");
 		gen_cntl_0 &= ~ENBL_64;
 	}
 
-	if (gen_cntl_0 & ENBL_64) hprintk("64-bit transfers enabled\n");
+	if (gen_cntl_0 & ENBL_64)
+		hprintk("64-bit transfers enabled\n");
 
 	pci_write_config_dword(pci_dev, GEN_CNTL_0, gen_cntl_0);
 
 	/* 4.7 read prom contents */
-	for(i=0; i<PROD_ID_LEN; ++i)
+	for (i=0; i<PROD_ID_LEN; ++i)
 		he_dev->prod_id[i] = read_prom_byte(he_dev, PROD_ID + i);
 
 	he_dev->media = read_prom_byte(he_dev, MEDIA);
 
-	for(i=0; i<6; ++i)
+	for (i=0; i<6; ++i)
 		dev->esi[i] = read_prom_byte(he_dev, MAC_ADDR + i);
 
 	hprintk("%s%s, %x:%x:%x:%x:%x:%x\n",
@@ -1205,7 +1173,8 @@
 	he_writel(he_dev, lb_swap, LB_SWAP);
 
 	/* 4.10 initialize the interrupt queues */
-	if ((err = he_init_irq(he_dev)) != 0) return err;
+	if ((err = he_init_irq(he_dev)) != 0)
+		return err;
 
 #ifdef USE_TASKLET
 	tasklet_init(&he_dev->tasklet, he_tasklet, (unsigned long) he_dev);
@@ -1258,27 +1227,23 @@
 	he_dev->vcibits = CONFIG_DEFAULT_VCIBITS;
 	he_dev->vpibits = CONFIG_DEFAULT_VPIBITS;
 
-	if (nvpibits != -1 && nvcibits != -1 && nvpibits+nvcibits != HE_MAXCIDBITS)
-	{
+	if (nvpibits != -1 && nvcibits != -1 && nvpibits+nvcibits != HE_MAXCIDBITS) {
 		hprintk("nvpibits + nvcibits != %d\n", HE_MAXCIDBITS);
 		return -ENODEV;
 	}
 
-	if (nvpibits != -1)
-	{
+	if (nvpibits != -1) {
 		he_dev->vpibits = nvpibits;
 		he_dev->vcibits = HE_MAXCIDBITS - nvpibits;
 	}
 
-	if (nvcibits != -1)
-	{
+	if (nvcibits != -1) {
 		he_dev->vcibits = nvcibits;
 		he_dev->vpibits = HE_MAXCIDBITS - nvcibits;
 	}
 
 
-	if (he_is622(he_dev))
-	{
+	if (he_is622(he_dev)) {
 		he_dev->cells_per_row = 40;
 		he_dev->bytes_per_row = 2048;
 		he_dev->r0_numrows = 256;
@@ -1287,9 +1252,7 @@
 		he_dev->r0_startrow = 0;
 		he_dev->tx_startrow = 256;
 		he_dev->r1_startrow = 768;
-	}
-	else
-	{
+	} else {
 		he_dev->cells_per_row = 20;
 		he_dev->bytes_per_row = 1024;
 		he_dev->r0_numrows = 512;
@@ -1304,15 +1267,18 @@
 	he_dev->buffer_limit = 4;
 	he_dev->r0_numbuffs = he_dev->r0_numrows *
 				he_dev->cells_per_row / he_dev->cells_per_lbuf;
-	if (he_dev->r0_numbuffs > 2560) he_dev->r0_numbuffs = 2560;
+	if (he_dev->r0_numbuffs > 2560)
+		he_dev->r0_numbuffs = 2560;
 
 	he_dev->r1_numbuffs = he_dev->r1_numrows *
 				he_dev->cells_per_row / he_dev->cells_per_lbuf;
-	if (he_dev->r1_numbuffs > 2560) he_dev->r1_numbuffs = 2560;
+	if (he_dev->r1_numbuffs > 2560)
+		he_dev->r1_numbuffs = 2560;
 
 	he_dev->tx_numbuffs = he_dev->tx_numrows *
 				he_dev->cells_per_row / he_dev->cells_per_lbuf;
-	if (he_dev->tx_numbuffs > 5120) he_dev->tx_numbuffs = 5120;
+	if (he_dev->tx_numbuffs > 5120)
+		he_dev->tx_numbuffs = 5120;
 
 	/* 5.1.2 configure hardware dependent registers */
 
@@ -1355,10 +1321,10 @@
 
 	/* 5.1.3 initialize connection memory */
 
-	for(i=0; i < TCM_MEM_SIZE; ++i)
+	for (i=0; i < TCM_MEM_SIZE; ++i)
 		he_writel_tcm(he_dev, 0, i);
 
-	for(i=0; i < RCM_MEM_SIZE; ++i)
+	for (i=0; i < RCM_MEM_SIZE; ++i)
 		he_writel_rcm(he_dev, 0, i);
 
 	/*
@@ -1448,8 +1414,7 @@
 
 	/* 5.1.5 initialize intermediate receive queues */
 
-	if (he_is622(he_dev))
-	{
+	if (he_is622(he_dev)) {
 		he_writel(he_dev, 0x000f, G0_INMQ_S);
 		he_writel(he_dev, 0x200f, G0_INMQ_L);
 
@@ -1473,9 +1438,7 @@
 
 		he_writel(he_dev, 0x007f, G7_INMQ_S);
 		he_writel(he_dev, 0x207f, G7_INMQ_L);
-	}
-	else
-	{
+	} else {
 		he_writel(he_dev, 0x0000, G0_INMQ_S);
 		he_writel(he_dev, 0x0008, G0_INMQ_L);
 
@@ -1523,8 +1486,7 @@
 #ifdef USE_TPD_POOL
 	he_dev->tpd_pool = pci_pool_create("tpd", he_dev->pci_dev,
 		sizeof(struct he_tpd), TPD_ALIGNMENT, 0);
-	if (he_dev->tpd_pool == NULL)
-	{
+	if (he_dev->tpd_pool == NULL) {
 		hprintk("unable to create tpd pci_pool\n");
 		return -ENOMEM;         
 	}
@@ -1536,8 +1498,7 @@
 	if (!he_dev->tpd_base)
 		return -ENOMEM;
 
-	for(i = 0; i < CONFIG_NUMTPDS; ++i)
-	{
+	for (i = 0; i < CONFIG_NUMTPDS; ++i) {
 		he_dev->tpd_base[i].status = (i << TPD_ADDR_SHIFT);
 		he_dev->tpd_base[i].inuse = 0;
 	}
@@ -1549,8 +1510,7 @@
 	if (he_init_group(he_dev, 0) != 0)
 		return -ENOMEM;
 
-	for (group = 1; group < HE_NUM_GROUPS; ++group)
-	{
+	for (group = 1; group < HE_NUM_GROUPS; ++group) {
 		he_writel(he_dev, 0x0, G0_RBPS_S + (group * 32));
 		he_writel(he_dev, 0x0, G0_RBPS_T + (group * 32));
 		he_writel(he_dev, 0x0, G0_RBPS_QI + (group * 32));
@@ -1580,8 +1540,7 @@
 
 	he_dev->hsp = pci_alloc_consistent(he_dev->pci_dev,
 				sizeof(struct he_hsp), &he_dev->hsp_phys);
-	if (he_dev->hsp == NULL)
-	{
+	if (he_dev->hsp == NULL) {
 		hprintk("failed to allocate host status page\n");
 		return -ENOMEM;
 	}
@@ -1596,8 +1555,7 @@
 		he_dev->atm_dev->phy->start(he_dev->atm_dev);
 #endif /* CONFIG_ATM_HE_USE_SUNI */
 
-	if (sdh)
-	{
+	if (sdh) {
 		/* this really should be in suni.c but for now... */
 
 		int val;
@@ -1620,8 +1578,7 @@
 #ifndef USE_HE_FIND_VCC
 	he_dev->he_vcc_table = kmalloc(sizeof(struct he_vcc_table) * 
 			(1 << (he_dev->vcibits + he_dev->vpibits)), GFP_KERNEL);
-	if (he_dev->he_vcc_table == NULL)
-	{
+	if (he_dev->he_vcc_table == NULL) {
 		hprintk("failed to alloc he_vcc_table\n");
 		return -ENOMEM;
 	}
@@ -1629,8 +1586,7 @@
 				(1 << (he_dev->vcibits + he_dev->vpibits)));
 #endif
 
-	for (i = 0; i < HE_NUM_CS_STPER; ++i)
-	{
+	for (i = 0; i < HE_NUM_CS_STPER; ++i) {
 		he_dev->cs_stper[i].inuse = 0;
 		he_dev->cs_stper[i].pcr = -1;
 	}
@@ -1663,8 +1619,7 @@
 
 	/* disable interrupts */
 
-	if (he_dev->membase)
-	{
+	if (he_dev->membase) {
 		pci_read_config_dword(pci_dev, GEN_CNTL_0, &gen_cntl_0);
 		gen_cntl_0 &= ~(INT_PROC_ENBL | INIT_ENB);
 		pci_write_config_dword(pci_dev, GEN_CNTL_0, gen_cntl_0);
@@ -1689,8 +1644,7 @@
 		he_dev->atm_dev->phy->stop(he_dev->atm_dev);
 #endif /* CONFIG_ATM_HE_USE_SUNI */
 
-	if (he_dev->irq)
-	{
+	if (he_dev->irq) {
 #ifdef BUS_INT_WAR
 		sn_delete_polled_interrupt(he_dev->irq);
 #endif
@@ -1705,11 +1659,9 @@
 		pci_free_consistent(he_dev->pci_dev, sizeof(struct he_hsp),
 						he_dev->hsp, he_dev->hsp_phys);
 
-	if (he_dev->rbpl_base)
-	{
+	if (he_dev->rbpl_base) {
 #ifdef USE_RBPL_POOL
-		for (i=0; i<CONFIG_RBPL_SIZE; ++i)
-		{
+		for (i=0; i<CONFIG_RBPL_SIZE; ++i) {
 			void *cpuaddr = he_dev->rbpl_virt[i].virt;
 			dma_addr_t dma_handle = he_dev->rbpl_base[i].phys;
 
@@ -1729,11 +1681,9 @@
 #endif
 
 #ifdef USE_RBPS
-	if (he_dev->rbps_base)
-	{
+	if (he_dev->rbps_base) {
 #ifdef USE_RBPS_POOL
-		for (i=0; i<CONFIG_RBPS_SIZE; ++i)
-		{
+		for (i=0; i<CONFIG_RBPS_SIZE; ++i) {
 			void *cpuaddr = he_dev->rbps_virt[i].virt;
 			dma_addr_t dma_handle = he_dev->rbps_base[i].phys;
 
@@ -1780,14 +1730,14 @@
 		kfree(he_dev->he_vcc_table);
 #endif
 
-	if (he_dev->pci_dev)
-	{
+	if (he_dev->pci_dev) {
 		pci_read_config_word(he_dev->pci_dev, PCI_COMMAND, &command);
 		command &= ~(PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER);
 		pci_write_config_word(he_dev->pci_dev, PCI_COMMAND, command);
 	}
 	
-	if (he_dev->membase) iounmap((void *) he_dev->membase);
+	if (he_dev->membase)
+		iounmap((void *) he_dev->membase);
 }
 
 static struct he_tpd *
@@ -1811,8 +1761,7 @@
 #else
 	int i;
 
-	for(i = 0; i < CONFIG_NUMTPDS; ++i)
-	{
+	for (i = 0; i < CONFIG_NUMTPDS; ++i) {
 		++he_dev->tpd_head;
 		if (he_dev->tpd_head > he_dev->tpd_end) {
 			he_dev->tpd_head = he_dev->tpd_base;
@@ -1862,8 +1811,7 @@
 	int pdus_assembled = 0;
 	int updated = 0;
 
-	while (he_dev->rbrq_head != rbrq_tail)
-	{
+	while (he_dev->rbrq_head != rbrq_tail) {
 		++updated;
 
 		HPRINTK("%p rbrq%d 0x%x len=%d cid=0x%x %s%s%s%s%s%s\n",
@@ -1895,8 +1843,7 @@
 #else
 		vcc = HE_LOOKUP_VCC(he_dev, cid);
 #endif
-		if (vcc == NULL)
-		{
+		if (vcc == NULL) {
 			hprintk("vcc == NULL  (cid 0x%x)\n", cid);
 			if (!RBRQ_HBUF_ERR(he_dev->rbrq_head))
 					rbp->status &= ~RBP_LOANED;
@@ -1905,16 +1852,14 @@
 		}
 
 		he_vcc = HE_VCC(vcc);
-		if (he_vcc == NULL)
-		{
+		if (he_vcc == NULL) {
 			hprintk("he_vcc == NULL  (cid 0x%x)\n", cid);
 			if (!RBRQ_HBUF_ERR(he_dev->rbrq_head))
 					rbp->status &= ~RBP_LOANED;
 			goto next_rbrq_entry;
 		}
 
-		if (RBRQ_HBUF_ERR(he_dev->rbrq_head))
-		{
+		if (RBRQ_HBUF_ERR(he_dev->rbrq_head)) {
 			hprintk("HBUF_ERR!  (cid 0x%x)\n", cid);
 				atomic_inc(&vcc->stats->rx_drop);
 			goto return_host_buffers;
@@ -1925,8 +1870,7 @@
 		he_vcc->pdu_len += buf_len;
 		++he_vcc->iov_tail;
 
-		if (RBRQ_CON_CLOSED(he_dev->rbrq_head))
-		{
+		if (RBRQ_CON_CLOSED(he_dev->rbrq_head)) {
 			lastcid = -1;
 			HPRINTK("wake_up rx_waitq  (cid 0x%x)\n", cid);
 			wake_up(&he_vcc->rx_waitq);
@@ -1934,17 +1878,16 @@
 		}
 
 #ifdef notdef
-		if ((he_vcc->iov_tail - he_vcc->iov_head) > HE_MAXIOV)
-		{
+		if ((he_vcc->iov_tail - he_vcc->iov_head) > HE_MAXIOV) {
 			hprintk("iovec full!  cid 0x%x\n", cid);
 			goto return_host_buffers;
 		}
 #endif
-		if (!RBRQ_END_PDU(he_dev->rbrq_head)) goto next_rbrq_entry;
+		if (!RBRQ_END_PDU(he_dev->rbrq_head))
+			goto next_rbrq_entry;
 
 		if (RBRQ_LEN_ERR(he_dev->rbrq_head)
-				|| RBRQ_CRC_ERR(he_dev->rbrq_head))
-		{
+				|| RBRQ_CRC_ERR(he_dev->rbrq_head)) {
 			HPRINTK("%s%s (%d.%d)\n",
 				RBRQ_CRC_ERR(he_dev->rbrq_head)
 							? "CRC_ERR " : "",
@@ -1957,19 +1900,18 @@
 
 		skb = atm_alloc_charge(vcc, he_vcc->pdu_len + rx_skb_reserve,
 							GFP_ATOMIC);
-		if (!skb)
-		{
+		if (!skb) {
 			HPRINTK("charge failed (%d.%d)\n", vcc->vpi, vcc->vci);
 			goto return_host_buffers;
 		}
 
-		if (rx_skb_reserve > 0) skb_reserve(skb, rx_skb_reserve);
+		if (rx_skb_reserve > 0)
+			skb_reserve(skb, rx_skb_reserve);
 
 		do_gettimeofday(&skb->stamp);
 
-		for(iov = he_vcc->iov_head;
-				iov < he_vcc->iov_tail; ++iov)
-		{
+		for (iov = he_vcc->iov_head;
+				iov < he_vcc->iov_tail; ++iov) {
 #ifdef USE_RBPS
 			if (iov->iov_base & RBP_SMALLBUF)
 				memcpy(skb_put(skb, iov->iov_len),
@@ -1980,8 +1922,7 @@
 					he_dev->rbpl_virt[RBP_INDEX(iov->iov_base)].virt, iov->iov_len);
 		}
 
-		switch(vcc->qos.aal)
-		{
+		switch (vcc->qos.aal) {
 			case ATM_AAL0:
 				/* 2.10.1.5 raw cell receive */
 				skb->len = ATM_AAL0_SDU;
@@ -1993,8 +1934,7 @@
 				skb->len = AAL5_LEN(skb->data, he_vcc->pdu_len);
 				skb->tail = skb->data + skb->len;
 #ifdef USE_CHECKSUM_HW
-				if (vcc->vpi == 0 && vcc->vci >= ATM_NOT_RSV_VCI) 
-				{
+				if (vcc->vpi == 0 && vcc->vci >= ATM_NOT_RSV_VCI) {
 					skb->ip_summed = CHECKSUM_HW;
 					skb->csum = TCP_CKSUM(skb->data,
 							he_vcc->pdu_len);
@@ -2018,9 +1958,8 @@
 return_host_buffers:
 		++pdus_assembled;
 
-		for(iov = he_vcc->iov_head;
-				iov < he_vcc->iov_tail; ++iov)
-		{
+		for (iov = he_vcc->iov_head;
+				iov < he_vcc->iov_tail; ++iov) {
 #ifdef USE_RBPS
 			if (iov->iov_base & RBP_SMALLBUF)
 				rbp = &he_dev->rbps_base[RBP_INDEX(iov->iov_base)];
@@ -2041,9 +1980,9 @@
 
 	}
 
-	if (updated)
-	{
-		if (updated > he_dev->rbrq_peak) he_dev->rbrq_peak = updated;
+	if (updated) {
+		if (updated > he_dev->rbrq_peak)
+			he_dev->rbrq_peak = updated;
 
 		he_writel(he_dev, RBRQ_MASK(he_dev->rbrq_head),
 						G0_RBRQ_H + (group * 16));
@@ -2069,8 +2008,7 @@
 
 	/* 2.1.6 transmit buffer return queue */
 
-	while (he_dev->tbrq_head != tbrq_tail)
-	{
+	while (he_dev->tbrq_head != tbrq_tail) {
 		++updated;
 
 		HPRINTK("tbrq%d 0x%x%s%s\n",
@@ -2081,19 +2019,16 @@
 #ifdef USE_TPD_POOL
 		tpd = NULL;
 		p = &he_dev->outstanding_tpds;
-		while ((p = p->next) != &he_dev->outstanding_tpds)
-		{
+		while ((p = p->next) != &he_dev->outstanding_tpds) {
 			struct he_tpd *__tpd = list_entry(p, struct he_tpd, entry);
-			if (TPD_ADDR(__tpd->status) == TBRQ_TPD(he_dev->tbrq_head))
-			{
+			if (TPD_ADDR(__tpd->status) == TBRQ_TPD(he_dev->tbrq_head)) {
 				tpd = __tpd;
 				list_del(&__tpd->entry);
 				break;
 			}
 		}
 
-		if (tpd == NULL)
-		{
+		if (tpd == NULL) {
 			hprintk("unable to locate tpd for dma buffer %x\n",
 						TBRQ_TPD(he_dev->tbrq_head));
 			goto next_tbrq_entry;
@@ -2102,8 +2037,7 @@
 		tpd = &he_dev->tpd_base[ TPD_INDEX(TBRQ_TPD(he_dev->tbrq_head)) ];
 #endif
 
-		if (TBRQ_EOS(he_dev->tbrq_head))
-		{
+		if (TBRQ_EOS(he_dev->tbrq_head)) {
 			HPRINTK("wake_up(tx_waitq) cid 0x%x\n",
 				he_mkcid(he_dev, tpd->vcc->vpi, tpd->vcc->vci));
 			if (tpd->vcc)
@@ -2112,19 +2046,18 @@
 			goto next_tbrq_entry;
 		}
 
-		for(slot = 0; slot < TPD_MAXIOV; ++slot)
-		{
+		for (slot = 0; slot < TPD_MAXIOV; ++slot) {
 			if (tpd->iovec[slot].addr)
 				pci_unmap_single(he_dev->pci_dev,
 					tpd->iovec[slot].addr,
 					tpd->iovec[slot].len & TPD_LEN_MASK,
 							PCI_DMA_TODEVICE);
-			if (tpd->iovec[slot].len & TPD_LST) break;
+			if (tpd->iovec[slot].len & TPD_LST)
+				break;
 				
 		}
 
-		if (tpd->skb)	/* && !TBRQ_MULTIPLE(he_dev->tbrq_head) */
-		{
+		if (tpd->skb) {	/* && !TBRQ_MULTIPLE(he_dev->tbrq_head) */
 			if (tpd->vcc && tpd->vcc->pop)
 				tpd->vcc->pop(tpd->vcc, tpd->skb);
 			else
@@ -2133,7 +2066,8 @@
 
 next_tbrq_entry:
 #ifdef USE_TPD_POOL
-		if (tpd) pci_pool_free(he_dev->tpd_pool, tpd, TPD_ADDR(tpd->status));
+		if (tpd)
+			pci_pool_free(he_dev->tpd_pool, tpd, TPD_ADDR(tpd->status));
 #else
 		tpd->inuse = 0;
 #endif
@@ -2142,9 +2076,9 @@
 					TBRQ_MASK(++he_dev->tbrq_head));
 	}
 
-	if (updated)
-	{
-		if (updated > he_dev->tbrq_peak) he_dev->tbrq_peak = updated;
+	if (updated) {
+		if (updated > he_dev->tbrq_peak)
+			he_dev->tbrq_peak = updated;
 
 		he_writel(he_dev, TBRQ_MASK(he_dev->tbrq_head),
 						G0_TBRQ_H + (group * 16));
@@ -2165,8 +2099,7 @@
 	rbpl_head = (struct he_rbp *) ((unsigned long)he_dev->rbpl_base |
 					RBPL_MASK(he_readl(he_dev, G0_RBPL_S)));
 
-	for(;;)
-	{
+	for (;;) {
 		newtail = (struct he_rbp *) ((unsigned long)he_dev->rbpl_base |
 						RBPL_MASK(he_dev->rbpl_tail+1));
 
@@ -2177,7 +2110,6 @@
 		newtail->status |= RBP_LOANED;
 		he_dev->rbpl_tail = newtail;
 		++moved;
-
 	} 
 
 	if (moved) {
@@ -2199,8 +2131,7 @@
 	rbps_head = (struct he_rbp *) ((unsigned long)he_dev->rbps_base |
 					RBPS_MASK(he_readl(he_dev, G0_RBPS_S)));
 
-	for(;;)
-	{
+	for (;;) {
 		newtail = (struct he_rbp *) ((unsigned long)he_dev->rbps_base |
 						RBPS_MASK(he_dev->rbps_tail+1));
 
@@ -2211,7 +2142,6 @@
 		newtail->status |= RBP_LOANED;
 		he_dev->rbps_tail = newtail;
 		++moved;
-
 	} 
 
 	if (moved) {
@@ -2236,20 +2166,17 @@
 	HE_SPIN_LOCK(he_dev, flags);
 #endif
 
-	while(he_dev->irq_head != he_dev->irq_tail)
-	{
+	while (he_dev->irq_head != he_dev->irq_tail) {
 		++updated;
 
 		type = ITYPE_TYPE(he_dev->irq_head->isw);
 		group = ITYPE_GROUP(he_dev->irq_head->isw);
 
-		switch (type)
-		{
+		switch (type) {
 			case ITYPE_RBRQ_THRESH:
 				hprintk("rbrq%d threshold\n", group);
 			case ITYPE_RBRQ_TIMER:
-				if (he_service_rbrq(he_dev, group))
-				{
+				if (he_service_rbrq(he_dev, group)) {
 					he_service_rbpl(he_dev, group);
 #ifdef USE_RBPS
 					he_service_rbps(he_dev, group);
@@ -2290,8 +2217,7 @@
 				}
 				break;
 			default:
-				if (he_dev->irq_head->isw == ITYPE_INVALID)
-				{
+				if (he_dev->irq_head->isw == ITYPE_INVALID) {
 					/* see 8.1.1 -- check all queues */
 
 					HPRINTK("isw not updated 0x%x\n",
@@ -2314,9 +2240,9 @@
 		he_dev->irq_head = (struct he_irq *) NEXT_ENTRY(he_dev->irq_base, he_dev->irq_head, IRQ_MASK);
 	}
 
-	if (updated)
-	{
-		if (updated > he_dev->irq_peak) he_dev->irq_peak = updated;
+	if (updated) {
+		if (updated > he_dev->irq_peak)
+			he_dev->irq_peak = updated;
 
 		he_writel(he_dev,
 			IRQ_SIZE(CONFIG_IRQ_SIZE) |
@@ -2344,8 +2270,7 @@
 	he_dev->irq_tail = (struct he_irq *) (((unsigned long)he_dev->irq_base) |
 						(*he_dev->irq_tailoffset << 2));
 
-	if (he_dev->irq_tail == he_dev->irq_head)
-	{
+	if (he_dev->irq_tail == he_dev->irq_head) {
 		HPRINTK("tailoffset not updated?\n");
 		he_dev->irq_tail = (struct he_irq *) ((unsigned long)he_dev->irq_base |
 			((he_readl(he_dev, IRQ0_BASE) & IRQ_MASK) << 2));
@@ -2357,8 +2282,7 @@
 		hprintk("spurious (or shared) interrupt?\n");
 #endif
 
-	if (he_dev->irq_head != he_dev->irq_tail)
-	{
+	if (he_dev->irq_head != he_dev->irq_tail) {
 		handled = 1;
 #ifdef USE_TASKLET
 		tasklet_schedule(&he_dev->tasklet);
@@ -2395,14 +2319,12 @@
 	 * head for every enqueue would be unnecessarily slow)
 	 */
 
-	if (new_tail == he_dev->tpdrq_head)
-	{
+	if (new_tail == he_dev->tpdrq_head) {
 		he_dev->tpdrq_head = (struct he_tpdrq *)
 			(((unsigned long)he_dev->tpdrq_base) |
 				TPDRQ_MASK(he_readl(he_dev, TPDRQ_B_H)));
 
-		if (new_tail == he_dev->tpdrq_head)
-		{
+		if (new_tail == he_dev->tpdrq_head) {
 			hprintk("tpdrq full (cid 0x%x)\n", cid);
 			/*
 			 * FIXME
@@ -2410,8 +2332,7 @@
 			 * after service_tbrq, service the backlog
 			 * for now, we just drop the pdu
 			 */
-			if (tpd->skb)
-			{
+			if (tpd->skb) {
 				if (tpd->vcc->pop)
 					tpd->vcc->pop(tpd->vcc, tpd->skb);
 				else
@@ -2456,12 +2377,12 @@
 	unsigned cid, rsr0, rsr1, rsr4, tsr0, tsr0_aal, tsr4, period, reg, clock;
 
 	
-	if ((err = atm_find_ci(vcc, &vpi, &vci)))
-	{
+	if ((err = atm_find_ci(vcc, &vpi, &vci))) {
 		HPRINTK("atm_find_ci err = %d\n", err);
 		return err;
 	}
-	if (vci == ATM_VCI_UNSPEC || vpi == ATM_VPI_UNSPEC) return 0;
+	if (vci == ATM_VCI_UNSPEC || vpi == ATM_VPI_UNSPEC)
+		return 0;
 	vcc->vpi = vpi;
 	vcc->vci = vci;
 
@@ -2472,8 +2393,7 @@
 	cid = he_mkcid(he_dev, vpi, vci);
 
 	he_vcc = (struct he_vcc *) kmalloc(sizeof(struct he_vcc), GFP_ATOMIC);
-	if (he_vcc == NULL)
-	{
+	if (he_vcc == NULL) {
 		hprintk("unable to allocate he_vcc during open\n");
 		return -ENOMEM;
 	}
@@ -2487,8 +2407,7 @@
 
 	HE_VCC(vcc) = he_vcc;
 
-	if (vcc->qos.txtp.traffic_class != ATM_NONE)
-	{
+	if (vcc->qos.txtp.traffic_class != ATM_NONE) {
 		int pcr_goal;
 
                 pcr_goal = atm_pcr_goal(&vcc->qos.txtp);
@@ -2499,8 +2418,7 @@
 
 		HPRINTK("open tx cid 0x%x pcr_goal %d\n", cid, pcr_goal);
 
-		switch (vcc->qos.aal)
-		{
+		switch (vcc->qos.aal) {
 			case ATM_AAL5:
 				tsr0_aal = TSR0_AAL5;
 				tsr4 = TSR4_AAL5;
@@ -2518,15 +2436,13 @@
 		tsr0 = he_readl_tsr0(he_dev, cid);
 		HE_SPIN_UNLOCK(he_dev, flags);
 
-		if (TSR0_CONN_STATE(tsr0) != 0)
-		{
+		if (TSR0_CONN_STATE(tsr0) != 0) {
 			hprintk("cid 0x%x not idle (tsr0 = 0x%x)\n", cid, tsr0);
 			err = -EBUSY;
 			goto open_failed;
 		}
 
-		switch(vcc->qos.txtp.traffic_class)
-		{
+		switch (vcc->qos.txtp.traffic_class) {
 			case ATM_UBR:
 				/* 2.3.3.1 open connection ubr */
 
@@ -2548,13 +2464,12 @@
 				HE_SPIN_LOCK(he_dev, flags);			/* also protects he_dev->cs_stper[] */
 
 				/* find an unused cs_stper register */
-				for(reg = 0; reg < HE_NUM_CS_STPER; ++reg)
+				for (reg = 0; reg < HE_NUM_CS_STPER; ++reg)
 					if (he_dev->cs_stper[reg].inuse == 0 || 
-						he_dev->cs_stper[reg].pcr == pcr_goal)
-					break;
+					    he_dev->cs_stper[reg].pcr == pcr_goal)
+							break;
 
-				if (reg == HE_NUM_CS_STPER)
-				{
+				if (reg == HE_NUM_CS_STPER) {
 					err = -EBUSY;
 					HE_SPIN_UNLOCK(he_dev, flags);
 					goto open_failed;
@@ -2610,15 +2525,13 @@
 		HE_SPIN_UNLOCK(he_dev, flags);
 	}
 
-	if (vcc->qos.rxtp.traffic_class != ATM_NONE)
-	{
+	if (vcc->qos.rxtp.traffic_class != ATM_NONE) {
 		unsigned aal;
 
 		HPRINTK("open rx cid 0x%x (rx_waitq %p)\n", cid,
 		 				&HE_VCC(vcc)->rx_waitq);
 
-		switch (vcc->qos.aal)
-		{
+		switch (vcc->qos.aal) {
 			case ATM_AAL5:
 				aal = RSR0_AAL5;
 				break;
@@ -2633,8 +2546,7 @@
 		HE_SPIN_LOCK(he_dev, flags);
 
 		rsr0 = he_readl_rsr0(he_dev, cid);
-		if (rsr0 & RSR0_OPEN_CONN)
-		{
+		if (rsr0 & RSR0_OPEN_CONN) {
 			HE_SPIN_UNLOCK(he_dev, flags);
 
 			hprintk("cid 0x%x not idle (rsr0 = 0x%x)\n", cid, rsr0);
@@ -2653,7 +2565,8 @@
 				(RSR0_EPD_ENABLE|RSR0_PPD_ENABLE) : 0;
 
 #ifdef USE_CHECKSUM_HW
-		if (vpi == 0 && vci >= ATM_NOT_RSV_VCI) rsr0 |= RSR0_TCP_CKSUM;
+		if (vpi == 0 && vci >= ATM_NOT_RSV_VCI)
+			rsr0 |= RSR0_TCP_CKSUM;
 #endif
 
 		he_writel_rsr4(he_dev, rsr4, cid);
@@ -2675,9 +2588,9 @@
 
 open_failed:
 
-	if (err)
-	{
-		if (he_vcc) kfree(he_vcc);
+	if (err) {
+		if (he_vcc)
+			kfree(he_vcc);
 		clear_bit(ATM_VF_ADDR, &vcc->flags);
 	}
 	else
@@ -2703,8 +2616,7 @@
 	clear_bit(ATM_VF_READY, &vcc->flags);
 	cid = he_mkcid(he_dev, vcc->vpi, vcc->vci);
 
-	if (vcc->qos.rxtp.traffic_class != ATM_NONE)
-	{
+	if (vcc->qos.rxtp.traffic_class != ATM_NONE) {
 		int timeout;
 
 		HPRINTK("close rx cid 0x%x\n", cid);
@@ -2714,8 +2626,7 @@
 		/* wait for previous close (if any) to finish */
 
 		HE_SPIN_LOCK(he_dev, flags);
-		while(he_readl(he_dev, RCC_STAT) & RCC_BUSY)
-		{
+		while (he_readl(he_dev, RCC_STAT) & RCC_BUSY) {
 			HPRINTK("close cid 0x%x RCC_BUSY\n", cid);
 			udelay(250);
 		}
@@ -2745,8 +2656,7 @@
 
 	}
 
-	if (vcc->qos.txtp.traffic_class != ATM_NONE)
-	{
+	if (vcc->qos.txtp.traffic_class != ATM_NONE) {
 		volatile unsigned tsr4, tsr0;
 		int timeout;
 
@@ -2761,19 +2671,19 @@
 		 * TBRQ, the host issues the close command to the adapter.
 		 */
 
-		while (((tx_inuse = atomic_read(&vcc->sk->wmem_alloc)) > 0)
-							&& (retry < MAX_RETRY))
-		{
+		while (((tx_inuse = atomic_read(&vcc->sk->wmem_alloc)) > 0) &&
+		       (retry < MAX_RETRY)) {
 			set_current_state(TASK_UNINTERRUPTIBLE);
 			(void) schedule_timeout(sleep);
 			set_current_state(TASK_RUNNING);
-			if (sleep < HZ) sleep = sleep * 2;
+			if (sleep < HZ)
+				sleep = sleep * 2;
 
 			++retry;
 		}
 
-		if (tx_inuse) hprintk("close tx cid 0x%x tx_inuse = %d\n",
-								cid, tx_inuse);
+		if (tx_inuse)
+			hprintk("close tx cid 0x%x tx_inuse = %d\n", cid, tx_inuse);
 
 		/* 2.3.1.1 generic close operations with flush */
 
@@ -2784,8 +2694,7 @@
 		(void) he_readl_tsr4(he_dev, cid);
 #endif
 
-		switch(vcc->qos.txtp.traffic_class)
-		{
+		switch (vcc->qos.txtp.traffic_class) {
 			case ATM_UBR:
 				he_writel_tsr1(he_dev, 
 					TSR1_MCR(rate_to_atmf(200000))
@@ -2796,10 +2705,8 @@
 				break;
 		}
 
-
 		tpd = __alloc_tpd(he_dev);
-		if (tpd == NULL)
-		{
+		if (tpd == NULL) {
 			hprintk("close tx he_alloc_tpd failed cid 0x%x\n", cid);
 			goto close_tx_incomplete;
 		}
@@ -2818,30 +2725,25 @@
 		remove_wait_queue(&he_vcc->tx_waitq, &wait);
 		set_current_state(TASK_RUNNING);
 
-		if (timeout == 0)
-		{
+		if (timeout == 0) {
 			hprintk("close tx timeout cid 0x%x\n", cid);
 			goto close_tx_incomplete;
 		}
 
 		HE_SPIN_LOCK(he_dev, flags);
-		while (!((tsr4 = he_readl_tsr4(he_dev, cid))
-							& TSR4_SESSION_ENDED))
-		{
+		while (!((tsr4 = he_readl_tsr4(he_dev, cid)) & TSR4_SESSION_ENDED)) {
 			HPRINTK("close tx cid 0x%x !TSR4_SESSION_ENDED (tsr4 = 0x%x)\n", cid, tsr4);
 			udelay(250);
 		}
 
-		while (TSR0_CONN_STATE(tsr0 = he_readl_tsr0(he_dev, cid)) != 0)
-		{
+		while (TSR0_CONN_STATE(tsr0 = he_readl_tsr0(he_dev, cid)) != 0) {
 			HPRINTK("close tx cid 0x%x TSR0_CONN_STATE != 0 (tsr0 = 0x%x)\n", cid, tsr0);
 			udelay(250);
 		}
 
 close_tx_incomplete:
 
-		if (vcc->qos.txtp.traffic_class == ATM_CBR)
-		{
+		if (vcc->qos.txtp.traffic_class == ATM_CBR) {
 			int reg = he_vcc->rc_index;
 
 			HPRINTK("cs_stper reg = %d\n", reg);
@@ -2889,8 +2791,7 @@
 	HPRINTK("send %d.%d\n", vcc->vpi, vcc->vci);
 
 	if ((skb->len > HE_TPD_BUFSIZE) ||
-		((vcc->qos.aal == ATM_AAL0) && (skb->len != ATM_AAL0_SDU)))
-	{
+	    ((vcc->qos.aal == ATM_AAL0) && (skb->len != ATM_AAL0_SDU))) {
 		hprintk("buffer too large (or small) -- %d bytes\n", skb->len );
 		if (vcc->pop)
 			vcc->pop(vcc, skb);
@@ -2901,8 +2802,7 @@
 	}
 
 #ifndef USE_SCATTERGATHER
-	if (skb_shinfo(skb)->nr_frags)
-	{
+	if (skb_shinfo(skb)->nr_frags) {
 		hprintk("no scatter/gather support\n");
 		if (vcc->pop)
 			vcc->pop(vcc, skb);
@@ -2915,8 +2815,7 @@
 	HE_SPIN_LOCK(he_dev, flags);
 
 	tpd = __alloc_tpd(he_dev);
-	if (tpd == NULL)
-	{
+	if (tpd == NULL) {
 		if (vcc->pop)
 			vcc->pop(vcc, skb);
 		else
@@ -2928,15 +2827,15 @@
 
 	if (vcc->qos.aal == ATM_AAL5)
 		tpd->status |= TPD_CELLTYPE(TPD_USERCELL);
-	else
-	{
+	else {
 		char *pti_clp = (void *) (skb->data + 3);
 		int clp, pti;
 
 		pti = (*pti_clp & ATM_HDR_PTI_MASK) >> ATM_HDR_PTI_SHIFT; 
 		clp = (*pti_clp & ATM_HDR_CLP);
 		tpd->status |= TPD_CELLTYPE(pti);
-		if (clp) tpd->status |= TPD_CLP;
+		if (clp)
+			tpd->status |= TPD_CLP;
 
 		skb_pull(skb, ATM_AAL0_SDU - ATM_CELL_PAYLOAD);
 	}
@@ -2947,12 +2846,10 @@
 	tpd->iovec[slot].len = skb->len - skb->data_len;
 	++slot;
 
-	for (i = 0; i < skb_shinfo(skb)->nr_frags; i++)
-	{
+	for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
 		skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
 
-		if (slot == TPD_MAXIOV)		/* send tpd; start new tpd */
-		{
+		if (slot == TPD_MAXIOV) {	/* queue tpd; start new tpd */
 			tpd->vcc = vcc;
 			tpd->skb = NULL;	/* not the last fragment
 						   so dont ->push() yet */
@@ -2960,8 +2857,7 @@
 
 			__enqueue_tpd(he_dev, tpd, cid);
 			tpd = __alloc_tpd(he_dev);
-			if (tpd == NULL)
-			{
+			if (tpd == NULL) {
 				if (vcc->pop)
 					vcc->pop(vcc, skb);
 				else
@@ -3010,16 +2906,15 @@
 	struct he_ioctl_reg reg;
 	int err = 0;
 
-	switch (cmd)
-	{
+	switch (cmd) {
 		case HE_GET_REG:
-			if (!capable(CAP_NET_ADMIN)) return -EPERM;
+			if (!capable(CAP_NET_ADMIN))
+				return -EPERM;
 
 			copy_from_user(&reg, (struct he_ioctl_reg *) arg,
 						sizeof(struct he_ioctl_reg));
 			HE_SPIN_LOCK(he_dev, flags);
-			switch (reg.type)
-			{
+			switch (reg.type) {
 				case HE_REGTYPE_PCI:
 					reg.val = he_readl(he_dev, reg.addr);
 					break;
@@ -3040,7 +2935,8 @@
 					break;
 			}
 			HE_SPIN_UNLOCK(he_dev, flags);
-			if (err == 0) copy_to_user((struct he_ioctl_reg *) arg, &reg,
+			if (err == 0)
+				copy_to_user((struct he_ioctl_reg *) arg, &reg,
 							sizeof(struct he_ioctl_reg));
 			break;
 		default:
@@ -3048,7 +2944,7 @@
 			if (atm_dev->phy && atm_dev->phy->ioctl)
 				err = atm_dev->phy->ioctl(atm_dev, cmd, arg);
 #else /* CONFIG_ATM_HE_USE_SUNI */
-			return -EINVAL;
+			err = -EINVAL;
 #endif /* CONFIG_ATM_HE_USE_SUNI */
 			break;
 	}
@@ -3146,7 +3042,8 @@
         rbpl_tail = RBPL_MASK(he_readl(he_dev, G0_RBPL_T));
 
 	inuse = rbpl_head - rbpl_tail;
-	if (inuse < 0) inuse += CONFIG_RBPL_SIZE * sizeof(struct he_rbp);
+	if (inuse < 0)
+		inuse += CONFIG_RBPL_SIZE * sizeof(struct he_rbp);
 	inuse /= sizeof(struct he_rbp);
 
 	if (!left--)

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.1160  -> 1.1161 
#	    drivers/atm/he.c	1.9     -> 1.10   
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 03/05/25	chas@relax.cmf.nrl.navy.mil	1.1161
# misc irq handler cleanups
# --------------------------------------------
#
diff -Nru a/drivers/atm/he.c b/drivers/atm/he.c
--- a/drivers/atm/he.c	Thu May 29 11:49:13 2003
+++ b/drivers/atm/he.c	Thu May 29 11:49:13 2003
@@ -2232,7 +2232,8 @@
 
 		switch (type) {
 			case ITYPE_RBRQ_THRESH:
-				hprintk("rbrq%d threshold\n", group);
+				HPRINTK("rbrq%d threshold\n", group);
+				/* fall through */
 			case ITYPE_RBRQ_TIMER:
 				if (he_service_rbrq(he_dev, group)) {
 					he_service_rbpl(he_dev, group);
@@ -2242,7 +2243,8 @@
 				}
 				break;
 			case ITYPE_TBRQ_THRESH:
-				hprintk("tbrq%d threshold\n", group);
+				HPRINTK("tbrq%d threshold\n", group);
+				/* fall through */
 			case ITYPE_TPD_COMPLETE:
 				he_service_tbrq(he_dev, group);
 				break;
@@ -2255,17 +2257,16 @@
 #endif /* USE_RBPS */
 				break;
 			case ITYPE_PHY:
+				HPRINTK("phy interrupt\n");
 #ifdef CONFIG_ATM_HE_USE_SUNI
 				HE_SPIN_UNLOCK(he_dev, flags);
 				if (he_dev->atm_dev->phy && he_dev->atm_dev->phy->interrupt)
 					he_dev->atm_dev->phy->interrupt(he_dev->atm_dev);
 				HE_SPIN_LOCK(he_dev, flags);
 #endif
-				HPRINTK("phy interrupt\n");
 				break;
 			case ITYPE_OTHER:
-				switch (type|group)
-				{
+				switch (type|group) {
 					case ITYPE_PARITY:
 						hprintk("parity error\n");
 						break;
@@ -2274,23 +2275,20 @@
 						break;
 				}
 				break;
-			default:
-				if (he_dev->irq_head->isw == ITYPE_INVALID) {
-					/* see 8.1.1 -- check all queues */
+			case ITYPE_TYPE(ITYPE_INVALID):
+				/* see 8.1.1 -- check all queues */
 
-					HPRINTK("isw not updated 0x%x\n",
-						he_dev->irq_head->isw);
+				HPRINTK("isw not updated 0x%x\n", he_dev->irq_head->isw);
 
-					he_service_rbrq(he_dev, 0);
-					he_service_rbpl(he_dev, 0);
+				he_service_rbrq(he_dev, 0);
+				he_service_rbpl(he_dev, 0);
 #ifdef USE_RBPS
-					he_service_rbps(he_dev, 0);
+				he_service_rbps(he_dev, 0);
 #endif /* USE_RBPS */
-					he_service_tbrq(he_dev, 0);
-				}
-				else
-					hprintk("bad isw = 0x%x?\n",
-						he_dev->irq_head->isw);
+				he_service_tbrq(he_dev, 0);
+				break;
+			default:
+				hprintk("bad isw 0x%x?\n", he_dev->irq_head->isw);
 		}
 
 		he_dev->irq_head->isw = ITYPE_INVALID;

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.1161  -> 1.1162 
#	    drivers/atm/he.c	1.10    -> 1.11   
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 03/05/29	chas@relax.cmf.nrl.navy.mil	1.1162
# move rategrid off stack
# --------------------------------------------
#
diff -Nru a/drivers/atm/he.c b/drivers/atm/he.c
--- a/drivers/atm/he.c	Thu May 29 11:49:27 2003
+++ b/drivers/atm/he.c	Thu May 29 11:49:27 2003
@@ -672,10 +672,10 @@
 
 }
 
-static void __init
+static int __init
 he_init_cs_block_rcm(struct he_dev *he_dev)
 {
-	unsigned rategrid[16][16];
+	unsigned (*rategrid)[16][16];
 	unsigned rate, delta;
 	int i, j, reg;
 
@@ -683,6 +683,10 @@
 	unsigned long long rate_cps;
         int mult, buf, buf_limit = 4;
 
+	rategrid = kmalloc( sizeof(unsigned) * 16 * 16, GFP_KERNEL);
+	if (!rategrid)
+		return -ENOMEM;
+
 	/* initialize rate grid group table */
 
 	for (reg = 0x0; reg < 0xff; ++reg)
@@ -712,16 +716,16 @@
 	 */
 
 	for (j = 0; j < 16; j++) {
-		rategrid[0][j] = rate;
+		(*rategrid)[0][j] = rate;
 		rate -= delta;
 	}
 
 	for (i = 1; i < 16; i++)
 		for (j = 0; j < 16; j++)
 			if (i > 14)
-				rategrid[i][j] = rategrid[i - 1][j] / 4;
+				(*rategrid)[i][j] = (*rategrid)[i - 1][j] / 4;
 			else
-				rategrid[i][j] = rategrid[i - 1][j] / 2;
+				(*rategrid)[i][j] = (*rategrid)[i - 1][j] / 2;
 
 	/*
 	 * 2.4 transmit internal function
@@ -746,7 +750,7 @@
 			rate_cps = 10;	/* 2.2.1 minimum payload rate is 10 cps */
 
 		for (i = 255; i > 0; i--)
-			if (rategrid[i/16][i%16] >= rate_cps)
+			if ((*rategrid)[i/16][i%16] >= rate_cps)
 				break;	 /* pick nearest rate instead? */
 
 		/*
@@ -783,6 +787,9 @@
 
 		++rate_atmf;
 	}
+
+	kfree(rategrid);
+	return 0;
 }
 
 static int __init
@@ -1505,7 +1512,8 @@
 
 	/* 5.1.8 cs block connection memory initialization */
 	
-	he_init_cs_block_rcm(he_dev);
+	if (he_init_cs_block_rcm(he_dev) < 0)
+		return -ENOMEM;
 
 	/* 5.1.10 initialize host structures */
 

Re: [PATCH][ATM] assorted he driver cleanup

[Christoph Hellwig]

On Thu, May 29, 2003 at 12:09:54PM -0400, chas williams wrote:
> the three following changesets attempt to bring the he atm
> driver into conformance with the accepted linux coding style,
> fix some chattiness the irq handler, and address the stack
> usage issue in he_init_cs_block_rcm().

btw, could you also remove the BUS_INT_WAR hacks?  There shouldn't
be anz SHUB1.0 Altix systems around anymore..

Re: [PATCH][ATM] assorted he driver cleanup

[chas williams]

In message <20030529171927.A21828@infradead.org>,Christoph Hellwig writes:
>btw, could you also remove the BUS_INT_WAR hacks?  There shouldn't
>be anz SHUB1.0 Altix systems around anymore..

we still have a few :) (but they arent in service so i guess i could
remove it).

Re: [PATCH][ATM] assorted he driver cleanup

[Arnaldo Carvalho de Melo]

Em Thu, May 29, 2003 at 12:09:54PM -0400, chas williams escreveu:
> the three following changesets attempt to bring the he atm
> driver into conformance with the accepted linux coding style,
> fix some chattiness the irq handler, and address the stack
> usage issue in he_init_cs_block_rcm().

great, comments below
 
> 
> # This is a BitKeeper generated patch for the following project:
> # Project Name: Linux kernel tree
> # This patch format is intended for GNU patch command version 2.5 or higher.
> # This patch includes the following deltas:
> #	           ChangeSet	1.1158  -> 1.1159 
> #	    drivers/atm/he.c	1.7     -> 1.8    
> #
> # The following is the BitKeeper ChangeSet Log
> # --------------------------------------------
> # 03/05/23	chas@relax.cmf.nrl.navy.mil	1.1159
> # he coding style conformance
> # --------------------------------------------
> #
> diff -Nru a/drivers/atm/he.c b/drivers/atm/he.c
> --- a/drivers/atm/he.c	Thu May 29 11:48:40 2003
> +++ b/drivers/atm/he.c	Thu May 29 11:48:40 2003
> @@ -132,9 +132,9 @@
>  
>  #undef DEBUG
>  #ifdef DEBUG
> -#define HPRINTK(fmt,args...)	hprintk(fmt,args)
> +#define HPRINTK(fmt,args...)	printk(KERN_DEBUG DEV_LABEL "%d: " fmt, he_dev->number , ##args)
>  #else
> -#define HPRINTK(fmt,args...)	do { } while(0)
> +#define HPRINTK(fmt,args...)	do { } while (0)
>  #endif /* DEBUG */
>  
>  
> @@ -179,9 +179,7 @@
>     phy_put:	he_phy_put,
>     phy_get:	he_phy_get,
>     proc_read:	he_proc_read,
> -#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,4,1)
>     owner:	THIS_MODULE
> -#endif
>  };
>  
>  /* see the comments in he.h about global_lock */
> @@ -189,7 +187,7 @@
>  #define HE_SPIN_LOCK(dev, flags)	spin_lock_irqsave(&(dev)->global_lock, flags)
>  #define HE_SPIN_UNLOCK(dev, flags)	spin_unlock_irqrestore(&(dev)->global_lock, flags)

Is the above really needed?
  
> -#define he_writel(dev, val, reg)	do { writel(val, (dev)->membase + (reg)); wmb(); } while(0)
> +#define he_writel(dev, val, reg)	do { writel(val, (dev)->membase + (reg)); wmb(); } while (0)
>  #define he_readl(dev, reg)		readl((dev)->membase + (reg))
>  
>  /* section 2.12 connection memory access */
> @@ -203,7 +201,7 @@
>  	(void) he_readl(he_dev, CON_DAT);
>  #endif
>  	he_writel(he_dev, flags | CON_CTL_WRITE | CON_CTL_ADDR(addr), CON_CTL);
> -	while(he_readl(he_dev, CON_CTL) & CON_CTL_BUSY);
> +	while (he_readl(he_dev, CON_CTL) & CON_CTL_BUSY);
>  }
>  
>  #define he_writel_rcm(dev, val, reg) 				\
> @@ -219,7 +217,7 @@
>  he_readl_internal(struct he_dev *he_dev, unsigned addr, unsigned flags)
>  {
>  	he_writel(he_dev, flags | CON_CTL_READ | CON_CTL_ADDR(addr), CON_CTL);
> -	while(he_readl(he_dev, CON_CTL) & CON_CTL_BUSY);
> +	while (he_readl(he_dev, CON_CTL) & CON_CTL_BUSY);
>  	return he_readl(he_dev, CON_DAT);
>  }
>  
> @@ -374,7 +372,8 @@
>  
>  	printk(KERN_INFO "he: %s\n", version);
>  
> -	if (pci_enable_device(pci_dev)) return -EIO;
> +	if (pci_enable_device(pci_dev))
> +		return -EIO;
>  	if (pci_set_dma_mask(pci_dev, HE_DMA_MASK) != 0) {
>  		printk(KERN_WARNING "he: no suitable dma available\n");
>  		err = -EIO;
> @@ -407,7 +406,8 @@
>  		goto init_one_failure;
>  	}
>  	he_dev->next = NULL;
> -	if (he_devs) he_dev->next = he_devs;
> +	if (he_devs)
> +		he_dev->next = he_devs;
>  	he_devs = he_dev;
>  	return 0;
>  
> @@ -447,11 +447,11 @@
>  
>          unsigned exp = 0;
>  
> -        if (rate == 0) return(0);
> +        if (rate == 0)
> +		return(0);

return is not a function, so 'return 0;' is the preferred style

>  
>          rate <<= 9;
> -        while (rate > 0x3ff)
> -        {
> +        while (rate > 0x3ff) {
>                  ++exp;
>                  rate >>= 1;
>          }
> @@ -472,16 +472,14 @@
>  
>  	he_writel(he_dev, lbufd_index, RLBF0_H);
>  
> -        for (i = 0, lbuf_count = 0; i < he_dev->r0_numbuffs; ++i)
> -        {
> +        for (i = 0, lbuf_count = 0; i < he_dev->r0_numbuffs; ++i) {
>  		lbufd_index += 2;
>                  lbuf_addr = (row_offset + (lbuf_count * lbuf_bufsize)) / 32;
>  
>  		he_writel_rcm(he_dev, lbuf_addr, lbm_offset);
>  		he_writel_rcm(he_dev, lbufd_index, lbm_offset + 1);
>  
> -                if (++lbuf_count == lbufs_per_row)
> -                {
> +                if (++lbuf_count == lbufs_per_row) {
>                          lbuf_count = 0;
>                          row_offset += he_dev->bytes_per_row;
>                  }
> @@ -505,16 +503,14 @@
>  
>  	he_writel(he_dev, lbufd_index, RLBF1_H);
>  
> -        for (i = 0, lbuf_count = 0; i < he_dev->r1_numbuffs; ++i)
> -        {
> +        for (i = 0, lbuf_count = 0; i < he_dev->r1_numbuffs; ++i) {
>  		lbufd_index += 2;
>                  lbuf_addr = (row_offset + (lbuf_count * lbuf_bufsize)) / 32;
>  
>  		he_writel_rcm(he_dev, lbuf_addr, lbm_offset);
>  		he_writel_rcm(he_dev, lbufd_index, lbm_offset + 1);
>  
> -                if (++lbuf_count == lbufs_per_row)
> -                {
> +                if (++lbuf_count == lbufs_per_row) {
>                          lbuf_count = 0;
>                          row_offset += he_dev->bytes_per_row;
>                  }
> @@ -538,16 +534,14 @@
>  
>  	he_writel(he_dev, lbufd_index, TLBF_H);
>  
> -        for (i = 0, lbuf_count = 0; i < he_dev->tx_numbuffs; ++i)
> -        {
> +        for (i = 0, lbuf_count = 0; i < he_dev->tx_numbuffs; ++i) {
>  		lbufd_index += 1;
>                  lbuf_addr = (row_offset + (lbuf_count * lbuf_bufsize)) / 32;
>  
>  		he_writel_rcm(he_dev, lbuf_addr, lbm_offset);
>  		he_writel_rcm(he_dev, lbufd_index, lbm_offset + 1);
>  
> -                if (++lbuf_count == lbufs_per_row)
> -                {
> +                if (++lbuf_count == lbufs_per_row) {
>                          lbuf_count = 0;
>                          row_offset += he_dev->bytes_per_row;
>                  }
> @@ -562,8 +556,7 @@
>  {
>  	he_dev->tpdrq_base = pci_alloc_consistent(he_dev->pci_dev,
>  		CONFIG_TPDRQ_SIZE * sizeof(struct he_tpdrq), &he_dev->tpdrq_phys);
> -	if (he_dev->tpdrq_base == NULL) 
> -	{
> +	if (he_dev->tpdrq_base == NULL) {
>  		hprintk("failed to alloc tpdrq\n");
>  		return -ENOMEM;
>  	}
> @@ -588,7 +581,7 @@
>  
>  	/* 5.1.7 cs block initialization */
>  
> -	for(reg = 0; reg < 0x20; ++reg)
> +	for (reg = 0; reg < 0x20; ++reg)
>  		he_writel_mbox(he_dev, 0x0, CS_STTIM0 + reg);
>  
>  	/* rate grid timer reload values */
> @@ -597,8 +590,7 @@
>  	rate = he_dev->atm_dev->link_rate;
>  	delta = rate / 16 / 2;
>  
> -	for(reg = 0; reg < 0x10; ++reg)
> -	{
> +	for (reg = 0; reg < 0x10; ++reg) {
>  		/* 2.4 internal transmit function
>  		 *
>  	 	 * we initialize the first row in the rate grid.
> @@ -610,8 +602,7 @@
>  		rate -= delta;
>  	}
>  
> -	if (he_is622(he_dev))
> -	{
> +	if (he_is622(he_dev)) {
>  		/* table 5.2 (4 cells per lbuf) */
>  		he_writel_mbox(he_dev, 0x000800fa, CS_ERTHR0);
>  		he_writel_mbox(he_dev, 0x000c33cb, CS_ERTHR1);
> @@ -640,9 +631,7 @@
>  		/* table 5.9 */
>  		he_writel_mbox(he_dev, 0x5, CS_OTPPER);
>  		he_writel_mbox(he_dev, 0x14, CS_OTWPER);
> -	}
> -	else
> -	{
> +	} else {
>  		/* table 5.1 (4 cells per lbuf) */
>  		he_writel_mbox(he_dev, 0x000400ea, CS_ERTHR0);
>  		he_writel_mbox(he_dev, 0x00063388, CS_ERTHR1);
> @@ -671,12 +660,11 @@
>  		/* table 5.9 */
>  		he_writel_mbox(he_dev, 0x6, CS_OTPPER);
>  		he_writel_mbox(he_dev, 0x1e, CS_OTWPER);
> -
>  	}
>  
>  	he_writel_mbox(he_dev, 0x8, CS_OTTLIM);
>  
> -	for(reg = 0; reg < 0x8; ++reg)
> +	for (reg = 0; reg < 0x8; ++reg)
>  		he_writel_mbox(he_dev, 0x0, CS_HGRRT0 + reg);
>  
>  }
> @@ -720,8 +708,7 @@
>  	 * in order to construct the rate to group table below
>  	 */
>  
> -	for (j = 0; j < 16; j++)
> -	{
> +	for (j = 0; j < 16; j++) {
>  		rategrid[0][j] = rate;
>  		rate -= delta;
>  	}
> @@ -742,8 +729,7 @@
>  	 */
>  
>  	rate_atmf = 0;
> -	while (rate_atmf < 0x400)
> -	{
> +	while (rate_atmf < 0x400) {
>  		man = (rate_atmf & 0x1f) << 4;
>  		exp = rate_atmf >> 5;
>  
> @@ -753,12 +739,12 @@
>  		*/
>  		rate_cps = (unsigned long long) (1 << exp) * (man + 512) >> 9;
>  
> -		if (rate_cps < 10) rate_cps = 10;
> -				/* 2.2.1 minimum payload rate is 10 cps */
> +		if (rate_cps < 10)
> +			rate_cps = 10;	/* 2.2.1 minimum payload rate is 10 cps */
>  
>  		for (i = 255; i > 0; i--)
> -			if (rategrid[i/16][i%16] >= rate_cps) break;
> -				/* pick nearest rate instead? */
> +			if (rategrid[i/16][i%16] >= rate_cps)
> +				break;	 /* pick nearest rate instead? */
>  
>  		/*
>  		 * each table entry is 16 bits: (rate grid index (8 bits)
> @@ -773,12 +759,17 @@
>  		/* this is pretty, but avoids _divdu3 and is mostly correct */
>                  buf = 0;
>                  mult = he_dev->atm_dev->link_rate / ATM_OC3_PCR;
> -                if (rate_cps > (68 * mult)) buf = 1;
> -                if (rate_cps > (136 * mult)) buf = 2;
> -                if (rate_cps > (204 * mult)) buf = 3;
> -                if (rate_cps > (272 * mult)) buf = 4;
> +                if (rate_cps > (68 * mult))
> +			buf = 1;
> +                if (rate_cps > (136 * mult))
> +			buf = 2;
> +                if (rate_cps > (204 * mult))
> +			buf = 3;
> +                if (rate_cps > (272 * mult))
> +			buf = 4;
>  #endif
> -                if (buf > buf_limit) buf = buf_limit;
> +                if (buf > buf_limit)
> +			buf = buf_limit;
>  		reg = (reg<<16) | ((i<<8) | buf);
>  
>  #define RTGTBL_OFFSET 0x400
> @@ -801,8 +792,7 @@
>  #ifdef USE_RBPS_POOL
>  	he_dev->rbps_pool = pci_pool_create("rbps", he_dev->pci_dev,
>  			CONFIG_RBPS_BUFSIZE, 8, 0);
> -	if (he_dev->rbps_pool == NULL)
> -	{
> +	if (he_dev->rbps_pool == NULL) {
>  		hprintk("unable to create rbps pages\n");
>  		return -ENOMEM;
>  	}
> @@ -817,16 +807,14 @@
>  
>  	he_dev->rbps_base = pci_alloc_consistent(he_dev->pci_dev,
>  		CONFIG_RBPS_SIZE * sizeof(struct he_rbp), &he_dev->rbps_phys);
> -	if (he_dev->rbps_base == NULL)
> -	{
> +	if (he_dev->rbps_base == NULL) {
>  		hprintk("failed to alloc rbps\n");
>  		return -ENOMEM;
>  	}
>  	memset(he_dev->rbps_base, 0, CONFIG_RBPS_SIZE * sizeof(struct he_rbp));
>  	he_dev->rbps_virt = kmalloc(CONFIG_RBPS_SIZE * sizeof(struct he_virt), GFP_KERNEL);
>  
> -	for (i = 0; i < CONFIG_RBPS_SIZE; ++i)
> -	{
> +	for (i = 0; i < CONFIG_RBPS_SIZE; ++i) {
>  		dma_addr_t dma_handle;
>  		void *cpuaddr;
>  
> @@ -868,16 +856,14 @@
>  #ifdef USE_RBPL_POOL
>  	he_dev->rbpl_pool = pci_pool_create("rbpl", he_dev->pci_dev,
>  			CONFIG_RBPL_BUFSIZE, 8, 0);
> -	if (he_dev->rbpl_pool == NULL)
> -	{
> +	if (he_dev->rbpl_pool == NULL) {
>  		hprintk("unable to create rbpl pool\n");
>  		return -ENOMEM;
>  	}
>  #else /* !USE_RBPL_POOL */
>  	he_dev->rbpl_pages = (void *) pci_alloc_consistent(he_dev->pci_dev,
>  		CONFIG_RBPL_SIZE * CONFIG_RBPL_BUFSIZE, &he_dev->rbpl_pages_phys);
> -	if (he_dev->rbpl_pages == NULL)
> -	{
> +	if (he_dev->rbpl_pages == NULL) {
>  		hprintk("unable to create rbpl pages\n");
>  		return -ENOMEM;
>  	}
> @@ -885,16 +871,14 @@
>  
>  	he_dev->rbpl_base = pci_alloc_consistent(he_dev->pci_dev,
>  		CONFIG_RBPL_SIZE * sizeof(struct he_rbp), &he_dev->rbpl_phys);
> -	if (he_dev->rbpl_base == NULL)
> -	{
> +	if (he_dev->rbpl_base == NULL) {
>  		hprintk("failed to alloc rbpl\n");
>  		return -ENOMEM;
>  	}
>  	memset(he_dev->rbpl_base, 0, CONFIG_RBPL_SIZE * sizeof(struct he_rbp));
>  	he_dev->rbpl_virt = kmalloc(CONFIG_RBPL_SIZE * sizeof(struct he_virt), GFP_KERNEL);
>  
> -	for (i = 0; i < CONFIG_RBPL_SIZE; ++i)
> -	{
> +	for (i = 0; i < CONFIG_RBPL_SIZE; ++i) {
>  		dma_addr_t dma_handle;
>  		void *cpuaddr;
>  
> @@ -910,7 +894,6 @@
>  		he_dev->rbpl_virt[i].virt = cpuaddr;
>  		he_dev->rbpl_base[i].status = RBP_LOANED | (i << RBP_INDEX_OFF);
>  		he_dev->rbpl_base[i].phys = dma_handle;
> -
>  	}
>  	he_dev->rbpl_tail = &he_dev->rbpl_base[CONFIG_RBPL_SIZE-1];
>  
> @@ -929,8 +912,7 @@
>  
>  	he_dev->rbrq_base = pci_alloc_consistent(he_dev->pci_dev,
>  		CONFIG_RBRQ_SIZE * sizeof(struct he_rbrq), &he_dev->rbrq_phys);
> -	if (he_dev->rbrq_base == NULL)
> -	{
> +	if (he_dev->rbrq_base == NULL) {
>  		hprintk("failed to allocate rbrq\n");
>  		return -ENOMEM;
>  	}
> @@ -942,13 +924,11 @@
>  	he_writel(he_dev,
>  		RBRQ_THRESH(CONFIG_RBRQ_THRESH) | RBRQ_SIZE(CONFIG_RBRQ_SIZE-1),
>  						G0_RBRQ_Q + (group * 16));
> -	if (irq_coalesce)
> -	{
> +	if (irq_coalesce) {
>  		hprintk("coalescing interrupts\n");
>  		he_writel(he_dev, RBRQ_TIME(768) | RBRQ_COUNT(7),
>  						G0_RBRQ_I + (group * 16));
> -	}
> -	else
> +	} else
>  		he_writel(he_dev, RBRQ_TIME(0) | RBRQ_COUNT(1),
>  						G0_RBRQ_I + (group * 16));
>  
> @@ -956,8 +936,7 @@
>  
>  	he_dev->tbrq_base = pci_alloc_consistent(he_dev->pci_dev,
>  		CONFIG_TBRQ_SIZE * sizeof(struct he_tbrq), &he_dev->tbrq_phys);
> -	if (he_dev->tbrq_base == NULL)
> -	{
> +	if (he_dev->tbrq_base == NULL) {
>  		hprintk("failed to allocate tbrq\n");
>  		return -ENOMEM;
>  	}
> @@ -983,8 +962,7 @@
>  
>          he_dev->irq_base = pci_alloc_consistent(he_dev->pci_dev,
>  			(CONFIG_IRQ_SIZE+1) * sizeof(struct he_irq), &he_dev->irq_phys);
> -	if (he_dev->irq_base == NULL)
> -	{
> +	if (he_dev->irq_base == NULL) {
>  		hprintk("failed to allocate irq\n");
>  		return -ENOMEM;
>  	}
> @@ -994,7 +972,7 @@
>  	he_dev->irq_head = he_dev->irq_base;
>  	he_dev->irq_tail = he_dev->irq_base;
>  
> -	for(i=0; i < CONFIG_IRQ_SIZE; ++i)
> +	for (i=0; i < CONFIG_IRQ_SIZE; ++i)

use spaces before and after basic operators

	for (i = 0; i < CONFIG_IRQ_SIZE; ++i)

>  		he_dev->irq_base[i].isw = ITYPE_INVALID;
>  
>  	he_writel(he_dev, he_dev->irq_phys, IRQ0_BASE);
> @@ -1026,8 +1004,7 @@
>  	he_writel(he_dev, 0x0, GRP_54_MAP);
>  	he_writel(he_dev, 0x0, GRP_76_MAP);
>  
> -	if (request_irq(he_dev->pci_dev->irq, he_irq_handler, SA_INTERRUPT|SA_SHIRQ, DEV_LABEL, he_dev))
> -	{
> +	if (request_irq(he_dev->pci_dev->irq, he_irq_handler, SA_INTERRUPT|SA_SHIRQ, DEV_LABEL, he_dev)) {
>  		hprintk("irq %d already in use\n", he_dev->pci_dev->irq);
>  		return -EINVAL;
>          }   
> @@ -1067,46 +1044,39 @@
>  	 */
>  
>  	/* 4.3 pci bus controller-specific initialization */
> -	if (pci_read_config_dword(pci_dev, GEN_CNTL_0, &gen_cntl_0) != 0)
> -	{
> +	if (pci_read_config_dword(pci_dev, GEN_CNTL_0, &gen_cntl_0) != 0) {
>  		hprintk("can't read GEN_CNTL_0\n");

Humm, shouldn't we release the irq here? What about using gotos for exception
handing?

>  		return -EINVAL;
>  	}
>  	gen_cntl_0 |= (MRL_ENB | MRM_ENB | IGNORE_TIMEOUT);
> -	if (pci_write_config_dword(pci_dev, GEN_CNTL_0, gen_cntl_0) != 0)
> -	{
> +	if (pci_write_config_dword(pci_dev, GEN_CNTL_0, gen_cntl_0) != 0) {
>  		hprintk("can't write GEN_CNTL_0.\n");

ditto

>  		return -EINVAL;
>  	}
>  
> -	if (pci_read_config_word(pci_dev, PCI_COMMAND, &command) != 0)
> -	{
> +	if (pci_read_config_word(pci_dev, PCI_COMMAND, &command) != 0) {
>  		hprintk("can't read PCI_COMMAND.\n");

ditto

>  		return -EINVAL;
>  	}
>  
>  	command |= (PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER | PCI_COMMAND_INVALIDATE);
> -	if (pci_write_config_word(pci_dev, PCI_COMMAND, command) != 0)
> -	{
> +	if (pci_write_config_word(pci_dev, PCI_COMMAND, command) != 0) {
>  		hprintk("can't enable memory.\n");

ditto

>  		return -EINVAL;
>  	}
>  
> -	if (pci_read_config_byte(pci_dev, PCI_CACHE_LINE_SIZE, &cache_size))
> -	{
> +	if (pci_read_config_byte(pci_dev, PCI_CACHE_LINE_SIZE, &cache_size)) {
>  		hprintk("can't read cache line size?\n");

ditto

>  		return -EINVAL;
>  	}
>  
> -	if (cache_size < 16)
> -	{
> +	if (cache_size < 16) {
>  		cache_size = 16;
>  		if (pci_write_config_byte(pci_dev, PCI_CACHE_LINE_SIZE, cache_size))
>  			hprintk("can't set cache line size to %d\n", cache_size);
>  	}
>  
> -	if (pci_read_config_byte(pci_dev, PCI_LATENCY_TIMER, &timer))
> -	{
> +	if (pci_read_config_byte(pci_dev, PCI_LATENCY_TIMER, &timer)) {
>  		hprintk("can't read latency timer?\n");

ditto

I'll stop here :-)

Re: [PATCH][ATM] assorted he driver cleanup

[chas williams]

In message <20030529162125.GU24054@conectiva.com.br>,Arnaldo Carvalho de Melo w
rites:
>> @@ -189,7 +187,7 @@
>>  #define HE_SPIN_LOCK(dev, flags)	spin_lock_irqsave(&(dev)->global_lock, 
>flags)
>>  #define HE_SPIN_UNLOCK(dev, flags)	spin_unlock_irqrestore(&(dev)->global_l
>ock, flags)
>
>Is the above really needed?

well, according to the programmer's guide:

8.1.7   PCI Transaction Ordering Error

PROBLEM: The PCI Bus Controller, in addition to bus master and general
target functionality, acts as a bridge to a local bus. If a read is
issued to the local bus and the read COMPLETES on the local bus but is
not yet completed on the PCI bus, a subsequent write to the local bus
that completes on the PCI bus will cause the write data to be written
to the last local bus read address.

RESOLUTION: In an environment where PCI bus bridge settings cannot be
controlled, card drivers must lock read and write access to the ATM
Network Controller and the SONET Framer device. In an environment where
PCI bus bridge settings can be controlled, features that allow reordering
of reads and writes from separate processors (i.e., Posted memory write
features) should be disabled.

Re: [PATCH][ATM] assorted he driver cleanup

[Arnaldo Carvalho de Melo]

Em Thu, May 29, 2003 at 12:31:17PM -0400, chas williams escreveu:
> In message <20030529162125.GU24054@conectiva.com.br>,Arnaldo Carvalho de Melo w
> rites:
> >> @@ -189,7 +187,7 @@
> >>  #define HE_SPIN_LOCK(dev, flags)	spin_lock_irqsave(&(dev)->global_lock, 
> >flags)
> >>  #define HE_SPIN_UNLOCK(dev, flags)	spin_unlock_irqrestore(&(dev)->global_l
> >ock, flags)
> >
> >Is the above really needed?
> 
> well, according to the programmer's guide:

no, no, I was talking just about the need for HE_SPIN_LOCK wrapper, not the
locking, i.e. couldn't it be just:

spin_lock_irqsave(&dev->global_lock, flags)

used so that it is clear that it is a irqsave variation, etc?

- Arnaldo

Re: [PATCH][ATM] assorted he driver cleanup

[chas williams]

In message <20030529170621.GX24054@conectiva.com.br>,Arnaldo Carvalho de Melo w
rites:
>no, no, I was talking just about the need for HE_SPIN_LOCK wrapper, not the
>locking, i.e. couldn't it be just:
>
>spin_lock_irqsave(&dev->global_lock, flags)
>
>used so that it is clear that it is a irqsave variation, etc?

i suppose i could change it all back.  at one point, he_spin_lock()
was 'optmized' away on non smp machines (since a single cpu doesnt
tickle the particular h/w problem). 

i didnt want a ton of #ifdef CONFIG_SMP in the driver.

Re: [PATCH][ATM] assorted he driver cleanup

[Arnaldo Carvalho de Melo]

Em Thu, May 29, 2003 at 01:12:13PM -0400, chas williams escreveu:
> In message <20030529170621.GX24054@conectiva.com.br>,Arnaldo Carvalho de Melo w
> rites:
> >no, no, I was talking just about the need for HE_SPIN_LOCK wrapper, not the
> >locking, i.e. couldn't it be just:
> >
> >spin_lock_irqsave(&dev->global_lock, flags)
> >
> >used so that it is clear that it is a irqsave variation, etc?
> 
> i suppose i could change it all back.  at one point, he_spin_lock()
> was 'optmized' away on non smp machines (since a single cpu doesnt
> tickle the particular h/w problem). 
> 
> i didnt want a ton of #ifdef CONFIG_SMP in the driver.

Sure thing, but hey, spin_lock_irqsave and friends take care of how to behave
when in up or smp, i.e. its how all the other drivers use spinlocks 8)

- Arnaldo

Re: [PATCH][ATM] assorted he driver cleanup

[chas williams]

In message <20030529173637.GZ24054@conectiva.com.br>,Arnaldo Carvalho de Melo writes:
>Sure thing, but hey, spin_lock_irqsave and friends take care of how to behave
>when in up or smp, i.e. its how all the other drivers use spinlocks 8)

but on a single processor machine (i.e. #undef CONFIG_SMP) there is no
chance that there will be reads/writes from other processors so i dont
need any locking OR protection from interrupts.  so the degenerate case
of spin_lock_irqsave() isnt quite as dengerate as i would like for this
particular spin lock.

Re: [PATCH][ATM] assorted he driver cleanup

[Alan Cox]

On Sul, 2003-06-01 at 19:57, chas williams wrote:
> In message <20030529173637.GZ24054@conectiva.com.br>,Arnaldo Carvalho de Melo writes:
> >Sure thing, but hey, spin_lock_irqsave and friends take care of how to behave
> >when in up or smp, i.e. its how all the other drivers use spinlocks 8)
> 
> but on a single processor machine (i.e. #undef CONFIG_SMP) there is no
> chance that there will be reads/writes from other processors so i dont
> need any locking OR protection from interrupts.  so the degenerate case
> of spin_lock_irqsave() isnt quite as dengerate as i would like for this
> particular spin lock.

Then why are you using spin_lock_irqsave ?

Re: [PATCH][ATM] assorted he driver cleanup

[chas williams]

In message <1054497613.5863.4.camel@dhcp22.swansea.linux.org.uk>,Alan Cox writes:
>> but on a single processor machine (i.e. #undef CONFIG_SMP) there is no
>> chance that there will be reads/writes from other processors so i dont
>> need any locking OR protection from interrupts.  so the degenerate case
>> of spin_lock_irqsave() isnt quite as dengerate as i would like for this
>> particular spin lock.
>
>Then why are you using spin_lock_irqsave ?

meaning just use spin_lock() or what?

Re: [PATCH][ATM] assorted he driver cleanup

[David S. Miller]

   From: chas williams <chas@cmf.nrl.navy.mil>
   Date: Sun, 01 Jun 2003 18:58:26 -0400

   In message <1054497613.5863.4.camel@dhcp22.swansea.linux.org.uk>,Alan Cox writes:
   >Then why are you using spin_lock_irqsave ?
   
   meaning just use spin_lock() or what?
   
Alan/Chas, there are two different issues here:

1) Aparently the bug only needs to be worked around when
   multiple cpus can access the card at the same time.

   Therefore on uniprocessor the bug isn't relevant.

2) Therefore, the lock needs to protect register accesses
   from all contexts.  Therefore he needs an IRQ protecting
   lock.

Therefore it isn't legal for him to use a non-IRQ protecting
spinlock.

I personally don't think it's worth all the maintainence cost
to special case all of this junk for uniprocessor.

Re: [PATCH][ATM] assorted he driver cleanup

[Arnaldo Carvalho de Melo]

Em Sun, Jun 01, 2003 at 06:42:54PM -0700, David S. Miller escreveu:
>    From: chas williams <chas@cmf.nrl.navy.mil>
>    Date: Sun, 01 Jun 2003 18:58:26 -0400
> 
>    In message <1054497613.5863.4.camel@dhcp22.swansea.linux.org.uk>,Alan Cox writes:
>    >Then why are you using spin_lock_irqsave ?
>    
>    meaning just use spin_lock() or what?
>    
> Alan/Chas, there are two different issues here:
> 
> 1) Aparently the bug only needs to be worked around when
>    multiple cpus can access the card at the same time.
> 
>    Therefore on uniprocessor the bug isn't relevant.
> 
> 2) Therefore, the lock needs to protect register accesses
>    from all contexts.  Therefore he needs an IRQ protecting
>    lock.
> 
> Therefore it isn't legal for him to use a non-IRQ protecting
> spinlock.
> 
> I personally don't think it's worth all the maintainence cost
> to special case all of this junk for uniprocessor.

Agreed.

- Arnaldo

Re: [PATCH][ATM] assorted he driver cleanup

[chas williams]

In message <20030601.184254.71111683.davem@redhat.com>,"David S. Miller" writes:
>1) Aparently the bug only needs to be worked around when
>   multiple cpus can access the card at the same time.
>   Therefore on uniprocessor the bug isn't relevant.

right.  which is why i optimized the HE_SPIN_LOCK away on
!CONFIG_SMP.  actually, its probably not a problem on certain
smp machines either.  i dont believe the i386 will reorder
read/writes from multiple cpus so in theory it would be safe to
do away with this lock on smp i386's.  the only arch that 
can reorder (and actually has posted read/writes) is the ia64/sn2.

>I personally don't think it's worth all the maintainence cost
>to special case all of this junk for uniprocessor.

well it wasnt much cost but i essentially decided this some
time ago and just defined HE_SPIN_LOCK to be spin_lock_irqsave.
the lock probably needs to be more fine-grained though in the
tasklet handler though.

Re: [PATCH][ATM] assorted he driver cleanup

[Alan Cox]

On Llu, 2003-06-02 at 03:34, chas williams wrote:
> smp machines either.  i dont believe the i386 will reorder
> read/writes from multiple cpus so in theory it would be safe to
> do away with this lock on smp i386's.  the only arch that 
> can reorder (and actually has posted read/writes) is the ia64/sn2.

The only i386 platforms that will re-order writes I know of are the
Natsemi Geode (which will do magic so that the write order seen
by PCI is always ordered), Pentium Pro (fence bug) and IDT
Winchip (but only to main memory)

Re: [PATCH][ATM] assorted he driver cleanup

[David S. Miller]

   From: chas williams <chas@cmf.nrl.navy.mil>
   Date: Sun, 01 Jun 2003 14:57:02 -0400
   
   but on a single processor machine (i.e. #undef CONFIG_SMP) there is
   no chance that there will be reads/writes from other processors so
   i dont need any locking OR protection from interrupts.

Either you need to pretect a code sequence from an IRQ context
execution while holding the lock or you don't. :-)

Really, all your macros just make the driver that much less
portable.  And I don't see what you'll gain by having interrupts
enabled for such short sequences of code on uniprocessor.

Re: [PATCH][ATM] assorted he driver cleanup

[chas williams]

In message <20030529162125.GU24054@conectiva.com.br>,Arnaldo Carvalho de Melo w
rites:
>> @@ -1067,46 +1044,39 @@
>>  	 */
>>  
>>  	/* 4.3 pci bus controller-specific initialization */
>> -	if (pci_read_config_dword(pci_dev, GEN_CNTL_0, &gen_cntl_0) != 0)
>> -	{
>> +	if (pci_read_config_dword(pci_dev, GEN_CNTL_0, &gen_cntl_0) != 0) {
>>  		hprintk("can't read GEN_CNTL_0\n");
>
>Humm, shouldn't we release the irq here? What about using gotos for exception
>handing?

what irq?  the irq is grabbed in he_init_irq.  if he_start() fails, we 
call he_stop() and it frees the irq.

Re: [PATCH][ATM] assorted he driver cleanup

[Arnaldo Carvalho de Melo]

Em Thu, May 29, 2003 at 02:08:59PM -0400, chas williams escreveu:
> In message <20030529162125.GU24054@conectiva.com.br>,Arnaldo Carvalho de Melo w
> rites:
> >> @@ -1067,46 +1044,39 @@
> >>  	 */
> >>  
> >>  	/* 4.3 pci bus controller-specific initialization */
> >> -	if (pci_read_config_dword(pci_dev, GEN_CNTL_0, &gen_cntl_0) != 0)
> >> -	{
> >> +	if (pci_read_config_dword(pci_dev, GEN_CNTL_0, &gen_cntl_0) != 0) {
> >>  		hprintk("can't read GEN_CNTL_0\n");
> >
> >Humm, shouldn't we release the irq here? What about using gotos for exception
> >handing?
> 
> what irq?  the irq is grabbed in he_init_irq.  if he_start() fails, we 
> call he_stop() and it frees the irq.

OK sir, sorry for the noise on this one :-)

Re: [PATCH][ATM] assorted he driver cleanup

[David S. Miller]

   From: chas williams <chas@cmf.nrl.navy.mil>
   Date: Thu, 29 May 2003 12:09:54 -0400

   the three following changesets attempt to bring the he atm
   driver into conformance with the accepted linux coding style,
   fix some chattiness the irq handler, and address the stack
   usage issue in he_init_cs_block_rcm().

Applied, thanks.

You can integrate ideas posted in this thread in subsequent
patches.

BTW, can you use more consistent changeset messages?  I always
at least allude to what is being changed, for example I changed
all of your messages to be of the form:

	[ATM]: Blah blah blah in HE driver.

This tells the reader that:

1) It's an ATM change.

2) It's to the HE driver.

3) The change made was "Blah blah blah" :-)

Re: [PATCH][ATM] assorted he driver cleanup

[Dave Jones]

On Thu, May 29, 2003 at 08:01:01PM -0700, David S. Miller wrote:

 > BTW, can you use more consistent changeset messages?  I always
 > at least allude to what is being changed, for example I changed
 > all of your messages to be of the form:
 > 
 > 	[ATM]: Blah blah blah in HE driver.
 > 
 > This tells the reader that:
 > 
 > 1) It's an ATM change.
 > 2) It's to the HE driver.
 > 3) The change made was "Blah blah blah" :-)

I'd also add...
4) keep the first line a brief oneliner summary, as it
gets truncated when Linus generates the shortlog.
 
A number of people (self included) have adopted this
impromptu 'standard' for bk comments. Maybe it should
get documented in Documentation/BK-usage/bk-kernel-howto.txt

		Dave

Re: [PATCH][ATM] assorted he driver cleanup

[Jeff Garzik]

On Fri, May 30, 2003 at 09:57:26AM +0100, Dave Jones wrote:
> A number of people (self included) have adopted this
> impromptu 'standard' for bk comments. Maybe it should
> get documented in Documentation/BK-usage/bk-kernel-howto.txt

Post a patch!  ;-)

	Jeff

Re: [PATCH][ATM] assorted he driver cleanup

[chas williams]

In message <20030529.200101.118622651.davem@redhat.com>,"David S. Miller" write
s:
>You can integrate ideas posted in this thread in subsequent
>patches.

[ATM]: more coding style cleanups in HE driver

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.1161  -> 1.1162 
#	    drivers/atm/he.c	1.10    -> 1.11   
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 03/05/30	chas@relax.cmf.nrl.navy.mil	1.1162
# he.c:
#   [ATM]: more coding style cleanups in HE driver
# --------------------------------------------
#
diff -Nru a/drivers/atm/he.c b/drivers/atm/he.c
--- a/drivers/atm/he.c	Fri May 30 09:10:54 2003
+++ b/drivers/atm/he.c	Fri May 30 09:10:54 2003
@@ -77,14 +77,6 @@
 #include <linux/atmdev.h>
 #include <linux/atm.h>
 #include <linux/sonet.h>
-#ifndef ATM_OC12_PCR
-#define ATM_OC12_PCR (622080000/1080*1040/8/53)
-#endif
-
-#ifdef BUS_INT_WAR
-void sn_add_polled_interrupt(int irq, int interval);
-void sn_delete_polled_interrupt(int irq);
-#endif
 
 #define USE_TASKLET
 #define USE_HE_FIND_VCC
@@ -171,22 +163,17 @@
 
 static struct atmdev_ops he_ops =
 {
-   open:	he_open,
-   close:	he_close,	
-   ioctl:	he_ioctl,	
-   send:	he_send,
-   sg_send:	he_sg_send,	
-   phy_put:	he_phy_put,
-   phy_get:	he_phy_get,
-   proc_read:	he_proc_read,
-   owner:	THIS_MODULE
+	.open =		he_open,
+	.close =	he_close,	
+	.ioctl =	he_ioctl,	
+	.send =		he_send,
+	.sg_send =	he_sg_send,	
+	.phy_put =	he_phy_put,
+	.phy_get =	he_phy_get,
+	.proc_read =	he_proc_read,
+	.owner =	THIS_MODULE
 };
 
-/* see the comments in he.h about global_lock */
-
-#define HE_SPIN_LOCK(dev, flags)	spin_lock_irqsave(&(dev)->global_lock, flags)
-#define HE_SPIN_UNLOCK(dev, flags)	spin_unlock_irqrestore(&(dev)->global_lock, flags)
-
 #define he_writel(dev, val, reg)	do { writel(val, (dev)->membase + (reg)); wmb(); } while (0)
 #define he_readl(dev, reg)		readl((dev)->membase + (reg))
 
@@ -233,26 +220,26 @@
 
 /* figure 2.2 connection id */
 
-#define he_mkcid(dev, vpi, vci)		(((vpi<<(dev)->vcibits) | vci) & 0x1fff)
+#define he_mkcid(dev, vpi, vci)		(((vpi << (dev)->vcibits) | vci) & 0x1fff)
 
 /* 2.5.1 per connection transmit state registers */
 
 #define he_writel_tsr0(dev, val, cid) \
-		he_writel_tcm(dev, val, CONFIG_TSRA | (cid<<3) | 0)
+		he_writel_tcm(dev, val, CONFIG_TSRA | (cid << 3) | 0)
 #define he_readl_tsr0(dev, cid) \
-		he_readl_tcm(dev, CONFIG_TSRA | (cid<<3) | 0)
+		he_readl_tcm(dev, CONFIG_TSRA | (cid << 3) | 0)
 
 #define he_writel_tsr1(dev, val, cid) \
-		he_writel_tcm(dev, val, CONFIG_TSRA | (cid<<3) | 1)
+		he_writel_tcm(dev, val, CONFIG_TSRA | (cid << 3) | 1)
 
 #define he_writel_tsr2(dev, val, cid) \
-		he_writel_tcm(dev, val, CONFIG_TSRA | (cid<<3) | 2)
+		he_writel_tcm(dev, val, CONFIG_TSRA | (cid << 3) | 2)
 
 #define he_writel_tsr3(dev, val, cid) \
-		he_writel_tcm(dev, val, CONFIG_TSRA | (cid<<3) | 3)
+		he_writel_tcm(dev, val, CONFIG_TSRA | (cid << 3) | 3)
 
 #define he_writel_tsr4(dev, val, cid) \
-		he_writel_tcm(dev, val, CONFIG_TSRA | (cid<<3) | 4)
+		he_writel_tcm(dev, val, CONFIG_TSRA | (cid << 3) | 4)
 
 	/* from page 2-20
 	 *
@@ -263,43 +250,43 @@
 	 */
 
 #define he_writel_tsr4_upper(dev, val, cid) \
-		he_writel_internal(dev, val, CONFIG_TSRA | (cid<<3) | 4, \
+		he_writel_internal(dev, val, CONFIG_TSRA | (cid << 3) | 4, \
 							CON_CTL_TCM \
 							| CON_BYTE_DISABLE_2 \
 							| CON_BYTE_DISABLE_1 \
 							| CON_BYTE_DISABLE_0)
 
 #define he_readl_tsr4(dev, cid) \
-		he_readl_tcm(dev, CONFIG_TSRA | (cid<<3) | 4)
+		he_readl_tcm(dev, CONFIG_TSRA | (cid << 3) | 4)
 
 #define he_writel_tsr5(dev, val, cid) \
-		he_writel_tcm(dev, val, CONFIG_TSRA | (cid<<3) | 5)
+		he_writel_tcm(dev, val, CONFIG_TSRA | (cid << 3) | 5)
 
 #define he_writel_tsr6(dev, val, cid) \
-		he_writel_tcm(dev, val, CONFIG_TSRA | (cid<<3) | 6)
+		he_writel_tcm(dev, val, CONFIG_TSRA | (cid << 3) | 6)
 
 #define he_writel_tsr7(dev, val, cid) \
-		he_writel_tcm(dev, val, CONFIG_TSRA | (cid<<3) | 7)
+		he_writel_tcm(dev, val, CONFIG_TSRA | (cid << 3) | 7)
 
 
 #define he_writel_tsr8(dev, val, cid) \
-		he_writel_tcm(dev, val, CONFIG_TSRB | (cid<<2) | 0)
+		he_writel_tcm(dev, val, CONFIG_TSRB | (cid << 2) | 0)
 
 #define he_writel_tsr9(dev, val, cid) \
-		he_writel_tcm(dev, val, CONFIG_TSRB | (cid<<2) | 1)
+		he_writel_tcm(dev, val, CONFIG_TSRB | (cid << 2) | 1)
 
 #define he_writel_tsr10(dev, val, cid) \
-		he_writel_tcm(dev, val, CONFIG_TSRB | (cid<<2) | 2)
+		he_writel_tcm(dev, val, CONFIG_TSRB | (cid << 2) | 2)
 
 #define he_writel_tsr11(dev, val, cid) \
-		he_writel_tcm(dev, val, CONFIG_TSRB | (cid<<2) | 3)
+		he_writel_tcm(dev, val, CONFIG_TSRB | (cid << 2) | 3)
 
 
 #define he_writel_tsr12(dev, val, cid) \
-		he_writel_tcm(dev, val, CONFIG_TSRC | (cid<<1) | 0)
+		he_writel_tcm(dev, val, CONFIG_TSRC | (cid << 1) | 0)
 
 #define he_writel_tsr13(dev, val, cid) \
-		he_writel_tcm(dev, val, CONFIG_TSRC | (cid<<1) | 1)
+		he_writel_tcm(dev, val, CONFIG_TSRC | (cid << 1) | 1)
 
 
 #define he_writel_tsr14(dev, val, cid) \
@@ -315,30 +302,30 @@
 /* 2.7.1 per connection receive state registers */
 
 #define he_writel_rsr0(dev, val, cid) \
-		he_writel_rcm(dev, val, 0x00000 | (cid<<3) | 0)
+		he_writel_rcm(dev, val, 0x00000 | (cid << 3) | 0)
 #define he_readl_rsr0(dev, cid) \
-		he_readl_rcm(dev, 0x00000 | (cid<<3) | 0)
+		he_readl_rcm(dev, 0x00000 | (cid << 3) | 0)
 
 #define he_writel_rsr1(dev, val, cid) \
-		he_writel_rcm(dev, val, 0x00000 | (cid<<3) | 1)
+		he_writel_rcm(dev, val, 0x00000 | (cid << 3) | 1)
 
 #define he_writel_rsr2(dev, val, cid) \
-		he_writel_rcm(dev, val, 0x00000 | (cid<<3) | 2)
+		he_writel_rcm(dev, val, 0x00000 | (cid << 3) | 2)
 
 #define he_writel_rsr3(dev, val, cid) \
-		he_writel_rcm(dev, val, 0x00000 | (cid<<3) | 3)
+		he_writel_rcm(dev, val, 0x00000 | (cid << 3) | 3)
 
 #define he_writel_rsr4(dev, val, cid) \
-		he_writel_rcm(dev, val, 0x00000 | (cid<<3) | 4)
+		he_writel_rcm(dev, val, 0x00000 | (cid << 3) | 4)
 
 #define he_writel_rsr5(dev, val, cid) \
-		he_writel_rcm(dev, val, 0x00000 | (cid<<3) | 5)
+		he_writel_rcm(dev, val, 0x00000 | (cid << 3) | 5)
 
 #define he_writel_rsr6(dev, val, cid) \
-		he_writel_rcm(dev, val, 0x00000 | (cid<<3) | 6)
+		he_writel_rcm(dev, val, 0x00000 | (cid << 3) | 6)
 
 #define he_writel_rsr7(dev, val, cid) \
-		he_writel_rcm(dev, val, 0x00000 | (cid<<3) | 7)
+		he_writel_rcm(dev, val, 0x00000 | (cid << 3) | 7)
 
 static __inline__ struct atm_vcc*
 he_find_vcc(struct he_dev *he_dev, unsigned cid)
@@ -349,7 +336,7 @@
 	int vci;
 
 	vpi = cid >> he_dev->vcibits;
-	vci = cid & ((1<<he_dev->vcibits)-1);
+	vci = cid & ((1 << he_dev->vcibits) - 1);
 
 	spin_lock_irqsave(&he_dev->atm_dev->lock, flags);
 	for (vcc = he_dev->atm_dev->vccs; vcc; vcc = vcc->next)
@@ -443,12 +430,12 @@
 static unsigned
 rate_to_atmf(unsigned rate)		/* cps to atm forum format */
 {
-#define NONZERO (1<<14)
+#define NONZERO (1 << 14)
 
         unsigned exp = 0;
 
         if (rate == 0)
-		return(0);
+		return 0;
 
         rate <<= 9;
         while (rate > 0x3ff) {
@@ -669,10 +656,10 @@
 
 }
 
-static int __init
+static void __init
 he_init_cs_block_rcm(struct he_dev *he_dev)
 {
-	unsigned (*rategrid)[16][16];
+	unsigned rategrid[16][16];
 	unsigned rate, delta;
 	int i, j, reg;
 
@@ -680,10 +667,6 @@
 	unsigned long long rate_cps;
         int mult, buf, buf_limit = 4;
 
-	rategrid = kmalloc( sizeof(unsigned) * 16 * 16, GFP_KERNEL);
-	if (!rategrid)
-		return -ENOMEM;
-
 	/* initialize rate grid group table */
 
 	for (reg = 0x0; reg < 0xff; ++reg)
@@ -713,16 +696,16 @@
 	 */
 
 	for (j = 0; j < 16; j++) {
-		(*rategrid)[0][j] = rate;
+		rategrid[0][j] = rate;
 		rate -= delta;
 	}
 
 	for (i = 1; i < 16; i++)
 		for (j = 0; j < 16; j++)
 			if (i > 14)
-				(*rategrid)[i][j] = (*rategrid)[i - 1][j] / 4;
+				rategrid[i][j] = rategrid[i - 1][j] / 4;
 			else
-				(*rategrid)[i][j] = (*rategrid)[i - 1][j] / 2;
+				rategrid[i][j] = rategrid[i - 1][j] / 2;
 
 	/*
 	 * 2.4 transmit internal function
@@ -747,7 +730,7 @@
 			rate_cps = 10;	/* 2.2.1 minimum payload rate is 10 cps */
 
 		for (i = 255; i > 0; i--)
-			if ((*rategrid)[i/16][i%16] >= rate_cps)
+			if (rategrid[i/16][i%16] >= rate_cps)
 				break;	 /* pick nearest rate instead? */
 
 		/*
@@ -761,32 +744,30 @@
 				(he_dev->atm_dev->link_rate * 2);
 #else
 		/* this is pretty, but avoids _divdu3 and is mostly correct */
-                buf = 0;
                 mult = he_dev->atm_dev->link_rate / ATM_OC3_PCR;
-                if (rate_cps > (68 * mult))
-			buf = 1;
-                if (rate_cps > (136 * mult))
-			buf = 2;
-                if (rate_cps > (204 * mult))
-			buf = 3;
                 if (rate_cps > (272 * mult))
 			buf = 4;
+		else if (rate_cps > (204 * mult))
+			buf = 3;
+		else if (rate_cps > (136 * mult))
+			buf = 2;
+		else if (rate_cps > (68 * mult))
+			buf = 1;
+		else
+			buf = 0;
 #endif
                 if (buf > buf_limit)
 			buf = buf_limit;
-		reg = (reg<<16) | ((i<<8) | buf);
+		reg = (reg << 16) | ((i << 8) | buf);
 
 #define RTGTBL_OFFSET 0x400
 	  
 		if (rate_atmf & 0x1)
 			he_writel_rcm(he_dev, reg,
-				CONFIG_RCMABR + RTGTBL_OFFSET + (rate_atmf>>1));
+				CONFIG_RCMABR + RTGTBL_OFFSET + (rate_atmf >> 1));
 
 		++rate_atmf;
 	}
-
-	kfree(rategrid);
-	return 0;
 }
 
 static int __init
@@ -839,7 +820,7 @@
 		he_dev->rbps_base[i].phys = dma_handle;
 
 	}
-	he_dev->rbps_tail = &he_dev->rbps_base[CONFIG_RBPS_SIZE-1];
+	he_dev->rbps_tail = &he_dev->rbps_base[CONFIG_RBPS_SIZE - 1];
 
 	he_writel(he_dev, he_dev->rbps_phys, G0_RBPS_S + (group * 32));
 	he_writel(he_dev, RBPS_MASK(he_dev->rbps_tail),
@@ -848,7 +829,7 @@
 						G0_RBPS_BS + (group * 32));
 	he_writel(he_dev,
 			RBP_THRESH(CONFIG_RBPS_THRESH) |
-			RBP_QSIZE(CONFIG_RBPS_SIZE-1) |
+			RBP_QSIZE(CONFIG_RBPS_SIZE - 1) |
 			RBP_INT_ENB,
 						G0_RBPS_QI + (group * 32));
 #else /* !USE_RBPS */
@@ -902,7 +883,7 @@
 		he_dev->rbpl_base[i].status = RBP_LOANED | (i << RBP_INDEX_OFF);
 		he_dev->rbpl_base[i].phys = dma_handle;
 	}
-	he_dev->rbpl_tail = &he_dev->rbpl_base[CONFIG_RBPL_SIZE-1];
+	he_dev->rbpl_tail = &he_dev->rbpl_base[CONFIG_RBPL_SIZE - 1];
 
 	he_writel(he_dev, he_dev->rbpl_phys, G0_RBPL_S + (group * 32));
 	he_writel(he_dev, RBPL_MASK(he_dev->rbpl_tail),
@@ -911,7 +892,7 @@
 						G0_RBPL_BS + (group * 32));
 	he_writel(he_dev,
 			RBP_THRESH(CONFIG_RBPL_THRESH) |
-			RBP_QSIZE(CONFIG_RBPL_SIZE-1) |
+			RBP_QSIZE(CONFIG_RBPL_SIZE - 1) |
 			RBP_INT_ENB,
 						G0_RBPL_QI + (group * 32));
 
@@ -929,7 +910,7 @@
 	he_writel(he_dev, he_dev->rbrq_phys, G0_RBRQ_ST + (group * 16));
 	he_writel(he_dev, 0, G0_RBRQ_H + (group * 16));
 	he_writel(he_dev,
-		RBRQ_THRESH(CONFIG_RBRQ_THRESH) | RBRQ_SIZE(CONFIG_RBRQ_SIZE-1),
+		RBRQ_THRESH(CONFIG_RBRQ_THRESH) | RBRQ_SIZE(CONFIG_RBRQ_SIZE - 1),
 						G0_RBRQ_Q + (group * 16));
 	if (irq_coalesce) {
 		hprintk("coalescing interrupts\n");
@@ -979,7 +960,7 @@
 	he_dev->irq_head = he_dev->irq_base;
 	he_dev->irq_tail = he_dev->irq_base;
 
-	for (i=0; i < CONFIG_IRQ_SIZE; ++i)
+	for (i = 0; i < CONFIG_IRQ_SIZE; ++i)
 		he_dev->irq_base[i].isw = ITYPE_INVALID;
 
 	he_writel(he_dev, he_dev->irq_phys, IRQ0_BASE);
@@ -1018,11 +999,6 @@
 
 	he_dev->irq = he_dev->pci_dev->irq;
 
-#ifdef BUS_INT_WAR
-        HPRINTK("sn_add_polled_interrupt(irq %d, 1)\n", he_dev->irq);
-        sn_add_polled_interrupt(he_dev->irq, 1);
-#endif
-
 	return 0;
 }
 
@@ -1138,12 +1114,12 @@
 	pci_write_config_dword(pci_dev, GEN_CNTL_0, gen_cntl_0);
 
 	/* 4.7 read prom contents */
-	for (i=0; i<PROD_ID_LEN; ++i)
+	for (i = 0; i < PROD_ID_LEN; ++i)
 		he_dev->prod_id[i] = read_prom_byte(he_dev, PROD_ID + i);
 
 	he_dev->media = read_prom_byte(he_dev, MEDIA);
 
-	for (i=0; i<6; ++i)
+	for (i = 0; i < 6; ++i)
 		dev->esi[i] = read_prom_byte(he_dev, MAC_ADDR + i);
 
 	hprintk("%s%s, %x:%x:%x:%x:%x:%x\n",
@@ -1323,15 +1299,15 @@
 	he_writel(he_dev, 0x0, TXAAL5_PROTO);
 
 	he_writel(he_dev, PHY_INT_ENB |
-		(he_is622(he_dev) ? PTMR_PRE(67-1) : PTMR_PRE(50-1)),
+		(he_is622(he_dev) ? PTMR_PRE(67 - 1) : PTMR_PRE(50 - 1)),
 								RH_CONFIG);
 
 	/* 5.1.3 initialize connection memory */
 
-	for (i=0; i < TCM_MEM_SIZE; ++i)
+	for (i = 0; i < TCM_MEM_SIZE; ++i)
 		he_writel_tcm(he_dev, 0, i);
 
-	for (i=0; i < RCM_MEM_SIZE; ++i)
+	for (i = 0; i < RCM_MEM_SIZE; ++i)
 		he_writel_rcm(he_dev, 0, i);
 
 	/*
@@ -1484,8 +1460,7 @@
 
 	/* 5.1.8 cs block connection memory initialization */
 	
-	if (he_init_cs_block_rcm(he_dev) < 0)
-		return -ENOMEM;
+	he_init_cs_block_rcm(he_dev);
 
 	/* 5.1.10 initialize host structures */
 
@@ -1512,7 +1487,7 @@
 	}
 		
 	he_dev->tpd_head = he_dev->tpd_base;
-	he_dev->tpd_end = &he_dev->tpd_base[CONFIG_NUMTPDS-1];
+	he_dev->tpd_end = &he_dev->tpd_base[CONFIG_NUMTPDS - 1];
 #endif
 
 	if (he_init_group(he_dev, 0) != 0)
@@ -1652,12 +1627,8 @@
 		he_dev->atm_dev->phy->stop(he_dev->atm_dev);
 #endif /* CONFIG_ATM_HE_USE_SUNI */
 
-	if (he_dev->irq) {
-#ifdef BUS_INT_WAR
-		sn_delete_polled_interrupt(he_dev->irq);
-#endif
+	if (he_dev->irq)
 		free_irq(he_dev->irq, he_dev);
-	}
 
 	if (he_dev->irq_base)
 		pci_free_consistent(he_dev->pci_dev, (CONFIG_IRQ_SIZE+1)
@@ -1669,7 +1640,7 @@
 
 	if (he_dev->rbpl_base) {
 #ifdef USE_RBPL_POOL
-		for (i=0; i<CONFIG_RBPL_SIZE; ++i) {
+		for (i = 0; i < CONFIG_RBPL_SIZE; ++i) {
 			void *cpuaddr = he_dev->rbpl_virt[i].virt;
 			dma_addr_t dma_handle = he_dev->rbpl_base[i].phys;
 
@@ -1691,7 +1662,7 @@
 #ifdef USE_RBPS
 	if (he_dev->rbps_base) {
 #ifdef USE_RBPS_POOL
-		for (i=0; i<CONFIG_RBPS_SIZE; ++i) {
+		for (i = 0; i < CONFIG_RBPS_SIZE; ++i) {
 			void *cpuaddr = he_dev->rbps_virt[i].virt;
 			dma_addr_t dma_handle = he_dev->rbps_base[i].phys;
 
@@ -1790,7 +1761,7 @@
 }
 
 #define AAL5_LEN(buf,len) 						\
-			((((unsigned char *)(buf))[(len)-6]<<8) |	\
+			((((unsigned char *)(buf))[(len)-6] << 8) |	\
 				(((unsigned char *)(buf))[(len)-5]))
 
 /* 2.10.1.2 receive
@@ -1800,7 +1771,7 @@
  */
 
 #define TCP_CKSUM(buf,len) 						\
-			((((unsigned char *)(buf))[(len)-2]<<8) |	\
+			((((unsigned char *)(buf))[(len)-2] << 8) |	\
 				(((unsigned char *)(buf))[(len-1)]))
 
 static int
@@ -2171,7 +2142,7 @@
 
 	HPRINTK("tasklet (0x%lx)\n", data);
 #ifdef USE_TASKLET
-	HE_SPIN_LOCK(he_dev, flags);
+	spin_lock_irqsave(&he_dev->global_lock, flags);
 #endif
 
 	while (he_dev->irq_head != he_dev->irq_tail) {
@@ -2209,10 +2180,10 @@
 			case ITYPE_PHY:
 				HPRINTK("phy interrupt\n");
 #ifdef CONFIG_ATM_HE_USE_SUNI
-				HE_SPIN_UNLOCK(he_dev, flags);
+				spin_unlock_irqrestore(&he_dev->global_lock, flags);
 				if (he_dev->atm_dev->phy && he_dev->atm_dev->phy->interrupt)
 					he_dev->atm_dev->phy->interrupt(he_dev->atm_dev);
-				HE_SPIN_LOCK(he_dev, flags);
+				spin_lock_irqsave(&he_dev->global_lock, flags);
 #endif
 				break;
 			case ITYPE_OTHER:
@@ -2257,7 +2228,7 @@
 		(void) he_readl(he_dev, INT_FIFO); /* 8.1.2 controller errata */
 	}
 #ifdef USE_TASKLET
-	HE_SPIN_UNLOCK(he_dev, flags);
+	spin_unlock_irqrestore(&he_dev->global_lock, flags);
 #endif
 }
 
@@ -2271,7 +2242,7 @@
 	if (he_dev == NULL)
 		return IRQ_NONE;
 
-	HE_SPIN_LOCK(he_dev, flags);
+	spin_lock_irqsave(&he_dev->global_lock, flags);
 
 	he_dev->irq_tail = (struct he_irq *) (((unsigned long)he_dev->irq_base) |
 						(*he_dev->irq_tailoffset << 2));
@@ -2301,7 +2272,7 @@
 		(void) he_readl(he_dev, INT_FIFO);
 #endif
 	}
-	HE_SPIN_UNLOCK(he_dev, flags);
+	spin_unlock_irqrestore(&he_dev->global_lock, flags);
 	return IRQ_RETVAL(handled);
 
 }
@@ -2438,9 +2409,9 @@
 				goto open_failed;
 		}
 
-		HE_SPIN_LOCK(he_dev, flags);
+		spin_lock_irqsave(&he_dev->global_lock, flags);
 		tsr0 = he_readl_tsr0(he_dev, cid);
-		HE_SPIN_UNLOCK(he_dev, flags);
+		spin_unlock_irqrestore(&he_dev->global_lock, flags);
 
 		if (TSR0_CONN_STATE(tsr0) != 0) {
 			hprintk("cid 0x%x not idle (tsr0 = 0x%x)\n", cid, tsr0);
@@ -2467,7 +2438,7 @@
 					goto open_failed;
 				}
 
-				HE_SPIN_LOCK(he_dev, flags);			/* also protects he_dev->cs_stper[] */
+				spin_lock_irqsave(&he_dev->global_lock, flags);			/* also protects he_dev->cs_stper[] */
 
 				/* find an unused cs_stper register */
 				for (reg = 0; reg < HE_NUM_CS_STPER; ++reg)
@@ -2477,7 +2448,7 @@
 
 				if (reg == HE_NUM_CS_STPER) {
 					err = -EBUSY;
-					HE_SPIN_UNLOCK(he_dev, flags);
+					spin_unlock_irqrestore(&he_dev->global_lock, flags);
 					goto open_failed;
 				}
 
@@ -2495,7 +2466,7 @@
 
 				he_writel_mbox(he_dev, rate_to_atmf(period/2),
 							CS_STPER0 + reg);
-				HE_SPIN_UNLOCK(he_dev, flags);
+				spin_unlock_irqrestore(&he_dev->global_lock, flags);
 
 				tsr0 = TSR0_CBR | TSR0_GROUP(0) | tsr0_aal |
 							TSR0_RC_INDEX(reg);
@@ -2506,7 +2477,7 @@
 				goto open_failed;
 		}
 
-		HE_SPIN_LOCK(he_dev, flags);
+		spin_lock_irqsave(&he_dev->global_lock, flags);
 
 		he_writel_tsr0(he_dev, tsr0, cid);
 		he_writel_tsr4(he_dev, tsr4 | 1, cid);
@@ -2528,7 +2499,7 @@
 #ifdef CONFIG_IA64_SGI_SN2
 		(void) he_readl_tsr0(he_dev, cid);
 #endif
-		HE_SPIN_UNLOCK(he_dev, flags);
+		spin_unlock_irqrestore(&he_dev->global_lock, flags);
 	}
 
 	if (vcc->qos.rxtp.traffic_class != ATM_NONE) {
@@ -2549,11 +2520,11 @@
 				goto open_failed;
 		}
 
-		HE_SPIN_LOCK(he_dev, flags);
+		spin_lock_irqsave(&he_dev->global_lock, flags);
 
 		rsr0 = he_readl_rsr0(he_dev, cid);
 		if (rsr0 & RSR0_OPEN_CONN) {
-			HE_SPIN_UNLOCK(he_dev, flags);
+			spin_unlock_irqrestore(&he_dev->global_lock, flags);
 
 			hprintk("cid 0x%x not idle (rsr0 = 0x%x)\n", cid, rsr0);
 			err = -EBUSY;
@@ -2585,7 +2556,7 @@
 		(void) he_readl_rsr0(he_dev, cid);
 #endif
 
-		HE_SPIN_UNLOCK(he_dev, flags);
+		spin_unlock_irqrestore(&he_dev->global_lock, flags);
 
 #ifndef USE_HE_FIND_VCC
 		HE_LOOKUP_VCC(he_dev, cid) = vcc;
@@ -2631,7 +2602,7 @@
 
 		/* wait for previous close (if any) to finish */
 
-		HE_SPIN_LOCK(he_dev, flags);
+		spin_lock_irqsave(&he_dev->global_lock, flags);
 		while (he_readl(he_dev, RCC_STAT) & RCC_BUSY) {
 			HPRINTK("close cid 0x%x RCC_BUSY\n", cid);
 			udelay(250);
@@ -2645,7 +2616,7 @@
 		(void) he_readl_rsr0(he_dev, cid);
 #endif
 		he_writel_mbox(he_dev, cid, RXCON_CLOSE);
-		HE_SPIN_UNLOCK(he_dev, flags);
+		spin_unlock_irqrestore(&he_dev->global_lock, flags);
 
 		timeout = schedule_timeout(30*HZ);
 
@@ -2693,7 +2664,7 @@
 
 		/* 2.3.1.1 generic close operations with flush */
 
-		HE_SPIN_LOCK(he_dev, flags);
+		spin_lock_irqsave(&he_dev->global_lock, flags);
 		he_writel_tsr4_upper(he_dev, TSR4_FLUSH_CONN, cid);
 					/* also clears TSR4_SESSION_ENDED */
 #ifdef CONFIG_IA64_SGI_SN2
@@ -2724,7 +2695,7 @@
 		add_wait_queue(&he_vcc->tx_waitq, &wait);
 		set_current_state(TASK_UNINTERRUPTIBLE);
 		__enqueue_tpd(he_dev, tpd, cid);
-		HE_SPIN_UNLOCK(he_dev, flags);
+		spin_unlock_irqrestore(&he_dev->global_lock, flags);
 
 		timeout = schedule_timeout(30*HZ);
 
@@ -2736,7 +2707,7 @@
 			goto close_tx_incomplete;
 		}
 
-		HE_SPIN_LOCK(he_dev, flags);
+		spin_lock_irqsave(&he_dev->global_lock, flags);
 		while (!((tsr4 = he_readl_tsr4(he_dev, cid)) & TSR4_SESSION_ENDED)) {
 			HPRINTK("close tx cid 0x%x !TSR4_SESSION_ENDED (tsr4 = 0x%x)\n", cid, tsr4);
 			udelay(250);
@@ -2761,7 +2732,7 @@
 
 			he_dev->total_bw -= he_dev->cs_stper[reg].pcr;
 		}
-		HE_SPIN_UNLOCK(he_dev, flags);
+		spin_unlock_irqrestore(&he_dev->global_lock, flags);
 
 		HPRINTK("close tx cid 0x%x complete\n", cid);
 	}
@@ -2818,7 +2789,7 @@
 		return -EINVAL;
 	}
 #endif
-	HE_SPIN_LOCK(he_dev, flags);
+	spin_lock_irqsave(&he_dev->global_lock, flags);
 
 	tpd = __alloc_tpd(he_dev);
 	if (tpd == NULL) {
@@ -2827,7 +2798,7 @@
 		else
 			dev_kfree_skb_any(skb);
 		atomic_inc(&vcc->stats->tx_err);
-		HE_SPIN_UNLOCK(he_dev, flags);
+		spin_unlock_irqrestore(&he_dev->global_lock, flags);
 		return -ENOMEM;
 	}
 
@@ -2869,7 +2840,7 @@
 				else
 					dev_kfree_skb_any(skb);
 				atomic_inc(&vcc->stats->tx_err);
-				HE_SPIN_UNLOCK(he_dev, flags);
+				spin_unlock_irqrestore(&he_dev->global_lock, flags);
 				return -ENOMEM;
 			}
 			tpd->status |= TPD_USERCELL;
@@ -2884,7 +2855,7 @@
 
 	}
 
-	tpd->iovec[slot-1].len |= TPD_LST;
+	tpd->iovec[slot - 1].len |= TPD_LST;
 #else
 	tpd->address0 = pci_map_single(he_dev->pci_dev, skb->data, skb->len, PCI_DMA_TODEVICE);
 	tpd->length0 = skb->len | TPD_LST;
@@ -2897,7 +2868,7 @@
 	ATM_SKB(skb)->vcc = vcc;
 
 	__enqueue_tpd(he_dev, tpd, cid);
-	HE_SPIN_UNLOCK(he_dev, flags);
+	spin_unlock_irqrestore(&he_dev->global_lock, flags);
 
 	atomic_inc(&vcc->stats->tx);
 
@@ -2919,7 +2890,7 @@
 
 			copy_from_user(&reg, (struct he_ioctl_reg *) arg,
 						sizeof(struct he_ioctl_reg));
-			HE_SPIN_LOCK(he_dev, flags);
+			spin_lock_irqsave(&he_dev->global_lock, flags);
 			switch (reg.type) {
 				case HE_REGTYPE_PCI:
 					reg.val = he_readl(he_dev, reg.addr);
@@ -2940,7 +2911,7 @@
 					err = -EINVAL;
 					break;
 			}
-			HE_SPIN_UNLOCK(he_dev, flags);
+			spin_unlock_irqrestore(&he_dev->global_lock, flags);
 			if (err == 0)
 				copy_to_user((struct he_ioctl_reg *) arg, &reg,
 							sizeof(struct he_ioctl_reg));
@@ -2966,12 +2937,12 @@
 
 	HPRINTK("phy_put(val 0x%x, addr 0x%lx)\n", val, addr);
 
-	HE_SPIN_LOCK(he_dev, flags);
+	spin_lock_irqsave(&he_dev->global_lock, flags);
         he_writel(he_dev, val, FRAMER + (addr*4));
 #ifdef CONFIG_IA64_SGI_SN2
 	(void) he_readl(he_dev, FRAMER + (addr*4));
 #endif
-	HE_SPIN_UNLOCK(he_dev, flags);
+	spin_unlock_irqrestore(&he_dev->global_lock, flags);
 }
  
         
@@ -2982,9 +2953,9 @@
 	struct he_dev *he_dev = HE_DEV(atm_dev);
 	unsigned reg;
 
-	HE_SPIN_LOCK(he_dev, flags);
+	spin_lock_irqsave(&he_dev->global_lock, flags);
         reg = he_readl(he_dev, FRAMER + (addr*4));
-	HE_SPIN_UNLOCK(he_dev, flags);
+	spin_unlock_irqrestore(&he_dev->global_lock, flags);
 
 	HPRINTK("phy_get(addr 0x%lx) =0x%x\n", addr, reg);
 	return reg;
@@ -3015,12 +2986,12 @@
 	if (!left--)
 		return sprintf(page, "Mismatched Cells  VPI/VCI Not Open  Dropped Cells  RCM Dropped Cells\n");
 
-	HE_SPIN_LOCK(he_dev, flags);
+	spin_lock_irqsave(&he_dev->global_lock, flags);
 	mcc += he_readl(he_dev, MCC);
 	oec += he_readl(he_dev, OEC);
 	dcc += he_readl(he_dev, DCC);
 	cec += he_readl(he_dev, CEC);
-	HE_SPIN_UNLOCK(he_dev, flags);
+	spin_unlock_irqrestore(&he_dev->global_lock, flags);
 
 	if (!left--)
 		return sprintf(page, "%16ld  %16ld  %13ld  %17ld\n\n", 
@@ -3090,26 +3061,26 @@
 	he_writel(he_dev, val, HOST_CNTL);
        
 	/* Send READ instruction */
-	for (i=0; i<sizeof(readtab)/sizeof(readtab[0]); i++) {
+	for (i = 0; i < sizeof(readtab)/sizeof(readtab[0]); i++) {
 		he_writel(he_dev, val | readtab[i], HOST_CNTL);
 		udelay(EEPROM_DELAY);
 	}
        
         /* Next, we need to send the byte address to read from */
-	for (i=7; i>=0; i--) {
+	for (i = 7; i >= 0; i--) {
 		he_writel(he_dev, val | clocktab[j++] | (((addr >> i) & 1) << 9), HOST_CNTL);
 		udelay(EEPROM_DELAY);
 		he_writel(he_dev, val | clocktab[j++] | (((addr >> i) & 1) << 9), HOST_CNTL);
 		udelay(EEPROM_DELAY);
 	}
        
-	j=0;
+	j = 0;
 
 	val &= 0xFFFFF7FF;      /* Turn off write enable */
 	he_writel(he_dev, val, HOST_CNTL);
        
 	/* Now, we can read data from the EEPROM by clocking it in */
-	for (i=7; i>=0; i--) {
+	for (i = 7; i >= 0; i--) {
 		he_writel(he_dev, val | clocktab[j++], HOST_CNTL);
 	        udelay(EEPROM_DELAY);
 	        tmp_read = he_readl(he_dev, HOST_CNTL);

Re: [PATCH][ATM] assorted he driver cleanup

[chas williams]

>[ATM]: more coding style cleanups in HE driver

ignore this one... it undoes the rategrid fixes.

Re: [PATCH][ATM] assorted he driver cleanup

[David S. Miller]

   From: chas williams <chas@cmf.nrl.navy.mil>
   Date: Fri, 30 May 2003 10:29:58 -0400

   >ignore this one... it undoes the rategrid fixes.
   
   ok, this one should be right.  sorry about the previous nonsense.
   this ones a bit larger since i fixed some whitespace issues.
   
   [ATM]: more cleanup in HE driver
   
Applied, thanks.

Re: [PATCH][ATM] assorted he driver cleanup

[Francois Romieu]

An unconditional HE_SPIN_UNLOCK(he_dev, flags); stands behind the
'close_tx_incomplete' label in he_close(). The following patch should cure
a possible unlock of a non-locked lock (courtesy of kbugs.org, see
http://kbugs.org/cgi-bin/index.py?page=source&version=2.5.70&file=drivers/atm/he.c#line2840).

--- linux-2.5.70-1.1229.7.33-to-1.1330/drivers/atm/he.c	Mon Jun  2 00:33:38 2003
+++ linux-2.5.70-1.1229.7.33-to-1.1330/drivers/atm/he.c	Mon Jun  2 00:34:29 2003
@@ -2731,12 +2731,13 @@ he_close(struct atm_vcc *vcc)
 		remove_wait_queue(&he_vcc->tx_waitq, &wait);
 		set_current_state(TASK_RUNNING);
 
+		HE_SPIN_LOCK(he_dev, flags);
+
 		if (timeout == 0) {
 			hprintk("close tx timeout cid 0x%x\n", cid);
 			goto close_tx_incomplete;
 		}
 
-		HE_SPIN_LOCK(he_dev, flags);
 		while (!((tsr4 = he_readl_tsr4(he_dev, cid)) & TSR4_SESSION_ENDED)) {
 			HPRINTK("close tx cid 0x%x !TSR4_SESSION_ENDED (tsr4 = 0x%x)\n", cid, tsr4);
 			udelay(250);

Re: [PATCH][ATM] assorted he driver cleanup

[chas williams]

In message <20030602004232.A25795@electric-eye.fr.zoreil.com>,Francois Romieu w
rites:
>An unconditional HE_SPIN_UNLOCK(he_dev, flags); stands behind the
>'close_tx_incomplete' label in he_close(). The following patch should cure
>a possible unlock of a non-locked lock (courtesy of kbugs.org, see
>http://kbugs.org/cgi-bin/index.py?page=source&version=2.5.70&file=drivers/atm/
>he.c#line2840).

dave, please apply the following patch:

[atm]: he: cure possible unlock of a non-locked lock

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.1333  -> 1.1334 
#	    drivers/atm/he.c	1.15    -> 1.16   
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 03/06/18	chas@relax.cmf.nrl.navy.mil	1.1334
# fix possible unlock of a non-locked lock
# --------------------------------------------
#
diff -Nru a/drivers/atm/he.c b/drivers/atm/he.c
--- a/drivers/atm/he.c	Wed Jun 18 17:07:21 2003
+++ b/drivers/atm/he.c	Wed Jun 18 17:07:21 2003
@@ -2685,12 +2685,13 @@
 		remove_wait_queue(&he_vcc->tx_waitq, &wait);
 		set_current_state(TASK_RUNNING);
 
+		spin_lock_irqsave(&he_dev->global_lock, flags);
+
 		if (timeout == 0) {
 			hprintk("close tx timeout cid 0x%x\n", cid);
 			goto close_tx_incomplete;
 		}
 
-		spin_lock_irqsave(&he_dev->global_lock, flags);
 		while (!((tsr4 = he_readl_tsr4(he_dev, cid)) & TSR4_SESSION_ENDED)) {
 			HPRINTK("close tx cid 0x%x !TSR4_SESSION_ENDED (tsr4 = 0x%x)\n", cid, tsr4);
 			udelay(250);

Re: [PATCH][ATM] assorted he driver cleanup

[David S. Miller]

   From: chas williams <chas@cmf.nrl.navy.mil>
   Date: Wed, 18 Jun 2003 17:08:10 -0400
   
   dave, please apply the following patch:

Done, thanks.