Re: [PATCH v1 14/15] scsi: ufs: commit descriptors before setting the doorbell

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 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-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux