[PATCH RESEND 2/2] scsi/ufs: Fix evaluation of task_failed status

[PATCH RESEND 2/2] scsi/ufs: Fix evaluation of task_failed status

[Venkatraman S]

Else FAILED would be set even if task_result was
originally equal to UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED.

Reported-by: Joe Perches <joe@perches.com>
Signed-off-by: Venkatraman S <svenkatr@ti.com>
Reviewed-by: Namjae Jeon <linkinjeon@gmail.com>
Acked-by: Santosh Y <santoshsy@gmail.com>
CC: Andrew Morton <akpm@linux-foundation.org>
---
 drivers/scsi/ufs/ufshcd.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index aa97200..cd59f6f 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1160,7 +1160,7 @@ static int ufshcd_task_req_compl(struct ufs_hba *hba, u32 index)
 		task_result = be32_to_cpu(task_rsp_upiup->header.dword_1);
 		task_result = ((task_result & MASK_TASK_RESPONSE) >> 8);
 
-		if (task_result != UPIU_TASK_MANAGEMENT_FUNC_COMPL ||
+		if (task_result != UPIU_TASK_MANAGEMENT_FUNC_COMPL &&
 		    task_result != UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED)
 			task_result = FAILED;
 	} else {
-- 
1.7.5.4

Re: [PATCH RESEND 2/2] scsi/ufs: Fix evaluation of task_failed status

[Andrew Morton]

Please cc James and linux-scsi on drivers/scsi patches?

On Wed, 18 Apr 2012 13:08:53 +0530
Venkatraman S <svenkatr@ti.com> wrote:

> Else FAILED would be set even if task_result was
> originally equal to UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED.

This changelog is inadequate.  It failed to describe the end-user
impact of the bug.  Without this information we cannot decide which
kernel version(s) should be patched.  Please always fully describe a
bug when fixing it.

> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -1160,7 +1160,7 @@ static int ufshcd_task_req_compl(struct ufs_hba *hba, u32 index)
>  		task_result = be32_to_cpu(task_rsp_upiup->header.dword_1);
>  		task_result = ((task_result & MASK_TASK_RESPONSE) >> 8);
>  
> -		if (task_result != UPIU_TASK_MANAGEMENT_FUNC_COMPL ||
> +		if (task_result != UPIU_TASK_MANAGEMENT_FUNC_COMPL &&
>  		    task_result != UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED)
>  			task_result = FAILED;
>  	} else {

And the title of the patch is poor.  There is no "task_failed" here. 
Something like "fix evaluation of task completion code" would be
better?

Re: [PATCH RESEND 2/2] scsi/ufs: Fix evaluation of task_failed status

[S, Venkatraman]

On Thu, Apr 19, 2012 at 4:26 AM, Andrew Morton
<akpm@linux-foundation.org> wrote:
>
> Please cc James and linux-scsi on drivers/scsi patches?
>
> On Wed, 18 Apr 2012 13:08:53 +0530
> Venkatraman S <svenkatr@ti.com> wrote:
>
>> Else FAILED would be set even if task_result was
>> originally equal to UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED.
>
> This changelog is inadequate.  It failed to describe the end-user
> impact of the bug.  Without this information we cannot decide which
> kernel version(s) should be patched.  Please always fully describe a
> bug when fixing it.
>
>> --- a/drivers/scsi/ufs/ufshcd.c
>> +++ b/drivers/scsi/ufs/ufshcd.c
>> @@ -1160,7 +1160,7 @@ static int ufshcd_task_req_compl(struct ufs_hba *hba, u32 index)
>>               task_result = be32_to_cpu(task_rsp_upiup->header.dword_1);
>>               task_result = ((task_result & MASK_TASK_RESPONSE) >> 8);
>>
>> -             if (task_result != UPIU_TASK_MANAGEMENT_FUNC_COMPL ||
>> +             if (task_result != UPIU_TASK_MANAGEMENT_FUNC_COMPL &&
>>                   task_result != UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED)
>>                       task_result = FAILED;
>>       } else {
>
> And the title of the patch is poor.  There is no "task_failed" here.
> Something like "fix evaluation of task completion code" would be
> better?

Ok. I will send an updated version of the patch.
Regards,
Venkat.