| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAPDyKFqCo2-9fYYAoNpmJf7XOiTvLM07hQks1io-S0MvmbAjCQ@mail.gmail.com>
Date: Mon, 19 May 2025 14:24:28 +0200
From: Ulf Hansson <ulf.hansson@...aro.org>
To: Victor Shih <victorshihgli@...il.com>
Cc: adrian.hunter@...el.com, linux-mmc@...r.kernel.org,
linux-kernel@...r.kernel.org, benchuanggli@...il.com,
HL.Liu@...esyslogic.com.tw, Greg.tu@...esyslogic.com.tw,
Ben Chuang <ben.chuang@...esyslogic.com.tw>,
Victor Shih <victor.shih@...esyslogic.com.tw>
Subject: Re: [PATCH V1 2/2] mmc: sdhci-uhs2: Adjust some error messages and
register dump for SD UHS-II card
On Fri, 16 May 2025 at 11:27, Victor Shih <victorshihgli@...il.com> wrote:
>
> From: Victor Shih <victor.shih@...esyslogic.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@...esyslogic.com.tw>
> Signed-off-by: Victor Shih <victor.shih@...esyslogic.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
Powered by blists - more mailing lists