On Monday, September 02, 2013 03:18:51 PM Daniel Lezcano wrote:

On 09/02/2013 11:41 AM, Bartlomiej Zolnierkiewicz wrote:

quoted

On Monday, September 02, 2013 10:54:17 AM Daniel Lezcano wrote:

quoted

On 08/30/2013 12:21 PM, Bartlomiej Zolnierkiewicz wrote:

quoted

Add "cpuidle-exynos.max_states=" parameter to allow user to specify
the maximum of allowed CPU idle states for ARM EXYNOS cpuidle driver.

This change is needed because C1 state (AFTR mode) is often not able
to work properly due to incompatibility with some bootloader versions.

Usage examples:

"cpuidle-exynos.max_states=1" disables C1 state (AFTR mode).

"cpuidle-exynos.max_states=0" disables the driver completely.

Signed-off-by: Bartlomiej Zolnierkiewicz <redacted>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Tomasz Figa <redacted>
Cc: Amit Daniel Kachhap <redacted>

There is a max_cstate option for acpi and intel idle. There is also the
cpuidle.off=1 option. As the semantic is the same, I think adding a
common cpuidle option usable for all the drivers is better.

I thought about making the option common for all cpuidle drivers first
but due to support for multiple cpuidle drivers on one machine (i.e.
big.LITTLE), per-driver option looked like a better approach.

Should I make the option common and not worry about multiple drivers on
one machine support?

Mmh, that's a good point.

I am not in favor of multiple options spread across the different
drivers. Furthermore the max_cstate is used in the intel platform to
'discover' what states the firmware supports which is not the case of
the cpuidle ARM drivers (except new PSCI based). This option does not
really fits well here.

There is the kernel parameter 'cpuidle.off', so disabling the driver is ok.

You converted the cpuidle driver to a platform driver. Isn't possible to
pass information in the platform data field at boot time to tell AFTR is
not supported and then act on the 'disabled' field of this state ?

It might be possible but I don't know where the source of this data would
be, platform specific kernel parameter? It sounds just like moving the code
around and adding superfluous platform->driver code because the similar
kernel parameter to disable just AFTR can be added in cpuidle-exynos driver
as well.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics