Enable arm_global_timer for Zynq brakes boot

[Sören Brinkmann]

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:
> On Mon, Jul 22, 2013 at 12:54:26PM -0700, Stephen Boyd wrote:
> > On 07/22/13 11:25, Sören Brinkmann wrote:
> > > On Wed, Jul 17, 2013 at 06:22:35PM -0700, Stephen Boyd wrote:
> > >> On 07/17/13 17:59, Sören Brinkmann wrote:
> > >>> Hi Stephen,
> > >>>
> > >>> On Wed, Jul 17, 2013 at 04:50:34PM -0700, Stephen Boyd wrote:
> > >>>> On 07/17/13 14:04, Sören Brinkmann wrote:
> > >>>>> 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?

@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