Messages in this thread | | | Date | Wed, 10 May 2023 14:50:16 -0700 | From | Bart Van Assche <> | Subject | Re: [PATCH v3 3/7] ufs: mcq: Add supporting functions for mcq abort |
| |
On 5/9/23 22:24, Bao D. Nguyen wrote: > +/** > + * ufshcd_mcq_sq_cleanup - Clean up submission queue resources > + * associated with the pending command. > + * @hba - per adapter instance. > + * @task_tag - The command's task tag. > + * @result - Result of the clean up operation. > + * > + * Returns 0 and result on completion. Returns error code if > + * the operation fails. > + */ > +int ufshcd_mcq_sq_cleanup(struct ufs_hba *hba, int task_tag, int *result) > +{ > + struct ufshcd_lrb *lrbp = &hba->lrb[task_tag]; > + struct scsi_cmnd *cmd = lrbp->cmd; > + struct ufs_hw_queue *hwq; > + void __iomem *reg, *opr_sqd_base; > + u32 nexus, id, val; > + int err; > + > + if (task_tag != hba->nutrs - UFSHCD_NUM_RESERVED) { > + if (!cmd) > + return FAILED; > + hwq = ufshcd_mcq_req_to_hwq(hba, scsi_cmd_to_rq(cmd)); > + } else { > + hwq = hba->dev_cmd_queue; > + } > + > + id = hwq->id; > + > + mutex_lock(&hwq->sq_mutex); > + > + /* stop the SQ fetching before working on it */ > + err = ufshcd_mcq_sq_stop(hba, hwq); > + if (err) > + goto unlock; > + > + /* SQCTI = EXT_IID, IID, LUN, Task Tag */ > + nexus = lrbp->lun << 8 | task_tag; > + opr_sqd_base = mcq_opr_base(hba, OPR_SQD, id); > + writel(nexus, opr_sqd_base + REG_SQCTI); > + > + /* SQRTCy.ICU = 1 */ > + writel(SQ_ICU, opr_sqd_base + REG_SQRTC); > + > + /* Poll SQRTSy.CUS = 1. Return result from SQRTSy.RTC */ > + reg = opr_sqd_base + REG_SQRTS; > + err = read_poll_timeout(readl, val, val & SQ_CUS, 20, > + MCQ_POLL_US, false, reg); > + if (err) > + dev_err(hba->dev, "%s: failed. hwq=%d, lun=0x%x, tag=%d\n", > + __func__, id, lrbp->lun, task_tag); > + > + *result = FIELD_GET(SQ_ICU_ERR_CODE_MASK, readl(reg)); > + > + if (ufshcd_mcq_sq_start(hba, hwq)) > + err = FAILED; > + > +unlock: > + mutex_unlock(&hwq->sq_mutex); > + return err; > +}
Please do not use the FAILED / SUCCESS return values in this function. These values should only be returned by functions related to SCSI error handling. Please do the following: * Return a negative Unix error code in case of failure. * Return zero upon success. * Return the FIELD_GET() result as a positive value. * Remove the 'int *result' argument.
> + /* prevent concurrent access to sq hw */ > + struct mutex sq_mutex;
Hmm ... a submission queue (SQ) exists in host memory so I'm not sure the "hw" part of the above comment is correct.
Thanks,
Bart.
| |