2007-12-03 21:36:28

by David Chinner

[permalink] [raw]
Subject: Regression - 2.6.24-rc3 - umem nvram card driver oops

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


2007-12-04 00:14:26

by NeilBrown

[permalink] [raw]
Subject: Re: Regression - 2.6.24-rc3 - umem nvram card driver oops

On Tuesday December 4, [email protected] 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 <[email protected]>

### 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,

2007-12-04 02:20:36

by David Chinner

[permalink] [raw]
Subject: Re: Regression - 2.6.24-rc3 - umem nvram card driver oops

On Tue, Dec 04, 2007 at 11:14:12AM +1100, Neil Brown wrote:
> On Tuesday December 4, [email protected] 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 <[email protected]>

Cheers,

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

2007-12-04 02:49:10

by NeilBrown

[permalink] [raw]
Subject: Re: Regression - 2.6.24-rc3 - umem nvram card driver oops

On Tuesday December 4, [email protected] wrote:
> On Tue, Dec 04, 2007 at 11:14:12AM +1100, Neil Brown wrote:
> > On Tuesday December 4, [email protected] 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 <[email protected]>

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

NeilBrown

2007-12-04 09:37:35

by Ingo Molnar

[permalink] [raw]
Subject: Re: Regression - 2.6.24-rc3 - umem nvram card driver oops


* Neil Brown <[email protected]> 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

2007-12-16 21:17:09

by Randy Dunlap

[permalink] [raw]
Subject: [PATCH] umem nvram driver: clean up style

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 <[email protected]>

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 <[email protected]>
---
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]);
}

2007-12-16 21:55:24

by Alexey Dobriyan

[permalink] [raw]
Subject: Re: [PATCH] umem nvram driver: clean up style

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?

2007-12-17 00:52:16

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH] umem nvram driver: clean up style

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 <[email protected]>

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 <[email protected]>
---
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]);
}