2006-12-29 21:31:22

by Sumant Patro

[permalink] [raw]
Subject: [Patch] scsi: megaraid_{mm,mbox}: init fix for kdump


- Changes in Initialization to fix kdump failure.
Without this fix, megaraid driver either panics or fails to
initialize the adapter during the kdump's 2nd kernel boot
if there are pending commands or interrupts from other devices
sharing the same IRQ.
Fix:Send SYNC command on loading.
This command clears the pending commands in the adapter
and re-initialize its internal RAID structure.

Signed-Off By: Sumant Patro <[email protected]>

diff -uprN linux-2.6.orig/Documentation/scsi/ChangeLog.megaraid linux-2.6.new/Documentation/scsi/ChangeLog.megaraid
--- linux-2.6.orig/Documentation/scsi/ChangeLog.megaraid 2006-12-28 10:10:31.000000000 -0800
+++ linux-2.6.new/Documentation/scsi/ChangeLog.megaraid 2006-12-29 06:57:44.000000000 -0800
@@ -1,3 +1,16 @@
+Release Date : Thu Nov 16 15:32:35 EST 2006 - Sumant Patro <[email protected]>
+Current Version : 2.20.5.1 (scsi module), 2.20.2.6 (cmm module)
+Older Version : 2.20.4.9 (scsi module), 2.20.2.6 (cmm module)
+
+1. Changes in Initialization to fix kdump failure.
+Without this fix, megaraid driver either panics or fails to
+initialize the adapter during the kdump's 2nd kernel boot
+if there are pending commands or interrupts from other devices
+sharing the same IRQ.
+ - Send SYNC command on loading.
+ This command clears the pending commands in the adapter
+ and re-initialize its internal RAID structure.
+
Release Date : Fri May 19 09:31:45 EST 2006 - Seokmann Ju <[email protected]>
Current Version : 2.20.4.9 (scsi module), 2.20.2.6 (cmm module)
Older Version : 2.20.4.8 (scsi module), 2.20.2.6 (cmm module)
diff -uprN linux-2.6.orig/drivers/scsi/megaraid/megaraid_mbox.c linux-2.6.new/drivers/scsi/megaraid/megaraid_mbox.c
--- linux-2.6.orig/drivers/scsi/megaraid/megaraid_mbox.c 2006-12-28 09:56:04.000000000 -0800
+++ linux-2.6.new/drivers/scsi/megaraid/megaraid_mbox.c 2006-12-29 05:31:48.000000000 -0800
@@ -10,13 +10,13 @@
* 2 of the License, or (at your option) any later version.
*
* FILE : megaraid_mbox.c
- * Version : v2.20.4.9 (Jul 16 2006)
+ * Version : v2.20.5.1 (Nov 16 2006)
*
* Authors:
- * Atul Mukker <[email protected]>
- * Sreenivas Bagalkote <[email protected]>
- * Manoj Jose <[email protected]>
- * Seokmann Ju <[email protected]>
+ * Atul Mukker <[email protected]>
+ * Sreenivas Bagalkote <[email protected]>
+ * Manoj Jose <[email protected]>
+ * Seokmann Ju <[email protected]>
*
* List of supported controllers
*
@@ -107,6 +107,7 @@ static int megaraid_mbox_support_random_
static int megaraid_mbox_get_max_sg(adapter_t *);
static void megaraid_mbox_enum_raid_scsi(adapter_t *);
static void megaraid_mbox_flush_cache(adapter_t *);
+static int megaraid_mbox_fire_sync_cmd(adapter_t *);

static void megaraid_mbox_display_scb(adapter_t *, scb_t *);
static void megaraid_mbox_setup_device_map(adapter_t *);
@@ -137,7 +138,7 @@ static int wait_till_fw_empty(adapter_t



-MODULE_AUTHOR("[email protected]");
+MODULE_AUTHOR("[email protected]");
MODULE_DESCRIPTION("LSI Logic MegaRAID Mailbox Driver");
MODULE_LICENSE("GPL");
MODULE_VERSION(MEGARAID_VERSION);
@@ -779,6 +780,22 @@ megaraid_init_mbox(adapter_t *adapter)
goto out_release_regions;
}

+ // initialize the mutual exclusion lock for the mailbox
+ spin_lock_init(&raid_dev->mailbox_lock);
+
+ // allocate memory required for commands
+ if (megaraid_alloc_cmd_packets(adapter) != 0) {
+ goto out_iounmap;
+ }
+
+ /*
+ * Issue SYNC cmd to flush the pending cmds in the adapter
+ * and initialize its internal state
+ */
+
+ if (megaraid_mbox_fire_sync_cmd(adapter))
+ con_log(CL_ANN, ("megaraid: sync cmd failed\n"));
+
//
// Setup the rest of the soft state using the library of FW routines
//
@@ -789,22 +806,13 @@ megaraid_init_mbox(adapter_t *adapter)

con_log(CL_ANN, (KERN_WARNING
"megaraid: Couldn't register IRQ %d!\n", adapter->irq));
+ goto out_alloc_cmds;

- goto out_iounmap;
- }
-
-
- // initialize the mutual exclusion lock for the mailbox
- spin_lock_init(&raid_dev->mailbox_lock);
-
- // allocate memory required for commands
- if (megaraid_alloc_cmd_packets(adapter) != 0) {
- goto out_free_irq;
}

// Product info
if (megaraid_mbox_product_info(adapter) != 0) {
- goto out_alloc_cmds;
+ goto out_free_irq;
}

// Do we support extended CDBs
@@ -875,7 +883,7 @@ megaraid_init_mbox(adapter_t *adapter)
* accessed
*/
if (megaraid_sysfs_alloc_resources(adapter) != 0) {
- goto out_alloc_cmds;
+ goto out_free_irq;
}

// Set the DMA mask to 64-bit. All supported controllers as capable of
@@ -920,10 +928,10 @@ megaraid_init_mbox(adapter_t *adapter)

out_free_sysfs_res:
megaraid_sysfs_free_resources(adapter);
-out_alloc_cmds:
- megaraid_free_cmd_packets(adapter);
out_free_irq:
free_irq(adapter->irq, adapter);
+out_alloc_cmds:
+ megaraid_free_cmd_packets(adapter);
out_iounmap:
iounmap(raid_dev->baseaddr);
out_release_regions:
@@ -3380,6 +3388,86 @@ megaraid_mbox_flush_cache(adapter_t *ada


/**
+ * megaraid_mbox_fire_sync_cmd - fire the sync cmd
+ * @param adapter : soft state for the controller
+ */
+static int
+megaraid_mbox_fire_sync_cmd(adapter_t *adapter)
+{
+ mbox_t *mbox;
+ uint8_t raw_mbox[sizeof(mbox_t)];
+ mraid_device_t *raid_dev = ADAP2RAIDDEV(adapter);
+ mbox64_t *mbox64;
+ uint8_t status = 0;
+ int i;
+ uint32_t dword;
+
+ mbox = (mbox_t *)raw_mbox;
+
+ memset((caddr_t)raw_mbox, 0, sizeof(mbox_t));
+
+ raw_mbox[0] = 0xFF;
+
+ mbox64 = raid_dev->mbox64;
+ mbox = raid_dev->mbox;
+
+ /*
+ * Wait until mailbox is free
+ */
+ if (megaraid_busywait_mbox(raid_dev) != 0) {
+ status = 1;
+ goto blocked_mailbox;
+ }
+
+ /*
+ * Copy mailbox data into host structure
+ */
+ memcpy((caddr_t)mbox, (caddr_t)raw_mbox, 16);
+ mbox->cmdid = 0xFE;
+ mbox->busy = 1;
+ mbox->poll = 0;
+ mbox->ack = 0;
+ mbox->numstatus = 0;
+ mbox->status = 0;
+
+ wmb();
+ WRINDOOR(raid_dev, raid_dev->mbox_dma | 0x1);
+
+ // wait for maximum 1 min for status to post.
+ // If the Firmware SUPPORTS the ABOVE COMMAND,
+ // mbox->cmd will be set to 0
+ // else
+ // the firmware will reject the command with
+ // mbox->numstatus set to 1
+
+ i = 0;
+ status = 0;
+ while (!mbox->numstatus && mbox->cmd == 0xFF) {
+ rmb();
+ msleep(1);
+ i++;
+ if (i > 1000 * 60) {
+ status = 1;
+ break;
+ }
+ }
+ if (mbox->numstatus == 1)
+ status = 1; /*cmd not supported*/
+ /*
+ * Check for interrupt line
+ */
+ dword = RDOUTDOOR(raid_dev);
+ WROUTDOOR(raid_dev, dword);
+ WRINDOOR(raid_dev,2);
+
+ return status;
+
+blocked_mailbox:
+ con_log(CL_ANN, (KERN_WARNING "megaraid: blocked mailbox\n"));
+ return status;
+}
+
+/**
* megaraid_mbox_display_scb - display SCB information, mostly debug purposes
* @param adapter : controllers' soft state
* @param scb : SCB to be displayed
diff -uprN linux-2.6.orig/drivers/scsi/megaraid/megaraid_mbox.h linux-2.6.new/drivers/scsi/megaraid/megaraid_mbox.h
--- linux-2.6.orig/drivers/scsi/megaraid/megaraid_mbox.h 2006-12-28 09:56:04.000000000 -0800
+++ linux-2.6.new/drivers/scsi/megaraid/megaraid_mbox.h 2006-12-29 05:31:50.000000000 -0800
@@ -21,8 +21,8 @@
#include "megaraid_ioctl.h"


-#define MEGARAID_VERSION "2.20.4.9"
-#define MEGARAID_EXT_VERSION "(Release Date: Sun Jul 16 12:27:22 EST 2006)"
+#define MEGARAID_VERSION "2.20.5.1"
+#define MEGARAID_EXT_VERSION "(Release Date: Thu Nov 16 15:32:35 EST 2006)"


/*


Attachments:
megaraid-initialization-fix-for-kdump.patch (6.80 kB)

2006-12-29 21:49:20

by Randy Dunlap

[permalink] [raw]
Subject: Re: [Patch] scsi: megaraid_{mm,mbox}: init fix for kdump

On Fri, 29 Dec 2006 08:02:17 -0800 Sumant Patro wrote:

See Documentation/SubmittingPatches:
Please include output of "diffstat -p1 -w70" so that we can easily see
the scope of the changes.

and see Documentation/CodingStyle for comments below:


> diff -uprN linux-2.6.orig/drivers/scsi/megaraid/megaraid_mbox.c linux-2.6.new/drivers/scsi/megaraid/megaraid_mbox.c
> --- linux-2.6.orig/drivers/scsi/megaraid/megaraid_mbox.c 2006-12-28 09:56:04.000000000 -0800
> +++ linux-2.6.new/drivers/scsi/megaraid/megaraid_mbox.c 2006-12-29 05:31:48.000000000 -0800
> @@ -779,6 +780,22 @@ megaraid_init_mbox(adapter_t *adapter)
> goto out_release_regions;
> }
>
> + // initialize the mutual exclusion lock for the mailbox
> + spin_lock_init(&raid_dev->mailbox_lock);

Linux uses /*...*/ C89-style comments, not // C99 comments.

> + // allocate memory required for commands
> + if (megaraid_alloc_cmd_packets(adapter) != 0) {
> + goto out_iounmap;
> + }
> +
> + /*
> + * Issue SYNC cmd to flush the pending cmds in the adapter
> + * and initialize its internal state
> + */
> +
> + if (megaraid_mbox_fire_sync_cmd(adapter))
> + con_log(CL_ANN, ("megaraid: sync cmd failed\n"));
> +

> // Product info
> if (megaraid_mbox_product_info(adapter) != 0) {
> - goto out_alloc_cmds;
> + goto out_free_irq;

Don't uses {} braces around 1-statement "blocks".

> @@ -875,7 +883,7 @@ megaraid_init_mbox(adapter_t *adapter)
> * accessed
> */
> if (megaraid_sysfs_alloc_resources(adapter) != 0) {
> - goto out_alloc_cmds;
> + goto out_free_irq;

Ditto.

> }
>
> // Set the DMA mask to 64-bit. All supported controllers as capable of
> @@ -3380,6 +3388,86 @@ megaraid_mbox_flush_cache(adapter_t *ada
>
>
> /**
> + * megaraid_mbox_fire_sync_cmd - fire the sync cmd
> + * @param adapter : soft state for the controller
> + */
> +static int
> +megaraid_mbox_fire_sync_cmd(adapter_t *adapter)
> +{
> + mbox_t *mbox;
> + uint8_t raw_mbox[sizeof(mbox_t)];
> + mraid_device_t *raid_dev = ADAP2RAIDDEV(adapter);
> + mbox64_t *mbox64;
> + uint8_t status = 0;
> + int i;
> + uint32_t dword;
> +
> + mbox = (mbox_t *)raw_mbox;
> +
> + memset((caddr_t)raw_mbox, 0, sizeof(mbox_t));
> +
> + raw_mbox[0] = 0xFF;
> +
> + mbox64 = raid_dev->mbox64;
> + mbox = raid_dev->mbox;
> +
> + /*
> + * Wait until mailbox is free
> + */
> + if (megaraid_busywait_mbox(raid_dev) != 0) {
> + status = 1;
> + goto blocked_mailbox;
> + }
> +
> + /*
> + * Copy mailbox data into host structure
> + */
> + memcpy((caddr_t)mbox, (caddr_t)raw_mbox, 16);
> + mbox->cmdid = 0xFE;
> + mbox->busy = 1;
> + mbox->poll = 0;
> + mbox->ack = 0;
> + mbox->numstatus = 0;
> + mbox->status = 0;
> +
> + wmb();
> + WRINDOOR(raid_dev, raid_dev->mbox_dma | 0x1);
> +
> + // wait for maximum 1 min for status to post.
> + // If the Firmware SUPPORTS the ABOVE COMMAND,
> + // mbox->cmd will be set to 0
> + // else
> + // the firmware will reject the command with
> + // mbox->numstatus set to 1

Don't use // comment style. Also, for multi-line comments
in Linux, please use this preferred style:

/*
* This is the preferred style for multi-line
* comments in the Linux kernel source code.
* Please use it consistently.
*
* Description: A column of asterisks on the left side,
* with beginning and ending almost-blank lines.
*/

Thanks.
---
~Randy

2006-12-30 14:31:53

by Douglas Gilbert

[permalink] [raw]
Subject: Re: [Patch] scsi: megaraid_{mm,mbox}: init fix for kdump

Randy Dunlap wrote:
> On Fri, 29 Dec 2006 08:02:17 -0800 Sumant Patro wrote:
>
> See Documentation/SubmittingPatches:
> Please include output of "diffstat -p1 -w70" so that we can easily see
> the scope of the changes.
>
> and see Documentation/CodingStyle for comments below:
>
>
>> diff -uprN linux-2.6.orig/drivers/scsi/megaraid/megaraid_mbox.c linux-2.6.new/drivers/scsi/megaraid/megaraid_mbox.c
>> --- linux-2.6.orig/drivers/scsi/megaraid/megaraid_mbox.c 2006-12-28 09:56:04.000000000 -0800
>> +++ linux-2.6.new/drivers/scsi/megaraid/megaraid_mbox.c 2006-12-29 05:31:48.000000000 -0800
>> @@ -779,6 +780,22 @@ megaraid_init_mbox(adapter_t *adapter)
>> goto out_release_regions;
>> }
>>
>> + // initialize the mutual exclusion lock for the mailbox
>> + spin_lock_init(&raid_dev->mailbox_lock);
>
> Linux uses /*...*/ C89-style comments, not // C99 comments.

Randy
It is about time this absurd stipulation was dropped.
Are there any C compilers that can compile the linux
kernel and that don't accept both _standard_ comment styles?

Doug Gilbert

2006-12-30 15:51:48

by Randy Dunlap

[permalink] [raw]
Subject: Re: [Patch] scsi: megaraid_{mm,mbox}: init fix for kdump

On Sat, 30 Dec 2006 09:31:38 -0500 Douglas Gilbert wrote:

> Randy Dunlap wrote:
> > On Fri, 29 Dec 2006 08:02:17 -0800 Sumant Patro wrote:
> >
> > See Documentation/SubmittingPatches:
> > Please include output of "diffstat -p1 -w70" so that we can easily see
> > the scope of the changes.
> >
> > and see Documentation/CodingStyle for comments below:
> >
> >
> >> diff -uprN linux-2.6.orig/drivers/scsi/megaraid/megaraid_mbox.c linux-2.6.new/drivers/scsi/megaraid/megaraid_mbox.c
> >> --- linux-2.6.orig/drivers/scsi/megaraid/megaraid_mbox.c 2006-12-28 09:56:04.000000000 -0800
> >> +++ linux-2.6.new/drivers/scsi/megaraid/megaraid_mbox.c 2006-12-29 05:31:48.000000000 -0800
> >> @@ -779,6 +780,22 @@ megaraid_init_mbox(adapter_t *adapter)
> >> goto out_release_regions;
> >> }
> >>
> >> + // initialize the mutual exclusion lock for the mailbox
> >> + spin_lock_init(&raid_dev->mailbox_lock);
> >
> > Linux uses /*...*/ C89-style comments, not // C99 comments.
>
> Randy
> It is about time this absurd stipulation was dropped.
> Are there any C compilers that can compile the linux
> kernel and that don't accept both _standard_ comment styles?

It's not a technical issue, it's just a style point.

---
~Randy

2007-01-03 02:55:24

by Patro, Sumant

[permalink] [raw]
Subject: RE: [Patch] scsi: megaraid_{mm,mbox}: init fix for kdump


Thanks for the review.
I will resubmit the patch.

Regards,

Sumant

-----Original Message-----
From: Randy Dunlap [mailto:[email protected]]
Sent: Friday, December 29, 2006 1:38 PM
To: Patro, Sumant
Cc: [email protected]; [email protected];
[email protected]; [email protected]; Kolli, Neela;
Yang, Bo; Patro, Sumant
Subject: Re: [Patch] scsi: megaraid_{mm,mbox}: init fix for kdump

On Fri, 29 Dec 2006 08:02:17 -0800 Sumant Patro wrote:

See Documentation/SubmittingPatches:
Please include output of "diffstat -p1 -w70" so that we can easily see
the scope of the changes.

and see Documentation/CodingStyle for comments below:


> diff -uprN linux-2.6.orig/drivers/scsi/megaraid/megaraid_mbox.c
> linux-2.6.new/drivers/scsi/megaraid/megaraid_mbox.c
> --- linux-2.6.orig/drivers/scsi/megaraid/megaraid_mbox.c 2006-12-28
> 09:56:04.000000000 -0800
> +++ linux-2.6.new/drivers/scsi/megaraid/megaraid_mbox.c 2006-12-29
> +++ 05:31:48.000000000 -0800
> @@ -779,6 +780,22 @@ megaraid_init_mbox(adapter_t *adapter)
> goto out_release_regions;
> }
>
> + // initialize the mutual exclusion lock for the mailbox
> + spin_lock_init(&raid_dev->mailbox_lock);

Linux uses /*...*/ C89-style comments, not // C99 comments.

> + // allocate memory required for commands
> + if (megaraid_alloc_cmd_packets(adapter) != 0) {
> + goto out_iounmap;
> + }
> +
> + /*
> + * Issue SYNC cmd to flush the pending cmds in the adapter
> + * and initialize its internal state
> + */
> +
> + if (megaraid_mbox_fire_sync_cmd(adapter))
> + con_log(CL_ANN, ("megaraid: sync cmd failed\n"));
> +

> // Product info
> if (megaraid_mbox_product_info(adapter) != 0) {
> - goto out_alloc_cmds;
> + goto out_free_irq;

Don't uses {} braces around 1-statement "blocks".

> @@ -875,7 +883,7 @@ megaraid_init_mbox(adapter_t *adapter)
> * accessed
> */
> if (megaraid_sysfs_alloc_resources(adapter) != 0) {
> - goto out_alloc_cmds;
> + goto out_free_irq;

Ditto.

> }
>
> // Set the DMA mask to 64-bit. All supported controllers as
capable
> of @@ -3380,6 +3388,86 @@ megaraid_mbox_flush_cache(adapter_t *ada
>
>
> /**
> + * megaraid_mbox_fire_sync_cmd - fire the sync cmd
> + * @param adapter : soft state for the controller
> + */
> +static int
> +megaraid_mbox_fire_sync_cmd(adapter_t *adapter) {
> + mbox_t *mbox;
> + uint8_t raw_mbox[sizeof(mbox_t)];
> + mraid_device_t *raid_dev = ADAP2RAIDDEV(adapter);
> + mbox64_t *mbox64;
> + uint8_t status = 0;
> + int i;
> + uint32_t dword;
> +
> + mbox = (mbox_t *)raw_mbox;
> +
> + memset((caddr_t)raw_mbox, 0, sizeof(mbox_t));
> +
> + raw_mbox[0] = 0xFF;
> +
> + mbox64 = raid_dev->mbox64;
> + mbox = raid_dev->mbox;
> +
> + /*
> + * Wait until mailbox is free
> + */
> + if (megaraid_busywait_mbox(raid_dev) != 0) {
> + status = 1;
> + goto blocked_mailbox;
> + }
> +
> + /*
> + * Copy mailbox data into host structure
> + */
> + memcpy((caddr_t)mbox, (caddr_t)raw_mbox, 16);
> + mbox->cmdid = 0xFE;
> + mbox->busy = 1;
> + mbox->poll = 0;
> + mbox->ack = 0;
> + mbox->numstatus = 0;
> + mbox->status = 0;
> +
> + wmb();
> + WRINDOOR(raid_dev, raid_dev->mbox_dma | 0x1);
> +
> + // wait for maximum 1 min for status to post.
> + // If the Firmware SUPPORTS the ABOVE COMMAND,
> + // mbox->cmd will be set to 0
> + // else
> + // the firmware will reject the command with
> + // mbox->numstatus set to 1

Don't use // comment style. Also, for multi-line comments in Linux,
please use this preferred style:

/*
* This is the preferred style for multi-line
* comments in the Linux kernel source code.
* Please use it consistently.
*
* Description: A column of asterisks on the left side,
* with beginning and ending almost-blank lines.
*/

Thanks.
---
~Randy