Thread (29 messages) 29 messages, 4 authors, 2014-03-25
STALE4468d

[PATCH v4 12/13] cpuidle: mvebu: Add initial CPU idle support for Armada 370/XP SoC

From: Thomas Petazzoni <hidden>
Date: 2014-02-19 18:32:11
Also in: linux-pm

Dear Gregory CLEMENT,

On Wed, 19 Feb 2014 18:19:51 +0100, Gregory CLEMENT wrote:
quoted
Sorry to bring the naming issue, but it looks like the Armada 38x has a
PMSU that looks almost identical to the Armada XP PMSU, except that it
doesn't have the L2 fabric registers (probably because Armada 38x uses
the PL310 and not the Marvell L2 cache).

Therefore, should this cpuidle driver be named Armada 370/XP, or
ARMADA_MVEBU for example?
Well most of the code is related to the coherency and the L2 cache, so
a different L2 cache is a significant difference. The CPU are also different
for example the PJ4B can  use LDREX/STREX without MMU whereas the Cortex A9
can't.
Right, ok.
quoted
Also, I'm a bit unsure about your choice of mixing C and assembly here.
This function is already calling ll_clear_cpu_coherent() and
ll_set_cpu_coherent() that are assembly functions implement in
coherency_ll.S. Shouldn't we do the same for this final bit of assembly?
I made several tries when I converted most of the code in C, so I am not
sure but I think that using a C function didn't work here. But as Lorenzo
pointed they were mistakes in this code, so once I will have fixed them, I
will try again.
Having this is C would certainly be a lot better, but my comment was
merely to move this tiny bit of assembly somewhere else, but keep it as
assembly if it's really needed.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help