Re: [PATCH] Revert "mmc: sdhci-xenon: add runtime pm support and reimplement standby"

[Ulf Hansson <ulf.hansson@linaro.org>]

On Thu, 14 May 2020 at 07:45, Jisheng Zhang <Jisheng.Zhang@synaptics.com> 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@synaptics.com> 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