Re: [PATCH 1/4] scsi: ufs: Fix broken task management command implementation

[Sujit Reddy Thumma <sthumma@codeaurora.org>]

On 7/2/2013 9:21 PM, Santosh Y wrote:
> On Fri, Jun 28, 2013 at 5:02 PM, Sujit Reddy Thumma
> <sthumma@codeaurora.org> wrote:
>> On 6/27/2013 4:49 PM, Santosh Y wrote:
>>>>
>>>>> +       spin_lock_irqsave(host->host_lock, flags);
>>>>>          task_req_descp = hba->utmrdl_base_addr;
>>>>>          task_req_descp += free_slot;
>>>>>
>>>>> @@ -2353,38 +2387,39 @@ ufshcd_issue_tm_cmd(struct ufs_hba *hba,
>>>>>                  (struct utp_upiu_task_req *)
>>>>> task_req_descp->task_req_upiu;
>>>>>          task_req_upiup->header.dword_0 =
>>>>>                  UPIU_HEADER_DWORD(UPIU_TRANSACTION_TASK_REQ, 0,
>>>>> -                                             lrbp->lun,
>>>>> lrbp->task_tag);
>>>>> +                               lun_id, free_slot);
>>>
>>> Actually it still doesn't fix the problem. The*task tag*  used here
>>>
>>> should be unique across the SCSI/Query and Task Managment UPIUs.
>>
>>
>> I am sorry, I didn't get that. Why should it be unique across the
>> SCSI/Query? For example, if a machine supports 32 request slots and 8 task
>> management slots, then the task management command tag can be anything out
>> of 8 slots.
>>
>
> The spec(ufs 1.1) has the requirement under  '10.5.2 Basic Header
> Format'->'Task Tag'.
> Couple of devices I came across had similar behavior. The tracking of
> UPIUs --even belonging to a separate group-- seemed to be based on the
> 'task tag' value rather than 'type of UPIU'->'task tag'.

Thanks for the clarification. So to make the task tag unique we should 
do something like below:

@@ -2940,9 +2941,10 @@ static int ufshcd_issue_tm_cmd(struct ufs_hba 
*hba, int lun_id, int task_id,
         /* Configure task request UPIU */
         task_req_upiup =
                 (struct utp_upiu_task_req *) task_req_descp->task_req_upiu;
+       task_tag = hba->nutrs + free_slot;
         task_req_upiup->header.dword_0 =
                 UPIU_HEADER_DWORD(UPIU_TRANSACTION_TASK_REQ, 0,
-                               lun_id, free_slot);
+                               lun_id, task_tag);

Will this work for the devices you came across?


-- 
Regards,
Sujit