From patchwork Thu Mar 24 13:21:47 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adrian Hunter X-Patchwork-Id: 8660931 Return-Path: X-Original-To: patchwork-linux-samsung-soc@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 94FE49F36E for ; Thu, 24 Mar 2016 13:25:32 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 4F65F203AE for ; Thu, 24 Mar 2016 13:25:31 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 182BC201CD for ; Thu, 24 Mar 2016 13:25:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756080AbcCXNZ3 (ORCPT ); Thu, 24 Mar 2016 09:25:29 -0400 Received: from mga04.intel.com ([192.55.52.120]:16330 "EHLO mga04.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752310AbcCXNZ2 (ORCPT ); Thu, 24 Mar 2016 09:25:28 -0400 Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga104.fm.intel.com with ESMTP; 24 Mar 2016 06:25:27 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,385,1455004800"; d="scan'208";a="770625701" Received: from ahunter-desktop.fi.intel.com (HELO [10.237.72.168]) ([10.237.72.168]) by orsmga003.jf.intel.com with ESMTP; 24 Mar 2016 06:25:24 -0700 Subject: Re: Warnings for invalid VDD (sdhci-s3c) To: Adrian Hunter , Krzysztof Kozlowski , Jisheng Zhang , Jaehoon Chung References: <20160324155840.50474b98@xhacker> <56F3A0B7.4050509@samsung.com> <20160324162449.29c9cf78@xhacker> <56F3A88F.50603@samsung.com> <56F3E77D.7030201@intel.com> Cc: Ulf Hansson , ludovic.desroches@atmel.com, "Ivan T. Ivanov" , linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-samsung-soc@vger.kernel.org From: Adrian Hunter Organization: Intel Finland Oy, Registered Address: PL 281, 00181 Helsinki, Business Identity Code: 0357606 - 4, Domiciled in Helsinki Message-ID: <56F3E9EB.3090207@intel.com> Date: Thu, 24 Mar 2016 15:21:47 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <56F3E77D.7030201@intel.com> Sender: linux-samsung-soc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-samsung-soc@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 24/03/16 15:11, Adrian Hunter wrote: > On 24/03/16 10:42, Krzysztof Kozlowski wrote: >> On 24.03.2016 17:24, Jisheng Zhang wrote: >>> Hi, >>> >>> On Thu, 24 Mar 2016 17:09:27 +0900 Jaehoon Chung wrote: >>> >>>> Hi, >>>> >>>> On 03/24/2016 04:58 PM, Jisheng Zhang wrote: >>>>> Hi, >>>>> >>>>> On Thu, 24 Mar 2016 16:28:56 +0900 Krzysztof Kozlowski wrote: >>>>> >>>>>> Hi, >>>>>> >>>>>> After 918f4cbd4340 ("mmc: sdhci: restore behavior when setting VDD via >>>>>> external regulator") On Trats2 board I see warnings for invalid VDD >>>>>> value (2.8V): >>>>>> >>>>>> [ 3.119656] ------------[ cut here ]------------ >>>>>> [ 3.119666] WARNING: CPU: 3 PID: 90 at >>>>>> ../drivers/mmc/host/sdhci.c:1234 sdhci_do_set_ios+0x4cc/0x5e0 >>>>>> [ 3.119669] mmc0: Invalid vdd 0x10 >>>>> >>>>> Per my understanding, the wrong vdd indicates a wrong ocr, what's the voltage of >>>>> this host's vmmc regulator? >>>> >>>> As i know, it's fixed-voltage with gpio on trats2. It's 2.8V. >>>> I didn't check this entirely..need to check ocr value. >>>> >>> >>> I may know the reason. the vmmc is 2.8v, then mmc_regulator_get_supply() convert >>> the value to a ocr as 0x10. The key here is that the 2.8v is invalid in SDHCI >>> case and isn't accepted by current sdhci driver. >> >> Yeah, I already wrote that. It is the part of the warning and my email. >> Our regulator is fixed at 2.8 which is 0x10. :) >> >>> I dunno the elegant solution to handle this case, let's wait for sdhci maintainers >>> idea. >> >> Hmm... > > I haven't tested it, but what about this: And now with checkpatch complaints fixed: From: Adrian Hunter Date: Thu, 24 Mar 2016 14:29:24 +0200 Subject: [PATCH V2] mmc: sdhci: Fix regression setting power on Trats2 board Several commits relating to setting power have been introducing problems by putting driver-specific rules into generic SDHCI code. Fix by adding a 'set_power' callback and restoring the default behaviour prior to commit 918f4cbd4340 ("mmc: sdhci: restore behavior when setting VDD via external regulator"). The desired behaviour of that commit is gotten by having sdhci-pxav3 provide its own set_power callback. Reported-by: Krzysztof Kozlowski Fixes: 918f4cbd4340 ("mmc: sdhci: restore behavior when setting VDD...) Signed-off-by: Adrian Hunter Cc: stable@vger.kernel.org # v4.5+ Reviewed-by: Jisheng Zhang Tested-by: Jisheng Zhang Tested-by: Ludovic Desroches Tested-by: Krzysztof Kozlowski --- drivers/mmc/host/sdhci-pxav3.c | 17 +++++++++++++++++ drivers/mmc/host/sdhci.c | 38 ++++++++++++++++++++++++++++++-------- drivers/mmc/host/sdhci.h | 4 ++++ 3 files changed, 51 insertions(+), 8 deletions(-) diff --git a/drivers/mmc/host/sdhci-pxav3.c b/drivers/mmc/host/sdhci-pxav3.c index f5edf9d3a18a..673d1e8446a5 100644 --- a/drivers/mmc/host/sdhci-pxav3.c +++ b/drivers/mmc/host/sdhci-pxav3.c @@ -307,8 +307,25 @@ static void pxav3_set_uhs_signaling(struct sdhci_host *host, unsigned int uhs) __func__, uhs, ctrl_2); } +static void pxav3_set_power(struct sdhci_host *host, unsigned char mode, + unsigned short vdd) +{ + struct mmc_host *mmc = host->mmc; + u8 pwr = host->pwr; + + sdhci_set_power(host, mode, vdd); + + if (host->pwr == pwr) + return; + + spin_unlock_irq(&host->lock); + mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd); + spin_lock_irq(&host->lock); +} + static const struct sdhci_ops pxav3_sdhci_ops = { .set_clock = sdhci_set_clock, + .set_power = pxav3_set_power, .platform_send_init_74_clocks = pxav3_gen_init_74_clocks, .get_max_clock = sdhci_pltfm_clk_get_max_clock, .set_bus_width = sdhci_set_bus_width, diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index add9fdfd1d8f..282257222121 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -1269,10 +1269,24 @@ clock_set: } EXPORT_SYMBOL_GPL(sdhci_set_clock); -static void sdhci_set_power(struct sdhci_host *host, unsigned char mode, - unsigned short vdd) +static void sdhci_set_power_reg(struct sdhci_host *host, unsigned char mode, + unsigned short vdd) { struct mmc_host *mmc = host->mmc; + + spin_unlock_irq(&host->lock); + mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd); + spin_lock_irq(&host->lock); + + if (mode != MMC_POWER_OFF) + sdhci_writeb(host, SDHCI_POWER_ON, SDHCI_POWER_CONTROL); + else + sdhci_writeb(host, 0, SDHCI_POWER_CONTROL); +} + +void sdhci_set_power(struct sdhci_host *host, unsigned char mode, + unsigned short vdd) +{ u8 pwr = 0; if (mode != MMC_POWER_OFF) { @@ -1335,12 +1349,20 @@ static void sdhci_set_power(struct sdhci_host *host, unsigned char mode, if (host->quirks & SDHCI_QUIRK_DELAY_AFTER_POWER) mdelay(10); } +} +EXPORT_SYMBOL_GPL(sdhci_set_power); - if (!IS_ERR(mmc->supply.vmmc)) { - spin_unlock_irq(&host->lock); - mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd); - spin_lock_irq(&host->lock); - } +static void __sdhci_set_power(struct sdhci_host *host, unsigned char mode, + unsigned short vdd) +{ + struct mmc_host *mmc = host->mmc; + + if (host->ops->set_power) + host->ops->set_power(host, mode, vdd); + else if (!IS_ERR(mmc->supply.vmmc)) + sdhci_set_power_reg(host, mode, vdd); + else + sdhci_set_power(host, mode, vdd); } /*****************************************************************************\ @@ -1490,7 +1512,7 @@ static void sdhci_do_set_ios(struct sdhci_host *host, struct mmc_ios *ios) } } - sdhci_set_power(host, ios->power_mode, ios->vdd); + __sdhci_set_power(host, ios->power_mode, ios->vdd); if (host->ops->platform_send_init_74_clocks) host->ops->platform_send_init_74_clocks(host, ios->power_mode); diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h index 0115e9907bf8..033d72b5bbd5 100644 --- a/drivers/mmc/host/sdhci.h +++ b/drivers/mmc/host/sdhci.h @@ -529,6 +529,8 @@ struct sdhci_ops { #endif void (*set_clock)(struct sdhci_host *host, unsigned int clock); + void (*set_power)(struct sdhci_host *host, unsigned char mode, + unsigned short vdd); int (*enable_dma)(struct sdhci_host *host); unsigned int (*get_max_clock)(struct sdhci_host *host); @@ -660,6 +662,8 @@ static inline bool sdhci_sdio_irq_enabled(struct sdhci_host *host) } void sdhci_set_clock(struct sdhci_host *host, unsigned int clock); +void sdhci_set_power(struct sdhci_host *host, unsigned char mode, + unsigned short vdd); void sdhci_set_bus_width(struct sdhci_host *host, int width); void sdhci_reset(struct sdhci_host *host, u8 mask); void sdhci_set_uhs_signaling(struct sdhci_host *host, unsigned timing);