On Thu, 14 May 2020 at 07:45, Jisheng Zhang <Jisheng.Zhang@xxxxxxxxxxxxx> wrote: > > On Wed, 13 May 2020 14:15:21 +0200 Ulf Hansson wrote: > > > > > > > On Wed, 13 May 2020 at 11:47, Jisheng Zhang <Jisheng.Zhang@xxxxxxxxxxxxx> wrote: > > > > > > This reverts commit a027b2c5fed78851e69fab395b02d127a7759fc7. > > > > > > The HW supports auto clock gating, so it's useless to do runtime pm > > > in software. > > > > Runtime PM isn't soley about clock gating. Moreover it manages the > > Per my understanding, current xenon rpm implementation is just clock gating. > > > "pltfm_host->clk", which means even if the controller supports auto > > clock gating, gating/ungating the externally provided clock still > > makes sense. > > clock ----------- xenon IP > |___ rpm |__ HW Auto clock gate > > Per my understanding, with rpm, both clock and IP is clock gated; while with > Auto clock gate, the IP is clock gated. So the only difference is clock itself. > Considering the gain(suspect we have power consumption gain, see below), the > pay -- 56 LoCs and latency doesn't deserve gain. > > Even if considering from power consumption POV, sdhci_runtime_suspend_host(), > sdhci_runtime_resume_host(), and the retune process could be more than the clock > itself. Right. The re-tune may be costly, yes. However, whether the re-tune is *really* needed actually varies depending on the sdhci variant and the SoC. Additionally, re-tune isn't done for all types of (e)MMC/SD/SDIO cards. I see a few options that you can explore. 1. There is no requirement to call sdhci_runtime_suspend|resume_host() from sdhci-xenon's ->runtime_suspend|resume() callbacks - if that's not explicitly needed. The point is, you can do other things there, that suits your variant/SoC better. 2. Perhaps for embedded eMMCs, with a non-removable slot, the re-tuning is costly. If you want to prevent the device from entering runtime suspend for that slot, for example, just do an additional pm_runtime_get_noresume() during ->probe(). [...] Kind regards Uffe