From patchwork Fri Jun 20 21:01:09 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tim Kryger X-Patchwork-Id: 4392781 Return-Path: X-Original-To: patchwork-linux-samsung-soc@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 73499BEEAA for ; Fri, 20 Jun 2014 21:01:14 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 51026203DF for ; Fri, 20 Jun 2014 21:01:13 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 146AB203B6 for ; Fri, 20 Jun 2014 21:01:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S966614AbaFTVBL (ORCPT ); Fri, 20 Jun 2014 17:01:11 -0400 Received: from mail-ob0-f176.google.com ([209.85.214.176]:35222 "EHLO mail-ob0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S966723AbaFTVBK (ORCPT ); Fri, 20 Jun 2014 17:01:10 -0400 Received: by mail-ob0-f176.google.com with SMTP id wm4so1688556obc.21 for ; Fri, 20 Jun 2014 14:01:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=mUwKojuh1vl/i75WwvDIy1X3zIF7RJv2xNCwPC6Ygy8=; b=doDHZRGEeGwQ+znNAF0yRtCM7pD2EiDK2V38iVeTJyZuAEwMWRJKFQwtCwsRe7T7kV gFe+ylkIhvy0/lzlK2+kwuKDRZUYKzDAaVvLbV3ja+PXlo7tzbW07J27SUpeXKDJ8VMH aBuqPNo7TZQKXl8q7/SlANICFRFy7y9XY70VYujxfrD08gvnOso1huhrY+t5B0vrFyDX ok1fdLp2d/baK3oRFI83dQzYW2LWIrmPyVk4vmJUql6ahxMZff9Ya6gZVtxYYz+UnuQN IJxf4o2qTfZBucRJX2gtrLCeryT8iyYE/iTMSPZnOaDOCL05FVDaQFZwg8hZnKtGE2Ds xAxA== MIME-Version: 1.0 X-Received: by 10.60.45.105 with SMTP id l9mr6141655oem.65.1403298069264; Fri, 20 Jun 2014 14:01:09 -0700 (PDT) Received: by 10.60.84.175 with HTTP; Fri, 20 Jun 2014 14:01:09 -0700 (PDT) In-Reply-To: References: <53A2DA96.3070500@samsung.com> Date: Fri, 20 Jun 2014 14:01:09 -0700 Message-ID: Subject: Re: MMC error on Exynos4210 board From: Tim Kryger To: Sachin Kamat Cc: Jaehoon Chung , Markus Mayer , Matt Porter , Ulf Hansson , linux-mmc , linux-samsung-soc , Yuvaraj C D , "tgih.jun@samsung.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.8 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, T_DKIM_INVALID, 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 Thu, Jun 19, 2014 at 8:33 PM, Sachin Kamat wrote: > On Thu, Jun 19, 2014 at 6:11 PM, Jaehoon Chung wrote: >> On 06/19/2014 07:40 PM, Sachin Kamat wrote: >>> On Thu, Jun 19, 2014 at 2:40 PM, Tim Kryger wrote: >>>> On Thu, Jun 19, 2014 at 1:49 AM, Sachin Kamat wrote: >>>>> On Thu, Jun 19, 2014 at 2:12 PM, Tim Kryger wrote: >>>>>> On Wed, Jun 18, 2014 at 8:54 PM, Sachin Kamat wrote: >>>>>>>> On Wed, Jun 18, 2014 at 4:33 AM, Sachin Kamat wrote: >>>>>> >>>>>>>>> I see the below error on Exynos4210 based Origen board with linux-next >>>>>>>>> (20140618). >>>>>>>>> Reverting the below commit works fine. >>>>>>>>> >>>>>>>>> Commit: 8d02e775a6 "mmc: sdhci: Use mmc core regulator infrastucture" >>>>>> >>>>>>>>> >>>>>>>>> -- [ 2.068992] sdhci: Secure Digital Host Controller Interface driver >>>>>>>>> [ 2.075059] sdhci: Copyright(c) Pierre Ossman >>>>>>>>> [ 2.079762] of_get_named_gpiod_flags: can't parse gpios property of >>>>>>>>> node '/sdhci@12510000[0]' >>>>>>>>> [ 2.088021] s3c-sdhci 12510000.sdhci: clock source 2: mmc_busclk.2 >>>>>>>>> (50000000 Hz) >>>>>>>>> [ 2.095322] of_get_named_gpiod_flags: can't parse gpios property of >>>>>>>>> node '/sdhci@12510000[0]' >>>>>>>>> [ 2.103794] of_get_named_gpiod_flags: can't parse gpios property of >>>>>>>>> node '/sdhci@12510000[0]' >>>>>>>>> [ 2.112478] s3c-sdhci 12510000.sdhci: No vqmmc regulator found >>>>>>>>> [ 2.118117] mmc0: Hardware doesn't report any support voltages. >>>>>>>>> [ 2.124004] s3c-sdhci 12510000.sdhci: sdhci_add_host() failed >>>>>>>>> [ 2.130080] of_get_named_gpiod_flags: can't parse gpios property of >>>>>>>>> node '/sdhci@12530000[0]' >>>>>>>>> [ 2.138352] s3c-sdhci 12530000.sdhci: clock source 2: mmc_busclk.2 >>>>>>>>> (16666667 Hz) >>>>>>>>> [ 2.145661] of_get_named_gpiod_flags: can't parse gpios property of >>>>>>>>> node '/sdhci@12530000[0]' >>>>>>>>> [ 2.154139] of_get_named_gpiod_flags: can't parse gpios property of >>>>>>>>> node '/sdhci@12530000[0]' >>>>>>>>> [ 2.162834] s3c-sdhci 12530000.sdhci: No vqmmc regulator found >>>>>>>>> [ 2.168464] mmc0: Hardware doesn't report any support voltages. >>>>>>>>> [ 2.174349] s3c-sdhci 12530000.sdhci: sdhci_add_host() failed >>>>>> >>>>>>>>> [ 2.336148] Waiting for root device /dev/mmcblk0p1... >>>>>> >>>>>>> FYI, the board has a 2.8V fixed regulator supply connected to the MMC. >>>>>>> You may refer to arch/arm/boot/dts/exynos4210-origen.dts for more details. >>>>>> >>>>>> A 2.8v regulator results in mmc->ocr_avail being set to MMC_VDD_27_28 >>>>>> | MMC_VDD_28_29. >>>>>> >>>>>> The SDHCI capabilities register only indicates support of three voltage levels >>>>>> - 1.8v: SDHCI_CAN_VDD_180 => MMC_VDD_165_195 >>>>>> - 3.0v: SDHCI_CAN_VDD_300 => MMC_VDD_29_30 | MMC_VDD_30_31 >>>>>> - 3.3v: SDHCI_CAN_VDD_330 => MMC_VDD_32_33 | MMC_VDD_33_34 >> >> Right. sdhci capabilities only indicated them. >> But I think SoC can be support the specific VDD range. >> >>>>>> >>>>>> Even if all capability bits of the host controller were set, there >>>>>> still wouldn't be any overlap. Thus you see a "Hardware doesn't >>>>>> report any support voltages" message. >>>>>> >>>>>> Previously, this issue was being swept under the rug by cec2e21 mmc: >>>>>> sdhci: Use regulator min/max voltage range according to spec. That >>>>>> change hacked up the voltage range checks such that with your 2.8v >>>>>> fixed regulator, the driver would believe the host could support >>>>>> MMC_VDD_29_30 | MMC_VDD_30_31 | MMC_VDD_32_33 | MMC_VDD_33_34. The >>>>>> driver would start down the path of commanding 3.3v-3.4v (the highest >>>>>> voltage range believed to be supported). At the last second, the >>>>>> driver would see the regulator was fixed and blindly skip over the set >>>>>> voltage operation, saving it from failure. >>>>>> >>>>>> Since my patch eliminates the bogus voltage range checks, your board >>>>>> is now getting caught playing too loose with the SDHCI regulator >>>>>> voltages. >>>>>> >>>>>> Furthermore, the fixed regulator special-case logic that helped hide >>>>>> your issue should also be considered for removal given that fixed >>>>>> regulators now behave properly thanks to c00dc35 regulator: core: >>>>>> Allow regulator_set_voltage for fixed regulators. >>>>> >>>>> Thanks for the detailed explanation. What do you propose to get this fixed >>>> It would be nice if the driver could be extended >>>> to handle the peculiarities of your board in a deliberate manner but >>>> limiting the common sdhci driver to supporting only the three voltages >>>> from the spec also seems sensible. >>> >>> Until such time that the driver gets fixed to handle 2.8V fixed supply your >>> current patch leaves several of Exynos boards broken for now. >> >> the all of exynos used the fixed-regulator(2.8v) should be broken. >> (Maybe exynos4 series??) > > Probably. I haven't verified for the other boards. Hence would be good to handle > this case in the driver itself. The current external VDD regulator support in the SDHCI driver feels a bit tacked on. External regulators could reasonably support other voltage ranges, like MMC_VDD_27_28 | MMC_VDD_28_29, but those never appear in the final ocr_avail for the host. The driver only looks for the intersection of the ranges implied by the capabilities register and those of the external regulator. Later, when it comes time to set a voltage, the driver will BUG if it can't translate the requested voltage into one of the three voltage levels supported by the SDHCI Power Control register. I wonder if it is really necessary to constrain the driver this way. It seems like if we are using an external regulator, the capabilities of the host controller itself are irrelevant. Also, must we touch the Power Control register if we are using an external regulator? I would think not. Can you give the following patch a try and see if it resolves the issue you observed? case MMC_VDD_165_195: @@ -1284,12 +1291,6 @@ static void sdhci_set_power(struct sdhci_host *host, unsigned char mode, if (host->quirks & SDHCI_QUIRK_DELAY_AFTER_POWER) mdelay(10); } - - if (!IS_ERR(mmc->supply.vmmc)) { - spin_unlock_irq(&host->lock); - mmc_regulator_set_ocr(host->mmc, mmc->supply.vmmc, vdd); - spin_lock_irq(&host->lock); - } } /*****************************************************************************\ @@ -3092,8 +3093,9 @@ int sdhci_add_host(struct sdhci_host *host) SDHCI_MAX_CURRENT_MULTIPLIER; } + /* If OCR set by external regulators, use it instead */ if (mmc->ocr_avail) - ocr_avail &= mmc->ocr_avail; + ocr_avail = mmc->ocr_avail; if (host->ocr_mask) ocr_avail &= host->ocr_mask; --- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" 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 c23a872..07a2426 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -1226,6 +1226,13 @@ static void sdhci_set_power(struct sdhci_host *host, unsigned char mode, struct mmc_host *mmc = host->mmc; u8 pwr = 0; + if (!IS_ERR(mmc->supply.vmmc)) { + spin_unlock_irq(&host->lock); + mmc_regulator_set_ocr(host->mmc, mmc->supply.vmmc, vdd); + spin_lock_irq(&host->lock); + return; + } + if (mode != MMC_POWER_OFF) { switch (1 << vdd) {