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

[Adrian Hunter <adrian.hunter@intel.com>]

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 */