On Fri, Jan 16, 2015 at 04:11:19PM +0000, Ian Campbell wrote: > 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 <tred...@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.
As far as I can tell these registers are available in NS mode. They have to because a bunch of drivers rely on it. The powergate driver controls not only CPU power domains, but also display, SATA, PCIe and so on. Thierry
pgpCLvmMsXipb.pgp
Description: PGP signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
