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

[Thierry Reding <treding@nvidia.com>]

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.

I don't have any good ideas on how to solve this nicely. The best I
could come up with is to make sure that we grab a lock before doing
any PSCI calls from the kernel and release the lock upon return.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150116/c26b94e9/attachment.pgp>