Messages in this thread | | | Date | Thu, 25 Aug 2022 11:04:10 -0700 | Subject | Re: [RFC PATCH v2 1/2] scsi: ufs: Add Multi-Circular Queue support | From | Bart Van Assche <> |
| |
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.
| |