[PATCH] drivers: cpuidle: don't initialize big.LITTLE driver if MCPM is unavailable

[Lorenzo Pieralisi]

On Thu, Jan 08, 2015 at 11:11:40AM +0000, Daniel Lezcano wrote:

[...]

> >> IMO, it would be better to be more strict with the mcpm
> >> initialization and not let the system boot if something is wrong with
> >> it which I believe is coming from the firmware and let the user to
> >> figure out what is really happening by letting him to disable mcpm in
> >> the kernel configuration (which in turn will disable cpuidle).
> >
> > Again I fully agree, but in this case I manually switched to legacy boot
> > mode on TC2 and used same kernel with MCPM config enabled. Do you mean
> > to say we should not support that even when developer understand the
> > consequence of that ?
> 
> Well, I see there are the exynos5410/5420/5422. For the 5422 on 
> chromebook2 MCPM works well, IIUC. But for the 5422 on odroid-xu3, MCPM 
> does not work, hence cpuidle neither because of the firmware.
> 
> Silently disabling cpuidle because mcpm did not initialize will hide the 
> issue.

No. MCPM *will* initialize, Sudeep's patch does not silently disable
CPUidle.
To put it differently MCPM will initialize if CCI is in the DT and it
is "available", so unless defined differently in the dts mcpm will be
available and CPUidle will be initialized (and break if there is an issue
with the platform FW/HW).

I agree the mechanism to define if MCPM is available can be improved
but that's what it is at the moment.

The problem here is to boot a platform with different boot methods
and still have a single kernel image.

> I understand your point about switching to legacy without recompiling 
> the kernel.
> 
> I suggest we add a big fat WARN_ON when the mcpm initialization fails 
> with your patch.

I think there are multiple facets we are tackling at once here and they
should not be mixed.

1) We left static idle states there to cope with legacy DTBs that were
   published before we introduced idle states bindings. If we want to
   boot eg vexpress in legacy mode but single kernel image with MCPM on,
   we could remove the idle states in DT and the problem would be
   solved; we can't do that since we were forced to leave the static
   idle tables. Overall I think this is not the way to fix the issue.
2) The idle driver should be initialized if there is an idle state entry
   method, which in this case is MCPM. If I boot vexpress with MCPM
   enabled but legacy boot method (ie spin table) with a single kernel image
   I do not want to warn if the idle states entry method (MCPM) can't be
   initialized (and I do not want to get a warning if the idle driver is
   triggering a mcpm_cpu_suspend), so Sudeep's patch is valid and I am
   against adding a:

   if (WARN_ON(!mcpm_is_available())

3) Sudeep's patch is not hiding anything. If CCI is in DT, CCI is
   probed so mcpm_is_available() == true. If the firmware is borked
   the idle states will be entered and we will notice there is something
   wrong

So overall I think Sudeep's patch is sound. I also think we should
improve the way we detect if MCPM is available, and again, I think the
CPU operations on arm64 are a good example that we can and we should
replicate.

Lorenzo