Re: [PATCH 2/2] arm64: dts: rockchip: add eMMC's power domain support for rk3399

From: Ulf Hansson <ulf.hansson@linaro.org>
To: Ziyuan Xu <xzy.xu@rock-chips.com>
Cc: "Doug Anderson" <dianders@chromium.org>,
	"Shawn Lin" <shawn.lin@rock-chips.com>,
	"Heiko Stübner" <heiko@sntech.de>,
	"Mark Rutland" <mark.rutland@arm.com>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"Xing Zheng" <zhengxing@rock-chips.com>,
	"open list:ARM/Rockchip SoC..."
	<linux-rockchip@lists.infradead.org>,
	"Frank Wang" <frank.wang@rock-chips.com>,
	"Catalin Marinas" <catalin.marinas@arm.com>,
	"Elaine Zhang" <zhangqing@rock-chips.com>,
	"Will Deacon" <will.deacon@arm.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Brian Norris" <briannorris@chromium.org>,
	"Masahiro Yamada" <yamada.masahiro@socionext.com>,
	"Rob Herring" <robh+dt@kernel.org>,
	"David Wu" <david.wu@rock-chips.com>,
	"Caesar Wang" <wxt@rock-chips.com>,
	"Jianqun Xu" <jay.xu@rock-chips.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"Shunqian Zheng" <zhengsq@rock-chips.com>
Subject: Re: [PATCH 2/2] arm64: dts: rockchip: add eMMC's power domain support for rk3399
Date: Tue, 6 Sep 2016 14:34:53 +0200	[thread overview]
Message-ID: <CAPDyKFrAJW9D49wEgQXiuPFrgbVq6HjtQbTwj8FL6c_mx8o-wg@mail.gmail.com> (raw)
In-Reply-To: <10dfff58-88b2-a397-ef2e-9cd451253c40@rock-chips.com>

On 2 September 2016 at 16:23, Ziyuan Xu <xzy.xu@rock-chips.com> wrote:
> Hi Ulf,
>
>
> On 2016年09月02日 18:24, Ulf Hansson wrote:
>>
>> On 1 September 2016 at 23:50, Doug Anderson<dianders@chromium.org>  wrote:
>>>
>>> >Hi,
>>> >
>>> >On Thu, Sep 1, 2016 at 6:45 AM, Ulf Hansson<ulf.hansson@linaro.org>
>>> > wrote:
>>>>
>>>> >>I was reading the discussion regarding this change and browsing the DT
>>>> >>documentation around this... Can you guys explain what really goes on
>>>> >>here, please.
>>>> >>
>>>> >>To me, it seems like you are managing one device's resources in one
>>>> >>separate genpd. One genpd per device. Is that correct?
>>>> >>
>>>> >>Perhaps each device actually has its own PM domain and thus it makes
>>>> >>sense to assign one genpd per device?
>>>
>>> >
>>> >I'm not as familiar with genpd as I should be, so hopefully this makes
>>> > sense.
>>> >
>>> >...in hardware there is a "pd_emmc" that is the power domain for just
>>> >eMMC.  That will be referenced hooked up via device tree, like:
>>> >
>>> >power-domains = <&power RK3399_PD_EMMC>;
>>
>> Yes, I noticed that and this is what puzzles me a bit.
>>
>>> >
>>> >I believe that means that power will automatically be removed whenever
>>> >the device is runtime suspended or suspended.
>>
>> Well, it depends if the genpd has a subdomain or other devices in it
>> being runtime resumed.
>> The genpd will not power off unless all devices within it are runtime
>> enabled+suspended and that its subdomains are also powered off.
>>
>> So, in case you only have one device and no subdomains, then your
>> statement is correct.
>
>
> Yup, pd_emmc is a individual power domain which is only deployed to eMMC on
> rk3399. It has no subdomains.
>
>>
>>> >
>>> >If w're not supporting "autosuspend" and nobody is tweaking anything
>>
>> I guess you mean runtime PM autosuspend? Then why don't you support this?
>>
>> Wouldn't that allow you to avoid wasting power in runtime when the
>> device is idle?
>
>
> pd_emmc manages the sdhci controller, phy and corecfg_* stuff, if we support

Ohh, there are already mmc drivers dealing with phys. We can add this
for you driver as well.

Regarding corecfg_*, unless that also can be managed via a generic
interface (perhaps through syscon), then you need to manage that via
genpd. Is that the case?

> autosuspend in driver, we have to re-init context. I didn't test the

Yes, that's a common scenario, many device drivers do this - as to
avoid wasting power.

> latency, if it's acceptable, we will apply it.:-)

Latency constraints may be controlled via the genpd governor.

Moreover we also have the runtime PM autosuspend feature, which is
useful when you don't wont to runtime suspend the device between each
and every request. Thus avoiding latencies when there are a "burst" of
requests.

> But it's not a blocker, right?
>

Hmm, I just want to make sure we do the right things. Else it might
just bite us in the other end.

Again, my recommendation is to start with runtime PM then continue
with system PM. I can imagine you will get system PM "free" if using
the runtime PM centric approach.

Kind regards
Uffe