| Message ID | 1375163155-1473-2-git-send-email-sthumma@codeaurora.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
Tested-by: Dolev Raviv <draviv@codeaurora.org> > Currently, sending Task Management (TM) command to the card might > be broken in some scenarios as listed below: > > Problem: If there are more than 8 TM commands the implementation > returns error to the caller. > Fix: Wait for one of the slots to be emptied and send the command. > > Problem: Sometimes it is necessary for the caller to know the TM service > response code to determine the task status. > Fix: Propogate the service response to the caller. > > Problem: If the TM command times out no proper error recovery is > implemented. > Fix: Clear the command in the controller door-bell register, so that > further commands for the same slot don't fail. > > Problem: While preparing the TM command descriptor, the task tag used > should be unique across SCSI/NOP/QUERY/TM commands and not the > task tag of the command which the TM command is trying to manage. > Fix: Use a unique task tag instead of task tag of SCSI command. > > Problem: Since the TM command involves H/W communication, abruptly ending > the request on kill interrupt signal might cause h/w malfunction. > Fix: Wait for hardware completion interrupt with TASK_UNINTERRUPTIBLE > set. > > Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org> > --- > drivers/scsi/ufs/ufshcd.c | 173 > ++++++++++++++++++++++++++++++--------------- > drivers/scsi/ufs/ufshcd.h | 8 ++- > 2 files changed, 122 insertions(+), 59 deletions(-) > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > index b36ca9a..d7f3746 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -53,6 +53,9 @@ > /* Query request timeout */ > #define QUERY_REQ_TIMEOUT 30 /* msec */ > > +/* Task management command timeout */ > +#define TM_CMD_TIMEOUT 100 /* msecs */ > + > /* Expose the flag value from utp_upiu_query.value */ > #define MASK_QUERY_UPIU_FLAG_LOC 0xFF > > @@ -183,13 +186,35 @@ ufshcd_get_tmr_ocs(struct utp_task_req_desc > *task_req_descp) > /** > * ufshcd_get_tm_free_slot - get a free slot for task management request > * @hba: per adapter instance > + * @free_slot: pointer to variable with available slot value > * > - * Returns maximum number of task management request slots in case of > - * task management queue full or returns the free slot number > + * Get a free tag and lock it until ufshcd_put_tm_slot() is called. > + * Returns 0 if free slot is not available, else return 1 with tag value > + * in @free_slot. > */ > -static inline int ufshcd_get_tm_free_slot(struct ufs_hba *hba) > +static bool ufshcd_get_tm_free_slot(struct ufs_hba *hba, int *free_slot) > { > - return find_first_zero_bit(&hba->outstanding_tasks, hba->nutmrs); > + int tag; > + bool ret = false; > + > + if (!free_slot) > + goto out; > + > + do { > + tag = find_first_zero_bit(&hba->tm_slots_in_use, hba->nutmrs); > + if (tag >= hba->nutmrs) > + goto out; > + } while (test_and_set_bit_lock(tag, &hba->tm_slots_in_use)); > + > + *free_slot = tag; > + ret = true; > +out: > + return ret; > +} > + > +static inline void ufshcd_put_tm_slot(struct ufs_hba *hba, int slot) > +{ > + clear_bit_unlock(slot, &hba->tm_slots_in_use); > } > > /** > @@ -1700,10 +1725,11 @@ static void ufshcd_slave_destroy(struct > scsi_device *sdev) > * ufshcd_task_req_compl - handle task management request completion > * @hba: per adapter instance > * @index: index of the completed request > + * @resp: task management service response > * > - * Returns SUCCESS/FAILED > + * Returns non-zero value on error, zero on success > */ > -static int ufshcd_task_req_compl(struct ufs_hba *hba, u32 index) > +static int ufshcd_task_req_compl(struct ufs_hba *hba, u32 index, u8 > *resp) > { > struct utp_task_req_desc *task_req_descp; > struct utp_upiu_task_rsp *task_rsp_upiup; > @@ -1724,19 +1750,15 @@ static int ufshcd_task_req_compl(struct ufs_hba > *hba, u32 index) > task_req_descp[index].task_rsp_upiu; > 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 && > - task_result != UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED) > - task_result = FAILED; > - else > - task_result = SUCCESS; > + if (resp) > + *resp = (u8)task_result; > } else { > - task_result = FAILED; > - dev_err(hba->dev, > - "trc: Invalid ocs = %x\n", ocs_value); > + dev_err(hba->dev, "%s: failed, ocs = 0x%x\n", > + __func__, ocs_value); > } > spin_unlock_irqrestore(hba->host->host_lock, flags); > - return task_result; > + > + return ocs_value; > } > > /** > @@ -2237,7 +2259,7 @@ static void ufshcd_tmc_handler(struct ufs_hba *hba) > > tm_doorbell = ufshcd_readl(hba, REG_UTP_TASK_REQ_DOOR_BELL); > hba->tm_condition = tm_doorbell ^ hba->outstanding_tasks; > - wake_up_interruptible(&hba->ufshcd_tm_wait_queue); > + wake_up(&hba->tm_wq); > } > > /** > @@ -2287,38 +2309,58 @@ static irqreturn_t ufshcd_intr(int irq, void > *__hba) > return retval; > } > > +static int ufshcd_clear_tm_cmd(struct ufs_hba *hba, int tag) > +{ > + int err = 0; > + u32 mask = 1 << tag; > + unsigned long flags; > + > + if (!test_bit(tag, &hba->outstanding_reqs)) > + goto out; > + > + spin_lock_irqsave(hba->host->host_lock, flags); > + ufshcd_writel(hba, ~(1 << tag), REG_UTP_TASK_REQ_LIST_CLEAR); > + spin_unlock_irqrestore(hba->host->host_lock, flags); > + > + /* poll for max. 1 sec to clear door bell register by h/w */ > + err = ufshcd_wait_for_register(hba, > + REG_UTP_TASK_REQ_DOOR_BELL, > + mask, 0, 1000, 1000); > +out: > + return err; > +} > + > /** > * ufshcd_issue_tm_cmd - issues task management commands to controller > * @hba: per adapter instance > - * @lrbp: pointer to local reference block > + * @lun_id: LUN ID to which TM command is sent > + * @task_id: task ID to which the TM command is applicable > + * @tm_function: task management function opcode > + * @tm_response: task management service response return value > * > - * Returns SUCCESS/FAILED > + * Returns non-zero value on error, zero on success. > */ > -static int > -ufshcd_issue_tm_cmd(struct ufs_hba *hba, > - struct ufshcd_lrb *lrbp, > - u8 tm_function) > +static int ufshcd_issue_tm_cmd(struct ufs_hba *hba, int lun_id, int > task_id, > + u8 tm_function, u8 *tm_response) > { > struct utp_task_req_desc *task_req_descp; > struct utp_upiu_task_req *task_req_upiup; > struct Scsi_Host *host; > unsigned long flags; > - int free_slot = 0; > + int free_slot; > int err; > + int task_tag; > > host = hba->host; > > - spin_lock_irqsave(host->host_lock, flags); > - > - /* If task management queue is full */ > - free_slot = ufshcd_get_tm_free_slot(hba); > - if (free_slot >= hba->nutmrs) { > - spin_unlock_irqrestore(host->host_lock, flags); > - dev_err(hba->dev, "Task management queue full\n"); > - err = FAILED; > - goto out; > - } > + /* > + * Get free slot, sleep if slots are unavailable. > + * Even though we use wait_event() which sleeps indefinitely, > + * the maximum wait time is bounded by %TM_CMD_TIMEOUT. > + */ > + wait_event(hba->tm_tag_wq, ufshcd_get_tm_free_slot(hba, &free_slot)); > > + spin_lock_irqsave(host->host_lock, flags); > task_req_descp = hba->utmrdl_base_addr; > task_req_descp += free_slot; > > @@ -2330,18 +2372,15 @@ ufshcd_issue_tm_cmd(struct ufs_hba *hba, > /* 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, > - lrbp->lun, lrbp->task_tag); > + lun_id, task_tag); > task_req_upiup->header.dword_1 = > UPIU_HEADER_DWORD(0, tm_function, 0, 0); > > - task_req_upiup->input_param1 = lrbp->lun; > - task_req_upiup->input_param1 = > - cpu_to_be32(task_req_upiup->input_param1); > - task_req_upiup->input_param2 = lrbp->task_tag; > - task_req_upiup->input_param2 = > - cpu_to_be32(task_req_upiup->input_param2); > + task_req_upiup->input_param1 = cpu_to_be32(lun_id); > + task_req_upiup->input_param2 = cpu_to_be32(task_id); > > /* send command to the controller */ > __set_bit(free_slot, &hba->outstanding_tasks); > @@ -2350,20 +2389,24 @@ ufshcd_issue_tm_cmd(struct ufs_hba *hba, > spin_unlock_irqrestore(host->host_lock, flags); > > /* wait until the task management command is completed */ > - err = > - wait_event_interruptible_timeout(hba->ufshcd_tm_wait_queue, > - (test_bit(free_slot, > - &hba->tm_condition) != 0), > - 60 * HZ); > + err = wait_event_timeout(hba->tm_wq, > + test_bit(free_slot, &hba->tm_condition), > + msecs_to_jiffies(TM_CMD_TIMEOUT)); > if (!err) { > - dev_err(hba->dev, > - "Task management command timed-out\n"); > - err = FAILED; > - goto out; > + dev_err(hba->dev, "%s: task management cmd 0x%.2x timed-out\n", > + __func__, tm_function); > + if (ufshcd_clear_tm_cmd(hba, free_slot)) > + dev_WARN(hba->dev, "%s: unable clear tm cmd (slot %d) after > timeout\n", > + __func__, free_slot); > + err = -ETIMEDOUT; > + } else { > + err = ufshcd_task_req_compl(hba, free_slot, tm_response); > } > + > clear_bit(free_slot, &hba->tm_condition); > - err = ufshcd_task_req_compl(hba, free_slot); > -out: > + ufshcd_put_tm_slot(hba, free_slot); > + wake_up(&hba->tm_tag_wq); > + > return err; > } > > @@ -2380,14 +2423,21 @@ static int ufshcd_device_reset(struct scsi_cmnd > *cmd) > unsigned int tag; > u32 pos; > int err; > + u8 resp = 0xF; > + struct ufshcd_lrb *lrbp; > > host = cmd->device->host; > hba = shost_priv(host); > tag = cmd->request->tag; > > - err = ufshcd_issue_tm_cmd(hba, &hba->lrb[tag], UFS_LOGICAL_RESET); > - if (err == FAILED) > + lrbp = &hba->lrb[tag]; > + err = ufshcd_issue_tm_cmd(hba, lrbp->lun, 0, UFS_LOGICAL_RESET, &resp); > + if (err || resp != UPIU_TASK_MANAGEMENT_FUNC_COMPL) { > + err = FAILED; > goto out; > + } else { > + err = SUCCESS; > + } > > for (pos = 0; pos < hba->nutrs; pos++) { > if (test_bit(pos, &hba->outstanding_reqs) && > @@ -2444,6 +2494,8 @@ static int ufshcd_abort(struct scsi_cmnd *cmd) > unsigned long flags; > unsigned int tag; > int err; > + u8 resp = 0xF; > + struct ufshcd_lrb *lrbp; > > host = cmd->device->host; > hba = shost_priv(host); > @@ -2459,9 +2511,15 @@ static int ufshcd_abort(struct scsi_cmnd *cmd) > } > spin_unlock_irqrestore(host->host_lock, flags); > > - err = ufshcd_issue_tm_cmd(hba, &hba->lrb[tag], UFS_ABORT_TASK); > - if (err == FAILED) > + lrbp = &hba->lrb[tag]; > + err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag, > + UFS_ABORT_TASK, &resp); > + if (err || resp != UPIU_TASK_MANAGEMENT_FUNC_COMPL) { > + err = FAILED; > goto out; > + } else { > + err = SUCCESS; > + } > > scsi_dma_unmap(cmd); > > @@ -2682,7 +2740,8 @@ int ufshcd_init(struct device *dev, struct ufs_hba > **hba_handle, > host->max_cmd_len = MAX_CDB_SIZE; > > /* Initailize wait queue for task management */ > - init_waitqueue_head(&hba->ufshcd_tm_wait_queue); > + init_waitqueue_head(&hba->tm_wq); > + init_waitqueue_head(&hba->tm_tag_wq); > > /* Initialize work queues */ > INIT_WORK(&hba->feh_workq, ufshcd_fatal_err_handler); > diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h > index 59c9c48..fe7c947 100644 > --- a/drivers/scsi/ufs/ufshcd.h > +++ b/drivers/scsi/ufs/ufshcd.h > @@ -174,8 +174,10 @@ struct ufs_dev_cmd { > * @irq: Irq number of the controller > * @active_uic_cmd: handle of active UIC command > * @uic_cmd_mutex: mutex for uic command > - * @ufshcd_tm_wait_queue: wait queue for task management > + * @tm_wq: wait queue for task management > + * @tm_tag_wq: wait queue for free task management slots > * @tm_condition: condition variable for task management > + * @tm_slots_in_use: bit map of task management request slots in use > * @ufshcd_state: UFSHCD states > * @intr_mask: Interrupt Mask Bits > * @ee_ctrl_mask: Exception event control mask > @@ -216,8 +218,10 @@ struct ufs_hba { > struct uic_command *active_uic_cmd; > struct mutex uic_cmd_mutex; > > - wait_queue_head_t ufshcd_tm_wait_queue; > + wait_queue_head_t tm_wq; > + wait_queue_head_t tm_tag_wq; > unsigned long tm_condition; > + unsigned long tm_slots_in_use; > > u32 ufshcd_state; > u32 intr_mask; > -- > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member > of Code Aurora Forum, hosted by The Linux Foundation. > > -- > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
Reviewed-by: Yaniv Gardi <ygardi@codeaurora.org>
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
= > -----Original Message-----
= > From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-
= > owner@vger.kernel.org] On Behalf Of Dolev Raviv
= > Sent: Monday, August 12, 2013 4:01 PM
= > To: Sujit Reddy Thumma
= > Cc: Vinayak Holikatti; Santosh Y; James E.J. Bottomley; linux-
= > scsi@vger.kernel.org; Sujit Reddy Thumma; linux-arm-
= > msm@vger.kernel.org
= > Subject: Re: [PATCH V5 1/4] scsi: ufs: Fix broken task management
= > command implementation
= >
= > Tested-by: Dolev Raviv <draviv@codeaurora.org>
= >
= > > Currently, sending Task Management (TM) command to the card might
= > be
= > > broken in some scenarios as listed below:
= > >
= > > Problem: If there are more than 8 TM commands the implementation
= > > returns error to the caller.
= > > Fix: Wait for one of the slots to be emptied and send the command.
= > >
= > > Problem: Sometimes it is necessary for the caller to know the TM
service
= > > response code to determine the task status.
= > > Fix: Propogate the service response to the caller.
= > >
= > > Problem: If the TM command times out no proper error recovery is
= > > implemented.
= > > Fix: Clear the command in the controller door-bell register, so
that
= > > further commands for the same slot don't fail.
= > >
= > > Problem: While preparing the TM command descriptor, the task tag used
= > > should be unique across SCSI/NOP/QUERY/TM commands and not
= > the
= > > task tag of the command which the TM command is trying to
= > manage.
= > > Fix: Use a unique task tag instead of task tag of SCSI command.
= > >
= > > Problem: Since the TM command involves H/W communication, abruptly
= > ending
= > > the request on kill interrupt signal might cause h/w
malfunction.
= > > Fix: Wait for hardware completion interrupt with
= > TASK_UNINTERRUPTIBLE
= > > set.
= > >
= > > Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org>
= > > ---
= > > drivers/scsi/ufs/ufshcd.c | 173
= > > ++++++++++++++++++++++++++++++---------------
= > > drivers/scsi/ufs/ufshcd.h | 8 ++-
= > > 2 files changed, 122 insertions(+), 59 deletions(-)
= > >
= > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
= > > index b36ca9a..d7f3746 100644
= > > --- a/drivers/scsi/ufs/ufshcd.c
= > > +++ b/drivers/scsi/ufs/ufshcd.c
= > > @@ -53,6 +53,9 @@
= > > /* Query request timeout */
= > > #define QUERY_REQ_TIMEOUT 30 /* msec */
= > >
= > > +/* Task management command timeout */
= > > +#define TM_CMD_TIMEOUT 100 /* msecs */
= > > +
= > > /* Expose the flag value from utp_upiu_query.value */ #define
= > > MASK_QUERY_UPIU_FLAG_LOC 0xFF
= > >
= > > @@ -183,13 +186,35 @@ ufshcd_get_tmr_ocs(struct utp_task_req_desc
= > > *task_req_descp)
= > > /**
= > > * ufshcd_get_tm_free_slot - get a free slot for task management
= > request
= > > * @hba: per adapter instance
= > > + * @free_slot: pointer to variable with available slot value
= > > *
= > > - * Returns maximum number of task management request slots in case
= > of
= > > - * task management queue full or returns the free slot number
= > > + * Get a free tag and lock it until ufshcd_put_tm_slot() is called.
= > > + * Returns 0 if free slot is not available, else return 1 with tag
= > > + value
= > > + * in @free_slot.
= > > */
= > > -static inline int ufshcd_get_tm_free_slot(struct ufs_hba *hba)
= > > +static bool ufshcd_get_tm_free_slot(struct ufs_hba *hba, int
= > > +*free_slot)
= > > {
= > > - return find_first_zero_bit(&hba->outstanding_tasks, hba->nutmrs);
= > > + int tag;
= > > + bool ret = false;
= > > +
= > > + if (!free_slot)
= > > + goto out;
= > > +
= > > + do {
= > > + tag = find_first_zero_bit(&hba->tm_slots_in_use, hba-
= > >nutmrs);
= > > + if (tag >= hba->nutmrs)
= > > + goto out;
= > > + } while (test_and_set_bit_lock(tag, &hba->tm_slots_in_use));
= > > +
= > > + *free_slot = tag;
= > > + ret = true;
= > > +out:
= > > + return ret;
= > > +}
= > > +
= > > +static inline void ufshcd_put_tm_slot(struct ufs_hba *hba, int slot)
= > > +{
= > > + clear_bit_unlock(slot, &hba->tm_slots_in_use);
= > > }
= > >
= > > /**
= > > @@ -1700,10 +1725,11 @@ static void ufshcd_slave_destroy(struct
= > > scsi_device *sdev)
= > > * ufshcd_task_req_compl - handle task management request
= > completion
= > > * @hba: per adapter instance
= > > * @index: index of the completed request
= > > + * @resp: task management service response
= > > *
= > > - * Returns SUCCESS/FAILED
= > > + * Returns non-zero value on error, zero on success
= > > */
= > > -static int ufshcd_task_req_compl(struct ufs_hba *hba, u32 index)
= > > +static int ufshcd_task_req_compl(struct ufs_hba *hba, u32 index, u8
= > > *resp)
= > > {
= > > struct utp_task_req_desc *task_req_descp;
= > > struct utp_upiu_task_rsp *task_rsp_upiup; @@ -1724,19 +1750,15
= > @@
= > > static int ufshcd_task_req_compl(struct ufs_hba *hba, u32 index)
= > > task_req_descp[index].task_rsp_upiu;
= > > 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 &&
= > > - task_result !=
= > UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED)
= > > - task_result = FAILED;
= > > - else
= > > - task_result = SUCCESS;
= > > + if (resp)
= > > + *resp = (u8)task_result;
= > > } else {
= > > - task_result = FAILED;
= > > - dev_err(hba->dev,
= > > - "trc: Invalid ocs = %x\n", ocs_value);
= > > + dev_err(hba->dev, "%s: failed, ocs = 0x%x\n",
= > > + __func__, ocs_value);
= > > }
= > > spin_unlock_irqrestore(hba->host->host_lock, flags);
= > > - return task_result;
= > > +
= > > + return ocs_value;
= > > }
= > >
= > > /**
= > > @@ -2237,7 +2259,7 @@ static void ufshcd_tmc_handler(struct ufs_hba
= > > *hba)
= > >
= > > tm_doorbell = ufshcd_readl(hba,
= > REG_UTP_TASK_REQ_DOOR_BELL);
= > > hba->tm_condition = tm_doorbell ^ hba->outstanding_tasks;
= > > - wake_up_interruptible(&hba->ufshcd_tm_wait_queue);
= > > + wake_up(&hba->tm_wq);
= > > }
= > >
= > > /**
= > > @@ -2287,38 +2309,58 @@ static irqreturn_t ufshcd_intr(int irq, void
= > > *__hba)
= > > return retval;
= > > }
= > >
= > > +static int ufshcd_clear_tm_cmd(struct ufs_hba *hba, int tag) {
= > > + int err = 0;
= > > + u32 mask = 1 << tag;
= > > + unsigned long flags;
= > > +
= > > + if (!test_bit(tag, &hba->outstanding_reqs))
= > > + goto out;
= > > +
= > > + spin_lock_irqsave(hba->host->host_lock, flags);
= > > + ufshcd_writel(hba, ~(1 << tag), REG_UTP_TASK_REQ_LIST_CLEAR);
= > > + spin_unlock_irqrestore(hba->host->host_lock, flags);
= > > +
= > > + /* poll for max. 1 sec to clear door bell register by h/w */
= > > + err = ufshcd_wait_for_register(hba,
= > > + REG_UTP_TASK_REQ_DOOR_BELL,
= > > + mask, 0, 1000, 1000);
= > > +out:
= > > + return err;
= > > +}
= > > +
= > > /**
= > > * ufshcd_issue_tm_cmd - issues task management commands to
= > controller
= > > * @hba: per adapter instance
= > > - * @lrbp: pointer to local reference block
= > > + * @lun_id: LUN ID to which TM command is sent
= > > + * @task_id: task ID to which the TM command is applicable
= > > + * @tm_function: task management function opcode
= > > + * @tm_response: task management service response return value
= > > *
= > > - * Returns SUCCESS/FAILED
= > > + * Returns non-zero value on error, zero on success.
= > > */
= > > -static int
= > > -ufshcd_issue_tm_cmd(struct ufs_hba *hba,
= > > - struct ufshcd_lrb *lrbp,
= > > - u8 tm_function)
= > > +static int ufshcd_issue_tm_cmd(struct ufs_hba *hba, int lun_id, int
= > > task_id,
= > > + u8 tm_function, u8 *tm_response)
= > > {
= > > struct utp_task_req_desc *task_req_descp;
= > > struct utp_upiu_task_req *task_req_upiup;
= > > struct Scsi_Host *host;
= > > unsigned long flags;
= > > - int free_slot = 0;
= > > + int free_slot;
= > > int err;
= > > + int task_tag;
= > >
= > > host = hba->host;
= > >
= > > - spin_lock_irqsave(host->host_lock, flags);
= > > -
= > > - /* If task management queue is full */
= > > - free_slot = ufshcd_get_tm_free_slot(hba);
= > > - if (free_slot >= hba->nutmrs) {
= > > - spin_unlock_irqrestore(host->host_lock, flags);
= > > - dev_err(hba->dev, "Task management queue full\n");
= > > - err = FAILED;
= > > - goto out;
= > > - }
= > > + /*
= > > + * Get free slot, sleep if slots are unavailable.
= > > + * Even though we use wait_event() which sleeps indefinitely,
= > > + * the maximum wait time is bounded by %TM_CMD_TIMEOUT.
= > > + */
= > > + wait_event(hba->tm_tag_wq, ufshcd_get_tm_free_slot(hba,
= > > +&free_slot));
= > >
= > > + spin_lock_irqsave(host->host_lock, flags);
= > > task_req_descp = hba->utmrdl_base_addr;
= > > task_req_descp += free_slot;
= > >
= > > @@ -2330,18 +2372,15 @@ ufshcd_issue_tm_cmd(struct ufs_hba *hba,
= > > /* 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,
= > > - lrbp->lun, lrbp->task_tag);
= > > + lun_id, task_tag);
= > > task_req_upiup->header.dword_1 =
= > > UPIU_HEADER_DWORD(0, tm_function, 0, 0);
= > >
= > > - task_req_upiup->input_param1 = lrbp->lun;
= > > - task_req_upiup->input_param1 =
= > > - cpu_to_be32(task_req_upiup->input_param1);
= > > - task_req_upiup->input_param2 = lrbp->task_tag;
= > > - task_req_upiup->input_param2 =
= > > - cpu_to_be32(task_req_upiup->input_param2);
= > > + task_req_upiup->input_param1 = cpu_to_be32(lun_id);
= > > + task_req_upiup->input_param2 = cpu_to_be32(task_id);
= > >
= > > /* send command to the controller */
= > > __set_bit(free_slot, &hba->outstanding_tasks); @@ -2350,20
= > +2389,24
= > > @@ ufshcd_issue_tm_cmd(struct ufs_hba *hba,
= > > spin_unlock_irqrestore(host->host_lock, flags);
= > >
= > > /* wait until the task management command is completed */
= > > - err =
= > > - wait_event_interruptible_timeout(hba->ufshcd_tm_wait_queue,
= > > - (test_bit(free_slot,
= > > - &hba->tm_condition) != 0),
= > > - 60 * HZ);
= > > + err = wait_event_timeout(hba->tm_wq,
= > > + test_bit(free_slot, &hba->tm_condition),
= > > + msecs_to_jiffies(TM_CMD_TIMEOUT));
= > > if (!err) {
= > > - dev_err(hba->dev,
= > > - "Task management command timed-out\n");
= > > - err = FAILED;
= > > - goto out;
= > > + dev_err(hba->dev, "%s: task management cmd 0x%.2x
= > timed-out\n",
= > > + __func__, tm_function);
= > > + if (ufshcd_clear_tm_cmd(hba, free_slot))
= > > + dev_WARN(hba->dev, "%s: unable clear tm cmd
= > (slot %d) after
= > > timeout\n",
= > > + __func__, free_slot);
= > > + err = -ETIMEDOUT;
= > > + } else {
= > > + err = ufshcd_task_req_compl(hba, free_slot,
= > tm_response);
= > > }
= > > +
= > > clear_bit(free_slot, &hba->tm_condition);
= > > - err = ufshcd_task_req_compl(hba, free_slot);
= > > -out:
= > > + ufshcd_put_tm_slot(hba, free_slot);
= > > + wake_up(&hba->tm_tag_wq);
= > > +
= > > return err;
= > > }
= > >
= > > @@ -2380,14 +2423,21 @@ static int ufshcd_device_reset(struct
= > > scsi_cmnd
= > > *cmd)
= > > unsigned int tag;
= > > u32 pos;
= > > int err;
= > > + u8 resp = 0xF;
= > > + struct ufshcd_lrb *lrbp;
= > >
= > > host = cmd->device->host;
= > > hba = shost_priv(host);
= > > tag = cmd->request->tag;
= > >
= > > - err = ufshcd_issue_tm_cmd(hba, &hba->lrb[tag],
= > UFS_LOGICAL_RESET);
= > > - if (err == FAILED)
= > > + lrbp = &hba->lrb[tag];
= > > + err = ufshcd_issue_tm_cmd(hba, lrbp->lun, 0,
= > UFS_LOGICAL_RESET, &resp);
= > > + if (err || resp != UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
= > > + err = FAILED;
= > > goto out;
= > > + } else {
= > > + err = SUCCESS;
= > > + }
= > >
= > > for (pos = 0; pos < hba->nutrs; pos++) {
= > > if (test_bit(pos, &hba->outstanding_reqs) && @@ -2444,6
= > +2494,8 @@
= > > static int ufshcd_abort(struct scsi_cmnd *cmd)
= > > unsigned long flags;
= > > unsigned int tag;
= > > int err;
= > > + u8 resp = 0xF;
= > > + struct ufshcd_lrb *lrbp;
= > >
= > > host = cmd->device->host;
= > > hba = shost_priv(host);
= > > @@ -2459,9 +2511,15 @@ static int ufshcd_abort(struct scsi_cmnd
= > *cmd)
= > > }
= > > spin_unlock_irqrestore(host->host_lock, flags);
= > >
= > > - err = ufshcd_issue_tm_cmd(hba, &hba->lrb[tag],
= > UFS_ABORT_TASK);
= > > - if (err == FAILED)
= > > + lrbp = &hba->lrb[tag];
= > > + err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
= > > + UFS_ABORT_TASK, &resp);
= > > + if (err || resp != UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
= > > + err = FAILED;
= > > goto out;
= > > + } else {
= > > + err = SUCCESS;
= > > + }
= > >
= > > scsi_dma_unmap(cmd);
= > >
= > > @@ -2682,7 +2740,8 @@ int ufshcd_init(struct device *dev, struct
= > > ufs_hba **hba_handle,
= > > host->max_cmd_len = MAX_CDB_SIZE;
= > >
= > > /* Initailize wait queue for task management */
= > > - init_waitqueue_head(&hba->ufshcd_tm_wait_queue);
= > > + init_waitqueue_head(&hba->tm_wq);
= > > + init_waitqueue_head(&hba->tm_tag_wq);
= > >
= > > /* Initialize work queues */
= > > INIT_WORK(&hba->feh_workq, ufshcd_fatal_err_handler); diff --
= > git
= > > a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h index
= > > 59c9c48..fe7c947 100644
= > > --- a/drivers/scsi/ufs/ufshcd.h
= > > +++ b/drivers/scsi/ufs/ufshcd.h
= > > @@ -174,8 +174,10 @@ struct ufs_dev_cmd {
= > > * @irq: Irq number of the controller
= > > * @active_uic_cmd: handle of active UIC command
= > > * @uic_cmd_mutex: mutex for uic command
= > > - * @ufshcd_tm_wait_queue: wait queue for task management
= > > + * @tm_wq: wait queue for task management
= > > + * @tm_tag_wq: wait queue for free task management slots
= > > * @tm_condition: condition variable for task management
= > > + * @tm_slots_in_use: bit map of task management request slots in use
= > > * @ufshcd_state: UFSHCD states
= > > * @intr_mask: Interrupt Mask Bits
= > > * @ee_ctrl_mask: Exception event control mask @@ -216,8 +218,10
= > @@
= > > struct ufs_hba {
= > > struct uic_command *active_uic_cmd;
= > > struct mutex uic_cmd_mutex;
= > >
= > > - wait_queue_head_t ufshcd_tm_wait_queue;
= > > + wait_queue_head_t tm_wq;
= > > + wait_queue_head_t tm_tag_wq;
= > > unsigned long tm_condition;
= > > + unsigned long tm_slots_in_use;
= > >
= > > u32 ufshcd_state;
= > > u32 intr_mask;
= > > --
= > > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
= > > member of Code Aurora Forum, hosted by The Linux Foundation.
= > >
= > > --
= > > To unsubscribe from this list: send the line "unsubscribe linux-scsi"
= > > in the body of a message to majordomo@vger.kernel.org More
= > majordomo
= > > info at http://vger.kernel.org/majordomo-info.html
= > >
= >
= >
= > --
= > QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a
= > member of Code Aurora Forum, hosted by The Linux Foundation
= >
= > --
= > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the
= > body of a message to majordomo@vger.kernel.org More majordomo info
= > at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index b36ca9a..d7f3746 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -53,6 +53,9 @@ /* Query request timeout */ #define QUERY_REQ_TIMEOUT 30 /* msec */ +/* Task management command timeout */ +#define TM_CMD_TIMEOUT 100 /* msecs */ + /* Expose the flag value from utp_upiu_query.value */ #define MASK_QUERY_UPIU_FLAG_LOC 0xFF @@ -183,13 +186,35 @@ ufshcd_get_tmr_ocs(struct utp_task_req_desc *task_req_descp) /** * ufshcd_get_tm_free_slot - get a free slot for task management request * @hba: per adapter instance + * @free_slot: pointer to variable with available slot value * - * Returns maximum number of task management request slots in case of - * task management queue full or returns the free slot number + * Get a free tag and lock it until ufshcd_put_tm_slot() is called. + * Returns 0 if free slot is not available, else return 1 with tag value + * in @free_slot. */ -static inline int ufshcd_get_tm_free_slot(struct ufs_hba *hba) +static bool ufshcd_get_tm_free_slot(struct ufs_hba *hba, int *free_slot) { - return find_first_zero_bit(&hba->outstanding_tasks, hba->nutmrs); + int tag; + bool ret = false; + + if (!free_slot) + goto out; + + do { + tag = find_first_zero_bit(&hba->tm_slots_in_use, hba->nutmrs); + if (tag >= hba->nutmrs) + goto out; + } while (test_and_set_bit_lock(tag, &hba->tm_slots_in_use)); + + *free_slot = tag; + ret = true; +out: + return ret; +} + +static inline void ufshcd_put_tm_slot(struct ufs_hba *hba, int slot) +{ + clear_bit_unlock(slot, &hba->tm_slots_in_use); } /** @@ -1700,10 +1725,11 @@ static void ufshcd_slave_destroy(struct scsi_device *sdev) * ufshcd_task_req_compl - handle task management request completion * @hba: per adapter instance * @index: index of the completed request + * @resp: task management service response * - * Returns SUCCESS/FAILED + * Returns non-zero value on error, zero on success */ -static int ufshcd_task_req_compl(struct ufs_hba *hba, u32 index) +static int ufshcd_task_req_compl(struct ufs_hba *hba, u32 index, u8 *resp) { struct utp_task_req_desc *task_req_descp; struct utp_upiu_task_rsp *task_rsp_upiup; @@ -1724,19 +1750,15 @@ static int ufshcd_task_req_compl(struct ufs_hba *hba, u32 index) task_req_descp[index].task_rsp_upiu; 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 && - task_result != UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED) - task_result = FAILED; - else - task_result = SUCCESS; + if (resp) + *resp = (u8)task_result; } else { - task_result = FAILED; - dev_err(hba->dev, - "trc: Invalid ocs = %x\n", ocs_value); + dev_err(hba->dev, "%s: failed, ocs = 0x%x\n", + __func__, ocs_value); } spin_unlock_irqrestore(hba->host->host_lock, flags); - return task_result; + + return ocs_value; } /** @@ -2237,7 +2259,7 @@ static void ufshcd_tmc_handler(struct ufs_hba *hba) tm_doorbell = ufshcd_readl(hba, REG_UTP_TASK_REQ_DOOR_BELL); hba->tm_condition = tm_doorbell ^ hba->outstanding_tasks; - wake_up_interruptible(&hba->ufshcd_tm_wait_queue); + wake_up(&hba->tm_wq); } /** @@ -2287,38 +2309,58 @@ static irqreturn_t ufshcd_intr(int irq, void *__hba) return retval; } +static int ufshcd_clear_tm_cmd(struct ufs_hba *hba, int tag) +{ + int err = 0; + u32 mask = 1 << tag; + unsigned long flags; + + if (!test_bit(tag, &hba->outstanding_reqs)) + goto out; + + spin_lock_irqsave(hba->host->host_lock, flags); + ufshcd_writel(hba, ~(1 << tag), REG_UTP_TASK_REQ_LIST_CLEAR); + spin_unlock_irqrestore(hba->host->host_lock, flags); + + /* poll for max. 1 sec to clear door bell register by h/w */ + err = ufshcd_wait_for_register(hba, + REG_UTP_TASK_REQ_DOOR_BELL, + mask, 0, 1000, 1000); +out: + return err; +} + /** * ufshcd_issue_tm_cmd - issues task management commands to controller * @hba: per adapter instance - * @lrbp: pointer to local reference block + * @lun_id: LUN ID to which TM command is sent + * @task_id: task ID to which the TM command is applicable + * @tm_function: task management function opcode + * @tm_response: task management service response return value * - * Returns SUCCESS/FAILED + * Returns non-zero value on error, zero on success. */ -static int -ufshcd_issue_tm_cmd(struct ufs_hba *hba, - struct ufshcd_lrb *lrbp, - u8 tm_function) +static int ufshcd_issue_tm_cmd(struct ufs_hba *hba, int lun_id, int task_id, + u8 tm_function, u8 *tm_response) { struct utp_task_req_desc *task_req_descp; struct utp_upiu_task_req *task_req_upiup; struct Scsi_Host *host; unsigned long flags; - int free_slot = 0; + int free_slot; int err; + int task_tag; host = hba->host; - spin_lock_irqsave(host->host_lock, flags); - - /* If task management queue is full */ - free_slot = ufshcd_get_tm_free_slot(hba); - if (free_slot >= hba->nutmrs) { - spin_unlock_irqrestore(host->host_lock, flags); - dev_err(hba->dev, "Task management queue full\n"); - err = FAILED; - goto out; - } + /* + * Get free slot, sleep if slots are unavailable. + * Even though we use wait_event() which sleeps indefinitely, + * the maximum wait time is bounded by %TM_CMD_TIMEOUT. + */ + wait_event(hba->tm_tag_wq, ufshcd_get_tm_free_slot(hba, &free_slot)); + spin_lock_irqsave(host->host_lock, flags); task_req_descp = hba->utmrdl_base_addr; task_req_descp += free_slot; @@ -2330,18 +2372,15 @@ ufshcd_issue_tm_cmd(struct ufs_hba *hba, /* 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, - lrbp->lun, lrbp->task_tag); + lun_id, task_tag); task_req_upiup->header.dword_1 = UPIU_HEADER_DWORD(0, tm_function, 0, 0); - task_req_upiup->input_param1 = lrbp->lun; - task_req_upiup->input_param1 = - cpu_to_be32(task_req_upiup->input_param1); - task_req_upiup->input_param2 = lrbp->task_tag; - task_req_upiup->input_param2 = - cpu_to_be32(task_req_upiup->input_param2); + task_req_upiup->input_param1 = cpu_to_be32(lun_id); + task_req_upiup->input_param2 = cpu_to_be32(task_id); /* send command to the controller */ __set_bit(free_slot, &hba->outstanding_tasks); @@ -2350,20 +2389,24 @@ ufshcd_issue_tm_cmd(struct ufs_hba *hba, spin_unlock_irqrestore(host->host_lock, flags); /* wait until the task management command is completed */ - err = - wait_event_interruptible_timeout(hba->ufshcd_tm_wait_queue, - (test_bit(free_slot, - &hba->tm_condition) != 0), - 60 * HZ); + err = wait_event_timeout(hba->tm_wq, + test_bit(free_slot, &hba->tm_condition), + msecs_to_jiffies(TM_CMD_TIMEOUT)); if (!err) { - dev_err(hba->dev, - "Task management command timed-out\n"); - err = FAILED; - goto out; + dev_err(hba->dev, "%s: task management cmd 0x%.2x timed-out\n", + __func__, tm_function); + if (ufshcd_clear_tm_cmd(hba, free_slot)) + dev_WARN(hba->dev, "%s: unable clear tm cmd (slot %d) after timeout\n", + __func__, free_slot); + err = -ETIMEDOUT; + } else { + err = ufshcd_task_req_compl(hba, free_slot, tm_response); } + clear_bit(free_slot, &hba->tm_condition); - err = ufshcd_task_req_compl(hba, free_slot); -out: + ufshcd_put_tm_slot(hba, free_slot); + wake_up(&hba->tm_tag_wq); + return err; } @@ -2380,14 +2423,21 @@ static int ufshcd_device_reset(struct scsi_cmnd *cmd) unsigned int tag; u32 pos; int err; + u8 resp = 0xF; + struct ufshcd_lrb *lrbp; host = cmd->device->host; hba = shost_priv(host); tag = cmd->request->tag; - err = ufshcd_issue_tm_cmd(hba, &hba->lrb[tag], UFS_LOGICAL_RESET); - if (err == FAILED) + lrbp = &hba->lrb[tag]; + err = ufshcd_issue_tm_cmd(hba, lrbp->lun, 0, UFS_LOGICAL_RESET, &resp); + if (err || resp != UPIU_TASK_MANAGEMENT_FUNC_COMPL) { + err = FAILED; goto out; + } else { + err = SUCCESS; + } for (pos = 0; pos < hba->nutrs; pos++) { if (test_bit(pos, &hba->outstanding_reqs) && @@ -2444,6 +2494,8 @@ static int ufshcd_abort(struct scsi_cmnd *cmd) unsigned long flags; unsigned int tag; int err; + u8 resp = 0xF; + struct ufshcd_lrb *lrbp; host = cmd->device->host; hba = shost_priv(host); @@ -2459,9 +2511,15 @@ static int ufshcd_abort(struct scsi_cmnd *cmd) } spin_unlock_irqrestore(host->host_lock, flags); - err = ufshcd_issue_tm_cmd(hba, &hba->lrb[tag], UFS_ABORT_TASK); - if (err == FAILED) + lrbp = &hba->lrb[tag]; + err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag, + UFS_ABORT_TASK, &resp); + if (err || resp != UPIU_TASK_MANAGEMENT_FUNC_COMPL) { + err = FAILED; goto out; + } else { + err = SUCCESS; + } scsi_dma_unmap(cmd); @@ -2682,7 +2740,8 @@ int ufshcd_init(struct device *dev, struct ufs_hba **hba_handle, host->max_cmd_len = MAX_CDB_SIZE; /* Initailize wait queue for task management */ - init_waitqueue_head(&hba->ufshcd_tm_wait_queue); + init_waitqueue_head(&hba->tm_wq); + init_waitqueue_head(&hba->tm_tag_wq); /* Initialize work queues */ INIT_WORK(&hba->feh_workq, ufshcd_fatal_err_handler); diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h index 59c9c48..fe7c947 100644 --- a/drivers/scsi/ufs/ufshcd.h +++ b/drivers/scsi/ufs/ufshcd.h @@ -174,8 +174,10 @@ struct ufs_dev_cmd { * @irq: Irq number of the controller * @active_uic_cmd: handle of active UIC command * @uic_cmd_mutex: mutex for uic command - * @ufshcd_tm_wait_queue: wait queue for task management + * @tm_wq: wait queue for task management + * @tm_tag_wq: wait queue for free task management slots * @tm_condition: condition variable for task management + * @tm_slots_in_use: bit map of task management request slots in use * @ufshcd_state: UFSHCD states * @intr_mask: Interrupt Mask Bits * @ee_ctrl_mask: Exception event control mask @@ -216,8 +218,10 @@ struct ufs_hba { struct uic_command *active_uic_cmd; struct mutex uic_cmd_mutex; - wait_queue_head_t ufshcd_tm_wait_queue; + wait_queue_head_t tm_wq; + wait_queue_head_t tm_tag_wq; unsigned long tm_condition; + unsigned long tm_slots_in_use; u32 ufshcd_state; u32 intr_mask;
Currently, sending Task Management (TM) command to the card might be broken in some scenarios as listed below: Problem: If there are more than 8 TM commands the implementation returns error to the caller. Fix: Wait for one of the slots to be emptied and send the command. Problem: Sometimes it is necessary for the caller to know the TM service response code to determine the task status. Fix: Propogate the service response to the caller. Problem: If the TM command times out no proper error recovery is implemented. Fix: Clear the command in the controller door-bell register, so that further commands for the same slot don't fail. Problem: While preparing the TM command descriptor, the task tag used should be unique across SCSI/NOP/QUERY/TM commands and not the task tag of the command which the TM command is trying to manage. Fix: Use a unique task tag instead of task tag of SCSI command. Problem: Since the TM command involves H/W communication, abruptly ending the request on kill interrupt signal might cause h/w malfunction. Fix: Wait for hardware completion interrupt with TASK_UNINTERRUPTIBLE set. Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org> --- drivers/scsi/ufs/ufshcd.c | 173 ++++++++++++++++++++++++++++++--------------- drivers/scsi/ufs/ufshcd.h | 8 ++- 2 files changed, 122 insertions(+), 59 deletions(-)