[PATCH v2 0/9] Init runtime PM support for dw_mmc

[Ulf Hansson]

[...]

>>
>> Now, what also do puzzles me, is exactly that piece of code in
>> dw_mci_resume() that is being executed *when* the pm_flag contains
>> MMC_PM_KEEP_POWER:
>> if (slot->mmc->pm_flags & MMC_PM_KEEP_POWER) {
>>      dw_mci_set_ios(slot->mmc, &slot->mmc->ios);
>>      dw_mci_setup_bus(slot, true);
>> }
>
> There is no place to set this in dw_mmc controller..Maybe it should be located in other vendor driver.
> Almost all cards that used SDIO might be WiFi..Of course..there might be other cards.
> e.g) In exnyos, use the sdio as WiFi card(broadcom chipset) then broadcom driver also used..
> When broadcom driver initialized or suspending time, it should be set to this flag..
> As i mentioned..it's not normal case.
>
>>
>> So, in the system resume path, the above do makes sense as you need to
>> restore the registers etc for the dw_mmc controller to enable it to
>> operate the SDIO card. Such as bus width, clocks, etc.
>>
>> Although, I would expect something similar would be needed in the new
>> runtime resume path as well. And in particular also for eMMC/SD cards,
>> as you need to restore the dw_mmc registers to be able to operate the
>> card again. Don't you?
>
> Need to check :)
>
>>
>> So in the end, perhaps you should *always* call dw_mci_set_ios() and
>> dw_mci_setup_bus() in dw_mci_resume() instead of conditionally check
>> for MMC_PM_KEEP_POWER? Or maybe only a subset of those functions?
>
> It's reasonable..but it also needs to check on real case and stress test.

Yes, and we can try out that later on top of this series.

Anyway, I wanted to highlight my concern, as I think it's only a
matter of time before we needed this on for runtime PM for some SoCs
that uses dw_mmc.

Kind regards
Uffe