linux-kernel - Re: [PATCH] arm64: dts: sdm845: Add PSCI cpuidle low power states

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]

Date:   Tue, 23 Oct 2018 11:36:03 -0700
From:   Doug Anderson <dianders@...omium.org>
To:     Raju P L S S S N <rplsssn@...eaurora.org>
Cc:     Andy Gross <andy.gross@...aro.org>,
        David Brown <david.brown@...aro.org>,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        "open list:ARM/QUALCOMM SUPPORT" <linux-soc@...r.kernel.org>,
        Rajendra Nayak <rnayak@...eaurora.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        LKML <linux-kernel@...r.kernel.org>, linux-pm@...r.kernel.org,
        devicetree@...r.kernel.org, Stephen Boyd <sboyd@...nel.org>,
        Evan Green <evgreen@...omium.org>,
        Matthias Kaehlcke <mka@...omium.org>,
        Lina Iyer <ilina@...eaurora.org>
Subject: Re: [PATCH] arm64: dts: sdm845: Add PSCI cpuidle low power states

Hi,

On Mon, Oct 15, 2018 at 10:02 AM Raju P.L.S.S.S.N
<rplsssn@...eaurora.org> wrote:
> +               idle-states {
> +                       entry-method = "psci";
> +
> +                       C0_CPU_SPC: c0_spc {

nit: all these nodes should have dashes instead of spaces in the node
names (labels can still have spaces).  AKA:

C0_CPU_SPC: c0-spc {

> +                               compatible = "arm,idle-state";
> +                               arm,psci-suspend-param = <0x40000003>;
> +                               entry-latency-us = <350>;
> +                               exit-latency-us = <461>;
> +                               min-residency-us = <1890>;
> +                               local-timer-stop;
> +                               idle-state-name = "pc";

It seems weird that the idle state with the node name "spc" has the
name "pc" and the idle state with the node name "pc" has the name
"rail pc".  Can this be more consistent or is there a reason why they
need to be mismatched?

Also: AAHTUFSWDKWTM (acronyms are hard to understand for someone who
doesn't know what they mean).  If you really need to use an the
acronyms "PC" and "SPC" please document them somewhere.  In the very
least the commit message, but having a comment in the file is good
too.  ...or (even better) don't use the acronym and spell out what
you're talking about.  Please correct me if I'm wrong, but I don't
think it's obvious what the "PC" and "SPC" idle states mean.

-Doug