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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <10dfff58-88b2-a397-ef2e-9cd451253c40@rock-chips.com>
Date:   Fri, 2 Sep 2016 22:23:15 +0800
From:   Ziyuan Xu <xzy.xu@...k-chips.com>
To:     Ulf Hansson <ulf.hansson@...aro.org>,
        Doug Anderson <dianders@...omium.org>
Cc:     Shawn Lin <shawn.lin@...k-chips.com>,
        Heiko Stübner <heiko@...ech.de>,
        Mark Rutland <mark.rutland@....com>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        Xing Zheng <zhengxing@...k-chips.com>,
        "open list:ARM/Rockchip SoC..." <linux-rockchip@...ts.infradead.org>,
        Frank Wang <frank.wang@...k-chips.com>,
        Catalin Marinas <catalin.marinas@....com>,
        Elaine Zhang <zhangqing@...k-chips.com>,
        Will Deacon <will.deacon@....com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Brian Norris <briannorris@...omium.org>,
        Masahiro Yamada <yamada.masahiro@...ionext.com>,
        Rob Herring <robh+dt@...nel.org>,
        David Wu <david.wu@...k-chips.com>,
        Caesar Wang <wxt@...k-chips.com>,
        Jianqun Xu <jay.xu@...k-chips.com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        Shunqian Zheng <zhengsq@...k-chips.com>
Subject: Re: [PATCH 2/2] arm64: dts: rockchip: add eMMC's power domain support
 for rk3399

Hi Ulf,


On 2016年09月02日 18:24, Ulf Hansson wrote:
> On 1 September 2016 at 23:50, Doug Anderson<dianders@...omium.org>  wrote:
>> >Hi,
>> >
>> >On Thu, Sep 1, 2016 at 6:45 AM, Ulf Hansson<ulf.hansson@...aro.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 autosuspend in driver, we have to re-init context. I didn't test 
the latency, if it's acceptable, we will apply it.:-)
But it's not a blocker, right?

>
>> >manually, then it's possible (I think) that runtime suspend happens at
>> >exactly the same time as suspend.  ...but my point was that it was
> I am not sure I follow you here. You must not rely on that the device
> always becomes runtime suspended during system suspend, as there are
> no guarantees for this.
>
> Instead that is something you need to take care of in the
> subsystem/driver for the device, of course.
>
>> >cleaner to actually do it any restoring in the "runtime resume" hooks
>> >to match what genpd does.  This matches what you say: use runtime PM.
> Yes!
>
> Using genpd without deploying runtime PM for the devices doesn't make
> much sense, at least to me.
>
>> >
>> >...but it also sounds like it might not be terribly important to
>> >restore these values since they're a bit silly to begin with.  If
>> >that's true then I guess we don't need to do anything special here.
>> >
>> >
>> >Did that all make sense (it's entirely possible it didn't since
>> >somehow my brain still hasn't absorbed all runtime PM and genpd
>> >concepts)
> No worries. I understand this might be a bit tricky, that's why I also
> tries to help review related changes.
>
> Kind regards
> Uffe
>
>
>