ARM: mvebu: Minor fixes and improvements to coherency_ll.S
Subsystems (3)
cover ARM: mvebu: Minor fixes and improvements to coherency_ll.S
Jason, Andrew, Gregory, Sebastian,
Here is a small set of patches making fixes and improvements to the coherency_ll.S code.
The first patch is really important and fixes big endian booting in -next. The regression was introduced by the cpuidle changes, which are currently in -next.
The other three patches are only improvements, mainly cosmetic, but I believe they clarify a few things that took me a bit of time to re-understand this morning.
The patches are based on top of mvebu/soc.
Thanks,
Thomas
Thomas Petazzoni (4):
ARM: mvebu: fix big endian booting after coherency code rework
ARM: mvebu: fix indentation of assembly instructions in coherency_ll.S
ARM: mvebu: improve comments in coherency_ll.S
ARM: mvebu: returns ll_get_cpuid() to ll_get_coherency_cpumask()
arch/arm/mach-mvebu/coherency_ll.S | 77 ++++++++++++++++++++++++-------------- 1 file changed, 49 insertions(+), 28 deletions(-)
Signature · 1 line
1.9.3
PATCH 1/4 ARM: mvebu: fix big endian booting after coherency code rework
As part of the introduction of the cpuidle support for Armada XP, the coherency code was significantly reworked, especially in the coherency_ll.S file. However, when the ll_get_cpuid function was created, the big-endian specific code that switches the endianess of the register was not updated properly.
This patch fixes this code, and therefore makes big endian systems bootable again.
---
Jason, the offending commit is only scheduled for 3.16, so there is no need to push this patch for 3.15 or stable.
--- arch/arm/mach-mvebu/coherency_ll.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm/mach-mvebu/coherency_ll.S +1 -1
@@ -49,7 +49,7 @@ ENTRY(ll_get_cpuid) and r3, r3, #15 mov r2, #(1 << 24) lsl r3, r2, r3 -ARM_BE8(rev r1, r1) +ARM_BE8(rev r3, r3) mov pc, lr ENDPROC(ll_get_cpuid)
Signature · 1 line
1.9.3
PATCH 2/4 ARM: mvebu: fix indentation of assembly instructions in coherency_ll.S
In reply to Thomas PetazzoniThis commit does not make any functional change, it only fixes the indentation of a few assembly instructions in arch/arm/mach-mvebu/coherency_ll.S.
--- arch/arm/mach-mvebu/coherency_ll.S | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
arch/arm/mach-mvebu/coherency_ll.S +4 -4
@@ -66,10 +66,10 @@ ENTRY(ll_add_cpu_to_smp_group) * ll_get_cpuid, we can use it to save lr modifing it with the * following bl */ - mov r0, lr + mov r0, lr bl ll_get_coherency_base bl ll_get_cpuid - mov lr, r0 + mov lr, r0 add r0, r1, #ARMADA_XP_CFB_CFG_REG_OFFSET 1: ldrex r2, [r0]
@@ -108,10 +108,10 @@ ENTRY(ll_disable_coherency) * ll_get_cpuid, we can use it to save lr modifing it with the * following bl */ - mov r0, lr + mov r0, lr bl ll_get_coherency_base bl ll_get_cpuid - mov lr, r0 + mov lr, r0 add r0, r1, #ARMADA_XP_CFB_CTL_REG_OFFSET 1: ldrex r2, [r0]
Signature · 1 line
1.9.3
PATCH 3/4 ARM: mvebu: improve comments in coherency_ll.S
In reply to Thomas PetazzoniThis commit makes no functional change, it only improves a bit the various code comments in mach-mvebu/coherency_ll.S, by fixing a few typos and adding a few more details.
--- arch/arm/mach-mvebu/coherency_ll.S | 50 ++++++++++++++++++++++++-------------- 1 file changed, 32 insertions(+), 18 deletions(-)
arch/arm/mach-mvebu/coherency_ll.S +32 -18
@@ -24,26 +24,32 @@ #include <asm/cp15.h> .text -/* Returns with the coherency address in r1 (r0 is untouched)*/ +/* Returns the coherency base address in r1 (r0 is untouched) */ ENTRY(ll_get_coherency_base) mrc p15, 0, r1, c1, c0, 0 tst r1, #CR_M @ Check MMU bit enabled bne 1f - /* use physical address of the coherency register */ + /* + * MMU is disabled, use the physical address of the coherency + * base address. + */ adr r1, 3f ldr r3, [r1] ldr r1, [r1, r3] b 2f 1: - /* use virtual address of the coherency register */ + /* + * MMU is enabled, use the virtual address of the coherency + * base address. + */ ldr r1, =coherency_base ldr r1, [r1] 2: mov pc, lr ENDPROC(ll_get_coherency_base) -/* Returns with the CPU ID in r3 (r0 is untouched)*/ +/* Returns the CPU ID in r3 (r0 is untouched) */ ENTRY(ll_get_cpuid) mrc 15, 0, r3, cr0, cr0, 5 and r3, r3, #15
@@ -53,18 +59,22 @@ ARM_BE8(rev r3, r3) mov pc, lr ENDPROC(ll_get_cpuid) -/* ll_add_cpu_to_smp_group, ll_enable_coherency and - * ll_disable_coherency use strex/ldrex whereas MMU can be off. The - * Armada XP SoC has an exclusive monitor that can track transactions - * to Device and/or SO and as such also when MMU is disabled the - * exclusive transactions will be functional +/* + * ll_add_cpu_to_smp_group(), ll_enable_coherency() and + * ll_disable_coherency() use the strex/ldrex instructions while the + * MMU can be disabled. The Armada XP SoC has an exclusive monitor + * that tracks transactions to Device and/or SO memory and thanks to + * that, exclusive transactions are functional even when the MMU is + * disabled. */ ENTRY(ll_add_cpu_to_smp_group) /* - * r0 being untouched in ll_get_coherency_base and - * ll_get_cpuid, we can use it to save lr modifing it with the - * following bl + * As r0 is not modified by ll_get_coherency_base() and + * ll_get_cpuid(), we use it to temporarly save lr and avoid + * it being modified by the branch and link calls. This + * function is used very early in the secondary CPU boot, and + * no stack is available at this point. */ mov r0, lr bl ll_get_coherency_base
@@ -82,9 +92,11 @@ ENDPROC(ll_add_cpu_to_smp_group) ENTRY(ll_enable_coherency) /* - * r0 being untouched in ll_get_coherency_base and - * ll_get_cpuid, we can use it to save lr modifing it with the - * following bl + * As r0 is not modified by ll_get_coherency_base() and + * ll_get_cpuid(), we use it to temporarly save lr and avoid + * it being modified by the branch and link calls. This + * function is used very early in the secondary CPU boot, and + * no stack is available at this point. */ mov r0, lr bl ll_get_coherency_base
@@ -104,9 +116,11 @@ ENDPROC(ll_enable_coherency) ENTRY(ll_disable_coherency) /* - * r0 being untouched in ll_get_coherency_base and - * ll_get_cpuid, we can use it to save lr modifing it with the - * following bl + * As r0 is not modified by ll_get_coherency_base() and + * ll_get_cpuid(), we use it to temporarly save lr and avoid + * it being modified by the branch and link calls. This + * function is used very early in the secondary CPU boot, and + * no stack is available at this point. */ mov r0, lr bl ll_get_coherency_base
Signature · 1 line
1.9.3
PATCH 4/4 ARM: mvebu: returns ll_get_cpuid() to ll_get_coherency_cpumask()
In reply to Thomas PetazzoniIn the refactoring of the coherency fabric assembly code, a function called ll_get_cpuid() was created to factorize common logic between functions adding CPU to the SMP coherency group, enabling and disabling the coherency.
However, the name of the function is highly misleading: ll_get_cpuid() makes one think tat it returns the ID of the CPU, i.e 0 for CPU0, 1 for CPU1, etc. In fact, this is not at all what this function returns:
it returns a CPU mask for the current CPU, usable for the coherency fabric configuration and control registers.
Therefore this commit renames this function to ll_get_coherency_cpumask(), and adds additional comments on top of the function to explain in more details what it does, and also how the endianess issue is handled.
--- arch/arm/mach-mvebu/coherency_ll.S | 43 ++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 18 deletions(-)
arch/arm/mach-mvebu/coherency_ll.S +25 -18
@@ -49,15 +49,22 @@ ENTRY(ll_get_coherency_base) mov pc, lr ENDPROC(ll_get_coherency_base) -/* Returns the CPU ID in r3 (r0 is untouched) */ -ENTRY(ll_get_cpuid) +/* + * Returns the coherency CPU mask in r3 (r0 is untouched). This + * coherency CPU mask can be used with the coherency fabric + * configuration and control registers. Note that the mask is already + * endian-swapped as appropriate so that the calling functions do not + * have to care about endianness issues while accessing the coherency + * fabric registers + */ +ENTRY(ll_get_coherency_cpumask) mrc 15, 0, r3, cr0, cr0, 5 and r3, r3, #15 mov r2, #(1 << 24) lsl r3, r2, r3 ARM_BE8(rev r3, r3) mov pc, lr -ENDPROC(ll_get_cpuid) +ENDPROC(ll_get_coherency_cpumask) /* * ll_add_cpu_to_smp_group(), ll_enable_coherency() and
@@ -71,14 +78,14 @@ ENDPROC(ll_get_cpuid) ENTRY(ll_add_cpu_to_smp_group) /* * As r0 is not modified by ll_get_coherency_base() and - * ll_get_cpuid(), we use it to temporarly save lr and avoid - * it being modified by the branch and link calls. This - * function is used very early in the secondary CPU boot, and - * no stack is available at this point. + * ll_get_coherency_cpumask(), we use it to temporarly save lr + * and avoid it being modified by the branch and link + * calls. This function is used very early in the secondary + * CPU boot, and no stack is available at this point. */ mov r0, lr bl ll_get_coherency_base - bl ll_get_cpuid + bl ll_get_coherency_cpumask mov lr, r0 add r0, r1, #ARMADA_XP_CFB_CFG_REG_OFFSET 1:
@@ -93,14 +100,14 @@ ENDPROC(ll_add_cpu_to_smp_group) ENTRY(ll_enable_coherency) /* * As r0 is not modified by ll_get_coherency_base() and - * ll_get_cpuid(), we use it to temporarly save lr and avoid - * it being modified by the branch and link calls. This - * function is used very early in the secondary CPU boot, and - * no stack is available at this point. + * ll_get_coherency_cpumask(), we use it to temporarly save lr + * and avoid it being modified by the branch and link + * calls. This function is used very early in the secondary + * CPU boot, and no stack is available at this point. */ mov r0, lr bl ll_get_coherency_base - bl ll_get_cpuid + bl ll_get_coherency_cpumask mov lr, r0 add r0, r1, #ARMADA_XP_CFB_CTL_REG_OFFSET 1:
@@ -117,14 +124,14 @@ ENDPROC(ll_enable_coherency) ENTRY(ll_disable_coherency) /* * As r0 is not modified by ll_get_coherency_base() and - * ll_get_cpuid(), we use it to temporarly save lr and avoid - * it being modified by the branch and link calls. This - * function is used very early in the secondary CPU boot, and - * no stack is available at this point. + * ll_get_coherency_cpumask(), we use it to temporarly save lr + * and avoid it being modified by the branch and link + * calls. This function is used very early in the secondary + * CPU boot, and no stack is available@this point. */ mov r0, lr bl ll_get_coherency_base - bl ll_get_cpuid + bl ll_get_coherency_cpumask mov lr, r0 add r0, r1, #ARMADA_XP_CFB_CTL_REG_OFFSET 1:
Signature · 1 line
1.9.3
PATCH 4/4 ARM: mvebu: returns ll_get_cpuid() to ll_get_coherency_cpumask()
Hello Jason,
There is a typo in the title of this commit: s/returns/rename/.
Should I respin this patch, or can you fix directly in mvebu/soc ?
Thanks, and sorry for the mess :/
Thomas
Quoted 111 lines · Thomas Petazzoni
In the refactoring of the coherency fabric assembly code, a function called
ll_get_cpuid()was created to factorize common logic between functions adding CPU to the SMP coherency group, enabling and disabling the coherency.However, the name of the function is highly misleading:
ll_get_cpuid()makes one think tat it returns the ID of the CPU, i.e 0 for CPU0, 1 for CPU1, etc. In fact, this is not at all what this function returns:
it returns a CPU mask for the current CPU, usable for the coherency fabric configuration and control registers.Therefore this commit renames this function to
ll_get_coherency_cpumask(), and adds additional comments on top of the function to explain in more details what it does, and also how the endianess issue is handled.Signed-off-by Thomas Petazzoni <[email protected]>--- arch/arm/mach-mvebu/coherency_ll.S | 43 ++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 18 deletions(-)arch/arm/mach-mvebu/coherency_ll.S +25
-18@@ -49,15 +49,22 @@ ENTRY(ll_get_coherency_base) mov pc, lr ENDPROC(ll_get_coherency_base) -/* Returns the CPU ID in r3 (r0 is untouched) */ -ENTRY(ll_get_cpuid) +/* + * Returns the coherency CPU mask in r3 (r0 is untouched). This + * coherency CPU mask can be used with the coherency fabric + * configuration and control registers. Note that the mask is already + * endian-swapped as appropriate so that the calling functions do not + * have to care about endianness issues while accessing the coherency + * fabric registers + */ +ENTRY(ll_get_coherency_cpumask) mrc 15, 0, r3, cr0, cr0, 5 and r3, r3, #15 mov r2, #(1 << 24) lsl r3, r2, r3 ARM_BE8(rev r3, r3) mov pc, lr -ENDPROC(ll_get_cpuid) +ENDPROC(ll_get_coherency_cpumask) /* * ll_add_cpu_to_smp_group(), ll_enable_coherency() and@@ -71,14 +78,14 @@ ENDPROC(ll_get_cpuid) ENTRY(ll_add_cpu_to_smp_group) /* * As r0 is not modified by ll_get_coherency_base() and - * ll_get_cpuid(), we use it to temporarly save lr and avoid - * it being modified by the branch and link calls. This - * function is used very early in the secondary CPU boot, and - * no stack is available at this point. + * ll_get_coherency_cpumask(), we use it to temporarly save lr + * and avoid it being modified by the branch and link + * calls. This function is used very early in the secondary + * CPU boot, and no stack is available at this point. */ mov r0, lr bl ll_get_coherency_base - bl ll_get_cpuid + bl ll_get_coherency_cpumask mov lr, r0 add r0, r1, #ARMADA_XP_CFB_CFG_REG_OFFSET 1:@@ -93,14 +100,14 @@ ENDPROC(ll_add_cpu_to_smp_group) ENTRY(ll_enable_coherency) /* * As r0 is not modified by ll_get_coherency_base() and - * ll_get_cpuid(), we use it to temporarly save lr and avoid - * it being modified by the branch and link calls. This - * function is used very early in the secondary CPU boot, and - * no stack is available at this point. + * ll_get_coherency_cpumask(), we use it to temporarly save lr + * and avoid it being modified by the branch and link + * calls. This function is used very early in the secondary + * CPU boot, and no stack is available at this point. */ mov r0, lr bl ll_get_coherency_base - bl ll_get_cpuid + bl ll_get_coherency_cpumask mov lr, r0 add r0, r1, #ARMADA_XP_CFB_CTL_REG_OFFSET 1:@@ -117,14 +124,14 @@ ENDPROC(ll_enable_coherency) ENTRY(ll_disable_coherency) /* * As r0 is not modified by ll_get_coherency_base() and - * ll_get_cpuid(), we use it to temporarly save lr and avoid - * it being modified by the branch and link calls. This - * function is used very early in the secondary CPU boot, and - * no stack is available at this point. + * ll_get_coherency_cpumask(), we use it to temporarly save lr + * and avoid it being modified by the branch and link + * calls. This function is used very early in the secondary + * CPU boot, and no stack is available at this point. */ mov r0, lr bl ll_get_coherency_base - bl ll_get_cpuid + bl ll_get_coherency_cpumask mov lr, r0 add r0, r1, #ARMADA_XP_CFB_CTL_REG_OFFSET 1:
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
PATCH 4/4 ARM: mvebu: returns ll_get_cpuid() to ll_get_coherency_cpumask()
Quoted 5 lines · Thomas Petazzoni
Hello Jason,
There is a typo in the title of this commit: s/returns/rename/.
Should I respin this patch, or can you fix directly in mvebu/soc ?
Sorry, it's too late. It's already in arm-soc. And with the final rush of work for arm-soc, I'm not inclined to go through the pain of coordinating a branch drop and resend for a typo, as annoying as it is. :)
sorry,
Jason.
PATCH 4/4 ARM: mvebu: returns ll_get_cpuid() to ll_get_coherency_cpumask()
Dear Jason Cooper,
Quoted 5 lines · Jason Cooper
Quoted 4 lines · Thomas Petazzoni
Hello Jason,
There is a typo in the title of this commit: s/returns/rename/.
Quoted 8 lines
Quoted 1 line
Should I respin this patch, or can you fix directly in mvebu/soc ?
Sorry, it's too late. It's already in arm-soc. And with the final rush of work for arm-soc, I'm not inclined to go through the pain of coordinating a branch drop and resend for a typo, as annoying as it is. :)
sorry,
No problem. It's just a minor typo. If the patch had not already been pulled, then it would have been easy to fix. Now it's too late, but it's not that bad.
Thanks for your feedback,
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
cover ARM: mvebu: Minor fixes and improvements to coherency_ll.S
In reply to Thomas PetazzoniHi Thomas,
Quoted 9 lines · Thomas Petazzoni
Jason, Andrew, Gregory, Sebastian,
Here is a small set of patches making fixes and improvements to the
coherency_ll.Scode.The first patch is really important and fixes big endian booting in -next. The regression was introduced by the cpuidle changes, which are currently in -next.
Quoted 6 lines
The other three patches are only improvements, mainly cosmetic, but I believe they clarify a few things that took me a bit of time to re-understand this morning.
The patches are based on top of mvebu/soc.
for the whole series:
Thanks,
Gregory
Quoted 13 lines
Thanks,
Thomas
Thomas Petazzoni (4):
ARM: mvebu: fix big endian booting after coherency code rework
ARM: mvebu: fix indentation of assembly instructions incoherency_ll.S
ARM: mvebu: improve comments incoherency_ll.S
ARM: mvebu: returnsll_get_cpuid()toll_get_coherency_cpumask()arch/arm/mach-mvebu/coherency_ll.S | 77 ++++++++++++++++++++++++-------------- 1 file changed, 49 insertions(+), 28 deletions(-)
Signature · 4 lines
Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com
cover ARM: mvebu: Minor fixes and improvements to coherency_ll.S
In reply to Thomas PetazzoniQuoted 19 lines · Thomas Petazzoni
Jason, Andrew, Gregory, Sebastian,
Here is a small set of patches making fixes and improvements to the
coherency_ll.Scode.The first patch is really important and fixes big endian booting in -next. The regression was introduced by the cpuidle changes, which are currently in -next.
The other three patches are only improvements, mainly cosmetic, but I believe they clarify a few things that took me a bit of time to re-understand this morning.
The patches are based on top of mvebu/soc.
Thanks,
Thomas
Quoted 8 lines
Thomas Petazzoni (4):
ARM: mvebu: fix big endian booting after coherency code rework
ARM: mvebu: fix indentation of assembly instructions incoherency_ll.S
ARM: mvebu: improve comments incoherency_ll.S
ARM: mvebu: returnsll_get_cpuid()toll_get_coherency_cpumask()arch/arm/mach-mvebu/coherency_ll.S | 77 ++++++++++++++++++++++++-------------- 1 file changed, 49 insertions(+), 28 deletions(-)
Whole series applied to mvebu/soc with Gregory's Ack.
thx,
Jason.