Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754655AbdDMPF6 (ORCPT ); Thu, 13 Apr 2017 11:05:58 -0400 Received: from mail-wm0-f41.google.com ([74.125.82.41]:36325 "EHLO mail-wm0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754007AbdDMPFz (ORCPT ); Thu, 13 Apr 2017 11:05:55 -0400 MIME-Version: 1.0 In-Reply-To: References: <20170411225543.987-1-dianders@chromium.org> From: Doug Anderson Date: Thu, 13 Apr 2017 08:05:52 -0700 X-Google-Sender-Auth: qYQolo-BZda0puibI_11Uej-TqA Message-ID: Subject: Re: [PATCH] mmc: dw_mmc: Don't allow Runtime PM for SDIO cards To: Ulf Hansson Cc: Jaehoon Chung , Brian Norris , "open list:ARM/Rockchip SoC..." , Shawn Lin , Heiko Stuebner , kevin@archlinuxarm.org, Alexandru Stan , Ziyuan Xu , "# 4.0+" , "linux-mmc@vger.kernel.org" , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3027 Lines: 65 Hi, On Thu, Apr 13, 2017 at 2:32 AM, Ulf Hansson wrote: > On 12 April 2017 at 00:55, Douglas Anderson wrote: >> According to the SDIO standard interrupts are normally signalled in a >> very complicated way. They require the card clock to be running and >> require the controller to be paying close attention to the signals >> coming from the card. This simply can't happen with the clock stopped >> or with the controller in a low power mode. > > Right - unless we have a possibility to re-route the SDIO irq line to > GPIO irq for wake-up when the controller enters low power state. Yup. Hence the "normally signalled". ;) >> To that end, we'll disable runtime_pm when we detect that an SDIO card >> was inserted. This is much like with what we do with the special >> "SDMMC_CLKEN_LOW_PWR" bit that dw_mmc supports. > > I wonder whether this is related to SDIO IRQs but not generic to SDIO. > > For example, when we have a WIFI chip with a dedicated and separate > IRQ line, enabling SDMMC_CLKEN_LOW_PWR could make sense. Yes, I believe this to be the case too. It has always been on my TODO list to see if I could figure out how to make this work, but it never rose to the top. One thing I wasn't sure about: is SDIO truly stateless enough that you could fully power down the controller while you were waiting for an interrupt? I've never dug down that far into the protocol. For certain, though, you wouldn't want to remove VMMC from an SDIO card while waiting for an interrupt. This contrasts to an SD or eMMC memory card where you could plausibly do that if you're not actively reading from or writing to the card (though I think some cards may do background garbage collection, so maybe?? you could interfere with that?) >> To fix the above isues we'd want to put something in the standard >> sdio_irq code that makes sure to call pm_runtime get/put when >> interrupts are being actively being processed. That's possible to do, >> but it seems like a more complicated mechanism when we really just >> want the runtime pm disabled always for SDIO cards given that all the >> other bits needed to get Runtime PM vs. SDIO just aren't there. >> >> NOTE: at some point in time someone might come up with a fancy way to >> do SDIO interrupts and still allow (some) amount of runtime PM. >> Technically we could turn off the card clock if we used an alternate >> way of signaling SDIO interrupts (and out of band interrupt is one way >> to do this). We probably wouldn't actually want to fully runtime >> suspend in this case though--at least not with the current >> dw_mci_runtime_resume() which basically fully resets the controller at >> resume time. > > I understand this "quick" fix in dw_mmc takes care of the problem. > However, allow me to try to make this a bit more generic. I intend to > post something within a few days. If I fail, let's go with this > solution. Most certainly! If you can come up with something better then I'd be happy! -Doug