On 07/23/2013 12:41 AM, S?ren Brinkmann wrote:

Hi,

adding LKML and LAKML (which I forgot on the original email, sorry)

On Mon, Jul 22, 2013 at 01:13:48PM -0700, S?ren Brinkmann wrote:

quoted

On Mon, Jul 22, 2013 at 12:54:26PM -0700, Stephen Boyd wrote:

quoted

On 07/22/13 11:25, S?ren Brinkmann wrote:

quoted

On Wed, Jul 17, 2013 at 06:22:35PM -0700, Stephen Boyd wrote:

quoted

On 07/17/13 17:59, S?ren Brinkmann wrote:

quoted

Hi Stephen,

On Wed, Jul 17, 2013 at 04:50:34PM -0700, Stephen Boyd wrote:

quoted

On 07/17/13 14:04, S?ren Brinkmann wrote:

quoted

Hi all,

I'm trying to enable the arm_global_timer on Zynq platforms with the
attached patch. Unfortunately that patch breaks booting up. It hangs
when handing over to init/early userspace (see attached boot.log).

The funny thing is, if I remove either the global timer or the
arm,cortex-a9-twd-timer node from my dts, it works. So, it looks like
the two timer (drivers) interfere somehow. Does anybody have an idea of
what is going on and probably even how to resolve it?

The patch is based on commit c0d15cc in Linus' tree.

If you boot with one CPU does it hang? It looks like secondary CPUs
aren't getting interrupts but I'm not sure why. Maybe you can try this
patch and/or put some prints in the timer interrupt handler to see if
interrupts are firing on secondary CPUs.

Your proposed patch does not seem to make a difference, but adding
'maxcpus=1' to the kernel command line makes the system boot.

Hmm I guess that doesn't really confirm much because TWD doesn't
register itself on maxcpus=1 systems, so it's the same as removing the
node from DT. Probably easier to put a printk in the interrupt handler
and confirm that you're receiving interrupts on secondary CPUs.

Turns out it does work when I disable Zynq's cpuidle driver. I think I
can blame that driver.

Hmm.. Perhaps the arm_global_timer driver also needs FEAT_C3_STOP added
to it. Do you know if that timer is reset during low power modes?

Our cpudidle driver is not powering off anything, AFAIK. I think it just
executes 'wfi' on the CPU. I don't know how the timer core handles it,
but I'd expect the CPU should find the timer just a it was left before
entering idle (well, the timer continues to run I assume, but other than
that).
I'll do some debugging and see if I can find out what exactly causes the
hang.

So, what I found:
The Zynq cpuidle driver provides two idle states, which are both
basically just ARM wfi states. But the second one set's these flags:
	.flags = CPUIDLE_FLAG_TIME_VALID | CPUIDLE_FLAG_TIMER_STOP,

I don't know what these flags cause in detail. But the
CPUIDLE_FLAG_TIMER_STOP seemed suspicious, since wfi does not have any
effect on the timer. So, I removed that one and things are working.

I also tried the other approach: Leaving cpuidle as is and adding the
C3STOP flag to the global timer. That solves it too.

Does anybody know what the correct solution is?
In case the C3STOP flag is considered to be corret for the timer, I
could prepare a patch for that and bundle it with the one to enable the
timer for Zynq?

Hi Soren,

the CPUIDLE_FLAG_TIMER_STOP flag tells the cpuidle framework the local
timer will be stopped when entering to the idle state. In this case, the
cpuidle framework will call clockevents_notify(ENTER) and switches to a
broadcast timer and will call clockevents_notify(EXIT) when exiting the
idle state, switching the local timer back in use.

The C3STOP flag has a similar semantic than the CPUIDLE_FLAG_TIMER_STOP,
that is the timer can be shutdown with a specific idle state. This flag
is used by the tick broadcast code.

If the C3STOP flag is not set for a local timer, the
CPUIDLE_FLAG_TIMER_STOP does not make sense because it will be ignored
by the tick-broadcast code.

If the local timer could be shutdown at idle time, you *must* specify
this flag.

If the idle state shutdowns the cpu with its local timer, you *must*
specify the CPUIDLE_FLAG_TIMER_STOP flag for this specific state.

At the first glance, the idle state #2 is aimed to do DDR self refresh
and to switch to WFI, so no power gating, then no local timer down. The
CPUIDLE_FLAG_TIMER_STOP shouldn't be used here.

IIUC, the global timer does not belong to the CPU and the cluster power
domains, so it can't be shutdown: the C3STOP shouldn't be used.

I hope that helps.

  -- Daniel

@Michal: What do we do about the cpuidle driver? If you asked me, we
should rip out the second state completely. We have it as disabled
placeholder in our vendor tree and it seems to break things having it
enabled in mainline.


	S?ren

-- 
 <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog