Hi Sudeep, On 五, 2月 17, 2017 at 10:28:00上午 +0000, Sudeep Holla wrote: > > > On 17/02/17 07:28, Chunyan Zhang wrote: > > Hi Sudeep, > > > > On 二, 2月 14, 2017 at 04:44:53下午 +0000, Sudeep Holla wrote: > >> On Tue, Feb 14, 2017 at 9:19 AM, Chunyan Zhang > >> <chunyan.zhang@xxxxxxxxxxxxxx> wrote: > > [..] > > >> > >>> + idle-states{ > >>> + entry-method = "arm,psci"; > >>> + > >>> + CORE_PD: core_pd { > >>> + compatible = "arm,idle-state"; > >>> + entry-latency-us = <1000>; > >>> + exit-latency-us = <700>; > >>> + min-residency-us = <2500>; > >>> + local-timer-stop; > >>> + arm,psci-suspend-param = <0x00010002>; > >>> + }; > >>> + > >>> + CLUSTER_PD: cluster_pd { > >>> + compatible = "arm,idle-state"; > >>> + entry-latency-us = <1000>; > >>> + exit-latency-us = <1000>; > >>> + min-residency-us = <3000>; > >>> + local-timer-stop; > >>> + arm,psci-suspend-param = <0x01010003>; > >>> + }; > >>> + > >>> + DEEP_SLEEP: deep_sleep { > >>> + compatible = "arm,idle-state"; > >>> + wakeup-latency-us = <0xffffffff>; > >> > >> A value > 4294 seconds(i.e >1 hour) seems suspicious. > >> Are you working around the firmware issue with high latency value so > >> that it's never entered ? Why not remove advertising the state from DT. > >> > > > > Haved checked with related colleagues, this node 'deep_sleep' was not for working > > around any firmware issue, but was a trick utilization of idle subsystem, and that > > Really ? Any latency greater few milliseconds are sounds useless. I > still don't understand what you mean by "trick utilization of idle > subsystem". > Sorry for confused expression, I meant it was not a right way to utilize idle mechanism and shouldn't be upstreamed. > > was definitely not elegant, the author indeed intendly didn't want CPU entered this > > state, I will remove this node therefore. > > It's quick and dirty "HACK* to retain and advertise the state but > ensure it's never entered and obstruct the boot. It's not a trick to > exploit any idle subsystem utilization. > Right, actually deep_sleep was for 'suspend' (forces idleness upon the OS until a wake-up event resumes the OS from suspend), for example when users press power key on mobile phone to turn off the screen. So the author implemented 'suspend' using cpu_psci_ops::cpu_suspend I figure that this way is not correct, I will remove this state from DT. I would appreciate any suggestion for how to implement this kind of function properly. Thanks, Chunyan > > > >> Can you get me the dump of: > >> grep "" /sys/devices/system/cpu/cpu*/cpuidle/state*/{time,usage} > >> > > > > FYI: https://www.irccloud.com/pastebin/XyEMLzfq/ > > > > As expected, state3(deep_sleep) is never entered. > > -- > Regards, > Sudeep -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html