mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* RE: [PATCH 1/1] megaraid_{mm,mbox}: updated fix-a-bug-in-reset-handler
@ 2006-05-02 20:04 Ju, Seokmann
  2006-05-03 13:21 ` Andrew Morton
  0 siblings, 1 reply; 4+ messages in thread
From: Ju, Seokmann @ 2006-05-02 20:04 UTC (permalink / raw)
  To: Ju, Seokmann, Andrew Morton, andre, James.Bottomley
  Cc: linux-kernel, linux-scsi

Hi Andrew,

Is this patch accepted?
If not, can you please provide comment.

Thank you,

> -----Original Message-----
> From: Ju, Seokmann 
> Sent: Tuesday, April 18, 2006 4:52 PM
> To: 'Andrew Morton'; andre@linux-ide.org; James.Bottomley@SteelEye.com
> Cc: Ju, Seokmann; linux-kernel@vger.kernel.org; 
> linux-scsi@vger.kernel.org
> Subject: [PATCH 1/1] megaraid_{mm,mbox}: updated 
> fix-a-bug-in-reset-handler
> 
> Hi,
> 
> This patch has created against 2.6.17-rc1-mm3.
> 
> The patch contains changes addressing following concerns 
> brought by previous megaraid_mmmbox-fix-a-bug-in-reset-handler.patch.
> 
> 1.Andrew Morton:
> 	> +			scb->status = -EFAULT;
> 
> 	What is the significance of -EFAULT here?  Seems inappropriate?
> 2. Andrew Morton:
> 	And if that mbox is in main memory, the duration of 
> this spin will vary by
> 	a factor of many tens across all the different machines 
> on which this
> 	driver must operate.
> 
> 	Careful use of ndelay() or udelay() would fix that.
> 
> Thank you,
> 
> Seokmann
> 
> Signed-Off By: Seokmann Ju <seokmann.ju@lsil.com>
> ---
> diff -Naur old/drivers/scsi/megaraid/megaraid_mbox.c 
> new/drivers/scsi/megaraid/megaraid_mbox.c
> --- old/drivers/scsi/megaraid/megaraid_mbox.c	2006-04-18 
> 17:17:06.288025720 -0400
> +++ new/drivers/scsi/megaraid/megaraid_mbox.c	2006-04-13 
> 16:46:53.000000000 -0400
> @@ -2278,6 +2278,7 @@
>  	unsigned long		flags;
>  	uint8_t			c;
>  	int			status;
> +	uioc_t			*kioc;
>  
>  
>  	if (!adapter) return;
> @@ -2320,6 +2321,9 @@
>  			// remove from local clist
>  			list_del_init(&scb->list);
>  
> +			kioc			= (uioc_t *)scb->gp;
> +			kioc->status		= 0;
> +
>  			megaraid_mbox_mm_done(adapter, scb);
>  
>  			continue;
> @@ -2636,6 +2640,7 @@
>  	int		recovery_window;
>  	int		recovering;
>  	int		i;
> +	uioc_t		*kioc;
>  
>  	adapter		= SCP2ADAPTER(scp);
>  	raid_dev	= ADAP2RAIDDEV(adapter);
> @@ -2662,7 +2667,10 @@
>  			"megaraid: IOCTL packet with %d[%d:%d] 
> being reset\n",
>  			scb->sno, scb->dev_channel, scb->dev_target));
>  
> -			scb->status = -EFAULT;
> +			scb->status = -1;
> +
> +			kioc			= (uioc_t *)scb->gp;
> +			kioc->status		= -EFAULT;
>  
>  			megaraid_mbox_mm_done(adapter, scb);
>  		} else {
> @@ -2933,12 +2941,13 @@
>  	wmb();
>  	WRINDOOR(raid_dev, raid_dev->mbox_dma | 0x1);
>  
> -	for (i = 0; i < 0xFFFFFF; i++) {
> +	for (i = 0; i < MBOX_SYNC_WAIT_CNT; i++) {
>  		if (mbox->numstatus != 0xFF) break;
>  		rmb();
> +		udelay(MBOX_SYNC_DELAY_200);
>  	}
>  
> -	if (i == 0xFFFFFF) {
> +	if (i == MBOX_SYNC_WAIT_CNT) {
>  		// We may need to re-calibrate the counter
>  		con_log(CL_ANN, (KERN_CRIT
>  			"megaraid: fast sync command timed out\n"));
> @@ -3717,7 +3726,6 @@
>  	unsigned long		flags;
>  
>  	kioc			= (uioc_t *)scb->gp;
> -	kioc->status		= 0;
>  	mbox64			= (mbox64_t *)(unsigned 
> long)kioc->cmdbuf;
>  	mbox64->mbox32.status	= scb->status;
>  	raw_mbox		= (uint8_t *)&mbox64->mbox32;
> diff -Naur old/drivers/scsi/megaraid/megaraid_mbox.h 
> new/drivers/scsi/megaraid/megaraid_mbox.h
> --- old/drivers/scsi/megaraid/megaraid_mbox.h	2006-04-18 
> 17:17:06.289025568 -0400
> +++ new/drivers/scsi/megaraid/megaraid_mbox.h	2006-04-13 
> 16:05:30.000000000 -0400
> @@ -100,6 +100,9 @@
>  #define MBOX_BUSY_WAIT		10	// max usec to 
> wait for busy mailbox
>  #define MBOX_RESET_WAIT		180	// wait these 
> many seconds in reset
>  #define MBOX_RESET_EXT_WAIT	120	// extended wait reset
> +#define MBOX_SYNC_WAIT_CNT	0xFFFF	// wait loop index for 
> synchronous mode
> +
> +#define MBOX_SYNC_DELAY_200	200	// 200 micro-seconds
>  
>  /*
>   * maximum transfer that can happen through the firmware 
> commands issued
> ---
> 

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

* Re: [PATCH 1/1] megaraid_{mm,mbox}: updated fix-a-bug-in-reset-handler
  2006-05-02 20:04 [PATCH 1/1] megaraid_{mm,mbox}: updated fix-a-bug-in-reset-handler Ju, Seokmann
@ 2006-05-03 13:21 ` Andrew Morton
  2006-05-03 13:55   ` James Bottomley
  0 siblings, 1 reply; 4+ messages in thread
From: Andrew Morton @ 2006-05-03 13:21 UTC (permalink / raw)
  To: Ju, Seokmann
  Cc: Seokmann.Ju, andre, James.Bottomley, linux-kernel, linux-scsi

On Tue, 2 May 2006 14:04:06 -0600
"Ju, Seokmann" <Seokmann.Ju@lsil.com> wrote:

> Is this patch accepted?

James is the scsi-patch-accepting guy.  I do pick up lots of patches which
belong to subsystem maintainers (mainly because they're lossy...) but they
still go through the subsytem maintainer.

> If not, can you please provide comment.

I don't appear to have a copy queued up, and I'm struggling a bit to
identify the right patch (partly because your email client appears to
support neither In-Reply-To: nor References:)

So.   Sorry, please resend.

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

* Re: [PATCH 1/1] megaraid_{mm,mbox}: updated fix-a-bug-in-reset-handler
  2006-05-03 13:21 ` Andrew Morton
@ 2006-05-03 13:55   ` James Bottomley
  0 siblings, 0 replies; 4+ messages in thread
From: James Bottomley @ 2006-05-03 13:55 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Ju, Seokmann, Seokmann.Ju, andre, linux-kernel, linux-scsi

On Wed, 2006-05-03 at 06:21 -0700, Andrew Morton wrote:
> On Tue, 2 May 2006 14:04:06 -0600
> "Ju, Seokmann" <Seokmann.Ju@lsil.com> wrote:
> 
> > Is this patch accepted?
> 
> James is the scsi-patch-accepting guy.  I do pick up lots of patches which
> belong to subsystem maintainers (mainly because they're lossy...) but they
> still go through the subsytem maintainer.
> 
> > If not, can you please provide comment.
> 
> I don't appear to have a copy queued up, and I'm struggling a bit to
> identify the right patch (partly because your email client appears to
> support neither In-Reply-To: nor References:)

This is it, isn't it?

http://www.kernel.org/git/?p=linux/kernel/git/jejb/scsi-rc-fixes-2.6.git;a=commit;h=c005fb4fb2d23ba29ad21dee5042b2f8451ca8ba

James



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

* [PATCH 1/1] megaraid_{mm,mbox}: updated fix-a-bug-in-reset-handler
@ 2006-04-18 20:52 Ju, Seokmann
  0 siblings, 0 replies; 4+ messages in thread
From: Ju, Seokmann @ 2006-04-18 20:52 UTC (permalink / raw)
  To: Andrew Morton, andre, James.Bottomley
  Cc: Ju, Seokmann, linux-kernel, linux-scsi

[-- Attachment #1: Type: text/plain, Size: 3120 bytes --]

Hi,

This patch has created against 2.6.17-rc1-mm3.

The patch contains changes addressing following concerns brought by
previous megaraid_mmmbox-fix-a-bug-in-reset-handler.patch.

1.Andrew Morton:
	> +			scb->status = -EFAULT;

	What is the significance of -EFAULT here?  Seems inappropriate?
2. Andrew Morton:
	And if that mbox is in main memory, the duration of this spin
will vary by
	a factor of many tens across all the different machines on which
this
	driver must operate.

	Careful use of ndelay() or udelay() would fix that.

Thank you,

Seokmann

Signed-Off By: Seokmann Ju <seokmann.ju@lsil.com>
---
diff -Naur old/drivers/scsi/megaraid/megaraid_mbox.c
new/drivers/scsi/megaraid/megaraid_mbox.c
--- old/drivers/scsi/megaraid/megaraid_mbox.c	2006-04-18
17:17:06.288025720 -0400
+++ new/drivers/scsi/megaraid/megaraid_mbox.c	2006-04-13
16:46:53.000000000 -0400
@@ -2278,6 +2278,7 @@
 	unsigned long		flags;
 	uint8_t			c;
 	int			status;
+	uioc_t			*kioc;
 
 
 	if (!adapter) return;
@@ -2320,6 +2321,9 @@
 			// remove from local clist
 			list_del_init(&scb->list);
 
+			kioc			= (uioc_t *)scb->gp;
+			kioc->status		= 0;
+
 			megaraid_mbox_mm_done(adapter, scb);
 
 			continue;
@@ -2636,6 +2640,7 @@
 	int		recovery_window;
 	int		recovering;
 	int		i;
+	uioc_t		*kioc;
 
 	adapter		= SCP2ADAPTER(scp);
 	raid_dev	= ADAP2RAIDDEV(adapter);
@@ -2662,7 +2667,10 @@
 			"megaraid: IOCTL packet with %d[%d:%d] being
reset\n",
 			scb->sno, scb->dev_channel, scb->dev_target));
 
-			scb->status = -EFAULT;
+			scb->status = -1;
+
+			kioc			= (uioc_t *)scb->gp;
+			kioc->status		= -EFAULT;
 
 			megaraid_mbox_mm_done(adapter, scb);
 		} else {
@@ -2933,12 +2941,13 @@
 	wmb();
 	WRINDOOR(raid_dev, raid_dev->mbox_dma | 0x1);
 
-	for (i = 0; i < 0xFFFFFF; i++) {
+	for (i = 0; i < MBOX_SYNC_WAIT_CNT; i++) {
 		if (mbox->numstatus != 0xFF) break;
 		rmb();
+		udelay(MBOX_SYNC_DELAY_200);
 	}
 
-	if (i == 0xFFFFFF) {
+	if (i == MBOX_SYNC_WAIT_CNT) {
 		// We may need to re-calibrate the counter
 		con_log(CL_ANN, (KERN_CRIT
 			"megaraid: fast sync command timed out\n"));
@@ -3717,7 +3726,6 @@
 	unsigned long		flags;
 
 	kioc			= (uioc_t *)scb->gp;
-	kioc->status		= 0;
 	mbox64			= (mbox64_t *)(unsigned
long)kioc->cmdbuf;
 	mbox64->mbox32.status	= scb->status;
 	raw_mbox		= (uint8_t *)&mbox64->mbox32;
diff -Naur old/drivers/scsi/megaraid/megaraid_mbox.h
new/drivers/scsi/megaraid/megaraid_mbox.h
--- old/drivers/scsi/megaraid/megaraid_mbox.h	2006-04-18
17:17:06.289025568 -0400
+++ new/drivers/scsi/megaraid/megaraid_mbox.h	2006-04-13
16:05:30.000000000 -0400
@@ -100,6 +100,9 @@
 #define MBOX_BUSY_WAIT		10	// max usec to wait for busy
mailbox
 #define MBOX_RESET_WAIT		180	// wait these many
seconds in reset
 #define MBOX_RESET_EXT_WAIT	120	// extended wait reset
+#define MBOX_SYNC_WAIT_CNT	0xFFFF	// wait loop index for
synchronous mode
+
+#define MBOX_SYNC_DELAY_200	200	// 200 micro-seconds
 
 /*
  * maximum transfer that can happen through the firmware commands
issued
---

[-- Attachment #2: megaraid_mm_mbox_II.patch --]
[-- Type: application/octet-stream, Size: 2387 bytes --]

diff -Naur old/drivers/scsi/megaraid/megaraid_mbox.c new/drivers/scsi/megaraid/megaraid_mbox.c
--- old/drivers/scsi/megaraid/megaraid_mbox.c	2006-04-18 17:17:06.288025720 -0400
+++ new/drivers/scsi/megaraid/megaraid_mbox.c	2006-04-13 16:46:53.000000000 -0400
@@ -2278,6 +2278,7 @@
 	unsigned long		flags;
 	uint8_t			c;
 	int			status;
+	uioc_t			*kioc;
 
 
 	if (!adapter) return;
@@ -2320,6 +2321,9 @@
 			// remove from local clist
 			list_del_init(&scb->list);
 
+			kioc			= (uioc_t *)scb->gp;
+			kioc->status		= 0;
+
 			megaraid_mbox_mm_done(adapter, scb);
 
 			continue;
@@ -2636,6 +2640,7 @@
 	int		recovery_window;
 	int		recovering;
 	int		i;
+	uioc_t		*kioc;
 
 	adapter		= SCP2ADAPTER(scp);
 	raid_dev	= ADAP2RAIDDEV(adapter);
@@ -2662,7 +2667,10 @@
 			"megaraid: IOCTL packet with %d[%d:%d] being reset\n",
 			scb->sno, scb->dev_channel, scb->dev_target));
 
-			scb->status = -EFAULT;
+			scb->status = -1;
+
+			kioc			= (uioc_t *)scb->gp;
+			kioc->status		= -EFAULT;
 
 			megaraid_mbox_mm_done(adapter, scb);
 		} else {
@@ -2933,12 +2941,13 @@
 	wmb();
 	WRINDOOR(raid_dev, raid_dev->mbox_dma | 0x1);
 
-	for (i = 0; i < 0xFFFFFF; i++) {
+	for (i = 0; i < MBOX_SYNC_WAIT_CNT; i++) {
 		if (mbox->numstatus != 0xFF) break;
 		rmb();
+		udelay(MBOX_SYNC_DELAY_200);
 	}
 
-	if (i == 0xFFFFFF) {
+	if (i == MBOX_SYNC_WAIT_CNT) {
 		// We may need to re-calibrate the counter
 		con_log(CL_ANN, (KERN_CRIT
 			"megaraid: fast sync command timed out\n"));
@@ -3717,7 +3726,6 @@
 	unsigned long		flags;
 
 	kioc			= (uioc_t *)scb->gp;
-	kioc->status		= 0;
 	mbox64			= (mbox64_t *)(unsigned long)kioc->cmdbuf;
 	mbox64->mbox32.status	= scb->status;
 	raw_mbox		= (uint8_t *)&mbox64->mbox32;
diff -Naur old/drivers/scsi/megaraid/megaraid_mbox.h new/drivers/scsi/megaraid/megaraid_mbox.h
--- old/drivers/scsi/megaraid/megaraid_mbox.h	2006-04-18 17:17:06.289025568 -0400
+++ new/drivers/scsi/megaraid/megaraid_mbox.h	2006-04-13 16:05:30.000000000 -0400
@@ -100,6 +100,9 @@
 #define MBOX_BUSY_WAIT		10	// max usec to wait for busy mailbox
 #define MBOX_RESET_WAIT		180	// wait these many seconds in reset
 #define MBOX_RESET_EXT_WAIT	120	// extended wait reset
+#define MBOX_SYNC_WAIT_CNT	0xFFFF	// wait loop index for synchronous mode
+
+#define MBOX_SYNC_DELAY_200	200	// 200 micro-seconds
 
 /*
  * maximum transfer that can happen through the firmware commands issued

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

end of thread, other threads:[~2006-05-03 13:55 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-05-02 20:04 [PATCH 1/1] megaraid_{mm,mbox}: updated fix-a-bug-in-reset-handler Ju, Seokmann
2006-05-03 13:21 ` Andrew Morton
2006-05-03 13:55   ` James Bottomley
  -- strict thread matches above, loose matches on Subject: below --
2006-04-18 20:52 Ju, Seokmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®