[U-Boot] [PATCH v1 0/4] Jetson-TK1 support for PSCI

[Ian Campbell <ijc@hellion.org.uk>]

On Fri, 2015-01-16 at 17:03 +0100, Thierry Reding wrote:
> On Fri, Jan 16, 2015 at 10:24:03AM +0000, Ian Campbell wrote:
> > On Fri, 2015-01-16 at 11:05 +0100, Thierry Reding wrote:
> > > On Fri, Jan 16, 2015 at 09:43:22AM +0000, Ian Campbell wrote:
> > > > On Thu, 2015-01-15 at 15:55 +0100, Thierry Reding wrote:
> > > > > On Wed, Jan 14, 2015 at 08:58:41AM +0000, Ian Campbell wrote:
> > > > > > On Wed, 2015-01-14 at 08:57 +0100, Thierry Reding wrote:
> > > > > > > > I also pushed my tree to gitorious:
> > > > > > > >         https://gitorious.org/ijc/u-boot jetson-psci-v1
> > > > > > > > 
> > > > > > > > I would Ack your patch, but I don't think you've posted it and it has no
> > > > > > > > S-o-b so that would seem a bit premature/rude of me. For the same reason
> > > > > > > > I've not actually included it in the series posted (but it is in the
> > > > > > > > gitorious branch).
> > > > > > > 
> > > > > > > Feel free to take ownership of that patch. I currently don't have the
> > > > > > > time to work on this and it seems you've made good progress on it.
> > > > > > 
> > > > > > Will do. Could you offer a S-o-b for it please so I can pick it up.
> > > > > 
> > > > > Sure:
> > > > > 
> > > > > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > > > 
> > > > Thanks!
> > > > 
> > > > > > > It could probably use some cleanup because there's a bit of debug output
> > > > > > > still in there. Also...
> > > > > > > 
> > > > > > > > FWIW I think you could drop your stub versions of psci_cpu_off and
> > > > > > > > psci_cpu_suspend (assuming you don't want to implement them) since the
> > > > > > > > common code has stubs.
> > > > > > > 
> > > > > > > ... I'd think you'd need to implement these so that you can get proper
> > > > > > > suspend/resume support in the kernel. I've had to disable cpuidle (via
> > > > > > > #undef CONFIG_PM_SLEEP in arch/arm/mach-tegra/cpuidle-tegra114.c) in the
> > > > > > > kernel to make that code not powergate CPUs. Ideally I think the kernel
> > > > > > > would check that it's running with PSCI support and disable the cpuidle
> > > > > > > driver. Maybe that could be done by introducing a new cpuidle driver
> > > > > > > that checks for PSCI availability and uses it when present.
> > > > > > 
> > > > > > Hrm, I'm not sure how this all fits together, it's not a problem I've
> > > > > > noted before.
> > > > > > 
> > > > > > FWIW I think cpu_off and cpu_suspend are optional in PSCI v0.1 so an
> > > > > > initial version doesn't necessarily need to implement them (sunxi
> > > > > > doesn't for example), but as you say they do enable useful features.
> > > > > 
> > > > > I think when I tried last time, without disable the cpuidle driver
> > > > > things would hang at boot. I would expect that problem to exist for
> > > > > any board. Perhaps you've disabled PM_SLEEP in your config?
> > > > 
> > > > I don't think so:
> > > > # grep PM_SLEEP /boot/config-3.18.0-trunk-armmp-lpae 
> > > > CONFIG_PM_SLEEP=y
> > > > CONFIG_PM_SLEEP_SMP=y
> > > > CONFIG_PM_SLEEP_DEBUG=y
> > > > 
> > > > I don't see anything about cpuidle in dmesg either.
> > > > 
> > > > Did you perhaps mean CPU_IDLE rather than PM_SLEEP because:
> > > > # CONFIG_CPU_IDLE is not set
> > > 
> > > Yes, I think that would have the same effect as disable PM_SLEEP (at
> > > least regarding the powergate stuff that's conflicting with the PSCI
> > > implementation).
> > > 
> > > Note also, as mentioned in another reply, that with the PSCI support
> > > there's now two sources that can simultaneously access the powergate
> > > functionality in the PMC. We have some locking in place to make sure
> > > that concurrent accesses from within the kernel are serialized, but
> > > there's no mechanism in place to protect from concurrent accesses in
> > > secure firmware and the kernel.
> > 
> > The docs are on another machine, but I take it the PMC registers are
> > available to NS mode? Is that configurable (from S mode) perhaps?
> 
> I don't see how that's relevant. Even if it was possible to secure the
> registers against access from NS mode, there's no way we could do that
> because many drivers rely on controlling their power domain using that
> functionality.
> 
> Or perhaps I misunderstand what you're suggesting.

If the PMC registers aren't available to NS then the damage the
powergate driver can do by conflicting with PSCI is more limited i.e not
going to fry the h/w somehow.

Ian.