lkml.org 
[lkml]   [2018]   [Oct]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] arm64: dts: sdm845: Add PSCI cpuidle low power states
From
Date


On 10/24/2018 12:06 AM, Doug Anderson wrote:
> Hi,
>
> On Mon, Oct 15, 2018 at 10:02 AM Raju P.L.S.S.S.N
> <rplsssn@codeaurora.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 {

Sure. I will change this. (However, from device tree specification v0.2,
I see that Table 2.1 mentions underscore as valid character for node.
Please correct me if I'm missing something)

>
>
>> + 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?

Agree. They are confusing. Will change this.

>
> 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.

Sure. Will change this.


Thanks for feedback Doug.

>
>
> -Doug
>

\
 
 \ /
  Last update: 2018-10-30 17:25    [W:0.972 / U:0.096 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site