| Message ID | 20230601225048.12228-1-stanley.chu@mediatek.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2] ufs: core: Combine ufshcd_mq_poll_cqe functions | expand |
>Currently, ufshcd_mcq_poll_cqe_nolock() is only called by >ufshcd_mcq_poll_cqe_lock() with the addition of a spinlock wrapper >for ufshcd_mcq_poll_cqe_nolock(). Combining these two functions >into one would result in cleaner code. > >Reviewed-by: Bao D. Nguyen <quic_nguyenb@quicinc.com> >Signed-off-by: Stanley Chu <stanley.chu@mediatek.com> >--- > drivers/ufs/core/ufs-mcq.c | 17 ++++------------- > 1 file changed, 4 insertions(+), 13 deletions(-) > >diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c >index 920eb954f594..785fc9762cad 100644 >--- a/drivers/ufs/core/ufs-mcq.c >+++ b/drivers/ufs/core/ufs-mcq.c >@@ -307,11 +307,13 @@ void ufshcd_mcq_compl_all_cqes_lock(struct ufs_hba *hba, > spin_unlock_irqrestore(&hwq->cq_lock, flags); > } > >-static unsigned long ufshcd_mcq_poll_cqe_nolock(struct ufs_hba *hba, >- struct ufs_hw_queue *hwq) >+unsigned long ufshcd_mcq_poll_cqe_lock(struct ufs_hba *hba, >+ struct ufs_hw_queue *hwq) > { > unsigned long completed_reqs = 0; >+ unsigned long flags; > >+ spin_lock_irqsave(&hwq->cq_lock, flags); > ufshcd_mcq_update_cq_tail_slot(hwq); > while (!ufshcd_mcq_is_cq_empty(hwq)) { > ufshcd_mcq_process_cqe(hba, hwq); >@@ -321,17 +323,6 @@ static unsigned long ufshcd_mcq_poll_cqe_nolock(struct ufs_hba *hba, > > if (completed_reqs) > ufshcd_mcq_update_cq_head(hwq); >- >- return completed_reqs; >-} >- >-unsigned long ufshcd_mcq_poll_cqe_lock(struct ufs_hba *hba, >- struct ufs_hw_queue *hwq) >-{ >- unsigned long completed_reqs, flags; >- >- spin_lock_irqsave(&hwq->cq_lock, flags); >- completed_reqs = ufshcd_mcq_poll_cqe_nolock(hba, hwq); > spin_unlock_irqrestore(&hwq->cq_lock, flags); > > return completed_reqs; >-- >2.18.0 > Looks good to me. Reviewed-by: Keoseong Park <keosung.park@samsung.com> Best Regards, Keoseong
On 02.06.23 12:50 AM, Stanley Chu wrote: > Currently, ufshcd_mcq_poll_cqe_nolock() is only called by > ufshcd_mcq_poll_cqe_lock() with the addition of a spinlock wrapper > for ufshcd_mcq_poll_cqe_nolock(). Combining these two functions > into one would result in cleaner code. > > Reviewed-by: Bao D. Nguyen<quic_nguyenb@quicinc.com> > Signed-off-by: Stanley Chu<stanley.chu@mediatek.com> Acked-by: Bean Huo <beanhuo@micron.com>
On 6/1/23 15:50, Stanley Chu wrote: > Currently, ufshcd_mcq_poll_cqe_nolock() is only called by > ufshcd_mcq_poll_cqe_lock() with the addition of a spinlock wrapper > for ufshcd_mcq_poll_cqe_nolock(). Combining these two functions > into one would result in cleaner code. For future patches, please use the imperative mood for the patch description ("would result in" -> "results in"). Additionally, a follow-up patch that renames ufshcd_mcq_poll_cqe_lock() into ufshcd_mcq_poll_cqe() would be welcome. Anyway: Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Stanley, > Currently, ufshcd_mcq_poll_cqe_nolock() is only called by > ufshcd_mcq_poll_cqe_lock() with the addition of a spinlock wrapper > for ufshcd_mcq_poll_cqe_nolock(). Combining these two functions > into one would result in cleaner code. Applied to 6.5/scsi-staging, thanks!
diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c index 920eb954f594..785fc9762cad 100644 --- a/drivers/ufs/core/ufs-mcq.c +++ b/drivers/ufs/core/ufs-mcq.c @@ -307,11 +307,13 @@ void ufshcd_mcq_compl_all_cqes_lock(struct ufs_hba *hba, spin_unlock_irqrestore(&hwq->cq_lock, flags); } -static unsigned long ufshcd_mcq_poll_cqe_nolock(struct ufs_hba *hba, - struct ufs_hw_queue *hwq) +unsigned long ufshcd_mcq_poll_cqe_lock(struct ufs_hba *hba, + struct ufs_hw_queue *hwq) { unsigned long completed_reqs = 0; + unsigned long flags; + spin_lock_irqsave(&hwq->cq_lock, flags); ufshcd_mcq_update_cq_tail_slot(hwq); while (!ufshcd_mcq_is_cq_empty(hwq)) { ufshcd_mcq_process_cqe(hba, hwq); @@ -321,17 +323,6 @@ static unsigned long ufshcd_mcq_poll_cqe_nolock(struct ufs_hba *hba, if (completed_reqs) ufshcd_mcq_update_cq_head(hwq); - - return completed_reqs; -} - -unsigned long ufshcd_mcq_poll_cqe_lock(struct ufs_hba *hba, - struct ufs_hw_queue *hwq) -{ - unsigned long completed_reqs, flags; - - spin_lock_irqsave(&hwq->cq_lock, flags); - completed_reqs = ufshcd_mcq_poll_cqe_nolock(hba, hwq); spin_unlock_irqrestore(&hwq->cq_lock, flags); return completed_reqs;