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
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
