| Message ID | 1642699582-14785-4-git-send-email-quic_c_sbhanu@quicinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mmc: add error statistics for eMMC and SD card | expand |
On 20/01/2022 19:26, Shaik Sajida Bhanu wrote: > Add changes to capture eMMC and SD card errors. > This is useful for debug and testing. > > Signed-off-by: Shaik Sajida Bhanu <quic_c_sbhanu@quicinc.com> > Signed-off-by: Liangliang Lu <luliang@codeaurora.org> > Signed-off-by: Sayali Lokhande <sayalil@codeaurora.org> > Signed-off-by: Bao D. Nguyen <nguyenb@codeaurora.org> > Signed-off-by: Ram Prakash Gupta <quic_rampraka@quicinc.com> > --- > drivers/mmc/core/core.c | 8 ++++++++ > drivers/mmc/core/queue.c | 3 +++ > 2 files changed, 11 insertions(+) > > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c > index 368f104..c586d69 100644 > --- a/drivers/mmc/core/core.c > +++ b/drivers/mmc/core/core.c > @@ -2242,6 +2242,14 @@ void mmc_rescan(struct work_struct *work) > if (freqs[i] <= host->f_min) > break; > } > + > + /* > + * Ignore the command timeout errors observed during > + * the card init as those are excepted. > + */ > + Please remove blank line here. > + if (host && host->err_stats_enabled) The condition is not needed. > + host->err_stats[MMC_ERR_CMD_TIMEOUT] = 0; Please put this after successful call to mmc_rescan_try_freq > mmc_release_host(host); > > out: > diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c > index c69b2d9..7dc9dfb 100644 > --- a/drivers/mmc/core/queue.c > +++ b/drivers/mmc/core/queue.c > @@ -100,6 +100,9 @@ static enum blk_eh_timer_return mmc_cqe_timed_out(struct request *req) > enum mmc_issue_type issue_type = mmc_issue_type(mq, req); > bool recovery_needed = false; > > + if (host->err_stats_enabled) > + mmc_debugfs_err_stats_inc(host, MMC_ERR_CMDQ_REQ_TIMEOUT); Doesn't this get covered by the drivers. It seems like this should not be needed. > + > switch (issue_type) { > case MMC_ISSUE_ASYNC: > case MMC_ISSUE_DCMD: >
Hi, Thanks for the review. Please find the inline comments. Thanks, Sajida -----Original Message----- From: Adrian Hunter <adrian.hunter@intel.com> Sent: Friday, January 21, 2022 1:50 PM To: Sajida Bhanu (Temp) (QUIC) <quic_c_sbhanu@quicinc.com>; Asutosh Das (QUIC) <quic_asutoshd@quicinc.com>; ulf.hansson@linaro.org; agross@kernel.org; bjorn.andersson@linaro.org; linux-mmc@vger.kernel.org; linux-arm-msm@vger.kernel.org; linux-kernel@vger.kernel.org Cc: stummala@codeaurora.org; vbadigan@codeaurora.org; Ram Prakash Gupta (QUIC) <quic_rampraka@quicinc.com>; Pradeep Pragallapati (QUIC) <quic_pragalla@quicinc.com>; sartgarg@codeaurora.org; nitirawa@codeaurora.org; sayalil@codeaurora.org; Liangliang Lu <luliang@codeaurora.org>; Bao D . Nguyen <nguyenb@codeaurora.org> Subject: Re: [PATCH V3 3/4] mmc: core: Capture eMMC and SD card errors On 20/01/2022 19:26, Shaik Sajida Bhanu wrote: > Add changes to capture eMMC and SD card errors. > This is useful for debug and testing. > > Signed-off-by: Shaik Sajida Bhanu <quic_c_sbhanu@quicinc.com> > Signed-off-by: Liangliang Lu <luliang@codeaurora.org> > Signed-off-by: Sayali Lokhande <sayalil@codeaurora.org> > Signed-off-by: Bao D. Nguyen <nguyenb@codeaurora.org> > Signed-off-by: Ram Prakash Gupta <quic_rampraka@quicinc.com> > --- > drivers/mmc/core/core.c | 8 ++++++++ drivers/mmc/core/queue.c | 3 > +++ > 2 files changed, 11 insertions(+) > > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index > 368f104..c586d69 100644 > --- a/drivers/mmc/core/core.c > +++ b/drivers/mmc/core/core.c > @@ -2242,6 +2242,14 @@ void mmc_rescan(struct work_struct *work) > if (freqs[i] <= host->f_min) > break; > } > + > + /* > + * Ignore the command timeout errors observed during > + * the card init as those are excepted. > + */ > + Please remove blank line here. >>>>Sure. > + if (host && host->err_stats_enabled) The condition is not needed. >>>>Sure . > + host->err_stats[MMC_ERR_CMD_TIMEOUT] = 0; Please put this after successful call to mmc_rescan_try_freq >>>>>Sure. > mmc_release_host(host); > > out: > diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c index > c69b2d9..7dc9dfb 100644 > --- a/drivers/mmc/core/queue.c > +++ b/drivers/mmc/core/queue.c > @@ -100,6 +100,9 @@ static enum blk_eh_timer_return mmc_cqe_timed_out(struct request *req) > enum mmc_issue_type issue_type = mmc_issue_type(mq, req); > bool recovery_needed = false; > > + if (host->err_stats_enabled) > + mmc_debugfs_err_stats_inc(host, MMC_ERR_CMDQ_REQ_TIMEOUT); Doesn't this get covered by the drivers. It seems like this should not be needed. >>>>>>Okay > + > switch (issue_type) { > case MMC_ISSUE_ASYNC: > case MMC_ISSUE_DCMD: >
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 368f104..c586d69 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -2242,6 +2242,14 @@ void mmc_rescan(struct work_struct *work) if (freqs[i] <= host->f_min) break; } + + /* + * Ignore the command timeout errors observed during + * the card init as those are excepted. + */ + + if (host && host->err_stats_enabled) + host->err_stats[MMC_ERR_CMD_TIMEOUT] = 0; mmc_release_host(host); out: diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c index c69b2d9..7dc9dfb 100644 --- a/drivers/mmc/core/queue.c +++ b/drivers/mmc/core/queue.c @@ -100,6 +100,9 @@ static enum blk_eh_timer_return mmc_cqe_timed_out(struct request *req) enum mmc_issue_type issue_type = mmc_issue_type(mq, req); bool recovery_needed = false; + if (host->err_stats_enabled) + mmc_debugfs_err_stats_inc(host, MMC_ERR_CMDQ_REQ_TIMEOUT); + switch (issue_type) { case MMC_ISSUE_ASYNC: case MMC_ISSUE_DCMD: