On 2014-12-03 10:25, Lucas Stach wrote:
> Am Dienstag, den 02.12.2014, 17:47 +0100 schrieb Stefan Agner:
>> Enable IPG clock for sdio interrupts while runtime PM, since this
>> clock is needed for register access. The need of this clock has been
>> verified on Vybrid, but this is probably true for i.MX53 and maybe
>> others. The need for bus access during runtime suspend has been
>> introduced with be138554a792 ("mmc: sdhci: allow sdio interrupts
>> while sdhci runtime suspended").
>>
> Am I reading this wrong, or is this commit actually doing something
> different than what the commit message says?
>
> This does _not_ enable the IPG clock during runtime PM, in fact it is
> disabling it while suspending even with SDIO ints enabled, which is
> wrong, as no SDIO interrupts will be delivered with this clock disabled.
You are completely right. What I wanted was what is written in the
commit message, but I misinterpreted the code!
=> This patch is invalid.
> One thing that seems to be wrong with the current code is that the state
> of sdhci_sdio_irq_enabled could change between a runtime suspend and
> resume, so to keep the clocks balanced the driver has to remember if it
> disabled those two clocks in the suspend path and always enable them in
> that case in the resume path.
As far as I can see, the only place where that flag changes is in
sdhci_enable_sdio_irq. But this function makes sure runtime PM is off
while changing that, so I guess it's ok....
--
Stefan
>
>> Signed-off-by: Stefan Agner <stefan@xxxxxxxx>
>> ---
>> drivers/mmc/host/sdhci-esdhc-imx.c | 12 ++++++------
>> 1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
>> index 587ee0e..b7e9ad1 100644
>> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
>> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
>> @@ -1193,10 +1193,10 @@ static int sdhci_esdhc_runtime_suspend(struct device *dev)
>>
>> ret = sdhci_runtime_suspend_host(host);
>>
>> - if (!sdhci_sdio_irq_enabled(host)) {
>> + if (!sdhci_sdio_irq_enabled(host))
>> clk_disable_unprepare(imx_data->clk_per);
>> - clk_disable_unprepare(imx_data->clk_ipg);
>> - }
>> +
>> + clk_disable_unprepare(imx_data->clk_ipg);
>> clk_disable_unprepare(imx_data->clk_ahb);
>>
>> return ret;
>> @@ -1208,10 +1208,10 @@ static int sdhci_esdhc_runtime_resume(struct device *dev)
>> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> struct pltfm_imx_data *imx_data = pltfm_host->priv;
>>
>> - if (!sdhci_sdio_irq_enabled(host)) {
>> + if (!sdhci_sdio_irq_enabled(host))
>> clk_prepare_enable(imx_data->clk_per);
>> - clk_prepare_enable(imx_data->clk_ipg);
>> - }
>> +
>> + clk_prepare_enable(imx_data->clk_ipg);
>> clk_prepare_enable(imx_data->clk_ahb);
>>
>> return sdhci_runtime_resume_host(host);
--
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
