Hi Simon, On Thu, Aug 24, 2023 at 2:48 AM Simon Glass <[email protected]> wrote: > > The current condition does not handle the samus_tpl case where it sets > up the RAM in SPL but needs to commit the MTRRs in U-Boot proper. > > Add another case to handle this and update the comment. > > Signed-off-by: Simon Glass <[email protected]> > --- > > arch/x86/lib/init_helpers.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/lib/init_helpers.c b/arch/x86/lib/init_helpers.c > index 60a2707dcf1..bf0c921577d 100644 > --- a/arch/x86/lib/init_helpers.c > +++ b/arch/x86/lib/init_helpers.c > @@ -15,7 +15,8 @@ DECLARE_GLOBAL_DATA_PTR; > int init_cache_f_r(void) > { > bool do_mtrr = CONFIG_IS_ENABLED(X86_32BIT_INIT) || > - IS_ENABLED(CONFIG_FSP_VERSION2); > + IS_ENABLED(CONFIG_FSP_VERSION2) || > + (IS_ENABLED(CONFIG_TPL) && IS_ENABLED(CONFIG_HAVE_MRC));
So this combination is only for Samus TPL case, not a generic one, right? I hate this as it's easy to break. Maybe we should be calling an API from the board specific codes to determine whether we should do the MTRR programming. Thoughts? > int ret; > > /* > @@ -23,11 +24,9 @@ int init_cache_f_r(void) > * > * booting from slimbootloader - MTRRs are already set up > * booting with FSPv1 - MTRRs are already set up > - * booting with FSPv2 - MTRRs must be set here > + * booting with FSPv2 or MRC - MTRRs must be set here > * booting from coreboot - in this case there is no SPL, so we set up > * the MTRRs here > - * Note: if there is an SPL, then it has already set up MTRRs so we > - * don't need to do that here > */ > do_mtrr &= !IS_ENABLED(CONFIG_FSP_VERSION1) && > !IS_ENABLED(CONFIG_SYS_SLIMBOOTLOADER); > -- Regards, Bin
