[PATCH v1 2/3] scsi: ufs: Refine error history functions
Can Guo
cang at codeaurora.org
Thu Dec 3 00:46:42 EST 2020
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 at 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 at codeaurora.org>
More information about the Linux-mediatek
mailing list