Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200513174706.3eeddb2b@xhacker.debian> <CAPDyKFpE_uqiNQ22Fq9hDfb5pzMBdgmwgUbasEsEdXFkEOmq6A@mail.gmail.com>
 <20200514134507.54c17936@xhacker.debian>
In-Reply-To: <20200514134507.54c17936@xhacker.debian>
From:   Ulf Hansson <ulf.hansson@linaro.org>
Date:   Thu, 14 May 2020 12:18:58 +0200
Message-ID: <CAPDyKFpUv=HGBAEchH25tdnRdUSAvbCgGGCgN8uuvPCQ92xwZg@mail.gmail.com>
Subject: Re: [PATCH] Revert "mmc: sdhci-xenon: add runtime pm support and
 reimplement standby"
To:     Jisheng Zhang <Jisheng.Zhang@synaptics.com>
Cc:     Adrian Hunter <adrian.hunter@intel.com>,
        "linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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