linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Regression - 2.6.24-rc3 - umem nvram card driver oops
@ 2007-12-03 21:35 David Chinner
  2007-12-04  0:14 ` Neil Brown
  0 siblings, 1 reply; 8+ messages in thread
From: David Chinner @ 2007-12-03 21:35 UTC (permalink / raw)
  To: lkml; +Cc: neilb

Neil,

I just upgraded an ia64 (Altix, 16k page size) test box to 2.6.24-rc3
from 2.6.23 and I get it panicing on boot in the umem driver.

[   55.499300] v2.3 : Micro Memory(tm) PCI memory board block driver
[   55.519331] ACPI: Unable to derive IRQ for device 0006:00:01.0
[   55.528294] ACPI: PCI Interrupt 0006:00:01.0[A]: no GSI
[   55.529214] umem 0006:00:01.0: Micro Memory(tm) controller found (PCI Mem Module (Battery Backup))
[   55.530968] umem 0006:00:01.0: CSR 0xc00000080da00000 -> 0xc00000080da00000 (0x100)
[   55.552881] umem 0006:00:01.0: Size 1048576 KB, Battery 1 Disabled (FAILURE), Battery 2 Disabled (FAILURE)
[   55.559064] umem 0006:00:01.0: Window size 16777216 bytes, IRQ 64
[   55.560131] umem 0006:00:01.0: memory already initialized
[   55.561501]  umema:<1>Unable to handle kernel NULL pointer dereference (address 000000000000002a)
[   55.580231] swapper[0]: Oops 8813272891392 [1]
[   55.581096] Modules linked in: umem qla2xxx
[   55.582022]
[   55.582023] Pid: 0, CPU 0, comm:              swapper
[   55.608663] psr : 0000101008026018 ifs : 8000000000000b9c ip  : [<a0000002046e1ce0>]    Not tainted
[   55.610226] ip is at process_page+0x1c0/0x760 [umem]
[   55.611107] unat: 0000000000000000 pfs : 0000000000000b9c rsc : 0000000000000003
[   55.660528] rnat: e0000030023e5e40 bsps: e000003002563000 pr  : a56911155aa696a5
[   55.661866] ldrs: 0000000000000000 ccv : 0000000000000000 fpsr: 0009804c0270033f
[   55.663204] csd : 0000000000000000 ssd : 0000000000000000
[   55.712930] b0  : a0000002046e1b70 b6  : a0000002046e1b20 b7  : a00000010008d300
[   55.714267] f6  : 1003e0000060100000000 f7  : 1003e0000000000400000
[   55.715395] f8  : 1003e0000000000180400 f9  : 1003e0000000080000000
[   55.816292] f10 : 1003e0000000000000400 f11 : 1003e0000000000049919
[   55.817422] r1  : a0000002046ebe80 r2  : a0000002046f7640 r3  : 0000000000000040
[   55.818747] r8  : 0000000000000000 r9  : 0000000000000000 r10 : 0000000000000000
[   55.866550] r11 : 0000000000000003 r12 : a000000100bfbbb0 r13 : a000000100bf4000
[   55.867886] r14 : 000000000000002a r15 : 0000000000000000 r16 : 0000000000000003
[   55.894776] r17 : a0000002046e5630 r18 : 0000000000000000 r19 : 0000000000000003
[   55.896113] r20 : e00000398c980030 r21 : e000003988381f40 r22 : 0000000000000000
[   55.897450] r23 : 0000000000000001 r24 : 0000000000000001 r25 : e000003988381f28
[   55.927571] r26 : 0000000000000001 r27 : 0000000000000000 r28 : 0000000000040000
[   55.928734] r29 : 00000000cf0c01d0 r30 : e00000398c980038 r31 : 0000000000000000
[   55.930075]
[   55.930077] Call Trace:
[   56.031008]  [<a000000100015e00>] show_stack+0x80/0xa0
[   56.031010]                                 sp=a000000100bfb780 bsp=a000000100bf5238
[   56.033460]  [<a0000001000166f0>] show_regs+0x870/0x8a0
[   56.033462]                                 sp=a000000100bfb950 bsp=a000000100bf51d8
[   56.093627]  [<a00000010003e830>] die+0x210/0x3a0
[   56.093629]                                 sp=a000000100bfb950 bsp=a000000100bf5190
[   56.213913]  [<a00000010006eca0>] ia64_do_page_fault+0x9c0/0xb00
[   56.213915]                                 sp=a000000100bfb950 bsp=a000000100bf5138
[   56.231427]  [<a00000010000b640>] ia64_leave_kernel+0x0/0x280
[   56.231429]                                 sp=a000000100bfb9e0 bsp=a000000100bf5138
[   56.278001]  [<a0000002046e1ce0>] process_page+0x1c0/0x760 [umem]
[   56.278004]                                 sp=a000000100bfbbb0 bsp=a000000100bf5058
[   56.280476]  [<a0000001000dab30>] tasklet_action+0x270/0x360
[   56.280478]                                 sp=a000000100bfbbb0 bsp=a000000100bf5018
[   56.370160]  [<a0000001000d9a60>] __do_softirq+0x200/0x240
[   56.370162]                                 sp=a000000100bfbbb0 bsp=a000000100bf4f80
[   56.476607]  [<a0000001000d9b10>] do_softirq+0x70/0xc0
[   56.476609]                                 sp=a000000100bfbbb0 bsp=a000000100bf4f20
[   56.478889]  [<a0000001000d9be0>] irq_exit+0x80/0xc0
[   56.478891]                                 sp=a000000100bfbbb0 bsp=a000000100bf4f08
[   56.529780]  [<a000000100012b90>] ia64_handle_irq+0x1b0/0x3c0
[   56.529782]                                 sp=a000000100bfbbb0 bsp=a000000100bf4e98
[   56.636341]  [<a00000010000b640>] ia64_leave_kernel+0x0/0x280
[   56.636343]                                 sp=a000000100bfbbb0 bsp=a000000100bf4e98
[   56.638823]  [<a000000100017140>] default_idle+0x1a0/0x1c0
[   56.638825]                                 sp=a000000100bfbd80 bsp=a000000100bf4e30
[   56.689924]  [<a000000100015a70>] cpu_idle+0x210/0x440
[   56.689926]                                 sp=a000000100bfbe20 bsp=a000000100bf4db8
[   56.796362]  [<a000000100928850>] rest_init+0x110/0x140
[   56.796364]                                 sp=a000000100bfbe20 bsp=a000000100bf4da0
[   56.798668]  [<a000000100b49d20>] start_kernel+0x7c0/0x880
[   56.798670]                                 sp=a000000100bfbe20 bsp=a000000100bf4d28
[   56.850239]  [<a000000100930670>] __kprobes_text_end+0x6d0/0x6f0
[   56.850241]                                 sp=a000000100bfbe30 bsp=a000000100bf4c40

-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group

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

* Re: Regression - 2.6.24-rc3 - umem nvram card driver oops
  2007-12-03 21:35 Regression - 2.6.24-rc3 - umem nvram card driver oops David Chinner
@ 2007-12-04  0:14 ` Neil Brown
  2007-12-04  2:20   ` David Chinner
  2007-12-04  9:37   ` Ingo Molnar
  0 siblings, 2 replies; 8+ messages in thread
From: Neil Brown @ 2007-12-04  0:14 UTC (permalink / raw)
  To: David Chinner; +Cc: lkml

On Tuesday December 4, dgc@sgi.com wrote:
> Neil,
> 
> I just upgraded an ia64 (Altix, 16k page size) test box to 2.6.24-rc3
> from 2.6.23 and I get it panicing on boot in the umem driver.

Cool - someone is using umem!  And even testing it.  Thanks!

A quick look shows a probable NULL deref.  Let me know if this fixes
it.  I'll read through the offending patch more carefully and make
sure there is nothing else wrong.

NeilBrown


Fix possible NULL dereference in umem.c

Signed-off-by: Neil Brown <neilb@suse.de>

### Diffstat output
 ./drivers/block/umem.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff .prev/drivers/block/umem.c ./drivers/block/umem.c
--- .prev/drivers/block/umem.c	2007-12-04 11:11:30.000000000 +1100
+++ ./drivers/block/umem.c	2007-12-04 11:11:42.000000000 +1100
@@ -484,7 +484,8 @@ static void process_page(unsigned long d
 		page->idx++;
 		if (page->idx >= bio->bi_vcnt) {
 			page->bio = bio->bi_next;
-			page->idx = page->bio->bi_idx;
+			if (page->bio)
+				page->idx = page->bio->bi_idx;
 		}
 
 		pci_unmap_page(card->dev, desc->data_dma_handle,



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

* Re: Regression - 2.6.24-rc3 - umem nvram card driver oops
  2007-12-04  0:14 ` Neil Brown
@ 2007-12-04  2:20   ` David Chinner
  2007-12-04  2:48     ` Neil Brown
  2007-12-04  9:37   ` Ingo Molnar
  1 sibling, 1 reply; 8+ messages in thread
From: David Chinner @ 2007-12-04  2:20 UTC (permalink / raw)
  To: Neil Brown; +Cc: David Chinner, lkml

On Tue, Dec 04, 2007 at 11:14:12AM +1100, Neil Brown wrote:
> On Tuesday December 4, dgc@sgi.com wrote:
> > Neil,
> > 
> > I just upgraded an ia64 (Altix, 16k page size) test box to 2.6.24-rc3
> > from 2.6.23 and I get it panicing on boot in the umem driver.
> 
> Cool - someone is using umem!  And even testing it.  Thanks!
> 
> A quick look shows a probable NULL deref.  Let me know if this fixes
> it.  I'll read through the offending patch more carefully and make
> sure there is nothing else wrong.

Yeah, that appears to fix the problem.

Tested-by: Dave Chinner <dgc@sgi.com>

Cheers,

Dave.
-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group

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

* Re: Regression - 2.6.24-rc3 - umem nvram card driver oops
  2007-12-04  2:20   ` David Chinner
@ 2007-12-04  2:48     ` Neil Brown
  0 siblings, 0 replies; 8+ messages in thread
From: Neil Brown @ 2007-12-04  2:48 UTC (permalink / raw)
  To: David Chinner; +Cc: lkml

On Tuesday December 4, dgc@sgi.com wrote:
> On Tue, Dec 04, 2007 at 11:14:12AM +1100, Neil Brown wrote:
> > On Tuesday December 4, dgc@sgi.com wrote:
> > > Neil,
> > > 
> > > I just upgraded an ia64 (Altix, 16k page size) test box to 2.6.24-rc3
> > > from 2.6.23 and I get it panicing on boot in the umem driver.
> > 
> > Cool - someone is using umem!  And even testing it.  Thanks!
> > 
> > A quick look shows a probable NULL deref.  Let me know if this fixes
> > it.  I'll read through the offending patch more carefully and make
> > sure there is nothing else wrong.
> 
> Yeah, that appears to fix the problem.
> 
> Tested-by: Dave Chinner <dgc@sgi.com>

Thanks. 
I couldn't find any other issues in the code.

NeilBrown

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

* Re: Regression - 2.6.24-rc3 - umem nvram card driver oops
  2007-12-04  0:14 ` Neil Brown
  2007-12-04  2:20   ` David Chinner
@ 2007-12-04  9:37   ` Ingo Molnar
  2007-12-16 21:15     ` [PATCH] umem nvram driver: clean up style Randy Dunlap
  1 sibling, 1 reply; 8+ messages in thread
From: Ingo Molnar @ 2007-12-04  9:37 UTC (permalink / raw)
  To: Neil Brown; +Cc: David Chinner, lkml


* Neil Brown <neilb@suse.de> wrote:

> ### Diffstat output
>  ./drivers/block/umem.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff .prev/drivers/block/umem.c ./drivers/block/umem.c
> --- .prev/drivers/block/umem.c	2007-12-04 11:11:30.000000000 +1100
> +++ ./drivers/block/umem.c	2007-12-04 11:11:42.000000000 +1100
> @@ -484,7 +484,8 @@ static void process_page(unsigned long d
>  		page->idx++;
>  		if (page->idx >= bio->bi_vcnt) {
>  			page->bio = bio->bi_next;
> -			page->idx = page->bio->bi_idx;
> +			if (page->bio)
> +				page->idx = page->bio->bi_idx;
>  		}
>  
>  		pci_unmap_page(card->dev, desc->data_dma_handle,

btw., if anyone feels so inclined, this file has quite a number of 
coding style issues, as per scripts/checkpatch.pl output:

 total: 28 errors, 54 warnings, 1221 lines checked

	Ingo

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

* [PATCH] umem nvram driver: clean up style
  2007-12-04  9:37   ` Ingo Molnar
@ 2007-12-16 21:15     ` Randy Dunlap
  2007-12-16 21:55       ` Alexey Dobriyan
  0 siblings, 1 reply; 8+ messages in thread
From: Randy Dunlap @ 2007-12-16 21:15 UTC (permalink / raw)
  To: Ingo Molnar, akpm; +Cc: Neil Brown, David Chinner, lkml

On Tue, 4 Dec 2007 10:37:16 +0100 Ingo Molnar wrote:

> btw., if anyone feels so inclined, this file has quite a number of 
> coding style issues, as per scripts/checkpatch.pl output:
> 
>  total: 28 errors, 54 warnings, 1221 lines checked

From: Randy Dunlap <randy.dunlap@oracle.com>

Cleanup umem driver: fix all checkpatch warnings, conform to kernel
coding style.

  linux-2.6.24-rc5-git3> checkpatch.pl-next  patches/block-umem-ckpatch.patch
  total: 0 errors, 0 warnings, 546 lines checked

  patches/block-umem-ckpatch.patch has no obvious style problems and is ready for submission.

Only change in generated object file is due to not initializing a
static global variable to 0.

Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
---
 drivers/block/umem.c |  249 ++++++++++++++++++-----------------------
 1 file changed, 112 insertions(+), 137 deletions(-)

--- linux-2.6.24-rc5-git3.orig/drivers/block/umem.c
+++ linux-2.6.24-rc5-git3/drivers/block/umem.c
@@ -34,7 +34,7 @@
  *			 - set initialised bit then.
  */
 
-//#define DEBUG /* uncomment if you want debugging info (pr_debug) */
+#undef DEBUG	/* #define DEBUG if you want debugging info (pr_debug) */
 #include <linux/fs.h>
 #include <linux/bio.h>
 #include <linux/kernel.h>
@@ -143,17 +143,15 @@ static struct cardinfo cards[MM_MAXCARDS
 static struct block_device_operations mm_fops;
 static struct timer_list battery_timer;
 
-static int num_cards = 0;
+static int num_cards;
 
 static struct gendisk *mm_gendisk[MM_MAXCARDS];
 
 static void check_batteries(struct cardinfo *card);
 
 /*
------------------------------------------------------------------------------------
---                           get_userbit
------------------------------------------------------------------------------------
-*/
+ *                           get_userbit
+ */
 static int get_userbit(struct cardinfo *card, int bit)
 {
 	unsigned char led;
@@ -162,10 +160,8 @@ static int get_userbit(struct cardinfo *
 	return led & bit;
 }
 /*
------------------------------------------------------------------------------------
---                            set_userbit
------------------------------------------------------------------------------------
-*/
+ *                            set_userbit
+ */
 static int set_userbit(struct cardinfo *card, int bit, unsigned char state)
 {
 	unsigned char led;
@@ -180,10 +176,8 @@ static int set_userbit(struct cardinfo *
 	return 0;
 }
 /*
------------------------------------------------------------------------------------
---                             set_led
------------------------------------------------------------------------------------
-*/
+ *                             set_led
+ */
 /*
  * NOTE: For the power LED, use the LED_POWER_* macros since they differ
  */
@@ -204,10 +198,8 @@ static void set_led(struct cardinfo *car
 
 #ifdef MM_DIAG
 /*
------------------------------------------------------------------------------------
---                              dump_regs
------------------------------------------------------------------------------------
-*/
+ *                              dump_regs
+ */
 static void dump_regs(struct cardinfo *card)
 {
 	unsigned char *p;
@@ -225,31 +217,29 @@ static void dump_regs(struct cardinfo *c
 }
 #endif
 /*
------------------------------------------------------------------------------------
---                            dump_dmastat
------------------------------------------------------------------------------------
-*/
+ *                            dump_dmastat
+ */
 static void dump_dmastat(struct cardinfo *card, unsigned int dmastat)
 {
 	dev_printk(KERN_DEBUG, &card->dev->dev, "DMAstat - ");
 	if (dmastat & DMASCR_ANY_ERR)
-		printk("ANY_ERR ");
+		printk(KERN_CONT "ANY_ERR ");
 	if (dmastat & DMASCR_MBE_ERR)
-		printk("MBE_ERR ");
+		printk(KERN_CONT "MBE_ERR ");
 	if (dmastat & DMASCR_PARITY_ERR_REP)
-		printk("PARITY_ERR_REP ");
+		printk(KERN_CONT "PARITY_ERR_REP ");
 	if (dmastat & DMASCR_PARITY_ERR_DET)
-		printk("PARITY_ERR_DET ");
+		printk(KERN_CONT "PARITY_ERR_DET ");
 	if (dmastat & DMASCR_SYSTEM_ERR_SIG)
-		printk("SYSTEM_ERR_SIG ");
+		printk(KERN_CONT "SYSTEM_ERR_SIG ");
 	if (dmastat & DMASCR_TARGET_ABT)
-		printk("TARGET_ABT ");
+		printk(KERN_CONT "TARGET_ABT ");
 	if (dmastat & DMASCR_MASTER_ABT)
-		printk("MASTER_ABT ");
+		printk(KERN_CONT "MASTER_ABT ");
 	if (dmastat & DMASCR_CHAIN_COMPLETE)
-		printk("CHAIN_COMPLETE ");
+		printk(KERN_CONT "CHAIN_COMPLETE ");
 	if (dmastat & DMASCR_DMA_COMPLETE)
-		printk("DMA_COMPLETE ");
+		printk(KERN_CONT "DMA_COMPLETE ");
 	printk("\n");
 }
 
@@ -286,7 +276,8 @@ static void mm_start_io(struct cardinfo 
 
 	/* make the last descriptor end the chain */
 	page = &card->mm_pages[card->Active];
-	pr_debug("start_io: %d %d->%d\n", card->Active, page->headcnt, page->cnt-1);
+	pr_debug("start_io: %d %d->%d\n",
+		card->Active, page->headcnt, page->cnt - 1);
 	desc = &page->desc[page->cnt-1];
 
 	desc->control_bits |= cpu_to_le32(DMASCR_CHAIN_COMP_EN);
@@ -310,8 +301,8 @@ static void mm_start_io(struct cardinfo 
 	writel(0, card->csr_remap + DMA_SEMAPHORE_ADDR);
 	writel(0, card->csr_remap + DMA_SEMAPHORE_ADDR + 4);
 
-	offset = ((char*)desc) - ((char*)page->desc);
-	writel(cpu_to_le32((page->page_dma+offset)&0xffffffff),
+	offset = ((char *)desc) - ((char *)page->desc);
+	writel(cpu_to_le32((page->page_dma+offset) & 0xffffffff),
 	       card->csr_remap + DMA_DESCRIPTOR_ADDR);
 	/* Force the value to u64 before shifting otherwise >> 32 is undefined C
 	 * and on some ports will do nothing ! */
@@ -352,7 +343,7 @@ static inline void reset_page(struct mm_
 	page->cnt = 0;
 	page->headcnt = 0;
 	page->bio = NULL;
-	page->biotail = & page->bio;
+	page->biotail = &page->bio;
 }
 
 static void mm_unplug_device(struct request_queue *q)
@@ -408,7 +399,7 @@ static int add_bio(struct cardinfo *card
 				  vec->bv_page,
 				  vec->bv_offset,
 				  len,
-				  (rw==READ) ?
+				  (rw == READ) ?
 				  PCI_DMA_FROMDEVICE : PCI_DMA_TODEVICE);
 
 	p = &card->mm_pages[card->Ready];
@@ -427,10 +418,10 @@ static int add_bio(struct cardinfo *card
 	desc->pci_addr = cpu_to_le64((u64)desc->data_dma_handle);
 	desc->local_addr = cpu_to_le64(card->current_sector << 9);
 	desc->transfer_size = cpu_to_le32(len);
-	offset = ( ((char*)&desc->sem_control_bits) - ((char*)p->desc));
+	offset = (((char *)&desc->sem_control_bits) - ((char *)p->desc));
 	desc->sem_addr = cpu_to_le64((u64)(p->page_dma+offset));
 	desc->zero1 = desc->zero2 = 0;
-	offset = ( ((char*)(desc+1)) - ((char*)p->desc));
+	offset = (((char *)(desc+1)) - ((char *)p->desc));
 	desc->next_desc_addr = cpu_to_le64(p->page_dma+offset);
 	desc->control_bits = cpu_to_le32(DMASCR_GO|DMASCR_ERR_INT_EN|
 					 DMASCR_PARITY_INT_EN|
@@ -455,11 +446,11 @@ static void process_page(unsigned long d
 	/* check if any of the requests in the page are DMA_COMPLETE,
 	 * and deal with them appropriately.
 	 * If we find a descriptor without DMA_COMPLETE in the semaphore, then
-	 * dma must have hit an error on that descriptor, so use dma_status instead
-	 * and assume that all following descriptors must be re-tried.
+	 * dma must have hit an error on that descriptor, so use dma_status
+	 * instead and assume that all following descriptors must be re-tried.
 	 */
 	struct mm_page *page;
-	struct bio *return_bio=NULL;
+	struct bio *return_bio = NULL;
 	struct cardinfo *card = (struct cardinfo *)data;
 	unsigned int dma_status = card->dma_status;
 
@@ -472,12 +463,12 @@ static void process_page(unsigned long d
 		struct bio *bio = page->bio;
 		struct mm_dma_desc *desc = &page->desc[page->headcnt];
 		int control = le32_to_cpu(desc->sem_control_bits);
-		int last=0;
+		int last = 0;
 		int idx;
 
 		if (!(control & DMASCR_DMA_COMPLETE)) {
 			control = dma_status;
-			last=1;
+			last = 1;
 		}
 		page->headcnt++;
 		idx = page->idx;
@@ -489,8 +480,8 @@ static void process_page(unsigned long d
 		}
 
 		pci_unmap_page(card->dev, desc->data_dma_handle,
-			       bio_iovec_idx(bio,idx)->bv_len,
-				 (control& DMASCR_TRANSFER_READ) ?
+			       bio_iovec_idx(bio, idx)->bv_len,
+				 (control & DMASCR_TRANSFER_READ) ?
 				PCI_DMA_TODEVICE : PCI_DMA_FROMDEVICE);
 		if (control & DMASCR_HARD_ERROR) {
 			/* error */
@@ -501,9 +492,11 @@ static void process_page(unsigned long d
 				le32_to_cpu(desc->transfer_size));
 			dump_dmastat(card, control);
 		} else if (test_bit(BIO_RW, &bio->bi_rw) &&
-			   le32_to_cpu(desc->local_addr)>>9 == card->init_size) {
-			card->init_size += le32_to_cpu(desc->transfer_size)>>9;
-			if (card->init_size>>1 >= card->mm_size) {
+			   le32_to_cpu(desc->local_addr) >> 9 ==
+				card->init_size) {
+			card->init_size += le32_to_cpu(desc->transfer_size)
+							>> 9;
+			if (card->init_size >> 1 >= card->mm_size) {
 				dev_printk(KERN_INFO, &card->dev->dev,
 					"memory now initialised\n");
 				set_userbit(card, MEMORY_INITIALIZED, 1);
@@ -514,7 +507,8 @@ static void process_page(unsigned long d
 			return_bio = bio;
 		}
 
-		if (last) break;
+		if (last)
+			break;
 	}
 
 	if (debug & DEBUG_LED_ON_TRANSFER)
@@ -536,7 +530,7 @@ static void process_page(unsigned long d
  out_unlock:
 	spin_unlock_bh(&card->lock);
 
-	while(return_bio) {
+	while (return_bio) {
 		struct bio *bio = return_bio;
 
 		return_bio = bio->bi_next;
@@ -546,10 +540,8 @@ static void process_page(unsigned long d
 }
 
 /*
------------------------------------------------------------------------------------
---                              mm_make_request
------------------------------------------------------------------------------------
-*/
+ *                              mm_make_request
+ */
 static int mm_make_request(struct request_queue *q, struct bio *bio)
 {
 	struct cardinfo *card = q->queuedata;
@@ -567,10 +559,8 @@ static int mm_make_request(struct reques
 }
 
 /*
------------------------------------------------------------------------------------
---                              mm_interrupt
------------------------------------------------------------------------------------
-*/
+ *                              mm_interrupt
+ */
 static irqreturn_t mm_interrupt(int irq, void *__card)
 {
 	struct cardinfo *card = (struct cardinfo *) __card;
@@ -584,15 +574,15 @@ HW_TRACE(0x30);
 	if (!(dma_status & (DMASCR_ERROR_MASK | DMASCR_CHAIN_COMPLETE))) {
 		/* interrupt wasn't for me ... */
 		return IRQ_NONE;
-        }
+	}
 
 	/* clear COMPLETION interrupts */
 	if (card->flags & UM_FLAG_NO_BYTE_STATUS)
 		writel(cpu_to_le32(DMASCR_DMA_COMPLETE|DMASCR_CHAIN_COMPLETE),
-		       card->csr_remap+ DMA_STATUS_CTRL);
+		       card->csr_remap + DMA_STATUS_CTRL);
 	else
 		writeb((DMASCR_DMA_COMPLETE|DMASCR_CHAIN_COMPLETE) >> 16,
-		       card->csr_remap+ DMA_STATUS_CTRL + 2);
+		       card->csr_remap + DMA_STATUS_CTRL + 2);
 
 	/* log errors and clear interrupt status */
 	if (dma_status & DMASCR_ANY_ERR) {
@@ -602,9 +592,12 @@ HW_TRACE(0x30);
 
 		stat = readb(card->csr_remap + MEMCTRLCMD_ERRSTATUS);
 
-		data_log1 = le32_to_cpu(readl(card->csr_remap + ERROR_DATA_LOG));
-		data_log2 = le32_to_cpu(readl(card->csr_remap + ERROR_DATA_LOG + 4));
-		addr_log1 = le32_to_cpu(readl(card->csr_remap + ERROR_ADDR_LOG));
+		data_log1 = le32_to_cpu(readl(card->csr_remap +
+						ERROR_DATA_LOG));
+		data_log2 = le32_to_cpu(readl(card->csr_remap +
+						ERROR_DATA_LOG + 4));
+		addr_log1 = le32_to_cpu(readl(card->csr_remap +
+						ERROR_ADDR_LOG));
 		addr_log2 = readb(card->csr_remap + ERROR_ADDR_LOG + 4);
 
 		count = readb(card->csr_remap + ERROR_COUNT);
@@ -672,10 +665,8 @@ HW_TRACE(0x36);
 	return IRQ_HANDLED;
 }
 /*
------------------------------------------------------------------------------------
---                         set_fault_to_battery_status
------------------------------------------------------------------------------------
-*/
+ *                         set_fault_to_battery_status
+ */
 /*
  * If both batteries are good, no LED
  * If either battery has been warned, solid LED
@@ -698,10 +689,8 @@ static void init_battery_timer(void);
 
 
 /*
------------------------------------------------------------------------------------
---                            check_battery
------------------------------------------------------------------------------------
-*/
+ *                            check_battery
+ */
 static int check_battery(struct cardinfo *card, int battery, int status)
 {
 	if (status != card->battery[battery].good) {
@@ -731,10 +720,8 @@ static int check_battery(struct cardinfo
 	return 0;
 }
 /*
------------------------------------------------------------------------------------
---                              check_batteries
------------------------------------------------------------------------------------
-*/
+ *                              check_batteries
+ */
 static void check_batteries(struct cardinfo *card)
 {
 	/* NOTE: this must *never* be called while the card
@@ -776,10 +763,8 @@ static void check_all_batteries(unsigned
 	init_battery_timer();
 }
 /*
------------------------------------------------------------------------------------
---                            init_battery_timer
------------------------------------------------------------------------------------
-*/
+ *                            init_battery_timer
+ */
 static void init_battery_timer(void)
 {
 	init_timer(&battery_timer);
@@ -788,19 +773,15 @@ static void init_battery_timer(void)
 	add_timer(&battery_timer);
 }
 /*
------------------------------------------------------------------------------------
---                              del_battery_timer
------------------------------------------------------------------------------------
-*/
+ *                              del_battery_timer
+ */
 static void del_battery_timer(void)
 {
 	del_timer(&battery_timer);
 }
 /*
------------------------------------------------------------------------------------
---                                mm_revalidate
------------------------------------------------------------------------------------
-*/
+ *                                mm_revalidate
+ */
 /*
  * Note no locks taken out here.  In a worst case scenario, we could drop
  * a chunk of system memory.  But that should never happen, since validation
@@ -833,33 +814,29 @@ static int mm_getgeo(struct block_device
 }
 
 /*
------------------------------------------------------------------------------------
---                                mm_check_change
------------------------------------------------------------------------------------
-  Future support for removable devices
-*/
+ *                                mm_check_change
+ *
+ * Future support for removable devices
+ */
 static int mm_check_change(struct gendisk *disk)
 {
 /*  struct cardinfo *dev = disk->private_data; */
 	return 0;
 }
 /*
------------------------------------------------------------------------------------
---                             mm_fops
------------------------------------------------------------------------------------
-*/
+ *                             mm_fops
+ */
 static struct block_device_operations mm_fops = {
 	.owner		= THIS_MODULE,
 	.getgeo		= mm_getgeo,
-	.revalidate_disk= mm_revalidate,
+	.revalidate_disk = mm_revalidate,
 	.media_changed	= mm_check_change,
 };
 /*
------------------------------------------------------------------------------------
---                                mm_pci_probe
------------------------------------------------------------------------------------
-*/
-static int __devinit mm_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
+ *                                mm_pci_probe
+ */
+static int __devinit mm_pci_probe(struct pci_dev *dev,
+				const struct pci_device_id *id)
 {
 	int ret = -ENODEV;
 	struct cardinfo *card = &cards[num_cards];
@@ -889,7 +866,8 @@ static int __devinit mm_pci_probe(struct
 		return -ENODEV;
 
 	dev_printk(KERN_INFO, &dev->dev,
-		"Micro Memory(tm) controller found (PCI Mem Module (Battery Backup))\n");
+		"Micro Memory(tm) controller found "
+		"(PCI Mem Module (Battery Backup))\n");
 
 	if (pci_set_dma_mask(dev, DMA_64BIT_MASK) &&
 	    pci_set_dma_mask(dev, DMA_32BIT_MASK)) {
@@ -917,7 +895,7 @@ static int __devinit mm_pci_probe(struct
 		"CSR 0x%08lx -> 0x%p (0x%lx)\n",
 	       csr_base, card->csr_remap, csr_len);
 
-	switch(card->dev->device) {
+	switch (card->dev->device) {
 	case 0x5415:
 		card->flags |= UM_FLAG_NO_BYTE_STATUS | UM_FLAG_NO_BATTREG;
 		magic_number = 0x59;
@@ -929,7 +907,8 @@ static int __devinit mm_pci_probe(struct
 		break;
 
 	case 0x6155:
-		card->flags |= UM_FLAG_NO_BYTE_STATUS | UM_FLAG_NO_BATTREG | UM_FLAG_NO_BATT;
+		card->flags |= UM_FLAG_NO_BYTE_STATUS |
+				UM_FLAG_NO_BATTREG | UM_FLAG_NO_BATT;
 		magic_number = 0x99;
 		break;
 
@@ -945,11 +924,11 @@ static int __devinit mm_pci_probe(struct
 	}
 
 	card->mm_pages[0].desc = pci_alloc_consistent(card->dev,
-						      PAGE_SIZE*2,
-						      &card->mm_pages[0].page_dma);
+						PAGE_SIZE * 2,
+						&card->mm_pages[0].page_dma);
 	card->mm_pages[1].desc = pci_alloc_consistent(card->dev,
-						      PAGE_SIZE*2,
-						      &card->mm_pages[1].page_dma);
+						PAGE_SIZE * 2,
+						&card->mm_pages[1].page_dma);
 	if (card->mm_pages[0].desc == NULL ||
 	    card->mm_pages[1].desc == NULL) {
 		dev_printk(KERN_ERR, &card->dev->dev, "alloc failed\n");
@@ -1013,9 +992,11 @@ static int __devinit mm_pci_probe(struct
 		dev_printk(KERN_INFO, &card->dev->dev,
 			"Size %d KB, Battery 1 %s (%s), Battery 2 %s (%s)\n",
 		       card->mm_size,
-		       (batt_status & BATTERY_1_DISABLED ? "Disabled" : "Enabled"),
+		       batt_status & BATTERY_1_DISABLED ? "Disabled"
+					: "Enabled",
 		       card->battery[0].good ? "OK" : "FAILURE",
-		       (batt_status & BATTERY_2_DISABLED ? "Disabled" : "Enabled"),
+		       batt_status & BATTERY_2_DISABLED ? "Disabled"
+					: "Enabled",
 		       card->battery[1].good ? "OK" : "FAILURE");
 
 		set_fault_to_battery_status(card);
@@ -1030,18 +1011,18 @@ static int __devinit mm_pci_probe(struct
 	data = ~data;
 	data += 1;
 
-	if (request_irq(dev->irq, mm_interrupt, IRQF_SHARED, DRIVER_NAME, card)) {
+	if (request_irq(dev->irq, mm_interrupt, IRQF_SHARED, DRIVER_NAME,
+			card)) {
 		dev_printk(KERN_ERR, &card->dev->dev,
 			"Unable to allocate IRQ\n");
 		ret = -ENODEV;
-
 		goto failed_req_irq;
 	}
 
 	dev_printk(KERN_INFO, &card->dev->dev,
 		"Window size %d bytes, IRQ %d\n", data, dev->irq);
 
-        spin_lock_init(&card->lock);
+	spin_lock_init(&card->lock);
 
 	pci_set_drvdata(dev, card);
 
@@ -1060,7 +1041,8 @@ static int __devinit mm_pci_probe(struct
 
 	if (!get_userbit(card, MEMORY_INITIALIZED)) {
 		dev_printk(KERN_INFO, &card->dev->dev,
-			"memory NOT initialized. Consider over-writing whole device.\n");
+			"memory NOT initialized. Consider "
+			"over-writing whole device.\n");
 		card->init_size = 0;
 	} else {
 		dev_printk(KERN_INFO, &card->dev->dev,
@@ -1092,10 +1074,8 @@ static int __devinit mm_pci_probe(struct
 	return ret;
 }
 /*
------------------------------------------------------------------------------------
---                              mm_pci_remove
------------------------------------------------------------------------------------
-*/
+ *                              mm_pci_remove
+ */
 static void mm_pci_remove(struct pci_dev *dev)
 {
 	struct cardinfo *card = pci_get_drvdata(dev);
@@ -1119,16 +1099,16 @@ static void mm_pci_remove(struct pci_dev
 }
 
 static const struct pci_device_id mm_pci_ids[] = {
-    {PCI_DEVICE(PCI_VENDOR_ID_MICRO_MEMORY,PCI_DEVICE_ID_MICRO_MEMORY_5415CN)},
-    {PCI_DEVICE(PCI_VENDOR_ID_MICRO_MEMORY,PCI_DEVICE_ID_MICRO_MEMORY_5425CN)},
-    {PCI_DEVICE(PCI_VENDOR_ID_MICRO_MEMORY,PCI_DEVICE_ID_MICRO_MEMORY_6155)},
+    {PCI_DEVICE(PCI_VENDOR_ID_MICRO_MEMORY, PCI_DEVICE_ID_MICRO_MEMORY_5415CN)},
+    {PCI_DEVICE(PCI_VENDOR_ID_MICRO_MEMORY, PCI_DEVICE_ID_MICRO_MEMORY_5425CN)},
+    {PCI_DEVICE(PCI_VENDOR_ID_MICRO_MEMORY, PCI_DEVICE_ID_MICRO_MEMORY_6155)},
     {
 	.vendor	=	0x8086,
 	.device	=	0xB555,
-	.subvendor=	0x1332,
-	.subdevice=	0x5460,
-	.class	=	0x050000,
-	.class_mask=	0,
+	.subvendor =	0x1332,
+	.subdevice =	0x5460,
+	.class =	0x050000,
+	.class_mask =	0,
     }, { /* end: all zeroes */ }
 };
 
@@ -1142,11 +1122,8 @@ static struct pci_driver mm_pci_driver =
 };
 
 /*
------------------------------------------------------------------------------------
---                               mm_init
------------------------------------------------------------------------------------
-*/
-
+ *                               mm_init
+ */
 static int __init mm_init(void)
 {
 	int retval, i;
@@ -1194,17 +1171,15 @@ out:
 	return -ENOMEM;
 }
 /*
------------------------------------------------------------------------------------
---                             mm_cleanup
------------------------------------------------------------------------------------
-*/
+ *                             mm_cleanup
+ */
 static void __exit mm_cleanup(void)
 {
 	int i;
 
 	del_battery_timer();
 
-	for (i=0; i < num_cards ; i++) {
+	for (i = 0; i < num_cards ; i++) {
 		del_gendisk(mm_gendisk[i]);
 		put_disk(mm_gendisk[i]);
 	}

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

* Re: [PATCH] umem nvram driver: clean up style
  2007-12-16 21:15     ` [PATCH] umem nvram driver: clean up style Randy Dunlap
@ 2007-12-16 21:55       ` Alexey Dobriyan
  2007-12-17  0:49         ` Randy Dunlap
  0 siblings, 1 reply; 8+ messages in thread
From: Alexey Dobriyan @ 2007-12-16 21:55 UTC (permalink / raw)
  To: Randy Dunlap; +Cc: Ingo Molnar, akpm, Neil Brown, David Chinner, lkml

On Sun, Dec 16, 2007 at 01:15:58PM -0800, Randy Dunlap wrote:
> > btw., if anyone feels so inclined, this file has quite a number of 
> > coding style issues, as per scripts/checkpatch.pl output:
> > 
> >  total: 28 errors, 54 warnings, 1221 lines checked

> Cleanup umem driver: fix all checkpatch warnings, conform to kernel
> coding style.

> --- linux-2.6.24-rc5-git3.orig/drivers/block/umem.c
> +++ linux-2.6.24-rc5-git3/drivers/block/umem.c

>  static void check_batteries(struct cardinfo *card);
>  
>  /*
> ------------------------------------------------------------------------------------
> ---                           get_userbit
> ------------------------------------------------------------------------------------
> -*/
> + *                           get_userbit
> + */

useless comments, they aren't becoming useful after prettifying, right. ;-)

>  static void dump_dmastat(struct cardinfo *card, unsigned int dmastat)
>  {
>  	dev_printk(KERN_DEBUG, &card->dev->dev, "DMAstat - ");
>  	if (dmastat & DMASCR_ANY_ERR)
> -		printk("ANY_ERR ");
> +		printk(KERN_CONT "ANY_ERR ");
>  	if (dmastat & DMASCR_MBE_ERR)
> -		printk("MBE_ERR ");
> +		printk(KERN_CONT "MBE_ERR ");
>  	if (dmastat & DMASCR_PARITY_ERR_REP)
> -		printk("PARITY_ERR_REP ");
> +		printk(KERN_CONT "PARITY_ERR_REP ");
>  	if (dmastat & DMASCR_PARITY_ERR_DET)
> -		printk("PARITY_ERR_DET ");
> +		printk(KERN_CONT "PARITY_ERR_DET ");
>  	if (dmastat & DMASCR_SYSTEM_ERR_SIG)
> -		printk("SYSTEM_ERR_SIG ");
> +		printk(KERN_CONT "SYSTEM_ERR_SIG ");
>  	if (dmastat & DMASCR_TARGET_ABT)
> -		printk("TARGET_ABT ");
> +		printk(KERN_CONT "TARGET_ABT ");
>  	if (dmastat & DMASCR_MASTER_ABT)
> -		printk("MASTER_ABT ");
> +		printk(KERN_CONT "MASTER_ABT ");
>  	if (dmastat & DMASCR_CHAIN_COMPLETE)
> -		printk("CHAIN_COMPLETE ");
> +		printk(KERN_CONT "CHAIN_COMPLETE ");
>  	if (dmastat & DMASCR_DMA_COMPLETE)
> -		printk("DMA_COMPLETE ");
> +		printk(KERN_CONT "DMA_COMPLETE ");

KERN_NOISE, it's pretty hard to not notice transformation of
bitmask to string representation, isn't it?

> @@ -889,7 +866,8 @@ static int __devinit mm_pci_probe(struct
>  		return -ENODEV;
>  
>  	dev_printk(KERN_INFO, &dev->dev,
> -		"Micro Memory(tm) controller found (PCI Mem Module (Battery Backup))\n");
> +		"Micro Memory(tm) controller found "
> +		"(PCI Mem Module (Battery Backup))\n");

For the record, string splitting occasionaly makes grepping much more annoying:
grep. nothing? kernel in bugreport definitely spit it! [minute later]
Oh, I need to grep for smaller strings.

> @@ -1030,18 +1011,18 @@ static int __devinit mm_pci_probe(struct
>  	data = ~data;
>  	data += 1;
>  
> -	if (request_irq(dev->irq, mm_interrupt, IRQF_SHARED, DRIVER_NAME, card)) {
> +	if (request_irq(dev->irq, mm_interrupt, IRQF_SHARED, DRIVER_NAME,
> +			card)) {

Oh, c'mon!



And my pet peeve with checkpatch.pl crap: why does it force me to _add_
KERN_ markers when my patch changes code leaving printk intact, because
the patch wasn't about printk at all?

Or more philosophically, why a tool which doesn't parse C compiler-style
suddenly starts making suggestions on how C code should look like?

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

* Re: [PATCH] umem nvram driver: clean up style
  2007-12-16 21:55       ` Alexey Dobriyan
@ 2007-12-17  0:49         ` Randy Dunlap
  0 siblings, 0 replies; 8+ messages in thread
From: Randy Dunlap @ 2007-12-17  0:49 UTC (permalink / raw)
  To: Alexey Dobriyan; +Cc: Ingo Molnar, akpm, Neil Brown, David Chinner, lkml

On Mon, 17 Dec 2007 00:55:00 +0300 Alexey Dobriyan wrote:

> >  static void check_batteries(struct cardinfo *card);
> >  
> >  /*
> > ------------------------------------------------------------------------------------
> > ---                           get_userbit
> > ------------------------------------------------------------------------------------
> > -*/
> > + *                           get_userbit
> > + */
> 
> useless comments, they aren't becoming useful after prettifying, right. ;-)

OK, gone.

> >  static void dump_dmastat(struct cardinfo *card, unsigned int dmastat)
> >  {
> >  	dev_printk(KERN_DEBUG, &card->dev->dev, "DMAstat - ");
> >  	if (dmastat & DMASCR_ANY_ERR)
> > -		printk("ANY_ERR ");
> > +		printk(KERN_CONT "ANY_ERR ");
> >  	if (dmastat & DMASCR_MBE_ERR)
> > -		printk("MBE_ERR ");
> > +		printk(KERN_CONT "MBE_ERR ");
> >  	if (dmastat & DMASCR_PARITY_ERR_REP)
> > -		printk("PARITY_ERR_REP ");
> > +		printk(KERN_CONT "PARITY_ERR_REP ");
> >  	if (dmastat & DMASCR_PARITY_ERR_DET)
> > -		printk("PARITY_ERR_DET ");
> > +		printk(KERN_CONT "PARITY_ERR_DET ");
> >  	if (dmastat & DMASCR_SYSTEM_ERR_SIG)
> > -		printk("SYSTEM_ERR_SIG ");
> > +		printk(KERN_CONT "SYSTEM_ERR_SIG ");
> >  	if (dmastat & DMASCR_TARGET_ABT)
> > -		printk("TARGET_ABT ");
> > +		printk(KERN_CONT "TARGET_ABT ");
> >  	if (dmastat & DMASCR_MASTER_ABT)
> > -		printk("MASTER_ABT ");
> > +		printk(KERN_CONT "MASTER_ABT ");
> >  	if (dmastat & DMASCR_CHAIN_COMPLETE)
> > -		printk("CHAIN_COMPLETE ");
> > +		printk(KERN_CONT "CHAIN_COMPLETE ");
> >  	if (dmastat & DMASCR_DMA_COMPLETE)
> > -		printk("DMA_COMPLETE ");
> > +		printk(KERN_CONT "DMA_COMPLETE ");
> 
> KERN_NOISE, it's pretty hard to not notice transformation of
> bitmask to string representation, isn't it?

Was there a suggestion here?

> > @@ -889,7 +866,8 @@ static int __devinit mm_pci_probe(struct
> >  		return -ENODEV;
> >  
> >  	dev_printk(KERN_INFO, &dev->dev,
> > -		"Micro Memory(tm) controller found (PCI Mem Module (Battery Backup))\n");
> > +		"Micro Memory(tm) controller found "
> > +		"(PCI Mem Module (Battery Backup))\n");
> 
> For the record, string splitting occasionaly makes grepping much more annoying:
> grep. nothing? kernel in bugreport definitely spit it! [minute later]
> Oh, I need to grep for smaller strings.

Oh well, restored fwiw.  I don't mind a few source lines of 81 characters.

> > @@ -1030,18 +1011,18 @@ static int __devinit mm_pci_probe(struct
> >  	data = ~data;
> >  	data += 1;
> >  
> > -	if (request_irq(dev->irq, mm_interrupt, IRQF_SHARED, DRIVER_NAME, card)) {
> > +	if (request_irq(dev->irq, mm_interrupt, IRQF_SHARED, DRIVER_NAME,
> > +			card)) {
> 
> Oh, c'mon!

What's the big deal?  It's not like the grep argument.


> And my pet peeve with checkpatch.pl crap: why does it force me to _add_
> KERN_ markers when my patch changes code leaving printk intact, because
> the patch wasn't about printk at all?

It doesn't force you at all.  It suggests.

> Or more philosophically, why a tool which doesn't parse C compiler-style
> suddenly starts making suggestions on how C code should look like?

I look forward to better tools.
New patch below.

---

From: Randy Dunlap <randy.dunlap@oracle.com>

Cleanup umem driver: fix most checkpatch warnings, conform to kernel
coding style.

  linux-2.6.24-rc5-git3> checkpatch.pl-next  patches/block-umem-ckpatch.patch
  total: 0 errors, 5 warnings, 530 lines checked

All of these are line-length warnings.

Only change in generated object file is due to not initializing a
static global variable to 0.

Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
---
 drivers/block/umem.c |  237 ++++++++++++++---------------------------
 1 file changed, 81 insertions(+), 156 deletions(-)

--- linux-2.6.24-rc5-git3.orig/drivers/block/umem.c
+++ linux-2.6.24-rc5-git3/drivers/block/umem.c
@@ -34,7 +34,7 @@
  *			 - set initialised bit then.
  */
 
-//#define DEBUG /* uncomment if you want debugging info (pr_debug) */
+#undef DEBUG	/* #define DEBUG if you want debugging info (pr_debug) */
 #include <linux/fs.h>
 #include <linux/bio.h>
 #include <linux/kernel.h>
@@ -143,17 +143,12 @@ static struct cardinfo cards[MM_MAXCARDS
 static struct block_device_operations mm_fops;
 static struct timer_list battery_timer;
 
-static int num_cards = 0;
+static int num_cards;
 
 static struct gendisk *mm_gendisk[MM_MAXCARDS];
 
 static void check_batteries(struct cardinfo *card);
 
-/*
------------------------------------------------------------------------------------
---                           get_userbit
------------------------------------------------------------------------------------
-*/
 static int get_userbit(struct cardinfo *card, int bit)
 {
 	unsigned char led;
@@ -161,11 +156,7 @@ static int get_userbit(struct cardinfo *
 	led = readb(card->csr_remap + MEMCTRLCMD_LEDCTRL);
 	return led & bit;
 }
-/*
------------------------------------------------------------------------------------
---                            set_userbit
------------------------------------------------------------------------------------
-*/
+
 static int set_userbit(struct cardinfo *card, int bit, unsigned char state)
 {
 	unsigned char led;
@@ -179,11 +170,7 @@ static int set_userbit(struct cardinfo *
 
 	return 0;
 }
-/*
------------------------------------------------------------------------------------
---                             set_led
------------------------------------------------------------------------------------
-*/
+
 /*
  * NOTE: For the power LED, use the LED_POWER_* macros since they differ
  */
@@ -203,11 +190,6 @@ static void set_led(struct cardinfo *car
 }
 
 #ifdef MM_DIAG
-/*
------------------------------------------------------------------------------------
---                              dump_regs
------------------------------------------------------------------------------------
-*/
 static void dump_regs(struct cardinfo *card)
 {
 	unsigned char *p;
@@ -224,32 +206,28 @@ static void dump_regs(struct cardinfo *c
 	}
 }
 #endif
-/*
------------------------------------------------------------------------------------
---                            dump_dmastat
------------------------------------------------------------------------------------
-*/
+
 static void dump_dmastat(struct cardinfo *card, unsigned int dmastat)
 {
 	dev_printk(KERN_DEBUG, &card->dev->dev, "DMAstat - ");
 	if (dmastat & DMASCR_ANY_ERR)
-		printk("ANY_ERR ");
+		printk(KERN_CONT "ANY_ERR ");
 	if (dmastat & DMASCR_MBE_ERR)
-		printk("MBE_ERR ");
+		printk(KERN_CONT "MBE_ERR ");
 	if (dmastat & DMASCR_PARITY_ERR_REP)
-		printk("PARITY_ERR_REP ");
+		printk(KERN_CONT "PARITY_ERR_REP ");
 	if (dmastat & DMASCR_PARITY_ERR_DET)
-		printk("PARITY_ERR_DET ");
+		printk(KERN_CONT "PARITY_ERR_DET ");
 	if (dmastat & DMASCR_SYSTEM_ERR_SIG)
-		printk("SYSTEM_ERR_SIG ");
+		printk(KERN_CONT "SYSTEM_ERR_SIG ");
 	if (dmastat & DMASCR_TARGET_ABT)
-		printk("TARGET_ABT ");
+		printk(KERN_CONT "TARGET_ABT ");
 	if (dmastat & DMASCR_MASTER_ABT)
-		printk("MASTER_ABT ");
+		printk(KERN_CONT "MASTER_ABT ");
 	if (dmastat & DMASCR_CHAIN_COMPLETE)
-		printk("CHAIN_COMPLETE ");
+		printk(KERN_CONT "CHAIN_COMPLETE ");
 	if (dmastat & DMASCR_DMA_COMPLETE)
-		printk("DMA_COMPLETE ");
+		printk(KERN_CONT "DMA_COMPLETE ");
 	printk("\n");
 }
 
@@ -286,7 +264,8 @@ static void mm_start_io(struct cardinfo 
 
 	/* make the last descriptor end the chain */
 	page = &card->mm_pages[card->Active];
-	pr_debug("start_io: %d %d->%d\n", card->Active, page->headcnt, page->cnt-1);
+	pr_debug("start_io: %d %d->%d\n",
+		card->Active, page->headcnt, page->cnt - 1);
 	desc = &page->desc[page->cnt-1];
 
 	desc->control_bits |= cpu_to_le32(DMASCR_CHAIN_COMP_EN);
@@ -310,8 +289,8 @@ static void mm_start_io(struct cardinfo 
 	writel(0, card->csr_remap + DMA_SEMAPHORE_ADDR);
 	writel(0, card->csr_remap + DMA_SEMAPHORE_ADDR + 4);
 
-	offset = ((char*)desc) - ((char*)page->desc);
-	writel(cpu_to_le32((page->page_dma+offset)&0xffffffff),
+	offset = ((char *)desc) - ((char *)page->desc);
+	writel(cpu_to_le32((page->page_dma+offset) & 0xffffffff),
 	       card->csr_remap + DMA_DESCRIPTOR_ADDR);
 	/* Force the value to u64 before shifting otherwise >> 32 is undefined C
 	 * and on some ports will do nothing ! */
@@ -352,7 +331,7 @@ static inline void reset_page(struct mm_
 	page->cnt = 0;
 	page->headcnt = 0;
 	page->bio = NULL;
-	page->biotail = & page->bio;
+	page->biotail = &page->bio;
 }
 
 static void mm_unplug_device(struct request_queue *q)
@@ -408,7 +387,7 @@ static int add_bio(struct cardinfo *card
 				  vec->bv_page,
 				  vec->bv_offset,
 				  len,
-				  (rw==READ) ?
+				  (rw == READ) ?
 				  PCI_DMA_FROMDEVICE : PCI_DMA_TODEVICE);
 
 	p = &card->mm_pages[card->Ready];
@@ -427,10 +406,10 @@ static int add_bio(struct cardinfo *card
 	desc->pci_addr = cpu_to_le64((u64)desc->data_dma_handle);
 	desc->local_addr = cpu_to_le64(card->current_sector << 9);
 	desc->transfer_size = cpu_to_le32(len);
-	offset = ( ((char*)&desc->sem_control_bits) - ((char*)p->desc));
+	offset = (((char *)&desc->sem_control_bits) - ((char *)p->desc));
 	desc->sem_addr = cpu_to_le64((u64)(p->page_dma+offset));
 	desc->zero1 = desc->zero2 = 0;
-	offset = ( ((char*)(desc+1)) - ((char*)p->desc));
+	offset = (((char *)(desc+1)) - ((char *)p->desc));
 	desc->next_desc_addr = cpu_to_le64(p->page_dma+offset);
 	desc->control_bits = cpu_to_le32(DMASCR_GO|DMASCR_ERR_INT_EN|
 					 DMASCR_PARITY_INT_EN|
@@ -455,11 +434,11 @@ static void process_page(unsigned long d
 	/* check if any of the requests in the page are DMA_COMPLETE,
 	 * and deal with them appropriately.
 	 * If we find a descriptor without DMA_COMPLETE in the semaphore, then
-	 * dma must have hit an error on that descriptor, so use dma_status instead
-	 * and assume that all following descriptors must be re-tried.
+	 * dma must have hit an error on that descriptor, so use dma_status
+	 * instead and assume that all following descriptors must be re-tried.
 	 */
 	struct mm_page *page;
-	struct bio *return_bio=NULL;
+	struct bio *return_bio = NULL;
 	struct cardinfo *card = (struct cardinfo *)data;
 	unsigned int dma_status = card->dma_status;
 
@@ -472,12 +451,12 @@ static void process_page(unsigned long d
 		struct bio *bio = page->bio;
 		struct mm_dma_desc *desc = &page->desc[page->headcnt];
 		int control = le32_to_cpu(desc->sem_control_bits);
-		int last=0;
+		int last = 0;
 		int idx;
 
 		if (!(control & DMASCR_DMA_COMPLETE)) {
 			control = dma_status;
-			last=1;
+			last = 1;
 		}
 		page->headcnt++;
 		idx = page->idx;
@@ -489,8 +468,8 @@ static void process_page(unsigned long d
 		}
 
 		pci_unmap_page(card->dev, desc->data_dma_handle,
-			       bio_iovec_idx(bio,idx)->bv_len,
-				 (control& DMASCR_TRANSFER_READ) ?
+			       bio_iovec_idx(bio, idx)->bv_len,
+				 (control & DMASCR_TRANSFER_READ) ?
 				PCI_DMA_TODEVICE : PCI_DMA_FROMDEVICE);
 		if (control & DMASCR_HARD_ERROR) {
 			/* error */
@@ -501,9 +480,10 @@ static void process_page(unsigned long d
 				le32_to_cpu(desc->transfer_size));
 			dump_dmastat(card, control);
 		} else if (test_bit(BIO_RW, &bio->bi_rw) &&
-			   le32_to_cpu(desc->local_addr)>>9 == card->init_size) {
-			card->init_size += le32_to_cpu(desc->transfer_size)>>9;
-			if (card->init_size>>1 >= card->mm_size) {
+			   le32_to_cpu(desc->local_addr) >> 9 ==
+				card->init_size) {
+			card->init_size += le32_to_cpu(desc->transfer_size) >> 9;
+			if (card->init_size >> 1 >= card->mm_size) {
 				dev_printk(KERN_INFO, &card->dev->dev,
 					"memory now initialised\n");
 				set_userbit(card, MEMORY_INITIALIZED, 1);
@@ -514,7 +494,8 @@ static void process_page(unsigned long d
 			return_bio = bio;
 		}
 
-		if (last) break;
+		if (last)
+			break;
 	}
 
 	if (debug & DEBUG_LED_ON_TRANSFER)
@@ -536,7 +517,7 @@ static void process_page(unsigned long d
  out_unlock:
 	spin_unlock_bh(&card->lock);
 
-	while(return_bio) {
+	while (return_bio) {
 		struct bio *bio = return_bio;
 
 		return_bio = bio->bi_next;
@@ -545,11 +526,6 @@ static void process_page(unsigned long d
 	}
 }
 
-/*
------------------------------------------------------------------------------------
---                              mm_make_request
------------------------------------------------------------------------------------
-*/
 static int mm_make_request(struct request_queue *q, struct bio *bio)
 {
 	struct cardinfo *card = q->queuedata;
@@ -566,11 +542,6 @@ static int mm_make_request(struct reques
 	return 0;
 }
 
-/*
------------------------------------------------------------------------------------
---                              mm_interrupt
------------------------------------------------------------------------------------
-*/
 static irqreturn_t mm_interrupt(int irq, void *__card)
 {
 	struct cardinfo *card = (struct cardinfo *) __card;
@@ -584,15 +555,15 @@ HW_TRACE(0x30);
 	if (!(dma_status & (DMASCR_ERROR_MASK | DMASCR_CHAIN_COMPLETE))) {
 		/* interrupt wasn't for me ... */
 		return IRQ_NONE;
-        }
+	}
 
 	/* clear COMPLETION interrupts */
 	if (card->flags & UM_FLAG_NO_BYTE_STATUS)
 		writel(cpu_to_le32(DMASCR_DMA_COMPLETE|DMASCR_CHAIN_COMPLETE),
-		       card->csr_remap+ DMA_STATUS_CTRL);
+		       card->csr_remap + DMA_STATUS_CTRL);
 	else
 		writeb((DMASCR_DMA_COMPLETE|DMASCR_CHAIN_COMPLETE) >> 16,
-		       card->csr_remap+ DMA_STATUS_CTRL + 2);
+		       card->csr_remap + DMA_STATUS_CTRL + 2);
 
 	/* log errors and clear interrupt status */
 	if (dma_status & DMASCR_ANY_ERR) {
@@ -602,9 +573,12 @@ HW_TRACE(0x30);
 
 		stat = readb(card->csr_remap + MEMCTRLCMD_ERRSTATUS);
 
-		data_log1 = le32_to_cpu(readl(card->csr_remap + ERROR_DATA_LOG));
-		data_log2 = le32_to_cpu(readl(card->csr_remap + ERROR_DATA_LOG + 4));
-		addr_log1 = le32_to_cpu(readl(card->csr_remap + ERROR_ADDR_LOG));
+		data_log1 = le32_to_cpu(readl(card->csr_remap +
+						ERROR_DATA_LOG));
+		data_log2 = le32_to_cpu(readl(card->csr_remap +
+						ERROR_DATA_LOG + 4));
+		addr_log1 = le32_to_cpu(readl(card->csr_remap +
+						ERROR_ADDR_LOG));
 		addr_log2 = readb(card->csr_remap + ERROR_ADDR_LOG + 4);
 
 		count = readb(card->csr_remap + ERROR_COUNT);
@@ -671,11 +645,7 @@ HW_TRACE(0x36);
 
 	return IRQ_HANDLED;
 }
-/*
------------------------------------------------------------------------------------
---                         set_fault_to_battery_status
------------------------------------------------------------------------------------
-*/
+
 /*
  * If both batteries are good, no LED
  * If either battery has been warned, solid LED
@@ -696,12 +666,6 @@ static void set_fault_to_battery_status(
 
 static void init_battery_timer(void);
 
-
-/*
------------------------------------------------------------------------------------
---                            check_battery
------------------------------------------------------------------------------------
-*/
 static int check_battery(struct cardinfo *card, int battery, int status)
 {
 	if (status != card->battery[battery].good) {
@@ -730,11 +694,7 @@ static int check_battery(struct cardinfo
 
 	return 0;
 }
-/*
------------------------------------------------------------------------------------
---                              check_batteries
------------------------------------------------------------------------------------
-*/
+
 static void check_batteries(struct cardinfo *card)
 {
 	/* NOTE: this must *never* be called while the card
@@ -775,11 +735,7 @@ static void check_all_batteries(unsigned
 
 	init_battery_timer();
 }
-/*
------------------------------------------------------------------------------------
---                            init_battery_timer
------------------------------------------------------------------------------------
-*/
+
 static void init_battery_timer(void)
 {
 	init_timer(&battery_timer);
@@ -787,20 +743,12 @@ static void init_battery_timer(void)
 	battery_timer.expires = jiffies + (HZ * 60);
 	add_timer(&battery_timer);
 }
-/*
------------------------------------------------------------------------------------
---                              del_battery_timer
------------------------------------------------------------------------------------
-*/
+
 static void del_battery_timer(void)
 {
 	del_timer(&battery_timer);
 }
-/*
------------------------------------------------------------------------------------
---                                mm_revalidate
------------------------------------------------------------------------------------
-*/
+
 /*
  * Note no locks taken out here.  In a worst case scenario, we could drop
  * a chunk of system memory.  But that should never happen, since validation
@@ -833,33 +781,23 @@ static int mm_getgeo(struct block_device
 }
 
 /*
------------------------------------------------------------------------------------
---                                mm_check_change
------------------------------------------------------------------------------------
-  Future support for removable devices
-*/
+ * Future support for removable devices
+ */
 static int mm_check_change(struct gendisk *disk)
 {
 /*  struct cardinfo *dev = disk->private_data; */
 	return 0;
 }
-/*
------------------------------------------------------------------------------------
---                             mm_fops
------------------------------------------------------------------------------------
-*/
+
 static struct block_device_operations mm_fops = {
 	.owner		= THIS_MODULE,
 	.getgeo		= mm_getgeo,
-	.revalidate_disk= mm_revalidate,
+	.revalidate_disk = mm_revalidate,
 	.media_changed	= mm_check_change,
 };
-/*
------------------------------------------------------------------------------------
---                                mm_pci_probe
------------------------------------------------------------------------------------
-*/
-static int __devinit mm_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
+
+static int __devinit mm_pci_probe(struct pci_dev *dev,
+				const struct pci_device_id *id)
 {
 	int ret = -ENODEV;
 	struct cardinfo *card = &cards[num_cards];
@@ -889,7 +827,7 @@ static int __devinit mm_pci_probe(struct
 		return -ENODEV;
 
 	dev_printk(KERN_INFO, &dev->dev,
-		"Micro Memory(tm) controller found (PCI Mem Module (Battery Backup))\n");
+	  "Micro Memory(tm) controller found (PCI Mem Module (Battery Backup))\n");
 
 	if (pci_set_dma_mask(dev, DMA_64BIT_MASK) &&
 	    pci_set_dma_mask(dev, DMA_32BIT_MASK)) {
@@ -917,7 +855,7 @@ static int __devinit mm_pci_probe(struct
 		"CSR 0x%08lx -> 0x%p (0x%lx)\n",
 	       csr_base, card->csr_remap, csr_len);
 
-	switch(card->dev->device) {
+	switch (card->dev->device) {
 	case 0x5415:
 		card->flags |= UM_FLAG_NO_BYTE_STATUS | UM_FLAG_NO_BATTREG;
 		magic_number = 0x59;
@@ -929,7 +867,8 @@ static int __devinit mm_pci_probe(struct
 		break;
 
 	case 0x6155:
-		card->flags |= UM_FLAG_NO_BYTE_STATUS | UM_FLAG_NO_BATTREG | UM_FLAG_NO_BATT;
+		card->flags |= UM_FLAG_NO_BYTE_STATUS |
+				UM_FLAG_NO_BATTREG | UM_FLAG_NO_BATT;
 		magic_number = 0x99;
 		break;
 
@@ -945,11 +884,11 @@ static int __devinit mm_pci_probe(struct
 	}
 
 	card->mm_pages[0].desc = pci_alloc_consistent(card->dev,
-						      PAGE_SIZE*2,
-						      &card->mm_pages[0].page_dma);
+						PAGE_SIZE * 2,
+						&card->mm_pages[0].page_dma);
 	card->mm_pages[1].desc = pci_alloc_consistent(card->dev,
-						      PAGE_SIZE*2,
-						      &card->mm_pages[1].page_dma);
+						PAGE_SIZE * 2,
+						&card->mm_pages[1].page_dma);
 	if (card->mm_pages[0].desc == NULL ||
 	    card->mm_pages[1].desc == NULL) {
 		dev_printk(KERN_ERR, &card->dev->dev, "alloc failed\n");
@@ -1013,9 +952,9 @@ static int __devinit mm_pci_probe(struct
 		dev_printk(KERN_INFO, &card->dev->dev,
 			"Size %d KB, Battery 1 %s (%s), Battery 2 %s (%s)\n",
 		       card->mm_size,
-		       (batt_status & BATTERY_1_DISABLED ? "Disabled" : "Enabled"),
+		       batt_status & BATTERY_1_DISABLED ? "Disabled" : "Enabled",
 		       card->battery[0].good ? "OK" : "FAILURE",
-		       (batt_status & BATTERY_2_DISABLED ? "Disabled" : "Enabled"),
+		       batt_status & BATTERY_2_DISABLED ? "Disabled" : "Enabled",
 		       card->battery[1].good ? "OK" : "FAILURE");
 
 		set_fault_to_battery_status(card);
@@ -1030,18 +969,18 @@ static int __devinit mm_pci_probe(struct
 	data = ~data;
 	data += 1;
 
-	if (request_irq(dev->irq, mm_interrupt, IRQF_SHARED, DRIVER_NAME, card)) {
+	if (request_irq(dev->irq, mm_interrupt, IRQF_SHARED, DRIVER_NAME,
+			card)) {
 		dev_printk(KERN_ERR, &card->dev->dev,
 			"Unable to allocate IRQ\n");
 		ret = -ENODEV;
-
 		goto failed_req_irq;
 	}
 
 	dev_printk(KERN_INFO, &card->dev->dev,
 		"Window size %d bytes, IRQ %d\n", data, dev->irq);
 
-        spin_lock_init(&card->lock);
+	spin_lock_init(&card->lock);
 
 	pci_set_drvdata(dev, card);
 
@@ -1060,7 +999,7 @@ static int __devinit mm_pci_probe(struct
 
 	if (!get_userbit(card, MEMORY_INITIALIZED)) {
 		dev_printk(KERN_INFO, &card->dev->dev,
-			"memory NOT initialized. Consider over-writing whole device.\n");
+		  "memory NOT initialized. Consider over-writing whole device.\n");
 		card->init_size = 0;
 	} else {
 		dev_printk(KERN_INFO, &card->dev->dev,
@@ -1091,11 +1030,7 @@ static int __devinit mm_pci_probe(struct
 
 	return ret;
 }
-/*
------------------------------------------------------------------------------------
---                              mm_pci_remove
------------------------------------------------------------------------------------
-*/
+
 static void mm_pci_remove(struct pci_dev *dev)
 {
 	struct cardinfo *card = pci_get_drvdata(dev);
@@ -1119,16 +1054,16 @@ static void mm_pci_remove(struct pci_dev
 }
 
 static const struct pci_device_id mm_pci_ids[] = {
-    {PCI_DEVICE(PCI_VENDOR_ID_MICRO_MEMORY,PCI_DEVICE_ID_MICRO_MEMORY_5415CN)},
-    {PCI_DEVICE(PCI_VENDOR_ID_MICRO_MEMORY,PCI_DEVICE_ID_MICRO_MEMORY_5425CN)},
-    {PCI_DEVICE(PCI_VENDOR_ID_MICRO_MEMORY,PCI_DEVICE_ID_MICRO_MEMORY_6155)},
+    {PCI_DEVICE(PCI_VENDOR_ID_MICRO_MEMORY, PCI_DEVICE_ID_MICRO_MEMORY_5415CN)},
+    {PCI_DEVICE(PCI_VENDOR_ID_MICRO_MEMORY, PCI_DEVICE_ID_MICRO_MEMORY_5425CN)},
+    {PCI_DEVICE(PCI_VENDOR_ID_MICRO_MEMORY, PCI_DEVICE_ID_MICRO_MEMORY_6155)},
     {
 	.vendor	=	0x8086,
 	.device	=	0xB555,
-	.subvendor=	0x1332,
-	.subdevice=	0x5460,
-	.class	=	0x050000,
-	.class_mask=	0,
+	.subvendor =	0x1332,
+	.subdevice =	0x5460,
+	.class =	0x050000,
+	.class_mask =	0,
     }, { /* end: all zeroes */ }
 };
 
@@ -1141,12 +1076,6 @@ static struct pci_driver mm_pci_driver =
 	.remove		= mm_pci_remove,
 };
 
-/*
------------------------------------------------------------------------------------
---                               mm_init
------------------------------------------------------------------------------------
-*/
-
 static int __init mm_init(void)
 {
 	int retval, i;
@@ -1193,18 +1122,14 @@ out:
 		put_disk(mm_gendisk[i]);
 	return -ENOMEM;
 }
-/*
------------------------------------------------------------------------------------
---                             mm_cleanup
------------------------------------------------------------------------------------
-*/
+
 static void __exit mm_cleanup(void)
 {
 	int i;
 
 	del_battery_timer();
 
-	for (i=0; i < num_cards ; i++) {
+	for (i = 0; i < num_cards ; i++) {
 		del_gendisk(mm_gendisk[i]);
 		put_disk(mm_gendisk[i]);
 	}

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

end of thread, other threads:[~2007-12-17  0:52 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-12-03 21:35 Regression - 2.6.24-rc3 - umem nvram card driver oops David Chinner
2007-12-04  0:14 ` Neil Brown
2007-12-04  2:20   ` David Chinner
2007-12-04  2:48     ` Neil Brown
2007-12-04  9:37   ` Ingo Molnar
2007-12-16 21:15     ` [PATCH] umem nvram driver: clean up style Randy Dunlap
2007-12-16 21:55       ` Alexey Dobriyan
2007-12-17  0:49         ` Randy Dunlap

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