| Message ID | 68b786f390dbd93218a482d18c513bc332e82da3.1678338926.git.quic_nguyenb@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ufs: core: mcq: Add ufshcd_abort() support in MCQ mode | expand |
On 9/03/23 07:28, Bao D. Nguyen wrote: > Add supporting functions to handle ufs abort in mcq mode. > > Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com> > --- > drivers/ufs/core/ufs-mcq.c | 181 +++++++++++++++++++++++++++++++++++++++++ > drivers/ufs/core/ufshcd-priv.h | 14 ++++ > drivers/ufs/core/ufshcd.c | 3 +- > include/ufs/ufshcd.h | 1 + > include/ufs/ufshci.h | 16 ++++ > 5 files changed, 214 insertions(+), 1 deletion(-) > > diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c > index 31df052..4c33c1a 100644 > --- a/drivers/ufs/core/ufs-mcq.c > +++ b/drivers/ufs/core/ufs-mcq.c > @@ -12,6 +12,9 @@ > #include <linux/module.h> > #include <linux/platform_device.h> > #include "ufshcd-priv.h" > +#include <linux/delay.h> > +#include <scsi/scsi_cmnd.h> > +#include <linux/bitfield.h> > > #define MAX_QUEUE_SUP GENMASK(7, 0) > #define UFS_MCQ_MIN_RW_QUEUES 2 > @@ -27,6 +30,9 @@ > #define MCQ_ENTRY_SIZE_IN_DWORD 8 > #define CQE_UCD_BA GENMASK_ULL(63, 7) > > +/* Max mcq register polling time in millisecond unit */ > +#define MCQ_POLL_MS 500 > + > static int rw_queue_count_set(const char *val, const struct kernel_param *kp) > { > return param_set_uint_minmax(val, kp, UFS_MCQ_MIN_RW_QUEUES, > @@ -429,3 +435,178 @@ int ufshcd_mcq_init(struct ufs_hba *hba) > host->host_tagset = 1; > return 0; > } > + > +static int ufshcd_mcq_poll_register(void __iomem *reg, u32 mask, > + u32 val, unsigned long timeout_ms) > +{ > + unsigned long timeout = jiffies + msecs_to_jiffies(timeout_ms); > + int err = 0; > + > + /* ignore bits that we don't intend to wait on */ > + val = val & mask; > + > + while ((readl(reg) & mask) != val) { > + usleep_range(10, 50); > + if (time_after(jiffies, timeout)) { > + err = -ETIMEDOUT; > + break; > + } > + } Should be able to use a macro from include/linux/iopoll.h here > + > + return err; > +}
On 3/8/23 21:28, Bao D. Nguyen wrote: > +static bool ufshcd_mcq_sqe_search(struct ufs_hba *hba, > + struct ufs_hw_queue *hwq, int task_tag) > +{ > + struct utp_transfer_req_desc *utrd; > + u32 mask = hwq->max_entries - 1; > + bool ret = false; > + u64 addr, match; > + u32 i; The variable name "i" is usually used for a loop index. In this case it represents a slot in the submission queue. How about renaming "i" into "slot"? > +static inline void ufshcd_mcq_update_sq_head_slot(struct ufs_hw_queue *q) > +{ > + u32 val = readl(q->mcq_sq_head); > + > + q->sq_head_slot = val / sizeof(struct utp_transfer_req_desc); > +} Please modify this function such that it returns the head slot value instead of storing it in a member variable and remove the sq_head_slot member variable. Storing the sq_head_slot value in a member variable seems wrong to me since the value of that variable will be outdated as soon as the submission queue is restarted. > +static inline bool ufshcd_mcq_is_sq_empty(struct ufs_hw_queue *q) > +{ > + return q->sq_head_slot == q->sq_tail_slot; > +} Please remove this function and inline this function into its callers. Thanks, Bart.
On 3/9/2023 10:15 AM, Bart Van Assche wrote: > On 3/8/23 21:28, Bao D. Nguyen wrote: >> +static bool ufshcd_mcq_sqe_search(struct ufs_hba *hba, >> + struct ufs_hw_queue *hwq, int task_tag) >> +{ >> + struct utp_transfer_req_desc *utrd; >> + u32 mask = hwq->max_entries - 1; >> + bool ret = false; >> + u64 addr, match; >> + u32 i; > > The variable name "i" is usually used for a loop index. In this case > it represents a slot in the submission queue. How about renaming "i" > into "slot"? I will make the change. > >> +static inline void ufshcd_mcq_update_sq_head_slot(struct >> ufs_hw_queue *q) >> +{ >> + u32 val = readl(q->mcq_sq_head); >> + >> + q->sq_head_slot = val / sizeof(struct utp_transfer_req_desc); >> +} > > Please modify this function such that it returns the head slot value > instead of storing it in a member variable and remove the sq_head_slot > member variable. Storing the sq_head_slot value in a member variable > seems wrong to me since the value of that variable will be outdated as > soon as the submission queue is restarted. > I can modify the function that I am introducing in this patch namely ufshcd_mcq_update_sq_head_slot() according to your suggestion. However, to keep the original mcq code consistent with this change, should I make the same modifications to these existing functions ufshcd_mcq_update_cq_tail_slot(), ufshcd_mcq_update_cq_head() in a separate patch and include in this series? >> +static inline bool ufshcd_mcq_is_sq_empty(struct ufs_hw_queue *q) >> +{ >> + return q->sq_head_slot == q->sq_tail_slot; >> +} > > Please remove this function and inline this function into its callers. Same comment. Should I also update the existing ufshcd_mcq_is_cq_empty() in a separate patch together with ufshcd_mcq_update_cq_tail_slot(), ufshcd_mcq_update_cq_head() mentioned above? Thanks, Bao > > Thanks, > > Bart.
On 3/9/23 14:47, Bao D. Nguyen wrote: > On 3/9/2023 10:15 AM, Bart Van Assche wrote: >> On 3/8/23 21:28, Bao D. Nguyen wrote: >>> +static inline bool ufshcd_mcq_is_sq_empty(struct ufs_hw_queue *q) >>> +{ >>> + return q->sq_head_slot == q->sq_tail_slot; >>> +} >> >> Please remove this function and inline this function into its callers. > > Same comment. Should I also update the existing ufshcd_mcq_is_cq_empty() > in a separate patch together with ufshcd_mcq_update_cq_tail_slot(), > ufshcd_mcq_update_cq_head() mentioned above? Hi Bao, Modifying the existing code may improve uniformity of the UFS host controller driver. If any existing code is refactored, please do that via a separate patch. Thanks, Bart.
On 3/9/2023 2:54 PM, Bart Van Assche wrote: > On 3/9/23 14:47, Bao D. Nguyen wrote: >> On 3/9/2023 10:15 AM, Bart Van Assche wrote: >>> On 3/8/23 21:28, Bao D. Nguyen wrote: >>>> +static inline bool ufshcd_mcq_is_sq_empty(struct ufs_hw_queue *q) >>>> +{ >>>> + return q->sq_head_slot == q->sq_tail_slot; >>>> +} >>> >>> Please remove this function and inline this function into its callers. >> >> Same comment. Should I also update the existing >> ufshcd_mcq_is_cq_empty() in a separate patch together with >> ufshcd_mcq_update_cq_tail_slot(), ufshcd_mcq_update_cq_head() >> mentioned above? > > Hi Bao, > > Modifying the existing code may improve uniformity of the UFS host > controller driver. If any existing code is refactored, please do that > via a separate patch. Thanks Bart. I will make that change in a separate patch. > > Thanks, > > Bart. >
diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c index 31df052..4c33c1a 100644 --- a/drivers/ufs/core/ufs-mcq.c +++ b/drivers/ufs/core/ufs-mcq.c @@ -12,6 +12,9 @@ #include <linux/module.h> #include <linux/platform_device.h> #include "ufshcd-priv.h" +#include <linux/delay.h> +#include <scsi/scsi_cmnd.h> +#include <linux/bitfield.h> #define MAX_QUEUE_SUP GENMASK(7, 0) #define UFS_MCQ_MIN_RW_QUEUES 2 @@ -27,6 +30,9 @@ #define MCQ_ENTRY_SIZE_IN_DWORD 8 #define CQE_UCD_BA GENMASK_ULL(63, 7) +/* Max mcq register polling time in millisecond unit */ +#define MCQ_POLL_MS 500 + static int rw_queue_count_set(const char *val, const struct kernel_param *kp) { return param_set_uint_minmax(val, kp, UFS_MCQ_MIN_RW_QUEUES, @@ -429,3 +435,178 @@ int ufshcd_mcq_init(struct ufs_hba *hba) host->host_tagset = 1; return 0; } + +static int ufshcd_mcq_poll_register(void __iomem *reg, u32 mask, + u32 val, unsigned long timeout_ms) +{ + unsigned long timeout = jiffies + msecs_to_jiffies(timeout_ms); + int err = 0; + + /* ignore bits that we don't intend to wait on */ + val = val & mask; + + while ((readl(reg) & mask) != val) { + usleep_range(10, 50); + if (time_after(jiffies, timeout)) { + err = -ETIMEDOUT; + break; + } + } + + return err; +} + +static int ufshcd_mcq_sq_stop(struct ufs_hba *hba, struct ufs_hw_queue *hwq) +{ + void __iomem *reg; + u32 i = hwq->id; + int err; + + writel(SQ_STOP, mcq_opr_base(hba, OPR_SQD, i) + REG_SQRTC); + reg = mcq_opr_base(hba, OPR_SQD, i) + REG_SQRTS; + err = ufshcd_mcq_poll_register(reg, SQ_STS, SQ_STS, MCQ_POLL_MS); + if (err) + dev_err(hba->dev, "%s: failed. hwq-id=%d, err=%d\n", + __func__, i, err); + return err; +} + +static int ufshcd_mcq_sq_start(struct ufs_hba *hba, struct ufs_hw_queue *hwq) +{ + void __iomem *reg; + u32 i = hwq->id; + int err; + + writel(SQ_START, mcq_opr_base(hba, OPR_SQD, i) + REG_SQRTC); + reg = mcq_opr_base(hba, OPR_SQD, i) + REG_SQRTS; + err = ufshcd_mcq_poll_register(reg, SQ_STS, 0, MCQ_POLL_MS); + if (err) + dev_err(hba->dev, "%s: failed. hwq-id=%d, err=%d\n", + __func__, i, err); + return err; +} + +/** + * 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, i; + int err; + + hwq = ufshcd_mcq_req_to_hwq(hba, scsi_cmd_to_rq(cmd)); + i = hwq->id; + + spin_lock(&hwq->sq_lock); + + /* 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, i); + 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 = ufshcd_mcq_poll_register(reg, SQ_CUS, SQ_CUS, MCQ_POLL_MS); + if (err) { + dev_err(hba->dev, "%s: failed. hwq=%d, lun=0x%x, tag=%d\n", + __func__, i, lrbp->lun, task_tag); + *result = FIELD_GET(SQ_ICU_ERR_CODE_MASK, readl(reg)); + } + + err = ufshcd_mcq_sq_start(hba, hwq); + +unlock: + spin_unlock(&hwq->sq_lock); + return err; +} + +static u64 ufshcd_mcq_get_cmd_desc_addr(struct ufs_hba *hba, + int task_tag) +{ + struct ufshcd_lrb *lrbp = &hba->lrb[task_tag]; + __le32 hi = lrbp->utr_descriptor_ptr->command_desc_base_addr_hi; + __le32 lo = lrbp->utr_descriptor_ptr->command_desc_base_addr_lo; + + return le64_to_cpu((__le64)hi << 32 | lo); +} + +/** + * ufshcd_mcq_nullify_cmd - Nullify utrd. Host controller does not fetch + * transfer with Command Type = 0xF. post the Completion Queue with OCS=ABORTED. + * @hba - per adapter instance. + * @hwq - Hardware Queue of the nullified utrd. + */ +static void ufshcd_mcq_nullify_cmd(struct ufs_hba *hba, struct ufs_hw_queue *hwq) +{ + struct utp_transfer_req_desc *utrd; + u32 dword_0; + + utrd = (struct utp_transfer_req_desc *)(hwq->sqe_base_addr + + hwq->id * sizeof(struct utp_transfer_req_desc)); + dword_0 = le32_to_cpu(utrd->header.dword_0); + dword_0 &= ~UPIU_COMMAND_TYPE_MASK; + dword_0 |= FIELD_PREP(UPIU_COMMAND_TYPE_MASK, 0xF); + utrd->header.dword_0 = cpu_to_le32(dword_0); +} + +/** + * ufshcd_mcq_sqe_search - Search for the cmd in the Submission Queue (SQ) + * @hba - per adapter instance. + * @hwq - Hardware Queue to be searched. + * @task_tag - The command's task tag. + * + * Returns true if the SQE containing the command is present in the SQ + * (not fetched by the controller); returns false if the SQE is not in the SQ. + */ +static bool ufshcd_mcq_sqe_search(struct ufs_hba *hba, + struct ufs_hw_queue *hwq, int task_tag) +{ + struct utp_transfer_req_desc *utrd; + u32 mask = hwq->max_entries - 1; + bool ret = false; + u64 addr, match; + u32 i; + + spin_lock(&hwq->sq_lock); + + ufshcd_mcq_update_sq_head_slot(hwq); + if (ufshcd_mcq_is_sq_empty(hwq)) + goto out; + + addr = ufshcd_mcq_get_cmd_desc_addr(hba, task_tag); + i = hwq->sq_head_slot; + while (i != hwq->sq_tail_slot) { + utrd = (struct utp_transfer_req_desc *)(hwq->sqe_base_addr + + i * sizeof(struct utp_transfer_req_desc)); + match = le64_to_cpu((__le64)utrd->command_desc_base_addr_hi << 32 | + utrd->command_desc_base_addr_lo) & CQE_UCD_BA; + if (addr == match) { + ret = true; + goto out; + } + i = (i + 1) & mask; + } + +out: + spin_unlock(&hwq->sq_lock); + return ret; +} diff --git a/drivers/ufs/core/ufshcd-priv.h b/drivers/ufs/core/ufshcd-priv.h index 529f850..e25a852 100644 --- a/drivers/ufs/core/ufshcd-priv.h +++ b/drivers/ufs/core/ufshcd-priv.h @@ -78,6 +78,8 @@ struct ufs_hw_queue *ufshcd_mcq_req_to_hwq(struct ufs_hba *hba, unsigned long ufshcd_mcq_poll_cqe_lock(struct ufs_hba *hba, struct ufs_hw_queue *hwq); +int ufshcd_mcq_sq_cleanup(struct ufs_hba *hba, int task_tag, int *result); + #define UFSHCD_MCQ_IO_QUEUE_OFFSET 1 #define SD_ASCII_STD true #define SD_RAW false @@ -403,4 +405,16 @@ static inline struct cq_entry *ufshcd_mcq_cur_cqe(struct ufs_hw_queue *q) return cqe + q->cq_head_slot; } + +static inline void ufshcd_mcq_update_sq_head_slot(struct ufs_hw_queue *q) +{ + u32 val = readl(q->mcq_sq_head); + + q->sq_head_slot = val / sizeof(struct utp_transfer_req_desc); +} + +static inline bool ufshcd_mcq_is_sq_empty(struct ufs_hw_queue *q) +{ + return q->sq_head_slot == q->sq_tail_slot; +} #endif /* _UFSHCD_PRIV_H_ */ diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index 629442c..49a05909 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -56,7 +56,8 @@ #define NOP_OUT_RETRIES 10 /* Timeout after 50 msecs if NOP OUT hangs without response */ #define NOP_OUT_TIMEOUT 50 /* msecs */ - +/* Maximum MCQ registers polling time */ +#define MCQ_POLL_TIMEOUT 500 /* Query request retries */ #define QUERY_REQ_RETRIES 3 /* Query request timeout */ diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h index 05e4164..6ee532e0 100644 --- a/include/ufs/ufshcd.h +++ b/include/ufs/ufshcd.h @@ -1105,6 +1105,7 @@ struct ufs_hw_queue { u32 max_entries; u32 id; u32 sq_tail_slot; + u32 sq_head_slot; spinlock_t sq_lock; u32 cq_tail_slot; u32 cq_head_slot; diff --git a/include/ufs/ufshci.h b/include/ufs/ufshci.h index 11424bb..252e731 100644 --- a/include/ufs/ufshci.h +++ b/include/ufs/ufshci.h @@ -99,6 +99,9 @@ enum { enum { REG_SQHP = 0x0, REG_SQTP = 0x4, + REG_SQRTC = 0x8, + REG_SQCTI = 0xC, + REG_SQRTS = 0x10, }; enum { @@ -111,6 +114,19 @@ enum { REG_CQIE = 0x4, }; +enum { + SQ_START = 0x0, + SQ_STOP = 0x1, + SQ_ICU = 0x2, +}; + +enum { + SQ_STS = 0x1, + SQ_CUS = 0x2, +}; + +#define SQ_ICU_ERR_CODE_MASK GENMASK(7, 4) +#define UPIU_COMMAND_TYPE_MASK GENMASK(31, 28) #define UFS_MASK(mask, offset) ((mask) << (offset)) /* UFS Version 08h */
Add supporting functions to handle ufs abort in mcq mode. Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com> --- drivers/ufs/core/ufs-mcq.c | 181 +++++++++++++++++++++++++++++++++++++++++ drivers/ufs/core/ufshcd-priv.h | 14 ++++ drivers/ufs/core/ufshcd.c | 3 +- include/ufs/ufshcd.h | 1 + include/ufs/ufshci.h | 16 ++++ 5 files changed, 214 insertions(+), 1 deletion(-)