From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752378AbcBLMEV (ORCPT ); Fri, 12 Feb 2016 07:04:21 -0500 Received: from eusmtp01.atmel.com ([212.144.249.242]:13827 "EHLO eusmtp01.atmel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752174AbcBLMET (ORCPT ); Fri, 12 Feb 2016 07:04:19 -0500 Date: Fri, 12 Feb 2016 13:04:15 +0100 From: Ludovic Desroches To: Ulf Hansson CC: linux-mmc , "linux-kernel@vger.kernel.org" , Nicolas Ferre , Adrian Hunter , Ludovic Desroches Subject: Re: [PATCH] mmc: sdhci-of-at91: fix card detect when using runtime PM Message-ID: <20160212120415.GJ14937@odux.rfo.atmel.com> Mail-Followup-To: Ulf Hansson , linux-mmc , "linux-kernel@vger.kernel.org" , Nicolas Ferre , Adrian Hunter References: <1455198537-21791-1-git-send-email-ludovic.desroches@atmel.com> <20160212083828.GI14937@odux.rfo.atmel.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Feb 12, 2016 at 12:01:39PM +0100, Ulf Hansson wrote: > >> > >> According to the below commit, SDHCI_QUIRK_BROKEN_CARD_DETECTION was > >> invented because of unreliable card detection mechanism inside the > >> sdhci controller. > >> Therefore it required polling to be used, but also to make ->get_cd() > >> to always return 1 in these cases. > >> > >> Although, as I understand it that's not the case here. You can still > >> rely on card detection to work, but as you don't have wakeups you > >> can't fully make use of card detect, when combined with runtime PM. > >> I am not sure we should add more users of > >> SDHCI_QUIRK_BROKEN_CARD_DETECTION, especially since in this case it's > >> not reflecting the capability of the hardware. > >> > >> Can't we think of another way? > > > > Sorry but I am not sure to understand. In the previous thread, you told > > me to use MMC_CAP_NEEDS_POLL which is set if we have > > SDHCI_QUIRK_BROKEN_CARD_DETECTION. I was not confortable to do this > > because as you say it is not reflecting the capability of the hardware. > > > > Do you mean that I can simply add MMC_CAP_NEEDS_POLL after sdhci_add_host()? > > Yes, something like that, but... > > Within this context, I realize that the DT binding "broken-cd" has two > different meanings, while comparing the generic MMC bindings towards > SDHCI's. That's bad. > > In the SDHCI case it means, enable MMC_CAP_NEEDS_POLL *and* make > ->get_cd() to always return 1 (via adding > SDHCI_QUIRK_BROKEN_CARD_DETECTION). > > In the generic MMC case, it means only to enable MMC_CAP_NEEDS_POLL, > which is exactly what you want. > > Perhaps you wonder why I think it's a good good idea to use DT to > decide if MMC_CAP_NEEDS_POLL should be enabled? > It allows flexibility for future platforms. For example, there may be > platforms adding GPIO card detect support or even cards that's > non-removable. I agree. > > I realize that the fix to solve this regression would then mean that > sdhci-of-at91 need to clear SDHCI_QUIRK_BROKEN_CARD_DETECTION after > parsing the shdci DTB, but then the DTB for your platform also needs > an update as the "broken-cd" options needs to be set. > > Do you think this can work? > It should but as SDHCI_QUIRK_BROKEN_CARD_DETECTION, broken-cd is not reflecting the capability of the hardware. I was thinking about checking non-removable and using mmc_gpio_get_cd() in my runtime_suspend callback. If I have a non removable device or a gpio for card detection then I can disable all clocks and call sdhci_runtime_suspend_host(). If not, I keep enabled the clock for the 'interface', disable the other one and that's all. The controller won't be set as runtime suspended but I would save some power. Does it sounds good? Regards Ludovic