| Message ID | 20180102164223.15230-1-stefan@agner.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jan 02, 2018 at 05:42:17PM +0100, Stefan Agner wrote: > When the CPU is in ARM power off state the ARM architected > timers are stopped. The flag is already present in the higher > power WAIT mode. > > This allows to use the ARM generic timer on i.MX 6UL/6ULL SoC. > Without the flag the kernel freezes when the timer enters the > first time ARM power off mode. > > Cc: Anson Huang <anson.huang@nxp.com> > Signed-off-by: Stefan Agner <stefan@agner.ch> It seems ok at my side. Did you meet the real issue? If yes, how to reproduce? Both mx6sx and mx6ul are using GPT which do not need that flag, suppose we should remove it, right? Anson can help confirm it. Regards Dong Aisheng > --- > arch/arm/mach-imx/cpuidle-imx6sx.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm/mach-imx/cpuidle-imx6sx.c b/arch/arm/mach-imx/cpuidle-imx6sx.c > index c5a5c3a70ab1..d0f14b761ff7 100644 > --- a/arch/arm/mach-imx/cpuidle-imx6sx.c > +++ b/arch/arm/mach-imx/cpuidle-imx6sx.c > @@ -89,6 +89,7 @@ static struct cpuidle_driver imx6sx_cpuidle_driver = { > */ > .exit_latency = 300, > .target_residency = 500, > + .flags = CPUIDLE_FLAG_TIMER_STOP, > .enter = imx6sx_enter_wait, > .name = "LOW-POWER-IDLE", > .desc = "ARM power off", > -- > 2.15.1 >
Best Regards! Anson Huang > -----Original Message----- > From: Dong Aisheng [mailto:dongas86@gmail.com] > Sent: 2018-01-09 5:23 PM > To: Stefan Agner <stefan@agner.ch> > Cc: shawnguo@kernel.org; kernel@pengutronix.de; Fabio Estevam > <fabio.estevam@nxp.com>; robh+dt@kernel.org; mark.rutland@arm.com; > linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org; linux- > kernel@vger.kernel.org; Anson Huang <anson.huang@nxp.com>; dl-linux-imx > <linux-imx@nxp.com> > Subject: Re: [PATCH 1/7] ARM: imx: add timer stop flag to ARM power off state > > On Tue, Jan 02, 2018 at 05:42:17PM +0100, Stefan Agner wrote: > > When the CPU is in ARM power off state the ARM architected timers are > > stopped. The flag is already present in the higher power WAIT mode. > > > > This allows to use the ARM generic timer on i.MX 6UL/6ULL SoC. > > Without the flag the kernel freezes when the timer enters the first > > time ARM power off mode. > > > > Cc: Anson Huang <anson.huang@nxp.com> > > Signed-off-by: Stefan Agner <stefan@agner.ch> > > It seems ok at my side. > Did you meet the real issue? If yes, how to reproduce? > > Both mx6sx and mx6ul are using GPT which do not need that flag, suppose we > should remove it, right? > Anson can help confirm it. For UP system like i.MX6SX, we do NOT enable "cortex-a9-twd-timer", so local timer is NOT used, GPT is used instead, GPT's clock is NOT disabled when cpuidle, so I think we should remove all these Timer stop flag for 6SX CPUIDLE. Anson. > > Regards > Dong Aisheng > > > --- > > arch/arm/mach-imx/cpuidle-imx6sx.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/arch/arm/mach-imx/cpuidle-imx6sx.c > > b/arch/arm/mach-imx/cpuidle-imx6sx.c > > index c5a5c3a70ab1..d0f14b761ff7 100644 > > --- a/arch/arm/mach-imx/cpuidle-imx6sx.c > > +++ b/arch/arm/mach-imx/cpuidle-imx6sx.c > > @@ -89,6 +89,7 @@ static struct cpuidle_driver imx6sx_cpuidle_driver = { > > */ > > .exit_latency = 300, > > .target_residency = 500, > > + .flags = CPUIDLE_FLAG_TIMER_STOP, > > .enter = imx6sx_enter_wait, > > .name = "LOW-POWER-IDLE", > > .desc = "ARM power off", > > -- > > 2.15.1 > >
Am Dienstag, den 09.01.2018, 09:25 +0000 schrieb Anson Huang: > > Best Regards! > Anson Huang > > > > -----Original Message----- > > From: Dong Aisheng [mailto:dongas86@gmail.com] > > Sent: 2018-01-09 5:23 PM > > To: Stefan Agner <stefan@agner.ch> > > Cc: shawnguo@kernel.org; kernel@pengutronix.de; Fabio Estevam > > <fabio.estevam@nxp.com>; robh+dt@kernel.org; mark.rutland@arm.com; > > linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org; > > linux- > > kernel@vger.kernel.org; Anson Huang <anson.huang@nxp.com>; dl- > > linux-imx > > <linux-imx@nxp.com> > > Subject: Re: [PATCH 1/7] ARM: imx: add timer stop flag to ARM power > > off state > > > > On Tue, Jan 02, 2018 at 05:42:17PM +0100, Stefan Agner wrote: > > > When the CPU is in ARM power off state the ARM architected timers > > > are > > > stopped. The flag is already present in the higher power WAIT > > > mode. > > > > > > This allows to use the ARM generic timer on i.MX 6UL/6ULL SoC. > > > Without the flag the kernel freezes when the timer enters the > > > first > > > time ARM power off mode. > > > > > > Cc: Anson Huang <anson.huang@nxp.com> > > > Signed-off-by: Stefan Agner <stefan@agner.ch> > > > > It seems ok at my side. > > Did you meet the real issue? If yes, how to reproduce? > > > > Both mx6sx and mx6ul are using GPT which do not need that flag, > > suppose we > > should remove it, right? > > Anson can help confirm it. > > For UP system like i.MX6SX, we do NOT enable "cortex-a9-twd-timer", > so local > timer is NOT used, GPT is used instead, GPT's clock is NOT disabled > when cpuidle, > so I think we should remove all these Timer stop flag for 6SX > CPUIDLE. It's correct to set the flag even on UP systems, as the flag means the CPU _local_ timer is stopped in this sleep mode. Also there are systems out there which are using the TWD on UP, as it operates at a higher frequency leading to better wakeup granularity. Regards, Lucas
On 2018-01-09 10:22, Dong Aisheng wrote: > On Tue, Jan 02, 2018 at 05:42:17PM +0100, Stefan Agner wrote: >> When the CPU is in ARM power off state the ARM architected >> timers are stopped. The flag is already present in the higher >> power WAIT mode. >> >> This allows to use the ARM generic timer on i.MX 6UL/6ULL SoC. >> Without the flag the kernel freezes when the timer enters the >> first time ARM power off mode. >> >> Cc: Anson Huang <anson.huang@nxp.com> >> Signed-off-by: Stefan Agner <stefan@agner.ch> > > It seems ok at my side. > Did you meet the real issue? If yes, how to reproduce? Enable the timer added with Patch 5, use a U-Boot with this patchset applied: https://www.mail-archive.com/u-boot@lists.denx.de/msg273287.html And boot... For me it freezed somewhere early during systemd boot phase, presumably the first time the CPU got into this idle mode. -- Stefan > > Both mx6sx and mx6ul are using GPT which do not need that flag, suppose > we should remove it, right? > Anson can help confirm it. > > Regards > Dong Aisheng > >> --- >> arch/arm/mach-imx/cpuidle-imx6sx.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/arch/arm/mach-imx/cpuidle-imx6sx.c b/arch/arm/mach-imx/cpuidle-imx6sx.c >> index c5a5c3a70ab1..d0f14b761ff7 100644 >> --- a/arch/arm/mach-imx/cpuidle-imx6sx.c >> +++ b/arch/arm/mach-imx/cpuidle-imx6sx.c >> @@ -89,6 +89,7 @@ static struct cpuidle_driver imx6sx_cpuidle_driver = { >> */ >> .exit_latency = 300, >> .target_residency = 500, >> + .flags = CPUIDLE_FLAG_TIMER_STOP, >> .enter = imx6sx_enter_wait, >> .name = "LOW-POWER-IDLE", >> .desc = "ARM power off", >> -- >> 2.15.1 >>
On 2018-01-09 11:13, Lucas Stach wrote: > Am Dienstag, den 09.01.2018, 09:25 +0000 schrieb Anson Huang: >> >> Best Regards! >> Anson Huang >> >> >> > -----Original Message----- >> > From: Dong Aisheng [mailto:dongas86@gmail.com] >> > Sent: 2018-01-09 5:23 PM >> > To: Stefan Agner <stefan@agner.ch> >> > Cc: shawnguo@kernel.org; kernel@pengutronix.de; Fabio Estevam >> > <fabio.estevam@nxp.com>; robh+dt@kernel.org; mark.rutland@arm.com; >> > linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org; >> > linux- >> > kernel@vger.kernel.org; Anson Huang <anson.huang@nxp.com>; dl- >> > linux-imx >> > <linux-imx@nxp.com> >> > Subject: Re: [PATCH 1/7] ARM: imx: add timer stop flag to ARM power >> > off state >> > >> > On Tue, Jan 02, 2018 at 05:42:17PM +0100, Stefan Agner wrote: >> > > When the CPU is in ARM power off state the ARM architected timers >> > > are >> > > stopped. The flag is already present in the higher power WAIT >> > > mode. >> > > >> > > This allows to use the ARM generic timer on i.MX 6UL/6ULL SoC. >> > > Without the flag the kernel freezes when the timer enters the >> > > first >> > > time ARM power off mode. >> > > >> > > Cc: Anson Huang <anson.huang@nxp.com> >> > > Signed-off-by: Stefan Agner <stefan@agner.ch> >> > >> > It seems ok at my side. >> > Did you meet the real issue? If yes, how to reproduce? >> > >> > Both mx6sx and mx6ul are using GPT which do not need that flag, >> > suppose we >> > should remove it, right? >> > Anson can help confirm it. >> >> For UP system like i.MX6SX, we do NOT enable "cortex-a9-twd-timer", >> so local >> timer is NOT used, GPT is used instead, GPT's clock is NOT disabled >> when cpuidle, >> so I think we should remove all these Timer stop flag for 6SX >> CPUIDLE. > > It's correct to set the flag even on UP systems, as the flag means the > CPU _local_ timer is stopped in this sleep mode. Also there are systems > out there which are using the TWD on UP, as it operates at a higher > frequency leading to better wakeup granularity. Documentation/devicetree/bindings/arm/twd.txt states that TWD provides "per-cpu local timer". But as far as I can see TWD still uses SPI interrupts, routed through GIC, so is this the differentiation? -- Stefan
Am Dienstag, den 09.01.2018, 14:37 +0100 schrieb Stefan Agner: > On 2018-01-09 11:13, Lucas Stach wrote: > > Am Dienstag, den 09.01.2018, 09:25 +0000 schrieb Anson Huang: > > > > > > Best Regards! > > > Anson Huang > > > > > > > > > > -----Original Message----- > > > > From: Dong Aisheng [mailto:dongas86@gmail.com] > > > > Sent: 2018-01-09 5:23 PM > > > > To: Stefan Agner <stefan@agner.ch> > > > > Cc: shawnguo@kernel.org; kernel@pengutronix.de; Fabio Estevam > > > > <fabio.estevam@nxp.com>; robh+dt@kernel.org; mark.rutland@arm.c > > > > om; > > > > linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.or > > > > g; > > > > linux- > > > > kernel@vger.kernel.org; Anson Huang <anson.huang@nxp.com>; dl- > > > > linux-imx > > > > <linux-imx@nxp.com> > > > > Subject: Re: [PATCH 1/7] ARM: imx: add timer stop flag to ARM > > > > power > > > > off state > > > > > > > > On Tue, Jan 02, 2018 at 05:42:17PM +0100, Stefan Agner wrote: > > > > > When the CPU is in ARM power off state the ARM architected > > > > > timers > > > > > are > > > > > stopped. The flag is already present in the higher power WAIT > > > > > mode. > > > > > > > > > > This allows to use the ARM generic timer on i.MX 6UL/6ULL > > > > > SoC. > > > > > Without the flag the kernel freezes when the timer enters the > > > > > first > > > > > time ARM power off mode. > > > > > > > > > > Cc: Anson Huang <anson.huang@nxp.com> > > > > > Signed-off-by: Stefan Agner <stefan@agner.ch> > > > > > > > > It seems ok at my side. > > > > Did you meet the real issue? If yes, how to reproduce? > > > > > > > > Both mx6sx and mx6ul are using GPT which do not need that flag, > > > > suppose we > > > > should remove it, right? > > > > Anson can help confirm it. > > > > > > For UP system like i.MX6SX, we do NOT enable "cortex-a9-twd- > > > timer", > > > so local > > > timer is NOT used, GPT is used instead, GPT's clock is NOT > > > disabled > > > when cpuidle, > > > so I think we should remove all these Timer stop flag for 6SX > > > CPUIDLE. > > > > It's correct to set the flag even on UP systems, as the flag means > > the > > CPU _local_ timer is stopped in this sleep mode. Also there are > > systems > > out there which are using the TWD on UP, as it operates at a higher > > frequency leading to better wakeup granularity. > > Documentation/devicetree/bindings/arm/twd.txt states that TWD > provides > "per-cpu local timer". But as far as I can see TWD still uses SPI > interrupts, routed through GIC, so is this the differentiation? Maybe what I wrote wasn't entirely clear. I completely agree with this patch. The TWD on Cortex-A9 is a CPU local timer, same as the architected timer in later cores. It doesn't provide all the benefits of the architected timer (the clock frequency varies with CPU core clock and it's not virt capable), but some systems still prefer it over the i.MX GPT, as it provides much better wakeup granularity. So annotating the CPU idle states with the timer stop flag is the right thing to do. This flag has nothing to with the usage of GPT or TWD on a specific system. Regards, Lucas
On 2018-01-09 15:04, Lucas Stach wrote: > Am Dienstag, den 09.01.2018, 14:37 +0100 schrieb Stefan Agner: >> On 2018-01-09 11:13, Lucas Stach wrote: >> > Am Dienstag, den 09.01.2018, 09:25 +0000 schrieb Anson Huang: >> > > >> > > Best Regards! >> > > Anson Huang >> > > >> > > >> > > > -----Original Message----- >> > > > From: Dong Aisheng [mailto:dongas86@gmail.com] >> > > > Sent: 2018-01-09 5:23 PM >> > > > To: Stefan Agner <stefan@agner.ch> >> > > > Cc: shawnguo@kernel.org; kernel@pengutronix.de; Fabio Estevam >> > > > <fabio.estevam@nxp.com>; robh+dt@kernel.org; mark.rutland@arm.c >> > > > om; >> > > > linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.or >> > > > g; >> > > > linux- >> > > > kernel@vger.kernel.org; Anson Huang <anson.huang@nxp.com>; dl- >> > > > linux-imx >> > > > <linux-imx@nxp.com> >> > > > Subject: Re: [PATCH 1/7] ARM: imx: add timer stop flag to ARM >> > > > power >> > > > off state >> > > > >> > > > On Tue, Jan 02, 2018 at 05:42:17PM +0100, Stefan Agner wrote: >> > > > > When the CPU is in ARM power off state the ARM architected >> > > > > timers >> > > > > are >> > > > > stopped. The flag is already present in the higher power WAIT >> > > > > mode. >> > > > > >> > > > > This allows to use the ARM generic timer on i.MX 6UL/6ULL >> > > > > SoC. >> > > > > Without the flag the kernel freezes when the timer enters the >> > > > > first >> > > > > time ARM power off mode. >> > > > > >> > > > > Cc: Anson Huang <anson.huang@nxp.com> >> > > > > Signed-off-by: Stefan Agner <stefan@agner.ch> >> > > > >> > > > It seems ok at my side. >> > > > Did you meet the real issue? If yes, how to reproduce? >> > > > >> > > > Both mx6sx and mx6ul are using GPT which do not need that flag, >> > > > suppose we >> > > > should remove it, right? >> > > > Anson can help confirm it. >> > > >> > > For UP system like i.MX6SX, we do NOT enable "cortex-a9-twd- >> > > timer", >> > > so local >> > > timer is NOT used, GPT is used instead, GPT's clock is NOT >> > > disabled >> > > when cpuidle, >> > > so I think we should remove all these Timer stop flag for 6SX >> > > CPUIDLE. >> > >> > It's correct to set the flag even on UP systems, as the flag means >> > the >> > CPU _local_ timer is stopped in this sleep mode. Also there are >> > systems >> > out there which are using the TWD on UP, as it operates at a higher >> > frequency leading to better wakeup granularity. >> >> Documentation/devicetree/bindings/arm/twd.txt states that TWD >> provides >> "per-cpu local timer". But as far as I can see TWD still uses SPI >> interrupts, routed through GIC, so is this the differentiation? > > Maybe what I wrote wasn't entirely clear. I completely agree with this > patch. > > The TWD on Cortex-A9 is a CPU local timer, same as the architected > timer in later cores. It doesn't provide all the benefits of the > architected timer (the clock frequency varies with CPU core clock and > it's not virt capable), but some systems still prefer it over the i.MX > GPT, as it provides much better wakeup granularity. > > So annotating the CPU idle states with the timer stop flag is the right > thing to do. This flag has nothing to with the usage of GPT or TWD on a > specific system. Can I take that as an Acked-by? -- Stefan
Am Dienstag, den 02.01.2018, 17:42 +0100 schrieb Stefan Agner: > When the CPU is in ARM power off state the ARM architected > timers are stopped. The flag is already present in the higher > power WAIT mode. > > This allows to use the ARM generic timer on i.MX 6UL/6ULL SoC. > Without the flag the kernel freezes when the timer enters the > first time ARM power off mode. > > Cc: Anson Huang <anson.huang@nxp.com> > Signed-off-by: Stefan Agner <stefan@agner.ch> Reviewed-by: Lucas Stach <l.stach@pengutronix.de> > --- > arch/arm/mach-imx/cpuidle-imx6sx.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm/mach-imx/cpuidle-imx6sx.c b/arch/arm/mach- > imx/cpuidle-imx6sx.c > index c5a5c3a70ab1..d0f14b761ff7 100644 > --- a/arch/arm/mach-imx/cpuidle-imx6sx.c > +++ b/arch/arm/mach-imx/cpuidle-imx6sx.c > @@ -89,6 +89,7 @@ static struct cpuidle_driver imx6sx_cpuidle_driver > = { > */ > .exit_latency = 300, > .target_residency = 500, > + .flags = CPUIDLE_FLAG_TIMER_STOP, > .enter = imx6sx_enter_wait, > .name = "LOW-POWER-IDLE", > .desc = "ARM power off",
diff --git a/arch/arm/mach-imx/cpuidle-imx6sx.c b/arch/arm/mach-imx/cpuidle-imx6sx.c index c5a5c3a70ab1..d0f14b761ff7 100644 --- a/arch/arm/mach-imx/cpuidle-imx6sx.c +++ b/arch/arm/mach-imx/cpuidle-imx6sx.c @@ -89,6 +89,7 @@ static struct cpuidle_driver imx6sx_cpuidle_driver = { */ .exit_latency = 300, .target_residency = 500, + .flags = CPUIDLE_FLAG_TIMER_STOP, .enter = imx6sx_enter_wait, .name = "LOW-POWER-IDLE", .desc = "ARM power off",
When the CPU is in ARM power off state the ARM architected timers are stopped. The flag is already present in the higher power WAIT mode. This allows to use the ARM generic timer on i.MX 6UL/6ULL SoC. Without the flag the kernel freezes when the timer enters the first time ARM power off mode. Cc: Anson Huang <anson.huang@nxp.com> Signed-off-by: Stefan Agner <stefan@agner.ch> --- arch/arm/mach-imx/cpuidle-imx6sx.c | 1 + 1 file changed, 1 insertion(+)