[PATCH v2 0/2] ARM: PJ4B: fixes for 3.10-rc

[PATCH v2 0/2] ARM: PJ4B: fixes for 3.10-rc

[Gregory CLEMENT]

Hello,

This path set allows to fix some errata for PJ4B CPUs.

Without the first one, the Armada 370 based board can't boot anymore
since 3.10-rc1 . So this one really need to be applied on 3.10-rc.

It should be a good thing to apply the other one on the 3.10-rc, even
if we didn't observe get feedback from user who had a problem related
to these errata, it may happen.

Note that currently PJ4B is a symbol for 2 different CPUs:

- the one used in Armada 370 is a PJ4B
- the ones used in Armada XP are actually PJ4B-MP

So I planed to introduce a new symbol PJ4B-MP but for 3.11 as it is
not related to a critical fix. Awaiting these new symbols I made the
errata depends on ARMADA_370 in the Kconfig.

Russell,

I am aware that you didn't read the first version as you were on not
available. But this first version was usefull as we get many
interesting comments from Will Deacon. I addressed all of them when it
was possible.

Thanks,

Changelog:
V1 -> V2;
 - Fixed various coding style and typo issues
 - Replaced the "mcr p15, 0, r0, c7, c10, 4" by  "dsb"
 - Removed the fix for errata 4611 in discard_old_kernel_data function,
   as we don't use it

Lior Amsalem (2):
  ARM PJ4B: Add support for errata 4742
  ARM PJ4B: Add support for errata 4611

 arch/arm/Kconfig                | 22 ++++++++++++++++++++++
 arch/arm/include/asm/tlbflush.h | 22 ++++++++++++++++++++++
 arch/arm/mm/proc-macros.S       | 13 +++++++++++++
 arch/arm/mm/proc-v7.S           |  3 +++
 4 files changed, 60 insertions(+)

-- 
1.8.1.2

[PATCH v2 1/2] ARM PJ4B: Add support for errata 4742

[Gregory CLEMENT]

From: Lior Amsalem <alior@marvell.com>

This commit fixes the regression on Armada 370 (the kernal hang during
boot) introduced by the commit: "ARM: 7691/1: mm: kill unused
TLB_CAN_READ_FROM_L1_CACHE and use ALT_SMP instead".

When coming out of either a Wait for Interrupt (WFI) or a Wait for
Event (WFE) IDLE states, a specific timing sensitivity exists between
the retiring WFI/WFE instructions and the newly issued subsequent
instructions. This sensitivity can result in a CPU hang scenario.  The
workaround is to insert either a Data Synchronization Barrier (DSB) or
Data Memory Barrier (DMB) command immediately after the WFI/WFE
instruction

[gregory.clement at free-electrons.com:add errata description in changelog]
[gregory.clement at free-electrons.com:make this errata depend on Armada 370]
Signed-off-by: Lior Amsalem <alior@marvell.com>
Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 arch/arm/Kconfig      | 13 +++++++++++++
 arch/arm/mm/proc-v7.S |  3 +++
 2 files changed, 16 insertions(+)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 49d993c..95cbe9d 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1087,6 +1087,19 @@ if !MMU
 source "arch/arm/Kconfig-nommu"
 endif
 
+config PJ4B_ERRATA_4742
+	bool "PJ4B Errata 4742: IDLE Wake Up Commands can Cause the CPU Core to Cease Operation"
+	depends on CPU_PJ4B && MACH_ARMADA_370
+	help
+	  When coming out of either a Wait for Interrupt (WFI) or a Wait for
+	  Event (WFE) IDLE states, a specific timing sensitivity exists between
+	  the retiring WFI/WFE instructions and the newly issued subsequent
+	  instructions.  This sensitivity can result in a CPU hang scenario.
+	  Workaround:
+	  The software must insert either a Data Synchronization Barrier (DSB)
+	  or Data Memory Barrier (DMB) command immediately after the WFI/WFE
+	  instruction
+
 config ARM_ERRATA_326103
 	bool "ARM errata: FSR write bit incorrect on a SWP to read-only memory"
 	depends on CPU_V6
diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
index 2c73a73..784bef1 100644
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
@@ -71,6 +71,9 @@ ENDPROC(cpu_v7_reset)
 ENTRY(cpu_v7_do_idle)
 	dsb					@ WFI may enter a low-power mode
 	wfi
+#ifdef CONFIG_PJ4B_ERRATA_4742
+	dsb					@barrier
+#endif
 	mov	pc, lr
 ENDPROC(cpu_v7_do_idle)
 
-- 
1.8.1.2

[PATCH v2 2/2] ARM PJ4B: Add support for errata 4611

[Gregory CLEMENT]

From: Lior Amsalem <alior@marvell.com>

A CP15 clean operation can result in a dead lock state if it is hit by
an incoming snoop event. The fix to this issue is the following:
before any CP15 clean type operation in Cache Coherency mode, issue a
Data Memory Barrier (DMB) or a Data Synchronization Barrier (DSB)
instruction.

[gregory.clement at free-electrons.com:add errata description in changelog]
[gregory.clement at free-electrons.com:make this errata depend on Armada 370]
Signed-off-by: Lior Amsalem <alior@marvell.com>
Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 arch/arm/Kconfig                |  9 +++++++++
 arch/arm/include/asm/tlbflush.h | 22 ++++++++++++++++++++++
 arch/arm/mm/proc-macros.S       | 13 +++++++++++++
 3 files changed, 44 insertions(+)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 95cbe9d..3126557 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1087,6 +1087,15 @@ if !MMU
 source "arch/arm/Kconfig-nommu"
 endif
 
+config PJ4B_ERRATA_4611
+	bool "PJ4B Errata 4611: A Deadlock can Occur if a CP15 Clean/Clean and Invalidate Operation is Hit By a Snoop Event"
+	depends on CPU_PJ4B && MACH_ARMADA_370
+	help
+	  A CP15 clean operation can result in a dead lock state if it is hit
+	  by an incoming snoop event.  Workaround: Before any CP15 clean type
+	  operation in Cache Coherency mode, issue a Data Memory Barrier (DMB)
+	  or a Data Synchronization Barrier (DSB) instruction.
+
 config PJ4B_ERRATA_4742
 	bool "PJ4B Errata 4742: IDLE Wake Up Commands can Cause the CPU Core to Cease Operation"
 	depends on CPU_PJ4B && MACH_ARMADA_370
diff --git a/arch/arm/include/asm/tlbflush.h b/arch/arm/include/asm/tlbflush.h
index a3625d1..f9129e4 100644
--- a/arch/arm/include/asm/tlbflush.h
+++ b/arch/arm/include/asm/tlbflush.h
@@ -475,7 +475,18 @@ static inline void flush_pmd_entry(void *pmd)
 {
 	const unsigned int __tlb_flag = __cpu_tlb_flags;
 
+#ifdef CONFIG_PJ4B_ERRATA_4611
+	unsigned long flags;
+	raw_local_irq_save(flags);
+	dmb();
+#endif
+
 	tlb_op(TLB_DCLEAN, "c7, c10, 1	@ flush_pmd", pmd);
+
+#ifdef CONFIG_PJ4B_ERRATA_4611
+	raw_local_irq_restore(flags);
+#endif
+
 	tlb_l2_op(TLB_L2CLEAN_FR, "c15, c9, 1  @ L2 flush_pmd", pmd);
 
 	if (tlb_flag(TLB_WB))
@@ -486,7 +497,18 @@ static inline void clean_pmd_entry(void *pmd)
 {
 	const unsigned int __tlb_flag = __cpu_tlb_flags;
 
+#ifdef CONFIG_PJ4B_ERRATA_4611
+	unsigned long flags;
+	raw_local_irq_save(flags);
+	dmb();
+#endif
+
 	tlb_op(TLB_DCLEAN, "c7, c10, 1	@ flush_pmd", pmd);
+
+#ifdef CONFIG_PJ4B_ERRATA_4611
+	raw_local_irq_restore(flags);
+#endif
+
 	tlb_l2_op(TLB_L2CLEAN_FR, "c15, c9, 1  @ L2 flush_pmd", pmd);
 }
 
diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S
index f9a0aa7..9930e14 100644
--- a/arch/arm/mm/proc-macros.S
+++ b/arch/arm/mm/proc-macros.S
@@ -178,7 +178,20 @@
 #endif
 
 	str	r3, [r0]
+
+#ifdef CONFIG_PJ4B_ERRATA_4611
+	mrs     r2, cpsr
+	orr     r3, r2, #PSR_F_BIT | PSR_I_BIT
+	msr     cpsr_c, r3			@ Disable interrupts
+	dmb					@ ensure ordering with previous memory accesses
+#endif
+
 	mcr	p15, 0, r0, c7, c10, 1		@ flush_pte
+
+#ifdef CONFIG_PJ4B_ERRATA_4611
+	msr     cpsr_c, r2			@ Restore interrupts
+	dsb					@ drain write buffer
+#endif
 	.endm
 
 
-- 
1.8.1.2

[PATCH v2 1/2] ARM PJ4B: Add support for errata 4742

[Russell King - ARM Linux]

On Tue, Jun 04, 2013 at 04:03:17PM +0200, Gregory CLEMENT wrote:
> From: Lior Amsalem <alior@marvell.com>
> 
> This commit fixes the regression on Armada 370 (the kernal hang during
> boot) introduced by the commit: "ARM: 7691/1: mm: kill unused
> TLB_CAN_READ_FROM_L1_CACHE and use ALT_SMP instead".
> 
> When coming out of either a Wait for Interrupt (WFI) or a Wait for
> Event (WFE) IDLE states, a specific timing sensitivity exists between
> the retiring WFI/WFE instructions and the newly issued subsequent
> instructions. This sensitivity can result in a CPU hang scenario.  The
> workaround is to insert either a Data Synchronization Barrier (DSB) or
> Data Memory Barrier (DMB) command immediately after the WFI/WFE
> instruction

Wouldn't a DMB be cheaper than a full blown DSB here?

Also, we could detect the PJ4B and only implement the workaround
on those CPUs rather than having to have it on every v7 CPU - which
is important when you consider the single zImage issues.

[PATCH v2 2/2] ARM PJ4B: Add support for errata 4611

[Will Deacon]

On Tue, Jun 04, 2013 at 03:03:18PM +0100, Gregory CLEMENT wrote:
> From: Lior Amsalem <alior@marvell.com>
> 
> A CP15 clean operation can result in a dead lock state if it is hit by
> an incoming snoop event. The fix to this issue is the following:
> before any CP15 clean type operation in Cache Coherency mode, issue a
> Data Memory Barrier (DMB) or a Data Synchronization Barrier (DSB)
> instruction.
> 
> [gregory.clement at free-electrons.com:add errata description in changelog]
> [gregory.clement at free-electrons.com:make this errata depend on Armada 370]
> Signed-off-by: Lior Amsalem <alior@marvell.com>
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>

[...]

> diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S
> index f9a0aa7..9930e14 100644
> --- a/arch/arm/mm/proc-macros.S
> +++ b/arch/arm/mm/proc-macros.S
> @@ -178,7 +178,20 @@
>  #endif
>  
>  	str	r3, [r0]
> +
> +#ifdef CONFIG_PJ4B_ERRATA_4611
> +	mrs     r2, cpsr
> +	orr     r3, r2, #PSR_F_BIT | PSR_I_BIT
> +	msr     cpsr_c, r3			@ Disable interrupts
> +	dmb					@ ensure ordering with previous memory accesses
> +#endif
> +
>  	mcr	p15, 0, r0, c7, c10, 1		@ flush_pte
> +
> +#ifdef CONFIG_PJ4B_ERRATA_4611
> +	msr     cpsr_c, r2			@ Restore interrupts
> +	dsb					@ drain write buffer
> +#endif
>  	.endm
> 

There are save/restore IRQ macros in assembler.h, so you can use those here
instead of open-coding the cpsr manipulation. Sorry for missing this before.

Will

[PATCH v2 1/2] ARM PJ4B: Add support for errata 4742

[Gregory CLEMENT]

Dear Russell King,

On 06/04/2013 04:59 PM, Russell King - ARM Linux wrote:
> On Tue, Jun 04, 2013 at 04:03:17PM +0200, Gregory CLEMENT wrote:
>> From: Lior Amsalem <alior@marvell.com>
>>
>> This commit fixes the regression on Armada 370 (the kernal hang during
>> boot) introduced by the commit: "ARM: 7691/1: mm: kill unused
>> TLB_CAN_READ_FROM_L1_CACHE and use ALT_SMP instead".
>>
>> When coming out of either a Wait for Interrupt (WFI) or a Wait for
>> Event (WFE) IDLE states, a specific timing sensitivity exists between
>> the retiring WFI/WFE instructions and the newly issued subsequent
>> instructions. This sensitivity can result in a CPU hang scenario.  The
>> workaround is to insert either a Data Synchronization Barrier (DSB) or
>> Data Memory Barrier (DMB) command immediately after the WFI/WFE
>> instruction
> 
> Wouldn't a DMB be cheaper than a full blown DSB here?

I let Lior answer to this question.

> 
> Also, we could detect the PJ4B and only implement the workaround
> on those CPUs rather than having to have it on every v7 CPU - which
> is important when you consider the single zImage issues.

At least we can detect the PJ4B and maybe even the detect the if it
is the PJ4B or the PJ4B-MP.

Do you mean to add this test in the cpu_v7_do_idle() function?

Or patching the pointer to the cpu_v7_do_idle() on the fly? In this
last case it seems a little tricky to me, as we should have to modify
a pointer function in the .proc.info.init section.

Thanks for your review,

Gregory

[PATCH v2 1/2] ARM PJ4B: Add support for errata 4742

[Will Deacon]

On Tue, Jun 04, 2013 at 06:19:35PM +0100, Gregory CLEMENT wrote:
> On 06/04/2013 04:59 PM, Russell King - ARM Linux wrote:
> > On Tue, Jun 04, 2013 at 04:03:17PM +0200, Gregory CLEMENT wrote:
> >> From: Lior Amsalem <alior@marvell.com>
> >>
> >> This commit fixes the regression on Armada 370 (the kernal hang during
> >> boot) introduced by the commit: "ARM: 7691/1: mm: kill unused
> >> TLB_CAN_READ_FROM_L1_CACHE and use ALT_SMP instead".
> >>
> >> When coming out of either a Wait for Interrupt (WFI) or a Wait for
> >> Event (WFE) IDLE states, a specific timing sensitivity exists between
> >> the retiring WFI/WFE instructions and the newly issued subsequent
> >> instructions. This sensitivity can result in a CPU hang scenario.  The
> >> workaround is to insert either a Data Synchronization Barrier (DSB) or
> >> Data Memory Barrier (DMB) command immediately after the WFI/WFE
> >> instruction
> > 
> > Wouldn't a DMB be cheaper than a full blown DSB here?
> 
> I let Lior answer to this question.
> 
> > 
> > Also, we could detect the PJ4B and only implement the workaround
> > on those CPUs rather than having to have it on every v7 CPU - which
> > is important when you consider the single zImage issues.
> 
> At least we can detect the PJ4B and maybe even the detect the if it
> is the PJ4B or the PJ4B-MP.
> 
> Do you mean to add this test in the cpu_v7_do_idle() function?

I don't like the sound of that -- coming out of idle is basically all
overhead, so adding to it doesn't sound pleasant. Also, given that we won't
actually have any pending memory transactions at this point, a dsb needn't
be too expensive because there's nothing to make visible (obviously requires
benchmarking).

An alternative is disabling wfi/wfe on your core, if that's possible.

> Or patching the pointer to the cpu_v7_do_idle() on the fly? In this
> last case it seems a little tricky to me, as we should have to modify
> a pointer function in the .proc.info.init section.

Maybe you could hack glue-proc.h to define MULTI_CPU if this workaround is
selected, then patch the pointer in struct processor. (apologies if this
suggestion made you ill :).

Will

[PATCH v2 1/2] ARM PJ4B: Add support for errata 4742

[Russell King - ARM Linux]

On Tue, Jun 04, 2013 at 07:19:35PM +0200, Gregory CLEMENT wrote:
> At least we can detect the PJ4B and maybe even the detect the if it
> is the PJ4B or the PJ4B-MP.
> 
> Do you mean to add this test in the cpu_v7_do_idle() function?
> 
> Or patching the pointer to the cpu_v7_do_idle() on the fly? In this
> last case it seems a little tricky to me, as we should have to modify
> a pointer function in the .proc.info.init section.

No.  We already have a pj4 cpu type entry in the tables, so we just need
to specify a separate set of processor functions.  This isn't 100%
trivial to do with the macro-isation of stuff, but here's an attempt
(untested):

 arch/arm/mm/proc-v7-2level.S |    4 ++++
 arch/arm/mm/proc-v7-3level.S |    4 ++++
 arch/arm/mm/proc-v7.S        |   28 +++++++++++++++++++++++++---
 3 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mm/proc-v7-2level.S b/arch/arm/mm/proc-v7-2level.S
index 9704097..d03c15c 100644
--- a/arch/arm/mm/proc-v7-2level.S
+++ b/arch/arm/mm/proc-v7-2level.S
@@ -38,6 +38,7 @@
  *	- we are not using split page tables
  */
 ENTRY(cpu_v7_switch_mm)
+ENTRY(cpu_pj4b_switch_mm)
 #ifdef CONFIG_MMU
 	mov	r2, #0
 	mmid	r1, r1				@ get mm->context.id
@@ -61,6 +62,7 @@ ENTRY(cpu_v7_switch_mm)
 #endif
 	mov	pc, lr
 ENDPROC(cpu_v7_switch_mm)
+ENDPROC(cpu_pj4b_switch_mm)
 
 /*
  *	cpu_v7_set_pte_ext(ptep, pte)
@@ -73,6 +75,7 @@ ENDPROC(cpu_v7_switch_mm)
  *	- ext	- value for extended PTE bits
  */
 ENTRY(cpu_v7_set_pte_ext)
+ENTRY(cpu_pj4b_set_pte_ext)
 #ifdef CONFIG_MMU
 	str	r1, [r0]			@ linux version
 
@@ -115,6 +118,7 @@ ENTRY(cpu_v7_set_pte_ext)
 #endif
 	mov	pc, lr
 ENDPROC(cpu_v7_set_pte_ext)
+ENDPROC(cpu_pj4b_set_pte_ext)
 
 	/*
 	 * Memory region attributes with SCTLR.TRE=1
diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S
index 363027e..2d819ff 100644
--- a/arch/arm/mm/proc-v7-3level.S
+++ b/arch/arm/mm/proc-v7-3level.S
@@ -46,6 +46,7 @@
  * the new TTB).
  */
 ENTRY(cpu_v7_switch_mm)
+ENTRY(cpu_pj4b_switch_mm)
 #ifdef CONFIG_MMU
 	mmid	r1, r1				@ get mm->context.id
 	asid	r3, r1
@@ -55,6 +56,7 @@ ENTRY(cpu_v7_switch_mm)
 #endif
 	mov	pc, lr
 ENDPROC(cpu_v7_switch_mm)
+ENDPROC(cpu_pj4b_switch_mm)
 
 /*
  * cpu_v7_set_pte_ext(ptep, pte)
@@ -64,6 +66,7 @@ ENDPROC(cpu_v7_switch_mm)
  * - pte - PTE value to store (64-bit in r2 and r3)
  */
 ENTRY(cpu_v7_set_pte_ext)
+ENTRY(cpu_pj4b_set_pte_ext)
 #ifdef CONFIG_MMU
 	tst	r2, #L_PTE_VALID
 	beq	1f
@@ -78,6 +81,7 @@ ENTRY(cpu_v7_set_pte_ext)
 #endif
 	mov	pc, lr
 ENDPROC(cpu_v7_set_pte_ext)
+ENDPROC(cpu_pj4b_set_pte_ext)
 
 	/*
 	 * Memory region attributes for LPAE (defined in pgtable-3level.h):
diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
index 2c73a73..9964e84 100644
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
@@ -26,16 +26,20 @@
 #endif
 
 ENTRY(cpu_v7_proc_init)
+ENTRY(cpu_pj4b_proc_init)
 	mov	pc, lr
 ENDPROC(cpu_v7_proc_init)
+ENDPROC(cpu_pj4b_proc_init)
 
 ENTRY(cpu_v7_proc_fin)
+ENTRY(cpu_pj4b_proc_fin)
 	mrc	p15, 0, r0, c1, c0, 0		@ ctrl register
 	bic	r0, r0, #0x1000			@ ...i............
 	bic	r0, r0, #0x0006			@ .............ca.
 	mcr	p15, 0, r0, c1, c0, 0		@ disable caches
 	mov	pc, lr
 ENDPROC(cpu_v7_proc_fin)
+ENDPROC(cpu_pj4b_proc_fin)
 
 /*
  *	cpu_v7_reset(loc)
@@ -52,6 +56,7 @@ ENDPROC(cpu_v7_proc_fin)
 	.align	5
 	.pushsection	.idmap.text, "ax"
 ENTRY(cpu_v7_reset)
+ENTRY(cpu_pj4b_reset)
 	mrc	p15, 0, r1, c1, c0, 0		@ ctrl register
 	bic	r1, r1, #0x1			@ ...............m
  THUMB(	bic	r1, r1, #1 << 30 )		@ SCTLR.TE (Thumb exceptions)
@@ -59,6 +64,7 @@ ENTRY(cpu_v7_reset)
 	isb
 	bx	r0
 ENDPROC(cpu_v7_reset)
+ENDPROC(cpu_pj4b_reset)
 	.popsection
 
 /*
@@ -74,7 +80,17 @@ ENTRY(cpu_v7_do_idle)
 	mov	pc, lr
 ENDPROC(cpu_v7_do_idle)
 
+ENTRY(cpu_pj4b_do_idle)
+	dsb
+	wfi
+#ifdef CONFIG_PJ4B_ERRATA_4742
+	dsb
+#endif
+	mov	pc, lr
+ENDPROC(cpu_pj4b_do_idle)
+
 ENTRY(cpu_v7_dcache_clean_area)
+ENTRY(cpu_pj4b_dcache_clean_area)
 	ALT_SMP(mov	pc, lr)			@ MP extensions imply L1 PTW
 	ALT_UP(W(nop))
 	dcache_line_size r2, r3
@@ -85,6 +101,7 @@ ENTRY(cpu_v7_dcache_clean_area)
 	dsb
 	mov	pc, lr
 ENDPROC(cpu_v7_dcache_clean_area)
+ENDPROC(cpu_pj4b_dcache_clean_area)
 
 	string	cpu_v7_name, "ARMv7 Processor"
 	.align
@@ -94,6 +111,7 @@ ENDPROC(cpu_v7_dcache_clean_area)
 .equ	cpu_v7_suspend_size, 4 * 8
 #ifdef CONFIG_ARM_CPU_SUSPEND
 ENTRY(cpu_v7_do_suspend)
+ENTRY(cpu_pj4b_do_suspend)
 	stmfd	sp!, {r4 - r10, lr}
 	mrc	p15, 0, r4, c13, c0, 0	@ FCSE/PID
 	mrc	p15, 0, r5, c13, c0, 3	@ User r/o thread ID
@@ -107,8 +125,10 @@ ENTRY(cpu_v7_do_suspend)
 	stmia	r0, {r6 - r11}
 	ldmfd	sp!, {r4 - r10, pc}
 ENDPROC(cpu_v7_do_suspend)
+ENDPROC(cpu_pj4b_do_suspend)
 
 ENTRY(cpu_v7_do_resume)
+ENTRY(cpu_pj4b_do_resume)
 	mov	ip, #0
 	mcr	p15, 0, ip, c8, c7, 0	@ invalidate TLBs
 	mcr	p15, 0, ip, c7, c5, 0	@ invalidate I cache
@@ -138,6 +158,7 @@ ENTRY(cpu_v7_do_resume)
 	mov	r0, r8			@ control register
 	b	cpu_resume_mmu
 ENDPROC(cpu_v7_do_resume)
+ENDPROC(cpu_pj4b_do_resume)
 #endif
 
 	__CPUINIT
@@ -350,6 +371,7 @@ __v7_setup_stack:
 
 	@ define struct processor (see <asm/proc-fns.h> and proc-macros.S)
 	define_processor_functions v7, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
+	define_processor_functions pj4b, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
 
 	.section ".rodata"
 
@@ -362,7 +384,7 @@ __v7_setup_stack:
 	/*
 	 * Standard v7 proc info content
 	 */
-.macro __v7_proc initfunc, mm_mmuflags = 0, io_mmuflags = 0, hwcaps = 0
+.macro __v7_proc initfunc, mm_mmuflags = 0, io_mmuflags = 0, hwcaps = 0, proc_fns = v7_processor_functions
 	ALT_SMP(.long	PMD_TYPE_SECT | PMD_SECT_AP_WRITE | PMD_SECT_AP_READ | \
 			PMD_SECT_AF | PMD_FLAGS_SMP | \mm_mmuflags)
 	ALT_UP(.long	PMD_TYPE_SECT | PMD_SECT_AP_WRITE | PMD_SECT_AP_READ | \
@@ -375,7 +397,7 @@ __v7_setup_stack:
 	.long	HWCAP_SWP | HWCAP_HALF | HWCAP_THUMB | HWCAP_FAST_MULT | \
 		HWCAP_EDSP | HWCAP_TLS | \hwcaps
 	.long	cpu_v7_name
-	.long	v7_processor_functions
+	.long	proc_fns
 	.long	v7wbi_tlb_fns
 	.long	v6_user_fns
 	.long	v7_cache_fns
@@ -411,7 +433,7 @@ __v7_ca9mp_proc_info:
 __v7_pj4b_proc_info:
 	.long	0x562f5840
 	.long	0xfffffff0
-	__v7_proc __v7_pj4b_setup
+	__v7_proc __v7_pj4b_setup, proc_fns = pj4b_processor_functions
 	.size	__v7_pj4b_proc_info, . - __v7_pj4b_proc_info
 
 	/*

[PATCH v2 1/2] ARM PJ4B: Add support for errata 4742

[Nicolas Pitre]

On Tue, 4 Jun 2013, Russell King - ARM Linux wrote:

> On Tue, Jun 04, 2013 at 07:19:35PM +0200, Gregory CLEMENT wrote:
> > At least we can detect the PJ4B and maybe even the detect the if it
> > is the PJ4B or the PJ4B-MP.
> > 
> > Do you mean to add this test in the cpu_v7_do_idle() function?
> > 
> > Or patching the pointer to the cpu_v7_do_idle() on the fly? In this
> > last case it seems a little tricky to me, as we should have to modify
> > a pointer function in the .proc.info.init section.
> 
> No.  We already have a pj4 cpu type entry in the tables, so we just need
> to specify a separate set of processor functions.  This isn't 100%
> trivial to do with the macro-isation of stuff, but here's an attempt

It could be nicer on the symbol table especially when debugging if the 
pj4b was made into a separate branch table e.g.:

ENTRY(cpu_pj4b_switch_mm)
	b	cpu_v7_switch_mm
ENDPROC(cpu_pj4b_switch_mm)

Otherwise the objdump output will always select the first symbol for any 
given address in alphabetical order, masking away cpu_v7_switch_mm.

Alternatively, the following could be done:

#ifdef CONFIG_CPU_PJ4B

	.equ	cpu_pj4b_switch_mm,	cpu_v7_switch_mm
	.equ	cpu_pj4b_set_pte_ex,	cpu_v7_set_pte_ex
	...

#ifdef CONFIG_PJ4B_ERRATA_4742
ENTRY(cpu_pj4b_do_idle)
	dsb
	wfi
	dsb
	mov     pc, lr
ENDPROC(cpu_pj4b_do_idle)
#else
	.equ	cpu_pj4b_do_idle,	cpu_v7_do_idle
#endif

#endif

That saves on the extra branch cost and allows for removing the pj4b 
symbols entirely when not configured in, and keeps the changes gathered 
together which is IMHO a little prettier.

> diff --git a/arch/arm/mm/proc-v7-2level.S b/arch/arm/mm/proc-v7-2level.S
> index 9704097..d03c15c 100644
> --- a/arch/arm/mm/proc-v7-2level.S
> +++ b/arch/arm/mm/proc-v7-2level.S
> @@ -38,6 +38,7 @@
>   *	- we are not using split page tables
>   */
>  ENTRY(cpu_v7_switch_mm)
> +ENTRY(cpu_pj4b_switch_mm)
>  #ifdef CONFIG_MMU
>  	mov	r2, #0
>  	mmid	r1, r1				@ get mm->context.id
> @@ -61,6 +62,7 @@ ENTRY(cpu_v7_switch_mm)
>  #endif
>  	mov	pc, lr
>  ENDPROC(cpu_v7_switch_mm)
> +ENDPROC(cpu_pj4b_switch_mm)
>  
>  /*
>   *	cpu_v7_set_pte_ext(ptep, pte)
> @@ -73,6 +75,7 @@ ENDPROC(cpu_v7_switch_mm)
>   *	- ext	- value for extended PTE bits
>   */
>  ENTRY(cpu_v7_set_pte_ext)
> +ENTRY(cpu_pj4b_set_pte_ext)
>  #ifdef CONFIG_MMU
>  	str	r1, [r0]			@ linux version
>  
> @@ -115,6 +118,7 @@ ENTRY(cpu_v7_set_pte_ext)
>  #endif
>  	mov	pc, lr
>  ENDPROC(cpu_v7_set_pte_ext)
> +ENDPROC(cpu_pj4b_set_pte_ext)
>  
>  	/*
>  	 * Memory region attributes with SCTLR.TRE=1
> diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S
> index 363027e..2d819ff 100644
> --- a/arch/arm/mm/proc-v7-3level.S
> +++ b/arch/arm/mm/proc-v7-3level.S
> @@ -46,6 +46,7 @@
>   * the new TTB).
>   */
>  ENTRY(cpu_v7_switch_mm)
> +ENTRY(cpu_pj4b_switch_mm)
>  #ifdef CONFIG_MMU
>  	mmid	r1, r1				@ get mm->context.id
>  	asid	r3, r1
> @@ -55,6 +56,7 @@ ENTRY(cpu_v7_switch_mm)
>  #endif
>  	mov	pc, lr
>  ENDPROC(cpu_v7_switch_mm)
> +ENDPROC(cpu_pj4b_switch_mm)
>  
>  /*
>   * cpu_v7_set_pte_ext(ptep, pte)
> @@ -64,6 +66,7 @@ ENDPROC(cpu_v7_switch_mm)
>   * - pte - PTE value to store (64-bit in r2 and r3)
>   */
>  ENTRY(cpu_v7_set_pte_ext)
> +ENTRY(cpu_pj4b_set_pte_ext)
>  #ifdef CONFIG_MMU
>  	tst	r2, #L_PTE_VALID
>  	beq	1f
> @@ -78,6 +81,7 @@ ENTRY(cpu_v7_set_pte_ext)
>  #endif
>  	mov	pc, lr
>  ENDPROC(cpu_v7_set_pte_ext)
> +ENDPROC(cpu_pj4b_set_pte_ext)
>  
>  	/*
>  	 * Memory region attributes for LPAE (defined in pgtable-3level.h):
> diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
> index 2c73a73..9964e84 100644
> --- a/arch/arm/mm/proc-v7.S
> +++ b/arch/arm/mm/proc-v7.S
> @@ -26,16 +26,20 @@
>  #endif
>  
>  ENTRY(cpu_v7_proc_init)
> +ENTRY(cpu_pj4b_proc_init)
>  	mov	pc, lr
>  ENDPROC(cpu_v7_proc_init)
> +ENDPROC(cpu_pj4b_proc_init)
>  
>  ENTRY(cpu_v7_proc_fin)
> +ENTRY(cpu_pj4b_proc_fin)
>  	mrc	p15, 0, r0, c1, c0, 0		@ ctrl register
>  	bic	r0, r0, #0x1000			@ ...i............
>  	bic	r0, r0, #0x0006			@ .............ca.
>  	mcr	p15, 0, r0, c1, c0, 0		@ disable caches
>  	mov	pc, lr
>  ENDPROC(cpu_v7_proc_fin)
> +ENDPROC(cpu_pj4b_proc_fin)
>  
>  /*
>   *	cpu_v7_reset(loc)
> @@ -52,6 +56,7 @@ ENDPROC(cpu_v7_proc_fin)
>  	.align	5
>  	.pushsection	.idmap.text, "ax"
>  ENTRY(cpu_v7_reset)
> +ENTRY(cpu_pj4b_reset)
>  	mrc	p15, 0, r1, c1, c0, 0		@ ctrl register
>  	bic	r1, r1, #0x1			@ ...............m
>   THUMB(	bic	r1, r1, #1 << 30 )		@ SCTLR.TE (Thumb exceptions)
> @@ -59,6 +64,7 @@ ENTRY(cpu_v7_reset)
>  	isb
>  	bx	r0
>  ENDPROC(cpu_v7_reset)
> +ENDPROC(cpu_pj4b_reset)
>  	.popsection
>  
>  /*
> @@ -74,7 +80,17 @@ ENTRY(cpu_v7_do_idle)
>  	mov	pc, lr
>  ENDPROC(cpu_v7_do_idle)
>  
> +ENTRY(cpu_pj4b_do_idle)
> +	dsb
> +	wfi
> +#ifdef CONFIG_PJ4B_ERRATA_4742
> +	dsb
> +#endif
> +	mov	pc, lr
> +ENDPROC(cpu_pj4b_do_idle)
> +
>  ENTRY(cpu_v7_dcache_clean_area)
> +ENTRY(cpu_pj4b_dcache_clean_area)
>  	ALT_SMP(mov	pc, lr)			@ MP extensions imply L1 PTW
>  	ALT_UP(W(nop))
>  	dcache_line_size r2, r3
> @@ -85,6 +101,7 @@ ENTRY(cpu_v7_dcache_clean_area)
>  	dsb
>  	mov	pc, lr
>  ENDPROC(cpu_v7_dcache_clean_area)
> +ENDPROC(cpu_pj4b_dcache_clean_area)
>  
>  	string	cpu_v7_name, "ARMv7 Processor"
>  	.align
> @@ -94,6 +111,7 @@ ENDPROC(cpu_v7_dcache_clean_area)
>  .equ	cpu_v7_suspend_size, 4 * 8
>  #ifdef CONFIG_ARM_CPU_SUSPEND
>  ENTRY(cpu_v7_do_suspend)
> +ENTRY(cpu_pj4b_do_suspend)
>  	stmfd	sp!, {r4 - r10, lr}
>  	mrc	p15, 0, r4, c13, c0, 0	@ FCSE/PID
>  	mrc	p15, 0, r5, c13, c0, 3	@ User r/o thread ID
> @@ -107,8 +125,10 @@ ENTRY(cpu_v7_do_suspend)
>  	stmia	r0, {r6 - r11}
>  	ldmfd	sp!, {r4 - r10, pc}
>  ENDPROC(cpu_v7_do_suspend)
> +ENDPROC(cpu_pj4b_do_suspend)
>  
>  ENTRY(cpu_v7_do_resume)
> +ENTRY(cpu_pj4b_do_resume)
>  	mov	ip, #0
>  	mcr	p15, 0, ip, c8, c7, 0	@ invalidate TLBs
>  	mcr	p15, 0, ip, c7, c5, 0	@ invalidate I cache
> @@ -138,6 +158,7 @@ ENTRY(cpu_v7_do_resume)
>  	mov	r0, r8			@ control register
>  	b	cpu_resume_mmu
>  ENDPROC(cpu_v7_do_resume)
> +ENDPROC(cpu_pj4b_do_resume)
>  #endif
>  
>  	__CPUINIT
> @@ -350,6 +371,7 @@ __v7_setup_stack:
>  
>  	@ define struct processor (see <asm/proc-fns.h> and proc-macros.S)
>  	define_processor_functions v7, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
> +	define_processor_functions pj4b, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
>  
>  	.section ".rodata"
>  
> @@ -362,7 +384,7 @@ __v7_setup_stack:
>  	/*
>  	 * Standard v7 proc info content
>  	 */
> -.macro __v7_proc initfunc, mm_mmuflags = 0, io_mmuflags = 0, hwcaps = 0
> +.macro __v7_proc initfunc, mm_mmuflags = 0, io_mmuflags = 0, hwcaps = 0, proc_fns = v7_processor_functions
>  	ALT_SMP(.long	PMD_TYPE_SECT | PMD_SECT_AP_WRITE | PMD_SECT_AP_READ | \
>  			PMD_SECT_AF | PMD_FLAGS_SMP | \mm_mmuflags)
>  	ALT_UP(.long	PMD_TYPE_SECT | PMD_SECT_AP_WRITE | PMD_SECT_AP_READ | \
> @@ -375,7 +397,7 @@ __v7_setup_stack:
>  	.long	HWCAP_SWP | HWCAP_HALF | HWCAP_THUMB | HWCAP_FAST_MULT | \
>  		HWCAP_EDSP | HWCAP_TLS | \hwcaps
>  	.long	cpu_v7_name
> -	.long	v7_processor_functions
> +	.long	proc_fns
>  	.long	v7wbi_tlb_fns
>  	.long	v6_user_fns
>  	.long	v7_cache_fns
> @@ -411,7 +433,7 @@ __v7_ca9mp_proc_info:
>  __v7_pj4b_proc_info:
>  	.long	0x562f5840
>  	.long	0xfffffff0
> -	__v7_proc __v7_pj4b_setup
> +	__v7_proc __v7_pj4b_setup, proc_fns = pj4b_processor_functions
>  	.size	__v7_pj4b_proc_info, . - __v7_pj4b_proc_info
>  
>  	/*
> 

[PATCH v2 1/2] ARM PJ4B: Add support for errata 4742

[Russell King - ARM Linux]

On Tue, Jun 04, 2013 at 02:07:29PM -0400, Nicolas Pitre wrote:
> It could be nicer on the symbol table especially when debugging if the 
> pj4b was made into a separate branch table e.g.:
> 
> ENTRY(cpu_pj4b_switch_mm)
> 	b	cpu_v7_switch_mm
> ENDPROC(cpu_pj4b_switch_mm)
> 
> Otherwise the objdump output will always select the first symbol for any 
> given address in alphabetical order, masking away cpu_v7_switch_mm.

That adds unnecessary runtime expense.

> Alternatively, the following could be done:
> 
> #ifdef CONFIG_CPU_PJ4B
> 
> 	.equ	cpu_pj4b_switch_mm,	cpu_v7_switch_mm
> 	.equ	cpu_pj4b_set_pte_ex,	cpu_v7_set_pte_ex

No, that doesn't work.  I did forget a hunk from my patch though, which
needs the pj4b symbols to be global:

diff --git a/arch/arm/include/asm/glue-proc.h b/arch/arm/include/asm/glue-proc.h
index ac1dd54..8017e94 100644
--- a/arch/arm/include/asm/glue-proc.h
+++ b/arch/arm/include/asm/glue-proc.h
@@ -230,6 +230,15 @@
 # endif
 #endif
 
+#ifdef CONFIG_CPU_PJ4B
+# ifdef CPU_NAME
+#  undef  MULTI_CPU
+#  define MULTI_CPU
+# else
+#  define CPU_NAME cpu_pj4b
+# endif
+#endif
+
 #ifndef MULTI_CPU
 #define cpu_proc_init			__glue(CPU_NAME,_proc_init)
 #define cpu_proc_fin			__glue(CPU_NAME,_proc_fin)

[PATCH v2 1/2] ARM PJ4B: Add support for errata 4742

[Nicolas Pitre]

On Tue, 4 Jun 2013, Russell King - ARM Linux wrote:

> On Tue, Jun 04, 2013 at 02:07:29PM -0400, Nicolas Pitre wrote:
> > Alternatively, the following could be done:
> > 
> > #ifdef CONFIG_CPU_PJ4B
> > 
> > 	.equ	cpu_pj4b_switch_mm,	cpu_v7_switch_mm
> > 	.equ	cpu_pj4b_set_pte_ex,	cpu_v7_set_pte_ex
> 
> No, that doesn't work.  I did forget a hunk from my patch though, which
> needs the pj4b symbols to be global:

	.macro globl_equ x, y
	.globl	x
	.equ	x, y
	.endm

	.globl_equ	cpu_pj4b_switch_mm,     cpu_v7_switch_mm
	.globl_equ	cpu_pj4b_set_pte_ex,	cpu_v7_set_pte_ex
	...


Nicolas

[PATCH v2 1/2] ARM PJ4B: Add support for errata 4742

[Russell King - ARM Linux]

On Tue, Jun 04, 2013 at 02:19:32PM -0400, Nicolas Pitre wrote:
> On Tue, 4 Jun 2013, Russell King - ARM Linux wrote:
> 
> > On Tue, Jun 04, 2013 at 02:07:29PM -0400, Nicolas Pitre wrote:
> > > Alternatively, the following could be done:
> > > 
> > > #ifdef CONFIG_CPU_PJ4B
> > > 
> > > 	.equ	cpu_pj4b_switch_mm,	cpu_v7_switch_mm
> > > 	.equ	cpu_pj4b_set_pte_ex,	cpu_v7_set_pte_ex
> > 
> > No, that doesn't work.  I did forget a hunk from my patch though, which
> > needs the pj4b symbols to be global:
> 
> 	.macro globl_equ x, y
> 	.globl	x
> 	.equ	x, y
> 	.endm
> 
> 	.globl_equ	cpu_pj4b_switch_mm,     cpu_v7_switch_mm
> 	.globl_equ	cpu_pj4b_set_pte_ex,	cpu_v7_set_pte_ex

Which is really no different from doing what I did.

[PATCH v2 1/2] ARM PJ4B: Add support for errata 4742

[Nicolas Pitre]

On Tue, 4 Jun 2013, Russell King - ARM Linux wrote:

> On Tue, Jun 04, 2013 at 02:19:32PM -0400, Nicolas Pitre wrote:
> > On Tue, 4 Jun 2013, Russell King - ARM Linux wrote:
> > 
> > > On Tue, Jun 04, 2013 at 02:07:29PM -0400, Nicolas Pitre wrote:
> > > > Alternatively, the following could be done:
> > > > 
> > > > #ifdef CONFIG_CPU_PJ4B
> > > > 
> > > > 	.equ	cpu_pj4b_switch_mm,	cpu_v7_switch_mm
> > > > 	.equ	cpu_pj4b_set_pte_ex,	cpu_v7_set_pte_ex
> > > 
> > > No, that doesn't work.  I did forget a hunk from my patch though, which
> > > needs the pj4b symbols to be global:
> > 
> > 	.macro globl_equ x, y
> > 	.globl	x
> > 	.equ	x, y
> > 	.endm
> > 
> > 	.globl_equ	cpu_pj4b_switch_mm,     cpu_v7_switch_mm
> > 	.globl_equ	cpu_pj4b_set_pte_ex,	cpu_v7_set_pte_ex
> 
> Which is really no different from doing what I did.

This can be wrapped inside #ifdef CONFIG_CPU_PJ4B clearing the symbol 
aliasing confusion I mentioned when not configured, and can be done in a 
single location instead of being spread all over.  Other than that, this 
is functionally equivalent to what you did indeed.


Nicolas

[PATCH v2 1/2] ARM PJ4B: Add support for errata 4742

[Gregory CLEMENT]

On 06/04/2013 08:32 PM, Nicolas Pitre wrote:
> On Tue, 4 Jun 2013, Russell King - ARM Linux wrote:
> 
>> On Tue, Jun 04, 2013 at 02:19:32PM -0400, Nicolas Pitre wrote:
>>> On Tue, 4 Jun 2013, Russell King - ARM Linux wrote:
>>>
>>>> On Tue, Jun 04, 2013 at 02:07:29PM -0400, Nicolas Pitre wrote:
>>>>> Alternatively, the following could be done:
>>>>>
>>>>> #ifdef CONFIG_CPU_PJ4B
>>>>>
>>>>> 	.equ	cpu_pj4b_switch_mm,	cpu_v7_switch_mm
>>>>> 	.equ	cpu_pj4b_set_pte_ex,	cpu_v7_set_pte_ex
>>>>
>>>> No, that doesn't work.  I did forget a hunk from my patch though, which
>>>> needs the pj4b symbols to be global:
>>>
>>> 	.macro globl_equ x, y
>>> 	.globl	x
>>> 	.equ	x, y
>>> 	.endm
>>>
>>> 	.globl_equ	cpu_pj4b_switch_mm,     cpu_v7_switch_mm
>>> 	.globl_equ	cpu_pj4b_set_pte_ex,	cpu_v7_set_pte_ex
>>
>> Which is really no different from doing what I did.
> 
> This can be wrapped inside #ifdef CONFIG_CPU_PJ4B clearing the symbol 
> aliasing confusion I mentioned when not configured, and can be done in a 
> single location instead of being spread all over.  Other than that, this 
> is functionally equivalent to what you did indeed.
> 
> 
Russell, Nicolas,

Thanks for both of you for help. As you both agree that your variants are functionally
equivalent and as the variant of Nicolas seems to be more easy to debug, I will
adopt Nicolas' version. Unless, there was a drawback you didn't mention yet.

Regards,

Gregory