[U-Boot] [PATCH] omap3: ARM Cortex-A8 errata workarounds config option

[Paul Kocialkowski <contact@paulk.fr>]

Le dimanche 22 f?vrier 2015 ? 16:45 +0100, Paul Kocialkowski a ?crit :
> > > Workarounds applied in omap3_setup_aux_cr are only necessary for ARM core
> > > revisions prior to r3p2 (such as OMAP35xx but apparently not OMAP36xx and
> > > DM37xx)
> > 
> > If this is true, I can see some (potential) problems with the patch.
> > Please, check out my thoughts on this below.
> 
> In any case, those workarounds were already enabled before, so it won't
> cause any regression. As it is now, the patch doesn't change anything,
> it just opens up the possibility of not using
> CONFIG_SYS_ARM_CORTEXA8_ERRATA for a board that doesn't need the
> workarounds and has them disabled in the kernel.
> 
> > > and require similar workarounds in the kernel, or it will cause numerous
> > > segmentation faults. This allows (when the option is not used) properly booting
> > > kernels that do not include the workaround.
> > > 
> > > Follow-up to the discussion from July 2013:
> > > http://lists.denx.de/pipermail/u-boot/2013-July/158377.html
> > > 
> > > Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> > > ---
> > >  README                             | 8 ++++++++
> > >  arch/arm/cpu/armv7/omap3/board.c   | 2 ++
> > >  include/configs/am3517_crane.h     | 2 ++
> > >  include/configs/am3517_evm.h       | 2 ++
> > >  include/configs/cm_t35.h           | 2 ++
> > >  include/configs/cm_t3517.h         | 2 ++
> > >  include/configs/dig297.h           | 2 ++
> > >  include/configs/mcx.h              | 2 ++
> > >  include/configs/nokia_rx51.h       | 2 ++
> > >  include/configs/omap3_evm_common.h | 2 ++
> > >  include/configs/omap3_logic.h      | 2 ++
> > >  include/configs/omap3_mvblx.h      | 2 ++
> > >  include/configs/omap3_pandora.h    | 2 ++
> > >  include/configs/omap3_sdp3430.h    | 2 ++
> > >  include/configs/omap3_zoom1.h      | 2 ++
> > >  include/configs/tam3517-common.h   | 2 ++
> > >  include/configs/tao3530.h          | 2 ++
> > >  include/configs/ti_omap3_common.h  | 2 ++
> > >  include/configs/tricorder.h        | 2 ++
> > >  19 files changed, 44 insertions(+)
> > > 
> > > diff --git a/README b/README
> > > index ba57dc5..a39420d 100644
> > > --- a/README
> > > +++ b/README
> > > @@ -621,6 +621,14 @@ The following options need to be configured:
> > >  		exists, unlike the similar options in the Linux kernel. Do not
> > >  		set these options unless they apply!
> > >  
> > > +		CONFIG_SYS_ARM_CORTEXA8_ERRATA
> > > +
> > > +		Enables workarounds for ARM Cortex-A8 errata 454179, 430973
> > > +		and 621766. This is only necessary for ARM core revisions prior
> > > +		to r3p2. Enabling those workarounds requires to enable the same
> > > +		workarounds in the kernel, or it will cause multiple
> > > +		segmentation faults. This is currently only effective on OMAP3.
> > > +
> > >  - Driver Model
> > >  		Driver model is a new framework for devices in U-Boot
> > >  		introduced in early 2014. U-Boot is being progressively
> > > diff --git a/arch/arm/cpu/armv7/omap3/board.c b/arch/arm/cpu/armv7/omap3/board.c
> > > index 90d6ae7..813f35b 100644
> > > --- a/arch/arm/cpu/armv7/omap3/board.c
> > > +++ b/arch/arm/cpu/armv7/omap3/board.c
> > > @@ -246,8 +246,10 @@ void s_init(void)
> > >  
> > >  	try_unlock_memory();
> > >  
> > > +#ifdef CONFIG_SYS_ARM_CORTEXA8_ERRATA
> > >  	/* Errata workarounds */
> > >  	omap3_setup_aux_cr();
> > 
> > Can we have the revision dynamically checked instead of/along with
> > introducing a config option?
> > We have boards, OMAP3 based, which are supported by the same board file
> > and the same config file, see below.
> 
> After carefully looking at it, I think dynamically checking the revision
> might be the cleanest way to do this.
> 
> I was afraid that, in the case where the workarounds are disabled in
> u-boot but enabled in Linux, the same various segmentation faults would
> happen, but they apparently don't, as I just tested.
> 
> Thus, checking the revision should work best. According to the code in
> Linux, this only affects r1p* ARM core revisions, so that's what I'll be
> checking against.

Looking at the Linux code more closely, it shows that erratum 430973
only affects r1p* and erratum 458693 only affects r2p0.

Now in U-Boot, everything is mixed together and while erratum 458693 is
not mentionned, it does set the L1NEON bit to 1 like the workaround for
erratum 458693 does in Linux.

What do you think I should do here? Follow the Linux way (split it up)
or settle on applying both things for revisions <= r2p0.

> Thanks for your comments!
> 
> > > +#endif
> > >  
> > >  #ifndef CONFIG_SYS_L2CACHE_OFF
> > >  	/* Invalidate L2-cache from secure mode */
> > 
> > [...]
> > 
> > > diff --git a/include/configs/cm_t35.h b/include/configs/cm_t35.h
> > > index b2a9f35..150b419 100644
> > > --- a/include/configs/cm_t35.h
> > > +++ b/include/configs/cm_t35.h
> > > @@ -38,6 +38,8 @@
> > >  #define CONFIG_DISPLAY_CPUINFO
> > >  #define CONFIG_DISPLAY_BOARDINFO
> > >  
> > > +#define CONFIG_SYS_ARM_CORTEXA8_ERRATA
> > 
> > This config file is used for both cm-t35 and cm-t3730.
> > cm-t35 has OMAP3530, but cm-t3730 has DM3730. 
> > The same U-Boot binary is used for both boards.
> > 
> > > +
> > >  /* Clock Defines */
> > >  #define V_OSCK			26000000	/* Clock output from T2 */
> > >  #define V_SCLK			(V_OSCK >> 1)
> > 
> > [...]
> > 
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150222/1572486e/attachment.sig>