On 21 March 2017 at 13:34, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote:
> On 21/03/17 12:00, Ulf Hansson wrote:
>> On 15 March 2017 at 17:31, Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
>>> SDIO cards may need clock to send the card interrupt to the host.
>>>
>>> On a cherrytrail tablet with a RTL8723BS wifi chip, without this patch
>>> pinging the tablet results in:
>>>
>>> PING 192.168.1.14 (192.168.1.14) 56(84) bytes of data.
>>> 64 bytes from 192.168.1.14: icmp_seq=1 ttl=64 time=78.6 ms
>>> 64 bytes from 192.168.1.14: icmp_seq=2 ttl=64 time=1760 ms
>>> 64 bytes from 192.168.1.14: icmp_seq=3 ttl=64 time=753 ms
>>> 64 bytes from 192.168.1.14: icmp_seq=4 ttl=64 time=3.88 ms
>>> 64 bytes from 192.168.1.14: icmp_seq=5 ttl=64 time=795 ms
>>> 64 bytes from 192.168.1.14: icmp_seq=6 ttl=64 time=1841 ms
>>> 64 bytes from 192.168.1.14: icmp_seq=7 ttl=64 time=810 ms
>>> 64 bytes from 192.168.1.14: icmp_seq=8 ttl=64 time=1860 ms
>>> 64 bytes from 192.168.1.14: icmp_seq=9 ttl=64 time=812 ms
>>> 64 bytes from 192.168.1.14: icmp_seq=10 ttl=64 time=48.6 ms
>>>
>>> Where as with this patch I get:
>>>
>>> PING 192.168.1.14 (192.168.1.14) 56(84) bytes of data.
>>> 64 bytes from 192.168.1.14: icmp_seq=1 ttl=64 time=3.96 ms
>>> 64 bytes from 192.168.1.14: icmp_seq=2 ttl=64 time=1.97 ms
>>> 64 bytes from 192.168.1.14: icmp_seq=3 ttl=64 time=17.2 ms
>>> 64 bytes from 192.168.1.14: icmp_seq=4 ttl=64 time=2.46 ms
>>> 64 bytes from 192.168.1.14: icmp_seq=5 ttl=64 time=2.83 ms
>>> 64 bytes from 192.168.1.14: icmp_seq=6 ttl=64 time=1.40 ms
>>> 64 bytes from 192.168.1.14: icmp_seq=7 ttl=64 time=2.10 ms
>>> 64 bytes from 192.168.1.14: icmp_seq=8 ttl=64 time=1.40 ms
>>> 64 bytes from 192.168.1.14: icmp_seq=9 ttl=64 time=2.04 ms
>>> 64 bytes from 192.168.1.14: icmp_seq=10 ttl=64 time=1.40 ms
>>>
>>> Cc: Dong Aisheng <b29396@xxxxxxxxxxxxx>
>>> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
>>> ---
>>> Changes in v2:
>>> -New patch replacing "mmc: sdhci: sdio-intel: Set
>>> SDHCI_QUIRK2_CARD_ON_NEEDS_BUS_ON quirk"
>>> ---
>>> drivers/mmc/host/sdhci-acpi.c | 1 +
>>> drivers/mmc/host/sdhci-pci-core.c | 1 +
>>> drivers/mmc/host/sdhci.c | 6 ++++++
>>> drivers/mmc/host/sdhci.h | 2 ++
>>> 4 files changed, 10 insertions(+)
>>>
>>> diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
>>> index 237f318..53b431a 100644
>>> --- a/drivers/mmc/host/sdhci-acpi.c
>>> +++ b/drivers/mmc/host/sdhci-acpi.c
>>> @@ -288,6 +288,7 @@ static const struct sdhci_acpi_slot sdhci_acpi_slot_int_emmc = {
>>> static const struct sdhci_acpi_slot sdhci_acpi_slot_int_sdio = {
>>> .quirks = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC,
>>> .quirks2 = SDHCI_QUIRK2_HOST_OFF_CARD_ON |
>>> + SDHCI_QUIRK2_SDIO_IRQ_NO_RUNTIME_PM |
>>> SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
>>> .caps = MMC_CAP_NONREMOVABLE | MMC_CAP_POWER_OFF_CARD |
>>> MMC_CAP_WAIT_WHILE_BUSY,
>>> diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
>>> index 982b3e3..a3b242e 100644
>>> --- a/drivers/mmc/host/sdhci-pci-core.c
>>> +++ b/drivers/mmc/host/sdhci-pci-core.c
>>> @@ -498,6 +498,7 @@ static const struct sdhci_pci_fixes sdhci_ni_byt_sdio = {
>>> static const struct sdhci_pci_fixes sdhci_intel_byt_sdio = {
>>> .quirks = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC,
>>> .quirks2 = SDHCI_QUIRK2_HOST_OFF_CARD_ON |
>>> + SDHCI_QUIRK2_SDIO_IRQ_NO_RUNTIME_PM |
>>> SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
>>> .allow_runtime_pm = true,
>>> .probe_slot = byt_sdio_probe_slot,
>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>> index 6fdd7a7..59c5cad 100644
>>> --- a/drivers/mmc/host/sdhci.c
>>> +++ b/drivers/mmc/host/sdhci.c
>>> @@ -1828,6 +1828,9 @@ static void sdhci_enable_sdio_irq(struct mmc_host *mmc, int enable)
>>> struct sdhci_host *host = mmc_priv(mmc);
>>> unsigned long flags;
>>>
>>> + if ((host->quirks2 & SDHCI_QUIRK2_SDIO_IRQ_NO_RUNTIME_PM) && enable)
>>> + pm_runtime_get(host->mmc->parent);
>>
>> There are currently no sdhci variant for any SoC that implements
>> wakeup support in runtime suspend for SDIO irqs. In other words, it
>> seems all sdhci hosts suffers from the same problem.
>
> There might still be drivers that expect SDIO interrupts during
> runtime suspend, considering this patch:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=be138554a7923658ded799b0e8794d9c1d08a6e5
Right. Thanks for pointing it out.
However I don't think that is how runtime PM should be deployed when
it is used for controlling devices like sdhci. Let's change that once
we have the common issue for sdhci and SDIO irqs resolved.
[...]
Kind regards
Uffe
--
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
