2022-03-02 22:50:15

by Shaik Sajida Bhanu

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

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


2022-03-02 23:39:09

by Shaik Sajida Bhanu

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

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

Signed-off-by: Liangliang Lu <[email protected]>
Signed-off-by: Sayali Lokhande <[email protected]>
Signed-off-by: Bao D. Nguyen <[email protected]>
Signed-off-by: Shaik Sajida Bhanu <[email protected]>
---
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

2022-03-03 00:14:15

by Shaik Sajida Bhanu

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

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 <[email protected]>
Signed-off-by: Sayali Lokhande <[email protected]>
Signed-off-by: Bao D. Nguyen <[email protected]>
Signed-off-by: Ram Prakash Gupta <[email protected]>
Signed-off-by: Shaik Sajida Bhanu <[email protected]>
---
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

2022-03-09 01:43:16

by Adrian Hunter

[permalink] [raw]
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 <[email protected]>
> Signed-off-by: Sayali Lokhande <[email protected]>
> Signed-off-by: Bao D. Nguyen <[email protected]>
> Signed-off-by: Shaik Sajida Bhanu <[email protected]>
> ---
> 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);

2022-03-13 21:36:59

by Shaik Sajida Bhanu

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

Hi,

Thanks for the review.

Please find the inline comments.

Thanks,
Sajida
> -----Original Message-----
> From: Adrian Hunter <[email protected]>
> Sent: Tuesday, March 8, 2022 3:20 PM
> To: Sajida Bhanu (Temp) (QUIC) <[email protected]>; Asutosh
> Das (asd) <[email protected]>; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; linux-
> [email protected]
> Cc: Veerabhadrarao Badiganti (QUIC) <[email protected]>; Ram
> Prakash Gupta (QUIC) <[email protected]>; Pradeep Pragallapati
> (QUIC) <[email protected]>; Sarthak Garg (QUIC)
> <[email protected]>; Nitin Rawat (QUIC)
> <[email protected]>; Sayali Lokhande (QUIC)
> <[email protected]>; quic_nguyenb <[email protected]>
> 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 <[email protected]>
> > Signed-off-by: Sayali Lokhande <[email protected]>
> > Signed-off-by: Bao D. Nguyen <[email protected]>
> > Signed-off-by: Shaik Sajida Bhanu <[email protected]>
> > ---
> > 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);