Hi Simon, >Hi Bernhard, > >On Thu, 30 Apr 2020 at 03:16, Bernhard Messerklinger ><[email protected]> wrote: >> >> Only load VBT if it's present in the u-boot.rom. >> > >I think you can drop the RFC from this series. The approach seems >good to me. > >Also, what APL boards have you tested with? I tested our custom APL smarc board. I also have a Oxbow Hill CRB but can't test it there at the moment (currently I am working from home).
> >> Signed-off-by: Bernhard Messerklinger ><[email protected]> >> --- >> >> arch/x86/cpu/apollolake/fsp_s.c | 18 ++++++++---------- >> 1 file changed, 8 insertions(+), 10 deletions(-) >> >> diff --git a/arch/x86/cpu/apollolake/fsp_s.c >b/arch/x86/cpu/apollolake/fsp_s.c >> index 17cf1682ad..8f1d6f3008 100644 >> --- a/arch/x86/cpu/apollolake/fsp_s.c >> +++ b/arch/x86/cpu/apollolake/fsp_s.c >> @@ -327,16 +327,17 @@ int fsps_update_config(struct udevice *dev, >ulong rom_offset, >> { >> struct fsp_s_config *cfg = &upd->config; >> struct apl_config *apl; >> +#ifdef CONFIG_HAVE_VBT > >Please use if (IS_ENABLED(CONFIG_HAVE_VBT)) > >We should try to avoid #ifdef unless needed as it adds different >build paths. > >> struct binman_entry vbt; >> - void *buf; >> + void *vbt_buf; >> int ret; >> >> ret = binman_entry_find("intel-vbt", &vbt); >> if (ret) >> return log_msg_ret("Cannot find VBT", ret); >> vbt.image_pos += rom_offset; >> - buf = malloc(vbt.size); >> - if (!buf) >> + vbt_buf = malloc(vbt.size); >> + if (!vbt_buf) >> return log_msg_ret("Alloc VBT", -ENOMEM); >> >> /* >> @@ -344,16 +345,13 @@ int fsps_update_config(struct udevice *dev, >ulong rom_offset, >> * memory-mapped SPI at present. >> */ >> bootstage_start(BOOTSTAGE_ID_ACCUM_MMAP_SPI, "mmap_spi"); >> - memcpy(buf, (void *)vbt.image_pos, vbt.size); >> + memcpy(vbt_buf, (void *)vbt.image_pos, vbt.size); >> bootstage_accum(BOOTSTAGE_ID_ACCUM_MMAP_SPI); >> - if (*(u32 *)buf != VBT_SIGNATURE) >> + if (*(u32 *)vbt_buf != VBT_SIGNATURE) >> return log_msg_ret("VBT signature", -EINVAL); >> - cfg->graphics_config_ptr = (ulong)buf; >> >> - apl = malloc(sizeof(*apl)); >> - if (!apl) >> - return log_msg_ret("config", -ENOMEM); >> - get_config(dev, apl); > >Should not drop the above code. > >> + cfg->graphics_config_ptr = (ulong)vbt_buf; >> +#endif >> >> cfg->ish_enable = 0; >> cfg->enable_sata = 0; >> -- >> 2.26.0 >> >> > >Regards, >Simon Regards, Bernhard
