"Madhusudhan" <madhu.cr@xxxxxx> writes:
> <snip>
>
>> err_irq:
>> mmc_host_disable(host->mmc);
>> - clk_disable(host->iclk);
>> + pm_runtime_suspend(host->dev);
>
> Why not pm_runtime_put_sync() here? It can replace the calls:
> mmc_host_disable(host->mmc);
> clk_disable(host->iclk);
Not knowing a ton about this driver, I'd rather keep the
mmc_host_disable() since it matches the mmc_host_enable() earlier.
>> clk_put(host->fclk);
>> clk_put(host->iclk);
>> +
>> if (host->got_dbclk) {
>> clk_disable(host->dbclk);
>> clk_put(host->dbclk);
>> @@ -2216,7 +2164,8 @@ static int omap_hsmmc_remove(struct platform_device
>> *pdev)
>> flush_scheduled_work();
>>
>> mmc_host_disable(host->mmc);
>> - clk_disable(host->iclk);
>> + pm_runtime_suspend(host->dev);
>> +
>
> Ditto
>
>
>> clk_put(host->fclk);
>> clk_put(host->iclk);
>> if (host->got_dbclk) {
>> @@ -2272,7 +2221,7 @@ static int omap_hsmmc_suspend(struct device *dev)
>> OMAP_HSMMC_WRITE(host->base, HCTL,
>> OMAP_HSMMC_READ(host->base, HCTL) & ~SDBP);
>> mmc_host_disable(host->mmc);
>> - clk_disable(host->iclk);
>> +
>> if (host->got_dbclk)
>> clk_disable(host->dbclk);
>> } else {
>> @@ -2287,6 +2236,11 @@ static int omap_hsmmc_suspend(struct device *dev)
>> mmc_host_disable(host->mmc);
>> }
>>
>> + /*
>> + * HACK: "extra" put to compensate for DPM core keeping
>> + * runtime PM disabled. -- khilman
>> + */
>> + pm_runtime_put_sync(host->dev);
>> }
>> return ret;
>> }
>> @@ -2302,12 +2256,14 @@ static int omap_hsmmc_resume(struct device *dev)
>> return 0;
>>
>> if (host) {
>> - ret = clk_enable(host->iclk);
>> - if (ret)
>> - goto clk_en_err;
>> + /*
>> + * HACK: "extra" get to compensate for DPM core keeping
>> + * runtime PM disabled. -- khilman
>> + */
>> + pm_runtime_get_sync(host->dev);
>>
>> if (mmc_host_enable(host->mmc) != 0) {
>> - clk_disable(host->iclk);
>> + pm_runtime_suspend(host->dev);
>> goto clk_en_err;
>> }
>>
>> @@ -2346,9 +2302,37 @@ clk_en_err:
>> #define omap_hsmmc_resume NULL
>> #endif
>>
>> +/* called just before device is disabled */
>> +static int omap_hsmmc_runtime_suspend(struct device *dev)
>> +{
>> + struct omap_hsmmc_host *host;
>> +
>> + dev_dbg(dev, "%s\n", __func__);
>> +
>> + host = platform_get_drvdata(to_platform_device(dev));
>> + omap_hsmmc_context_save(host);
>
> The context_save fn is now empty. How does it help here?
It doesn't add anything, but I put it there to show that this is the
only place that context save would be needed, and as an example to other
drivers.
Kevin
>> +
>> + return 0;
>> +}
>> +
>> +/* called after device is (re)enabled, ONLY if context was lost */
>> +static int omap_hsmmc_runtime_resume(struct device *dev)
>> +{
>> + struct omap_hsmmc_host *host;
>> +
>> + dev_dbg(dev, "%s\n", __func__);
>> +
>> + host = platform_get_drvdata(to_platform_device(dev));
>> + omap_hsmmc_context_restore(host);
>> +
>> + return 0;
>> +}
>> +
>> static struct dev_pm_ops omap_hsmmc_dev_pm_ops = {
>> .suspend = omap_hsmmc_suspend,
>> .resume = omap_hsmmc_resume,
>> + .runtime_suspend = omap_hsmmc_runtime_suspend,
>> + .runtime_resume = omap_hsmmc_runtime_resume,
>> };
>>
>> static struct platform_driver omap_hsmmc_driver = {
>> --
>> 1.6.6
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
