[RESEND PATCH] ARM: dts: make arch-timer always on in rk3288 soc

We need use the hrtimer, which need the arch-timer to be 'always-on'

Signed-off-by: Kever Yang <redacted>
---

 arch/arm/boot/dts/rk3288.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index 5950b0a..698e6ea 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi

@@ -76,6 +76,7 @@
 			     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
 			     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
 		clock-frequency = <24000000>;
+		always-on;
 	};
 
 	i2c1: i2c at ff140000 {

-- 
1.9.1

Hi Kever,

On Thu, Aug 28, 2014 at 02:40:17AM +0100, Kever Yang wrote:

We need use the hrtimer, which need the arch-timer to be 'always-on'

I asked a question on the last posting [1]. Can you please confirm
either way?

Thanks,
Mark.

[1] lists.infradead.org/pipermail/linux-arm-kernel/2014-August/282327.html

On Thu, Aug 28, 2014 at 10:17:58AM +0100, Mark Rutland wrote:

Hi Kever,

On Thu, Aug 28, 2014 at 02:40:17AM +0100, Kever Yang wrote:

quoted

We need use the hrtimer, which need the arch-timer to be 'always-on'

I asked a question on the last posting [1]. Can you please confirm
either way?

Thanks,
Mark.

[1] lists.infradead.org/pipermail/linux-arm-kernel/2014-August/282327.html

To clarify: if there are low power states that the CPU can enter where
we lose state, then this patch isn't correct.

A more general approach would be to enable the broadcast hrtimer for
arm, as has been done for arm64.

See commit 5d1638acb9f6 (tick: Introduce hrtimer based broadcast) which
introduced the broadcast hrtimer, and commit 9358d755bd5c (arm64:
kernel: initialize broadcast hrtimer based clock event device) which
added the requisite plumbing for arm64.

Mark.

quoted

Signed-off-by: Kever Yang <redacted>
---

 arch/arm/boot/dts/rk3288.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index 5950b0a..698e6ea 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi

@@ -76,6 +76,7 @@
 			     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
 			     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
 		clock-frequency = <24000000>;
+		always-on;
 	};
 
 	i2c1: i2c at ff140000 {

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Mark,

     Thanks for your reply and advice.

On 08/28/2014 11:11 PM, Mark Rutland wrote:

On Thu, Aug 28, 2014 at 10:17:58AM +0100, Mark Rutland wrote:

quoted

Hi Kever,

On Thu, Aug 28, 2014 at 02:40:17AM +0100, Kever Yang wrote:

quoted

We need use the hrtimer, which need the arch-timer to be 'always-on'

I asked a question on the last posting [1]. Can you please confirm
either way?

Thanks,
Mark.

[1] lists.infradead.org/pipermail/linux-arm-kernel/2014-August/282327.html

To clarify: if there are low power states that the CPU can enter where
we lose state, then this patch isn't correct.

rk3288 has low power state and may turn off the cpu power domain
which will lost any logic state in cpu.

A more general approach would be to enable the broadcast hrtimer for
arm, as has been done for arm64.

See commit 5d1638acb9f6 (tick: Introduce hrtimer based broadcast) which
introduced the broadcast hrtimer, and commit 9358d755bd5c (arm64:
kernel: initialize broadcast hrtimer based clock event device) which
added the requisite plumbing for arm64.

I'll going to implement this and send another patch.

Thanks.

-Kever

Hi, Mark:

? 2014?08?28? 23:11, Mark Rutland ??:

To clarify: if there are low power states that the CPU can enter where
we lose state, then this patch isn't correct.

Right now, the software of RK3288 SoC only support CPU hotplug
(cpu_on/off) and power off all CPUs on suspend.
We do not implement cpuidle to power off CPU. Do you think we should
introduce a broadcast timer?
On our early kernel, I never see any interrupt on a broadcast timer
(yes, we implement it with a external timer).

A more general approach would be to enable the broadcast hrtimer for
arm, as has been done for arm64.

Yes. I think it should be done by arm framework.

See commit 5d1638acb9f6 (tick: Introduce hrtimer based broadcast) which
introduced the broadcast hrtimer, and commit 9358d755bd5c (arm64:
kernel: initialize broadcast hrtimer based clock event device) which
added the requisite plumbing for arm64.

On Fri, Aug 29, 2014 at 04:06:40AM +0100, Huang Tao wrote:

Hi, Mark:

Hi,

? 2014?08?28? 23:11, Mark Rutland ??:

quoted

To clarify: if there are low power states that the CPU can enter where
we lose state, then this patch isn't correct.

Right now, the software of RK3288 SoC only support CPU hotplug
(cpu_on/off) and power off all CPUs on suspend.

Sure, but that's a Linux implementation detail rather than a fixed
property of the hardware. Given those states exist, the "always-on"
property is not appropriate.

We do not implement cpuidle to power off CPU. Do you think we should
introduce a broadcast timer?

If one is present, yes. 

On our early kernel, I never see any interrupt on a broadcast timer
(yes, we implement it with a external timer).

That's fine; Linux doesn't need to use it just yet. However, when we
want to use low power states later, it will be necessary to enable
placing all CPUS into a low power state.

If your external timer is already supported by an existing driver, there
is no reason not to add it now.

quoted

A more general approach would be to enable the broadcast hrtimer for
arm, as has been done for arm64.

Yes. I think it should be done by arm framework.

Patches welcome.

I also think it would make sense to enable this for arm.

Thanks,
Mark.

Hi,

? 2014?08?29? 19:22, Mark Rutland ??:

quoted

? 2014?08?28? 23:11, Mark Rutland ??:

quoted

To clarify: if there are low power states that the CPU can enter where
we lose state, then this patch isn't correct.

Right now, the software of RK3288 SoC only support CPU hotplug
(cpu_on/off) and power off all CPUs on suspend.

Sure, but that's a Linux implementation detail rather than a fixed
property of the hardware. Given those states exist, the "always-on"
property is not appropriate.

quoted

We do not implement cpuidle to power off CPU. Do you think we should
introduce a broadcast timer?

If one is present, yes. 

quoted

On our early kernel, I never see any interrupt on a broadcast timer
(yes, we implement it with a external timer).

That's fine; Linux doesn't need to use it just yet. However, when we
want to use low power states later, it will be necessary to enable
placing all CPUS into a low power state.

If your external timer is already supported by an existing driver, there
is no reason not to add it now.

quoted

quoted

A more general approach would be to enable the broadcast hrtimer for
arm, as has been done for arm64.

Yes. I think it should be done by arm framework.

Patches welcome.

I also think it would make sense to enable this for arm.

Thanks,
Mark.

Okay, so this patch is wrong as I expected.
Thank you!