On Wed, Mar 9, 2011 at 3:01 PM, Grant Grundler <grundler@xxxxxxxxxx> wrote:
> On Wed, Mar 9, 2011 at 12:24 PM, Andrei Warkentin <andreiw@xxxxxxxxxxxx> wrote:
>> On Tue, Mar 8, 2011 at 3:39 PM, Grant Grundler <grundler@xxxxxxxxxx> wrote:
>>>
>>> Save and restore SDHCI interrupt mask during suspend/resume.
>>> Enables ARM Tegra2 board to suspend/resume.
>>>
>>> Signed-off-by: Venkat Rao <vrao@xxxxxxxxxxxx>
>>> Reviewed-by: Olof Johansson <olofj@xxxxxxxxxxxx>
>>> Reviewed-by: Grant Grundler <grundler@xxxxxxxxxxxx>
>>> ---
>>> drivers/mmc/host/sdhci.c | 5 +++++
>>> include/linux/mmc/sdhci.h | 2 ++
>>> 2 files changed, 7 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>> index 3c6358f..68d2e36 100644
>>> --- a/drivers/mmc/host/sdhci.c
>>> +++ b/drivers/mmc/host/sdhci.c
>>> @@ -1654,6 +1654,8 @@ int sdhci_suspend_host(struct sdhci_host *host, pm_message_t state)
>>> if (mmc->card && (mmc->card->type != MMC_TYPE_SDIO))
>>> ret = mmc_suspend_host(host->mmc);
>>>
>>> + /* Save the original intmask to restore later */
>>> + host->save_intmask = sdhci_readl(host, SDHCI_INT_ENABLE);
>>> sdhci_mask_irqs(host, SDHCI_INT_ALL_MASK);
>>>
>>> if (host->vmmc)
>>> @@ -1695,6 +1697,9 @@ int sdhci_resume_host(struct sdhci_host *host)
>>>
>>> sdhci_enable_card_detection(host);
>>>
>>> + /* Restore the original intmask */
>>> + sdhci_unmask_irqs(host, host->save_intmask);
>>> +
>>> return ret;
>>> }
>>>
>>> diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
>>> index dfb2106..5b9f6db 100644
>>> --- a/include/linux/mmc/sdhci.h
>>> +++ b/include/linux/mmc/sdhci.h
>>> @@ -141,6 +141,8 @@ struct sdhci_host {
>>>
>>> unsigned int caps; /* Alternative capabilities */
>>>
>>> + unsigned int save_intmask; /* Store original intmask */
>>> +
>>> unsigned long private[0] ____cacheline_aligned;
>>> };
>>> #endif /* __SDHCI_H */
>>> --
>>> 1.7.2.3
>>>
>>> --
>>
>> Hi Grant,
>>
>> Why is this necessary?
>
> Hi Andrei,
> Sorry - I didn't CC the original author of the patch...(now CC'd) and
> hopefully he or Sandeep can explain.
>
>> If I understand correctly, the sdhci resume
>> sequence sets some base needed bits inside SDHCI_INT_ENABLE inside
>> sdhci_clear_set_irqs. Only if you have some outstanding DMA, but
>> surely all DMA must be completed prior to your suspending
>> (mmc_queue_suspend ensures nothing is outstanding)?
>
> That's a good question. Given this is a WIFI device, my initial
> assumption is we could receive packets or state/event changes any
> time. But the device might in fact be quiesced during the suspend
> operation. I have to check what the brcm80211/brcmfmac driver is doing
> in this case.
>
>> I'm asking since we're on Tegra 2 and have suspend/resume working fine
>> with eMMC.
>
> We do too - but only if the brcmfmac driver isn't loaded.
>
> I'll look up if there are any side effects of writing SDHCI_INT_ENABLE
> as well. It's possible this is unique to having BRCM4329 connected to
> the eMMC controller instead of a flash device.
>
> thanks,
> grant
>
+Dmitry Shmidt
Ah, interesting. So this is an SDIO device. We also have a BRCM4329
:-). Different driver, though, although I'm sure it's the same
Broadcom DHD stuff...
I took a look (yay web archives,
http://groups.google.com/a/chromium.org/group/chromium-os-reviews/browse_thread/thread/bfe4ece948222f96?pli=1),
and it seems you have SDHCI_QUIRK_NO_SDIO_IRQ disabled in your
sdhci-tegra. I see why above patch is necessary now.
This is bit OT, but are the LP2 idle gains significant here?
Thanks,
A
--
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
