On Tue, Jan 05, 2016 at 01:27:01PM +0000, Lorenzo Pieralisi wrote:

On Tue, Jan 05, 2016 at 12:51:42PM +0000, Russell King - ARM Linux wrote:

quoted

On ARM, that option is CONFIG_ARM_CPU_SUSPEND - that option solely
controls whether cpu_suspend/resume are present.  ARM64 just needs
to adopt this, and use that to control the code which is built in
drivers/firmware/psci.c.

However, I don't think it's as simple as just adding that to ARM64,
as you need to be careful of the Kconfig dependencies.  For ARM,
this is:

Generic code:
- SUSPEND defaults to y, depends on ARCH_SUSPEND_POSSIBLE (which is set for
   any cpu_suspend enabled CPU.)
- PM_SLEEP if SUSPEND || HIBERNATE_CALLBACKS
ARM sets:
- CPU_PM if SUSPEND || CPU_IDLE.
- ARM_CPU_SUSPEND if PM_SLEEP || BL_SWITCHER || (ARCH_PXA && PM)

What this means is that CPU_PM is entirely independent of
ARM_CPU_SUSPEND.  One does not imply the other, so I think you need
to consider carefully what ifdef you need in drivers/firmware/psci.c.

This is why I think fixing this is not simple as it first looks.

Not saying it is nice, but unless I find a cleaner way I was keener on
adding a silent config entry in drivers/firmware, say:

config ARM_PSCI_CPU_IDLE
	def_bool ARM_PSCI_FW && CPU_IDLE
	select ARM_CPU_SUSPEND if ARM

and use that to either guard the code or stub it out and compile it
if that config option is enabled.

That immediately worries me, because it bypasses the CPU dependencies
for ARM_CPU_SUSPEND implicitly applied via ARCH_SUSPEND_POSSIBLE.  I'd
prefer instead:

config ARM_PSCI_CPU_IDLE
	def_bool ARM_PSCI_FW && CPU_IDLE && (!ARM || ARM_CPU_SUSPEND)

Really, the answer is to stop ARM64 diverging from ARM, so we stop
having these architecture conditionals all over the place.  If ARM64
builds its cpu_suspend code in the same way that ARM does (iow,
keyed off ARM_CPU_SUSPEND, which it can select), then we end up
with the above being:

config ARM_PSCI_CPU_IDLE
	def_bool ARM_PSCI_FW && CPU_IDLE && ARM_CPU_SUSPEND

which is a helll of a lot simpler.  The dependency on ARM_PSCI_FW
could be regarded as redundant if we're only using ARM_PSCI_CPU_IDLE
to control code built within drivers/firmware/psci.c, as that won't
be built unless ARM_PSCI_FW is set.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.