2015-10-25 23:40 GMT+09:00 <ygardi@xxxxxxxxxxxxxx>: >> 2015-09-02 19:13 GMT+09:00 Yaniv Gardi <ygardi@xxxxxxxxxxxxxx>: >>> 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@xxxxxxxxxxxxxx> >>> Signed-off-by: Yaniv Gardi <ygardi@xxxxxxxxxxxxxx> >>> >>> --- >>> 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. >> >>> >>> 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@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