Messages in this thread | | | Date | Thu, 03 Dec 2020 13:46:42 +0800 | From | Can Guo <> | Subject | Re: [PATCH v1 2/3] scsi: ufs: Refine error history functions |
| |
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>
| |