From patchwork Wed Jul 19 07:16:34 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adrian Hunter X-Patchwork-Id: 9850597 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 8CAFF602C8 for ; Wed, 19 Jul 2017 07:22:52 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 7A17528600 for ; Wed, 19 Jul 2017 07:22:52 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6E35328608; Wed, 19 Jul 2017 07:22:52 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E66DA28600 for ; Wed, 19 Jul 2017 07:22:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753073AbdGSHWv (ORCPT ); Wed, 19 Jul 2017 03:22:51 -0400 Received: from mga03.intel.com ([134.134.136.65]:44477 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752759AbdGSHWu (ORCPT ); Wed, 19 Jul 2017 03:22:50 -0400 Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 19 Jul 2017 00:22:49 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.40,380,1496127600"; d="scan'208";a="994606933" Received: from ahunter-desktop.fi.intel.com (HELO [10.237.72.168]) ([10.237.72.168]) by orsmga003.jf.intel.com with ESMTP; 19 Jul 2017 00:22:48 -0700 Subject: Re: [RFC PATCH 6/6] mmc: sdhci: remove CONFIG_MMC_DEBUG from the driver To: Shawn Lin Cc: Ulf Hansson , linux-mmc@vger.kernel.org References: <1500368376-160272-1-git-send-email-shawn.lin@rock-chips.com> <1500368503-160501-1-git-send-email-shawn.lin@rock-chips.com> <1500368503-160501-2-git-send-email-shawn.lin@rock-chips.com> <9e8e1a62-84ca-ce43-5b8d-debcda6e103c@rock-chips.com> From: Adrian Hunter Organization: Intel Finland Oy, Registered Address: PL 281, 00181 Helsinki, Business Identity Code: 0357606 - 4, Domiciled in Helsinki Message-ID: <0daed43c-f648-b536-9a5c-dc43cde7da63@intel.com> Date: Wed, 19 Jul 2017 10:16:34 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <9e8e1a62-84ca-ce43-5b8d-debcda6e103c@rock-chips.com> Content-Language: en-US Sender: linux-mmc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-mmc@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 19/07/17 03:32, Shawn Lin wrote: > Hi Adrian, > > On 2017/7/18 17:38, Adrian Hunter wrote: >> On 18/07/17 12:01, 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, git-blame points out that CONFIG_MMC_DEBUG for >>> sdhci_add_host was added since it's merged for the first time. >>> I don't know what exactly the intention was, but I guess folks >>> don't need it at all? IMHO, it's another all-or-none proposition. >>> I'd prefer to remove this sdhci_dumpregs from sdhci_add_host totally. >>> >>> Signed-off-by: Shawn Lin >>> >>> --- >>> >>> drivers/mmc/host/sdhci.c | 8 -------- >>> 1 file changed, 8 deletions(-) >>> >>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c >>> index ecd0d43..82f1761 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) >>> { >>> @@ -3747,10 +3743,6 @@ int __sdhci_add_host(struct sdhci_host *host) >>> goto untasklet; >>> } >>> -#ifdef CONFIG_MMC_DEBUG >>> - sdhci_dumpregs(host); >>> -#endif >> >> We should still DBG() the interesting registers like SDHCI_HOST_VERSION, >> SDHCI_PRESENT_STATE, SDHCI_CAPABILITIES, SDHCI_CAPABILITIES_1. Better to do >> it earlier on though, like in sdhci_setup_host() after checking >> mmc_regulator_get_supply(). >> > > Okay. So would you like to kill the CONFIG_MMC_DEBUG around > this dempregs and keep the output log always there OR you still > want to keep it under CONFIG_MMC_DEBUG? We absolutely want register dumps if there are unexpected errors, so the dump cannot use dynamic debug. But we also want to be able to see the interesting registers at probe time. We could make use of DYNAMIC_DEBUG_BRANCH and DEFINE_DYNAMIC_DEBUG_METADATA but that is not something that is commonly done. So that leaves adding a few extra pr_debugs (DBG macro for sdhci) at probe time i.e. Remove CONFIG_MMC_DEBUG and add something like: --- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index ecd0d4350e8a..1b619ccc27f5 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -3230,6 +3230,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;