1. Patchew
  2. linux
  3. [PATCH RESEND V2 0/2] Adjust some error messages for SD UHS-II initialization process
  4. View patch

[PATCH RESEND V2 2/2] mmc: sdhci-uhs2: Adjust some error messages and register dump for SD UHS-II card

Victor Shih posted 2 patches 7 months ago
There is a newer version of this series
[PATCH RESEND V2 2/2] mmc: sdhci-uhs2: Adjust some error messages and register dump for SD UHS-II card
Posted by Victor Shih 7 months ago 
From: Victor Shih <victor.shih@genesyslogic.com.tw>

Adjust some error messages to debug mode and register dump to dynamic
debug mode to avoid causing misunderstanding it is an error.

Signed-off-by: Victor Shih <victor.shih@genesyslogic.com.tw>
---
 drivers/mmc/host/sdhci-uhs2.c | 20 ++++++++++----------
 drivers/mmc/host/sdhci.h      | 16 ++++++++++++++++
 2 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/host/sdhci-uhs2.c b/drivers/mmc/host/sdhci-uhs2.c
index c53b64d50c0d..0efeb9d0c376 100644
--- a/drivers/mmc/host/sdhci-uhs2.c
+++ b/drivers/mmc/host/sdhci-uhs2.c
@@ -99,8 +99,8 @@ void sdhci_uhs2_reset(struct sdhci_host *host, u16 mask)
 	/* hw clears the bit when it's done */
 	if (read_poll_timeout_atomic(sdhci_readw, val, !(val & mask), 10,
 				     UHS2_RESET_TIMEOUT_100MS, true, host, SDHCI_UHS2_SW_RESET)) {
-		pr_warn("%s: %s: Reset 0x%x never completed. %s: clean reset bit.\n", __func__,
-			mmc_hostname(host->mmc), (int)mask, mmc_hostname(host->mmc));
+		pr_debug("%s: %s: Reset 0x%x never completed. %s: clean reset bit.\n", __func__,
+			 mmc_hostname(host->mmc), (int)mask, mmc_hostname(host->mmc));
 		sdhci_writeb(host, 0, SDHCI_UHS2_SW_RESET);
 		return;
 	}
@@ -335,8 +335,8 @@ static int sdhci_uhs2_interface_detect(struct sdhci_host *host)
 	if (read_poll_timeout(sdhci_readl, val, (val & SDHCI_UHS2_IF_DETECT),
 			      100, UHS2_INTERFACE_DETECT_TIMEOUT_100MS, true,
 			      host, SDHCI_PRESENT_STATE)) {
-		pr_warn("%s: not detect UHS2 interface in 100ms.\n", mmc_hostname(host->mmc));
-		sdhci_dumpregs(host);
+		pr_debug("%s: not detect UHS2 interface in 100ms.\n", mmc_hostname(host->mmc));
+		sdhci_dbg_dumpregs(host, "UHS2 interface detect timeout in 100ms");
 		return -EIO;
 	}
 
@@ -345,8 +345,8 @@ static int sdhci_uhs2_interface_detect(struct sdhci_host *host)
 
 	if (read_poll_timeout(sdhci_readl, val, (val & SDHCI_UHS2_LANE_SYNC),
 			      100, UHS2_LANE_SYNC_TIMEOUT_150MS, true, host, SDHCI_PRESENT_STATE)) {
-		pr_warn("%s: UHS2 Lane sync fail in 150ms.\n", mmc_hostname(host->mmc));
-		sdhci_dumpregs(host);
+		pr_debug("%s: UHS2 Lane sync fail in 150ms.\n", mmc_hostname(host->mmc));
+		sdhci_dbg_dumpregs(host, "UHS2 Lane sync fail in 150ms");
 		return -EIO;
 	}
 
@@ -417,12 +417,12 @@ static int sdhci_uhs2_do_detect_init(struct mmc_host *mmc)
 		host->ops->uhs2_pre_detect_init(host);
 
 	if (sdhci_uhs2_interface_detect(host)) {
-		pr_warn("%s: cannot detect UHS2 interface.\n", mmc_hostname(host->mmc));
+		pr_debug("%s: cannot detect UHS2 interface.\n", mmc_hostname(host->mmc));
 		return -EIO;
 	}
 
 	if (sdhci_uhs2_init(host)) {
-		pr_warn("%s: UHS2 init fail.\n", mmc_hostname(host->mmc));
+		pr_debug("%s: UHS2 init fail.\n", mmc_hostname(host->mmc));
 		return -EIO;
 	}
 
@@ -504,8 +504,8 @@ static int sdhci_uhs2_check_dormant(struct sdhci_host *host)
 	if (read_poll_timeout(sdhci_readl, val, (val & SDHCI_UHS2_IN_DORMANT_STATE),
 			      100, UHS2_CHECK_DORMANT_TIMEOUT_100MS, true, host,
 			      SDHCI_PRESENT_STATE)) {
-		pr_warn("%s: UHS2 IN_DORMANT fail in 100ms.\n", mmc_hostname(host->mmc));
-		sdhci_dumpregs(host);
+		pr_debug("%s: UHS2 IN_DORMANT fail in 100ms.\n", mmc_hostname(host->mmc));
+		sdhci_dbg_dumpregs(host, "UHS2 IN_DORMANT fail in 100ms");
 		return -EIO;
 	}
 	return 0;
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index f9d65dd0f2b2..70ada1857a4c 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -900,4 +900,20 @@ void sdhci_switch_external_dma(struct sdhci_host *host, bool en);
 void sdhci_set_data_timeout_irq(struct sdhci_host *host, bool enable);
 void __sdhci_set_timeout(struct sdhci_host *host, struct mmc_command *cmd);
 
+#if defined(CONFIG_DYNAMIC_DEBUG) || \
+	(defined(CONFIG_DYNAMIC_DEBUG_CORE) && defined(DYNAMIC_DEBUG_MODULE))
+#define SDHCI_DBG_ANYWAY 0
+#elif defined(DEBUG)
+#define SDHCI_DBG_ANYWAY 1
+#else
+#define SDHCI_DBG_ANYWAY 0
+#endif
+
+#define sdhci_dbg_dumpregs(host, fmt)					\
+do {									\
+	DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt);			\
+	if (DYNAMIC_DEBUG_BRANCH(descriptor) ||	SDHCI_DBG_ANYWAY)	\
+		sdhci_dumpregs(host);					\
+} while (0)
+
 #endif /* __SDHCI_HW_H */
-- 
2.43.0
Re: [PATCH RESEND V2 2/2] mmc: sdhci-uhs2: Adjust some error messages and register dump for SD UHS-II card
Posted by Adrian Hunter 6 months, 3 weeks ago 
On 23/05/2025 14:01, Victor Shih wrote:
> From: Victor Shih <victor.shih@genesyslogic.com.tw>
> 
> Adjust some error messages to debug mode and register dump to dynamic
> debug mode to avoid causing misunderstanding it is an error.
> 
> Signed-off-by: Victor Shih <victor.shih@genesyslogic.com.tw>

I think Ulf asked for the drivers/mmc/host/sdhci.h change to be
a separate patch.

In any case:

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci-uhs2.c | 20 ++++++++++----------
>  drivers/mmc/host/sdhci.h      | 16 ++++++++++++++++
>  2 files changed, 26 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-uhs2.c b/drivers/mmc/host/sdhci-uhs2.c
> index c53b64d50c0d..0efeb9d0c376 100644
> --- a/drivers/mmc/host/sdhci-uhs2.c
> +++ b/drivers/mmc/host/sdhci-uhs2.c
> @@ -99,8 +99,8 @@ void sdhci_uhs2_reset(struct sdhci_host *host, u16 mask)
>  	/* hw clears the bit when it's done */
>  	if (read_poll_timeout_atomic(sdhci_readw, val, !(val & mask), 10,
>  				     UHS2_RESET_TIMEOUT_100MS, true, host, SDHCI_UHS2_SW_RESET)) {
> -		pr_warn("%s: %s: Reset 0x%x never completed. %s: clean reset bit.\n", __func__,
> -			mmc_hostname(host->mmc), (int)mask, mmc_hostname(host->mmc));
> +		pr_debug("%s: %s: Reset 0x%x never completed. %s: clean reset bit.\n", __func__,
> +			 mmc_hostname(host->mmc), (int)mask, mmc_hostname(host->mmc));
>  		sdhci_writeb(host, 0, SDHCI_UHS2_SW_RESET);
>  		return;
>  	}
> @@ -335,8 +335,8 @@ static int sdhci_uhs2_interface_detect(struct sdhci_host *host)
>  	if (read_poll_timeout(sdhci_readl, val, (val & SDHCI_UHS2_IF_DETECT),
>  			      100, UHS2_INTERFACE_DETECT_TIMEOUT_100MS, true,
>  			      host, SDHCI_PRESENT_STATE)) {
> -		pr_warn("%s: not detect UHS2 interface in 100ms.\n", mmc_hostname(host->mmc));
> -		sdhci_dumpregs(host);
> +		pr_debug("%s: not detect UHS2 interface in 100ms.\n", mmc_hostname(host->mmc));
> +		sdhci_dbg_dumpregs(host, "UHS2 interface detect timeout in 100ms");
>  		return -EIO;
>  	}
>  
> @@ -345,8 +345,8 @@ static int sdhci_uhs2_interface_detect(struct sdhci_host *host)
>  
>  	if (read_poll_timeout(sdhci_readl, val, (val & SDHCI_UHS2_LANE_SYNC),
>  			      100, UHS2_LANE_SYNC_TIMEOUT_150MS, true, host, SDHCI_PRESENT_STATE)) {
> -		pr_warn("%s: UHS2 Lane sync fail in 150ms.\n", mmc_hostname(host->mmc));
> -		sdhci_dumpregs(host);
> +		pr_debug("%s: UHS2 Lane sync fail in 150ms.\n", mmc_hostname(host->mmc));
> +		sdhci_dbg_dumpregs(host, "UHS2 Lane sync fail in 150ms");
>  		return -EIO;
>  	}
>  
> @@ -417,12 +417,12 @@ static int sdhci_uhs2_do_detect_init(struct mmc_host *mmc)
>  		host->ops->uhs2_pre_detect_init(host);
>  
>  	if (sdhci_uhs2_interface_detect(host)) {
> -		pr_warn("%s: cannot detect UHS2 interface.\n", mmc_hostname(host->mmc));
> +		pr_debug("%s: cannot detect UHS2 interface.\n", mmc_hostname(host->mmc));
>  		return -EIO;
>  	}
>  
>  	if (sdhci_uhs2_init(host)) {
> -		pr_warn("%s: UHS2 init fail.\n", mmc_hostname(host->mmc));
> +		pr_debug("%s: UHS2 init fail.\n", mmc_hostname(host->mmc));
>  		return -EIO;
>  	}
>  
> @@ -504,8 +504,8 @@ static int sdhci_uhs2_check_dormant(struct sdhci_host *host)
>  	if (read_poll_timeout(sdhci_readl, val, (val & SDHCI_UHS2_IN_DORMANT_STATE),
>  			      100, UHS2_CHECK_DORMANT_TIMEOUT_100MS, true, host,
>  			      SDHCI_PRESENT_STATE)) {
> -		pr_warn("%s: UHS2 IN_DORMANT fail in 100ms.\n", mmc_hostname(host->mmc));
> -		sdhci_dumpregs(host);
> +		pr_debug("%s: UHS2 IN_DORMANT fail in 100ms.\n", mmc_hostname(host->mmc));
> +		sdhci_dbg_dumpregs(host, "UHS2 IN_DORMANT fail in 100ms");
>  		return -EIO;
>  	}
>  	return 0;
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index f9d65dd0f2b2..70ada1857a4c 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -900,4 +900,20 @@ void sdhci_switch_external_dma(struct sdhci_host *host, bool en);
>  void sdhci_set_data_timeout_irq(struct sdhci_host *host, bool enable);
>  void __sdhci_set_timeout(struct sdhci_host *host, struct mmc_command *cmd);
>  
> +#if defined(CONFIG_DYNAMIC_DEBUG) || \
> +	(defined(CONFIG_DYNAMIC_DEBUG_CORE) && defined(DYNAMIC_DEBUG_MODULE))
> +#define SDHCI_DBG_ANYWAY 0
> +#elif defined(DEBUG)
> +#define SDHCI_DBG_ANYWAY 1
> +#else
> +#define SDHCI_DBG_ANYWAY 0
> +#endif
> +
> +#define sdhci_dbg_dumpregs(host, fmt)					\
> +do {									\
> +	DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt);			\
> +	if (DYNAMIC_DEBUG_BRANCH(descriptor) ||	SDHCI_DBG_ANYWAY)	\
> +		sdhci_dumpregs(host);					\
> +} while (0)
> +
>  #endif /* __SDHCI_HW_H */
Re: [PATCH RESEND V2 2/2] mmc: sdhci-uhs2: Adjust some error messages and register dump for SD UHS-II card
Posted by Victor Shih 6 months, 2 weeks ago 
On Wed, May 28, 2025 at 8:10 PM Adrian Hunter <adrian.hunter@intel.com> wrote:
>
> On 23/05/2025 14:01, Victor Shih wrote:
> > From: Victor Shih <victor.shih@genesyslogic.com.tw>
> >
> > Adjust some error messages to debug mode and register dump to dynamic
> > debug mode to avoid causing misunderstanding it is an error.
> >
> > Signed-off-by: Victor Shih <victor.shih@genesyslogic.com.tw>
>
> I think Ulf asked for the drivers/mmc/host/sdhci.h change to be
> a separate patch.
>
> In any case:
>
> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
>

Hi, Adrian

I left this part out, I'll resend the patch for the new series to
match this thing,
and I will also keep your acked tag.

Thanks, Victor Shih

> > ---
> >  drivers/mmc/host/sdhci-uhs2.c | 20 ++++++++++----------
> >  drivers/mmc/host/sdhci.h      | 16 ++++++++++++++++
> >  2 files changed, 26 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci-uhs2.c b/drivers/mmc/host/sdhci-uhs2.c
> > index c53b64d50c0d..0efeb9d0c376 100644
> > --- a/drivers/mmc/host/sdhci-uhs2.c
> > +++ b/drivers/mmc/host/sdhci-uhs2.c
> > @@ -99,8 +99,8 @@ void sdhci_uhs2_reset(struct sdhci_host *host, u16 mask)
> >       /* hw clears the bit when it's done */
> >       if (read_poll_timeout_atomic(sdhci_readw, val, !(val & mask), 10,
> >                                    UHS2_RESET_TIMEOUT_100MS, true, host, SDHCI_UHS2_SW_RESET)) {
> > -             pr_warn("%s: %s: Reset 0x%x never completed. %s: clean reset bit.\n", __func__,
> > -                     mmc_hostname(host->mmc), (int)mask, mmc_hostname(host->mmc));
> > +             pr_debug("%s: %s: Reset 0x%x never completed. %s: clean reset bit.\n", __func__,
> > +                      mmc_hostname(host->mmc), (int)mask, mmc_hostname(host->mmc));
> >               sdhci_writeb(host, 0, SDHCI_UHS2_SW_RESET);
> >               return;
> >       }
> > @@ -335,8 +335,8 @@ static int sdhci_uhs2_interface_detect(struct sdhci_host *host)
> >       if (read_poll_timeout(sdhci_readl, val, (val & SDHCI_UHS2_IF_DETECT),
> >                             100, UHS2_INTERFACE_DETECT_TIMEOUT_100MS, true,
> >                             host, SDHCI_PRESENT_STATE)) {
> > -             pr_warn("%s: not detect UHS2 interface in 100ms.\n", mmc_hostname(host->mmc));
> > -             sdhci_dumpregs(host);
> > +             pr_debug("%s: not detect UHS2 interface in 100ms.\n", mmc_hostname(host->mmc));
> > +             sdhci_dbg_dumpregs(host, "UHS2 interface detect timeout in 100ms");
> >               return -EIO;
> >       }
> >
> > @@ -345,8 +345,8 @@ static int sdhci_uhs2_interface_detect(struct sdhci_host *host)
> >
> >       if (read_poll_timeout(sdhci_readl, val, (val & SDHCI_UHS2_LANE_SYNC),
> >                             100, UHS2_LANE_SYNC_TIMEOUT_150MS, true, host, SDHCI_PRESENT_STATE)) {
> > -             pr_warn("%s: UHS2 Lane sync fail in 150ms.\n", mmc_hostname(host->mmc));
> > -             sdhci_dumpregs(host);
> > +             pr_debug("%s: UHS2 Lane sync fail in 150ms.\n", mmc_hostname(host->mmc));
> > +             sdhci_dbg_dumpregs(host, "UHS2 Lane sync fail in 150ms");
> >               return -EIO;
> >       }
> >
> > @@ -417,12 +417,12 @@ static int sdhci_uhs2_do_detect_init(struct mmc_host *mmc)
> >               host->ops->uhs2_pre_detect_init(host);
> >
> >       if (sdhci_uhs2_interface_detect(host)) {
> > -             pr_warn("%s: cannot detect UHS2 interface.\n", mmc_hostname(host->mmc));
> > +             pr_debug("%s: cannot detect UHS2 interface.\n", mmc_hostname(host->mmc));
> >               return -EIO;
> >       }
> >
> >       if (sdhci_uhs2_init(host)) {
> > -             pr_warn("%s: UHS2 init fail.\n", mmc_hostname(host->mmc));
> > +             pr_debug("%s: UHS2 init fail.\n", mmc_hostname(host->mmc));
> >               return -EIO;
> >       }
> >
> > @@ -504,8 +504,8 @@ static int sdhci_uhs2_check_dormant(struct sdhci_host *host)
> >       if (read_poll_timeout(sdhci_readl, val, (val & SDHCI_UHS2_IN_DORMANT_STATE),
> >                             100, UHS2_CHECK_DORMANT_TIMEOUT_100MS, true, host,
> >                             SDHCI_PRESENT_STATE)) {
> > -             pr_warn("%s: UHS2 IN_DORMANT fail in 100ms.\n", mmc_hostname(host->mmc));
> > -             sdhci_dumpregs(host);
> > +             pr_debug("%s: UHS2 IN_DORMANT fail in 100ms.\n", mmc_hostname(host->mmc));
> > +             sdhci_dbg_dumpregs(host, "UHS2 IN_DORMANT fail in 100ms");
> >               return -EIO;
> >       }
> >       return 0;
> > diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> > index f9d65dd0f2b2..70ada1857a4c 100644
> > --- a/drivers/mmc/host/sdhci.h
> > +++ b/drivers/mmc/host/sdhci.h
> > @@ -900,4 +900,20 @@ void sdhci_switch_external_dma(struct sdhci_host *host, bool en);
> >  void sdhci_set_data_timeout_irq(struct sdhci_host *host, bool enable);
> >  void __sdhci_set_timeout(struct sdhci_host *host, struct mmc_command *cmd);
> >
> > +#if defined(CONFIG_DYNAMIC_DEBUG) || \
> > +     (defined(CONFIG_DYNAMIC_DEBUG_CORE) && defined(DYNAMIC_DEBUG_MODULE))
> > +#define SDHCI_DBG_ANYWAY 0
> > +#elif defined(DEBUG)
> > +#define SDHCI_DBG_ANYWAY 1
> > +#else
> > +#define SDHCI_DBG_ANYWAY 0
> > +#endif
> > +
> > +#define sdhci_dbg_dumpregs(host, fmt)                                        \
> > +do {                                                                 \
> > +     DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt);                 \
> > +     if (DYNAMIC_DEBUG_BRANCH(descriptor) || SDHCI_DBG_ANYWAY)       \
> > +             sdhci_dumpregs(host);                                   \
> > +} while (0)
> > +
> >  #endif /* __SDHCI_HW_H */
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.