> On Aug 21, 2015 3:10 PM, "Yaniv Gardi" <ygardi@xxxxxxxxxxxxxx> wrote: >> >> Add a write memory barrier to make sure descriptors prepared are >> actually >> written to memory before ringing the doorbell. We have also added the >> write memory barrier after ringing the doorbell register so that >> controller sees the new request immediately. >> >> Signed-off-by: Yaniv Gardi <ygardi@xxxxxxxxxxxxxx> >> >> --- >> drivers/scsi/ufs/ufshcd.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c >> index fef0660..876148b 100644 >> --- a/drivers/scsi/ufs/ufshcd.c >> +++ b/drivers/scsi/ufs/ufshcd.c >> @@ -833,6 +833,8 @@ void ufshcd_send_command(struct ufs_hba *hba, >> unsigned int task_tag) >> ufshcd_clk_scaling_start_busy(hba); >> __set_bit(task_tag, &hba->outstanding_reqs); >> ufshcd_writel(hba, 1 << task_tag, >> REG_UTP_TRANSFER_REQ_DOOR_BELL); >> + /* Make sure that doorbell is committed immediately */ >> + wmb(); > > Is this really necessary? Is there a measurable difference? I'm not sure if there is a measurable difference, but as the Door-Bell register is the one that actually responsible for the HW execution of the requests, anyhow, it's recommended to its value will be written instantly to the memory. Also, as the Interrupt context reads this register, and compare it to the SW mirroring value (hba->outstanding_reqs) in order to realize what requests are already completed, it's important to get the correct value by reading this register, otherwise we might realize a request completion while it was never even submitted. > >> } >> >> /** >> @@ -1418,6 +1420,8 @@ static int ufshcd_queuecommand(struct Scsi_Host >> *host, struct scsi_cmnd *cmd) >> goto out; >> } >> >> + /* Make sure descriptors are ready before ringing the doorbell >> */ >> + wmb(); > > The writel for the doorbell will do a barrier first. (I didn't check > what exactly ufshcd_writel does, but that is why I don't like these > private access wrappers.) > >> /* issue command to the controller */ >> spin_lock_irqsave(hba->host->host_lock, flags); >> ufshcd_send_command(hba, tag); >> @@ -1627,6 +1631,8 @@ static int ufshcd_exec_dev_cmd(struct ufs_hba >> *hba, >> >> hba->dev_cmd.complete = &wait; >> >> + /* Make sure descriptors are ready before ringing the doorbell >> */ >> + wmb(); >> spin_lock_irqsave(hba->host->host_lock, flags); >> ufshcd_send_command(hba, tag); >> spin_unlock_irqrestore(hba->host->host_lock, flags); >> -- >> 1.8.5.2 >> >> -- >> QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a >> member of Code Aurora Forum, hosted by The Linux Foundation > -- > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
