[PATCH v2 0/2] Add UFS LINERESET handling

[PATCH v2 0/2] Add UFS LINERESET handling

[Can Guo]

PA Layer issues a LINERESET to the PHY at the recovery step in the Power
Mode change operation. If it happens during auto or mannual hibern8 enter,
even if hibern8 enter succeeds, UFS power mode shall be set to PWM-G1 mode
and kept in that mode after exit from hibern8, leading to bad performance.
Handle the LINERESET in the eh_work by restoring power mode to HS mode
after all pending reqs and tasks are cleared from doorbell.

Change since v1:
- Made some cleanup to the 2nd change.

Can Guo (2):
  scsi: ufs: Abort tasks before clear them from doorbell
  scsi: ufs: Handle LINERESET indication in err handler

 drivers/scsi/ufs/ufshcd.c | 283 +++++++++++++++++++++++++++++++---------------
 drivers/scsi/ufs/ufshcd.h |   2 +
 drivers/scsi/ufs/ufshci.h |   1 +
 drivers/scsi/ufs/unipro.h |   3 +
 4 files changed, 196 insertions(+), 93 deletions(-)

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[Can Guo]

To recovery non-fatal errors, no full reset is required, err_handler only
clears those pending TRs/TMRs so that scsi layer can re-issue them. In
current err_handler, TRs are directly cleared from UFS host's doorbell but
not aborted from device side. However, according to the UFSHCI JEDEC spec,
the host software shall use UTP Transfer Request List CLear Register to
clear a task from UFS host's doorbell only when a UTP Transfer Request is
expected to not be completed, e.g. when the host software receives a
“FUNCTION COMPLETE” Task Management response which means a Transfer Request
was aborted. To follow the UFSHCI JEDEC spec, in err_handler, aborts one TR
before clearing it from doorbell.

Signed-off-by: Can Guo <cang@codeaurora.org>
Acked-by: Stanley Chu <stanley.chu@mediatek.com>
---
 drivers/scsi/ufs/ufshcd.c | 143 ++++++++++++++++++++++++++--------------------
 1 file changed, 81 insertions(+), 62 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 06e2439..72afe12 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -238,6 +238,7 @@ static int ufshcd_setup_hba_vreg(struct ufs_hba *hba, bool on);
 static int ufshcd_setup_vreg(struct ufs_hba *hba, bool on);
 static inline int ufshcd_config_vreg_hpm(struct ufs_hba *hba,
 					 struct ufs_vreg *vreg);
+static int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag);
 static int ufshcd_wb_buf_flush_enable(struct ufs_hba *hba);
 static int ufshcd_wb_buf_flush_disable(struct ufs_hba *hba);
 static int ufshcd_wb_ctrl(struct ufs_hba *hba, bool enable);
@@ -5666,8 +5667,8 @@ static void ufshcd_err_handler(struct work_struct *work)
 {
 	struct ufs_hba *hba;
 	unsigned long flags;
-	u32 err_xfer = 0;
-	u32 err_tm = 0;
+	bool err_xfer = false;
+	bool err_tm = false;
 	int err = 0;
 	int tag;
 	bool needs_reset = false;
@@ -5743,7 +5744,7 @@ static void ufshcd_err_handler(struct work_struct *work)
 	spin_unlock_irqrestore(hba->host->host_lock, flags);
 	/* Clear pending transfer requests */
 	for_each_set_bit(tag, &hba->outstanding_reqs, hba->nutrs) {
-		if (ufshcd_clear_cmd(hba, tag)) {
+		if (ufshcd_try_to_abort_task(hba, tag)) {
 			err_xfer = true;
 			goto lock_skip_pending_xfer_clear;
 		}
@@ -6495,7 +6496,7 @@ static void ufshcd_set_req_abort_skip(struct ufs_hba *hba, unsigned long bitmap)
 }
 
 /**
- * ufshcd_abort - abort a specific command
+ * ufshcd_try_to_abort_task - abort a specific task
  * @cmd: SCSI command pointer
  *
  * Abort the pending command in device by sending UFS_ABORT_TASK task management
@@ -6504,6 +6505,80 @@ static void ufshcd_set_req_abort_skip(struct ufs_hba *hba, unsigned long bitmap)
  * issued. To avoid that, first issue UFS_QUERY_TASK to check if the command is
  * really issued and then try to abort it.
  *
+ * Returns zero on success, non-zero on failure
+ */
+static int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
+{
+	struct ufshcd_lrb *lrbp = &hba->lrb[tag];
+	int err = 0;
+	int poll_cnt;
+	u8 resp = 0xF;
+	u32 reg;
+
+	for (poll_cnt = 100; poll_cnt; poll_cnt--) {
+		err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
+				UFS_QUERY_TASK, &resp);
+		if (!err && resp == UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED) {
+			/* cmd pending in the device */
+			dev_err(hba->dev, "%s: cmd pending in the device. tag = %d\n",
+				__func__, tag);
+			break;
+		} else if (!err && resp == UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
+			/*
+			 * cmd not pending in the device, check if it is
+			 * in transition.
+			 */
+			dev_err(hba->dev, "%s: cmd at tag %d not pending in the device.\n",
+				__func__, tag);
+			reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
+			if (reg & (1 << tag)) {
+				/* sleep for max. 200us to stabilize */
+				usleep_range(100, 200);
+				continue;
+			}
+			/* command completed already */
+			dev_err(hba->dev, "%s: cmd at tag %d successfully cleared from DB.\n",
+				__func__, tag);
+			goto out;
+		} else {
+			dev_err(hba->dev,
+				"%s: no response from device. tag = %d, err %d\n",
+				__func__, tag, err);
+			if (!err)
+				err = resp; /* service response error */
+			goto out;
+		}
+	}
+
+	if (!poll_cnt) {
+		err = -EBUSY;
+		goto out;
+	}
+
+	err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
+			UFS_ABORT_TASK, &resp);
+	if (err || resp != UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
+		if (!err) {
+			err = resp; /* service response error */
+			dev_err(hba->dev, "%s: issued. tag = %d, err %d\n",
+				__func__, tag, err);
+		}
+		goto out;
+	}
+
+	err = ufshcd_clear_cmd(hba, tag);
+	if (err)
+		dev_err(hba->dev, "%s: Failed clearing cmd at tag %d, err %d\n",
+			__func__, tag, err);
+
+out:
+	return err;
+}
+
+/**
+ * ufshcd_abort - scsi host template eh_abort_handler callback
+ * @cmd: SCSI command pointer
+ *
  * Returns SUCCESS/FAILED
  */
 static int ufshcd_abort(struct scsi_cmnd *cmd)
@@ -6513,8 +6588,6 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
 	unsigned long flags;
 	unsigned int tag;
 	int err = 0;
-	int poll_cnt;
-	u8 resp = 0xF;
 	struct ufshcd_lrb *lrbp;
 	u32 reg;
 
@@ -6583,63 +6656,9 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
 		goto out;
 	}
 
-	for (poll_cnt = 100; poll_cnt; poll_cnt--) {
-		err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
-				UFS_QUERY_TASK, &resp);
-		if (!err && resp == UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED) {
-			/* cmd pending in the device */
-			dev_err(hba->dev, "%s: cmd pending in the device. tag = %d\n",
-				__func__, tag);
-			break;
-		} else if (!err && resp == UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
-			/*
-			 * cmd not pending in the device, check if it is
-			 * in transition.
-			 */
-			dev_err(hba->dev, "%s: cmd at tag %d not pending in the device.\n",
-				__func__, tag);
-			reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
-			if (reg & (1 << tag)) {
-				/* sleep for max. 200us to stabilize */
-				usleep_range(100, 200);
-				continue;
-			}
-			/* command completed already */
-			dev_err(hba->dev, "%s: cmd at tag %d successfully cleared from DB.\n",
-				__func__, tag);
-			goto out;
-		} else {
-			dev_err(hba->dev,
-				"%s: no response from device. tag = %d, err %d\n",
-				__func__, tag, err);
-			if (!err)
-				err = resp; /* service response error */
-			goto out;
-		}
-	}
-
-	if (!poll_cnt) {
-		err = -EBUSY;
-		goto out;
-	}
-
-	err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
-			UFS_ABORT_TASK, &resp);
-	if (err || resp != UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
-		if (!err) {
-			err = resp; /* service response error */
-			dev_err(hba->dev, "%s: issued. tag = %d, err %d\n",
-				__func__, tag, err);
-		}
-		goto out;
-	}
-
-	err = ufshcd_clear_cmd(hba, tag);
-	if (err) {
-		dev_err(hba->dev, "%s: Failed clearing cmd at tag %d, err %d\n",
-			__func__, tag, err);
+	err = ufshcd_try_to_abort_task(hba, tag);
+	if (err)
 		goto out;
-	}
 
 	spin_lock_irqsave(host->host_lock, flags);
 	__ufshcd_transfer_req_compl(hba, (1UL << tag));
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[Can Guo]

To recovery non-fatal errors, no full reset is required, err_handler only
clears those pending TRs/TMRs so that scsi layer can re-issue them. In
current err_handler, TRs are directly cleared from UFS host's doorbell but
not aborted from device side. However, according to the UFSHCI JEDEC spec,
the host software shall use UTP Transfer Request List CLear Register to
clear a task from UFS host's doorbell only when a UTP Transfer Request is
expected to not be completed, e.g. when the host software receives a
“FUNCTION COMPLETE” Task Management response which means a Transfer Request
was aborted. To follow the UFSHCI JEDEC spec, in err_handler, aborts one TR
before clearing it from doorbell.

Signed-off-by: Can Guo <cang@codeaurora.org>
Acked-by: Stanley Chu <stanley.chu@mediatek.com>
---
 drivers/scsi/ufs/ufshcd.c | 143 ++++++++++++++++++++++++++--------------------
 1 file changed, 81 insertions(+), 62 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 06e2439..72afe12 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -238,6 +238,7 @@ static int ufshcd_setup_hba_vreg(struct ufs_hba *hba, bool on);
 static int ufshcd_setup_vreg(struct ufs_hba *hba, bool on);
 static inline int ufshcd_config_vreg_hpm(struct ufs_hba *hba,
 					 struct ufs_vreg *vreg);
+static int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag);
 static int ufshcd_wb_buf_flush_enable(struct ufs_hba *hba);
 static int ufshcd_wb_buf_flush_disable(struct ufs_hba *hba);
 static int ufshcd_wb_ctrl(struct ufs_hba *hba, bool enable);
@@ -5666,8 +5667,8 @@ static void ufshcd_err_handler(struct work_struct *work)
 {
 	struct ufs_hba *hba;
 	unsigned long flags;
-	u32 err_xfer = 0;
-	u32 err_tm = 0;
+	bool err_xfer = false;
+	bool err_tm = false;
 	int err = 0;
 	int tag;
 	bool needs_reset = false;
@@ -5743,7 +5744,7 @@ static void ufshcd_err_handler(struct work_struct *work)
 	spin_unlock_irqrestore(hba->host->host_lock, flags);
 	/* Clear pending transfer requests */
 	for_each_set_bit(tag, &hba->outstanding_reqs, hba->nutrs) {
-		if (ufshcd_clear_cmd(hba, tag)) {
+		if (ufshcd_try_to_abort_task(hba, tag)) {
 			err_xfer = true;
 			goto lock_skip_pending_xfer_clear;
 		}
@@ -6495,7 +6496,7 @@ static void ufshcd_set_req_abort_skip(struct ufs_hba *hba, unsigned long bitmap)
 }
 
 /**
- * ufshcd_abort - abort a specific command
+ * ufshcd_try_to_abort_task - abort a specific task
  * @cmd: SCSI command pointer
  *
  * Abort the pending command in device by sending UFS_ABORT_TASK task management
@@ -6504,6 +6505,80 @@ static void ufshcd_set_req_abort_skip(struct ufs_hba *hba, unsigned long bitmap)
  * issued. To avoid that, first issue UFS_QUERY_TASK to check if the command is
  * really issued and then try to abort it.
  *
+ * Returns zero on success, non-zero on failure
+ */
+static int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
+{
+	struct ufshcd_lrb *lrbp = &hba->lrb[tag];
+	int err = 0;
+	int poll_cnt;
+	u8 resp = 0xF;
+	u32 reg;
+
+	for (poll_cnt = 100; poll_cnt; poll_cnt--) {
+		err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
+				UFS_QUERY_TASK, &resp);
+		if (!err && resp == UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED) {
+			/* cmd pending in the device */
+			dev_err(hba->dev, "%s: cmd pending in the device. tag = %d\n",
+				__func__, tag);
+			break;
+		} else if (!err && resp == UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
+			/*
+			 * cmd not pending in the device, check if it is
+			 * in transition.
+			 */
+			dev_err(hba->dev, "%s: cmd at tag %d not pending in the device.\n",
+				__func__, tag);
+			reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
+			if (reg & (1 << tag)) {
+				/* sleep for max. 200us to stabilize */
+				usleep_range(100, 200);
+				continue;
+			}
+			/* command completed already */
+			dev_err(hba->dev, "%s: cmd at tag %d successfully cleared from DB.\n",
+				__func__, tag);
+			goto out;
+		} else {
+			dev_err(hba->dev,
+				"%s: no response from device. tag = %d, err %d\n",
+				__func__, tag, err);
+			if (!err)
+				err = resp; /* service response error */
+			goto out;
+		}
+	}
+
+	if (!poll_cnt) {
+		err = -EBUSY;
+		goto out;
+	}
+
+	err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
+			UFS_ABORT_TASK, &resp);
+	if (err || resp != UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
+		if (!err) {
+			err = resp; /* service response error */
+			dev_err(hba->dev, "%s: issued. tag = %d, err %d\n",
+				__func__, tag, err);
+		}
+		goto out;
+	}
+
+	err = ufshcd_clear_cmd(hba, tag);
+	if (err)
+		dev_err(hba->dev, "%s: Failed clearing cmd at tag %d, err %d\n",
+			__func__, tag, err);
+
+out:
+	return err;
+}
+
+/**
+ * ufshcd_abort - scsi host template eh_abort_handler callback
+ * @cmd: SCSI command pointer
+ *
  * Returns SUCCESS/FAILED
  */
 static int ufshcd_abort(struct scsi_cmnd *cmd)
@@ -6513,8 +6588,6 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
 	unsigned long flags;
 	unsigned int tag;
 	int err = 0;
-	int poll_cnt;
-	u8 resp = 0xF;
 	struct ufshcd_lrb *lrbp;
 	u32 reg;
 
@@ -6583,63 +6656,9 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
 		goto out;
 	}
 
-	for (poll_cnt = 100; poll_cnt; poll_cnt--) {
-		err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
-				UFS_QUERY_TASK, &resp);
-		if (!err && resp == UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED) {
-			/* cmd pending in the device */
-			dev_err(hba->dev, "%s: cmd pending in the device. tag = %d\n",
-				__func__, tag);
-			break;
-		} else if (!err && resp == UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
-			/*
-			 * cmd not pending in the device, check if it is
-			 * in transition.
-			 */
-			dev_err(hba->dev, "%s: cmd at tag %d not pending in the device.\n",
-				__func__, tag);
-			reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
-			if (reg & (1 << tag)) {
-				/* sleep for max. 200us to stabilize */
-				usleep_range(100, 200);
-				continue;
-			}
-			/* command completed already */
-			dev_err(hba->dev, "%s: cmd at tag %d successfully cleared from DB.\n",
-				__func__, tag);
-			goto out;
-		} else {
-			dev_err(hba->dev,
-				"%s: no response from device. tag = %d, err %d\n",
-				__func__, tag, err);
-			if (!err)
-				err = resp; /* service response error */
-			goto out;
-		}
-	}
-
-	if (!poll_cnt) {
-		err = -EBUSY;
-		goto out;
-	}
-
-	err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
-			UFS_ABORT_TASK, &resp);
-	if (err || resp != UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
-		if (!err) {
-			err = resp; /* service response error */
-			dev_err(hba->dev, "%s: issued. tag = %d, err %d\n",
-				__func__, tag, err);
-		}
-		goto out;
-	}
-
-	err = ufshcd_clear_cmd(hba, tag);
-	if (err) {
-		dev_err(hba->dev, "%s: Failed clearing cmd at tag %d, err %d\n",
-			__func__, tag, err);
+	err = ufshcd_try_to_abort_task(hba, tag);
+	if (err)
 		goto out;
-	}
 
 	spin_lock_irqsave(host->host_lock, flags);
 	__ufshcd_transfer_req_compl(hba, (1UL << tag));
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[Can Guo]

To recovery non-fatal errors, no full reset is required, err_handler only
clears those pending TRs/TMRs so that scsi layer can re-issue them. In
current err_handler, TRs are directly cleared from UFS host's doorbell but
not aborted from device side. However, according to the UFSHCI JEDEC spec,
the host software shall use UTP Transfer Request List CLear Register to
clear a task from UFS host's doorbell only when a UTP Transfer Request is
expected to not be completed, e.g. when the host software receives a
“FUNCTION COMPLETE” Task Management response which means a Transfer Request
was aborted. To follow the UFSHCI JEDEC spec, in err_handler, aborts one TR
before clearing it from doorbell.

Signed-off-by: Can Guo <cang@codeaurora.org>
Acked-by: Stanley Chu <stanley.chu@mediatek.com>
---
 drivers/scsi/ufs/ufshcd.c | 143 ++++++++++++++++++++++++++--------------------
 1 file changed, 81 insertions(+), 62 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 06e2439..72afe12 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -238,6 +238,7 @@ static int ufshcd_setup_hba_vreg(struct ufs_hba *hba, bool on);
 static int ufshcd_setup_vreg(struct ufs_hba *hba, bool on);
 static inline int ufshcd_config_vreg_hpm(struct ufs_hba *hba,
 					 struct ufs_vreg *vreg);
+static int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag);
 static int ufshcd_wb_buf_flush_enable(struct ufs_hba *hba);
 static int ufshcd_wb_buf_flush_disable(struct ufs_hba *hba);
 static int ufshcd_wb_ctrl(struct ufs_hba *hba, bool enable);
@@ -5666,8 +5667,8 @@ static void ufshcd_err_handler(struct work_struct *work)
 {
 	struct ufs_hba *hba;
 	unsigned long flags;
-	u32 err_xfer = 0;
-	u32 err_tm = 0;
+	bool err_xfer = false;
+	bool err_tm = false;
 	int err = 0;
 	int tag;
 	bool needs_reset = false;
@@ -5743,7 +5744,7 @@ static void ufshcd_err_handler(struct work_struct *work)
 	spin_unlock_irqrestore(hba->host->host_lock, flags);
 	/* Clear pending transfer requests */
 	for_each_set_bit(tag, &hba->outstanding_reqs, hba->nutrs) {
-		if (ufshcd_clear_cmd(hba, tag)) {
+		if (ufshcd_try_to_abort_task(hba, tag)) {
 			err_xfer = true;
 			goto lock_skip_pending_xfer_clear;
 		}
@@ -6495,7 +6496,7 @@ static void ufshcd_set_req_abort_skip(struct ufs_hba *hba, unsigned long bitmap)
 }
 
 /**
- * ufshcd_abort - abort a specific command
+ * ufshcd_try_to_abort_task - abort a specific task
  * @cmd: SCSI command pointer
  *
  * Abort the pending command in device by sending UFS_ABORT_TASK task management
@@ -6504,6 +6505,80 @@ static void ufshcd_set_req_abort_skip(struct ufs_hba *hba, unsigned long bitmap)
  * issued. To avoid that, first issue UFS_QUERY_TASK to check if the command is
  * really issued and then try to abort it.
  *
+ * Returns zero on success, non-zero on failure
+ */
+static int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
+{
+	struct ufshcd_lrb *lrbp = &hba->lrb[tag];
+	int err = 0;
+	int poll_cnt;
+	u8 resp = 0xF;
+	u32 reg;
+
+	for (poll_cnt = 100; poll_cnt; poll_cnt--) {
+		err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
+				UFS_QUERY_TASK, &resp);
+		if (!err && resp == UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED) {
+			/* cmd pending in the device */
+			dev_err(hba->dev, "%s: cmd pending in the device. tag = %d\n",
+				__func__, tag);
+			break;
+		} else if (!err && resp == UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
+			/*
+			 * cmd not pending in the device, check if it is
+			 * in transition.
+			 */
+			dev_err(hba->dev, "%s: cmd at tag %d not pending in the device.\n",
+				__func__, tag);
+			reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
+			if (reg & (1 << tag)) {
+				/* sleep for max. 200us to stabilize */
+				usleep_range(100, 200);
+				continue;
+			}
+			/* command completed already */
+			dev_err(hba->dev, "%s: cmd at tag %d successfully cleared from DB.\n",
+				__func__, tag);
+			goto out;
+		} else {
+			dev_err(hba->dev,
+				"%s: no response from device. tag = %d, err %d\n",
+				__func__, tag, err);
+			if (!err)
+				err = resp; /* service response error */
+			goto out;
+		}
+	}
+
+	if (!poll_cnt) {
+		err = -EBUSY;
+		goto out;
+	}
+
+	err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
+			UFS_ABORT_TASK, &resp);
+	if (err || resp != UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
+		if (!err) {
+			err = resp; /* service response error */
+			dev_err(hba->dev, "%s: issued. tag = %d, err %d\n",
+				__func__, tag, err);
+		}
+		goto out;
+	}
+
+	err = ufshcd_clear_cmd(hba, tag);
+	if (err)
+		dev_err(hba->dev, "%s: Failed clearing cmd at tag %d, err %d\n",
+			__func__, tag, err);
+
+out:
+	return err;
+}
+
+/**
+ * ufshcd_abort - scsi host template eh_abort_handler callback
+ * @cmd: SCSI command pointer
+ *
  * Returns SUCCESS/FAILED
  */
 static int ufshcd_abort(struct scsi_cmnd *cmd)
@@ -6513,8 +6588,6 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
 	unsigned long flags;
 	unsigned int tag;
 	int err = 0;
-	int poll_cnt;
-	u8 resp = 0xF;
 	struct ufshcd_lrb *lrbp;
 	u32 reg;
 
@@ -6583,63 +6656,9 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
 		goto out;
 	}
 
-	for (poll_cnt = 100; poll_cnt; poll_cnt--) {
-		err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
-				UFS_QUERY_TASK, &resp);
-		if (!err && resp == UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED) {
-			/* cmd pending in the device */
-			dev_err(hba->dev, "%s: cmd pending in the device. tag = %d\n",
-				__func__, tag);
-			break;
-		} else if (!err && resp == UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
-			/*
-			 * cmd not pending in the device, check if it is
-			 * in transition.
-			 */
-			dev_err(hba->dev, "%s: cmd at tag %d not pending in the device.\n",
-				__func__, tag);
-			reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
-			if (reg & (1 << tag)) {
-				/* sleep for max. 200us to stabilize */
-				usleep_range(100, 200);
-				continue;
-			}
-			/* command completed already */
-			dev_err(hba->dev, "%s: cmd at tag %d successfully cleared from DB.\n",
-				__func__, tag);
-			goto out;
-		} else {
-			dev_err(hba->dev,
-				"%s: no response from device. tag = %d, err %d\n",
-				__func__, tag, err);
-			if (!err)
-				err = resp; /* service response error */
-			goto out;
-		}
-	}
-
-	if (!poll_cnt) {
-		err = -EBUSY;
-		goto out;
-	}
-
-	err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
-			UFS_ABORT_TASK, &resp);
-	if (err || resp != UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
-		if (!err) {
-			err = resp; /* service response error */
-			dev_err(hba->dev, "%s: issued. tag = %d, err %d\n",
-				__func__, tag, err);
-		}
-		goto out;
-	}
-
-	err = ufshcd_clear_cmd(hba, tag);
-	if (err) {
-		dev_err(hba->dev, "%s: Failed clearing cmd at tag %d, err %d\n",
-			__func__, tag, err);
+	err = ufshcd_try_to_abort_task(hba, tag);
+	if (err)
 		goto out;
-	}
 
 	spin_lock_irqsave(host->host_lock, flags);
 	__ufshcd_transfer_req_compl(hba, (1UL << tag));
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[James Bottomley]

I can't reconcile this hunk:

On Wed, 2020-09-02 at 19:24 -0700, Can Guo wrote:
> @@ -6504,6 +6505,80 @@ static void ufshcd_set_req_abort_skip(struct
> ufs_hba *hba, unsigned long bitmap)
>   * issued. To avoid that, first issue UFS_QUERY_TASK to check if the
> command is
>   * really issued and then try to abort it.
>   *
> + * Returns zero on success, non-zero on failure
> + */
> +static int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
> +{
> +	struct ufshcd_lrb *lrbp = &hba->lrb[tag];
> +	int err = 0;
> +	int poll_cnt;
> +	u8 resp = 0xF;
> +	u32 reg;
> +
> +	for (poll_cnt = 100; poll_cnt; poll_cnt--) {
> +		err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp-
> >task_tag,
> +				UFS_QUERY_TASK, &resp);
> +		if (!err && resp ==
> UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED) {
> +			/* cmd pending in the device */
> +			dev_err(hba->dev, "%s: cmd pending in the
> device. tag = %d\n",
> +				__func__, tag);
> +			break;
> +		} else if (!err && resp ==
> UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
> +			/*
> +			 * cmd not pending in the device, check if
> it is
> +			 * in transition.
> +			 */
> +			dev_err(hba->dev, "%s: cmd at tag %d not
> pending in the device.\n",
> +				__func__, tag);
> +			reg = ufshcd_readl(hba,
> REG_UTP_TRANSFER_REQ_DOOR_BELL);
> +			if (reg & (1 << tag)) {
> +				/* sleep for max. 200us to stabilize
> */
> +				usleep_range(100, 200);
> +				continue;
> +			}
> +			/* command completed already */
> +			dev_err(hba->dev, "%s: cmd at tag %d
> successfully cleared from DB.\n",
> +				__func__, tag);
> +			goto out;
> +		} else {
> +			dev_err(hba->dev,
> +				"%s: no response from device. tag =
> %d, err %d\n",
> +				__func__, tag, err);
> +			if (!err)
> +				err = resp; /* service response
> error */
> +			goto out;
> +		}
> +	}
> +
> +	if (!poll_cnt) {
> +		err = -EBUSY;
> +		goto out;
> +	}
> +
> +	err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
> +			UFS_ABORT_TASK, &resp);
> +	if (err || resp != UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
> +		if (!err) {
> +			err = resp; /* service response error */
> +			dev_err(hba->dev, "%s: issued. tag = %d, err
> %d\n",
> +				__func__, tag, err);
> +		}
> +		goto out;
> +	}
> +
> +	err = ufshcd_clear_cmd(hba, tag);
> +	if (err)
> +		dev_err(hba->dev, "%s: Failed clearing cmd at tag
> %d, err %d\n",
> +			__func__, tag, err);
> +
> +out:
> +	return err;
> +}
> +
> +/**
> + * ufshcd_abort - scsi host template eh_abort_handler callback
> + * @cmd: SCSI command pointer
> + *
>   * Returns SUCCESS/FAILED
>   */
>  static int ufshcd_abort(struct scsi_cmnd *cmd)
> @@ -6513,8 +6588,6 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
>  	unsigned long flags;
>  	unsigned int tag;
>  	int err = 0;
> -	int poll_cnt;
> -	u8 resp = 0xF;
>  	struct ufshcd_lrb *lrbp;
>  	u32 reg;
>  
> @@ -6583,63 +6656,9 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
>  		goto out;
>  	}
>  
> -	for (poll_cnt = 100; poll_cnt; poll_cnt--) {
> -		err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp-
> >task_tag,
> -				UFS_QUERY_TASK, &resp);
> -		if (!err && resp ==
> UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED) {
> -			/* cmd pending in the device */
> -			dev_err(hba->dev, "%s: cmd pending in the
> device. tag = %d\n",
> -				__func__, tag);
> -			break;
> -		} else if (!err && resp ==
> UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
> -			/*
> -			 * cmd not pending in the device, check if
> it is
> -			 * in transition.
> -			 */
> -			dev_err(hba->dev, "%s: cmd at tag %d not
> pending in the device.\n",
> -				__func__, tag);
> -			reg = ufshcd_readl(hba,
> REG_UTP_TRANSFER_REQ_DOOR_BELL);
> -			if (reg & (1 << tag)) {
> -				/* sleep for max. 200us to stabilize
> */
> -				usleep_range(100, 200);
> -				continue;
> -			}
> -			/* command completed already */
> -			dev_err(hba->dev, "%s: cmd at tag %d
> successfully cleared from DB.\n",
> -				__func__, tag);
> -			goto out;
> -		} else {
> -			dev_err(hba->dev,
> -				"%s: no response from device. tag =
> %d, err %d\n",
> -				__func__, tag, err);
> -			if (!err)
> -				err = resp; /* service response
> error */
> -			goto out;
> -		}
> -	}
> -
> -	if (!poll_cnt) {
> -		err = -EBUSY;
> -		goto out;
> -	}
> -
> -	err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
> -			UFS_ABORT_TASK, &resp);
> -	if (err || resp != UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
> -		if (!err) {
> -			err = resp; /* service response error */
> -			dev_err(hba->dev, "%s: issued. tag = %d, err
> %d\n",
> -				__func__, tag, err);
> -		}
> -		goto out;
> -	}
> -
> -	err = ufshcd_clear_cmd(hba, tag);
> -	if (err) {
> -		dev_err(hba->dev, "%s: Failed clearing cmd at tag
> %d, err %d\n",
> -			__func__, tag, err);
> +	err = ufshcd_try_to_abort_task(hba, tag);
> +	if (err)
>  		goto out;
> -	}
>  
>  	spin_lock_irqsave(host->host_lock, flags);
>  	__ufshcd_transfer_req_compl(hba, (1UL << tag));


With the change in this fix:

commit b10178ee7fa88b68a9e8adc06534d2605cb0ec23
Author: Stanley Chu <stanley.chu@mediatek.com>
Date:   Tue Aug 11 16:18:58 2020 +0200

    scsi: ufs: Clean up completed request without interrupt
notification


It looks like there have to be two separate error returns from your new
ufshcd_try_to_abort_function() so it knows to continue with
usfhcd_transfer_req_complete(), or the whole function needs to be
refactored, but if this goes upstream as is it looks like it will
eliminate the bug fix.

James

Re: [PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[James Bottomley]

I can't reconcile this hunk:

On Wed, 2020-09-02 at 19:24 -0700, Can Guo wrote:
> @@ -6504,6 +6505,80 @@ static void ufshcd_set_req_abort_skip(struct
> ufs_hba *hba, unsigned long bitmap)
>   * issued. To avoid that, first issue UFS_QUERY_TASK to check if the
> command is
>   * really issued and then try to abort it.
>   *
> + * Returns zero on success, non-zero on failure
> + */
> +static int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
> +{
> +	struct ufshcd_lrb *lrbp = &hba->lrb[tag];
> +	int err = 0;
> +	int poll_cnt;
> +	u8 resp = 0xF;
> +	u32 reg;
> +
> +	for (poll_cnt = 100; poll_cnt; poll_cnt--) {
> +		err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp-
> >task_tag,
> +				UFS_QUERY_TASK, &resp);
> +		if (!err && resp ==
> UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED) {
> +			/* cmd pending in the device */
> +			dev_err(hba->dev, "%s: cmd pending in the
> device. tag = %d\n",
> +				__func__, tag);
> +			break;
> +		} else if (!err && resp ==
> UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
> +			/*
> +			 * cmd not pending in the device, check if
> it is
> +			 * in transition.
> +			 */
> +			dev_err(hba->dev, "%s: cmd at tag %d not
> pending in the device.\n",
> +				__func__, tag);
> +			reg = ufshcd_readl(hba,
> REG_UTP_TRANSFER_REQ_DOOR_BELL);
> +			if (reg & (1 << tag)) {
> +				/* sleep for max. 200us to stabilize
> */
> +				usleep_range(100, 200);
> +				continue;
> +			}
> +			/* command completed already */
> +			dev_err(hba->dev, "%s: cmd at tag %d
> successfully cleared from DB.\n",
> +				__func__, tag);
> +			goto out;
> +		} else {
> +			dev_err(hba->dev,
> +				"%s: no response from device. tag =
> %d, err %d\n",
> +				__func__, tag, err);
> +			if (!err)
> +				err = resp; /* service response
> error */
> +			goto out;
> +		}
> +	}
> +
> +	if (!poll_cnt) {
> +		err = -EBUSY;
> +		goto out;
> +	}
> +
> +	err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
> +			UFS_ABORT_TASK, &resp);
> +	if (err || resp != UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
> +		if (!err) {
> +			err = resp; /* service response error */
> +			dev_err(hba->dev, "%s: issued. tag = %d, err
> %d\n",
> +				__func__, tag, err);
> +		}
> +		goto out;
> +	}
> +
> +	err = ufshcd_clear_cmd(hba, tag);
> +	if (err)
> +		dev_err(hba->dev, "%s: Failed clearing cmd at tag
> %d, err %d\n",
> +			__func__, tag, err);
> +
> +out:
> +	return err;
> +}
> +
> +/**
> + * ufshcd_abort - scsi host template eh_abort_handler callback
> + * @cmd: SCSI command pointer
> + *
>   * Returns SUCCESS/FAILED
>   */
>  static int ufshcd_abort(struct scsi_cmnd *cmd)
> @@ -6513,8 +6588,6 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
>  	unsigned long flags;
>  	unsigned int tag;
>  	int err = 0;
> -	int poll_cnt;
> -	u8 resp = 0xF;
>  	struct ufshcd_lrb *lrbp;
>  	u32 reg;
>  
> @@ -6583,63 +6656,9 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
>  		goto out;
>  	}
>  
> -	for (poll_cnt = 100; poll_cnt; poll_cnt--) {
> -		err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp-
> >task_tag,
> -				UFS_QUERY_TASK, &resp);
> -		if (!err && resp ==
> UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED) {
> -			/* cmd pending in the device */
> -			dev_err(hba->dev, "%s: cmd pending in the
> device. tag = %d\n",
> -				__func__, tag);
> -			break;
> -		} else if (!err && resp ==
> UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
> -			/*
> -			 * cmd not pending in the device, check if
> it is
> -			 * in transition.
> -			 */
> -			dev_err(hba->dev, "%s: cmd at tag %d not
> pending in the device.\n",
> -				__func__, tag);
> -			reg = ufshcd_readl(hba,
> REG_UTP_TRANSFER_REQ_DOOR_BELL);
> -			if (reg & (1 << tag)) {
> -				/* sleep for max. 200us to stabilize
> */
> -				usleep_range(100, 200);
> -				continue;
> -			}
> -			/* command completed already */
> -			dev_err(hba->dev, "%s: cmd at tag %d
> successfully cleared from DB.\n",
> -				__func__, tag);
> -			goto out;
> -		} else {
> -			dev_err(hba->dev,
> -				"%s: no response from device. tag =
> %d, err %d\n",
> -				__func__, tag, err);
> -			if (!err)
> -				err = resp; /* service response
> error */
> -			goto out;
> -		}
> -	}
> -
> -	if (!poll_cnt) {
> -		err = -EBUSY;
> -		goto out;
> -	}
> -
> -	err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
> -			UFS_ABORT_TASK, &resp);
> -	if (err || resp != UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
> -		if (!err) {
> -			err = resp; /* service response error */
> -			dev_err(hba->dev, "%s: issued. tag = %d, err
> %d\n",
> -				__func__, tag, err);
> -		}
> -		goto out;
> -	}
> -
> -	err = ufshcd_clear_cmd(hba, tag);
> -	if (err) {
> -		dev_err(hba->dev, "%s: Failed clearing cmd at tag
> %d, err %d\n",
> -			__func__, tag, err);
> +	err = ufshcd_try_to_abort_task(hba, tag);
> +	if (err)
>  		goto out;
> -	}
>  
>  	spin_lock_irqsave(host->host_lock, flags);
>  	__ufshcd_transfer_req_compl(hba, (1UL << tag));


With the change in this fix:

commit b10178ee7fa88b68a9e8adc06534d2605cb0ec23
Author: Stanley Chu <stanley.chu@mediatek.com>
Date:   Tue Aug 11 16:18:58 2020 +0200

    scsi: ufs: Clean up completed request without interrupt
notification


It looks like there have to be two separate error returns from your new
ufshcd_try_to_abort_function() so it knows to continue with
usfhcd_transfer_req_complete(), or the whole function needs to be
refactored, but if this goes upstream as is it looks like it will
eliminate the bug fix.

James


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[James Bottomley]

I can't reconcile this hunk:

On Wed, 2020-09-02 at 19:24 -0700, Can Guo wrote:
> @@ -6504,6 +6505,80 @@ static void ufshcd_set_req_abort_skip(struct
> ufs_hba *hba, unsigned long bitmap)
>   * issued. To avoid that, first issue UFS_QUERY_TASK to check if the
> command is
>   * really issued and then try to abort it.
>   *
> + * Returns zero on success, non-zero on failure
> + */
> +static int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
> +{
> +	struct ufshcd_lrb *lrbp = &hba->lrb[tag];
> +	int err = 0;
> +	int poll_cnt;
> +	u8 resp = 0xF;
> +	u32 reg;
> +
> +	for (poll_cnt = 100; poll_cnt; poll_cnt--) {
> +		err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp-
> >task_tag,
> +				UFS_QUERY_TASK, &resp);
> +		if (!err && resp ==
> UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED) {
> +			/* cmd pending in the device */
> +			dev_err(hba->dev, "%s: cmd pending in the
> device. tag = %d\n",
> +				__func__, tag);
> +			break;
> +		} else if (!err && resp ==
> UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
> +			/*
> +			 * cmd not pending in the device, check if
> it is
> +			 * in transition.
> +			 */
> +			dev_err(hba->dev, "%s: cmd at tag %d not
> pending in the device.\n",
> +				__func__, tag);
> +			reg = ufshcd_readl(hba,
> REG_UTP_TRANSFER_REQ_DOOR_BELL);
> +			if (reg & (1 << tag)) {
> +				/* sleep for max. 200us to stabilize
> */
> +				usleep_range(100, 200);
> +				continue;
> +			}
> +			/* command completed already */
> +			dev_err(hba->dev, "%s: cmd at tag %d
> successfully cleared from DB.\n",
> +				__func__, tag);
> +			goto out;
> +		} else {
> +			dev_err(hba->dev,
> +				"%s: no response from device. tag =
> %d, err %d\n",
> +				__func__, tag, err);
> +			if (!err)
> +				err = resp; /* service response
> error */
> +			goto out;
> +		}
> +	}
> +
> +	if (!poll_cnt) {
> +		err = -EBUSY;
> +		goto out;
> +	}
> +
> +	err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
> +			UFS_ABORT_TASK, &resp);
> +	if (err || resp != UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
> +		if (!err) {
> +			err = resp; /* service response error */
> +			dev_err(hba->dev, "%s: issued. tag = %d, err
> %d\n",
> +				__func__, tag, err);
> +		}
> +		goto out;
> +	}
> +
> +	err = ufshcd_clear_cmd(hba, tag);
> +	if (err)
> +		dev_err(hba->dev, "%s: Failed clearing cmd at tag
> %d, err %d\n",
> +			__func__, tag, err);
> +
> +out:
> +	return err;
> +}
> +
> +/**
> + * ufshcd_abort - scsi host template eh_abort_handler callback
> + * @cmd: SCSI command pointer
> + *
>   * Returns SUCCESS/FAILED
>   */
>  static int ufshcd_abort(struct scsi_cmnd *cmd)
> @@ -6513,8 +6588,6 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
>  	unsigned long flags;
>  	unsigned int tag;
>  	int err = 0;
> -	int poll_cnt;
> -	u8 resp = 0xF;
>  	struct ufshcd_lrb *lrbp;
>  	u32 reg;
>  
> @@ -6583,63 +6656,9 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
>  		goto out;
>  	}
>  
> -	for (poll_cnt = 100; poll_cnt; poll_cnt--) {
> -		err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp-
> >task_tag,
> -				UFS_QUERY_TASK, &resp);
> -		if (!err && resp ==
> UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED) {
> -			/* cmd pending in the device */
> -			dev_err(hba->dev, "%s: cmd pending in the
> device. tag = %d\n",
> -				__func__, tag);
> -			break;
> -		} else if (!err && resp ==
> UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
> -			/*
> -			 * cmd not pending in the device, check if
> it is
> -			 * in transition.
> -			 */
> -			dev_err(hba->dev, "%s: cmd at tag %d not
> pending in the device.\n",
> -				__func__, tag);
> -			reg = ufshcd_readl(hba,
> REG_UTP_TRANSFER_REQ_DOOR_BELL);
> -			if (reg & (1 << tag)) {
> -				/* sleep for max. 200us to stabilize
> */
> -				usleep_range(100, 200);
> -				continue;
> -			}
> -			/* command completed already */
> -			dev_err(hba->dev, "%s: cmd at tag %d
> successfully cleared from DB.\n",
> -				__func__, tag);
> -			goto out;
> -		} else {
> -			dev_err(hba->dev,
> -				"%s: no response from device. tag =
> %d, err %d\n",
> -				__func__, tag, err);
> -			if (!err)
> -				err = resp; /* service response
> error */
> -			goto out;
> -		}
> -	}
> -
> -	if (!poll_cnt) {
> -		err = -EBUSY;
> -		goto out;
> -	}
> -
> -	err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
> -			UFS_ABORT_TASK, &resp);
> -	if (err || resp != UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
> -		if (!err) {
> -			err = resp; /* service response error */
> -			dev_err(hba->dev, "%s: issued. tag = %d, err
> %d\n",
> -				__func__, tag, err);
> -		}
> -		goto out;
> -	}
> -
> -	err = ufshcd_clear_cmd(hba, tag);
> -	if (err) {
> -		dev_err(hba->dev, "%s: Failed clearing cmd at tag
> %d, err %d\n",
> -			__func__, tag, err);
> +	err = ufshcd_try_to_abort_task(hba, tag);
> +	if (err)
>  		goto out;
> -	}
>  
>  	spin_lock_irqsave(host->host_lock, flags);
>  	__ufshcd_transfer_req_compl(hba, (1UL << tag));


With the change in this fix:

commit b10178ee7fa88b68a9e8adc06534d2605cb0ec23
Author: Stanley Chu <stanley.chu@mediatek.com>
Date:   Tue Aug 11 16:18:58 2020 +0200

    scsi: ufs: Clean up completed request without interrupt
notification


It looks like there have to be two separate error returns from your new
ufshcd_try_to_abort_function() so it knows to continue with
usfhcd_transfer_req_complete(), or the whole function needs to be
refactored, but if this goes upstream as is it looks like it will
eliminate the bug fix.

James


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[Martin K. Petersen]

Can and Stanley,

> I can't reconcile this hunk:

Please provide a resolution for these conflicting commits in fixes and
queue:

307348f6ab14 scsi: ufs: Abort tasks before clearing them from doorbell

b10178ee7fa8 scsi: ufs: Clean up completed request without interrupt
notification

Thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[Martin K. Petersen]

Can and Stanley,

> I can't reconcile this hunk:

Please provide a resolution for these conflicting commits in fixes and
queue:

307348f6ab14 scsi: ufs: Abort tasks before clearing them from doorbell

b10178ee7fa8 scsi: ufs: Clean up completed request without interrupt
notification

Thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[Martin K. Petersen]

Can and Stanley,

> I can't reconcile this hunk:

Please provide a resolution for these conflicting commits in fixes and
queue:

307348f6ab14 scsi: ufs: Abort tasks before clearing them from doorbell

b10178ee7fa8 scsi: ufs: Clean up completed request without interrupt
notification

Thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[Stanley Chu]

Hi Martin, Can,

On Wed, 2020-09-09 at 22:32 -0400, Martin K. Petersen wrote:
> Can and Stanley,
> 
> > I can't reconcile this hunk:
> 
> Please provide a resolution for these conflicting commits in fixes and
> queue:
> 
> 307348f6ab14 scsi: ufs: Abort tasks before clearing them from doorbell
> 
> b10178ee7fa8 scsi: ufs: Clean up completed request without interrupt
> notification
> 

Can's patch has considered my fix in the new flow.

To be more clear, for the fixing case in my patch,
ufshcd_try_to_abort_task() will return 0 (err = 0) and finally the
target tag can be completed and cleared by __ufshcd_transfer_req_compl()
in Can's new flow.

Thus I think the resolution can just using the code in Can's patch.

Can, please correct me if I was wrong.

Thanks,
Stanley Chu


> Thanks!
> 

Re: [PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[Stanley Chu]

Hi Martin, Can,

On Wed, 2020-09-09 at 22:32 -0400, Martin K. Petersen wrote:
> Can and Stanley,
> 
> > I can't reconcile this hunk:
> 
> Please provide a resolution for these conflicting commits in fixes and
> queue:
> 
> 307348f6ab14 scsi: ufs: Abort tasks before clearing them from doorbell
> 
> b10178ee7fa8 scsi: ufs: Clean up completed request without interrupt
> notification
> 

Can's patch has considered my fix in the new flow.

To be more clear, for the fixing case in my patch,
ufshcd_try_to_abort_task() will return 0 (err = 0) and finally the
target tag can be completed and cleared by __ufshcd_transfer_req_compl()
in Can's new flow.

Thus I think the resolution can just using the code in Can's patch.

Can, please correct me if I was wrong.

Thanks,
Stanley Chu


> Thanks!
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[Stanley Chu]

Hi Martin, Can,

On Wed, 2020-09-09 at 22:32 -0400, Martin K. Petersen wrote:
> Can and Stanley,
> 
> > I can't reconcile this hunk:
> 
> Please provide a resolution for these conflicting commits in fixes and
> queue:
> 
> 307348f6ab14 scsi: ufs: Abort tasks before clearing them from doorbell
> 
> b10178ee7fa8 scsi: ufs: Clean up completed request without interrupt
> notification
> 

Can's patch has considered my fix in the new flow.

To be more clear, for the fixing case in my patch,
ufshcd_try_to_abort_task() will return 0 (err = 0) and finally the
target tag can be completed and cleared by __ufshcd_transfer_req_compl()
in Can's new flow.

Thus I think the resolution can just using the code in Can's patch.

Can, please correct me if I was wrong.

Thanks,
Stanley Chu


> Thanks!
> 

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[James Bottomley]

On Thu, 2020-09-10 at 10:48 +0800, Stanley Chu wrote:
> Hi Martin, Can,
> 
> On Wed, 2020-09-09 at 22:32 -0400, Martin K. Petersen wrote:
> > Can and Stanley,
> > 
> > > I can't reconcile this hunk:
> > 
> > Please provide a resolution for these conflicting commits in fixes
> > and
> > queue:
> > 
> > 307348f6ab14 scsi: ufs: Abort tasks before clearing them from
> > doorbell
> > 
> > b10178ee7fa8 scsi: ufs: Clean up completed request without
> > interrupt
> > notification
> > 
> 
> Can's patch has considered my fix in the new flow.
> 
> To be more clear, for the fixing case in my patch,
> ufshcd_try_to_abort_task() will return 0 (err = 0) and finally the
> target tag can be completed and cleared by
> __ufshcd_transfer_req_compl()
> in Can's new flow.
> 
> Thus I think the resolution can just using the code in Can's patch.
> 
> Can, please correct me if I was wrong.

Well, that really doesn't make for an easy merge. The resolution I took
is below.

James

---

commit 5399a4aa684d491c35a386effe385c06b41398fa
Merge: 59958f7a956b 8c6572356646
Author: James Bottomley <James.Bottomley@HansenPartnership.com>
Date:   Wed Sep 9 23:12:52 2020 -0700

    Merge branch 'misc' into for-next
    
    Conflicts:
            drivers/scsi/ufs/ufshcd.c
            drivers/scsi/ufs/ufshcd.h

diff --cc drivers/scsi/ufs/ufshcd.c
index 34e1ab407b05,05716f62febe..49478c8a601f
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@@ -6574,84 -6739,22 +6736,25 @@@ static int ufshcd_abort(struct scsi_cmn
  	}
  	hba->req_abort_count++;
  
 -	/* Skip task abort in case previous aborts failed and report failure */
 -	if (lrbp->req_abort_skip) {
 -		err = -EIO;
 -		goto out;
 +	if (!(reg & (1 << tag))) {
 +		dev_err(hba->dev,
 +		"%s: cmd was completed, but without a notifying intr, tag = %d",
 +		__func__, tag);
 +		goto cleanup;
  	}
  
 -	err = ufshcd_try_to_abort_task(hba, tag);
 -	if (err)
 -		goto out;
 -
 -	spin_lock_irqsave(host->host_lock, flags);
 -	__ufshcd_transfer_req_compl(hba, (1UL << tag));
 -	spin_unlock_irqrestore(host->host_lock, flags);
 +	/* Skip task abort in case previous aborts failed and report failure */
- 	if (lrbp->req_abort_skip) {
++	if (lrbp->req_abort_skip)
 +		err = -EIO;
- 		goto out;
- 	}
- 
- 	for (poll_cnt = 100; poll_cnt; poll_cnt--) {
- 		err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
- 				UFS_QUERY_TASK, &resp);
- 		if (!err && resp == UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED) {
- 			/* cmd pending in the device */
- 			dev_err(hba->dev, "%s: cmd pending in the device. tag = %d\n",
- 				__func__, tag);
- 			break;
- 		} else if (!err && resp == UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
- 			/*
- 			 * cmd not pending in the device, check if it is
- 			 * in transition.
- 			 */
- 			dev_err(hba->dev, "%s: cmd at tag %d not pending in the device.\n",
- 				__func__, tag);
- 			reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
- 			if (reg & (1 << tag)) {
- 				/* sleep for max. 200us to stabilize */
- 				usleep_range(100, 200);
- 				continue;
- 			}
- 			/* command completed already */
- 			dev_err(hba->dev, "%s: cmd at tag %d successfully cleared from DB.\n",
- 				__func__, tag);
- 			goto cleanup;
- 		} else {
- 			dev_err(hba->dev,
- 				"%s: no response from device. tag = %d, err %d\n",
- 				__func__, tag, err);
- 			if (!err)
- 				err = resp; /* service response error */
- 			goto out;
- 		}
- 	}
- 
- 	if (!poll_cnt) {
- 		err = -EBUSY;
- 		goto out;
- 	}
- 
- 	err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
- 			UFS_ABORT_TASK, &resp);
- 	if (err || resp != UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
- 		if (!err) {
- 			err = resp; /* service response error */
- 			dev_err(hba->dev, "%s: issued. tag = %d, err %d\n",
- 				__func__, tag, err);
- 		}
- 		goto out;
- 	}
- 
- 	err = ufshcd_clear_cmd(hba, tag);
- 	if (err) {
- 		dev_err(hba->dev, "%s: Failed clearing cmd at tag %d, err %d\n",
- 			__func__, tag, err);
- 		goto out;
- 	}
++	else
++		err = ufshcd_try_to_abort_task(hba, tag);
  
 -out:
+ 	if (!err) {
 +cleanup:
- 	spin_lock_irqsave(host->host_lock, flags);
- 	__ufshcd_transfer_req_compl(hba, (1UL << tag));
- 	spin_unlock_irqrestore(host->host_lock, flags);
- 
++		spin_lock_irqsave(host->host_lock, flags);
++		__ufshcd_transfer_req_compl(hba, (1UL << tag));
++		spin_unlock_irqrestore(host->host_lock, flags);
 +out:
- 	if (!err) {
  		err = SUCCESS;
  	} else {
  		dev_err(hba->dev, "%s: failed with err %d\n", __func__, err);
diff --cc drivers/scsi/ufs/ufshcd.h
index b5b2761456fb,8011fdc89fb1..6663325ed8a0
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@@ -531,11 -531,10 +531,16 @@@ enum ufshcd_quirks 
  	 */
  	UFSHCD_QUIRK_BROKEN_OCS_FATAL_ERROR		= 1 << 10,
  
 +	/*
 +	 * This quirk needs to be enabled if the host controller has
 +	 * auto-hibernate capability but it doesn't work.
 +	 */
 +	UFSHCD_QUIRK_BROKEN_AUTO_HIBERN8		= 1 << 11,
++
+ 	/*
+ 	 * This quirk needs to disable manual flush for write booster
+ 	 */
 -	UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL		= 1 << 11,
++	UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL		= 1 << 12,
  };
  
  enum ufshcd_caps {

Re: [PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[James Bottomley]

On Thu, 2020-09-10 at 10:48 +0800, Stanley Chu wrote:
> Hi Martin, Can,
> 
> On Wed, 2020-09-09 at 22:32 -0400, Martin K. Petersen wrote:
> > Can and Stanley,
> > 
> > > I can't reconcile this hunk:
> > 
> > Please provide a resolution for these conflicting commits in fixes
> > and
> > queue:
> > 
> > 307348f6ab14 scsi: ufs: Abort tasks before clearing them from
> > doorbell
> > 
> > b10178ee7fa8 scsi: ufs: Clean up completed request without
> > interrupt
> > notification
> > 
> 
> Can's patch has considered my fix in the new flow.
> 
> To be more clear, for the fixing case in my patch,
> ufshcd_try_to_abort_task() will return 0 (err = 0) and finally the
> target tag can be completed and cleared by
> __ufshcd_transfer_req_compl()
> in Can's new flow.
> 
> Thus I think the resolution can just using the code in Can's patch.
> 
> Can, please correct me if I was wrong.

Well, that really doesn't make for an easy merge. The resolution I took
is below.

James

---

commit 5399a4aa684d491c35a386effe385c06b41398fa
Merge: 59958f7a956b 8c6572356646
Author: James Bottomley <James.Bottomley@HansenPartnership.com>
Date:   Wed Sep 9 23:12:52 2020 -0700

    Merge branch 'misc' into for-next
    
    Conflicts:
            drivers/scsi/ufs/ufshcd.c
            drivers/scsi/ufs/ufshcd.h

diff --cc drivers/scsi/ufs/ufshcd.c
index 34e1ab407b05,05716f62febe..49478c8a601f
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@@ -6574,84 -6739,22 +6736,25 @@@ static int ufshcd_abort(struct scsi_cmn
  	}
  	hba->req_abort_count++;
  
 -	/* Skip task abort in case previous aborts failed and report failure */
 -	if (lrbp->req_abort_skip) {
 -		err = -EIO;
 -		goto out;
 +	if (!(reg & (1 << tag))) {
 +		dev_err(hba->dev,
 +		"%s: cmd was completed, but without a notifying intr, tag = %d",
 +		__func__, tag);
 +		goto cleanup;
  	}
  
 -	err = ufshcd_try_to_abort_task(hba, tag);
 -	if (err)
 -		goto out;
 -
 -	spin_lock_irqsave(host->host_lock, flags);
 -	__ufshcd_transfer_req_compl(hba, (1UL << tag));
 -	spin_unlock_irqrestore(host->host_lock, flags);
 +	/* Skip task abort in case previous aborts failed and report failure */
- 	if (lrbp->req_abort_skip) {
++	if (lrbp->req_abort_skip)
 +		err = -EIO;
- 		goto out;
- 	}
- 
- 	for (poll_cnt = 100; poll_cnt; poll_cnt--) {
- 		err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
- 				UFS_QUERY_TASK, &resp);
- 		if (!err && resp == UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED) {
- 			/* cmd pending in the device */
- 			dev_err(hba->dev, "%s: cmd pending in the device. tag = %d\n",
- 				__func__, tag);
- 			break;
- 		} else if (!err && resp == UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
- 			/*
- 			 * cmd not pending in the device, check if it is
- 			 * in transition.
- 			 */
- 			dev_err(hba->dev, "%s: cmd at tag %d not pending in the device.\n",
- 				__func__, tag);
- 			reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
- 			if (reg & (1 << tag)) {
- 				/* sleep for max. 200us to stabilize */
- 				usleep_range(100, 200);
- 				continue;
- 			}
- 			/* command completed already */
- 			dev_err(hba->dev, "%s: cmd at tag %d successfully cleared from DB.\n",
- 				__func__, tag);
- 			goto cleanup;
- 		} else {
- 			dev_err(hba->dev,
- 				"%s: no response from device. tag = %d, err %d\n",
- 				__func__, tag, err);
- 			if (!err)
- 				err = resp; /* service response error */
- 			goto out;
- 		}
- 	}
- 
- 	if (!poll_cnt) {
- 		err = -EBUSY;
- 		goto out;
- 	}
- 
- 	err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
- 			UFS_ABORT_TASK, &resp);
- 	if (err || resp != UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
- 		if (!err) {
- 			err = resp; /* service response error */
- 			dev_err(hba->dev, "%s: issued. tag = %d, err %d\n",
- 				__func__, tag, err);
- 		}
- 		goto out;
- 	}
- 
- 	err = ufshcd_clear_cmd(hba, tag);
- 	if (err) {
- 		dev_err(hba->dev, "%s: Failed clearing cmd at tag %d, err %d\n",
- 			__func__, tag, err);
- 		goto out;
- 	}
++	else
++		err = ufshcd_try_to_abort_task(hba, tag);
  
 -out:
+ 	if (!err) {
 +cleanup:
- 	spin_lock_irqsave(host->host_lock, flags);
- 	__ufshcd_transfer_req_compl(hba, (1UL << tag));
- 	spin_unlock_irqrestore(host->host_lock, flags);
- 
++		spin_lock_irqsave(host->host_lock, flags);
++		__ufshcd_transfer_req_compl(hba, (1UL << tag));
++		spin_unlock_irqrestore(host->host_lock, flags);
 +out:
- 	if (!err) {
  		err = SUCCESS;
  	} else {
  		dev_err(hba->dev, "%s: failed with err %d\n", __func__, err);
diff --cc drivers/scsi/ufs/ufshcd.h
index b5b2761456fb,8011fdc89fb1..6663325ed8a0
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@@ -531,11 -531,10 +531,16 @@@ enum ufshcd_quirks 
  	 */
  	UFSHCD_QUIRK_BROKEN_OCS_FATAL_ERROR		= 1 << 10,
  
 +	/*
 +	 * This quirk needs to be enabled if the host controller has
 +	 * auto-hibernate capability but it doesn't work.
 +	 */
 +	UFSHCD_QUIRK_BROKEN_AUTO_HIBERN8		= 1 << 11,
++
+ 	/*
+ 	 * This quirk needs to disable manual flush for write booster
+ 	 */
 -	UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL		= 1 << 11,
++	UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL		= 1 << 12,
  };
  
  enum ufshcd_caps {

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[James Bottomley]

On Thu, 2020-09-10 at 10:48 +0800, Stanley Chu wrote:
> Hi Martin, Can,
> 
> On Wed, 2020-09-09 at 22:32 -0400, Martin K. Petersen wrote:
> > Can and Stanley,
> > 
> > > I can't reconcile this hunk:
> > 
> > Please provide a resolution for these conflicting commits in fixes
> > and
> > queue:
> > 
> > 307348f6ab14 scsi: ufs: Abort tasks before clearing them from
> > doorbell
> > 
> > b10178ee7fa8 scsi: ufs: Clean up completed request without
> > interrupt
> > notification
> > 
> 
> Can's patch has considered my fix in the new flow.
> 
> To be more clear, for the fixing case in my patch,
> ufshcd_try_to_abort_task() will return 0 (err = 0) and finally the
> target tag can be completed and cleared by
> __ufshcd_transfer_req_compl()
> in Can's new flow.
> 
> Thus I think the resolution can just using the code in Can's patch.
> 
> Can, please correct me if I was wrong.

Well, that really doesn't make for an easy merge. The resolution I took
is below.

James

---

commit 5399a4aa684d491c35a386effe385c06b41398fa
Merge: 59958f7a956b 8c6572356646
Author: James Bottomley <James.Bottomley@HansenPartnership.com>
Date:   Wed Sep 9 23:12:52 2020 -0700

    Merge branch 'misc' into for-next
    
    Conflicts:
            drivers/scsi/ufs/ufshcd.c
            drivers/scsi/ufs/ufshcd.h

diff --cc drivers/scsi/ufs/ufshcd.c
index 34e1ab407b05,05716f62febe..49478c8a601f
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@@ -6574,84 -6739,22 +6736,25 @@@ static int ufshcd_abort(struct scsi_cmn
  	}
  	hba->req_abort_count++;
  
 -	/* Skip task abort in case previous aborts failed and report failure */
 -	if (lrbp->req_abort_skip) {
 -		err = -EIO;
 -		goto out;
 +	if (!(reg & (1 << tag))) {
 +		dev_err(hba->dev,
 +		"%s: cmd was completed, but without a notifying intr, tag = %d",
 +		__func__, tag);
 +		goto cleanup;
  	}
  
 -	err = ufshcd_try_to_abort_task(hba, tag);
 -	if (err)
 -		goto out;
 -
 -	spin_lock_irqsave(host->host_lock, flags);
 -	__ufshcd_transfer_req_compl(hba, (1UL << tag));
 -	spin_unlock_irqrestore(host->host_lock, flags);
 +	/* Skip task abort in case previous aborts failed and report failure */
- 	if (lrbp->req_abort_skip) {
++	if (lrbp->req_abort_skip)
 +		err = -EIO;
- 		goto out;
- 	}
- 
- 	for (poll_cnt = 100; poll_cnt; poll_cnt--) {
- 		err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
- 				UFS_QUERY_TASK, &resp);
- 		if (!err && resp == UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED) {
- 			/* cmd pending in the device */
- 			dev_err(hba->dev, "%s: cmd pending in the device. tag = %d\n",
- 				__func__, tag);
- 			break;
- 		} else if (!err && resp == UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
- 			/*
- 			 * cmd not pending in the device, check if it is
- 			 * in transition.
- 			 */
- 			dev_err(hba->dev, "%s: cmd at tag %d not pending in the device.\n",
- 				__func__, tag);
- 			reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
- 			if (reg & (1 << tag)) {
- 				/* sleep for max. 200us to stabilize */
- 				usleep_range(100, 200);
- 				continue;
- 			}
- 			/* command completed already */
- 			dev_err(hba->dev, "%s: cmd at tag %d successfully cleared from DB.\n",
- 				__func__, tag);
- 			goto cleanup;
- 		} else {
- 			dev_err(hba->dev,
- 				"%s: no response from device. tag = %d, err %d\n",
- 				__func__, tag, err);
- 			if (!err)
- 				err = resp; /* service response error */
- 			goto out;
- 		}
- 	}
- 
- 	if (!poll_cnt) {
- 		err = -EBUSY;
- 		goto out;
- 	}
- 
- 	err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
- 			UFS_ABORT_TASK, &resp);
- 	if (err || resp != UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
- 		if (!err) {
- 			err = resp; /* service response error */
- 			dev_err(hba->dev, "%s: issued. tag = %d, err %d\n",
- 				__func__, tag, err);
- 		}
- 		goto out;
- 	}
- 
- 	err = ufshcd_clear_cmd(hba, tag);
- 	if (err) {
- 		dev_err(hba->dev, "%s: Failed clearing cmd at tag %d, err %d\n",
- 			__func__, tag, err);
- 		goto out;
- 	}
++	else
++		err = ufshcd_try_to_abort_task(hba, tag);
  
 -out:
+ 	if (!err) {
 +cleanup:
- 	spin_lock_irqsave(host->host_lock, flags);
- 	__ufshcd_transfer_req_compl(hba, (1UL << tag));
- 	spin_unlock_irqrestore(host->host_lock, flags);
- 
++		spin_lock_irqsave(host->host_lock, flags);
++		__ufshcd_transfer_req_compl(hba, (1UL << tag));
++		spin_unlock_irqrestore(host->host_lock, flags);
 +out:
- 	if (!err) {
  		err = SUCCESS;
  	} else {
  		dev_err(hba->dev, "%s: failed with err %d\n", __func__, err);
diff --cc drivers/scsi/ufs/ufshcd.h
index b5b2761456fb,8011fdc89fb1..6663325ed8a0
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@@ -531,11 -531,10 +531,16 @@@ enum ufshcd_quirks 
  	 */
  	UFSHCD_QUIRK_BROKEN_OCS_FATAL_ERROR		= 1 << 10,
  
 +	/*
 +	 * This quirk needs to be enabled if the host controller has
 +	 * auto-hibernate capability but it doesn't work.
 +	 */
 +	UFSHCD_QUIRK_BROKEN_AUTO_HIBERN8		= 1 << 11,
++
+ 	/*
+ 	 * This quirk needs to disable manual flush for write booster
+ 	 */
 -	UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL		= 1 << 11,
++	UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL		= 1 << 12,
  };
  
  enum ufshcd_caps {

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[Stanley Chu]

Hi James,

On Wed, 2020-09-09 at 23:18 -0700, James Bottomley wrote:
> On Thu, 2020-09-10 at 10:48 +0800, Stanley Chu wrote:
> > Hi Martin, Can,
> > 
> > On Wed, 2020-09-09 at 22:32 -0400, Martin K. Petersen wrote:
> > > Can and Stanley,
> > > 
> > > > I can't reconcile this hunk:
> > > 
> > > Please provide a resolution for these conflicting commits in fixes
> > > and
> > > queue:
> > > 
> > > 307348f6ab14 scsi: ufs: Abort tasks before clearing them from
> > > doorbell
> > > 
> > > b10178ee7fa8 scsi: ufs: Clean up completed request without
> > > interrupt
> > > notification
> > > 
> > 
> > Can's patch has considered my fix in the new flow.
> > 
> > To be more clear, for the fixing case in my patch,
> > ufshcd_try_to_abort_task() will return 0 (err = 0) and finally the
> > target tag can be completed and cleared by
> > __ufshcd_transfer_req_compl()
> > in Can's new flow.
> > 
> > Thus I think the resolution can just using the code in Can's patch.
> > 
> > Can, please correct me if I was wrong.
> 
> Well, that really doesn't make for an easy merge. The resolution I took
> is below.
> 
> James
> 
> ---
> 
> commit 5399a4aa684d491c35a386effe385c06b41398fa
> Merge: 59958f7a956b 8c6572356646
> Author: James Bottomley <James.Bottomley@HansenPartnership.com>
> Date:   Wed Sep 9 23:12:52 2020 -0700
> 
>     Merge branch 'misc' into for-next
>     
>     Conflicts:
>             drivers/scsi/ufs/ufshcd.c
>             drivers/scsi/ufs/ufshcd.h
> 
> diff --cc drivers/scsi/ufs/ufshcd.c
> index 34e1ab407b05,05716f62febe..49478c8a601f
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@@ -6574,84 -6739,22 +6736,25 @@@ static int ufshcd_abort(struct scsi_cmn
>   	}
>   	hba->req_abort_count++;
>   
>  -	/* Skip task abort in case previous aborts failed and report failure */
>  -	if (lrbp->req_abort_skip) {
>  -		err = -EIO;
>  -		goto out;
>  +	if (!(reg & (1 << tag))) {
>  +		dev_err(hba->dev,
>  +		"%s: cmd was completed, but without a notifying intr, tag = %d",
>  +		__func__, tag);
>  +		goto cleanup;
>   	}
>   
>  -	err = ufshcd_try_to_abort_task(hba, tag);
>  -	if (err)
>  -		goto out;
>  -
>  -	spin_lock_irqsave(host->host_lock, flags);
>  -	__ufshcd_transfer_req_compl(hba, (1UL << tag));
>  -	spin_unlock_irqrestore(host->host_lock, flags);
>  +	/* Skip task abort in case previous aborts failed and report failure */
> - 	if (lrbp->req_abort_skip) {
> ++	if (lrbp->req_abort_skip)
>  +		err = -EIO;
> - 		goto out;
> - 	}
> - 
> - 	for (poll_cnt = 100; poll_cnt; poll_cnt--) {
> - 		err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
> - 				UFS_QUERY_TASK, &resp);
> - 		if (!err && resp == UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED) {
> - 			/* cmd pending in the device */
> - 			dev_err(hba->dev, "%s: cmd pending in the device. tag = %d\n",
> - 				__func__, tag);
> - 			break;
> - 		} else if (!err && resp == UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
> - 			/*
> - 			 * cmd not pending in the device, check if it is
> - 			 * in transition.
> - 			 */
> - 			dev_err(hba->dev, "%s: cmd at tag %d not pending in the device.\n",
> - 				__func__, tag);
> - 			reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
> - 			if (reg & (1 << tag)) {
> - 				/* sleep for max. 200us to stabilize */
> - 				usleep_range(100, 200);
> - 				continue;
> - 			}
> - 			/* command completed already */
> - 			dev_err(hba->dev, "%s: cmd at tag %d successfully cleared from DB.\n",
> - 				__func__, tag);
> - 			goto cleanup;
> - 		} else {
> - 			dev_err(hba->dev,
> - 				"%s: no response from device. tag = %d, err %d\n",
> - 				__func__, tag, err);
> - 			if (!err)
> - 				err = resp; /* service response error */
> - 			goto out;
> - 		}
> - 	}
> - 
> - 	if (!poll_cnt) {
> - 		err = -EBUSY;
> - 		goto out;
> - 	}
> - 
> - 	err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
> - 			UFS_ABORT_TASK, &resp);
> - 	if (err || resp != UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
> - 		if (!err) {
> - 			err = resp; /* service response error */
> - 			dev_err(hba->dev, "%s: issued. tag = %d, err %d\n",
> - 				__func__, tag, err);
> - 		}
> - 		goto out;
> - 	}
> - 
> - 	err = ufshcd_clear_cmd(hba, tag);
> - 	if (err) {
> - 		dev_err(hba->dev, "%s: Failed clearing cmd at tag %d, err %d\n",
> - 			__func__, tag, err);
> - 		goto out;
> - 	}
> ++	else
> ++		err = ufshcd_try_to_abort_task(hba, tag);
>   
>  -out:
> + 	if (!err) {
>  +cleanup:

Yeah, considering Bean Huo's patch "scsi: ufs: No need to send Abort
Task if the task in DB was cleared", "cleanup" label shall be added back
here.

So your resolution looks good to me.

Thanks so much : )

Stanley Chu

> - 	spin_lock_irqsave(host->host_lock, flags);
> - 	__ufshcd_transfer_req_compl(hba, (1UL << tag));
> - 	spin_unlock_irqrestore(host->host_lock, flags);
> - 
> ++		spin_lock_irqsave(host->host_lock, flags);
> ++		__ufshcd_transfer_req_compl(hba, (1UL << tag));
> ++		spin_unlock_irqrestore(host->host_lock, flags);
>  +out:
> - 	if (!err) {
>   		err = SUCCESS;
>   	} else {
>   		dev_err(hba->dev, "%s: failed with err %d\n", __func__, err);
> diff --cc drivers/scsi/ufs/ufshcd.h
> index b5b2761456fb,8011fdc89fb1..6663325ed8a0
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@@ -531,11 -531,10 +531,16 @@@ enum ufshcd_quirks 
>   	 */
>   	UFSHCD_QUIRK_BROKEN_OCS_FATAL_ERROR		= 1 << 10,
>   
>  +	/*
>  +	 * This quirk needs to be enabled if the host controller has
>  +	 * auto-hibernate capability but it doesn't work.
>  +	 */
>  +	UFSHCD_QUIRK_BROKEN_AUTO_HIBERN8		= 1 << 11,
> ++
> + 	/*
> + 	 * This quirk needs to disable manual flush for write booster
> + 	 */
>  -	UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL		= 1 << 11,
> ++	UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL		= 1 << 12,
>   };
>   
>   enum ufshcd_caps {

Re: [PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[Stanley Chu]

Hi James,

On Wed, 2020-09-09 at 23:18 -0700, James Bottomley wrote:
> On Thu, 2020-09-10 at 10:48 +0800, Stanley Chu wrote:
> > Hi Martin, Can,
> > 
> > On Wed, 2020-09-09 at 22:32 -0400, Martin K. Petersen wrote:
> > > Can and Stanley,
> > > 
> > > > I can't reconcile this hunk:
> > > 
> > > Please provide a resolution for these conflicting commits in fixes
> > > and
> > > queue:
> > > 
> > > 307348f6ab14 scsi: ufs: Abort tasks before clearing them from
> > > doorbell
> > > 
> > > b10178ee7fa8 scsi: ufs: Clean up completed request without
> > > interrupt
> > > notification
> > > 
> > 
> > Can's patch has considered my fix in the new flow.
> > 
> > To be more clear, for the fixing case in my patch,
> > ufshcd_try_to_abort_task() will return 0 (err = 0) and finally the
> > target tag can be completed and cleared by
> > __ufshcd_transfer_req_compl()
> > in Can's new flow.
> > 
> > Thus I think the resolution can just using the code in Can's patch.
> > 
> > Can, please correct me if I was wrong.
> 
> Well, that really doesn't make for an easy merge. The resolution I took
> is below.
> 
> James
> 
> ---
> 
> commit 5399a4aa684d491c35a386effe385c06b41398fa
> Merge: 59958f7a956b 8c6572356646
> Author: James Bottomley <James.Bottomley@HansenPartnership.com>
> Date:   Wed Sep 9 23:12:52 2020 -0700
> 
>     Merge branch 'misc' into for-next
>     
>     Conflicts:
>             drivers/scsi/ufs/ufshcd.c
>             drivers/scsi/ufs/ufshcd.h
> 
> diff --cc drivers/scsi/ufs/ufshcd.c
> index 34e1ab407b05,05716f62febe..49478c8a601f
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@@ -6574,84 -6739,22 +6736,25 @@@ static int ufshcd_abort(struct scsi_cmn
>   	}
>   	hba->req_abort_count++;
>   
>  -	/* Skip task abort in case previous aborts failed and report failure */
>  -	if (lrbp->req_abort_skip) {
>  -		err = -EIO;
>  -		goto out;
>  +	if (!(reg & (1 << tag))) {
>  +		dev_err(hba->dev,
>  +		"%s: cmd was completed, but without a notifying intr, tag = %d",
>  +		__func__, tag);
>  +		goto cleanup;
>   	}
>   
>  -	err = ufshcd_try_to_abort_task(hba, tag);
>  -	if (err)
>  -		goto out;
>  -
>  -	spin_lock_irqsave(host->host_lock, flags);
>  -	__ufshcd_transfer_req_compl(hba, (1UL << tag));
>  -	spin_unlock_irqrestore(host->host_lock, flags);
>  +	/* Skip task abort in case previous aborts failed and report failure */
> - 	if (lrbp->req_abort_skip) {
> ++	if (lrbp->req_abort_skip)
>  +		err = -EIO;
> - 		goto out;
> - 	}
> - 
> - 	for (poll_cnt = 100; poll_cnt; poll_cnt--) {
> - 		err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
> - 				UFS_QUERY_TASK, &resp);
> - 		if (!err && resp == UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED) {
> - 			/* cmd pending in the device */
> - 			dev_err(hba->dev, "%s: cmd pending in the device. tag = %d\n",
> - 				__func__, tag);
> - 			break;
> - 		} else if (!err && resp == UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
> - 			/*
> - 			 * cmd not pending in the device, check if it is
> - 			 * in transition.
> - 			 */
> - 			dev_err(hba->dev, "%s: cmd at tag %d not pending in the device.\n",
> - 				__func__, tag);
> - 			reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
> - 			if (reg & (1 << tag)) {
> - 				/* sleep for max. 200us to stabilize */
> - 				usleep_range(100, 200);
> - 				continue;
> - 			}
> - 			/* command completed already */
> - 			dev_err(hba->dev, "%s: cmd at tag %d successfully cleared from DB.\n",
> - 				__func__, tag);
> - 			goto cleanup;
> - 		} else {
> - 			dev_err(hba->dev,
> - 				"%s: no response from device. tag = %d, err %d\n",
> - 				__func__, tag, err);
> - 			if (!err)
> - 				err = resp; /* service response error */
> - 			goto out;
> - 		}
> - 	}
> - 
> - 	if (!poll_cnt) {
> - 		err = -EBUSY;
> - 		goto out;
> - 	}
> - 
> - 	err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
> - 			UFS_ABORT_TASK, &resp);
> - 	if (err || resp != UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
> - 		if (!err) {
> - 			err = resp; /* service response error */
> - 			dev_err(hba->dev, "%s: issued. tag = %d, err %d\n",
> - 				__func__, tag, err);
> - 		}
> - 		goto out;
> - 	}
> - 
> - 	err = ufshcd_clear_cmd(hba, tag);
> - 	if (err) {
> - 		dev_err(hba->dev, "%s: Failed clearing cmd at tag %d, err %d\n",
> - 			__func__, tag, err);
> - 		goto out;
> - 	}
> ++	else
> ++		err = ufshcd_try_to_abort_task(hba, tag);
>   
>  -out:
> + 	if (!err) {
>  +cleanup:

Yeah, considering Bean Huo's patch "scsi: ufs: No need to send Abort
Task if the task in DB was cleared", "cleanup" label shall be added back
here.

So your resolution looks good to me.

Thanks so much : )

Stanley Chu

> - 	spin_lock_irqsave(host->host_lock, flags);
> - 	__ufshcd_transfer_req_compl(hba, (1UL << tag));
> - 	spin_unlock_irqrestore(host->host_lock, flags);
> - 
> ++		spin_lock_irqsave(host->host_lock, flags);
> ++		__ufshcd_transfer_req_compl(hba, (1UL << tag));
> ++		spin_unlock_irqrestore(host->host_lock, flags);
>  +out:
> - 	if (!err) {
>   		err = SUCCESS;
>   	} else {
>   		dev_err(hba->dev, "%s: failed with err %d\n", __func__, err);
> diff --cc drivers/scsi/ufs/ufshcd.h
> index b5b2761456fb,8011fdc89fb1..6663325ed8a0
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@@ -531,11 -531,10 +531,16 @@@ enum ufshcd_quirks 
>   	 */
>   	UFSHCD_QUIRK_BROKEN_OCS_FATAL_ERROR		= 1 << 10,
>   
>  +	/*
>  +	 * This quirk needs to be enabled if the host controller has
>  +	 * auto-hibernate capability but it doesn't work.
>  +	 */
>  +	UFSHCD_QUIRK_BROKEN_AUTO_HIBERN8		= 1 << 11,
> ++
> + 	/*
> + 	 * This quirk needs to disable manual flush for write booster
> + 	 */
>  -	UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL		= 1 << 11,
> ++	UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL		= 1 << 12,
>   };
>   
>   enum ufshcd_caps {

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[Stanley Chu]

Hi James,

On Wed, 2020-09-09 at 23:18 -0700, James Bottomley wrote:
> On Thu, 2020-09-10 at 10:48 +0800, Stanley Chu wrote:
> > Hi Martin, Can,
> > 
> > On Wed, 2020-09-09 at 22:32 -0400, Martin K. Petersen wrote:
> > > Can and Stanley,
> > > 
> > > > I can't reconcile this hunk:
> > > 
> > > Please provide a resolution for these conflicting commits in fixes
> > > and
> > > queue:
> > > 
> > > 307348f6ab14 scsi: ufs: Abort tasks before clearing them from
> > > doorbell
> > > 
> > > b10178ee7fa8 scsi: ufs: Clean up completed request without
> > > interrupt
> > > notification
> > > 
> > 
> > Can's patch has considered my fix in the new flow.
> > 
> > To be more clear, for the fixing case in my patch,
> > ufshcd_try_to_abort_task() will return 0 (err = 0) and finally the
> > target tag can be completed and cleared by
> > __ufshcd_transfer_req_compl()
> > in Can's new flow.
> > 
> > Thus I think the resolution can just using the code in Can's patch.
> > 
> > Can, please correct me if I was wrong.
> 
> Well, that really doesn't make for an easy merge. The resolution I took
> is below.
> 
> James
> 
> ---
> 
> commit 5399a4aa684d491c35a386effe385c06b41398fa
> Merge: 59958f7a956b 8c6572356646
> Author: James Bottomley <James.Bottomley@HansenPartnership.com>
> Date:   Wed Sep 9 23:12:52 2020 -0700
> 
>     Merge branch 'misc' into for-next
>     
>     Conflicts:
>             drivers/scsi/ufs/ufshcd.c
>             drivers/scsi/ufs/ufshcd.h
> 
> diff --cc drivers/scsi/ufs/ufshcd.c
> index 34e1ab407b05,05716f62febe..49478c8a601f
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@@ -6574,84 -6739,22 +6736,25 @@@ static int ufshcd_abort(struct scsi_cmn
>   	}
>   	hba->req_abort_count++;
>   
>  -	/* Skip task abort in case previous aborts failed and report failure */
>  -	if (lrbp->req_abort_skip) {
>  -		err = -EIO;
>  -		goto out;
>  +	if (!(reg & (1 << tag))) {
>  +		dev_err(hba->dev,
>  +		"%s: cmd was completed, but without a notifying intr, tag = %d",
>  +		__func__, tag);
>  +		goto cleanup;
>   	}
>   
>  -	err = ufshcd_try_to_abort_task(hba, tag);
>  -	if (err)
>  -		goto out;
>  -
>  -	spin_lock_irqsave(host->host_lock, flags);
>  -	__ufshcd_transfer_req_compl(hba, (1UL << tag));
>  -	spin_unlock_irqrestore(host->host_lock, flags);
>  +	/* Skip task abort in case previous aborts failed and report failure */
> - 	if (lrbp->req_abort_skip) {
> ++	if (lrbp->req_abort_skip)
>  +		err = -EIO;
> - 		goto out;
> - 	}
> - 
> - 	for (poll_cnt = 100; poll_cnt; poll_cnt--) {
> - 		err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
> - 				UFS_QUERY_TASK, &resp);
> - 		if (!err && resp == UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED) {
> - 			/* cmd pending in the device */
> - 			dev_err(hba->dev, "%s: cmd pending in the device. tag = %d\n",
> - 				__func__, tag);
> - 			break;
> - 		} else if (!err && resp == UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
> - 			/*
> - 			 * cmd not pending in the device, check if it is
> - 			 * in transition.
> - 			 */
> - 			dev_err(hba->dev, "%s: cmd at tag %d not pending in the device.\n",
> - 				__func__, tag);
> - 			reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
> - 			if (reg & (1 << tag)) {
> - 				/* sleep for max. 200us to stabilize */
> - 				usleep_range(100, 200);
> - 				continue;
> - 			}
> - 			/* command completed already */
> - 			dev_err(hba->dev, "%s: cmd at tag %d successfully cleared from DB.\n",
> - 				__func__, tag);
> - 			goto cleanup;
> - 		} else {
> - 			dev_err(hba->dev,
> - 				"%s: no response from device. tag = %d, err %d\n",
> - 				__func__, tag, err);
> - 			if (!err)
> - 				err = resp; /* service response error */
> - 			goto out;
> - 		}
> - 	}
> - 
> - 	if (!poll_cnt) {
> - 		err = -EBUSY;
> - 		goto out;
> - 	}
> - 
> - 	err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
> - 			UFS_ABORT_TASK, &resp);
> - 	if (err || resp != UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
> - 		if (!err) {
> - 			err = resp; /* service response error */
> - 			dev_err(hba->dev, "%s: issued. tag = %d, err %d\n",
> - 				__func__, tag, err);
> - 		}
> - 		goto out;
> - 	}
> - 
> - 	err = ufshcd_clear_cmd(hba, tag);
> - 	if (err) {
> - 		dev_err(hba->dev, "%s: Failed clearing cmd at tag %d, err %d\n",
> - 			__func__, tag, err);
> - 		goto out;
> - 	}
> ++	else
> ++		err = ufshcd_try_to_abort_task(hba, tag);
>   
>  -out:
> + 	if (!err) {
>  +cleanup:

Yeah, considering Bean Huo's patch "scsi: ufs: No need to send Abort
Task if the task in DB was cleared", "cleanup" label shall be added back
here.

So your resolution looks good to me.

Thanks so much : )

Stanley Chu

> - 	spin_lock_irqsave(host->host_lock, flags);
> - 	__ufshcd_transfer_req_compl(hba, (1UL << tag));
> - 	spin_unlock_irqrestore(host->host_lock, flags);
> - 
> ++		spin_lock_irqsave(host->host_lock, flags);
> ++		__ufshcd_transfer_req_compl(hba, (1UL << tag));
> ++		spin_unlock_irqrestore(host->host_lock, flags);
>  +out:
> - 	if (!err) {
>   		err = SUCCESS;
>   	} else {
>   		dev_err(hba->dev, "%s: failed with err %d\n", __func__, err);
> diff --cc drivers/scsi/ufs/ufshcd.h
> index b5b2761456fb,8011fdc89fb1..6663325ed8a0
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@@ -531,11 -531,10 +531,16 @@@ enum ufshcd_quirks 
>   	 */
>   	UFSHCD_QUIRK_BROKEN_OCS_FATAL_ERROR		= 1 << 10,
>   
>  +	/*
>  +	 * This quirk needs to be enabled if the host controller has
>  +	 * auto-hibernate capability but it doesn't work.
>  +	 */
>  +	UFSHCD_QUIRK_BROKEN_AUTO_HIBERN8		= 1 << 11,
> ++
> + 	/*
> + 	 * This quirk needs to disable manual flush for write booster
> + 	 */
>  -	UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL		= 1 << 11,
> ++	UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL		= 1 << 12,
>   };
>   
>   enum ufshcd_caps {

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[James Bottomley]

On Thu, 2020-09-10 at 16:18 +0800, Stanley Chu wrote:
[...]
> > + 	if (!err) {
> >  +cleanup:
> 
> Yeah, considering Bean Huo's patch "scsi: ufs: No need to send Abort
> Task if the task in DB was cleared", "cleanup" label shall be added
> back here.
> 
> So your resolution looks good to me.
> 
> Thanks so much : )

You're welcome ... but just remember I have to explain this to Linus
when the merge window opens.  It would be a lot easier if this hadn't
happened so please don't make it any worse ...

James

Re: [PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[James Bottomley]

On Thu, 2020-09-10 at 16:18 +0800, Stanley Chu wrote:
[...]
> > + 	if (!err) {
> >  +cleanup:
> 
> Yeah, considering Bean Huo's patch "scsi: ufs: No need to send Abort
> Task if the task in DB was cleared", "cleanup" label shall be added
> back here.
> 
> So your resolution looks good to me.
> 
> Thanks so much : )

You're welcome ... but just remember I have to explain this to Linus
when the merge window opens.  It would be a lot easier if this hadn't
happened so please don't make it any worse ...

James


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[James Bottomley]

On Thu, 2020-09-10 at 16:18 +0800, Stanley Chu wrote:
[...]
> > + 	if (!err) {
> >  +cleanup:
> 
> Yeah, considering Bean Huo's patch "scsi: ufs: No need to send Abort
> Task if the task in DB was cleared", "cleanup" label shall be added
> back here.
> 
> So your resolution looks good to me.
> 
> Thanks so much : )

You're welcome ... but just remember I have to explain this to Linus
when the merge window opens.  It would be a lot easier if this hadn't
happened so please don't make it any worse ...

James


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[Can Guo]

Hi James and Stanley,

On 2020-09-11 00:09, James Bottomley wrote:
> On Thu, 2020-09-10 at 16:18 +0800, Stanley Chu wrote:
> [...]
>> > + 	if (!err) {
>> >  +cleanup:
>> 
>> Yeah, considering Bean Huo's patch "scsi: ufs: No need to send Abort
>> Task if the task in DB was cleared", "cleanup" label shall be added
>> back here.
>> 
>> So your resolution looks good to me.
>> 
>> Thanks so much : )
> 
> You're welcome ... but just remember I have to explain this to Linus
> when the merge window opens.  It would be a lot easier if this hadn't
> happened so please don't make it any worse ...
> 
> James

Sorry that my changes got you confused and thank you for help resolve 
the
conflicts. My change ("scsi: ufs: Abort tasks before clearing them from
doorbell") is to serve my fixes to ufs error recovery which only got 
picked
up on scsi-queue-5.10. So I checked out to scsi-queue-5.10 and made my
changes on the tip of scsi-queue-5.10, below 2 changes were not even
present in scsi-queue-5.10 back that time.

scsi: ufs: Clean up completed request without interrupt notification
scsi: ufs: No need to send Abort Task if the task in DB was cleared

Is there anything wrong with my work flow above? Please let me know the
right process so that I can avoid such conflicts in my next changes, 
which
also touch the func ufshcd_abort(). Thanks!

Regards,

Can Guo.

Re: [PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[Bean Huo]

On Fri, 2020-09-11 at 02:16 +0000, Can Guo wrote:
> > > 
> > > So your resolution looks good to me.
> > > 
> > > Thanks so much : )
> > 
> > You're welcome ... but just remember I have to explain this to
> > Linus
> > when the merge window opens.  It would be a lot easier if this
> > hadn't
> > happened so please don't make it any worse ...
> > 
> > James
> 
> Sorry that my changes got you confused and thank you for help
> resolve 
> the
> conflicts. My change ("scsi: ufs: Abort tasks before clearing them
> from
> doorbell") is to serve my fixes to ufs error recovery which only got 
> picked
> up on scsi-queue-5.10. So I checked out to scsi-queue-5.10 and made
> my
> changes on the tip of scsi-queue-5.10, below 2 changes were not even
> present in scsi-queue-5.10 back that time.

I mentioned here https://patchwork.kernel.org/patch/11734713/

this change (scsi: ufs: Abort tasks before clearing them from doorbell)
has conflicts with the scsi-fixes branch. I don't know which branch is
the main branch we should focus on.


Bean 

Re: [PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[Bean Huo]

On Fri, 2020-09-11 at 02:16 +0000, Can Guo wrote:
> > > 
> > > So your resolution looks good to me.
> > > 
> > > Thanks so much : )
> > 
> > You're welcome ... but just remember I have to explain this to
> > Linus
> > when the merge window opens.  It would be a lot easier if this
> > hadn't
> > happened so please don't make it any worse ...
> > 
> > James
> 
> Sorry that my changes got you confused and thank you for help
> resolve 
> the
> conflicts. My change ("scsi: ufs: Abort tasks before clearing them
> from
> doorbell") is to serve my fixes to ufs error recovery which only got 
> picked
> up on scsi-queue-5.10. So I checked out to scsi-queue-5.10 and made
> my
> changes on the tip of scsi-queue-5.10, below 2 changes were not even
> present in scsi-queue-5.10 back that time.

I mentioned here https://patchwork.kernel.org/patch/11734713/

this change (scsi: ufs: Abort tasks before clearing them from doorbell)
has conflicts with the scsi-fixes branch. I don't know which branch is
the main branch we should focus on.


Bean 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[Bean Huo]

On Fri, 2020-09-11 at 02:16 +0000, Can Guo wrote:
> > > 
> > > So your resolution looks good to me.
> > > 
> > > Thanks so much : )
> > 
> > You're welcome ... but just remember I have to explain this to
> > Linus
> > when the merge window opens.  It would be a lot easier if this
> > hadn't
> > happened so please don't make it any worse ...
> > 
> > James
> 
> Sorry that my changes got you confused and thank you for help
> resolve 
> the
> conflicts. My change ("scsi: ufs: Abort tasks before clearing them
> from
> doorbell") is to serve my fixes to ufs error recovery which only got 
> picked
> up on scsi-queue-5.10. So I checked out to scsi-queue-5.10 and made
> my
> changes on the tip of scsi-queue-5.10, below 2 changes were not even
> present in scsi-queue-5.10 back that time.

I mentioned here https://patchwork.kernel.org/patch/11734713/

this change (scsi: ufs: Abort tasks before clearing them from doorbell)
has conflicts with the scsi-fixes branch. I don't know which branch is
the main branch we should focus on.


Bean 


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[Can Guo]

On 2020-09-11 17:09, Bean Huo wrote:
> On Fri, 2020-09-11 at 02:16 +0000, Can Guo wrote:
>> > >
>> > > So your resolution looks good to me.
>> > >
>> > > Thanks so much : )
>> >
>> > You're welcome ... but just remember I have to explain this to
>> > Linus
>> > when the merge window opens.  It would be a lot easier if this
>> > hadn't
>> > happened so please don't make it any worse ...
>> >
>> > James
>> 
>> Sorry that my changes got you confused and thank you for help
>> resolve
>> the
>> conflicts. My change ("scsi: ufs: Abort tasks before clearing them
>> from
>> doorbell") is to serve my fixes to ufs error recovery which only got
>> picked
>> up on scsi-queue-5.10. So I checked out to scsi-queue-5.10 and made
>> my
>> changes on the tip of scsi-queue-5.10, below 2 changes were not even
>> present in scsi-queue-5.10 back that time.
> 
> I mentioned here https://patchwork.kernel.org/patch/11734713/
> 
> this change (scsi: ufs: Abort tasks before clearing them from doorbell)
> has conflicts with the scsi-fixes branch. I don't know which branch is
> the main branch we should focus on.
> 
> 
> Bean

Yeah, I know that one, but I was not even working on scsi-fixes branch
at that time. Now I have two more fixes to ufshcd_abort(), not sure
which branch I should work on, so asking the same here.

Regards,

Can Guo.

Re: [PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[Can Guo]

On 2020-09-11 17:09, Bean Huo wrote:
> On Fri, 2020-09-11 at 02:16 +0000, Can Guo wrote:
>> > >
>> > > So your resolution looks good to me.
>> > >
>> > > Thanks so much : )
>> >
>> > You're welcome ... but just remember I have to explain this to
>> > Linus
>> > when the merge window opens.  It would be a lot easier if this
>> > hadn't
>> > happened so please don't make it any worse ...
>> >
>> > James
>> 
>> Sorry that my changes got you confused and thank you for help
>> resolve
>> the
>> conflicts. My change ("scsi: ufs: Abort tasks before clearing them
>> from
>> doorbell") is to serve my fixes to ufs error recovery which only got
>> picked
>> up on scsi-queue-5.10. So I checked out to scsi-queue-5.10 and made
>> my
>> changes on the tip of scsi-queue-5.10, below 2 changes were not even
>> present in scsi-queue-5.10 back that time.
> 
> I mentioned here https://patchwork.kernel.org/patch/11734713/
> 
> this change (scsi: ufs: Abort tasks before clearing them from doorbell)
> has conflicts with the scsi-fixes branch. I don't know which branch is
> the main branch we should focus on.
> 
> 
> Bean

Yeah, I know that one, but I was not even working on scsi-fixes branch
at that time. Now I have two more fixes to ufshcd_abort(), not sure
which branch I should work on, so asking the same here.

Regards,

Can Guo.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[Can Guo]

On 2020-09-11 17:09, Bean Huo wrote:
> On Fri, 2020-09-11 at 02:16 +0000, Can Guo wrote:
>> > >
>> > > So your resolution looks good to me.
>> > >
>> > > Thanks so much : )
>> >
>> > You're welcome ... but just remember I have to explain this to
>> > Linus
>> > when the merge window opens.  It would be a lot easier if this
>> > hadn't
>> > happened so please don't make it any worse ...
>> >
>> > James
>> 
>> Sorry that my changes got you confused and thank you for help
>> resolve
>> the
>> conflicts. My change ("scsi: ufs: Abort tasks before clearing them
>> from
>> doorbell") is to serve my fixes to ufs error recovery which only got
>> picked
>> up on scsi-queue-5.10. So I checked out to scsi-queue-5.10 and made
>> my
>> changes on the tip of scsi-queue-5.10, below 2 changes were not even
>> present in scsi-queue-5.10 back that time.
> 
> I mentioned here https://patchwork.kernel.org/patch/11734713/
> 
> this change (scsi: ufs: Abort tasks before clearing them from doorbell)
> has conflicts with the scsi-fixes branch. I don't know which branch is
> the main branch we should focus on.
> 
> 
> Bean

Yeah, I know that one, but I was not even working on scsi-fixes branch
at that time. Now I have two more fixes to ufshcd_abort(), not sure
which branch I should work on, so asking the same here.

Regards,

Can Guo.

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[Can Guo]

Hi Bean,

On 2020-09-11 17:09, Bean Huo wrote:
> On Fri, 2020-09-11 at 02:16 +0000, Can Guo wrote:
>> > >
>> > > So your resolution looks good to me.
>> > >
>> > > Thanks so much : )
>> >
>> > You're welcome ... but just remember I have to explain this to
>> > Linus
>> > when the merge window opens.  It would be a lot easier if this
>> > hadn't
>> > happened so please don't make it any worse ...
>> >
>> > James
>> 
>> Sorry that my changes got you confused and thank you for help
>> resolve
>> the
>> conflicts. My change ("scsi: ufs: Abort tasks before clearing them
>> from
>> doorbell") is to serve my fixes to ufs error recovery which only got
>> picked
>> up on scsi-queue-5.10. So I checked out to scsi-queue-5.10 and made
>> my
>> changes on the tip of scsi-queue-5.10, below 2 changes were not even
>> present in scsi-queue-5.10 back that time.
> 
> I mentioned here https://patchwork.kernel.org/patch/11734713/

Do you know when can this change be picked up in scsi-queue-5.10?
If I push my fixes to ufshcd_abort() on scsi-queue-5.10, they will
run into conflicts with this one again, right? How should I move
forward now? Thanks.

Regards,

Can Guo.

> 
> this change (scsi: ufs: Abort tasks before clearing them from doorbell)
> has conflicts with the scsi-fixes branch. I don't know which branch is
> the main branch we should focus on.
> 
> 
> Bean

Re: [PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[Can Guo]

Hi Bean,

On 2020-09-11 17:09, Bean Huo wrote:
> On Fri, 2020-09-11 at 02:16 +0000, Can Guo wrote:
>> > >
>> > > So your resolution looks good to me.
>> > >
>> > > Thanks so much : )
>> >
>> > You're welcome ... but just remember I have to explain this to
>> > Linus
>> > when the merge window opens.  It would be a lot easier if this
>> > hadn't
>> > happened so please don't make it any worse ...
>> >
>> > James
>> 
>> Sorry that my changes got you confused and thank you for help
>> resolve
>> the
>> conflicts. My change ("scsi: ufs: Abort tasks before clearing them
>> from
>> doorbell") is to serve my fixes to ufs error recovery which only got
>> picked
>> up on scsi-queue-5.10. So I checked out to scsi-queue-5.10 and made
>> my
>> changes on the tip of scsi-queue-5.10, below 2 changes were not even
>> present in scsi-queue-5.10 back that time.
> 
> I mentioned here https://patchwork.kernel.org/patch/11734713/

Do you know when can this change be picked up in scsi-queue-5.10?
If I push my fixes to ufshcd_abort() on scsi-queue-5.10, they will
run into conflicts with this one again, right? How should I move
forward now? Thanks.

Regards,

Can Guo.

> 
> this change (scsi: ufs: Abort tasks before clearing them from doorbell)
> has conflicts with the scsi-fixes branch. I don't know which branch is
> the main branch we should focus on.
> 
> 
> Bean

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[Can Guo]

Hi Bean,

On 2020-09-11 17:09, Bean Huo wrote:
> On Fri, 2020-09-11 at 02:16 +0000, Can Guo wrote:
>> > >
>> > > So your resolution looks good to me.
>> > >
>> > > Thanks so much : )
>> >
>> > You're welcome ... but just remember I have to explain this to
>> > Linus
>> > when the merge window opens.  It would be a lot easier if this
>> > hadn't
>> > happened so please don't make it any worse ...
>> >
>> > James
>> 
>> Sorry that my changes got you confused and thank you for help
>> resolve
>> the
>> conflicts. My change ("scsi: ufs: Abort tasks before clearing them
>> from
>> doorbell") is to serve my fixes to ufs error recovery which only got
>> picked
>> up on scsi-queue-5.10. So I checked out to scsi-queue-5.10 and made
>> my
>> changes on the tip of scsi-queue-5.10, below 2 changes were not even
>> present in scsi-queue-5.10 back that time.
> 
> I mentioned here https://patchwork.kernel.org/patch/11734713/

Do you know when can this change be picked up in scsi-queue-5.10?
If I push my fixes to ufshcd_abort() on scsi-queue-5.10, they will
run into conflicts with this one again, right? How should I move
forward now? Thanks.

Regards,

Can Guo.

> 
> this change (scsi: ufs: Abort tasks before clearing them from doorbell)
> has conflicts with the scsi-fixes branch. I don't know which branch is
> the main branch we should focus on.
> 
> 
> Bean

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[Martin K. Petersen]

Can,

> Do you know when can this change be picked up in scsi-queue-5.10?
> If I push my fixes to ufshcd_abort() on scsi-queue-5.10, they will
> run into conflicts with this one again, right? How should I move
> forward now?

You should be able to use 5.10/scsi-queue as baseline now.

For 5.11 I think I'll do a separate branch for UFS.

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[Martin K. Petersen]

Can,

> Do you know when can this change be picked up in scsi-queue-5.10?
> If I push my fixes to ufshcd_abort() on scsi-queue-5.10, they will
> run into conflicts with this one again, right? How should I move
> forward now?

You should be able to use 5.10/scsi-queue as baseline now.

For 5.11 I think I'll do a separate branch for UFS.

-- 
Martin K. Petersen	Oracle Linux Engineering

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[Martin K. Petersen]

Can,

> Do you know when can this change be picked up in scsi-queue-5.10?
> If I push my fixes to ufshcd_abort() on scsi-queue-5.10, they will
> run into conflicts with this one again, right? How should I move
> forward now?

You should be able to use 5.10/scsi-queue as baseline now.

For 5.11 I think I'll do a separate branch for UFS.

-- 
Martin K. Petersen	Oracle Linux Engineering

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[Can Guo]

On 2020-09-16 04:21, Martin K. Petersen wrote:
> Can,
> 
>> Do you know when can this change be picked up in scsi-queue-5.10?
>> If I push my fixes to ufshcd_abort() on scsi-queue-5.10, they will
>> run into conflicts with this one again, right? How should I move
>> forward now?
> 
> You should be able to use 5.10/scsi-queue as baseline now.
> 
> For 5.11 I think I'll do a separate branch for UFS.

Thanks for the information.

Regards,

Can Guo.

Re: [PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[Can Guo]

On 2020-09-16 04:21, Martin K. Petersen wrote:
> Can,
> 
>> Do you know when can this change be picked up in scsi-queue-5.10?
>> If I push my fixes to ufshcd_abort() on scsi-queue-5.10, they will
>> run into conflicts with this one again, right? How should I move
>> forward now?
> 
> You should be able to use 5.10/scsi-queue as baseline now.
> 
> For 5.11 I think I'll do a separate branch for UFS.

Thanks for the information.

Regards,

Can Guo.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[Can Guo]

On 2020-09-16 04:21, Martin K. Petersen wrote:
> Can,
> 
>> Do you know when can this change be picked up in scsi-queue-5.10?
>> If I push my fixes to ufshcd_abort() on scsi-queue-5.10, they will
>> run into conflicts with this one again, right? How should I move
>> forward now?
> 
> You should be able to use 5.10/scsi-queue as baseline now.
> 
> For 5.11 I think I'll do a separate branch for UFS.

Thanks for the information.

Regards,

Can Guo.

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[Can Guo]

Hi James and Stanley,

On 2020-09-11 00:09, James Bottomley wrote:
> On Thu, 2020-09-10 at 16:18 +0800, Stanley Chu wrote:
> [...]
>> > + 	if (!err) {
>> >  +cleanup:
>> 
>> Yeah, considering Bean Huo's patch "scsi: ufs: No need to send Abort
>> Task if the task in DB was cleared", "cleanup" label shall be added
>> back here.
>> 
>> So your resolution looks good to me.
>> 
>> Thanks so much : )
> 
> You're welcome ... but just remember I have to explain this to Linus
> when the merge window opens.  It would be a lot easier if this hadn't
> happened so please don't make it any worse ...
> 
> James

Sorry that my changes got you confused and thank you for help resolve 
the
conflicts. My change ("scsi: ufs: Abort tasks before clearing them from
doorbell") is to serve my fixes to ufs error recovery which only got 
picked
up on scsi-queue-5.10. So I checked out to scsi-queue-5.10 and made my
changes on the tip of scsi-queue-5.10, below 2 changes were not even
present in scsi-queue-5.10 back that time.

scsi: ufs: Clean up completed request without interrupt notification
scsi: ufs: No need to send Abort Task if the task in DB was cleared

Is there anything wrong with my work flow above? Please let me know the
right process so that I can avoid such conflicts in my next changes, 
which
also touch the func ufshcd_abort(). Thanks!

Regards,

Can Guo.

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell

[Can Guo]

Hi James and Stanley,

On 2020-09-11 00:09, James Bottomley wrote:
> On Thu, 2020-09-10 at 16:18 +0800, Stanley Chu wrote:
> [...]
>> > + 	if (!err) {
>> >  +cleanup:
>> 
>> Yeah, considering Bean Huo's patch "scsi: ufs: No need to send Abort
>> Task if the task in DB was cleared", "cleanup" label shall be added
>> back here.
>> 
>> So your resolution looks good to me.
>> 
>> Thanks so much : )
> 
> You're welcome ... but just remember I have to explain this to Linus
> when the merge window opens.  It would be a lot easier if this hadn't
> happened so please don't make it any worse ...
> 
> James

Sorry that my changes got you confused and thank you for help resolve 
the
conflicts. My change ("scsi: ufs: Abort tasks before clearing them from
doorbell") is to serve my fixes to ufs error recovery which only got 
picked
up on scsi-queue-5.10. So I checked out to scsi-queue-5.10 and made my
changes on the tip of scsi-queue-5.10, below 2 changes were not even
present in scsi-queue-5.10 back that time.

scsi: ufs: Clean up completed request without interrupt notification
scsi: ufs: No need to send Abort Task if the task in DB was cleared

Is there anything wrong with my work flow above? Please let me know the
right process so that I can avoid such conflicts in my next changes, 
which
also touch the func ufshcd_abort(). Thanks!

Regards,

Can Guo.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 2/2] scsi: ufs: Handle LINERESET indication in err handler

[Can Guo]

PA Layer issues a LINERESET to the PHY at the recovery step in the Power
Mode change operation. If it happens during auto or mannual hibern8 enter,
even if hibern8 enter succeeds, UFS power mode shall be set to PWM-G1 mode
and kept in that mode after exit from hibern8, leading to bad performance.
Handle the LINERESET in the eh_work by restoring power mode to HS mode
after all pending reqs and tasks are cleared from doorbell.

Signed-off-by: Can Guo <cang@codeaurora.org>
---
 drivers/scsi/ufs/ufshcd.c | 140 ++++++++++++++++++++++++++++++++++++----------
 drivers/scsi/ufs/ufshcd.h |   2 +
 drivers/scsi/ufs/ufshci.h |   1 +
 drivers/scsi/ufs/unipro.h |   3 +
 4 files changed, 115 insertions(+), 31 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 72afe12..93619bc 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -146,6 +146,7 @@ enum {
 	UFSHCD_UIC_NL_ERROR = (1 << 3), /* Network layer error */
 	UFSHCD_UIC_TL_ERROR = (1 << 4), /* Transport Layer error */
 	UFSHCD_UIC_DME_ERROR = (1 << 5), /* DME error */
+	UFSHCD_UIC_PA_GENERIC_ERROR = (1 << 6), /* Generic PA error */
 };
 
 #define ufshcd_set_eh_in_progress(h) \
@@ -4069,7 +4070,8 @@ static int ufshcd_change_power_mode(struct ufs_hba *hba,
 	int ret;
 
 	/* if already configured to the requested pwr_mode */
-	if (pwr_mode->gear_rx == hba->pwr_info.gear_rx &&
+	if (!hba->force_pmc &&
+	    pwr_mode->gear_rx == hba->pwr_info.gear_rx &&
 	    pwr_mode->gear_tx == hba->pwr_info.gear_tx &&
 	    pwr_mode->lane_rx == hba->pwr_info.lane_rx &&
 	    pwr_mode->lane_tx == hba->pwr_info.lane_tx &&
@@ -4503,6 +4505,8 @@ static int ufshcd_link_startup(struct ufs_hba *hba)
 	if (ret)
 		goto out;
 
+	/* Clear UECPA once due to LINERESET has happened during LINK_STARTUP */
+	ufshcd_readl(hba, REG_UIC_ERROR_CODE_PHY_ADAPTER_LAYER);
 	ret = ufshcd_make_hba_operational(hba);
 out:
 	if (ret) {
@@ -5659,6 +5663,22 @@ static inline void ufshcd_recover_pm_error(struct ufs_hba *hba)
 }
 #endif
 
+static bool ufshcd_is_pwr_mode_restore_needed(struct ufs_hba *hba)
+{
+	struct ufs_pa_layer_attr *pwr_info = &hba->pwr_info;
+	u32 mode;
+
+	ufshcd_dme_get(hba, UIC_ARG_MIB(PA_PWRMODE), &mode);
+
+	if (pwr_info->pwr_rx != ((mode >> PWRMODE_RX_OFFSET) & PWRMODE_MASK))
+		return true;
+
+	if (pwr_info->pwr_tx != (mode & PWRMODE_MASK))
+		return true;
+
+	return false;
+}
+
 /**
  * ufshcd_err_handler - handle UFS errors that require s/w attention
  * @work: pointer to work structure
@@ -5669,9 +5689,9 @@ static void ufshcd_err_handler(struct work_struct *work)
 	unsigned long flags;
 	bool err_xfer = false;
 	bool err_tm = false;
-	int err = 0;
+	int err = 0, pmc_err;
 	int tag;
-	bool needs_reset = false;
+	bool needs_reset = false, needs_restore = false;
 
 	hba = container_of(work, struct ufs_hba, eh_work);
 
@@ -5687,16 +5707,13 @@ static void ufshcd_err_handler(struct work_struct *work)
 	ufshcd_err_handling_prepare(hba);
 	spin_lock_irqsave(hba->host->host_lock, flags);
 	ufshcd_scsi_block_requests(hba);
+	hba->ufshcd_state = UFSHCD_STATE_RESET;
 	/*
 	 * A full reset and restore might have happened after preparation
 	 * is finished, double check whether we should stop.
 	 */
-	if (ufshcd_err_handling_should_stop(hba)) {
-		if (hba->ufshcd_state != UFSHCD_STATE_ERROR)
-			hba->ufshcd_state = UFSHCD_STATE_OPERATIONAL;
-		goto out;
-	}
-	hba->ufshcd_state = UFSHCD_STATE_RESET;
+	if (ufshcd_err_handling_should_stop(hba))
+		goto skip_err_handling;
 
 	/* Complete requests that have door-bell cleared by h/w */
 	ufshcd_complete_requests(hba);
@@ -5712,15 +5729,9 @@ static void ufshcd_err_handler(struct work_struct *work)
 			goto skip_err_handling;
 	}
 
-	if (hba->force_reset || ufshcd_is_link_broken(hba) ||
-	    ufshcd_is_saved_err_fatal(hba) ||
-	    ((hba->saved_err & UIC_ERROR) &&
-	     (hba->saved_uic_err & (UFSHCD_UIC_DL_NAC_RECEIVED_ERROR |
-				    UFSHCD_UIC_DL_TCx_REPLAY_ERROR))))
-		needs_reset = true;
-
-	if (hba->saved_err & (INT_FATAL_ERRORS | UIC_ERROR |
-			      UFSHCD_UIC_HIBERN8_MASK)) {
+	if ((hba->saved_err & (INT_FATAL_ERRORS | UFSHCD_UIC_HIBERN8_MASK)) ||
+	    (hba->saved_uic_err &&
+	     (hba->saved_uic_err != UFSHCD_UIC_PA_GENERIC_ERROR))) {
 		bool pr_prdt = !!(hba->saved_err & SYSTEM_BUS_FATAL_ERROR);
 
 		spin_unlock_irqrestore(hba->host->host_lock, flags);
@@ -5733,13 +5744,36 @@ static void ufshcd_err_handler(struct work_struct *work)
 	}
 
 	/*
-	 * if host reset is required then skip clearing the pending
+	 * If host reset is required then skip clearing the pending
 	 * transfers forcefully because they will get cleared during
 	 * host reset and restore
 	 */
-	if (needs_reset)
-		goto skip_pending_xfer_clear;
+	if (hba->force_reset || ufshcd_is_link_broken(hba) ||
+	    ufshcd_is_saved_err_fatal(hba) ||
+	    ((hba->saved_err & UIC_ERROR) &&
+	     (hba->saved_uic_err & (UFSHCD_UIC_DL_NAC_RECEIVED_ERROR |
+				    UFSHCD_UIC_DL_TCx_REPLAY_ERROR)))) {
+		needs_reset = true;
+		goto do_reset;
+	}
+
+	/*
+	 * If LINERESET was caught, UFS might have been put to PWM mode,
+	 * check if power mode restore is needed.
+	 */
+	if (hba->saved_uic_err & UFSHCD_UIC_PA_GENERIC_ERROR) {
+		hba->saved_uic_err &= ~UFSHCD_UIC_PA_GENERIC_ERROR;
+		if (!hba->saved_uic_err)
+			hba->saved_err &= ~UIC_ERROR;
+		spin_unlock_irqrestore(hba->host->host_lock, flags);
+		if (ufshcd_is_pwr_mode_restore_needed(hba))
+			needs_restore = true;
+		spin_lock_irqsave(hba->host->host_lock, flags);
+		if (!hba->saved_err && !needs_restore)
+			goto skip_err_handling;
+	}
 
+	hba->silence_err_logs = true;
 	/* release lock as clear command might sleep */
 	spin_unlock_irqrestore(hba->host->host_lock, flags);
 	/* Clear pending transfer requests */
@@ -5763,11 +5797,38 @@ static void ufshcd_err_handler(struct work_struct *work)
 
 	/* Complete the requests that are cleared by s/w */
 	ufshcd_complete_requests(hba);
+	hba->silence_err_logs = false;
 
-	if (err_xfer || err_tm)
+	if (err_xfer || err_tm) {
 		needs_reset = true;
+		goto do_reset;
+	}
 
-skip_pending_xfer_clear:
+	/*
+	 * After all reqs and tasks are cleared from doorbell,
+	 * now it is safe to retore power mode.
+	 */
+	if (needs_restore) {
+		spin_unlock_irqrestore(hba->host->host_lock, flags);
+		/*
+		 * Hold the scaling lock just in case dev cmds
+		 * are sent via bsg and/or sysfs.
+		 */
+		down_write(&hba->clk_scaling_lock);
+		hba->force_pmc = true;
+		pmc_err = ufshcd_config_pwr_mode(hba, &(hba->pwr_info));
+		if (pmc_err) {
+			needs_reset = true;
+			dev_err(hba->dev, "%s: Failed to restore power mode, err = %d\n",
+					__func__, pmc_err);
+		}
+		hba->force_pmc = false;
+		ufshcd_print_pwr_info(hba);
+		up_write(&hba->clk_scaling_lock);
+		spin_lock_irqsave(hba->host->host_lock, flags);
+	}
+
+do_reset:
 	/* Fatal errors need reset */
 	if (needs_reset) {
 		unsigned long max_doorbells = (1UL << hba->nutrs) - 1;
@@ -5802,8 +5863,6 @@ static void ufshcd_err_handler(struct work_struct *work)
 			dev_err_ratelimited(hba->dev, "%s: exit: saved_err 0x%x saved_uic_err 0x%x",
 			    __func__, hba->saved_err, hba->saved_uic_err);
 	}
-
-out:
 	ufshcd_clear_eh_in_progress(hba);
 	spin_unlock_irqrestore(hba->host->host_lock, flags);
 	ufshcd_scsi_unblock_requests(hba);
@@ -5823,17 +5882,34 @@ static irqreturn_t ufshcd_update_uic_error(struct ufs_hba *hba)
 	u32 reg;
 	irqreturn_t retval = IRQ_NONE;
 
-	/* PHY layer lane error */
+	/* PHY layer error */
 	reg = ufshcd_readl(hba, REG_UIC_ERROR_CODE_PHY_ADAPTER_LAYER);
-	/* Ignore LINERESET indication, as this is not an error */
 	if ((reg & UIC_PHY_ADAPTER_LAYER_ERROR) &&
-	    (reg & UIC_PHY_ADAPTER_LAYER_LANE_ERR_MASK)) {
+	    (reg & UIC_PHY_ADAPTER_LAYER_ERROR_CODE_MASK)) {
 		/*
 		 * To know whether this error is fatal or not, DB timeout
 		 * must be checked but this error is handled separately.
 		 */
-		dev_dbg(hba->dev, "%s: UIC Lane error reported\n", __func__);
-		ufshcd_update_reg_hist(&hba->ufs_stats.pa_err, reg);
+		if (reg & UIC_PHY_ADAPTER_LAYER_LANE_ERR_MASK) {
+			ufshcd_update_reg_hist(&hba->ufs_stats.pa_err, reg);
+			dev_dbg(hba->dev, "%s: UIC Lane error reported\n",
+					__func__);
+		}
+
+		/* Got a LINERESET indication. */
+		if (reg & UIC_PHY_ADAPTER_LAYER_GENERIC_ERROR) {
+			struct uic_command *cmd = NULL;
+
+			hba->uic_error |= UFSHCD_UIC_PA_GENERIC_ERROR;
+			if (hba->uic_async_done && hba->active_uic_cmd)
+				cmd = hba->active_uic_cmd;
+			/*
+			 * Ignore the LINERESET during power mode change
+			 * operation via DME_SET command.
+			 */
+			if (cmd && (cmd->command == UIC_CMD_DME_SET))
+				hba->uic_error &= ~UFSHCD_UIC_PA_GENERIC_ERROR;
+		}
 		retval |= IRQ_HANDLED;
 	}
 
@@ -5950,7 +6026,9 @@ static irqreturn_t ufshcd_check_errors(struct ufs_hba *hba)
 		hba->saved_uic_err |= hba->uic_error;
 
 		/* dump controller state before resetting */
-		if (hba->saved_err & (INT_FATAL_ERRORS | UIC_ERROR)) {
+		if ((hba->saved_err & (INT_FATAL_ERRORS)) ||
+		    (hba->saved_uic_err &&
+		     (hba->saved_uic_err != UFSHCD_UIC_PA_GENERIC_ERROR))) {
 			dev_err(hba->dev, "%s: saved_err 0x%x saved_uic_err 0x%x\n",
 					__func__, hba->saved_err,
 					hba->saved_uic_err);
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 618b253..8817103 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -629,6 +629,7 @@ struct ufs_hba_variant_params {
  * @saved_err: sticky error mask
  * @saved_uic_err: sticky UIC error mask
  * @force_reset: flag to force eh_work perform a full reset
+ * @force_pmc: flag to force a power mode change
  * @silence_err_logs: flag to silence error logs
  * @dev_cmd: ufs device management command information
  * @last_dme_cmd_tstamp: time stamp of the last completed DME command
@@ -728,6 +729,7 @@ struct ufs_hba {
 	u32 saved_uic_err;
 	struct ufs_stats ufs_stats;
 	bool force_reset;
+	bool force_pmc;
 	bool silence_err_logs;
 
 	/* Device management request data */
diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
index ba31b09..6795e1f 100644
--- a/drivers/scsi/ufs/ufshci.h
+++ b/drivers/scsi/ufs/ufshci.h
@@ -171,6 +171,7 @@ enum {
 #define UIC_PHY_ADAPTER_LAYER_ERROR			0x80000000
 #define UIC_PHY_ADAPTER_LAYER_ERROR_CODE_MASK		0x1F
 #define UIC_PHY_ADAPTER_LAYER_LANE_ERR_MASK		0xF
+#define UIC_PHY_ADAPTER_LAYER_GENERIC_ERROR		0x10
 
 /* UECDL - Host UIC Error Code Data Link Layer 3Ch */
 #define UIC_DATA_LINK_LAYER_ERROR		0x80000000
diff --git a/drivers/scsi/ufs/unipro.h b/drivers/scsi/ufs/unipro.h
index 4ee6478..f6b52ce 100644
--- a/drivers/scsi/ufs/unipro.h
+++ b/drivers/scsi/ufs/unipro.h
@@ -205,6 +205,9 @@ enum {
 	UNCHANGED	= 7,
 };
 
+#define PWRMODE_MASK		0xF
+#define PWRMODE_RX_OFFSET	4
+
 /* PA TX/RX Frequency Series */
 enum {
 	PA_HS_MODE_A	= 1,
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH v2 0/2] Add UFS LINERESET handling

[Martin K. Petersen]

On Wed, 2 Sep 2020 19:24:30 -0700, Can Guo wrote:

> PA Layer issues a LINERESET to the PHY at the recovery step in the Power
> Mode change operation. If it happens during auto or mannual hibern8 enter,
> even if hibern8 enter succeeds, UFS power mode shall be set to PWM-G1 mode
> and kept in that mode after exit from hibern8, leading to bad performance.
> Handle the LINERESET in the eh_work by restoring power mode to HS mode
> after all pending reqs and tasks are cleared from doorbell.
> 
> [...]

Applied to 5.10/scsi-queue, thanks!

[1/2] scsi: ufs: Abort tasks before clearing them from doorbell
      https://git.kernel.org/mkp/scsi/c/307348f6ab14
[2/2] scsi: ufs: Handle LINERESET indication in err handler
      https://git.kernel.org/mkp/scsi/c/2355b66ed20c

-- 
Martin K. Petersen	Oracle Linux Engineering