From: "Huang, Tao" <huangtao-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
To: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Cc: lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Subject: Re: [PATCH] arm64: dts: rockchip: remove wrongly added idle states on rk3368
Date: Tue, 7 Mar 2017 15:20:28 +0800 [thread overview]
Message-ID: <9e79905e-8dff-2980-ab8d-9cfd880275c0@rock-chips.com> (raw)
In-Reply-To: <20170301223849.13401-1-heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
Hi Heiko:
On 2017年03月02日 06:38, Heiko Stuebner wrote:
> As reported by Lorenzo, the residency/latency values defined in the
> idle-state for rk3368 "make no sense". When introducing them I
> simply took the idle-state node from the vendor kernel in error
> as I didn't look up if these values were sane in the first place.
>
> Talking to people and determining why they were used in this way
> showed that it was meant to make sure the cpu_suspend callback
> got initialized which at the 3.10 time was somehow required even
> for wfi-based idle handling.
>
> Of course the generic arch_cpu_idle() now does wfi-based idle-handling
> already, so there is no need for this.
>
> Reported-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
> This has been sitting ere for way to long :-(, but finally I got
> to finish the change.
>
> arch/arm64/boot/dts/rockchip/rk3368.dtsi | 20 --------------------
> 1 file changed, 20 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3368.dtsi b/arch/arm64/boot/dts/rockchip/rk3368.dtsi
> index 49d1191..5471ace 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3368.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3368.dtsi
> @@ -107,23 +107,10 @@
> };
> };
>
> - idle-states {
> - entry-method = "psci";
> -
> - cpu_sleep: cpu-sleep-0 {
> - compatible = "arm,idle-state";
> - arm,psci-suspend-param = <0x1010000>;
> - entry-latency-us = <0x3fffffff>;
> - exit-latency-us = <0x40000000>;
> - min-residency-us = <0xffffffff>;
> - };
> - };
> -
> cpu_l0: cpu@0 {
> device_type = "cpu";
> compatible = "arm,cortex-a53", "arm,armv8";
> reg = <0x0 0x0>;
> - cpu-idle-states = <&cpu_sleep>;
> enable-method = "psci";
>
> #cooling-cells = <2>; /* min followed by max */
> @@ -133,7 +120,6 @@
> device_type = "cpu";
> compatible = "arm,cortex-a53", "arm,armv8";
> reg = <0x0 0x1>;
> - cpu-idle-states = <&cpu_sleep>;
> enable-method = "psci";
> };
>
> @@ -141,7 +127,6 @@
> device_type = "cpu";
> compatible = "arm,cortex-a53", "arm,armv8";
> reg = <0x0 0x2>;
> - cpu-idle-states = <&cpu_sleep>;
> enable-method = "psci";
> };
>
> @@ -149,7 +134,6 @@
> device_type = "cpu";
> compatible = "arm,cortex-a53", "arm,armv8";
> reg = <0x0 0x3>;
> - cpu-idle-states = <&cpu_sleep>;
> enable-method = "psci";
> };
>
> @@ -157,7 +141,6 @@
> device_type = "cpu";
> compatible = "arm,cortex-a53", "arm,armv8";
> reg = <0x0 0x100>;
> - cpu-idle-states = <&cpu_sleep>;
> enable-method = "psci";
>
> #cooling-cells = <2>; /* min followed by max */
> @@ -167,7 +150,6 @@
> device_type = "cpu";
> compatible = "arm,cortex-a53", "arm,armv8";
> reg = <0x0 0x101>;
> - cpu-idle-states = <&cpu_sleep>;
> enable-method = "psci";
> };
>
> @@ -175,7 +157,6 @@
> device_type = "cpu";
> compatible = "arm,cortex-a53", "arm,armv8";
> reg = <0x0 0x102>;
> - cpu-idle-states = <&cpu_sleep>;
> enable-method = "psci";
> };
>
> @@ -183,7 +164,6 @@
> device_type = "cpu";
> compatible = "arm,cortex-a53", "arm,armv8";
> reg = <0x0 0x103>;
> - cpu-idle-states = <&cpu_sleep>;
> enable-method = "psci";
> };
> };
Looks good to me.
There are another redundant node - timer@ff810000, which can not probe
successfully because of missing clocks property. RK3368 don't support
power down core while idle, so there are not need add broadcast timer.
Should we delete the timer node or add missing clocks?
If add clocks, we should fix clk drivers first because timer clk is not
exported.
I suggest we should delete the timer node.
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
next prev parent reply other threads:[~2017-03-07 7:20 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-01 22:38 [PATCH] arm64: dts: rockchip: remove wrongly added idle states on rk3368 Heiko Stuebner
[not found] ` <20170301223849.13401-1-heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
2017-03-07 7:20 ` Huang, Tao [this message]
[not found] ` <9e79905e-8dff-2980-ab8d-9cfd880275c0-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
2017-03-07 11:06 ` Heiko Stübner
2017-03-16 13:21 ` Heiko Stuebner
2017-03-07 8:36 ` Lorenzo Pieralisi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9e79905e-8dff-2980-ab8d-9cfd880275c0@rock-chips.com \
--to=huangtao-tnx95d0mmh7dzftrwevzcw@public.gmane.org \
--cc=heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org \
--cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
--cc=linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
--cc=lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox