Hi Thomas,

quoted

it have only 2 cpuidle states. Thanks to the previous patches, adding

have -> has.

quoted

the support for this new SoCs required only the modification of the

required -> requires.

quoted

architecture specific part.

May not be true if we move the cpuidle states back into the cpuidle
driver, as I suggested in a comment to one of the previous patch.

Yes indeed :/

quoted

The message in case of failure to suspend the system was switched from
warn to debug. Indeed due to the "slow exit process from the deep idle
state" in Armada 370, this situation happens quite often. Using the
_debug version avoids spamming the kernel logs, but still allows to
enable it if needed.

Is it really expected that the system fails to enter the idle state? I
must say I don't really see the relation with the slow exit from deep
idle state, because the do_armada_xp_370_cpu_suspend() function is only
here to *enter* the idle state, so how is entering the deep idle state
related to an issue when exiting the idle state?

The issue is related to the cache management but not to the slow exit
contrary to what I wrote.

We can see in the public datasheet, Figure 105:Deep Idle Simplified
Flows. The last instruction done by software is WFI, but before being
really in deep idle, the HW flush the L2 cache. This procedure is long
and can take a lot of time. Any wakup event interrupt during the L2
flush in the power down procedure the HW can stop the flush and make the
CPU return after the WFI.

I will update the explanation.

[...]

quoted

-	/*
-	 * On Armada 370, there is "a slow exit process from the deep
-	 * idle state due to heavy L1/L2 cache cleanup operations
-	 * performed by the BootROM software". To avoid this, we
-	 * replace the restart code of the bootrom by a a simple jump
-	 * to the boot address. Then the code located at this boot
-	 * address will take care of the initialization.
-	 */
-	if (of_machine_is_compatible("marvell,armada370"))
-		mvebu_boot_addr_wa(ARMADA_370_CRYPT0_ENG_ID, pmsu_mp_phys_base +
-				PMSU_BOOT_ADDR_REDIRECT_OFFSET(0));

So we're almost entirely reverting/moving the code added by "[PATCH
05/16] ARM: mvebu: Add workaround for cpuidle support for Armada 370".
Maybe there is an issue in your series, and the workaround for cpuidle
support on Armada 370 should only be added in this patch, and not
earlier?

My idea when I kept "ARM: mvebu: Add workaround for cpuidle support for
Armada 370", was to justify the introduction of the common function for
the boot address work around. But it don't really make sens in the end.


Thanks,

Gregory


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com