"Madhusudhan" <madhu...@ti.com> 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 majord...@vger.kernel.org
>> 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to