* [RFC PATCH v2 3/6] mmc: core: turn the pr_info under CONFIG_MMC_DEBUG into pr_debug
2017-07-19 7:55 ` [RFC PATCH v2 2/6] mmc: core: always check the length of sglist with total data size Shawn Lin
@ 2017-07-19 7:55 ` Shawn Lin
2017-07-19 7:55 ` [RFC PATCH v2 4/6] mmc: Kconfig: downgrade CONFIG_MMC_DEBUG for host drivers only Shawn Lin
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Shawn Lin @ 2017-07-19 7:55 UTC (permalink / raw)
To: Ulf Hansson; +Cc: Adrian Hunter, linux-mmc, Shawn Lin
There are lots of debug message in core.c which use pr_debug
for better dynamic log level control. So it doesn't make sense
for those print to still keep working only under CONFIG_MMC_DEBUG.
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---
Changes in v2: None
drivers/mmc/core/core.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 634f9ca..731aa9f 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2436,10 +2436,9 @@ static int mmc_rescan_try_freq(struct mmc_host *host, unsigned freq)
{
host->f_init = freq;
-#ifdef CONFIG_MMC_DEBUG
- pr_info("%s: %s: trying to init card at %u Hz\n",
+ pr_debug("%s: %s: trying to init card at %u Hz\n",
mmc_hostname(host), __func__, host->f_init);
-#endif
+
mmc_power_up(host, host->ocr_avail);
/*
@@ -2670,9 +2669,7 @@ int mmc_power_save_host(struct mmc_host *host)
{
int ret = 0;
-#ifdef CONFIG_MMC_DEBUG
- pr_info("%s: %s: powering down\n", mmc_hostname(host), __func__);
-#endif
+ pr_debug("%s: %s: powering down\n", mmc_hostname(host), __func__);
mmc_bus_get(host);
@@ -2696,9 +2693,7 @@ int mmc_power_restore_host(struct mmc_host *host)
{
int ret;
-#ifdef CONFIG_MMC_DEBUG
- pr_info("%s: %s: powering up\n", mmc_hostname(host), __func__);
-#endif
+ pr_debug("%s: %s: powering up\n", mmc_hostname(host), __func__);
mmc_bus_get(host);
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [RFC PATCH v2 4/6] mmc: Kconfig: downgrade CONFIG_MMC_DEBUG for host drivers only
2017-07-19 7:55 ` [RFC PATCH v2 2/6] mmc: core: always check the length of sglist with total data size Shawn Lin
2017-07-19 7:55 ` [RFC PATCH v2 3/6] mmc: core: turn the pr_info under CONFIG_MMC_DEBUG into pr_debug Shawn Lin
@ 2017-07-19 7:55 ` Shawn Lin
2017-07-19 7:55 ` [RFC PATCH v2 5/6] mmc: wbsd: remove CONFIG_MMC_DEBUG from the driver Shawn Lin
2017-07-19 7:55 ` [RFC PATCH v2 6/6] mmc: sdhci: " Shawn Lin
3 siblings, 0 replies; 9+ messages in thread
From: Shawn Lin @ 2017-07-19 7:55 UTC (permalink / raw)
To: Ulf Hansson; +Cc: Adrian Hunter, linux-mmc, Shawn Lin
We have removed all code depending on CONFIG_MMC_DEBUG
from mmc core now. So it's safe to make CONFIG_MMC_DEBUG
just for host drivers only and we expect to kill this option
in the future.
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---
Changes in v2: None
drivers/mmc/Kconfig | 7 -------
drivers/mmc/Makefile | 2 --
drivers/mmc/host/Kconfig | 9 +++++++++
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
index 7e803fc4..ec21388 100644
--- a/drivers/mmc/Kconfig
+++ b/drivers/mmc/Kconfig
@@ -12,13 +12,6 @@ menuconfig MMC
If you want MMC/SD/SDIO support, you should say Y here and
also to your specific host controller driver.
-config MMC_DEBUG
- bool "MMC debugging"
- depends on MMC != n
- help
- This is an option for use by developers; most people should
- say N here. This enables MMC core and driver debugging.
-
if MMC
source "drivers/mmc/core/Kconfig"
diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
index 416b6d1..26ab7af 100644
--- a/drivers/mmc/Makefile
+++ b/drivers/mmc/Makefile
@@ -2,7 +2,5 @@
# Makefile for the kernel mmc device drivers.
#
-subdir-ccflags-$(CONFIG_MMC_DEBUG) := -DDEBUG
-
obj-$(CONFIG_MMC) += core/
obj-$(subst m,y,$(CONFIG_MMC)) += host/
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 2242633..66b1749 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -4,6 +4,15 @@
comment "MMC/SD/SDIO Host Controller Drivers"
+config MMC_DEBUG
+ bool "MMC host drivers debugginG"
+ depends on MMC != n
+ help
+ This is an option for use by developers; most people should
+ say N here. This enables MMC host driver debugging. And further
+ added host drivers please don't invent their private macro for
+ debugging.
+
config MMC_ARMMMCI
tristate "ARM AMBA Multimedia Card Interface support"
depends on ARM_AMBA
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [RFC PATCH v2 5/6] mmc: wbsd: remove CONFIG_MMC_DEBUG from the driver
2017-07-19 7:55 ` [RFC PATCH v2 2/6] mmc: core: always check the length of sglist with total data size Shawn Lin
2017-07-19 7:55 ` [RFC PATCH v2 3/6] mmc: core: turn the pr_info under CONFIG_MMC_DEBUG into pr_debug Shawn Lin
2017-07-19 7:55 ` [RFC PATCH v2 4/6] mmc: Kconfig: downgrade CONFIG_MMC_DEBUG for host drivers only Shawn Lin
@ 2017-07-19 7:55 ` Shawn Lin
2017-07-19 7:55 ` [RFC PATCH v2 6/6] mmc: sdhci: " Shawn Lin
3 siblings, 0 replies; 9+ messages in thread
From: Shawn Lin @ 2017-07-19 7:55 UTC (permalink / raw)
To: Ulf Hansson; +Cc: Adrian Hunter, linux-mmc, Shawn Lin
wbsd only use this to print some unsupported command.
However the pr_warn should be enough for dynamic log
control and CONFIG_MMC_DEBUG seems bogus here. Remove it.
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---
Changes in v2: None
drivers/mmc/host/wbsd.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/mmc/host/wbsd.c b/drivers/mmc/host/wbsd.c
index 9668616..546aaf8 100644
--- a/drivers/mmc/host/wbsd.c
+++ b/drivers/mmc/host/wbsd.c
@@ -802,10 +802,8 @@ static void wbsd_request(struct mmc_host *mmc, struct mmc_request *mrq)
break;
default:
-#ifdef CONFIG_MMC_DEBUG
pr_warn("%s: Data command %d is not supported by this controller\n",
mmc_hostname(host->mmc), cmd->opcode);
-#endif
cmd->error = -EINVAL;
goto done;
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [RFC PATCH v2 6/6] mmc: sdhci: remove CONFIG_MMC_DEBUG from the driver
2017-07-19 7:55 ` [RFC PATCH v2 2/6] mmc: core: always check the length of sglist with total data size Shawn Lin
` (2 preceding siblings ...)
2017-07-19 7:55 ` [RFC PATCH v2 5/6] mmc: wbsd: remove CONFIG_MMC_DEBUG from the driver Shawn Lin
@ 2017-07-19 7:55 ` Shawn Lin
2017-07-19 8:24 ` Adrian Hunter
3 siblings, 1 reply; 9+ messages in thread
From: Shawn Lin @ 2017-07-19 7:55 UTC (permalink / raw)
To: Ulf Hansson; +Cc: Adrian Hunter, linux-mmc, Shawn Lin
sdhci uses CONFIG_MMC_DEBUG for showing ADMA descriptor
when occurring ADMA error. And it's also used to dump the
registers whenever calling sdhci_add_host.
On one hand, I don't see any burden to always print the state
ADMA descriptor as it's rare and will help folks better understand
what was happening when seeing ADMA error.
On the other, folks may be interested in checking some registers
at probe time. So we remove the sdhci_dumpregs from __sdhci_add_host
and print some really useful registers in sdhci_setup_host.
Suggested-by: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---
Changes in v2:
- rework the changes for sdhci suggested by Adrian
drivers/mmc/host/sdhci.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index ecd0d43..cf2166e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2502,7 +2502,6 @@ static void sdhci_cmd_irq(struct sdhci_host *host, u32 intmask)
sdhci_finish_command(host);
}
-#ifdef CONFIG_MMC_DEBUG
static void sdhci_adma_show_error(struct sdhci_host *host)
{
void *desc = host->adma_table;
@@ -2530,9 +2529,6 @@ static void sdhci_adma_show_error(struct sdhci_host *host)
break;
}
}
-#else
-static void sdhci_adma_show_error(struct sdhci_host *host) { }
-#endif
static void sdhci_data_irq(struct sdhci_host *host, u32 intmask)
{
@@ -3230,6 +3226,13 @@ int sdhci_setup_host(struct sdhci_host *host)
if (ret == -EPROBE_DEFER)
return ret;
+ DBG("Version: 0x%08x | Present: 0x%08x\n",
+ sdhci_readw(host, SDHCI_HOST_VERSION),
+ sdhci_readl(host, SDHCI_PRESENT_STATE));
+ DBG("Caps: 0x%08x | Caps_1: 0x%08x\n",
+ sdhci_readl(host, SDHCI_CAPABILITIES),
+ sdhci_readl(host, SDHCI_CAPABILITIES_1));
+
sdhci_read_caps(host);
override_timeout_clk = host->timeout_clk;
@@ -3747,10 +3750,6 @@ int __sdhci_add_host(struct sdhci_host *host)
goto untasklet;
}
-#ifdef CONFIG_MMC_DEBUG
- sdhci_dumpregs(host);
-#endif
-
ret = sdhci_led_register(host);
if (ret) {
pr_err("%s: Failed to register LED device: %d\n",
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [RFC PATCH v2 6/6] mmc: sdhci: remove CONFIG_MMC_DEBUG from the driver
2017-07-19 7:55 ` [RFC PATCH v2 6/6] mmc: sdhci: " Shawn Lin
@ 2017-07-19 8:24 ` Adrian Hunter
0 siblings, 0 replies; 9+ messages in thread
From: Adrian Hunter @ 2017-07-19 8:24 UTC (permalink / raw)
To: Shawn Lin, Ulf Hansson; +Cc: linux-mmc
On 19/07/17 10:55, Shawn Lin wrote:
> sdhci uses CONFIG_MMC_DEBUG for showing ADMA descriptor
> when occurring ADMA error. And it's also used to dump the
> registers whenever calling sdhci_add_host.
>
> On one hand, I don't see any burden to always print the state
> ADMA descriptor as it's rare and will help folks better understand
> what was happening when seeing ADMA error.
>
> On the other, folks may be interested in checking some registers
> at probe time. So we remove the sdhci_dumpregs from __sdhci_add_host
> and print some really useful registers in sdhci_setup_host.
>
> Suggested-by: Adrian Hunter <adrian.hunter@intel.com>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
>
> ---
>
> Changes in v2:
> - rework the changes for sdhci suggested by Adrian
>
> drivers/mmc/host/sdhci.c | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index ecd0d43..cf2166e 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -2502,7 +2502,6 @@ static void sdhci_cmd_irq(struct sdhci_host *host, u32 intmask)
> sdhci_finish_command(host);
> }
>
> -#ifdef CONFIG_MMC_DEBUG
> static void sdhci_adma_show_error(struct sdhci_host *host)
> {
> void *desc = host->adma_table;
> @@ -2530,9 +2529,6 @@ static void sdhci_adma_show_error(struct sdhci_host *host)
> break;
> }
> }
> -#else
> -static void sdhci_adma_show_error(struct sdhci_host *host) { }
> -#endif
>
> static void sdhci_data_irq(struct sdhci_host *host, u32 intmask)
> {
> @@ -3230,6 +3226,13 @@ int sdhci_setup_host(struct sdhci_host *host)
> if (ret == -EPROBE_DEFER)
> return ret;
>
> + DBG("Version: 0x%08x | Present: 0x%08x\n",
> + sdhci_readw(host, SDHCI_HOST_VERSION),
> + sdhci_readl(host, SDHCI_PRESENT_STATE));
> + DBG("Caps: 0x%08x | Caps_1: 0x%08x\n",
> + sdhci_readl(host, SDHCI_CAPABILITIES),
> + sdhci_readl(host, SDHCI_CAPABILITIES_1));
> +
> sdhci_read_caps(host);
>
> override_timeout_clk = host->timeout_clk;
> @@ -3747,10 +3750,6 @@ int __sdhci_add_host(struct sdhci_host *host)
> goto untasklet;
> }
>
> -#ifdef CONFIG_MMC_DEBUG
> - sdhci_dumpregs(host);
> -#endif
> -
> ret = sdhci_led_register(host);
> if (ret) {
> pr_err("%s: Failed to register LED device: %d\n",
>
^ permalink raw reply [flat|nested] 9+ messages in thread