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 -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
