From patchwork Sun May 29 16:21:01 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Drake X-Patchwork-Id: 827822 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter2.kernel.org (8.14.4/8.14.3) with ESMTP id p4TGL3iu018024 for ; Sun, 29 May 2011 16:21:04 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754027Ab1E2QVD (ORCPT ); Sun, 29 May 2011 12:21:03 -0400 Received: from mail-px0-f179.google.com ([209.85.212.179]:50002 "EHLO mail-px0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753972Ab1E2QVB (ORCPT ); Sun, 29 May 2011 12:21:01 -0400 Received: by pxi2 with SMTP id 2so2041893pxi.10 for ; Sun, 29 May 2011 09:21:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type; bh=i6VmsJPuyMMY6eBDegaFZv6umWZRukYAmfZJLS+PKlo=; b=ouN2O6yOl998aY/p+T4YJOIrr070d8iIWcVIS9IwMWXqj9CXj38m0t1dYLYqnitTP4 p3mb8EWVXuQbTTvQMouDmFTwaErR/hvChhbqiEo2fBAEASa3iFjk1e2zg/htmyoeMT+O XW3xVqXZFnGlw85INQmFzRWv+aY5LF/TGlA70= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type; b=APifkynyMkTRXtkz61CZEG0PpsEPNz4QYrMPg85YMlqnhCXjx/F/gifOj4pBV54PdQ FVRYYpYcmfO44S9+KB32uwpgS5egLngoYaMeLd2aVZYQ6OodLEqelShkGGjDITPBgfL6 s7Y3tXAhkRORwtlRvSNV85KZCrsPYQFaPvI2Y= MIME-Version: 1.0 Received: by 10.68.44.200 with SMTP id g8mr1643957pbm.362.1306686061383; Sun, 29 May 2011 09:21:01 -0700 (PDT) Received: by 10.68.40.41 with HTTP; Sun, 29 May 2011 09:21:01 -0700 (PDT) In-Reply-To: References: Date: Sun, 29 May 2011 17:21:01 +0100 X-Google-Sender-Auth: lhdq00OAVDkRysyQpnjZvBu1Wxg Message-ID: Subject: Re: MMC runtime PM patches break libertas probe From: Daniel Drake To: Ohad Ben-Cohen Cc: linux-mmc@vger.kernel.org Sender: linux-mmc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-mmc@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter2.kernel.org [140.211.167.43]); Sun, 29 May 2011 16:21:04 +0000 (UTC) Hi Ohad, On 31 October 2010 19:06, Ohad Ben-Cohen wrote: > OK, as expected. > > So to summarize: > 1. Card is powered up at boot, and successfully initialized > 2. After mmc + sdio devices are added to the device tree, power is > (seemingly) taken down by runtime PM > 3. When the driver is loaded, card is powered up again, but doesn't > respond to CMD3 > > The only explanation I can think of why the card doesn't respond to > CMD3, after its power is brought up again, is that we didn't have a > full reset here (i.e. mmc_power_off() didn't completely power off > everything). I have investigated this again, as we'd like runtime PM to work. It's certainly possible that there's something weird about the hardware in question, but we *are* able to successfully power down and up the card with a hacky rfkill driver that calls mmc_stop_host / mmc_start_host. So I went around finding out what the difference was between these functions and the runtime PM implementation. Through this comparison I think mmc_power_save_host() does almost exactly the same as mmc_stop_host() (good), but mmc_power_restore_host() lacks some steps which would otherwise be taken by mmc_start_host(). These are: in mmc_rescan_try_freq(): /* * sdio_reset sends CMD52 to reset card. Since we do not know * if the card is being re-initialized, just send it. CMD52 * should be ignored by SD/eMMC cards. */ sdio_reset(host); mmc_go_idle(host); mmc_send_if_cond(host, host->ocr_avail); In mmc_attach_sdio(): err = mmc_send_io_op_cond(host, 0, &ocr); if (err) return err; mmc_attach_bus(host, &mmc_sdio_ops); if (host->ocr_avail_sdio) host->ocr_avail = host->ocr_avail_sdio; /* * Sanity check the voltages that the card claims to * support. */ if (ocr & 0x7F) { printk(KERN_WARNING "%s: card claims to support voltages " "below the defined range. These will be ignored.\n", mmc_hostname(host)); ocr &= ~0x7F; } host->ocr = mmc_select_voltage(host, ocr); /* * Can we support the voltage(s) of the card(s)? */ if (!host->ocr) { err = -EINVAL; goto err; } Should anything in those code snippets be running during runtime PM resume? I went ahead and ran the obvious test by putting those bits of code in the runtime PM resume path... the first snippet doesn't seem to improve or hurt anything, but the second snippet fixes the problem. At least it means I can boot, the card gets powered down during boot, then I load the module and it powers up and initialises correctly. Patch attached for clarity. Any thoughts? Thanks, Daniel diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c index 4d0c15b..3b06379 100644 --- a/drivers/mmc/core/sdio.c +++ b/drivers/mmc/core/sdio.c @@ -691,15 +691,47 @@ static int mmc_sdio_resume(struct mmc_host *host) static int mmc_sdio_power_restore(struct mmc_host *host) { int ret; + u32 ocr; BUG_ON(!host); BUG_ON(!host->card); mmc_claim_host(host); + + ret = mmc_send_io_op_cond(host, 0, &ocr); + if (ret) + goto out; + + if (host->ocr_avail_sdio) + host->ocr_avail = host->ocr_avail_sdio; + + /* + * Sanity check the voltages that the card claims to + * support. + */ + if (ocr & 0x7F) { + printk(KERN_WARNING "%s: card claims to support voltages " + "below the defined range. These will be ignored.\n", + mmc_hostname(host)); + ocr &= ~0x7F; + } + + host->ocr = mmc_select_voltage(host, ocr); + + /* + * Can we support the voltage(s) of the card(s)? + */ + if (!host->ocr) { + ret = -EINVAL; + goto out; + } + ret = mmc_sdio_init_card(host, host->ocr, host->card, mmc_card_keep_power(host)); if (!ret && host->sdio_irqs) mmc_signal_sdio_irq(host); + +out: mmc_release_host(host); return ret;