On 8/24/22 18:42, Asutosh Das (asd) wrote:
On 8/18/2022 7:41 PM, Bart Van Assche wrote:
On 8/11/22 03:33, Can Guo wrote:
+static inline void ufshcd_mcq_process_event(struct ufs_hba *hba,
+ struct ufs_hw_queue *hwq)
+{
+ struct cq_entry *cqe = ufshcd_mcq_cur_cqe(hwq);
+ int tag;
+
+ tag = ufshcd_mcq_get_tag(hba, hwq, cqe);
+ ufshcd_compl_one_task(hba, tag, cqe);
+}
Consider changing "process_event" into "process_cqe". Consider
renaming ufshcd_compl_one_task() into ufshcd_compl_one_cqe().
The preparatory patch that would precede this change would define
ufshcd_compl_one_task() in ufshcd.c. Since this function would be
invoked both from Single Doorbell mode and MCQ mode,
ufshcd_compl_one_task() sounds more relevant. What say?
The name "task" is confusing since in SCSI standard documents it refers
to "task management" while ufshcd_compl_one_task() is not related to
SCSI task management at all. So I would appreciate it if another name is
chosen than ufshcd_compl_one_task().
+static irqreturn_t ufshcd_handle_mcq_cq_events(struct ufs_hba *hba)
+{
+ struct ufs_hw_queue *hwq;
+ unsigned long outstanding_cqs;
+ unsigned int nr_queues;
+ int i, ret;
+ u32 events;
+
+ ret = ufshcd_vops_get_outstanding_cqs(hba, &outstanding_cqs);
+ if (ret)
+ outstanding_cqs = (1U << hba->nr_hw_queues) - 1;
+
+ /* Exclude the poll queues */
+ nr_queues = hba->nr_hw_queues - hba->nr_queues[HCTX_TYPE_POLL];
+ for_each_set_bit(i, &outstanding_cqs, nr_queues) {
+ hwq = &hba->uhq[i];
+
+ events = ufshcd_mcq_read_cqis(hba, i);
+ if (events)
+ ufshcd_mcq_write_cqis(hba, events, i);
+
+ if (events & UFSHCD_MCQ_CQIS_TEPS)
+ ufshcd_mcq_poll_cqe_nolock(hba, hwq);
+ }
+
+ return IRQ_HANDLED;
+}
Why the loop over the completion queues? Shouldn't UFSHCI 4.0
compliant controllers support one interrupt per completion queue?
MCQ specification doesn't define that UFSHCI 4.0 compliant HC should
support one interrupt per completion queue. I guess it would depend on
HC vendors. But it specifies ESI as an alternate method; which is
implemented in this patch.
It is unfortunate that support for the ESI mechanism is optional in the
UFSHCI 4.0 specification since I consider this as one of the most
important UFSHCI 4.0 features. I wouldn't mind if MCQ would only be
supported for UFSHCI 4.0 controllers that support ESI.
+ if (hba->nutrs != old_nutrs) {
+ ufshcd_release_sdb_queue(hba, old_nutrs);
+ ret = ufshcd_memory_alloc(hba);
+ if (ret)
+ return ret;
+ ufshcd_host_memory_configure(hba);
+ }
Can this freeing + reallocating be avoided?
Umm, we thought about this. Only after reading the device params, the
ext_iid support and the device queue depth be determined. So didn't look
like there's any other way than this. If you have any ideas, please let
us know.
