lkml.org 
[lkml]   [2022]   [Mar]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRE: [PATCH V4 6/7] mmc: sdhci: Set error state for mmc driver
    Date
    Hi,

    Thanks for the review.

    Please find the inline comments.

    Thanks,
    Sajida
    > -----Original Message-----
    > From: Adrian Hunter <adrian.hunter@intel.com>
    > Sent: Tuesday, March 8, 2022 3:36 PM
    > To: Sajida Bhanu (Temp) (QUIC) <quic_c_sbhanu@quicinc.com>; Asutosh
    > Das (asd) <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: Veerabhadrarao Badiganti (QUIC) <quic_vbadigan@quicinc.com>; Ram
    > Prakash Gupta (QUIC) <quic_rampraka@quicinc.com>; Pradeep Pragallapati
    > (QUIC) <quic_pragalla@quicinc.com>; Sarthak Garg (QUIC)
    > <quic_sartgarg@quicinc.com>; Nitin Rawat (QUIC)
    > <quic_nitirawa@quicinc.com>; Sayali Lokhande (QUIC)
    > <quic_sayalil@quicinc.com>; Sayali Lokhande <sayalil@codeaurora.org>; Bao
    > D . Nguyen <nguyenb@codeaurora.org>
    > Subject: Re: [PATCH V4 6/7] mmc: sdhci: Set error state for mmc driver
    >
    > On 2.3.2022 15.03, Shaik Sajida Bhanu wrote:
    > > Set error state if any errors observed in eMMC and SD card driver level.
    > >
    > > 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>
    > > Signed-off-by: Shaik Sajida Bhanu <quic_c_sbhanu@quicinc.com>
    > > ---
    > > drivers/mmc/host/sdhci.c | 22 ++++++++++++++++++++++
    > > 1 file changed, 22 insertions(+)
    > >
    > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index
    > > 741fb06..4ba3797 100644
    > > --- a/drivers/mmc/host/sdhci.c
    > > +++ b/drivers/mmc/host/sdhci.c
    > > @@ -224,6 +224,8 @@ void sdhci_reset(struct sdhci_host *host, u8 mask)
    > > if (timedout) {
    > > pr_err("%s: Reset 0x%x never completed.\n",
    > > mmc_hostname(host->mmc), (int)mask);
    > > + if (host->mmc && !host->mmc->err_state)
    > > + mmc_debugfs_err_stats_enable(host-
    > >mmc);
    >
    > I really do not understand why this cannot be another err_stats entry such as
    > MMC_ERR_DRIVER and then as below?
    >
    > sdhci_err_stats_inc(host, DRIVER);
    >
    >
    Hi,

    err_stats having all the error types.
    err_state is not the error type, if err_state set means some errors occurred in driver and we will go and check err_stats to know exact error.

    Thanks,
    Sajida
    > > sdhci_dumpregs(host);
    > > return;
    > > }
    > > @@ -1716,6 +1718,8 @@ static bool sdhci_send_command_retry(struct
    > sdhci_host *host,
    > > if (!timeout--) {
    > > pr_err("%s: Controller never released inhibit
    > bit(s).\n",
    > > mmc_hostname(host->mmc));
    > > + if (host->mmc && !host->mmc->err_state)
    > > + mmc_debugfs_err_stats_enable(host-
    > >mmc);
    > > sdhci_dumpregs(host);
    > > cmd->error = -EIO;
    > > return false;
    > > @@ -1965,6 +1969,8 @@ void sdhci_enable_clk(struct sdhci_host *host,
    > u16 clk)
    > > if (timedout) {
    > > pr_err("%s: Internal clock never stabilised.\n",
    > > mmc_hostname(host->mmc));
    > > + if (host->mmc && !host->mmc->err_state)
    > > + mmc_debugfs_err_stats_enable(host-
    > >mmc);
    > > sdhci_dumpregs(host);
    > > return;
    > > }
    > > @@ -1987,6 +1993,8 @@ void sdhci_enable_clk(struct sdhci_host *host,
    > u16 clk)
    > > if (timedout) {
    > > pr_err("%s: PLL clock never stabilised.\n",
    > > mmc_hostname(host->mmc));
    > > + if (host->mmc && !host->mmc->err_state)
    > > +
    > mmc_debugfs_err_stats_enable(host->mmc);
    > > sdhci_dumpregs(host);
    > > return;
    > > }
    > > @@ -3162,6 +3170,8 @@ static void sdhci_timeout_timer(struct timer_list
    > *t)
    > > mmc_debugfs_err_stats_inc(host->mmc,
    > MMC_ERR_REQ_TIMEOUT);
    > > pr_err("%s: Timeout waiting for hardware cmd interrupt.\n",
    > > mmc_hostname(host->mmc));
    > > + if (host->mmc && !host->mmc->err_state)
    > > + mmc_debugfs_err_stats_enable(host->mmc);
    > > sdhci_dumpregs(host);
    > >
    > > host->cmd->error = -ETIMEDOUT;
    > > @@ -3185,6 +3195,8 @@ static void sdhci_timeout_data_timer(struct
    > timer_list *t)
    > > mmc_debugfs_err_stats_inc(host->mmc,
    > MMC_ERR_REQ_TIMEOUT);
    > > pr_err("%s: Timeout waiting for hardware interrupt.\n",
    > > mmc_hostname(host->mmc));
    > > + if (host->mmc && !host->mmc->err_state)
    > > + mmc_debugfs_err_stats_enable(host->mmc);
    > > sdhci_dumpregs(host);
    > >
    > > if (host->data) {
    > > @@ -3236,6 +3248,8 @@ static void sdhci_cmd_irq(struct sdhci_host *host,
    > u32 intmask, u32 *intmask_p)
    > > return;
    > > pr_err("%s: Got command interrupt 0x%08x even though no
    > command operation was in progress.\n",
    > > mmc_hostname(host->mmc), (unsigned)intmask);
    > > + if (host->mmc && !host->mmc->err_state)
    > > + mmc_debugfs_err_stats_enable(host->mmc);
    > > sdhci_dumpregs(host);
    > > return;
    > > }
    > > @@ -3289,6 +3303,8 @@ static void sdhci_adma_show_error(struct
    > sdhci_host *host)
    > > void *desc = host->adma_table;
    > > dma_addr_t dma = host->adma_addr;
    > >
    > > + if (host->mmc && !host->mmc->err_state)
    > > + mmc_debugfs_err_stats_enable(host->mmc);
    > > sdhci_dumpregs(host);
    > >
    > > while (true) {
    > > @@ -3378,6 +3394,8 @@ static void sdhci_data_irq(struct sdhci_host
    > > *host, u32 intmask)
    > >
    > > pr_err("%s: Got data interrupt 0x%08x even though no data
    > operation was in progress.\n",
    > > mmc_hostname(host->mmc), (unsigned)intmask);
    > > + if (host->mmc && !host->mmc->err_state)
    > > + mmc_debugfs_err_stats_enable(host->mmc);
    > > sdhci_dumpregs(host);
    > >
    > > return;
    > > @@ -3599,6 +3617,8 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id)
    > > if (unexpected) {
    > > pr_err("%s: Unexpected interrupt 0x%08x.\n",
    > > mmc_hostname(host->mmc), unexpected);
    > > + if (host->mmc && !host->mmc->err_state)
    > > + mmc_debugfs_err_stats_enable(host->mmc);
    > > sdhci_dumpregs(host);
    > > }
    > >
    > > @@ -3962,6 +3982,8 @@ bool sdhci_cqe_irq(struct sdhci_host *host, u32
    > intmask, int *cmd_error,
    > > sdhci_writel(host, intmask, SDHCI_INT_STATUS);
    > > pr_err("%s: CQE: Unexpected interrupt 0x%08x.\n",
    > > mmc_hostname(host->mmc), intmask);
    > > + if (host->mmc && !host->mmc->err_state)
    > > + mmc_debugfs_err_stats_enable(host->mmc);
    > > sdhci_dumpregs(host);
    > > }
    > >

    \
     
     \ /
      Last update: 2022-03-12 19:16    [W:3.715 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site