[RFC PATCH 1/2] arch: x86: apl: Only load VBT if CONFIG_HAVE_VBT is enabled

[Simon Glass]

Hi Bernhard,

On Thu, 30 Apr 2020 at 03:16, Bernhard Messerklinger
<bernhard.messerklinger at br-automation.com> 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?

> Signed-off-by: Bernhard Messerklinger <bernhard.messerklinger at br-automation.com>
> ---
>
>  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