| Message ID | 1409190017-12656-1-git-send-email-kever.yang@rock-chips.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 > Signed-off-by: Kever Yang <kever.yang@rock-chips.com> > --- > > 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@ff140000 { > -- > 1.9.1 > > -- > To unsubscribe from this list: send the line "unsubscribe devicetree" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.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: > > 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. > > > Signed-off-by: Kever Yang <kever.yang@rock-chips.com> > > --- > > > > 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@ff140000 { > > -- > > 1.9.1 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe devicetree" in > > the body of a message to majordomo@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@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:
>> 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
> 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 ??: > > 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. > > 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 ??: >> ? 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. > 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. > >>> 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!
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@ff140000 {
We need use the hrtimer, which need the arch-timer to be 'always-on' Signed-off-by: Kever Yang <kever.yang@rock-chips.com> --- arch/arm/boot/dts/rk3288.dtsi | 1 + 1 file changed, 1 insertion(+)