[PATCH V4 0/7] mmc: add error statistics for eMMC and SD card

[PATCH V4 0/7] mmc: add error statistics for eMMC and SD card

[Shaik Sajida Bhanu]

Changes since V3:
	-Dropped error stats feature flag as suggested by Adrain Hunter.
	-Separated error state related changes in separate patches as
	 suggested by Adrain Hunter.
	  [PATCH V4 4/7] : error state debug fs
	  [PATCH V4 5/7] : error state enable function
	  [PATCH V4 6/7] : error state enable in error case
	 Note: we are enabling error state before calling sdhci_dumpregs
	 we couldn't add the err state in error stats array as err state
	 is not error type.
	-Corrected Signed-off-by order as suggested by Bjron Andersson.
	-Moved error state enable code from sdhci_dumpregs to error
	 conditions as suggested by Adrain Hunter.

Changes since V2:
	-Removed userspace error stats clear debug fs entry as suggested
	 by Adrain Hunter.
	-Split patch into 4 patches
	  [PATCH V3 1/4] : sdhci driver
	  [PATCH V3 2/4] : debug fs entries
	  [PATCH V3 3/4] : core driver
	  [PATCH V3 4/4] : cqhci driver
	-Used for loop to print error messages instead of using printf
	 statements for all error messages as suggested by Adrain Hunter.
	-Introduced one flag to enable error stats feature, if any other
	 client wants to use this feature, they need to enable that flag.
	-Moved reset command timeout error statement to card init flow
	 as suggested by Adrain Hunter.

Changes since V1:
	-Removed sysfs entry for eMMC and SD card error statistics and added

Shaik Sajida Bhanu (7):
  mmc: core: Capture eMMC and SD card errors
  mmc: sdhci: Capture eMMC and SD card errors
  mmc: debugfs: Add debug fs entry for mmc driver
  mmc: debugfs: Add debug fs error state entry for mmc driver
  mmc: core: Set error state for mmc driver
  mmc: sdhci: Set error state for mmc driver
  mmc: cqhci: Capture eMMC and SD card errors

 drivers/mmc/core/core.c       |  6 ++++
 drivers/mmc/core/debugfs.c    | 75 +++++++++++++++++++++++++++++++++++++++++++
 drivers/mmc/host/cqhci-core.c |  9 +++++-
 drivers/mmc/host/sdhci.c      | 74 +++++++++++++++++++++++++++++++++++-------
 include/linux/mmc/host.h      | 29 +++++++++++++++++
 5 files changed, 180 insertions(+), 13 deletions(-)

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member 
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH V4 1/7] mmc: core: Capture eMMC and SD card errors

[Shaik Sajida Bhanu]

Add changes to capture eMMC and SD card errors.
This is useful for debug and testing.

Signed-off-by: Liangliang Lu <quic_luliang@quicinc.com>
Signed-off-by: Sayali Lokhande <quic_sayalil@quicinc.com>
Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
Signed-off-by: Ram Prakash Gupta <quic_rampraka@quicinc.com>
Signed-off-by: Shaik Sajida Bhanu <quic_c_sbhanu@quicinc.com>
---
 drivers/mmc/core/core.c  |  6 ++++++
 include/linux/mmc/host.h | 23 +++++++++++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 368f104..f3679ed 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2242,6 +2242,12 @@ 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.
+	 */
+	host->err_stats[MMC_ERR_CMD_TIMEOUT] = 0;
 	mmc_release_host(host);
 
  out:
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 7afb57c..3b7f1e5 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -93,6 +93,23 @@ struct mmc_clk_phase_map {
 
 struct mmc_host;
 
+enum mmc_err_stat {
+	MMC_ERR_CMD_TIMEOUT,
+	MMC_ERR_CMD_CRC,
+	MMC_ERR_DAT_TIMEOUT,
+	MMC_ERR_DAT_CRC,
+	MMC_ERR_AUTO_CMD,
+	MMC_ERR_ADMA,
+	MMC_ERR_TUNING,
+	MMC_ERR_CMDQ_RED,
+	MMC_ERR_CMDQ_GCE,
+	MMC_ERR_CMDQ_ICCE,
+	MMC_ERR_REQ_TIMEOUT,
+	MMC_ERR_CMDQ_REQ_TIMEOUT,
+	MMC_ERR_ICE_CFG,
+	MMC_ERR_MAX,
+};
+
 struct mmc_host_ops {
 	/*
 	 * It is optional for the host to implement pre_req and post_req in
@@ -500,6 +517,7 @@ struct mmc_host {
 
 	/* Host Software Queue support */
 	bool			hsq_enabled;
+	u32                     err_stats[MMC_ERR_MAX];
 
 	unsigned long		private[] ____cacheline_aligned;
 };
@@ -635,6 +653,11 @@ static inline enum dma_data_direction mmc_get_dma_dir(struct mmc_data *data)
 	return data->flags & MMC_DATA_WRITE ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
 }
 
+static inline void mmc_debugfs_err_stats_inc(struct mmc_host *host,
+		enum mmc_err_stat stat) {
+	host->err_stats[stat] += 1;
+}
+
 int mmc_send_tuning(struct mmc_host *host, u32 opcode, int *cmd_error);
 int mmc_send_abort_tuning(struct mmc_host *host, u32 opcode);
 int mmc_get_ext_csd(struct mmc_card *card, u8 **new_ext_csd);
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member 
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH V4 2/7] mmc: sdhci: Capture eMMC and SD card errors

[Shaik Sajida Bhanu]

Add changes to capture eMMC and SD card errors.
This is useful for debug and testing.

Signed-off-by: Liangliang Lu <quic_luliang@quicinc.com>
Signed-off-by: Sayali Lokhande <quic_sayalil@quicinc.com>
Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
Signed-off-by: Shaik Sajida Bhanu <quic_c_sbhanu@quicinc.com>
---
 drivers/mmc/host/sdhci.c | 52 +++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 40 insertions(+), 12 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 07c6da1..741fb06 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -3159,6 +3159,7 @@ static void sdhci_timeout_timer(struct timer_list *t)
 	spin_lock_irqsave(&host->lock, flags);
 
 	if (host->cmd && !sdhci_data_line_cmd(host->cmd)) {
+		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));
 		sdhci_dumpregs(host);
@@ -3181,6 +3182,7 @@ static void sdhci_timeout_data_timer(struct timer_list *t)
 
 	if (host->data || host->data_cmd ||
 	    (host->cmd && sdhci_data_line_cmd(host->cmd))) {
+		mmc_debugfs_err_stats_inc(host->mmc, MMC_ERR_REQ_TIMEOUT);
 		pr_err("%s: Timeout waiting for hardware interrupt.\n",
 		       mmc_hostname(host->mmc));
 		sdhci_dumpregs(host);
@@ -3240,11 +3242,15 @@ static void sdhci_cmd_irq(struct sdhci_host *host, u32 intmask, u32 *intmask_p)
 
 	if (intmask & (SDHCI_INT_TIMEOUT | SDHCI_INT_CRC |
 		       SDHCI_INT_END_BIT | SDHCI_INT_INDEX)) {
-		if (intmask & SDHCI_INT_TIMEOUT)
+		if (intmask & SDHCI_INT_TIMEOUT) {
 			host->cmd->error = -ETIMEDOUT;
-		else
+			mmc_debugfs_err_stats_inc(host->mmc, MMC_ERR_CMD_TIMEOUT);
+		} else {
 			host->cmd->error = -EILSEQ;
-
+			if (host->cmd->opcode != MMC_SEND_TUNING_BLOCK ||
+					host->cmd->opcode != MMC_SEND_TUNING_BLOCK_HS200)
+				mmc_debugfs_err_stats_inc(host->mmc, MMC_ERR_CMD_CRC);
+		}
 		/* Treat data command CRC error the same as data CRC error */
 		if (host->cmd->data &&
 		    (intmask & (SDHCI_INT_CRC | SDHCI_INT_TIMEOUT)) ==
@@ -3265,6 +3271,7 @@ static void sdhci_cmd_irq(struct sdhci_host *host, u32 intmask, u32 *intmask_p)
 		int err = (auto_cmd_status & SDHCI_AUTO_CMD_TIMEOUT) ?
 			  -ETIMEDOUT :
 			  -EILSEQ;
+		mmc_debugfs_err_stats_inc(host->mmc, MMC_ERR_AUTO_CMD);
 
 		if (sdhci_auto_cmd23(host, mrq)) {
 			mrq->sbc->error = err;
@@ -3342,6 +3349,7 @@ static void sdhci_data_irq(struct sdhci_host *host, u32 intmask)
 			if (intmask & SDHCI_INT_DATA_TIMEOUT) {
 				host->data_cmd = NULL;
 				data_cmd->error = -ETIMEDOUT;
+				mmc_debugfs_err_stats_inc(host->mmc, MMC_ERR_CMD_TIMEOUT);
 				__sdhci_finish_mrq(host, data_cmd->mrq);
 				return;
 			}
@@ -3375,18 +3383,25 @@ static void sdhci_data_irq(struct sdhci_host *host, u32 intmask)
 		return;
 	}
 
-	if (intmask & SDHCI_INT_DATA_TIMEOUT)
+	if (intmask & SDHCI_INT_DATA_TIMEOUT) {
 		host->data->error = -ETIMEDOUT;
+		mmc_debugfs_err_stats_inc(host->mmc, MMC_ERR_DAT_TIMEOUT);
+	}
 	else if (intmask & SDHCI_INT_DATA_END_BIT)
 		host->data->error = -EILSEQ;
 	else if ((intmask & SDHCI_INT_DATA_CRC) &&
 		SDHCI_GET_CMD(sdhci_readw(host, SDHCI_COMMAND))
-			!= MMC_BUS_TEST_R)
+			!= MMC_BUS_TEST_R) {
 		host->data->error = -EILSEQ;
+		if (host->cmd->opcode != MMC_SEND_TUNING_BLOCK ||
+				host->cmd->opcode != MMC_SEND_TUNING_BLOCK_HS200)
+			mmc_debugfs_err_stats_inc(host->mmc, MMC_ERR_DAT_CRC);
+	}
 	else if (intmask & SDHCI_INT_ADMA_ERROR) {
 		pr_err("%s: ADMA error: 0x%08x\n", mmc_hostname(host->mmc),
 		       intmask);
 		sdhci_adma_show_error(host);
+		mmc_debugfs_err_stats_inc(host->mmc, MMC_ERR_ADMA);
 		host->data->error = -EIO;
 		if (host->ops->adma_workaround)
 			host->ops->adma_workaround(host, intmask);
@@ -3905,20 +3920,33 @@ bool sdhci_cqe_irq(struct sdhci_host *host, u32 intmask, int *cmd_error,
 	if (!host->cqe_on)
 		return false;
 
-	if (intmask & (SDHCI_INT_INDEX | SDHCI_INT_END_BIT | SDHCI_INT_CRC))
+	if (intmask & (SDHCI_INT_INDEX | SDHCI_INT_END_BIT | SDHCI_INT_CRC)) {
 		*cmd_error = -EILSEQ;
-	else if (intmask & SDHCI_INT_TIMEOUT)
+		if (intmask & SDHCI_INT_CRC) {
+			if (host->cmd->opcode != MMC_SEND_TUNING_BLOCK ||
+					host->cmd->opcode != MMC_SEND_TUNING_BLOCK_HS200)
+				mmc_debugfs_err_stats_inc(host->mmc, MMC_ERR_CMD_CRC);
+		}
+	} else if (intmask & SDHCI_INT_TIMEOUT) {
 		*cmd_error = -ETIMEDOUT;
-	else
+		mmc_debugfs_err_stats_inc(host->mmc, MMC_ERR_CMD_TIMEOUT);
+	} else
 		*cmd_error = 0;
 
-	if (intmask & (SDHCI_INT_DATA_END_BIT | SDHCI_INT_DATA_CRC))
+	if (intmask & (SDHCI_INT_DATA_END_BIT | SDHCI_INT_DATA_CRC)) {
 		*data_error = -EILSEQ;
-	else if (intmask & SDHCI_INT_DATA_TIMEOUT)
+		if (intmask & SDHCI_INT_DATA_CRC) {
+			if (host->cmd->opcode != MMC_SEND_TUNING_BLOCK ||
+					host->cmd->opcode != MMC_SEND_TUNING_BLOCK_HS200)
+				mmc_debugfs_err_stats_inc(host->mmc, MMC_ERR_DAT_CRC);
+		}
+	} else if (intmask & SDHCI_INT_DATA_TIMEOUT) {
 		*data_error = -ETIMEDOUT;
-	else if (intmask & SDHCI_INT_ADMA_ERROR)
+		mmc_debugfs_err_stats_inc(host->mmc, MMC_ERR_DAT_TIMEOUT);
+	} else if (intmask & SDHCI_INT_ADMA_ERROR) {
 		*data_error = -EIO;
-	else
+		mmc_debugfs_err_stats_inc(host->mmc, MMC_ERR_ADMA);
+	} else
 		*data_error = 0;
 
 	/* Clear selected interrupts. */
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member 
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH V4 3/7] mmc: debugfs: Add debug fs entry for mmc driver

[Shaik Sajida Bhanu]

Add debug fs entry to query eMMC and SD card errors statistics

Signed-off-by: Liangliang Lu <quic_luliang@quicinc.com>
Signed-off-by: Sayali Lokhande <quic_sayalil@quicinc.com>
Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
Signed-off-by: Shaik Sajida Bhanu <quic_c_sbhanu@quicinc.com>
---
 drivers/mmc/core/debugfs.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c
index 3fdbc80..db0988c 100644
--- a/drivers/mmc/core/debugfs.c
+++ b/drivers/mmc/core/debugfs.c
@@ -223,6 +223,63 @@ static int mmc_clock_opt_set(void *data, u64 val)
 DEFINE_DEBUGFS_ATTRIBUTE(mmc_clock_fops, mmc_clock_opt_get, mmc_clock_opt_set,
 	"%llu\n");
 
+static int mmc_err_stats_show(struct seq_file *file, void *data)
+{
+	struct mmc_host *host = (struct mmc_host *)file->private;
+	const char *desc[MMC_ERR_MAX] = {
+		[MMC_ERR_CMD_TIMEOUT] = "Command Timeout Occurred",
+		[MMC_ERR_CMD_CRC] = "Command CRC Errors Occurred",
+		[MMC_ERR_DAT_TIMEOUT] = "Data Timeout Occurred",
+		[MMC_ERR_DAT_CRC] = "Data CRC Errors Occurred",
+		[MMC_ERR_AUTO_CMD] = "Auto-Cmd Error Occurred",
+		[MMC_ERR_ADMA] = "ADMA Error Occurred",
+		[MMC_ERR_TUNING] = "Tuning Error Occurred",
+		[MMC_ERR_CMDQ_RED] = "CMDQ RED Errors",
+		[MMC_ERR_CMDQ_GCE] = "CMDQ GCE Errors",
+		[MMC_ERR_CMDQ_ICCE] = "CMDQ ICCE Errors",
+		[MMC_ERR_REQ_TIMEOUT] = "Request Timedout",
+		[MMC_ERR_CMDQ_REQ_TIMEOUT] = "CMDQ Request Timedout",
+		[MMC_ERR_ICE_CFG] = "ICE Config Errors",
+	};
+	int i;
+
+	if (!host)
+		return -EINVAL;
+
+	for (i = 0; i < MMC_ERR_MAX; i++) {
+		if (desc[i])
+			seq_printf(file, "# %s:\t %d\n",
+					desc[i], host->err_stats[i]);
+	}
+
+	return 0;
+}
+
+static int mmc_err_stats_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, mmc_err_stats_show, inode->i_private);
+}
+
+static ssize_t mmc_err_stats_write(struct file *filp, const char __user *ubuf,
+				   size_t cnt, loff_t *ppos)
+{
+	struct mmc_host *host = filp->f_mapping->host->i_private;
+
+	if (!host)
+		return -EINVAL;
+
+	pr_debug("%s: Resetting MMC error statistics\n", __func__);
+	memset(host->err_stats, 0, sizeof(host->err_stats));
+
+	return cnt;
+}
+
+static const struct file_operations mmc_err_stats_fops = {
+	.open	= mmc_err_stats_open,
+	.read	= seq_read,
+	.write	= mmc_err_stats_write,
+};
+
 void mmc_add_host_debugfs(struct mmc_host *host)
 {
 	struct dentry *root;
@@ -236,6 +293,9 @@ void mmc_add_host_debugfs(struct mmc_host *host)
 	debugfs_create_file_unsafe("clock", S_IRUSR | S_IWUSR, root, host,
 				   &mmc_clock_fops);
 
+	debugfs_create_file("err_stats", 0600, root, host,
+		&mmc_err_stats_fops);
+
 #ifdef CONFIG_FAIL_MMC_REQUEST
 	if (fail_request)
 		setup_fault_attr(&fail_default_attr, fail_request);
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member 
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH V4 4/7] mmc: debugfs: Add debug fs error state entry for mmc driver

[Shaik Sajida Bhanu]

Add debug fs entry error state to query eMMC and SD card errors statistics.
If any errors occurred in eMMC and SD card driver level then
err_state value will be set to 1.

Signed-off-by: Liangliang Lu <quic_luliang@quicinc.com>
Signed-off-by: Sayali Lokhande <quic_sayalil@quicinc.com>
Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
Signed-off-by: Shaik Sajida Bhanu <quic_c_sbhanu@quicinc.com>
---
 drivers/mmc/core/debugfs.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c
index db0988c..7d78eac 100644
--- a/drivers/mmc/core/debugfs.c
+++ b/drivers/mmc/core/debugfs.c
@@ -222,6 +222,19 @@ static int mmc_clock_opt_set(void *data, u64 val)
 
 DEFINE_DEBUGFS_ATTRIBUTE(mmc_clock_fops, mmc_clock_opt_get, mmc_clock_opt_set,
 	"%llu\n");
+static int mmc_err_state_get(void *data, u64 *val)
+{
+	struct mmc_host *host = data;
+
+	if (!host)
+		return -EINVAL;
+
+	*val = host->err_state ? 1 : 0;
+
+	return 0;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(mmc_err_state, mmc_err_state_get, NULL, "%llu\n");
 
 static int mmc_err_stats_show(struct seq_file *file, void *data)
 {
@@ -293,6 +306,8 @@ void mmc_add_host_debugfs(struct mmc_host *host)
 	debugfs_create_file_unsafe("clock", S_IRUSR | S_IWUSR, root, host,
 				   &mmc_clock_fops);
 
+	debugfs_create_file("err_state", 0600, root, host,
+		&mmc_err_state);
 	debugfs_create_file("err_stats", 0600, root, host,
 		&mmc_err_stats_fops);
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member 
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH V4 5/7] mmc: core: Set error state for mmc driver

[Shaik Sajida Bhanu]

If any errors observed in eMMC and SD card set error state.

User can read error state value and confirm any errors observed or not,
error state set means error obeserved and vice versa.

Signed-off-by: Liangliang Lu <quic_luliang@quicinc.com>
Signed-off-by: Sayali Lokhande <quic_sayalil@quicinc.com>
Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
Signed-off-by: Ram Prakash Gupta <quic_rampraka@quicinc.com>
Signed-off-by: Shaik Sajida Bhanu <quic_c_sbhanu@quicinc.com>
---
 include/linux/mmc/host.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 3b7f1e5..28baa07 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -518,6 +518,7 @@ struct mmc_host {
 	/* Host Software Queue support */
 	bool			hsq_enabled;
 	u32                     err_stats[MMC_ERR_MAX];
+	bool			err_state;
 
 	unsigned long		private[] ____cacheline_aligned;
 };
@@ -653,6 +654,11 @@ static inline enum dma_data_direction mmc_get_dma_dir(struct mmc_data *data)
 	return data->flags & MMC_DATA_WRITE ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
 }
 
+static inline void mmc_debugfs_err_stats_enable(struct mmc_host *host)
+{
+	host->err_state = true;
+}
+
 static inline void mmc_debugfs_err_stats_inc(struct mmc_host *host,
 		enum mmc_err_stat stat) {
 	host->err_stats[stat] += 1;
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member 
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH V4 6/7] mmc: sdhci: Set error state for mmc driver

[Shaik Sajida Bhanu]

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);
 			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);
 	}
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member 
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH V4 7/7] mmc: cqhci: Capture eMMC and SD card errors

[Shaik Sajida Bhanu]

Add changes to capture eMMC and SD card errors.
This is useful for debug and testing.

Signed-off-by: Liangliang Lu <quic_luliang@quicinc.com>
Signed-off-by: Sayali Lokhande <quic_sayalil@quicinc.com>
Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
Signed-off-by: Ram Prakash Gupta <quic_rampraka@quicinc.com>
Signed-off-by: Shaik Sajida Bhanu <quic_c_sbhanu@quicinc.com>
---
 drivers/mmc/host/cqhci-core.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/cqhci-core.c b/drivers/mmc/host/cqhci-core.c
index b0d30c3..b3d7d6d 100644
--- a/drivers/mmc/host/cqhci-core.c
+++ b/drivers/mmc/host/cqhci-core.c
@@ -822,8 +822,15 @@ irqreturn_t cqhci_irq(struct mmc_host *mmc, u32 intmask, int cmd_error,
 	pr_debug("%s: cqhci: IRQ status: 0x%08x\n", mmc_hostname(mmc), status);
 
 	if ((status & (CQHCI_IS_RED | CQHCI_IS_GCE | CQHCI_IS_ICCE)) ||
-	    cmd_error || data_error)
+	    cmd_error || data_error) {
+		if (status & CQHCI_IS_RED)
+			mmc_debugfs_err_stats_inc(mmc, MMC_ERR_CMDQ_RED);
+		if (status & CQHCI_IS_GCE)
+			mmc_debugfs_err_stats_inc(mmc, MMC_ERR_CMDQ_GCE);
+		if (status & CQHCI_IS_ICCE)
+			mmc_debugfs_err_stats_inc(mmc, MMC_ERR_CMDQ_ICCE);
 		cqhci_error_irq(mmc, status, cmd_error, data_error);
+	}
 
 	if (status & CQHCI_IS_TCC) {
 		/* read TCN and complete the request */
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member 
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH V4 2/7] mmc: sdhci: Capture eMMC and SD card errors

[Adrian Hunter]

On 2.3.2022 15.03, Shaik Sajida Bhanu wrote:
> Add changes to capture eMMC and SD card errors.
> This is useful for debug and testing.
> 
> Signed-off-by: Liangliang Lu <quic_luliang@quicinc.com>
> Signed-off-by: Sayali Lokhande <quic_sayalil@quicinc.com>
> Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
> Signed-off-by: Shaik Sajida Bhanu <quic_c_sbhanu@quicinc.com>
> ---
>  drivers/mmc/host/sdhci.c | 52 +++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 40 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 07c6da1..741fb06 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -3159,6 +3159,7 @@ static void sdhci_timeout_timer(struct timer_list *t)
>  	spin_lock_irqsave(&host->lock, flags);
>  
>  	if (host->cmd && !sdhci_data_line_cmd(host->cmd)) {
> +		mmc_debugfs_err_stats_inc(host->mmc, MMC_ERR_REQ_TIMEOUT);

Let's make a macro for this:

#define sdhci_err_stats_inc(host, err_name) \
	mmc_debugfs_err_stats_inc(host->mmc, MMC_ERR_ ## err_name)

>  		pr_err("%s: Timeout waiting for hardware cmd interrupt.\n",
>  		       mmc_hostname(host->mmc));

Please move the err_stats_inc here after the error message i.e.

		sdhci_err_stats_inc(host, REQ_TIMEOUT);

>  		sdhci_dumpregs(host);
> @@ -3181,6 +3182,7 @@ static void sdhci_timeout_data_timer(struct timer_list *t)
>  
>  	if (host->data || host->data_cmd ||
>  	    (host->cmd && sdhci_data_line_cmd(host->cmd))) {
> +		mmc_debugfs_err_stats_inc(host->mmc, MMC_ERR_REQ_TIMEOUT);
>  		pr_err("%s: Timeout waiting for hardware interrupt.\n",
>  		       mmc_hostname(host->mmc));

Please move the err_stats_inc here after the error message i.e.

		sdhci_err_stats_inc(host, REQ_TIMEOUT);

>  		sdhci_dumpregs(host);
> @@ -3240,11 +3242,15 @@ static void sdhci_cmd_irq(struct sdhci_host *host, u32 intmask, u32 *intmask_p)
>  
>  	if (intmask & (SDHCI_INT_TIMEOUT | SDHCI_INT_CRC |
>  		       SDHCI_INT_END_BIT | SDHCI_INT_INDEX)) {
> -		if (intmask & SDHCI_INT_TIMEOUT)
> +		if (intmask & SDHCI_INT_TIMEOUT) {
>  			host->cmd->error = -ETIMEDOUT;
> -		else
> +			mmc_debugfs_err_stats_inc(host->mmc, MMC_ERR_CMD_TIMEOUT);
> +		} else {
>  			host->cmd->error = -EILSEQ;
> -
> +			if (host->cmd->opcode != MMC_SEND_TUNING_BLOCK ||
> +					host->cmd->opcode != MMC_SEND_TUNING_BLOCK_HS200)

That should be && not ||

Please add a helper as below, then this can be:

			if (!mmc_op_tuning(host->cmd->opcode))
				sdhci_err_stats_inc(host, CMD_CRC);


diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
index d9a65c6a8816..d809bdfcc59b 100644
--- a/include/linux/mmc/mmc.h
+++ b/include/linux/mmc/mmc.h
@@ -99,6 +99,12 @@ static inline bool mmc_op_multi(u32 opcode)
 	       opcode == MMC_READ_MULTIPLE_BLOCK;
 }
 
+static inline bool mmc_op_tuning(u32 opcode)
+{
+	return opcode == MMC_SEND_TUNING_BLOCK ||
+	       opcode == MMC_SEND_TUNING_BLOCK_HS200;
+}
+
 /*
  * MMC_SWITCH argument format:
  *



> +				mmc_debugfs_err_stats_inc(host->mmc, MMC_ERR_CMD_CRC);
> +		}
>  		/* Treat data command CRC error the same as data CRC error */
>  		if (host->cmd->data &&
>  		    (intmask & (SDHCI_INT_CRC | SDHCI_INT_TIMEOUT)) ==
> @@ -3265,6 +3271,7 @@ static void sdhci_cmd_irq(struct sdhci_host *host, u32 intmask, u32 *intmask_p)
>  		int err = (auto_cmd_status & SDHCI_AUTO_CMD_TIMEOUT) ?
>  			  -ETIMEDOUT :
>  			  -EILSEQ;

Should be a blank line here after the local variable declarations.

> +		mmc_debugfs_err_stats_inc(host->mmc, MMC_ERR_AUTO_CMD);
>  
>  		if (sdhci_auto_cmd23(host, mrq)) {
>  			mrq->sbc->error = err;
> @@ -3342,6 +3349,7 @@ static void sdhci_data_irq(struct sdhci_host *host, u32 intmask)
>  			if (intmask & SDHCI_INT_DATA_TIMEOUT) {
>  				host->data_cmd = NULL;
>  				data_cmd->error = -ETIMEDOUT;
> +				mmc_debugfs_err_stats_inc(host->mmc, MMC_ERR_CMD_TIMEOUT);
>  				__sdhci_finish_mrq(host, data_cmd->mrq);
>  				return;
>  			}
> @@ -3375,18 +3383,25 @@ static void sdhci_data_irq(struct sdhci_host *host, u32 intmask)
>  		return;
>  	}
>  
> -	if (intmask & SDHCI_INT_DATA_TIMEOUT)
> +	if (intmask & SDHCI_INT_DATA_TIMEOUT) {
>  		host->data->error = -ETIMEDOUT;
> +		mmc_debugfs_err_stats_inc(host->mmc, MMC_ERR_DAT_TIMEOUT);
> +	}

The parenthesis should be with the 'else'.  Please use checkpatch with the --strict option
and fix the issues.

>  	else if (intmask & SDHCI_INT_DATA_END_BIT)
>  		host->data->error = -EILSEQ;
>  	else if ((intmask & SDHCI_INT_DATA_CRC) &&
>  		SDHCI_GET_CMD(sdhci_readw(host, SDHCI_COMMAND))
> -			!= MMC_BUS_TEST_R)
> +			!= MMC_BUS_TEST_R) {
>  		host->data->error = -EILSEQ;
> +		if (host->cmd->opcode != MMC_SEND_TUNING_BLOCK ||
> +				host->cmd->opcode != MMC_SEND_TUNING_BLOCK_HS200)
> +			mmc_debugfs_err_stats_inc(host->mmc, MMC_ERR_DAT_CRC);

As previously:

		if (!mmc_op_tuning(host->cmd->opcode))
			sdhci_err_stats_inc(host, DAT_CRC);

> +	}
>  	else if (intmask & SDHCI_INT_ADMA_ERROR) {
>  		pr_err("%s: ADMA error: 0x%08x\n", mmc_hostname(host->mmc),
>  		       intmask);
>  		sdhci_adma_show_error(host);
> +		mmc_debugfs_err_stats_inc(host->mmc, MMC_ERR_ADMA);
>  		host->data->error = -EIO;
>  		if (host->ops->adma_workaround)
>  			host->ops->adma_workaround(host, intmask);
> @@ -3905,20 +3920,33 @@ bool sdhci_cqe_irq(struct sdhci_host *host, u32 intmask, int *cmd_error,
>  	if (!host->cqe_on)
>  		return false;
>  
> -	if (intmask & (SDHCI_INT_INDEX | SDHCI_INT_END_BIT | SDHCI_INT_CRC))
> +	if (intmask & (SDHCI_INT_INDEX | SDHCI_INT_END_BIT | SDHCI_INT_CRC)) {
>  		*cmd_error = -EILSEQ;
> -	else if (intmask & SDHCI_INT_TIMEOUT)
> +		if (intmask & SDHCI_INT_CRC) {
> +			if (host->cmd->opcode != MMC_SEND_TUNING_BLOCK ||
> +					host->cmd->opcode != MMC_SEND_TUNING_BLOCK_HS200)

Please count INDEX and END_BIT errors same as CRC errors i.e. just

		if (!mmc_op_tuning(host->cmd->opcode))
			sdhci_err_stats_inc(host, CMD_CRC);

> +				mmc_debugfs_err_stats_inc(host->mmc, MMC_ERR_CMD_CRC);
> +		}
> +	} else if (intmask & SDHCI_INT_TIMEOUT) {
>  		*cmd_error = -ETIMEDOUT;
> -	else
> +		mmc_debugfs_err_stats_inc(host->mmc, MMC_ERR_CMD_TIMEOUT);
> +	} else
>  		*cmd_error = 0;
>  
> -	if (intmask & (SDHCI_INT_DATA_END_BIT | SDHCI_INT_DATA_CRC))
> +	if (intmask & (SDHCI_INT_DATA_END_BIT | SDHCI_INT_DATA_CRC)) {
>  		*data_error = -EILSEQ;
> -	else if (intmask & SDHCI_INT_DATA_TIMEOUT)
> +		if (intmask & SDHCI_INT_DATA_CRC) {
> +			if (host->cmd->opcode != MMC_SEND_TUNING_BLOCK ||
> +					host->cmd->opcode != MMC_SEND_TUNING_BLOCK_HS200)

Please count END_BIT error same as CRC error i.e. just

		if (!mmc_op_tuning(host->cmd->opcode))
			sdhci_err_stats_inc(host, DAT_CRC);

> +				mmc_debugfs_err_stats_inc(host->mmc, MMC_ERR_DAT_CRC);
> +		}
> +	} else if (intmask & SDHCI_INT_DATA_TIMEOUT) {
>  		*data_error = -ETIMEDOUT;
> -	else if (intmask & SDHCI_INT_ADMA_ERROR)
> +		mmc_debugfs_err_stats_inc(host->mmc, MMC_ERR_DAT_TIMEOUT);
> +	} else if (intmask & SDHCI_INT_ADMA_ERROR) {
>  		*data_error = -EIO;
> -	else
> +		mmc_debugfs_err_stats_inc(host->mmc, MMC_ERR_ADMA);
> +	} else
>  		*data_error = 0;
>  
>  	/* Clear selected interrupts. */

Re: [PATCH V4 1/7] mmc: core: Capture eMMC and SD card errors

[Adrian Hunter]

On 2.3.2022 15.03, Shaik Sajida Bhanu wrote:
> Add changes to capture eMMC and SD card errors.
> This is useful for debug and testing.
> 
> Signed-off-by: Liangliang Lu <quic_luliang@quicinc.com>
> Signed-off-by: Sayali Lokhande <quic_sayalil@quicinc.com>
> Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
> Signed-off-by: Ram Prakash Gupta <quic_rampraka@quicinc.com>
> Signed-off-by: Shaik Sajida Bhanu <quic_c_sbhanu@quicinc.com>
> ---
>  drivers/mmc/core/core.c  |  6 ++++++
>  include/linux/mmc/host.h | 23 +++++++++++++++++++++++
>  2 files changed, 29 insertions(+)
> 
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 368f104..f3679ed 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -2242,6 +2242,12 @@ 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.
> +	 */
> +	host->err_stats[MMC_ERR_CMD_TIMEOUT] = 0;
>  	mmc_release_host(host);
>  
>   out:
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 7afb57c..3b7f1e5 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -93,6 +93,23 @@ struct mmc_clk_phase_map {
>  
>  struct mmc_host;
>  
> +enum mmc_err_stat {
> +	MMC_ERR_CMD_TIMEOUT,
> +	MMC_ERR_CMD_CRC,
> +	MMC_ERR_DAT_TIMEOUT,
> +	MMC_ERR_DAT_CRC,
> +	MMC_ERR_AUTO_CMD,
> +	MMC_ERR_ADMA,
> +	MMC_ERR_TUNING,
> +	MMC_ERR_CMDQ_RED,
> +	MMC_ERR_CMDQ_GCE,
> +	MMC_ERR_CMDQ_ICCE,
> +	MMC_ERR_REQ_TIMEOUT,
> +	MMC_ERR_CMDQ_REQ_TIMEOUT,
> +	MMC_ERR_ICE_CFG,
> +	MMC_ERR_MAX,
> +};
> +
>  struct mmc_host_ops {
>  	/*
>  	 * It is optional for the host to implement pre_req and post_req in
> @@ -500,6 +517,7 @@ struct mmc_host {
>  
>  	/* Host Software Queue support */
>  	bool			hsq_enabled;
> +	u32                     err_stats[MMC_ERR_MAX];

This makes it look like err_stats has something to do with Host Software Queue.
Perhaps move it to be with debugfs_root. Also use tabs not spaces

	struct dentry		*debugfs_root;
	u32			err_stats[MMC_ERR_MAX];

>  
>  	unsigned long		private[] ____cacheline_aligned;
>  };
> @@ -635,6 +653,11 @@ static inline enum dma_data_direction mmc_get_dma_dir(struct mmc_data *data)
>  	return data->flags & MMC_DATA_WRITE ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
>  }
>  
> +static inline void mmc_debugfs_err_stats_inc(struct mmc_host *host,
> +		enum mmc_err_stat stat) {
> +	host->err_stats[stat] += 1;
> +}
> +
>  int mmc_send_tuning(struct mmc_host *host, u32 opcode, int *cmd_error);
>  int mmc_send_abort_tuning(struct mmc_host *host, u32 opcode);
>  int mmc_get_ext_csd(struct mmc_card *card, u8 **new_ext_csd);

Re: [PATCH V4 3/7] mmc: debugfs: Add debug fs entry for mmc driver

[Adrian Hunter]

On 2.3.2022 15.03, Shaik Sajida Bhanu wrote:
> Add debug fs entry to query eMMC and SD card errors statistics
> 
> Signed-off-by: Liangliang Lu <quic_luliang@quicinc.com>
> Signed-off-by: Sayali Lokhande <quic_sayalil@quicinc.com>
> Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
> Signed-off-by: Shaik Sajida Bhanu <quic_c_sbhanu@quicinc.com>
> ---
>  drivers/mmc/core/debugfs.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 60 insertions(+)
> 
> diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c
> index 3fdbc80..db0988c 100644
> --- a/drivers/mmc/core/debugfs.c
> +++ b/drivers/mmc/core/debugfs.c
> @@ -223,6 +223,63 @@ static int mmc_clock_opt_set(void *data, u64 val)
>  DEFINE_DEBUGFS_ATTRIBUTE(mmc_clock_fops, mmc_clock_opt_get, mmc_clock_opt_set,
>  	"%llu\n");
>  
> +static int mmc_err_stats_show(struct seq_file *file, void *data)
> +{
> +	struct mmc_host *host = (struct mmc_host *)file->private;
> +	const char *desc[MMC_ERR_MAX] = {
> +		[MMC_ERR_CMD_TIMEOUT] = "Command Timeout Occurred",
> +		[MMC_ERR_CMD_CRC] = "Command CRC Errors Occurred",
> +		[MMC_ERR_DAT_TIMEOUT] = "Data Timeout Occurred",
> +		[MMC_ERR_DAT_CRC] = "Data CRC Errors Occurred",
> +		[MMC_ERR_AUTO_CMD] = "Auto-Cmd Error Occurred",
> +		[MMC_ERR_ADMA] = "ADMA Error Occurred",
> +		[MMC_ERR_TUNING] = "Tuning Error Occurred",
> +		[MMC_ERR_CMDQ_RED] = "CMDQ RED Errors",
> +		[MMC_ERR_CMDQ_GCE] = "CMDQ GCE Errors",
> +		[MMC_ERR_CMDQ_ICCE] = "CMDQ ICCE Errors",
> +		[MMC_ERR_REQ_TIMEOUT] = "Request Timedout",
> +		[MMC_ERR_CMDQ_REQ_TIMEOUT] = "CMDQ Request Timedout",
> +		[MMC_ERR_ICE_CFG] = "ICE Config Errors",
> +	};
> +	int i;
> +
> +	if (!host)
> +		return -EINVAL;

Do not need to check host here

> +
> +	for (i = 0; i < MMC_ERR_MAX; i++) {
> +		if (desc[i])
> +			seq_printf(file, "# %s:\t %d\n",
> +					desc[i], host->err_stats[i]);
> +	}
> +
> +	return 0;
> +}
> +
> +static int mmc_err_stats_open(struct inode *inode, struct file *file)
> +{
> +	return single_open(file, mmc_err_stats_show, inode->i_private);
> +}
> +
> +static ssize_t mmc_err_stats_write(struct file *filp, const char __user *ubuf,
> +				   size_t cnt, loff_t *ppos)
> +{
> +	struct mmc_host *host = filp->f_mapping->host->i_private;
> +
> +	if (!host)
> +		return -EINVAL;

Do not need to check host here

> +
> +	pr_debug("%s: Resetting MMC error statistics\n", __func__);
> +	memset(host->err_stats, 0, sizeof(host->err_stats));
> +
> +	return cnt;
> +}
> +
> +static const struct file_operations mmc_err_stats_fops = {
> +	.open	= mmc_err_stats_open,
> +	.read	= seq_read,
> +	.write	= mmc_err_stats_write,
> +};
> +
>  void mmc_add_host_debugfs(struct mmc_host *host)
>  {
>  	struct dentry *root;
> @@ -236,6 +293,9 @@ void mmc_add_host_debugfs(struct mmc_host *host)
>  	debugfs_create_file_unsafe("clock", S_IRUSR | S_IWUSR, root, host,
>  				   &mmc_clock_fops);
>  
> +	debugfs_create_file("err_stats", 0600, root, host,
> +		&mmc_err_stats_fops);
> +
>  #ifdef CONFIG_FAIL_MMC_REQUEST
>  	if (fail_request)
>  		setup_fault_attr(&fail_default_attr, fail_request);

Re: [PATCH V4 6/7] mmc: sdhci: Set error state for mmc driver

[Adrian Hunter]

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);


>  			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);
>  	}
>  

RE: [PATCH V4 2/7] mmc: sdhci: Capture eMMC and SD card errors

[Sajida Bhanu (Temp) (QUIC)]

Hi,

Thanks for the review.

Please find the comments inline.

Thanks,
Sajida
> -----Original Message-----
> From: Adrian Hunter <adrian.hunter@intel.com>
> Sent: Tuesday, March 8, 2022 3:07 PM
> To: Sajida Bhanu (Temp) (QUIC) <quic_c_sbhanu@quicinc.com>;
> quic_riteshh@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>; Liangliang Lu <quic_luliang@quicinc.com>;
> quic_nguyenb <quic_nguyenb@quicinc.com>
> Subject: Re: [PATCH V4 2/7] mmc: sdhci: Capture eMMC and SD card errors
> 
> On 2.3.2022 15.03, Shaik Sajida Bhanu wrote:
> > Add changes to capture eMMC and SD card errors.
> > This is useful for debug and testing.
> >
> > Signed-off-by: Liangliang Lu <quic_luliang@quicinc.com>
> > Signed-off-by: Sayali Lokhande <quic_sayalil@quicinc.com>
> > Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
> > Signed-off-by: Shaik Sajida Bhanu <quic_c_sbhanu@quicinc.com>
> > ---
> >  drivers/mmc/host/sdhci.c | 52
> > +++++++++++++++++++++++++++++++++++++-----------
> >  1 file changed, 40 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index
> > 07c6da1..741fb06 100644
> > --- a/drivers/mmc/host/sdhci.c
> > +++ b/drivers/mmc/host/sdhci.c
> > @@ -3159,6 +3159,7 @@ static void sdhci_timeout_timer(struct timer_list
> *t)
> >  	spin_lock_irqsave(&host->lock, flags);
> >
> >  	if (host->cmd && !sdhci_data_line_cmd(host->cmd)) {
> > +		mmc_debugfs_err_stats_inc(host->mmc,
> MMC_ERR_REQ_TIMEOUT);
> 
> Let's make a macro for this:
> 
> #define sdhci_err_stats_inc(host, err_name) \
> 	mmc_debugfs_err_stats_inc(host->mmc, MMC_ERR_ ## err_name)
> 
Sure
> >  		pr_err("%s: Timeout waiting for hardware cmd interrupt.\n",
> >  		       mmc_hostname(host->mmc));
> 
> Please move the err_stats_inc here after the error message i.e.
> 
> 		sdhci_err_stats_inc(host, REQ_TIMEOUT);
> 
Ok 
> >  		sdhci_dumpregs(host);
> > @@ -3181,6 +3182,7 @@ static void sdhci_timeout_data_timer(struct
> > timer_list *t)
> >
> >  	if (host->data || host->data_cmd ||
> >  	    (host->cmd && sdhci_data_line_cmd(host->cmd))) {
> > +		mmc_debugfs_err_stats_inc(host->mmc,
> MMC_ERR_REQ_TIMEOUT);
> >  		pr_err("%s: Timeout waiting for hardware interrupt.\n",
> >  		       mmc_hostname(host->mmc));
> 
> Please move the err_stats_inc here after the error message i.e.
> 
> 		sdhci_err_stats_inc(host, REQ_TIMEOUT);
> 
Ok
> >  		sdhci_dumpregs(host);
> > @@ -3240,11 +3242,15 @@ static void sdhci_cmd_irq(struct sdhci_host
> > *host, u32 intmask, u32 *intmask_p)
> >
> >  	if (intmask & (SDHCI_INT_TIMEOUT | SDHCI_INT_CRC |
> >  		       SDHCI_INT_END_BIT | SDHCI_INT_INDEX)) {
> > -		if (intmask & SDHCI_INT_TIMEOUT)
> > +		if (intmask & SDHCI_INT_TIMEOUT) {
> >  			host->cmd->error = -ETIMEDOUT;
> > -		else
> > +			mmc_debugfs_err_stats_inc(host->mmc,
> MMC_ERR_CMD_TIMEOUT);
> > +		} else {
> >  			host->cmd->error = -EILSEQ;
> > -
> > +			if (host->cmd->opcode !=
> MMC_SEND_TUNING_BLOCK ||
> > +					host->cmd->opcode !=
> MMC_SEND_TUNING_BLOCK_HS200)
> 
> That should be && not ||
> 
> Please add a helper as below, then this can be:
> 
> 			if (!mmc_op_tuning(host->cmd->opcode))
> 				sdhci_err_stats_inc(host, CMD_CRC);
> 
> 
> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h index
> d9a65c6a8816..d809bdfcc59b 100644
> --- a/include/linux/mmc/mmc.h
> +++ b/include/linux/mmc/mmc.h
> @@ -99,6 +99,12 @@ static inline bool mmc_op_multi(u32 opcode)
>  	       opcode == MMC_READ_MULTIPLE_BLOCK;  }
> 
> +static inline bool mmc_op_tuning(u32 opcode) {
> +	return opcode == MMC_SEND_TUNING_BLOCK ||
> +	       opcode == MMC_SEND_TUNING_BLOCK_HS200; }
> +
>  /*
>   * MMC_SWITCH argument format:
>   *
> 
> 
> 
Sure will add a function and call
> > +				mmc_debugfs_err_stats_inc(host->mmc,
> MMC_ERR_CMD_CRC);
> > +		}
> >  		/* Treat data command CRC error the same as data CRC error
> */
> >  		if (host->cmd->data &&
> >  		    (intmask & (SDHCI_INT_CRC | SDHCI_INT_TIMEOUT)) ==
> @@ -3265,6
> > +3271,7 @@ static void sdhci_cmd_irq(struct sdhci_host *host, u32
> intmask, u32 *intmask_p)
> >  		int err = (auto_cmd_status & SDHCI_AUTO_CMD_TIMEOUT)
> ?
> >  			  -ETIMEDOUT :
> >  			  -EILSEQ;
> 
> Should be a blank line here after the local variable declarations.
> 
Sure
> > +		mmc_debugfs_err_stats_inc(host->mmc,
> MMC_ERR_AUTO_CMD);
> >
> >  		if (sdhci_auto_cmd23(host, mrq)) {
> >  			mrq->sbc->error = err;
> > @@ -3342,6 +3349,7 @@ static void sdhci_data_irq(struct sdhci_host
> *host, u32 intmask)
> >  			if (intmask & SDHCI_INT_DATA_TIMEOUT) {
> >  				host->data_cmd = NULL;
> >  				data_cmd->error = -ETIMEDOUT;
> > +				mmc_debugfs_err_stats_inc(host->mmc,
> MMC_ERR_CMD_TIMEOUT);
> >  				__sdhci_finish_mrq(host, data_cmd->mrq);
> >  				return;
> >  			}
> > @@ -3375,18 +3383,25 @@ static void sdhci_data_irq(struct sdhci_host
> *host, u32 intmask)
> >  		return;
> >  	}
> >
> > -	if (intmask & SDHCI_INT_DATA_TIMEOUT)
> > +	if (intmask & SDHCI_INT_DATA_TIMEOUT) {
> >  		host->data->error = -ETIMEDOUT;
> > +		mmc_debugfs_err_stats_inc(host->mmc,
> MMC_ERR_DAT_TIMEOUT);
> > +	}
> 
> The parenthesis should be with the 'else'.  Please use checkpatch with the --
> strict option and fix the issues.
> 
Sure Thank you
> >  	else if (intmask & SDHCI_INT_DATA_END_BIT)
> >  		host->data->error = -EILSEQ;
> >  	else if ((intmask & SDHCI_INT_DATA_CRC) &&
> >  		SDHCI_GET_CMD(sdhci_readw(host, SDHCI_COMMAND))
> > -			!= MMC_BUS_TEST_R)
> > +			!= MMC_BUS_TEST_R) {
> >  		host->data->error = -EILSEQ;
> > +		if (host->cmd->opcode != MMC_SEND_TUNING_BLOCK ||
> > +				host->cmd->opcode !=
> MMC_SEND_TUNING_BLOCK_HS200)
> > +			mmc_debugfs_err_stats_inc(host->mmc,
> MMC_ERR_DAT_CRC);
> 
> As previously:
> 
> 		if (!mmc_op_tuning(host->cmd->opcode))
> 			sdhci_err_stats_inc(host, DAT_CRC);
> 
Sure will update
> > +	}
> >  	else if (intmask & SDHCI_INT_ADMA_ERROR) {
> >  		pr_err("%s: ADMA error: 0x%08x\n", mmc_hostname(host-
> >mmc),
> >  		       intmask);
> >  		sdhci_adma_show_error(host);
> > +		mmc_debugfs_err_stats_inc(host->mmc,
> MMC_ERR_ADMA);
> >  		host->data->error = -EIO;
> >  		if (host->ops->adma_workaround)
> >  			host->ops->adma_workaround(host, intmask); @@ -
> 3905,20 +3920,33 @@
> > bool sdhci_cqe_irq(struct sdhci_host *host, u32 intmask, int *cmd_error,
> >  	if (!host->cqe_on)
> >  		return false;
> >
> > -	if (intmask & (SDHCI_INT_INDEX | SDHCI_INT_END_BIT |
> SDHCI_INT_CRC))
> > +	if (intmask & (SDHCI_INT_INDEX | SDHCI_INT_END_BIT |
> SDHCI_INT_CRC))
> > +{
> >  		*cmd_error = -EILSEQ;
> > -	else if (intmask & SDHCI_INT_TIMEOUT)
> > +		if (intmask & SDHCI_INT_CRC) {
> > +			if (host->cmd->opcode !=
> MMC_SEND_TUNING_BLOCK ||
> > +					host->cmd->opcode !=
> MMC_SEND_TUNING_BLOCK_HS200)
> 
> Please count INDEX and END_BIT errors same as CRC errors i.e. just
> 
> 		if (!mmc_op_tuning(host->cmd->opcode))
> 			sdhci_err_stats_inc(host, CMD_CRC);
> 
Sure
> > +				mmc_debugfs_err_stats_inc(host->mmc,
> MMC_ERR_CMD_CRC);
> > +		}
> > +	} else if (intmask & SDHCI_INT_TIMEOUT) {
> >  		*cmd_error = -ETIMEDOUT;
> > -	else
> > +		mmc_debugfs_err_stats_inc(host->mmc,
> MMC_ERR_CMD_TIMEOUT);
> > +	} else
> >  		*cmd_error = 0;
> >
> > -	if (intmask & (SDHCI_INT_DATA_END_BIT | SDHCI_INT_DATA_CRC))
> > +	if (intmask & (SDHCI_INT_DATA_END_BIT | SDHCI_INT_DATA_CRC))
> {
> >  		*data_error = -EILSEQ;
> > -	else if (intmask & SDHCI_INT_DATA_TIMEOUT)
> > +		if (intmask & SDHCI_INT_DATA_CRC) {
> > +			if (host->cmd->opcode !=
> MMC_SEND_TUNING_BLOCK ||
> > +					host->cmd->opcode !=
> MMC_SEND_TUNING_BLOCK_HS200)
> 
> Please count END_BIT error same as CRC error i.e. just
> 
> 		if (!mmc_op_tuning(host->cmd->opcode))
> 			sdhci_err_stats_inc(host, DAT_CRC);
> 
Sure
> > +				mmc_debugfs_err_stats_inc(host->mmc,
> MMC_ERR_DAT_CRC);
> > +		}
> > +	} else if (intmask & SDHCI_INT_DATA_TIMEOUT) {
> >  		*data_error = -ETIMEDOUT;
> > -	else if (intmask & SDHCI_INT_ADMA_ERROR)
> > +		mmc_debugfs_err_stats_inc(host->mmc,
> MMC_ERR_DAT_TIMEOUT);
> > +	} else if (intmask & SDHCI_INT_ADMA_ERROR) {
> >  		*data_error = -EIO;
> > -	else
> > +		mmc_debugfs_err_stats_inc(host->mmc,
> MMC_ERR_ADMA);
> > +	} else
> >  		*data_error = 0;
> >
> >  	/* Clear selected interrupts. */

RE: [PATCH V4 1/7] mmc: core: Capture eMMC and SD card errors

[Sajida Bhanu (Temp) (QUIC)]

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:15 PM
> To: Sajida Bhanu (Temp) (QUIC) <quic_c_sbhanu@quicinc.com>;
> quic_riteshh@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>; Liangliang Lu <quic_luliang@quicinc.com>;
> quic_nguyenb <quic_nguyenb@quicinc.com>
> Subject: Re: [PATCH V4 1/7] mmc: core: Capture eMMC and SD card errors
> 
> On 2.3.2022 15.03, Shaik Sajida Bhanu wrote:
> > Add changes to capture eMMC and SD card errors.
> > This is useful for debug and testing.
> >
> > Signed-off-by: Liangliang Lu <quic_luliang@quicinc.com>
> > Signed-off-by: Sayali Lokhande <quic_sayalil@quicinc.com>
> > Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
> > Signed-off-by: Ram Prakash Gupta <quic_rampraka@quicinc.com>
> > Signed-off-by: Shaik Sajida Bhanu <quic_c_sbhanu@quicinc.com>
> > ---
> >  drivers/mmc/core/core.c  |  6 ++++++
> >  include/linux/mmc/host.h | 23 +++++++++++++++++++++++
> >  2 files changed, 29 insertions(+)
> >
> > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index
> > 368f104..f3679ed 100644
> > --- a/drivers/mmc/core/core.c
> > +++ b/drivers/mmc/core/core.c
> > @@ -2242,6 +2242,12 @@ 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.
> > +	 */
> > +	host->err_stats[MMC_ERR_CMD_TIMEOUT] = 0;
> >  	mmc_release_host(host);
> >
> >   out:
> > diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h index
> > 7afb57c..3b7f1e5 100644
> > --- a/include/linux/mmc/host.h
> > +++ b/include/linux/mmc/host.h
> > @@ -93,6 +93,23 @@ struct mmc_clk_phase_map {
> >
> >  struct mmc_host;
> >
> > +enum mmc_err_stat {
> > +	MMC_ERR_CMD_TIMEOUT,
> > +	MMC_ERR_CMD_CRC,
> > +	MMC_ERR_DAT_TIMEOUT,
> > +	MMC_ERR_DAT_CRC,
> > +	MMC_ERR_AUTO_CMD,
> > +	MMC_ERR_ADMA,
> > +	MMC_ERR_TUNING,
> > +	MMC_ERR_CMDQ_RED,
> > +	MMC_ERR_CMDQ_GCE,
> > +	MMC_ERR_CMDQ_ICCE,
> > +	MMC_ERR_REQ_TIMEOUT,
> > +	MMC_ERR_CMDQ_REQ_TIMEOUT,
> > +	MMC_ERR_ICE_CFG,
> > +	MMC_ERR_MAX,
> > +};
> > +
> >  struct mmc_host_ops {
> >  	/*
> >  	 * It is optional for the host to implement pre_req and post_req in
> > @@ -500,6 +517,7 @@ struct mmc_host {
> >
> >  	/* Host Software Queue support */
> >  	bool			hsq_enabled;
> > +	u32                     err_stats[MMC_ERR_MAX];
> 
> This makes it look like err_stats has something to do with Host Software
> Queue.
> Perhaps move it to be with debugfs_root. Also use tabs not spaces
> 
> 	struct dentry		*debugfs_root;
> 	u32			err_stats[MMC_ERR_MAX];
> 
Sure will move
> >
> >  	unsigned long		private[] ____cacheline_aligned;
> >  };
> > @@ -635,6 +653,11 @@ static inline enum dma_data_direction
> mmc_get_dma_dir(struct mmc_data *data)
> >  	return data->flags & MMC_DATA_WRITE ? DMA_TO_DEVICE :
> > DMA_FROM_DEVICE;  }
> >
> > +static inline void mmc_debugfs_err_stats_inc(struct mmc_host *host,
> > +		enum mmc_err_stat stat) {
> > +	host->err_stats[stat] += 1;
> > +}
> > +
> >  int mmc_send_tuning(struct mmc_host *host, u32 opcode, int
> > *cmd_error);  int mmc_send_abort_tuning(struct mmc_host *host, u32
> > opcode);  int mmc_get_ext_csd(struct mmc_card *card, u8
> **new_ext_csd);

RE: [PATCH V4 3/7] mmc: debugfs: Add debug fs entry for mmc driver

[Sajida Bhanu (Temp) (QUIC)]

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:20 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>; quic_nguyenb <quic_nguyenb@quicinc.com>
> Subject: Re: [PATCH V4 3/7] mmc: debugfs: Add debug fs entry for mmc
> driver
> 
> On 2.3.2022 15.03, Shaik Sajida Bhanu wrote:
> > Add debug fs entry to query eMMC and SD card errors statistics
> >
> > Signed-off-by: Liangliang Lu <quic_luliang@quicinc.com>
> > Signed-off-by: Sayali Lokhande <quic_sayalil@quicinc.com>
> > Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
> > Signed-off-by: Shaik Sajida Bhanu <quic_c_sbhanu@quicinc.com>
> > ---
> >  drivers/mmc/core/debugfs.c | 60
> > ++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 60 insertions(+)
> >
> > diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c
> > index 3fdbc80..db0988c 100644
> > --- a/drivers/mmc/core/debugfs.c
> > +++ b/drivers/mmc/core/debugfs.c
> > @@ -223,6 +223,63 @@ static int mmc_clock_opt_set(void *data, u64 val)
> > DEFINE_DEBUGFS_ATTRIBUTE(mmc_clock_fops, mmc_clock_opt_get,
> mmc_clock_opt_set,
> >  	"%llu\n");
> >
> > +static int mmc_err_stats_show(struct seq_file *file, void *data) {
> > +	struct mmc_host *host = (struct mmc_host *)file->private;
> > +	const char *desc[MMC_ERR_MAX] = {
> > +		[MMC_ERR_CMD_TIMEOUT] = "Command Timeout
> Occurred",
> > +		[MMC_ERR_CMD_CRC] = "Command CRC Errors Occurred",
> > +		[MMC_ERR_DAT_TIMEOUT] = "Data Timeout Occurred",
> > +		[MMC_ERR_DAT_CRC] = "Data CRC Errors Occurred",
> > +		[MMC_ERR_AUTO_CMD] = "Auto-Cmd Error Occurred",
> > +		[MMC_ERR_ADMA] = "ADMA Error Occurred",
> > +		[MMC_ERR_TUNING] = "Tuning Error Occurred",
> > +		[MMC_ERR_CMDQ_RED] = "CMDQ RED Errors",
> > +		[MMC_ERR_CMDQ_GCE] = "CMDQ GCE Errors",
> > +		[MMC_ERR_CMDQ_ICCE] = "CMDQ ICCE Errors",
> > +		[MMC_ERR_REQ_TIMEOUT] = "Request Timedout",
> > +		[MMC_ERR_CMDQ_REQ_TIMEOUT] = "CMDQ Request
> Timedout",
> > +		[MMC_ERR_ICE_CFG] = "ICE Config Errors",
> > +	};
> > +	int i;
> > +
> > +	if (!host)
> > +		return -EINVAL;
> 
> Do not need to check host here
> 
Ok 
> > +
> > +	for (i = 0; i < MMC_ERR_MAX; i++) {
> > +		if (desc[i])
> > +			seq_printf(file, "# %s:\t %d\n",
> > +					desc[i], host->err_stats[i]);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int mmc_err_stats_open(struct inode *inode, struct file *file)
> > +{
> > +	return single_open(file, mmc_err_stats_show, inode->i_private); }
> > +
> > +static ssize_t mmc_err_stats_write(struct file *filp, const char __user
> *ubuf,
> > +				   size_t cnt, loff_t *ppos)
> > +{
> > +	struct mmc_host *host = filp->f_mapping->host->i_private;
> > +
> > +	if (!host)
> > +		return -EINVAL;
> 
> Do not need to check host here
> 
Sure will skip the host check.
> > +
> > +	pr_debug("%s: Resetting MMC error statistics\n", __func__);
> > +	memset(host->err_stats, 0, sizeof(host->err_stats));
> > +
> > +	return cnt;
> > +}
> > +
> > +static const struct file_operations mmc_err_stats_fops = {
> > +	.open	= mmc_err_stats_open,
> > +	.read	= seq_read,
> > +	.write	= mmc_err_stats_write,
> > +};
> > +
> >  void mmc_add_host_debugfs(struct mmc_host *host)  {
> >  	struct dentry *root;
> > @@ -236,6 +293,9 @@ void mmc_add_host_debugfs(struct mmc_host
> *host)
> >  	debugfs_create_file_unsafe("clock", S_IRUSR | S_IWUSR, root, host,
> >  				   &mmc_clock_fops);
> >
> > +	debugfs_create_file("err_stats", 0600, root, host,
> > +		&mmc_err_stats_fops);
> > +
> >  #ifdef CONFIG_FAIL_MMC_REQUEST
> >  	if (fail_request)
> >  		setup_fault_attr(&fail_default_attr, fail_request);

RE: [PATCH V4 6/7] mmc: sdhci: Set error state for mmc driver

[Sajida Bhanu (Temp) (QUIC)]

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);
> >  	}
> >

Re: [PATCH V4 6/7] mmc: sdhci: Set error state for mmc driver

[Adrian Hunter]

On 12/03/2022 20:15, Sajida Bhanu (Temp) (QUIC) wrote:
> 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.


In sdhci.c use only sdhci_err_stats_inc() adding err_stats entries as needed.
Seems like there are controller timeouts (say MMC_ERR_CTRL_TIMEOUT) and
unexpected interrupts (say MMC_ERR_UNEXPECTED_IRQ)

In drivers/mmc/core/debugfs.c calculate err_state as needed e.g.

static int mmc_err_state_get(void *data, u64 *val)
{
	struct mmc_host *host = data;

	*val = host->err_stats[MMC_ERR_REQ_TIMEOUT] ||
	       host->err_stats[MMC_ERR_CTRL_TIMEOUT] ||
	       host->err_stats[MMC_ERR_UNEXPECTED_IRQ] ||
	       <etc>

	return 0;
}

RE: [PATCH V4 6/7] mmc: sdhci: Set error state for mmc driver

[Sajida Bhanu (Temp) (QUIC)]

> -----Original Message-----
> From: Adrian Hunter <adrian.hunter@intel.com>
> Sent: Wednesday, March 16, 2022 11:28 AM
> 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 12/03/2022 20:15, Sajida Bhanu (Temp) (QUIC) wrote:
> > 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.
> 
> 
> In sdhci.c use only sdhci_err_stats_inc() adding err_stats entries as needed.
> Seems like there are controller timeouts (say MMC_ERR_CTRL_TIMEOUT)
> and unexpected interrupts (say MMC_ERR_UNEXPECTED_IRQ)
> 
> In drivers/mmc/core/debugfs.c calculate err_state as needed e.g.
> 
> static int mmc_err_state_get(void *data, u64 *val) {
> 	struct mmc_host *host = data;
> 
> 	*val = host->err_stats[MMC_ERR_REQ_TIMEOUT] ||
> 	       host->err_stats[MMC_ERR_CTRL_TIMEOUT] ||
> 	       host->err_stats[MMC_ERR_UNEXPECTED_IRQ] ||
> 	       <etc>
> 
> 	return 0;
> }
Hi,

Thank you for the suggestion, sure will make the changes.

Thanks,
Sajida