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

[Nishanth Menon]

On Wed, Feb 25, 2015 at 2:27 AM, Akshay Saraswat <akshay.s at samsung.com> wrote:
> 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
>

I think I can do a V3 on my series for a middle path between the two
approaches. Assuming that exynos does not need any specific custom smc
call for the l2actlr update. will split every erratum into a separate
patch of it's own.


>>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
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot



-- 
---
Regards,
Nishanth Menon