From: ygardi@codeaurora.org
To: "Akinobu Mita" <akinobu.mita@gmail.com>
Cc: "Yaniv Gardi" <ygardi@codeaurora.org>,
"Rob Herring" <robherring2@gmail.com>,
"Jej B" <james.bottomley@hansenpartnership.com>,
"Paul Bolle" <pebolle@tiscali.nl>,
"Christoph Hellwig" <hch@infradead.org>,
"LKML" <linux-kernel@vger.kernel.org>,
"linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
linux-arm-msm@vger.kernel.org, "Santosh Y" <santoshsy@gmail.com>,
linux-scsi-owner@vger.kernel.org,
"Subhash Jadavani" <subhashj@codeaurora.org>,
"Gilad Broner" <gbroner@codeaurora.org>,
"Dolev Raviv" <draviv@codeaurora.org>,
"Vinayak Holikatti" <vinholikatti@gmail.com>,
"James E.J. Bottomley" <jbottomley@odin.com>
Subject: Re: [PATCH v3 13/15] scsi: ufs: add missing memory barriers
Date: Mon, 26 Oct 2015 12:35:05 -0000 [thread overview]
Message-ID: <e8df00bf987f9d15d0da562498670dd7.squirrel@www.codeaurora.org> (raw)
In-Reply-To: <CAC5umygdHdYt7OuJyxg=-77bA5GctO9OgrUCGo9iY-ukjkxQUA@mail.gmail.com>
> 2015-10-25 23:40 GMT+09:00 <ygardi@codeaurora.org>:
>>> 2015-09-02 19:13 GMT+09:00 Yaniv Gardi <ygardi@codeaurora.org>:
>>>> Performing several writes to UFS host controller registers has
>>>> no gurrantee of ordering, so we must make sure register writes
>>>> to setup request list base address etc. are performed before the
>>>> run/stop register is enabled.
>>>> In addition, when setting up a task request, we must make sure
>>>> the updating of descriptors takes places before ringing the
>>>> doorbell, similarly to setting up a transfer request.
>>>>
>>>> Signed-off-by: Gilad Broner <gbroner@codeaurora.org>
>>>> Signed-off-by: Yaniv Gardi <ygardi@codeaurora.org>
>>>>
>>>> ---
>>>> drivers/scsi/ufs/ufshcd.c | 21 +++++++++++++++------
>>>> 1 file changed, 15 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>>>> index fc2a52d..298511a 100644
>>>> --- a/drivers/scsi/ufs/ufshcd.c
>>>> +++ b/drivers/scsi/ufs/ufshcd.c
>>>> @@ -401,11 +401,9 @@ static inline int ufshcd_get_lists_status(u32
>>>> reg)
>>>> * 1 UTRLRDY
>>>> * 2 UTMRLRDY
>>>> * 3 UCRDY
>>>> - * 4 HEI
>>>> - * 5 DEI
>>>> - * 6-7 reserved
>>>> + * 4-7 reserved
>>>> */
>>>> - return (((reg) & (0xFF)) >> 1) ^ (0x07);
>>>> + return ((reg & 0xFF) >> 1) ^ 0x07;
>>>> }
>>>>
>>>> /**
>>>> @@ -2726,7 +2724,7 @@ out:
>>>> * To bring UFS host controller to operational state,
>>>> * 1. Enable required interrupts
>>>> * 2. Configure interrupt aggregation
>>>> - * 3. Program UTRL and UTMRL base addres
>>>> + * 3. Program UTRL and UTMRL base address
>>>> * 4. Configure run-stop-registers
>>>> *
>>>> * Returns 0 on success, non-zero value on failure
>>>> @@ -2756,8 +2754,13 @@ static int ufshcd_make_hba_operational(struct
>>>> ufs_hba *hba)
>>>> REG_UTP_TASK_REQ_LIST_BASE_H);
>>>>
>>>> /*
>>>> + * Make sure base address and interrupt setup are updated
>>>> before
>>>> + * enabling the run/stop registers below.
>>>> + */
>>>> + wmb();
>>>> +
>>>> + /*
>>>> * UCRDY, UTMRLDY and UTRLRDY bits must be 1
>>>> - * DEI, HEI bits must be 0
>>>> */
>>>> reg = ufshcd_readl(hba, REG_CONTROLLER_STATUS);
>>>> if (!(ufshcd_get_lists_status(reg))) {
>>>> @@ -3920,7 +3923,13 @@ static int ufshcd_issue_tm_cmd(struct ufs_hba
>>>> *hba, int lun_id, int task_id,
>>>>
>>>> /* send command to the controller */
>>>> __set_bit(free_slot, &hba->outstanding_tasks);
>>>> +
>>>> + /* Make sure descriptors are ready before ringing the task
>>>> doorbell */
>>>> + wmb();
>>>> +
>>>> ufshcd_writel(hba, 1 << free_slot,
>>>> REG_UTP_TASK_REQ_DOOR_BELL);
>>>> + /* Make sure that doorbell is committed immediately */
>>>> + wmb();
>>>
>>> Is this wmb() after ringing tm doorbell is needed?
>>
>> Well, Mita, in the case of DB register (Request DB and TASK DB), every
>> write operation to the DB should be barrier, as if not, out of order
>> writing to this register might cause inconsistency in its value, and
>> thus,
>> un-handled requests/tasks.
>
> I couldn't fully understand why out of order writing to TASK DB
> register causes inconsistency.
> In the TASK request completion (ufshcd_tmc_handler), TASK DB register
> is read before handling finished requests, so it ensures that all
> write operations for TASK DB have been performed.
>
thanks, Mita
your explanation makes sense.
It will be fixed in v4.
>>>
>>>>
>>>> spin_unlock_irqrestore(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@vger.kernel.org
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>>
>>
>
next prev parent reply other threads:[~2015-10-26 12:35 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-02 10:13 [PATCH v3 00/15] Big fixes, retries, handle a race condition Yaniv Gardi
2015-09-02 10:13 ` [PATCH v3 01/15] scsi: ufs: clear UTRD, UPIU req and rsp before new transfers Yaniv Gardi
2015-09-02 10:13 ` [PATCH v3 02/15] scsi: ufs: clear fields " Yaniv Gardi
2015-10-22 7:14 ` subhashj
2015-09-02 10:13 ` [PATCH v3 03/15] scsi: ufs: verify command tag validity Yaniv Gardi
2015-10-21 14:48 ` Akinobu Mita
2015-10-25 13:29 ` ygardi
2015-10-22 7:17 ` subhashj
2015-09-02 10:13 ` [PATCH v3 04/15] scsi: ufs: clear outstanding_request bit in case query timeout Yaniv Gardi
2015-10-22 7:17 ` subhashj
2015-09-02 10:13 ` [PATCH v3 05/15] scsi: ufs: increase fDeviceInit query response timeout Yaniv Gardi
2015-10-23 11:18 ` Akinobu Mita
2015-10-25 13:38 ` ygardi
2015-10-26 12:59 ` Akinobu Mita
2015-09-02 10:13 ` [PATCH v3 06/15] scsi: ufs: avoid exception event handler racing with PM callbacks Yaniv Gardi
2015-09-02 10:13 ` [PATCH v3 07/15] scsi: ufs: set REQUEST_SENSE command size to 18 bytes Yaniv Gardi
2015-10-21 14:51 ` Akinobu Mita
2015-09-02 10:13 ` [PATCH v3 08/15] scsi: ufs: add retries to dme_peer get and set attribute Yaniv Gardi
2015-09-02 10:13 ` [PATCH v3 09/15] scsi: ufs: add retries for hibern8 enter Yaniv Gardi
2015-10-21 14:54 ` Akinobu Mita
2015-10-25 13:45 ` ygardi
2015-09-02 10:13 ` [PATCH v3 10/15] scsi: ufs: fix error recovery after the hibern8 exit failure Yaniv Gardi
2015-09-02 10:13 ` [PATCH v3 11/15] scsi: ufs: retry failed query flag requests Yaniv Gardi
2015-09-02 10:13 ` [PATCH v3 12/15] scsi: ufs: reduce the interrupts for power mode change requests Yaniv Gardi
2015-10-21 14:57 ` Akinobu Mita
2015-10-22 13:19 ` Akinobu Mita
2015-10-25 14:34 ` ygardi
2015-09-02 10:13 ` [PATCH v3 13/15] scsi: ufs: add missing memory barriers Yaniv Gardi
2015-10-22 13:21 ` Akinobu Mita
2015-10-25 14:40 ` ygardi
2015-10-26 12:23 ` Akinobu Mita
2015-10-26 12:35 ` ygardi [this message]
2015-10-27 10:21 ` ygardi
2015-09-02 10:13 ` [PATCH v3 14/15] scsi: ufs: commit descriptors before setting the doorbell Yaniv Gardi
2015-09-02 10:13 ` [PATCH v3 15/15] scsi: ufs: add wrapper for retrying sending query attribute Yaniv Gardi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e8df00bf987f9d15d0da562498670dd7.squirrel@www.codeaurora.org \
--to=ygardi@codeaurora.org \
--cc=akinobu.mita@gmail.com \
--cc=draviv@codeaurora.org \
--cc=gbroner@codeaurora.org \
--cc=hch@infradead.org \
--cc=james.bottomley@hansenpartnership.com \
--cc=jbottomley@odin.com \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi-owner@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=pebolle@tiscali.nl \
--cc=robherring2@gmail.com \
--cc=santoshsy@gmail.com \
--cc=subhashj@codeaurora.org \
--cc=vinholikatti@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).