[U-Boot] [PATCH v2 1/2] Errata/ARM57: Add basic constructs to handle and apply A57 specific erratas

[Arnab Basu]

On Fri, Jan 16, 2015 at 2:10 AM, Scott Wood <scottwood at freescale.com> wrote:

> On Wed, 2015-01-14 at 19:16 +0530, Bhupesh Sharma wrote:
> > This patch adds basic constructs in the ARMv8 u-boot code
> > to handle and apply Cortex-A57 specific erratas.
> >
> > As and example, the framework showcases how erratas 833069, 826974
> > and 828024 can be handled and applied.
> >
> > Later on this framework can be extended to include other
> > erratas.
> >
> > Signed-off-by: Bhupesh Sharma <bhupesh.sharma at freescale.com>
> > ---
> > Changes from v1:
> >       - Addressed York's comment about x29 usage and calling the
> >         core errata fxup function before the lowlevel_init function
> >         is called.
> >
> >  arch/arm/cpu/armv8/start.S   |   51
> ++++++++++++++++++++++++++++++++++++++++++
> >  arch/arm/include/asm/macro.h |   20 +++++++++++++++++
> >  2 files changed, 71 insertions(+)
> >
> > diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S
> > index 4b11aa4..df532f9 100644
> > --- a/arch/arm/cpu/armv8/start.S
> > +++ b/arch/arm/cpu/armv8/start.S
> > @@ -67,6 +67,9 @@ reset:
> >       msr     cpacr_el1, x0                   /* Enable FP/SIMD */
> >  0:
> >
> > +     /* Apply ARM core specific erratas */
> > +     bl      apply_core_errata
> > +
> >       /*
> >        * Cache/BPB/TLB Invalidate
> >        * i-cache is invalidated before enabled in icache_enable()
> > @@ -97,6 +100,54 @@ master_cpu:
> >
> >
> /*-----------------------------------------------------------------------*/
> >
> > +WEAK(apply_core_errata)
> > +
> > +     /* For now, we support Cortex-A57 specific errata only */
> > +
> > +     /* Check if we are running on a Cortex-A57 core */
> > +     branch_if_a57_core x0, 1f
> > +     b       2f
> > +1:
> > +     bl      apply_a57_core_errata
> > +
> > +2:
>
> This is awkward.  How often are you expecting to use this macro?  Why
> not just open-code it?  Or at least make it "branch_if_not_a57_core".
> Or fix your lr problem by doing "branch_if_a57_core
> apply_a57_core_errata" and having it branch back rather than "ret".
>
> But I think opencoding would be best.  Just define symbols for the
> actual core IDs.
>

How about implementing a table mapping core IDs to entry points (of the
"apply_*_core_errata" fns)
and iterating over that table here? That will make it easy to add new cores
in future too.

Thanks
Arnab

>
> > +WEAK(apply_a57_core_errata)
>
> Why is this weak?
>
> -Scott
>
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>