[PATCH 3/4] mvsas: Set Hardware Bit for TMF

[PATCH 3/4] mvsas: Set Hardware Bit for TMF

[Ying Chu]

>From a939392afc59960b4a9bd8244e2e8fe43b7016f6 Mon Sep 17 00:00:00 2001
From: Andy Yan <ayan@marvell.com>
Date: Mon, 4 May 2009 23:33:58 +0800
Subject: [PATCH 3/4] bug fix: task frame type setting

Set SSP Task Frame for TMF task.

Signed-off-by: Ying Chu <jasonchu@marvell.com>
Signed-off-by: Andy Yan <ayan@marvell.com>
Signed-off-by: Ke Wei <kewei@marvell.com>
---
 drivers/scsi/mvsas/mv_sas.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/mvsas/mv_sas.c b/drivers/scsi/mvsas/mv_sas.c
index cb002ef..83d5465 100644
--- a/drivers/scsi/mvsas/mv_sas.c
+++ b/drivers/scsi/mvsas/mv_sas.c
@@ -761,9 +761,9 @@ static int mvs_task_prep_ssp(struct mvs_info *mvi,
 		flags |= MCH_FBURST;
 		fburst = (1 << 7);
 	}
-	hdr->flags = cpu_to_le32(flags |
-				 (tei->n_elem << MCH_PRD_LEN_SHIFT) |
-				 (MCH_SSP_FR_CMD << MCH_SSP_FR_TYPE_SHIFT));
+	if (is_tmf)
+		flags |= (MCH_SSP_FR_TASK << MCH_SSP_FR_TYPE_SHIFT);
+	hdr->flags = cpu_to_le32(flags | (tei->n_elem << MCH_PRD_LEN_SHIFT));
 	hdr->tags = cpu_to_le32(tag);
 	hdr->data_len = cpu_to_le32(task->total_xfer_len);
 
-- 
1.6.0.3

-- 
Regards,
Ying Chu

Re: [PATCH 3/4] mvsas: Set Hardware Bit for TMF

[James Bottomley]

On Fri, 2009-05-08 at 14:17 +0800, Ying Chu wrote:
> -	hdr->flags = cpu_to_le32(flags |
> -				 (tei->n_elem << MCH_PRD_LEN_SHIFT) |
> -				 (MCH_SSP_FR_CMD << MCH_SSP_FR_TYPE_SHIFT));
> +	if (is_tmf)
> +		flags |= (MCH_SSP_FR_TASK << MCH_SSP_FR_TYPE_SHIFT);
> +	hdr->flags = cpu_to_le32(flags | (tei->n_elem << MCH_PRD_LEN_SHIFT));

If is_tmf isn't set, shouldn't we be setting the command frame
indicator, like the old code was? (I know it's sort of irrelevant
because MCH_SSP_FR_CMD is defined to be zero, so if you're just dumping
the definition MCH_SSP_FR_CMD, that's fine too).

James

Re: [PATCH 3/4] mvsas: Set Hardware Bit for TMF

[Jeff Garzik]

James Bottomley wrote:
> On Fri, 2009-05-08 at 14:17 +0800, Ying Chu wrote:
>> -	hdr->flags = cpu_to_le32(flags |
>> -				 (tei->n_elem << MCH_PRD_LEN_SHIFT) |
>> -				 (MCH_SSP_FR_CMD << MCH_SSP_FR_TYPE_SHIFT));
>> +	if (is_tmf)
>> +		flags |= (MCH_SSP_FR_TASK << MCH_SSP_FR_TYPE_SHIFT);
>> +	hdr->flags = cpu_to_le32(flags | (tei->n_elem << MCH_PRD_LEN_SHIFT));
> 
> If is_tmf isn't set, shouldn't we be setting the command frame
> indicator, like the old code was? (I know it's sort of irrelevant
> because MCH_SSP_FR_CMD is defined to be zero, so if you're just dumping
> the definition MCH_SSP_FR_CMD, that's fine too).

Yeah, it's largely a matter of taste.

But even if MCH_SSP_FR_CMD is never used, its definition should remain, 
because the hardware documentation is not public.

	Jeff

Re: [PATCH 3/4] mvsas: Set Hardware Bit for TMF

[Jeff Garzik]

Ying Chu wrote:
>>From a939392afc59960b4a9bd8244e2e8fe43b7016f6 Mon Sep 17 00:00:00 2001
> From: Andy Yan <ayan@marvell.com>
> Date: Mon, 4 May 2009 23:33:58 +0800
> Subject: [PATCH 3/4] bug fix: task frame type setting
> 
> Set SSP Task Frame for TMF task.
> 
> Signed-off-by: Ying Chu <jasonchu@marvell.com>
> Signed-off-by: Andy Yan <ayan@marvell.com>
> Signed-off-by: Ke Wei <kewei@marvell.com>

Acked-by: Jeff Garzik <jgarzik@redhat.com>