[PATCH v1 0/3] Refine error history and introduce notify_event vop

[PATCH v1 0/3] Refine error history and introduce notify_event vop

[Stanley Chu]

Hi,
This series refines error history functions and introduce a new notify_event vop to allow vendor to get notified of important events.

Stanley Chu (3):
  scsi: ufs: Add error history for abort event in UFS Device W-LUN
  scsi: ufs: Refine error history functions
  scsi: ufs: Introduce notify_event variant function

 drivers/scsi/ufs/ufshcd.c | 122 ++++++++++++++++++++++----------------
 drivers/scsi/ufs/ufshcd.h |  82 ++++++++++++-------------
 2 files changed, 112 insertions(+), 92 deletions(-)

-- 
2.18.0

[PATCH v1 1/3] scsi: ufs: Add error history for abort event in UFS Device W-LUN

[Stanley Chu]

Add error history for abort event in UFS Device W-LUN.
Besides, use specified value as parameter of ufshcd_update_reg_hist()
to identify the aborted tag or LUNs.

Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
---
 drivers/scsi/ufs/ufshcd.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 0e5473d4699b..28e4def13f21 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -6742,8 +6742,10 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
 	 * To avoid these unnecessary/illegal step we skip to the last error
 	 * handling stage: reset and restore.
 	 */
-	if (lrbp->lun == UFS_UPIU_UFS_DEVICE_WLUN)
+	if (lrbp->lun == UFS_UPIU_UFS_DEVICE_WLUN) {
+		ufshcd_update_reg_hist(&hba->ufs_stats.task_abort, lrbp->lun);
 		return ufshcd_eh_host_reset_handler(cmd);
+	}
 
 	ufshcd_hold(hba, false);
 	reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
@@ -6767,7 +6769,7 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
 	 */
 	scsi_print_command(hba->lrb[tag].cmd);
 	if (!hba->req_abort_count) {
-		ufshcd_update_reg_hist(&hba->ufs_stats.task_abort, 0);
+		ufshcd_update_reg_hist(&hba->ufs_stats.task_abort, tag);
 		ufshcd_print_host_regs(hba);
 		ufshcd_print_host_state(hba);
 		ufshcd_print_pwr_info(hba);
-- 
2.18.0

[PATCH v1 2/3] scsi: ufs: Refine error history functions

[Stanley Chu]

Nowadays UFS error history does not only have "history of errors"
but also have history of some other events which are not defined
as errors.

This patch fixes the confused naming of related functions,
and change the way for updating and printing history as preparation
of next patch.

This patch shall not change any functionality.

Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
---
 drivers/scsi/ufs/ufshcd.c | 118 +++++++++++++++++++++-----------------
 drivers/scsi/ufs/ufshcd.h |  71 ++++++++++-------------
 2 files changed, 97 insertions(+), 92 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 28e4def13f21..7194bed1f10b 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -413,20 +413,25 @@ static void ufshcd_print_clk_freqs(struct ufs_hba *hba)
 	}
 }
 
-static void ufshcd_print_err_hist(struct ufs_hba *hba,
-				  struct ufs_err_reg_hist *err_hist,
-				  char *err_name)
+static void ufshcd_print_evt(struct ufs_hba *hba, u32 id,
+			     char *err_name)
 {
 	int i;
 	bool found = false;
+	struct ufs_event_hist *e;
 
-	for (i = 0; i < UFS_ERR_REG_HIST_LENGTH; i++) {
-		int p = (i + err_hist->pos) % UFS_ERR_REG_HIST_LENGTH;
+	if (id >= UFS_EVT_CNT)
+		return;
+
+	e = &hba->ufs_stats.event[id];
 
-		if (err_hist->tstamp[p] == 0)
+	for (i = 0; i < UFS_EVENT_HIST_LENGTH; i++) {
+		int p = (i + e->pos) % UFS_EVENT_HIST_LENGTH;
+
+		if (e->tstamp[p] == 0)
 			continue;
 		dev_err(hba->dev, "%s[%d] = 0x%x at %lld us\n", err_name, p,
-			err_hist->reg[p], ktime_to_us(err_hist->tstamp[p]));
+			e->val[p], ktime_to_us(e->tstamp[p]));
 		found = true;
 	}
 
@@ -434,26 +439,26 @@ static void ufshcd_print_err_hist(struct ufs_hba *hba,
 		dev_err(hba->dev, "No record of %s\n", err_name);
 }
 
-static void ufshcd_print_host_regs(struct ufs_hba *hba)
+static void ufshcd_print_evt_hist(struct ufs_hba *hba)
 {
 	ufshcd_dump_regs(hba, 0, UFSHCI_REG_SPACE_SIZE, "host_regs: ");
 
-	ufshcd_print_err_hist(hba, &hba->ufs_stats.pa_err, "pa_err");
-	ufshcd_print_err_hist(hba, &hba->ufs_stats.dl_err, "dl_err");
-	ufshcd_print_err_hist(hba, &hba->ufs_stats.nl_err, "nl_err");
-	ufshcd_print_err_hist(hba, &hba->ufs_stats.tl_err, "tl_err");
-	ufshcd_print_err_hist(hba, &hba->ufs_stats.dme_err, "dme_err");
-	ufshcd_print_err_hist(hba, &hba->ufs_stats.auto_hibern8_err,
-			      "auto_hibern8_err");
-	ufshcd_print_err_hist(hba, &hba->ufs_stats.fatal_err, "fatal_err");
-	ufshcd_print_err_hist(hba, &hba->ufs_stats.link_startup_err,
-			      "link_startup_fail");
-	ufshcd_print_err_hist(hba, &hba->ufs_stats.resume_err, "resume_fail");
-	ufshcd_print_err_hist(hba, &hba->ufs_stats.suspend_err,
-			      "suspend_fail");
-	ufshcd_print_err_hist(hba, &hba->ufs_stats.dev_reset, "dev_reset");
-	ufshcd_print_err_hist(hba, &hba->ufs_stats.host_reset, "host_reset");
-	ufshcd_print_err_hist(hba, &hba->ufs_stats.task_abort, "task_abort");
+	ufshcd_print_evt(hba, UFS_EVT_PA_ERR, "pa_err");
+	ufshcd_print_evt(hba, UFS_EVT_DL_ERR, "dl_err");
+	ufshcd_print_evt(hba, UFS_EVT_NL_ERR, "nl_err");
+	ufshcd_print_evt(hba, UFS_EVT_TL_ERR, "tl_err");
+	ufshcd_print_evt(hba, UFS_EVT_DME_ERR, "dme_err");
+	ufshcd_print_evt(hba, UFS_EVT_AUTO_HIBERN8_ERR,
+			 "auto_hibern8_err");
+	ufshcd_print_evt(hba, UFS_EVT_FATAL_ERR, "fatal_err");
+	ufshcd_print_evt(hba, UFS_EVT_LINK_STARTUP_FAIL,
+			 "link_startup_fail");
+	ufshcd_print_evt(hba, UFS_EVT_RESUME_ERR, "resume_fail");
+	ufshcd_print_evt(hba, UFS_EVT_SUSPEND_ERR,
+			 "suspend_fail");
+	ufshcd_print_evt(hba, UFS_EVT_DEV_RESET, "dev_reset");
+	ufshcd_print_evt(hba, UFS_EVT_HOST_RESET, "host_reset");
+	ufshcd_print_evt(hba, UFS_EVT_ABORT, "task_abort");
 
 	ufshcd_vops_dbg_register_dump(hba);
 }
@@ -3856,7 +3861,7 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba *hba, struct uic_command *cmd)
 	if (ret) {
 		ufshcd_print_host_state(hba);
 		ufshcd_print_pwr_info(hba);
-		ufshcd_print_host_regs(hba);
+		ufshcd_print_evt_hist(hba);
 	}
 
 	spin_lock_irqsave(hba->host->host_lock, flags);
@@ -4468,14 +4473,19 @@ static inline int ufshcd_disable_device_tx_lcc(struct ufs_hba *hba)
 	return ufshcd_disable_tx_lcc(hba, true);
 }
 
-void ufshcd_update_reg_hist(struct ufs_err_reg_hist *reg_hist,
-			    u32 reg)
+void ufshcd_update_evt_hist(struct ufs_hba *hba, u32 id, u32 val)
 {
-	reg_hist->reg[reg_hist->pos] = reg;
-	reg_hist->tstamp[reg_hist->pos] = ktime_get();
-	reg_hist->pos = (reg_hist->pos + 1) % UFS_ERR_REG_HIST_LENGTH;
+	struct ufs_event_hist *e;
+
+	if (id >= UFS_EVT_CNT)
+		return;
+
+	e = &hba->ufs_stats.event[id];
+	e->val[e->pos] = val;
+	e->tstamp[e->pos] = ktime_get();
+	e->pos = (e->pos + 1) % UFS_EVENT_HIST_LENGTH;
 }
-EXPORT_SYMBOL_GPL(ufshcd_update_reg_hist);
+EXPORT_SYMBOL_GPL(ufshcd_update_evt_hist);
 
 /**
  * ufshcd_link_startup - Initialize unipro link startup
@@ -4504,7 +4514,8 @@ static int ufshcd_link_startup(struct ufs_hba *hba)
 
 		/* check if device is detected by inter-connect layer */
 		if (!ret && !ufshcd_is_device_present(hba)) {
-			ufshcd_update_reg_hist(&hba->ufs_stats.link_startup_err,
+			ufshcd_update_evt_hist(hba,
+					       UFS_EVT_LINK_STARTUP_FAIL,
 					       0);
 			dev_err(hba->dev, "%s: Device not present\n", __func__);
 			ret = -ENXIO;
@@ -4517,7 +4528,8 @@ static int ufshcd_link_startup(struct ufs_hba *hba)
 		 * succeeds. So reset the local Uni-Pro and try again.
 		 */
 		if (ret && ufshcd_hba_enable(hba)) {
-			ufshcd_update_reg_hist(&hba->ufs_stats.link_startup_err,
+			ufshcd_update_evt_hist(hba,
+					       UFS_EVT_LINK_STARTUP_FAIL,
 					       (u32)ret);
 			goto out;
 		}
@@ -4525,7 +4537,8 @@ static int ufshcd_link_startup(struct ufs_hba *hba)
 
 	if (ret) {
 		/* failed to get the link up... retire */
-		ufshcd_update_reg_hist(&hba->ufs_stats.link_startup_err,
+		ufshcd_update_evt_hist(hba,
+				       UFS_EVT_LINK_STARTUP_FAIL,
 				       (u32)ret);
 		goto out;
 	}
@@ -4559,7 +4572,7 @@ static int ufshcd_link_startup(struct ufs_hba *hba)
 		dev_err(hba->dev, "link startup failed %d\n", ret);
 		ufshcd_print_host_state(hba);
 		ufshcd_print_pwr_info(hba);
-		ufshcd_print_host_regs(hba);
+		ufshcd_print_evt_hist(hba);
 	}
 	return ret;
 }
@@ -4914,7 +4927,7 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
 		dev_err(hba->dev,
 				"OCS error from controller = %x for tag %d\n",
 				ocs, lrbp->task_tag);
-		ufshcd_print_host_regs(hba);
+		ufshcd_print_evt_hist(hba);
 		ufshcd_print_host_state(hba);
 		break;
 	} /* end of switch */
@@ -5796,7 +5809,7 @@ static void ufshcd_err_handler(struct work_struct *work)
 		spin_unlock_irqrestore(hba->host->host_lock, flags);
 		ufshcd_print_host_state(hba);
 		ufshcd_print_pwr_info(hba);
-		ufshcd_print_host_regs(hba);
+		ufshcd_print_evt_hist(hba);
 		ufshcd_print_tmrs(hba, hba->outstanding_tasks);
 		ufshcd_print_trs(hba, hba->outstanding_reqs, pr_prdt);
 		spin_lock_irqsave(hba->host->host_lock, flags);
@@ -5941,7 +5954,7 @@ static irqreturn_t ufshcd_update_uic_error(struct ufs_hba *hba)
 	reg = ufshcd_readl(hba, REG_UIC_ERROR_CODE_PHY_ADAPTER_LAYER);
 	if ((reg & UIC_PHY_ADAPTER_LAYER_ERROR) &&
 	    (reg & UIC_PHY_ADAPTER_LAYER_ERROR_CODE_MASK)) {
-		ufshcd_update_reg_hist(&hba->ufs_stats.pa_err, reg);
+		ufshcd_update_evt_hist(hba, UFS_EVT_PA_ERR, reg);
 		/*
 		 * To know whether this error is fatal or not, DB timeout
 		 * must be checked but this error is handled separately.
@@ -5971,7 +5984,7 @@ static irqreturn_t ufshcd_update_uic_error(struct ufs_hba *hba)
 	reg = ufshcd_readl(hba, REG_UIC_ERROR_CODE_DATA_LINK_LAYER);
 	if ((reg & UIC_DATA_LINK_LAYER_ERROR) &&
 	    (reg & UIC_DATA_LINK_LAYER_ERROR_CODE_MASK)) {
-		ufshcd_update_reg_hist(&hba->ufs_stats.dl_err, reg);
+		ufshcd_update_evt_hist(hba, UFS_EVT_DL_ERR, reg);
 
 		if (reg & UIC_DATA_LINK_LAYER_ERROR_PA_INIT)
 			hba->uic_error |= UFSHCD_UIC_DL_PA_INIT_ERROR;
@@ -5990,7 +6003,7 @@ static irqreturn_t ufshcd_update_uic_error(struct ufs_hba *hba)
 	reg = ufshcd_readl(hba, REG_UIC_ERROR_CODE_NETWORK_LAYER);
 	if ((reg & UIC_NETWORK_LAYER_ERROR) &&
 	    (reg & UIC_NETWORK_LAYER_ERROR_CODE_MASK)) {
-		ufshcd_update_reg_hist(&hba->ufs_stats.nl_err, reg);
+		ufshcd_update_evt_hist(hba, UFS_EVT_NL_ERR, reg);
 		hba->uic_error |= UFSHCD_UIC_NL_ERROR;
 		retval |= IRQ_HANDLED;
 	}
@@ -5998,7 +6011,7 @@ static irqreturn_t ufshcd_update_uic_error(struct ufs_hba *hba)
 	reg = ufshcd_readl(hba, REG_UIC_ERROR_CODE_TRANSPORT_LAYER);
 	if ((reg & UIC_TRANSPORT_LAYER_ERROR) &&
 	    (reg & UIC_TRANSPORT_LAYER_ERROR_CODE_MASK)) {
-		ufshcd_update_reg_hist(&hba->ufs_stats.tl_err, reg);
+		ufshcd_update_evt_hist(hba, UFS_EVT_TL_ERR, reg);
 		hba->uic_error |= UFSHCD_UIC_TL_ERROR;
 		retval |= IRQ_HANDLED;
 	}
@@ -6006,7 +6019,7 @@ static irqreturn_t ufshcd_update_uic_error(struct ufs_hba *hba)
 	reg = ufshcd_readl(hba, REG_UIC_ERROR_CODE_DME);
 	if ((reg & UIC_DME_ERROR) &&
 	    (reg & UIC_DME_ERROR_CODE_MASK)) {
-		ufshcd_update_reg_hist(&hba->ufs_stats.dme_err, reg);
+		ufshcd_update_evt_hist(hba, UFS_EVT_DME_ERR, reg);
 		hba->uic_error |= UFSHCD_UIC_DME_ERROR;
 		retval |= IRQ_HANDLED;
 	}
@@ -6048,7 +6061,8 @@ static irqreturn_t ufshcd_check_errors(struct ufs_hba *hba)
 	irqreturn_t retval = IRQ_NONE;
 
 	if (hba->errors & INT_FATAL_ERRORS) {
-		ufshcd_update_reg_hist(&hba->ufs_stats.fatal_err, hba->errors);
+		ufshcd_update_evt_hist(hba, UFS_EVT_FATAL_ERR,
+				       hba->errors);
 		queue_eh_work = true;
 	}
 
@@ -6065,7 +6079,7 @@ static irqreturn_t ufshcd_check_errors(struct ufs_hba *hba)
 			__func__, (hba->errors & UIC_HIBERNATE_ENTER) ?
 			"Enter" : "Exit",
 			hba->errors, ufshcd_get_upmcrs(hba));
-		ufshcd_update_reg_hist(&hba->ufs_stats.auto_hibern8_err,
+		ufshcd_update_evt_hist(hba, UFS_EVT_AUTO_HIBERN8_ERR,
 				       hba->errors);
 		ufshcd_set_link_broken(hba);
 		queue_eh_work = true;
@@ -6606,7 +6620,7 @@ static int ufshcd_eh_device_reset_handler(struct scsi_cmnd *cmd)
 
 out:
 	hba->req_abort_count = 0;
-	ufshcd_update_reg_hist(&hba->ufs_stats.dev_reset, (u32)err);
+	ufshcd_update_evt_hist(hba, UFS_EVT_DEV_RESET, (u32)err);
 	if (!err) {
 		err = SUCCESS;
 	} else {
@@ -6743,7 +6757,7 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
 	 * handling stage: reset and restore.
 	 */
 	if (lrbp->lun == UFS_UPIU_UFS_DEVICE_WLUN) {
-		ufshcd_update_reg_hist(&hba->ufs_stats.task_abort, lrbp->lun);
+		ufshcd_update_evt_hist(hba, UFS_EVT_ABORT, lrbp->lun);
 		return ufshcd_eh_host_reset_handler(cmd);
 	}
 
@@ -6769,8 +6783,8 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
 	 */
 	scsi_print_command(hba->lrb[tag].cmd);
 	if (!hba->req_abort_count) {
-		ufshcd_update_reg_hist(&hba->ufs_stats.task_abort, tag);
-		ufshcd_print_host_regs(hba);
+		ufshcd_update_evt_hist(hba, UFS_EVT_ABORT, tag);
+		ufshcd_print_evt_hist(hba);
 		ufshcd_print_host_state(hba);
 		ufshcd_print_pwr_info(hba);
 		ufshcd_print_trs(hba, 1 << tag, true);
@@ -6853,7 +6867,7 @@ static int ufshcd_host_reset_and_restore(struct ufs_hba *hba)
 out:
 	if (err)
 		dev_err(hba->dev, "%s: Host init failed %d\n", __func__, err);
-	ufshcd_update_reg_hist(&hba->ufs_stats.host_reset, (u32)err);
+	ufshcd_update_evt_hist(hba, UFS_EVT_HOST_RESET, (u32)err);
 	return err;
 }
 
@@ -8708,7 +8722,7 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
 	hba->pm_op_in_progress = 0;
 
 	if (ret)
-		ufshcd_update_reg_hist(&hba->ufs_stats.suspend_err, (u32)ret);
+		ufshcd_update_evt_hist(hba, UFS_EVT_SUSPEND_ERR, (u32)ret);
 	return ret;
 }
 
@@ -8832,7 +8846,7 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
 out:
 	hba->pm_op_in_progress = 0;
 	if (ret)
-		ufshcd_update_reg_hist(&hba->ufs_stats.resume_err, (u32)ret);
+		ufshcd_update_evt_hist(hba, UFS_EVT_RESUME_ERR, (u32)ret);
 	return ret;
 }
 
@@ -9284,7 +9298,7 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 	err = ufshcd_hba_enable(hba);
 	if (err) {
 		dev_err(hba->dev, "Host controller enable failed\n");
-		ufshcd_print_host_regs(hba);
+		ufshcd_print_evt_hist(hba);
 		ufshcd_print_host_state(hba);
 		goto free_tmf_queue;
 	}
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 61344c49c2cc..82c2fc5597bb 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -58,6 +58,29 @@ enum dev_cmd_type {
 	DEV_CMD_TYPE_QUERY		= 0x1,
 };
 
+enum ufs_event_type {
+	/* uic specific errors */
+	UFS_EVT_PA_ERR = 0,
+	UFS_EVT_DL_ERR,
+	UFS_EVT_NL_ERR,
+	UFS_EVT_TL_ERR,
+	UFS_EVT_DME_ERR,
+
+	/* fatal errors */
+	UFS_EVT_AUTO_HIBERN8_ERR,
+	UFS_EVT_FATAL_ERR,
+	UFS_EVT_LINK_STARTUP_FAIL,
+	UFS_EVT_RESUME_ERR,
+	UFS_EVT_SUSPEND_ERR,
+
+	/* abnormal events */
+	UFS_EVT_DEV_RESET,
+	UFS_EVT_HOST_RESET,
+	UFS_EVT_ABORT,
+
+	UFS_EVT_CNT,
+};
+
 /**
  * struct uic_command - UIC command structure
  * @command: UIC command
@@ -406,17 +429,17 @@ struct ufs_clk_scaling {
 	bool is_suspended;
 };
 
-#define UFS_ERR_REG_HIST_LENGTH 8
+#define UFS_EVENT_HIST_LENGTH 8
 /**
- * struct ufs_err_reg_hist - keeps history of errors
+ * struct ufs_event_hist - keeps history of errors
  * @pos: index to indicate cyclic buffer position
  * @reg: cyclic buffer for registers value
  * @tstamp: cyclic buffer for time stamp
  */
-struct ufs_err_reg_hist {
+struct ufs_event_hist {
 	int pos;
-	u32 reg[UFS_ERR_REG_HIST_LENGTH];
-	ktime_t tstamp[UFS_ERR_REG_HIST_LENGTH];
+	u32 val[UFS_EVENT_HIST_LENGTH];
+	ktime_t tstamp[UFS_EVENT_HIST_LENGTH];
 };
 
 /**
@@ -427,19 +450,6 @@ struct ufs_err_reg_hist {
  *		reset this after link-startup.
  * @last_hibern8_exit_tstamp: Set time after the hibern8 exit.
  *		Clear after the first successful command completion.
- * @pa_err: tracks pa-uic errors
- * @dl_err: tracks dl-uic errors
- * @nl_err: tracks nl-uic errors
- * @tl_err: tracks tl-uic errors
- * @dme_err: tracks dme errors
- * @auto_hibern8_err: tracks auto-hibernate errors
- * @fatal_err: tracks fatal errors
- * @linkup_err: tracks link-startup errors
- * @resume_err: tracks resume errors
- * @suspend_err: tracks suspend errors
- * @dev_reset: tracks device reset events
- * @host_reset: tracks host reset events
- * @tsk_abort: tracks task abort events
  */
 struct ufs_stats {
 	u32 last_intr_status;
@@ -447,25 +457,7 @@ struct ufs_stats {
 
 	u32 hibern8_exit_cnt;
 	ktime_t last_hibern8_exit_tstamp;
-
-	/* uic specific errors */
-	struct ufs_err_reg_hist pa_err;
-	struct ufs_err_reg_hist dl_err;
-	struct ufs_err_reg_hist nl_err;
-	struct ufs_err_reg_hist tl_err;
-	struct ufs_err_reg_hist dme_err;
-
-	/* fatal errors */
-	struct ufs_err_reg_hist auto_hibern8_err;
-	struct ufs_err_reg_hist fatal_err;
-	struct ufs_err_reg_hist link_startup_err;
-	struct ufs_err_reg_hist resume_err;
-	struct ufs_err_reg_hist suspend_err;
-
-	/* abnormal events */
-	struct ufs_err_reg_hist dev_reset;
-	struct ufs_err_reg_hist host_reset;
-	struct ufs_err_reg_hist task_abort;
+	struct ufs_event_hist event[UFS_EVT_CNT];
 };
 
 enum ufshcd_quirks {
@@ -909,8 +901,7 @@ int ufshcd_wait_for_register(struct ufs_hba *hba, u32 reg, u32 mask,
 				u32 val, unsigned long interval_us,
 				unsigned long timeout_ms);
 void ufshcd_parse_dev_ref_clk_freq(struct ufs_hba *hba, struct clk *refclk);
-void ufshcd_update_reg_hist(struct ufs_err_reg_hist *reg_hist,
-			    u32 reg);
+void ufshcd_update_evt_hist(struct ufs_hba *hba, u32 id, u32 val);
 
 static inline void check_upiu_size(void)
 {
@@ -1216,7 +1207,7 @@ static inline void ufshcd_vops_device_reset(struct ufs_hba *hba)
 		if (!err)
 			ufshcd_set_ufs_dev_active(hba);
 		if (err != -EOPNOTSUPP)
-			ufshcd_update_reg_hist(&hba->ufs_stats.dev_reset, err);
+			ufshcd_update_evt_hist(hba, UFS_EVT_DEV_RESET, err);
 	}
 }
 
-- 
2.18.0

[PATCH v1 3/3] scsi: ufs: Introduce notify_event variant function

[Stanley Chu]

Introduce notify_event variant function to allow
vendor to get notified of important events and connect
to any proprietary debugging facilities.

Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
---
 drivers/scsi/ufs/ufshcd.c |  2 ++
 drivers/scsi/ufs/ufshcd.h | 11 +++++++++++
 2 files changed, 13 insertions(+)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 7194bed1f10b..63fe37e1c908 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -4484,6 +4484,8 @@ void ufshcd_update_evt_hist(struct ufs_hba *hba, u32 id, u32 val)
 	e->val[e->pos] = val;
 	e->tstamp[e->pos] = ktime_get();
 	e->pos = (e->pos + 1) % UFS_EVENT_HIST_LENGTH;
+
+	ufshcd_vops_notify_event(hba, id, &val);
 }
 EXPORT_SYMBOL_GPL(ufshcd_update_evt_hist);
 
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 82c2fc5597bb..a81ca36c1715 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -317,6 +317,7 @@ struct ufs_pwr_mode_info {
  * @phy_initialization: used to initialize phys
  * @device_reset: called to issue a reset pulse on the UFS device
  * @program_key: program or evict an inline encryption key
+ * @notify_event: called to notify important events
  */
 struct ufs_hba_variant_ops {
 	const char *name;
@@ -352,6 +353,8 @@ struct ufs_hba_variant_ops {
 					void *data);
 	int	(*program_key)(struct ufs_hba *hba,
 			       const union ufs_crypto_cfg_entry *cfg, int slot);
+	void	(*notify_event)(struct ufs_hba *hba,
+				enum ufs_event_type evt, void *data);
 };
 
 /* clock gating state  */
@@ -1097,6 +1100,14 @@ static inline int ufshcd_vops_clk_scale_notify(struct ufs_hba *hba,
 	return 0;
 }
 
+static inline void ufshcd_vops_notify_event(struct ufs_hba *hba,
+					    enum ufs_event_type evt,
+					    void *data)
+{
+	if (hba->vops && hba->vops->notify_event)
+		hba->vops->notify_event(hba, evt, data);
+}
+
 static inline int ufshcd_vops_setup_clocks(struct ufs_hba *hba, bool on,
 					enum ufs_notify_change_status status)
 {
-- 
2.18.0

Re: [PATCH v1 1/3] scsi: ufs: Add error history for abort event in UFS Device W-LUN

[Can Guo]

On 2020-11-26 13:38, Stanley Chu wrote:
> Add error history for abort event in UFS Device W-LUN.
> Besides, use specified value as parameter of ufshcd_update_reg_hist()
> to identify the aborted tag or LUNs.
> 
> Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
> ---
>  drivers/scsi/ufs/ufshcd.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 0e5473d4699b..28e4def13f21 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -6742,8 +6742,10 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
>  	 * To avoid these unnecessary/illegal step we skip to the last error
>  	 * handling stage: reset and restore.
>  	 */
> -	if (lrbp->lun == UFS_UPIU_UFS_DEVICE_WLUN)
> +	if (lrbp->lun == UFS_UPIU_UFS_DEVICE_WLUN) {
> +		ufshcd_update_reg_hist(&hba->ufs_stats.task_abort, lrbp->lun);
>  		return ufshcd_eh_host_reset_handler(cmd);
> +	}
> 
>  	ufshcd_hold(hba, false);
>  	reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
> @@ -6767,7 +6769,7 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
>  	 */
>  	scsi_print_command(hba->lrb[tag].cmd);
>  	if (!hba->req_abort_count) {
> -		ufshcd_update_reg_hist(&hba->ufs_stats.task_abort, 0);
> +		ufshcd_update_reg_hist(&hba->ufs_stats.task_abort, tag);
>  		ufshcd_print_host_regs(hba);
>  		ufshcd_print_host_state(hba);
>  		ufshcd_print_pwr_info(hba);

Reviewed-by: Can Guo <cang@codeaurora.org>

Re: [PATCH v1 2/3] scsi: ufs: Refine error history functions

[Can Guo]

On 2020-11-26 13:38, Stanley Chu wrote:
> Nowadays UFS error history does not only have "history of errors"
> but also have history of some other events which are not defined
> as errors.
> 
> This patch fixes the confused naming of related functions,
> and change the way for updating and printing history as preparation
> of next patch.
> 
> This patch shall not change any functionality.
> 
> Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
> ---
>  drivers/scsi/ufs/ufshcd.c | 118 +++++++++++++++++++++-----------------
>  drivers/scsi/ufs/ufshcd.h |  71 ++++++++++-------------
>  2 files changed, 97 insertions(+), 92 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 28e4def13f21..7194bed1f10b 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -413,20 +413,25 @@ static void ufshcd_print_clk_freqs(struct ufs_hba 
> *hba)
>  	}
>  }
> 
> -static void ufshcd_print_err_hist(struct ufs_hba *hba,
> -				  struct ufs_err_reg_hist *err_hist,
> -				  char *err_name)
> +static void ufshcd_print_evt(struct ufs_hba *hba, u32 id,
> +			     char *err_name)
>  {
>  	int i;
>  	bool found = false;
> +	struct ufs_event_hist *e;
> 
> -	for (i = 0; i < UFS_ERR_REG_HIST_LENGTH; i++) {
> -		int p = (i + err_hist->pos) % UFS_ERR_REG_HIST_LENGTH;
> +	if (id >= UFS_EVT_CNT)
> +		return;
> +
> +	e = &hba->ufs_stats.event[id];
> 
> -		if (err_hist->tstamp[p] == 0)
> +	for (i = 0; i < UFS_EVENT_HIST_LENGTH; i++) {
> +		int p = (i + e->pos) % UFS_EVENT_HIST_LENGTH;
> +
> +		if (e->tstamp[p] == 0)
>  			continue;
>  		dev_err(hba->dev, "%s[%d] = 0x%x at %lld us\n", err_name, p,
> -			err_hist->reg[p], ktime_to_us(err_hist->tstamp[p]));
> +			e->val[p], ktime_to_us(e->tstamp[p]));
>  		found = true;
>  	}
> 
> @@ -434,26 +439,26 @@ static void ufshcd_print_err_hist(struct ufs_hba 
> *hba,
>  		dev_err(hba->dev, "No record of %s\n", err_name);
>  }
> 
> -static void ufshcd_print_host_regs(struct ufs_hba *hba)
> +static void ufshcd_print_evt_hist(struct ufs_hba *hba)
>  {
>  	ufshcd_dump_regs(hba, 0, UFSHCI_REG_SPACE_SIZE, "host_regs: ");
> 
> -	ufshcd_print_err_hist(hba, &hba->ufs_stats.pa_err, "pa_err");
> -	ufshcd_print_err_hist(hba, &hba->ufs_stats.dl_err, "dl_err");
> -	ufshcd_print_err_hist(hba, &hba->ufs_stats.nl_err, "nl_err");
> -	ufshcd_print_err_hist(hba, &hba->ufs_stats.tl_err, "tl_err");
> -	ufshcd_print_err_hist(hba, &hba->ufs_stats.dme_err, "dme_err");
> -	ufshcd_print_err_hist(hba, &hba->ufs_stats.auto_hibern8_err,
> -			      "auto_hibern8_err");
> -	ufshcd_print_err_hist(hba, &hba->ufs_stats.fatal_err, "fatal_err");
> -	ufshcd_print_err_hist(hba, &hba->ufs_stats.link_startup_err,
> -			      "link_startup_fail");
> -	ufshcd_print_err_hist(hba, &hba->ufs_stats.resume_err, 
> "resume_fail");
> -	ufshcd_print_err_hist(hba, &hba->ufs_stats.suspend_err,
> -			      "suspend_fail");
> -	ufshcd_print_err_hist(hba, &hba->ufs_stats.dev_reset, "dev_reset");
> -	ufshcd_print_err_hist(hba, &hba->ufs_stats.host_reset, "host_reset");
> -	ufshcd_print_err_hist(hba, &hba->ufs_stats.task_abort, "task_abort");
> +	ufshcd_print_evt(hba, UFS_EVT_PA_ERR, "pa_err");
> +	ufshcd_print_evt(hba, UFS_EVT_DL_ERR, "dl_err");
> +	ufshcd_print_evt(hba, UFS_EVT_NL_ERR, "nl_err");
> +	ufshcd_print_evt(hba, UFS_EVT_TL_ERR, "tl_err");
> +	ufshcd_print_evt(hba, UFS_EVT_DME_ERR, "dme_err");
> +	ufshcd_print_evt(hba, UFS_EVT_AUTO_HIBERN8_ERR,
> +			 "auto_hibern8_err");
> +	ufshcd_print_evt(hba, UFS_EVT_FATAL_ERR, "fatal_err");
> +	ufshcd_print_evt(hba, UFS_EVT_LINK_STARTUP_FAIL,
> +			 "link_startup_fail");
> +	ufshcd_print_evt(hba, UFS_EVT_RESUME_ERR, "resume_fail");
> +	ufshcd_print_evt(hba, UFS_EVT_SUSPEND_ERR,
> +			 "suspend_fail");
> +	ufshcd_print_evt(hba, UFS_EVT_DEV_RESET, "dev_reset");
> +	ufshcd_print_evt(hba, UFS_EVT_HOST_RESET, "host_reset");
> +	ufshcd_print_evt(hba, UFS_EVT_ABORT, "task_abort");
> 
>  	ufshcd_vops_dbg_register_dump(hba);
>  }
> @@ -3856,7 +3861,7 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba
> *hba, struct uic_command *cmd)
>  	if (ret) {
>  		ufshcd_print_host_state(hba);
>  		ufshcd_print_pwr_info(hba);
> -		ufshcd_print_host_regs(hba);
> +		ufshcd_print_evt_hist(hba);
>  	}
> 
>  	spin_lock_irqsave(hba->host->host_lock, flags);
> @@ -4468,14 +4473,19 @@ static inline int
> ufshcd_disable_device_tx_lcc(struct ufs_hba *hba)
>  	return ufshcd_disable_tx_lcc(hba, true);
>  }
> 
> -void ufshcd_update_reg_hist(struct ufs_err_reg_hist *reg_hist,
> -			    u32 reg)
> +void ufshcd_update_evt_hist(struct ufs_hba *hba, u32 id, u32 val)
>  {
> -	reg_hist->reg[reg_hist->pos] = reg;
> -	reg_hist->tstamp[reg_hist->pos] = ktime_get();
> -	reg_hist->pos = (reg_hist->pos + 1) % UFS_ERR_REG_HIST_LENGTH;
> +	struct ufs_event_hist *e;
> +
> +	if (id >= UFS_EVT_CNT)
> +		return;
> +
> +	e = &hba->ufs_stats.event[id];
> +	e->val[e->pos] = val;
> +	e->tstamp[e->pos] = ktime_get();
> +	e->pos = (e->pos + 1) % UFS_EVENT_HIST_LENGTH;
>  }
> -EXPORT_SYMBOL_GPL(ufshcd_update_reg_hist);
> +EXPORT_SYMBOL_GPL(ufshcd_update_evt_hist);
> 
>  /**
>   * ufshcd_link_startup - Initialize unipro link startup
> @@ -4504,7 +4514,8 @@ static int ufshcd_link_startup(struct ufs_hba 
> *hba)
> 
>  		/* check if device is detected by inter-connect layer */
>  		if (!ret && !ufshcd_is_device_present(hba)) {
> -			ufshcd_update_reg_hist(&hba->ufs_stats.link_startup_err,
> +			ufshcd_update_evt_hist(hba,
> +					       UFS_EVT_LINK_STARTUP_FAIL,
>  					       0);
>  			dev_err(hba->dev, "%s: Device not present\n", __func__);
>  			ret = -ENXIO;
> @@ -4517,7 +4528,8 @@ static int ufshcd_link_startup(struct ufs_hba 
> *hba)
>  		 * succeeds. So reset the local Uni-Pro and try again.
>  		 */
>  		if (ret && ufshcd_hba_enable(hba)) {
> -			ufshcd_update_reg_hist(&hba->ufs_stats.link_startup_err,
> +			ufshcd_update_evt_hist(hba,
> +					       UFS_EVT_LINK_STARTUP_FAIL,
>  					       (u32)ret);
>  			goto out;
>  		}
> @@ -4525,7 +4537,8 @@ static int ufshcd_link_startup(struct ufs_hba 
> *hba)
> 
>  	if (ret) {
>  		/* failed to get the link up... retire */
> -		ufshcd_update_reg_hist(&hba->ufs_stats.link_startup_err,
> +		ufshcd_update_evt_hist(hba,
> +				       UFS_EVT_LINK_STARTUP_FAIL,
>  				       (u32)ret);
>  		goto out;
>  	}
> @@ -4559,7 +4572,7 @@ static int ufshcd_link_startup(struct ufs_hba 
> *hba)
>  		dev_err(hba->dev, "link startup failed %d\n", ret);
>  		ufshcd_print_host_state(hba);
>  		ufshcd_print_pwr_info(hba);
> -		ufshcd_print_host_regs(hba);
> +		ufshcd_print_evt_hist(hba);
>  	}
>  	return ret;
>  }
> @@ -4914,7 +4927,7 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba,
> struct ufshcd_lrb *lrbp)
>  		dev_err(hba->dev,
>  				"OCS error from controller = %x for tag %d\n",
>  				ocs, lrbp->task_tag);
> -		ufshcd_print_host_regs(hba);
> +		ufshcd_print_evt_hist(hba);
>  		ufshcd_print_host_state(hba);
>  		break;
>  	} /* end of switch */
> @@ -5796,7 +5809,7 @@ static void ufshcd_err_handler(struct work_struct 
> *work)
>  		spin_unlock_irqrestore(hba->host->host_lock, flags);
>  		ufshcd_print_host_state(hba);
>  		ufshcd_print_pwr_info(hba);
> -		ufshcd_print_host_regs(hba);
> +		ufshcd_print_evt_hist(hba);
>  		ufshcd_print_tmrs(hba, hba->outstanding_tasks);
>  		ufshcd_print_trs(hba, hba->outstanding_reqs, pr_prdt);
>  		spin_lock_irqsave(hba->host->host_lock, flags);
> @@ -5941,7 +5954,7 @@ static irqreturn_t
> ufshcd_update_uic_error(struct ufs_hba *hba)
>  	reg = ufshcd_readl(hba, REG_UIC_ERROR_CODE_PHY_ADAPTER_LAYER);
>  	if ((reg & UIC_PHY_ADAPTER_LAYER_ERROR) &&
>  	    (reg & UIC_PHY_ADAPTER_LAYER_ERROR_CODE_MASK)) {
> -		ufshcd_update_reg_hist(&hba->ufs_stats.pa_err, reg);
> +		ufshcd_update_evt_hist(hba, UFS_EVT_PA_ERR, reg);
>  		/*
>  		 * To know whether this error is fatal or not, DB timeout
>  		 * must be checked but this error is handled separately.
> @@ -5971,7 +5984,7 @@ static irqreturn_t
> ufshcd_update_uic_error(struct ufs_hba *hba)
>  	reg = ufshcd_readl(hba, REG_UIC_ERROR_CODE_DATA_LINK_LAYER);
>  	if ((reg & UIC_DATA_LINK_LAYER_ERROR) &&
>  	    (reg & UIC_DATA_LINK_LAYER_ERROR_CODE_MASK)) {
> -		ufshcd_update_reg_hist(&hba->ufs_stats.dl_err, reg);
> +		ufshcd_update_evt_hist(hba, UFS_EVT_DL_ERR, reg);
> 
>  		if (reg & UIC_DATA_LINK_LAYER_ERROR_PA_INIT)
>  			hba->uic_error |= UFSHCD_UIC_DL_PA_INIT_ERROR;
> @@ -5990,7 +6003,7 @@ static irqreturn_t
> ufshcd_update_uic_error(struct ufs_hba *hba)
>  	reg = ufshcd_readl(hba, REG_UIC_ERROR_CODE_NETWORK_LAYER);
>  	if ((reg & UIC_NETWORK_LAYER_ERROR) &&
>  	    (reg & UIC_NETWORK_LAYER_ERROR_CODE_MASK)) {
> -		ufshcd_update_reg_hist(&hba->ufs_stats.nl_err, reg);
> +		ufshcd_update_evt_hist(hba, UFS_EVT_NL_ERR, reg);
>  		hba->uic_error |= UFSHCD_UIC_NL_ERROR;
>  		retval |= IRQ_HANDLED;
>  	}
> @@ -5998,7 +6011,7 @@ static irqreturn_t
> ufshcd_update_uic_error(struct ufs_hba *hba)
>  	reg = ufshcd_readl(hba, REG_UIC_ERROR_CODE_TRANSPORT_LAYER);
>  	if ((reg & UIC_TRANSPORT_LAYER_ERROR) &&
>  	    (reg & UIC_TRANSPORT_LAYER_ERROR_CODE_MASK)) {
> -		ufshcd_update_reg_hist(&hba->ufs_stats.tl_err, reg);
> +		ufshcd_update_evt_hist(hba, UFS_EVT_TL_ERR, reg);
>  		hba->uic_error |= UFSHCD_UIC_TL_ERROR;
>  		retval |= IRQ_HANDLED;
>  	}
> @@ -6006,7 +6019,7 @@ static irqreturn_t
> ufshcd_update_uic_error(struct ufs_hba *hba)
>  	reg = ufshcd_readl(hba, REG_UIC_ERROR_CODE_DME);
>  	if ((reg & UIC_DME_ERROR) &&
>  	    (reg & UIC_DME_ERROR_CODE_MASK)) {
> -		ufshcd_update_reg_hist(&hba->ufs_stats.dme_err, reg);
> +		ufshcd_update_evt_hist(hba, UFS_EVT_DME_ERR, reg);
>  		hba->uic_error |= UFSHCD_UIC_DME_ERROR;
>  		retval |= IRQ_HANDLED;
>  	}
> @@ -6048,7 +6061,8 @@ static irqreturn_t ufshcd_check_errors(struct
> ufs_hba *hba)
>  	irqreturn_t retval = IRQ_NONE;
> 
>  	if (hba->errors & INT_FATAL_ERRORS) {
> -		ufshcd_update_reg_hist(&hba->ufs_stats.fatal_err, hba->errors);
> +		ufshcd_update_evt_hist(hba, UFS_EVT_FATAL_ERR,
> +				       hba->errors);
>  		queue_eh_work = true;
>  	}
> 
> @@ -6065,7 +6079,7 @@ static irqreturn_t ufshcd_check_errors(struct
> ufs_hba *hba)
>  			__func__, (hba->errors & UIC_HIBERNATE_ENTER) ?
>  			"Enter" : "Exit",
>  			hba->errors, ufshcd_get_upmcrs(hba));
> -		ufshcd_update_reg_hist(&hba->ufs_stats.auto_hibern8_err,
> +		ufshcd_update_evt_hist(hba, UFS_EVT_AUTO_HIBERN8_ERR,
>  				       hba->errors);
>  		ufshcd_set_link_broken(hba);
>  		queue_eh_work = true;
> @@ -6606,7 +6620,7 @@ static int ufshcd_eh_device_reset_handler(struct
> scsi_cmnd *cmd)
> 
>  out:
>  	hba->req_abort_count = 0;
> -	ufshcd_update_reg_hist(&hba->ufs_stats.dev_reset, (u32)err);
> +	ufshcd_update_evt_hist(hba, UFS_EVT_DEV_RESET, (u32)err);
>  	if (!err) {
>  		err = SUCCESS;
>  	} else {
> @@ -6743,7 +6757,7 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
>  	 * handling stage: reset and restore.
>  	 */
>  	if (lrbp->lun == UFS_UPIU_UFS_DEVICE_WLUN) {
> -		ufshcd_update_reg_hist(&hba->ufs_stats.task_abort, lrbp->lun);
> +		ufshcd_update_evt_hist(hba, UFS_EVT_ABORT, lrbp->lun);
>  		return ufshcd_eh_host_reset_handler(cmd);
>  	}
> 
> @@ -6769,8 +6783,8 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
>  	 */
>  	scsi_print_command(hba->lrb[tag].cmd);
>  	if (!hba->req_abort_count) {
> -		ufshcd_update_reg_hist(&hba->ufs_stats.task_abort, tag);
> -		ufshcd_print_host_regs(hba);
> +		ufshcd_update_evt_hist(hba, UFS_EVT_ABORT, tag);
> +		ufshcd_print_evt_hist(hba);
>  		ufshcd_print_host_state(hba);
>  		ufshcd_print_pwr_info(hba);
>  		ufshcd_print_trs(hba, 1 << tag, true);
> @@ -6853,7 +6867,7 @@ static int ufshcd_host_reset_and_restore(struct
> ufs_hba *hba)
>  out:
>  	if (err)
>  		dev_err(hba->dev, "%s: Host init failed %d\n", __func__, err);
> -	ufshcd_update_reg_hist(&hba->ufs_stats.host_reset, (u32)err);
> +	ufshcd_update_evt_hist(hba, UFS_EVT_HOST_RESET, (u32)err);
>  	return err;
>  }
> 
> @@ -8708,7 +8722,7 @@ static int ufshcd_suspend(struct ufs_hba *hba,
> enum ufs_pm_op pm_op)
>  	hba->pm_op_in_progress = 0;
> 
>  	if (ret)
> -		ufshcd_update_reg_hist(&hba->ufs_stats.suspend_err, (u32)ret);
> +		ufshcd_update_evt_hist(hba, UFS_EVT_SUSPEND_ERR, (u32)ret);
>  	return ret;
>  }
> 
> @@ -8832,7 +8846,7 @@ static int ufshcd_resume(struct ufs_hba *hba,
> enum ufs_pm_op pm_op)
>  out:
>  	hba->pm_op_in_progress = 0;
>  	if (ret)
> -		ufshcd_update_reg_hist(&hba->ufs_stats.resume_err, (u32)ret);
> +		ufshcd_update_evt_hist(hba, UFS_EVT_RESUME_ERR, (u32)ret);
>  	return ret;
>  }
> 
> @@ -9284,7 +9298,7 @@ int ufshcd_init(struct ufs_hba *hba, void
> __iomem *mmio_base, unsigned int irq)
>  	err = ufshcd_hba_enable(hba);
>  	if (err) {
>  		dev_err(hba->dev, "Host controller enable failed\n");
> -		ufshcd_print_host_regs(hba);
> +		ufshcd_print_evt_hist(hba);
>  		ufshcd_print_host_state(hba);
>  		goto free_tmf_queue;
>  	}
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> index 61344c49c2cc..82c2fc5597bb 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -58,6 +58,29 @@ enum dev_cmd_type {
>  	DEV_CMD_TYPE_QUERY		= 0x1,
>  };
> 
> +enum ufs_event_type {
> +	/* uic specific errors */
> +	UFS_EVT_PA_ERR = 0,
> +	UFS_EVT_DL_ERR,
> +	UFS_EVT_NL_ERR,
> +	UFS_EVT_TL_ERR,
> +	UFS_EVT_DME_ERR,
> +
> +	/* fatal errors */
> +	UFS_EVT_AUTO_HIBERN8_ERR,
> +	UFS_EVT_FATAL_ERR,
> +	UFS_EVT_LINK_STARTUP_FAIL,
> +	UFS_EVT_RESUME_ERR,
> +	UFS_EVT_SUSPEND_ERR,
> +
> +	/* abnormal events */
> +	UFS_EVT_DEV_RESET,
> +	UFS_EVT_HOST_RESET,
> +	UFS_EVT_ABORT,
> +
> +	UFS_EVT_CNT,
> +};
> +
>  /**
>   * struct uic_command - UIC command structure
>   * @command: UIC command
> @@ -406,17 +429,17 @@ struct ufs_clk_scaling {
>  	bool is_suspended;
>  };
> 
> -#define UFS_ERR_REG_HIST_LENGTH 8
> +#define UFS_EVENT_HIST_LENGTH 8
>  /**
> - * struct ufs_err_reg_hist - keeps history of errors
> + * struct ufs_event_hist - keeps history of errors
>   * @pos: index to indicate cyclic buffer position
>   * @reg: cyclic buffer for registers value
>   * @tstamp: cyclic buffer for time stamp
>   */
> -struct ufs_err_reg_hist {
> +struct ufs_event_hist {
>  	int pos;
> -	u32 reg[UFS_ERR_REG_HIST_LENGTH];
> -	ktime_t tstamp[UFS_ERR_REG_HIST_LENGTH];
> +	u32 val[UFS_EVENT_HIST_LENGTH];
> +	ktime_t tstamp[UFS_EVENT_HIST_LENGTH];
>  };
> 
>  /**
> @@ -427,19 +450,6 @@ struct ufs_err_reg_hist {
>   *		reset this after link-startup.
>   * @last_hibern8_exit_tstamp: Set time after the hibern8 exit.
>   *		Clear after the first successful command completion.
> - * @pa_err: tracks pa-uic errors
> - * @dl_err: tracks dl-uic errors
> - * @nl_err: tracks nl-uic errors
> - * @tl_err: tracks tl-uic errors
> - * @dme_err: tracks dme errors
> - * @auto_hibern8_err: tracks auto-hibernate errors
> - * @fatal_err: tracks fatal errors
> - * @linkup_err: tracks link-startup errors
> - * @resume_err: tracks resume errors
> - * @suspend_err: tracks suspend errors
> - * @dev_reset: tracks device reset events
> - * @host_reset: tracks host reset events
> - * @tsk_abort: tracks task abort events
>   */
>  struct ufs_stats {
>  	u32 last_intr_status;
> @@ -447,25 +457,7 @@ struct ufs_stats {
> 
>  	u32 hibern8_exit_cnt;
>  	ktime_t last_hibern8_exit_tstamp;
> -
> -	/* uic specific errors */
> -	struct ufs_err_reg_hist pa_err;
> -	struct ufs_err_reg_hist dl_err;
> -	struct ufs_err_reg_hist nl_err;
> -	struct ufs_err_reg_hist tl_err;
> -	struct ufs_err_reg_hist dme_err;
> -
> -	/* fatal errors */
> -	struct ufs_err_reg_hist auto_hibern8_err;
> -	struct ufs_err_reg_hist fatal_err;
> -	struct ufs_err_reg_hist link_startup_err;
> -	struct ufs_err_reg_hist resume_err;
> -	struct ufs_err_reg_hist suspend_err;
> -
> -	/* abnormal events */
> -	struct ufs_err_reg_hist dev_reset;
> -	struct ufs_err_reg_hist host_reset;
> -	struct ufs_err_reg_hist task_abort;
> +	struct ufs_event_hist event[UFS_EVT_CNT];
>  };
> 
>  enum ufshcd_quirks {
> @@ -909,8 +901,7 @@ int ufshcd_wait_for_register(struct ufs_hba *hba,
> u32 reg, u32 mask,
>  				u32 val, unsigned long interval_us,
>  				unsigned long timeout_ms);
>  void ufshcd_parse_dev_ref_clk_freq(struct ufs_hba *hba, struct clk 
> *refclk);
> -void ufshcd_update_reg_hist(struct ufs_err_reg_hist *reg_hist,
> -			    u32 reg);
> +void ufshcd_update_evt_hist(struct ufs_hba *hba, u32 id, u32 val);
> 
>  static inline void check_upiu_size(void)
>  {
> @@ -1216,7 +1207,7 @@ static inline void
> ufshcd_vops_device_reset(struct ufs_hba *hba)
>  		if (!err)
>  			ufshcd_set_ufs_dev_active(hba);
>  		if (err != -EOPNOTSUPP)
> -			ufshcd_update_reg_hist(&hba->ufs_stats.dev_reset, err);
> +			ufshcd_update_evt_hist(hba, UFS_EVT_DEV_RESET, err);
>  	}
>  }

Reviewed-by: Can Guo <cang@codeaurora.org>

Re: [PATCH v1 3/3] scsi: ufs: Introduce notify_event variant function

[Can Guo]

On 2020-11-26 13:38, Stanley Chu wrote:
> Introduce notify_event variant function to allow
> vendor to get notified of important events and connect
> to any proprietary debugging facilities.
> 
> Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
> ---
>  drivers/scsi/ufs/ufshcd.c |  2 ++
>  drivers/scsi/ufs/ufshcd.h | 11 +++++++++++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 7194bed1f10b..63fe37e1c908 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -4484,6 +4484,8 @@ void ufshcd_update_evt_hist(struct ufs_hba *hba,
> u32 id, u32 val)
>  	e->val[e->pos] = val;
>  	e->tstamp[e->pos] = ktime_get();
>  	e->pos = (e->pos + 1) % UFS_EVENT_HIST_LENGTH;
> +
> +	ufshcd_vops_notify_event(hba, id, &val);
>  }
>  EXPORT_SYMBOL_GPL(ufshcd_update_evt_hist);
> 
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> index 82c2fc5597bb..a81ca36c1715 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -317,6 +317,7 @@ struct ufs_pwr_mode_info {
>   * @phy_initialization: used to initialize phys
>   * @device_reset: called to issue a reset pulse on the UFS device
>   * @program_key: program or evict an inline encryption key
> + * @notify_event: called to notify important events
>   */
>  struct ufs_hba_variant_ops {
>  	const char *name;
> @@ -352,6 +353,8 @@ struct ufs_hba_variant_ops {
>  					void *data);
>  	int	(*program_key)(struct ufs_hba *hba,
>  			       const union ufs_crypto_cfg_entry *cfg, int slot);
> +	void	(*notify_event)(struct ufs_hba *hba,
> +				enum ufs_event_type evt, void *data);
>  };
> 
>  /* clock gating state  */
> @@ -1097,6 +1100,14 @@ static inline int
> ufshcd_vops_clk_scale_notify(struct ufs_hba *hba,
>  	return 0;
>  }
> 
> +static inline void ufshcd_vops_notify_event(struct ufs_hba *hba,
> +					    enum ufs_event_type evt,
> +					    void *data)
> +{
> +	if (hba->vops && hba->vops->notify_event)
> +		hba->vops->notify_event(hba, evt, data);
> +}
> +
>  static inline int ufshcd_vops_setup_clocks(struct ufs_hba *hba, bool 
> on,
>  					enum ufs_notify_change_status status)
>  {

Reviewed-by: Can Guo <cang@codeaurora.org>

Re: [PATCH v1 0/3] Refine error history and introduce notify_event vop

[Asutosh Das (asd)]

On 11/25/2020 9:38 PM, Stanley Chu wrote:
> Hi,
> This series refines error history functions and introduce a new notify_event vop to allow vendor to get notified of important events.
> 
> Stanley Chu (3):
>    scsi: ufs: Add error history for abort event in UFS Device W-LUN
>    scsi: ufs: Refine error history functions
>    scsi: ufs: Introduce notify_event variant function
> 
>   drivers/scsi/ufs/ufshcd.c | 122 ++++++++++++++++++++++----------------
>   drivers/scsi/ufs/ufshcd.h |  82 ++++++++++++-------------
>   2 files changed, 112 insertions(+), 92 deletions(-)
> 

Hi Stanley,

Reviewed-by: Asutosh Das <asutoshd@codeaurora.org>

Please add to the series.


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