[U-Boot] [PATCH v4 03/10] Exynos542x: Add workaround for ARM errata 798870

[Akshay Saraswat]

Hi Nishanth,

>On 17:13-20150224, Nishanth Menon wrote:
>> On 13:27-20150220, Akshay Saraswat wrote:
>> > This patch adds workaround for ARM errata 798870 which says
>> > "If back-to-back speculative cache line fills (fill A and fill B) are
>> > issued from the L1 data cache of a CPU to the L2 cache, the second
>> > request (fill B) is then cancelled, and the second request would have
>> > detected a hazard against a recent write or eviction (write B) to the
>> > same cache line as fill B then the L2 logic might deadlock."
>> > 
>> > Signed-off-by: Kimoon Kim <kimoon.kim at samsung.com>
>> > Signed-off-by: Akshay Saraswat <akshay.s at samsung.com>
>> > Reviewed-by: Simon Glass <sjg at chromium.org>
>> > Tested-by: Simon Glass <sjg at chromium.org>
>> > ---
>> > Changes since v3:
>> > 	- Added errata number in comment.
>> > 	- Moved changes to arm generic armv7.h
>> > 
>> > Changes since v2:
>> > 	- No change.
>> > 
>> > Changes since v1:
>> > 	- Added Reviewed-by & Tested-by.
>> > 	- Added space before */ on line # 40.
>> > 
>> >  arch/arm/include/asm/armv7.h | 16 ++++++++++++++++
>> >  1 file changed, 16 insertions(+)
>> > 
>> > diff --git a/arch/arm/include/asm/armv7.h b/arch/arm/include/asm/armv7.h
>> > index a13da23..a2040b7 100644
>> > --- a/arch/arm/include/asm/armv7.h
>> > +++ b/arch/arm/include/asm/armv7.h
>> > @@ -69,6 +69,22 @@
>> >  #define CP15DSB	asm volatile ("mcr     p15, 0, %0, c7, c10, 4" : : "r" (0))
>> >  #define CP15DMB	asm volatile ("mcr     p15, 0, %0, c7, c10, 5" : : "r" (0))
>> >  
>> > +/*
>> > + * Workaround for ARM errata # 798870
>> > + * Set L2ACTLR[7] to reissue any memory transaction in the L2 that has been
>> > + * stalled for 1024 cycles to verify that its hazard condition still exists.
>> > + */
>> > +static inline void v7_enable_l2_hazard_detect(void)
>> > +{
>> > +	uint32_t val;
>> > +
>> > +	/* L2ACTLR[7]: Enable hazard detect timeout */
>> > +	asm volatile ("mrc     p15, 1, %0, c15, c0, 0\n\t" : "=r"(val));
>> > +	val |= (1 << 7);
>> > +	asm volatile ("mcr     p15, 1, %0, c15, c0, 0\n\t" : : "r"(val));
>> 
>> This wont work for us in DRA7/OMAP5 L2ACTLR cannot be modified by
>> u-boot. has to go to secure world using smc call.
>> 
>> > +}
>> > +
>> > +void v7_en_l2_hazard_detect(void);
>> >  void v7_outer_cache_enable(void);
>> >  void v7_outer_cache_disable(void);
>> >  void v7_outer_cache_flush_all(void);
>> 
>> How about this - using the series:
>> https://patchwork.ozlabs.org/patch/443261/                
>> https://patchwork.ozlabs.org/patch/443264/                
>> https://patchwork.ozlabs.org/patch/443268/                
>> https://patchwork.ozlabs.org/patch/443265/                
>> https://patchwork.ozlabs.org/patch/443263/                
>> https://patchwork.ozlabs.org/patch/443262/                
>> https://patchwork.ozlabs.org/patch/443267/                
>> https://patchwork.ozlabs.org/patch/443266/                
>> https://patchwork.ozlabs.org/patch/443260/                
>
>I think the following might work for exynos?
>

Yes, that will probably do and if you look at v3 of this patch,
we were doing something similar. But I was convinced with the
below mentioned comments I recieved over v3, so updated it.

" Having the ARM errata number mentioned in the comment here would make
the purpose of this code much more clear to anyone looking at it later
in the future.
Also isn't this a general purpose Cortex-A15 r2pX workaround,
also potentially useful for the other non-Exynos SoCs too? "

I don't think it hurts to have a generic function with ARM errata workaround
implementation. Whoever wish to use it can call it in their boot path. And it's
not even getting executed right now for any SoC other than Exynos542x, so those
who don't want it need not bother about it. This was the intention. :)

Please let me know if you still want me to move it from armv7.h to exynos/lowlevel_init.c

>diff --git a/arch/arm/cpu/armv7/exynos/lowlevel_init.c b/arch/arm/cpu/armv7/exynos/lowlevel_init.c
>index 83e1dcfc1e13..0157105524ca 100644
>--- a/arch/arm/cpu/armv7/exynos/lowlevel_init.c
>+++ b/arch/arm/cpu/armv7/exynos/lowlevel_init.c
>@@ -42,6 +42,23 @@ enum {
> 	DO_POWER	= 1 << 4,
> };
> 
>+#ifdef CONFIG_ARM_ARCH_CP15_ERRATA
>+void arch_cp15_errata_workaround(u32 midr, u32 variant, u32 rev, u32 comb)
>+{
>+#ifdef CONFIG_ARM_ERRATA_798870
>+	if (comb >= 0x20 && comb < 0x30) {
>+		uint32_t l2actlr;
>+
>+		mrc_l2_aux_ctlr(l2actlr);
>+		/* Disable ACE DVM/CMO message broadcas */
>+		l2actlr |= (1 << 7);
>+		mcr_l2_aux_ctlr(l2actlr);
>+		mrc_l2_ctlr(l2actlr);
>+	}
>+#endif
>+}
>+#endif
>+
> int do_lowlevel_init(void)
> {
> 	uint32_t reset_status;
>-- 
>Regards,
>Nishanth Menon

Regards,
Akshay Saraswat