[U-Boot] [PATCH 7/8] x86: fsp: Move VPD/UPD verification to update_fsp_configs()

[Simon Glass]

Hi Bin,

On 2 December 2015 at 22:22, Bin Meng <bmeng.cn at gmail.com> wrote:
> Hi Simon,
>
> On Thu, Dec 3, 2015 at 5:05 AM, Simon Glass <sjg at chromium.org> wrote:
>> Hi Bin,
>>
>> On 2 December 2015 at 01:59, Bin Meng <bmeng.cn at gmail.com> wrote:
>>> Since VPD/UPD may not exist on every FSP, move the codes that
>>> verifies VPD/UPD to chipset-specific update_fsp_configs().
>>> This also updates update_fsp_configs() signature to accpect
>>> FSP runtime buffer pointer as its second parameter.
>>>
>>> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
>>> ---
>>>
>>>  arch/x86/cpu/baytrail/fsp_configs.c    | 24 +++++++++++++++++++++++-
>>>  arch/x86/cpu/queensbay/fsp_configs.c   | 26 +++++++++++++++++++++++++-
>>>  arch/x86/include/asm/fsp/fsp_support.h |  4 +++-
>>>  arch/x86/lib/fsp/fsp_support.c         | 24 +-----------------------
>>>  4 files changed, 52 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/arch/x86/cpu/baytrail/fsp_configs.c b/arch/x86/cpu/baytrail/fsp_configs.c
>>> index 9810921..3a03392 100644
>>> --- a/arch/x86/cpu/baytrail/fsp_configs.c
>>> +++ b/arch/x86/cpu/baytrail/fsp_configs.c
>>> @@ -125,13 +125,35 @@ const struct pch_azalia_config azalia_config = {
>>>   * If the device tree does not specify an integer setting, use the default
>>>   * provided in Intel's Baytrail_FSP_Gold4.tgz release FSP/BayleyBayFsp.bsf file.
>>>   */
>>> -void update_fsp_configs(struct fsp_config_data *config)
>>> +void update_fsp_configs(struct fsp_config_data *config,
>>> +                       struct fspinit_rtbuf *rt_buf)
>>>  {
>>> +       struct fsp_header *fsp_hdr = config->fsp_hdr;
>>> +       struct vpd_region *fsp_vpd;
>>>         struct upd_region *fsp_upd = &config->fsp_upd;
>>>         struct memory_down_data *mem;
>>>         const void *blob = gd->fdt_blob;
>>>         int node;
>>>
>>> +       /* Initialize runtime buffer for fsp_init() */
>>> +       rt_buf->common.stack_top = config->stack_top - 32;
>>> +       rt_buf->common.boot_mode = config->boot_mode;
>>> +       rt_buf->common.upd_data = &config->fsp_upd;
>>> +
>>> +       /* Get VPD region start */
>>> +       fsp_vpd = (struct vpd_region *)(fsp_hdr->img_base +
>>> +                       fsp_hdr->cfg_region_off);
>>> +
>>> +       /* Verify the VPD data region is valid */
>>> +       assert(fsp_vpd->sign == VPD_IMAGE_ID);
>>> +
>>> +       /* Copy default data from Flash */
>>> +       memcpy(fsp_upd, (void *)(fsp_hdr->img_base + fsp_vpd->upd_offset),
>>> +              sizeof(struct upd_region));
>>> +
>>> +       /* Verify the UPD data region is valid */
>>> +       assert(fsp_upd->terminator == UPD_TERMINATOR);
>>> +
>>
>> Maybe rather than duplicating this code, it can go in a common
>> function that is called for these two boards?
>>
>
> Yes, we can create a common function to do these, but that requires we
> create a Kconfig option something like CONFIG_FSP_USE_UPD or
> CONFIG_FSP_NO_UPD to wrap these codes in arch/x86/lib/fsp_support.c. I
> don't want to create a Kconfig option so chose to duplicate the codes.
> What do you think?

Well I dislike duplicated code more, so I think CONFIG_FSP_USE_UPD
might be best.

>
> [snip]
>
> Regards,
> Bin

Regards,
Simon