On 7/1/2021 2:12 PM, Bart Van Assche wrote:
From arch/arm/include/asm/io.h
#define __iowmb() wmb()
[ ... ]
#define writel(v,c) ({ __iowmb(); writel_relaxed(v,c); })
From Documentation/memory-barriers.txt: "Note that, when using writel(), a
prior wmb() is not needed to guarantee that the cache coherent memory
writes have completed before writing to the MMIO region."
In other words, calling wmb() before writel() is not necessary. Hence
remove the wmb() calls that precede a writel() call. Remove the wmb()
calls that precede a ufshcd_send_command() call since the latter function
uses writel(). Remove the wmb() call from ufshcd_wait_for_dev_cmd()
since the following chain of events guarantees that the CPU will see
up-to-date LRB values:
* UFS controller writes to host memory.
* UFS controller posts completion interrupt after the memory writes from
the previous step are visible to the CPU.
* complete(hba->dev_cmd.complete) is called from the UFS interrupt handler.
* The wait_for_completion(hba->dev_cmd.complete) call in
ufshcd_wait_for_dev_cmd() returns.
Cc: Adrian Hunter <adrian.hunter@xxxxxxxxx>
Cc: Stanley Chu <stanley.chu@xxxxxxxxxxxx>
Cc: Can Guo <cang@xxxxxxxxxxxxxx>
Cc: Asutosh Das <asutoshd@xxxxxxxxxxxxxx>
Cc: Avri Altman <avri.altman@xxxxxxx>
Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx>
---
drivers/scsi/ufs/ufshcd.c | 11 -----------
1 file changed, 11 deletions(-)
Hi Bart,
Did you verify this change? I think we've seen OCS errors which were
fixed with the wmb() in queuecommand.
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 7091798e6245..25ab603acad1 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -2768,8 +2768,6 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd)
ufshcd_release(hba);
goto out;
}
- /* Make sure descriptors are ready before ringing the doorbell */
- wmb();
ufshcd_send_command(hba, tag);
out:
@@ -2879,8 +2877,6 @@ static int ufshcd_wait_for_dev_cmd(struct ufs_hba *hba,
time_left = wait_for_completion_timeout(hba->dev_cmd.complete,
msecs_to_jiffies(max_timeout));
- /* Make sure descriptors are ready before ringing the doorbell */
- wmb();
spin_lock_irqsave(hba->host->host_lock, flags);
hba->dev_cmd.complete = NULL;
if (likely(time_left)) {
@@ -2955,8 +2951,6 @@ static int ufshcd_exec_dev_cmd(struct ufs_hba *hba,
hba->dev_cmd.complete = &wait;
ufshcd_add_query_upiu_trace(hba, UFS_QUERY_SEND, lrbp->ucd_req_ptr);
- /* Make sure descriptors are ready before ringing the doorbell */
- wmb();
ufshcd_send_command(hba, tag);
err = ufshcd_wait_for_dev_cmd(hba, lrbp, timeout);
@@ -6514,9 +6508,6 @@ static int __ufshcd_issue_tm_cmd(struct ufs_hba *hba,
/* send command to the controller */
__set_bit(task_tag, &hba->outstanding_tasks);
- /* Make sure descriptors are ready before ringing the task doorbell */
- wmb();
-
ufshcd_writel(hba, 1 << task_tag, REG_UTP_TASK_REQ_DOOR_BELL);
/* Make sure that doorbell is committed immediately */
wmb();
@@ -6687,8 +6678,6 @@ static int ufshcd_issue_devman_upiu_cmd(struct ufs_hba *hba,
hba->dev_cmd.complete = &wait;
ufshcd_add_query_upiu_trace(hba, UFS_QUERY_SEND, lrbp->ucd_req_ptr);
- /* Make sure descriptors are ready before ringing the doorbell */
- wmb();
ufshcd_send_command(hba, tag);
/*
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
Linux Foundation Collaborative Project
