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: Ben Chuang <ben.chuang@genesyslogic.com.tw>
Signed-off-by: Victor Shih <victor.shih@genesyslogic.com.tw>
---
drivers/mmc/host/sdhci-uhs2.c | 18 +++++++++---------
drivers/mmc/host/sdhci.h | 16 ++++++++++++++++
2 files changed, 25 insertions(+), 9 deletions(-)
diff --git a/drivers/mmc/host/sdhci-uhs2.c b/drivers/mmc/host/sdhci-uhs2.c
index c53b64d50c0d..9ff867aee985 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));
+ DBG("%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);
+ DBG("%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);
+ DBG("%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));
+ DBG("%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));
+ DBG("%s: UHS2 init fail.\n", mmc_hostname(host->mmc));
return -EIO;
}
@@ -504,7 +504,7 @@ 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));
+ DBG("%s: UHS2 IN_DORMANT fail in 100ms.\n", mmc_hostname(host->mmc));
sdhci_dumpregs(host);
return -EIO;
}
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index cd0e35a80542..2c28240e6003 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -898,4 +898,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.0On Fri, 16 May 2025 at 11:27, Victor Shih <victorshihgli@gmail.com> 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.
Dumping the register may be useful for the debug level, I am not sure.
Maybe Adrian has an opinion?
>
> Signed-off-by: Ben Chuang <ben.chuang@genesyslogic.com.tw>
> Signed-off-by: Victor Shih <victor.shih@genesyslogic.com.tw>
> ---
> drivers/mmc/host/sdhci-uhs2.c | 18 +++++++++---------
> drivers/mmc/host/sdhci.h | 16 ++++++++++++++++
> 2 files changed, 25 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-uhs2.c b/drivers/mmc/host/sdhci-uhs2.c
> index c53b64d50c0d..9ff867aee985 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));
> + DBG("%s: %s: Reset 0x%x never completed. %s: clean reset bit.\n", __func__,
> + mmc_hostname(host->mmc), (int)mask, mmc_hostname(host->mmc));
As I said on patch1, please use pr_debug() and drop the macro.
> 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);
> + DBG("%s: not detect UHS2 interface in 100ms.\n", mmc_hostname(host->mmc));
> + sdhci_dbg_dumpregs(host, "UHS2 interface detect timeout in 100ms");
If we really need this, I think we should first introduce the helper
function in a separate patch, that precedes $subject patch in the
series.
> 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);
> + DBG("%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));
> + DBG("%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));
> + DBG("%s: UHS2 init fail.\n", mmc_hostname(host->mmc));
> return -EIO;
> }
>
> @@ -504,7 +504,7 @@ 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));
> + DBG("%s: UHS2 IN_DORMANT fail in 100ms.\n", mmc_hostname(host->mmc));
> sdhci_dumpregs(host);
> return -EIO;
> }
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index cd0e35a80542..2c28240e6003 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -898,4 +898,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
>
Kind regards
Uffe
On 19/05/2025 15:24, Ulf Hansson wrote:
> On Fri, 16 May 2025 at 11:27, Victor Shih <victorshihgli@gmail.com> 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.
>
> Dumping the register may be useful for the debug level, I am not sure.
> Maybe Adrian has an opinion?
My understanding was that the original issue was that these messages
appear when it is not a UHS-II card, so the register dump should also
become debug-only.
>
>>
>> Signed-off-by: Ben Chuang <ben.chuang@genesyslogic.com.tw>
>> Signed-off-by: Victor Shih <victor.shih@genesyslogic.com.tw>
>> ---
>> drivers/mmc/host/sdhci-uhs2.c | 18 +++++++++---------
>> drivers/mmc/host/sdhci.h | 16 ++++++++++++++++
>> 2 files changed, 25 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-uhs2.c b/drivers/mmc/host/sdhci-uhs2.c
>> index c53b64d50c0d..9ff867aee985 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));
>> + DBG("%s: %s: Reset 0x%x never completed. %s: clean reset bit.\n", __func__,
>> + mmc_hostname(host->mmc), (int)mask, mmc_hostname(host->mmc));
>
> As I said on patch1, please use pr_debug() and drop the macro.
>
>> 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);
>> + DBG("%s: not detect UHS2 interface in 100ms.\n", mmc_hostname(host->mmc));
>> + sdhci_dbg_dumpregs(host, "UHS2 interface detect timeout in 100ms");
>
> If we really need this, I think we should first introduce the helper
> function in a separate patch, that precedes $subject patch in the
> series.
>
>> 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);
>> + DBG("%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));
>> + DBG("%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));
>> + DBG("%s: UHS2 init fail.\n", mmc_hostname(host->mmc));
>> return -EIO;
>> }
>>
>> @@ -504,7 +504,7 @@ 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));
>> + DBG("%s: UHS2 IN_DORMANT fail in 100ms.\n", mmc_hostname(host->mmc));
>> sdhci_dumpregs(host);
If the message is to be a debug message, then the register dump
should be also i.e. use sdhci_dbg_dumpregs
>> return -EIO;
>> }
>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>> index cd0e35a80542..2c28240e6003 100644
>> --- a/drivers/mmc/host/sdhci.h
>> +++ b/drivers/mmc/host/sdhci.h
>> @@ -898,4 +898,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
>>
>
> Kind regards
> Uffe
On Wed, 21 May 2025 at 13:04, Adrian Hunter <adrian.hunter@intel.com> wrote: > > On 19/05/2025 15:24, Ulf Hansson wrote: > > On Fri, 16 May 2025 at 11:27, Victor Shih <victorshihgli@gmail.com> 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. > > > > Dumping the register may be useful for the debug level, I am not sure. > > Maybe Adrian has an opinion? > > My understanding was that the original issue was that these messages > appear when it is not a UHS-II card, so the register dump should also > become debug-only. Good point, I agree! [...] Kind regards Uffe
On Mon, May 19, 2025 at 8:25 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Fri, 16 May 2025 at 11:27, Victor Shih <victorshihgli@gmail.com> 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.
>
> Dumping the register may be useful for the debug level, I am not sure.
> Maybe Adrian has an opinion?
>
> >
> > Signed-off-by: Ben Chuang <ben.chuang@genesyslogic.com.tw>
> > Signed-off-by: Victor Shih <victor.shih@genesyslogic.com.tw>
> > ---
> > drivers/mmc/host/sdhci-uhs2.c | 18 +++++++++---------
> > drivers/mmc/host/sdhci.h | 16 ++++++++++++++++
> > 2 files changed, 25 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci-uhs2.c b/drivers/mmc/host/sdhci-uhs2.c
> > index c53b64d50c0d..9ff867aee985 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));
> > + DBG("%s: %s: Reset 0x%x never completed. %s: clean reset bit.\n", __func__,
> > + mmc_hostname(host->mmc), (int)mask, mmc_hostname(host->mmc));
>
> As I said on patch1, please use pr_debug() and drop the macro.
>
Hi, Ulf
This macro has been defined in previous patches not the first time it
has appeared here,
are we still going to drop this macro?
Thanks, Victor Shih
> > 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);
> > + DBG("%s: not detect UHS2 interface in 100ms.\n", mmc_hostname(host->mmc));
> > + sdhci_dbg_dumpregs(host, "UHS2 interface detect timeout in 100ms");
>
> If we really need this, I think we should first introduce the helper
> function in a separate patch, that precedes $subject patch in the
> series.
>
Hi, Ulf
Ok, I will make this helper function into a separate patch in the next version.
Thanks, Victor Shih
> > 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);
> > + DBG("%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));
> > + DBG("%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));
> > + DBG("%s: UHS2 init fail.\n", mmc_hostname(host->mmc));
> > return -EIO;
> > }
> >
> > @@ -504,7 +504,7 @@ 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));
> > + DBG("%s: UHS2 IN_DORMANT fail in 100ms.\n", mmc_hostname(host->mmc));
> > sdhci_dumpregs(host);
> > return -EIO;
> > }
> > diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> > index cd0e35a80542..2c28240e6003 100644
> > --- a/drivers/mmc/host/sdhci.h
> > +++ b/drivers/mmc/host/sdhci.h
> > @@ -898,4 +898,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
> >
>
> Kind regards
> Uffe
On Wed, 21 May 2025 at 12:45, Victor Shih <victorshihgli@gmail.com> wrote:
>
> On Mon, May 19, 2025 at 8:25 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> >
> > On Fri, 16 May 2025 at 11:27, Victor Shih <victorshihgli@gmail.com> 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.
> >
> > Dumping the register may be useful for the debug level, I am not sure.
> > Maybe Adrian has an opinion?
> >
> > >
> > > Signed-off-by: Ben Chuang <ben.chuang@genesyslogic.com.tw>
> > > Signed-off-by: Victor Shih <victor.shih@genesyslogic.com.tw>
> > > ---
> > > drivers/mmc/host/sdhci-uhs2.c | 18 +++++++++---------
> > > drivers/mmc/host/sdhci.h | 16 ++++++++++++++++
> > > 2 files changed, 25 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/mmc/host/sdhci-uhs2.c b/drivers/mmc/host/sdhci-uhs2.c
> > > index c53b64d50c0d..9ff867aee985 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));
> > > + DBG("%s: %s: Reset 0x%x never completed. %s: clean reset bit.\n", __func__,
> > > + mmc_hostname(host->mmc), (int)mask, mmc_hostname(host->mmc));
> >
> > As I said on patch1, please use pr_debug() and drop the macro.
> >
>
> Hi, Ulf
>
> This macro has been defined in previous patches not the first time it
> has appeared here,
> are we still going to drop this macro?
Yes, please.
We don't have macros for other log-prints, so let's not use a macro
for debug prints either.
[...]
Kind regards
Uffe
© 2016 - 2025 Red Hat, Inc.