diff mbox

[U-Boot] x86: Clean up the FSP support codes

Message ID 1418530514-5228-1-git-send-email-bmeng.cn@gmail.com
State Superseded
Delegated to: Simon Glass
Headers show

Commit Message

Bin Meng Dec. 14, 2014, 4:15 a.m. UTC
This is the follow-on patch to clean up the FSP support codes:

- Remove the _t suffix on the structures defines
- Use U-Boot's assert()
- Use standard bool true/false
- Remove read_unaligned64()
- Use memcmp() in the compare_guid()
- Remove the cast in the memset() call
- Replace some magic numbers with macros
- Use panic() when no valid FSP image header is found
- Change some FSP utility routines to use an fsp_ prefix
- Add comment blocks for asm_continuation and fsp_init_done
- Add comments to mention find_fsp_header() may be called in a
  stackless environment
- Add comments to mention init(&params) in fsp_init() cannot
  be removed

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---

 arch/x86/cpu/queensbay/fsp_configs.c               |   2 +-
 arch/x86/cpu/queensbay/fsp_support.c               | 216 +++++++++------------
 arch/x86/cpu/queensbay/tnc_dram.c                  |   6 +-
 arch/x86/include/asm/arch-queensbay/fsp/fsp_api.h  |  14 +-
 arch/x86/include/asm/arch-queensbay/fsp/fsp_ffs.h  |  24 +--
 arch/x86/include/asm/arch-queensbay/fsp/fsp_fv.h   |  14 +-
 arch/x86/include/asm/arch-queensbay/fsp/fsp_hob.h  |  52 ++---
 .../asm/arch-queensbay/fsp/fsp_infoheader.h        |   2 +-
 .../include/asm/arch-queensbay/fsp/fsp_platform.h  |   4 +-
 .../include/asm/arch-queensbay/fsp/fsp_support.h   |  63 +++---
 .../x86/include/asm/arch-queensbay/fsp/fsp_types.h |  17 +-
 arch/x86/include/asm/arch-queensbay/fsp/fsp_vpd.h  |   6 +-
 arch/x86/lib/cmd_hob.c                             |   2 +-
 13 files changed, 198 insertions(+), 224 deletions(-)

Comments

Simon Glass Dec. 14, 2014, 6:02 a.m. UTC | #1
Hi Bin,

On 13 December 2014 at 21:15, Bin Meng <bmeng.cn@gmail.com> wrote:
> This is the follow-on patch to clean up the FSP support codes:
>
> - Remove the _t suffix on the structures defines
> - Use U-Boot's assert()
> - Use standard bool true/false
> - Remove read_unaligned64()
> - Use memcmp() in the compare_guid()
> - Remove the cast in the memset() call
> - Replace some magic numbers with macros
> - Use panic() when no valid FSP image header is found
> - Change some FSP utility routines to use an fsp_ prefix
> - Add comment blocks for asm_continuation and fsp_init_done
> - Add comments to mention find_fsp_header() may be called in a
>   stackless environment
> - Add comments to mention init(&params) in fsp_init() cannot
>   be removed
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
>
>  arch/x86/cpu/queensbay/fsp_configs.c               |   2 +-
>  arch/x86/cpu/queensbay/fsp_support.c               | 216 +++++++++------------
>  arch/x86/cpu/queensbay/tnc_dram.c                  |   6 +-
>  arch/x86/include/asm/arch-queensbay/fsp/fsp_api.h  |  14 +-
>  arch/x86/include/asm/arch-queensbay/fsp/fsp_ffs.h  |  24 +--
>  arch/x86/include/asm/arch-queensbay/fsp/fsp_fv.h   |  14 +-
>  arch/x86/include/asm/arch-queensbay/fsp/fsp_hob.h  |  52 ++---
>  .../asm/arch-queensbay/fsp/fsp_infoheader.h        |   2 +-
>  .../include/asm/arch-queensbay/fsp/fsp_platform.h  |   4 +-
>  .../include/asm/arch-queensbay/fsp/fsp_support.h   |  63 +++---
>  .../x86/include/asm/arch-queensbay/fsp/fsp_types.h |  17 +-
>  arch/x86/include/asm/arch-queensbay/fsp/fsp_vpd.h  |   6 +-
>  arch/x86/lib/cmd_hob.c                             |   2 +-
>  13 files changed, 198 insertions(+), 224 deletions(-)

Yes this looks good. I will take this patch once we figure out the
microcode problem as emailed earlier.

I have a few more comments below also - hoping to simplify a little further.

>
> diff --git a/arch/x86/cpu/queensbay/fsp_configs.c b/arch/x86/cpu/queensbay/fsp_configs.c
> index aef18fc..af28e45 100644
> --- a/arch/x86/cpu/queensbay/fsp_configs.c
> +++ b/arch/x86/cpu/queensbay/fsp_configs.c
> @@ -8,7 +8,7 @@
>  #include <common.h>
>  #include <asm/arch/fsp/fsp_support.h>
>
> -void update_fsp_upd(struct upd_region_t *fsp_upd)
> +void update_fsp_upd(struct upd_region *fsp_upd)
>  {
>         /* Override any UPD setting if required */
>
> diff --git a/arch/x86/cpu/queensbay/fsp_support.c b/arch/x86/cpu/queensbay/fsp_support.c
> index f830eeb..dc2de71 100644
> --- a/arch/x86/cpu/queensbay/fsp_support.c
> +++ b/arch/x86/cpu/queensbay/fsp_support.c
> @@ -10,67 +10,48 @@
>  #include <asm/post.h>
>
>  /**
> - * Reads a 64-bit value from memory that may be unaligned.
> - *
> - * This function returns the 64-bit value pointed to by buf. The function
> - * guarantees that the read operation does not produce an alignment fault.
> - *
> - * If the buf is NULL, then ASSERT().
> - *
> - * @buf: Pointer to a 64-bit value that may be unaligned.
> - *
> - * @return: The 64-bit value read from buf.
> - */
> -static u64 read_unaligned64(const u64 *buf)
> -{
> -       ASSERT(buf != NULL);
> -
> -       return *buf;
> -}
> -
> -/**
>   * Compares two GUIDs
>   *
> - * If the GUIDs are identical then TRUE is returned.
> - * If there are any bit differences in the two GUIDs, then FALSE is returned.
> - *
> - * If guid1 is NULL, then ASSERT().
> - * If guid2 is NULL, then ASSERT().
> + * If the GUIDs are identical then true is returned.
> + * If there are any bit differences in the two GUIDs, then false is returned.
>   *
>   * @guid1:        A pointer to a 128 bit GUID.
>   * @guid2:        A pointer to a 128 bit GUID.
>   *
> - * @retval TRUE:  guid1 and guid2 are identical.
> - * @retval FALSE: guid1 and guid2 are not identical.
> + * @retval true:  guid1 and guid2 are identical.
> + * @retval false: guid1 and guid2 are not identical.
>   */
> -static unsigned char compare_guid(const struct efi_guid_t *guid1,
> -                                 const struct efi_guid_t *guid2)
> +static bool compare_guid(const struct efi_guid *guid1,
> +                        const struct efi_guid *guid2)
>  {
> -       u64 guid1_low;
> -       u64 guid2_low;
> -       u64 guid1_high;
> -       u64 guid2_high;
> -
> -       guid1_low  = read_unaligned64((const u64 *)guid1);
> -       guid2_low  = read_unaligned64((const u64 *)guid2);
> -       guid1_high = read_unaligned64((const u64 *)guid1 + 1);
> -       guid2_high = read_unaligned64((const u64 *)guid2 + 1);
> -
> -       return (unsigned char)(guid1_low == guid2_low && guid1_high == guid2_high);
> +       if (memcmp(guid1, guid2, sizeof(struct efi_guid)) == 0)
> +               return true;
> +       else
> +               return false;
>  }
>
>  u32 __attribute__((optimize("O0"))) find_fsp_header(void)
>  {
> +       /*
> +        * This function may be called before the a stack is established,
> +        * so special care must be taken. First, it cannot declare any local
> +        * variable using stack. Only register variable can be used here.
> +        * Secondly, some compiler version will add prolog or epilog code
> +        * for the C function. If so the function call may not work before
> +        * stack is ready.

OK, understood. This is supremely ugly - from what I understand this
function will write to the ROM stack, fail, and it doesn't matter
since the return address is already there?

> +        *
> +        * GCC 4.8.1 has been verified to be working for the following codes.
> +        */
>         volatile register u8 *fsp asm("eax");
>
>         /* Initalize the FSP base */
>         fsp = (u8 *)CONFIG_FSP_ADDR;
>
>         /* Check the FV signature, _FVH */
> -       if (((struct fv_header_t *)fsp)->sign == 0x4856465F) {
> +       if (((struct fv_header *)fsp)->sign == EFI_FVH_SIGNATURE) {
>                 /* Go to the end of the FV header and align the address */
> -               fsp += ((struct fv_header_t *)fsp)->ext_hdr_off;
> -               fsp += ((struct fv_ext_header_t *)fsp)->ext_hdr_size;
> +               fsp += ((struct fv_header *)fsp)->ext_hdr_off;
> +               fsp += ((struct fv_ext_header *)fsp)->ext_hdr_size;
>                 fsp  = (u8 *)(((u32)fsp + 7) & 0xFFFFFFF8);
>         } else {
>                 fsp  = 0;
> @@ -78,20 +59,20 @@ u32 __attribute__((optimize("O0"))) find_fsp_header(void)
>
>         /* Check the FFS GUID */
>         if (fsp &&
> -           (((u32 *)&(((struct ffs_file_header_t *)fsp)->name))[0] == 0x912740BE) &&
> -           (((u32 *)&(((struct ffs_file_header_t *)fsp)->name))[1] == 0x47342284) &&
> -           (((u32 *)&(((struct ffs_file_header_t *)fsp)->name))[2] == 0xB08471B9) &&
> -           (((u32 *)&(((struct ffs_file_header_t *)fsp)->name))[3] == 0x0C3F3527)) {
> +           (((u32 *)&(((struct ffs_file_header *)fsp)->name))[0] == 0x912740BE) &&
> +           (((u32 *)&(((struct ffs_file_header *)fsp)->name))[1] == 0x47342284) &&
> +           (((u32 *)&(((struct ffs_file_header *)fsp)->name))[2] == 0xB08471B9) &&
> +           (((u32 *)&(((struct ffs_file_header *)fsp)->name))[3] == 0x0C3F3527)) {

Perhaps another way to fix this code. Since you have gone to the
trouble of creating struct efi_guid, we could compare each field
against FSP_GUID_DATA1, FSP_GUID_DATA2, etc. Also so far as I can see
the data4 field is never used. Perhaps it could be changed to two
32-bit fields to simplify this?


>                 /* Add the FFS header size to find the raw section header */
> -               fsp += sizeof(struct ffs_file_header_t);
> +               fsp += sizeof(struct ffs_file_header);
>         } else {
>                 fsp = 0;
>         }
>
>         if (fsp &&
> -           ((struct raw_section_t *)fsp)->type == EFI_SECTION_RAW) {
> +           ((struct raw_section *)fsp)->type == EFI_SECTION_RAW) {
>                 /* Add the raw section header size to find the FSP header */
> -               fsp += sizeof(struct raw_section_t);
> +               fsp += sizeof(struct raw_section);
>         } else {
>                 fsp = 0;
>         }
> @@ -99,7 +80,7 @@ u32 __attribute__((optimize("O0"))) find_fsp_header(void)
>         return (u32)fsp;
>  }
>
> -void fsp_continue(struct shared_data_t *shared_data, u32 status, void *hob_list)
> +void fsp_continue(struct shared_data *shared_data, u32 status, void *hob_list)
>  {
>         u32 stack_len;
>         u32 stack_base;
> @@ -107,18 +88,18 @@ void fsp_continue(struct shared_data_t *shared_data, u32 status, void *hob_list)
>
>         post_code(POST_MRC);
>
> -       ASSERT(status == 0);
> +       assert(status == 0);
>
>         /* Get the migrated stack in normal memory */
> -       stack_base = (u32)get_bootloader_tmp_mem(hob_list, &stack_len);
> -       ASSERT(stack_base != 0);
> +       stack_base = (u32)fsp_get_bootloader_tmp_mem(hob_list, &stack_len);
> +       assert(stack_base != 0);
>         stack_top  = stack_base + stack_len - sizeof(u32);
>
>         /*
>          * Old stack base is stored at the very end of the stack top,
>          * use it to calculate the migrated shared data base
>          */
> -       shared_data = (struct shared_data_t *)(stack_base +
> +       shared_data = (struct shared_data *)(stack_base +
>                         ((u32)shared_data - *(u32 *)stack_top));
>
>         /* The boot loader main function entry */
> @@ -127,50 +108,50 @@ void fsp_continue(struct shared_data_t *shared_data, u32 status, void *hob_list)
>
>  void fsp_init(u32 stack_top, u32 boot_mode, void *nvs_buf)
>  {
> -       struct shared_data_t shared_data;
> +       struct shared_data shared_data;
>         fsp_init_f init;
> -       struct fsp_init_params_t params;
> -       struct fspinit_rtbuf_t rt_buf;
> -       struct vpd_region_t *fsp_vpd;
> -       struct fsp_header_t *fsp_hdr;
> -       struct fsp_init_params_t *params_ptr;
> -       struct upd_region_t *fsp_upd;
> -
> -       fsp_hdr = (struct fsp_header_t *)find_fsp_header();
> +       struct fsp_init_params params;
> +       struct fspinit_rtbuf rt_buf;
> +       struct vpd_region *fsp_vpd;
> +       struct fsp_header *fsp_hdr;
> +       struct fsp_init_params *params_ptr;
> +       struct upd_region *fsp_upd;
> +
> +       fsp_hdr = (struct fsp_header *)find_fsp_header();
>         if (fsp_hdr == NULL) {
>                 /* No valid FSP info header was found */
> -               ASSERT(FALSE);
> +               panic("Invalid FSP header");
>         }
>
> -       fsp_upd = (struct upd_region_t *)&shared_data.fsp_upd;
> -       memset((void *)&rt_buf, 0, sizeof(struct fspinit_rtbuf_t));
> +       fsp_upd = (struct upd_region *)&shared_data.fsp_upd;
> +       memset(&rt_buf, 0, sizeof(struct fspinit_rtbuf));
>
>         /* Reserve a gap in stack top */
>         rt_buf.common.stack_top = (u32 *)stack_top - 32;
>         rt_buf.common.boot_mode = boot_mode;
> -       rt_buf.common.upd_data = (struct upd_region_t *)fsp_upd;
> +       rt_buf.common.upd_data = (struct upd_region *)fsp_upd;
>
>         /* Get VPD region start */
> -       fsp_vpd = (struct vpd_region_t *)(fsp_hdr->img_base +
> +       fsp_vpd = (struct vpd_region *)(fsp_hdr->img_base +
>                         fsp_hdr->cfg_region_off);
>
>         /* Verifify the VPD data region is valid */
> -       ASSERT((fsp_vpd->img_rev == VPD_IMAGE_REV) &&
> +       assert((fsp_vpd->img_rev == VPD_IMAGE_REV) &&
>                (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_t));
> +              sizeof(struct upd_region));
>
>         /* Verifify the UPD data region is valid */
> -       ASSERT(fsp_upd->terminator == 0x55AA);
> +       assert(fsp_upd->terminator == UPD_TERMINATOR);
>
>         /* Override any UPD setting if required */
>         update_fsp_upd(fsp_upd);
>
> -       memset((void *)&params, 0, sizeof(struct fsp_init_params_t));
> +       memset(&params, 0, sizeof(struct fsp_init_params));
>         params.nvs_buf = nvs_buf;
> -       params.rt_buf = (struct fspinit_rtbuf_t *)&rt_buf;
> +       params.rt_buf = (struct fspinit_rtbuf *)&rt_buf;
>         params.continuation = (fsp_continuation_f)asm_continuation;
>
>         init = (fsp_init_f)(fsp_hdr->img_base + fsp_hdr->fsp_init);
> @@ -199,32 +180,28 @@ void fsp_init(u32 stack_top, u32 boot_mode, void *nvs_buf)
>
>         /*
>          * Should never get here.
> -        * Control will continue from romstage_main_continue_asm.
> +        * Control will continue from fsp_continue.
>          * This line below is to prevent the compiler from optimizing
>          * structure intialization.
> +        *
> +        * DO NOT REMOVE!
>          */
>         init(&params);
> -
> -       /*
> -        * Should never return.
> -        * Control will continue from ContinuationFunc
> -        */
> -       ASSERT(FALSE);
>  }
>
> -u32 fsp_notify(struct fsp_header_t *fsp_hdr, u32 phase)
> +u32 fsp_notify(struct fsp_header *fsp_hdr, u32 phase)
>  {
>         fsp_notify_f notify;
> -       struct fsp_notify_params_t params;
> -       struct fsp_notify_params_t *params_ptr;
> +       struct fsp_notify_params params;
> +       struct fsp_notify_params *params_ptr;
>         u32 status;
>
>         if (!fsp_hdr)
> -               fsp_hdr = (struct fsp_header_t *)find_fsp_header();
> +               fsp_hdr = (struct fsp_header *)find_fsp_header();
>
>         if (fsp_hdr == NULL) {
>                 /* No valid FSP info header */
> -               ASSERT(FALSE);
> +               panic("Invalid FSP header");
>         }
>
>         notify = (fsp_notify_f)(fsp_hdr->img_base + fsp_hdr->fsp_notify);
> @@ -245,9 +222,9 @@ u32 fsp_notify(struct fsp_header_t *fsp_hdr, u32 phase)
>         return status;
>  }
>
> -u32 get_usable_lowmem_top(const void *hob_list)
> +u32 fsp_get_usable_lowmem_top(const void *hob_list)
>  {
> -       union hob_pointers_t hob;
> +       union hob_pointers hob;
>         phys_addr_t phys_start;
>         u32 top;
>
> @@ -255,14 +232,14 @@ u32 get_usable_lowmem_top(const void *hob_list)
>         hob.raw = (void *)hob_list;
>
>         /* * Collect memory ranges */
> -       top = 0x100000;
> +       top = FSP_LOWMEM_BASE;
>         while (!END_OF_HOB(hob)) {
>                 if (hob.hdr->type == HOB_TYPE_RES_DESC) {
>                         if (hob.res_desc->type == RES_SYS_MEM) {
>                                 phys_start = hob.res_desc->phys_start;
>                                 /* Need memory above 1MB to be collected here */
> -                               if (phys_start >= 0x100000 &&
> -                                   phys_start < (phys_addr_t)0x100000000)
> +                               if (phys_start >= FSP_LOWMEM_BASE &&
> +                                   phys_start < (phys_addr_t)FSP_HIGHMEM_BASE)
>                                         top += (u32)(hob.res_desc->len);
>                         }
>                 }
> @@ -272,9 +249,9 @@ u32 get_usable_lowmem_top(const void *hob_list)
>         return top;
>  }
>
> -u64 get_usable_highmem_top(const void *hob_list)
> +u64 fsp_get_usable_highmem_top(const void *hob_list)
>  {
> -       union hob_pointers_t hob;
> +       union hob_pointers hob;
>         phys_addr_t phys_start;
>         u64 top;
>
> @@ -282,13 +259,13 @@ u64 get_usable_highmem_top(const void *hob_list)
>         hob.raw = (void *)hob_list;
>
>         /* Collect memory ranges */
> -       top = 0x100000000;
> +       top = FSP_HIGHMEM_BASE;
>         while (!END_OF_HOB(hob)) {
>                 if (hob.hdr->type == HOB_TYPE_RES_DESC) {
>                         if (hob.res_desc->type == RES_SYS_MEM) {
>                                 phys_start = hob.res_desc->phys_start;
>                                 /* Need memory above 1MB to be collected here */
> -                               if (phys_start >= (phys_addr_t)0x100000000)
> +                               if (phys_start >= (phys_addr_t)FSP_HIGHMEM_BASE)
>                                         top += (u32)(hob.res_desc->len);
>                         }
>                 }
> @@ -298,10 +275,10 @@ u64 get_usable_highmem_top(const void *hob_list)
>         return top;
>  }
>
> -u64 get_fsp_reserved_mem_from_guid(const void *hob_list, u64 *len,
> -                                  struct efi_guid_t *guid)
> +u64 fsp_get_reserved_mem_from_guid(const void *hob_list, u64 *len,
> +                                  struct efi_guid *guid)
>  {
> -       union hob_pointers_t hob;
> +       union hob_pointers hob;
>
>         /* Get the HOB list for processing */
>         hob.raw = (void *)hob_list;
> @@ -324,39 +301,39 @@ u64 get_fsp_reserved_mem_from_guid(const void *hob_list, u64 *len,
>         return 0;
>  }
>
> -u32 get_fsp_reserved_mem(const void *hob_list, u32 *len)
> +u32 fsp_get_fsp_reserved_mem(const void *hob_list, u32 *len)
>  {
> -       const struct efi_guid_t guid = FSP_HOB_RESOURCE_OWNER_FSP_GUID;
> +       const struct efi_guid guid = FSP_HOB_RESOURCE_OWNER_FSP_GUID;
>         u64 length;
>         u32 base;
>
> -       base = (u32)get_fsp_reserved_mem_from_guid(hob_list,
> -                       &length, (struct efi_guid_t *)&guid);
> +       base = (u32)fsp_get_reserved_mem_from_guid(hob_list,
> +                       &length, (struct efi_guid *)&guid);
>         if ((len != 0) && (base != 0))
>                 *len = (u32)length;
>
>         return base;
>  }
>
> -u32 get_tseg_reserved_mem(const void *hob_list, u32 *len)
> +u32 fsp_get_tseg_reserved_mem(const void *hob_list, u32 *len)
>  {
> -       const struct efi_guid_t guid = FSP_HOB_RESOURCE_OWNER_TSEG_GUID;
> +       const struct efi_guid guid = FSP_HOB_RESOURCE_OWNER_TSEG_GUID;
>         u64 length;
>         u32 base;
>
> -       base = (u32)get_fsp_reserved_mem_from_guid(hob_list,
> -                       &length, (struct efi_guid_t *)&guid);
> +       base = (u32)fsp_get_reserved_mem_from_guid(hob_list,
> +                       &length, (struct efi_guid *)&guid);
>         if ((len != 0) && (base != 0))
>                 *len = (u32)length;
>
>         return base;
>  }
>
> -void *get_next_hob(u16 type, const void *hob_list)
> +void *fsp_get_next_hob(u16 type, const void *hob_list)
>  {
> -       union hob_pointers_t hob;
> +       union hob_pointers hob;
>
> -       ASSERT(hob_list != NULL);
> +       assert(hob_list != NULL);
>
>         hob.raw = (u8 *)hob_list;
>
> @@ -371,12 +348,12 @@ void *get_next_hob(u16 type, const void *hob_list)
>         return NULL;
>  }
>
> -void *get_next_guid_hob(const struct efi_guid_t *guid, const void *hob_list)
> +void *fsp_get_next_guid_hob(const struct efi_guid *guid, const void *hob_list)
>  {
> -       union hob_pointers_t hob;
> +       union hob_pointers hob;
>
>         hob.raw = (u8 *)hob_list;
> -       while ((hob.raw = get_next_hob(HOB_TYPE_GUID_EXT,
> +       while ((hob.raw = fsp_get_next_hob(HOB_TYPE_GUID_EXT,
>                         hob.raw)) != NULL) {
>                 if (compare_guid(guid, &hob.guid->name))
>                         break;
> @@ -386,11 +363,12 @@ void *get_next_guid_hob(const struct efi_guid_t *guid, const void *hob_list)
>         return hob.raw;
>  }
>
> -void *get_guid_hob_data(const void *hob_list, u32 *len, struct efi_guid_t *guid)
> +void *fsp_get_guid_hob_data(const void *hob_list, u32 *len,
> +                           struct efi_guid *guid)
>  {
>         u8 *guid_hob;
>
> -       guid_hob = get_next_guid_hob(guid, hob_list);
> +       guid_hob = fsp_get_next_guid_hob(guid, hob_list);
>         if (guid_hob == NULL) {
>                 return NULL;
>         } else {
> @@ -401,16 +379,16 @@ void *get_guid_hob_data(const void *hob_list, u32 *len, struct efi_guid_t *guid)
>         }
>  }
>
> -void *get_fsp_nvs_data(const void *hob_list, u32 *len)
> +void *fsp_get_nvs_data(const void *hob_list, u32 *len)
>  {
> -       const struct efi_guid_t guid = FSP_NON_VOLATILE_STORAGE_HOB_GUID;
> +       const struct efi_guid guid = FSP_NON_VOLATILE_STORAGE_HOB_GUID;
>
> -       return get_guid_hob_data(hob_list, len, (struct efi_guid_t *)&guid);
> +       return fsp_get_guid_hob_data(hob_list, len, (struct efi_guid *)&guid);
>  }
>
> -void *get_bootloader_tmp_mem(const void *hob_list, u32 *len)
> +void *fsp_get_bootloader_tmp_mem(const void *hob_list, u32 *len)
>  {
> -       const struct efi_guid_t guid = FSP_BOOTLOADER_TEMP_MEM_HOB_GUID;
> +       const struct efi_guid guid = FSP_BOOTLOADER_TEMP_MEM_HOB_GUID;
>
> -       return get_guid_hob_data(hob_list, len, (struct efi_guid_t *)&guid);
> +       return fsp_get_guid_hob_data(hob_list, len, (struct efi_guid *)&guid);
>  }
> diff --git a/arch/x86/cpu/queensbay/tnc_dram.c b/arch/x86/cpu/queensbay/tnc_dram.c
> index dbc1710..378af01 100644
> --- a/arch/x86/cpu/queensbay/tnc_dram.c
> +++ b/arch/x86/cpu/queensbay/tnc_dram.c
> @@ -14,7 +14,7 @@ DECLARE_GLOBAL_DATA_PTR;
>  int dram_init(void)
>  {
>         phys_size_t ram_size = 0;
> -       union hob_pointers_t hob;
> +       union hob_pointers hob;
>
>         hob.raw = gd->arch.hob_list;
>         while (!END_OF_HOB(hob)) {
> @@ -49,14 +49,14 @@ void dram_init_banksize(void)
>   */
>  ulong board_get_usable_ram_top(ulong total_size)
>  {
> -       return get_usable_lowmem_top(gd->arch.hob_list);
> +       return fsp_get_usable_lowmem_top(gd->arch.hob_list);
>  }
>
>  unsigned install_e820_map(unsigned max_entries, struct e820entry *entries)
>  {
>         unsigned num_entries = 0;
>
> -       union hob_pointers_t hob;
> +       union hob_pointers hob;
>
>         hob.raw = gd->arch.hob_list;
>
> diff --git a/arch/x86/include/asm/arch-queensbay/fsp/fsp_api.h b/arch/x86/include/asm/arch-queensbay/fsp/fsp_api.h
> index 25b938f..95f0e06 100644
> --- a/arch/x86/include/asm/arch-queensbay/fsp/fsp_api.h
> +++ b/arch/x86/include/asm/arch-queensbay/fsp/fsp_api.h
> @@ -16,7 +16,7 @@ typedef void (*fsp_continuation_f)(u32 status, void *hob_list);
>
>  #pragma pack(1)
>
> -struct fsp_init_params_t {
> +struct fsp_init_params {
>         /* Non-volatile storage buffer pointer */
>         void                    *nvs_buf;
>         /* Runtime buffer pointer */
> @@ -25,7 +25,7 @@ struct fsp_init_params_t {
>         fsp_continuation_f      continuation;
>  };
>
> -struct common_buf_t {
> +struct common_buf {
>         /*
>          * Stack top pointer used by the bootloader. The new stack frame will be
>          * set up at this location after FspInit API call.
> @@ -36,24 +36,24 @@ struct common_buf_t {
>         u32     reserved[7];    /* Reserved */
>  };
>
> -enum fsp_phase_t {
> +enum fsp_phase {
>         /* Notification code for post PCI enuermation */
>         INIT_PHASE_PCI  = 0x20,
>         /* Notification code before transfering control to the payload */
>         INIT_PHASE_BOOT = 0x40
>  };
>
> -struct fsp_notify_params_t {
> +struct fsp_notify_params {
>         /* Notification phase used for NotifyPhase API */
> -       enum fsp_phase_t        phase;
> +       enum fsp_phase  phase;
>  };
>
>  #pragma pack()
>
>  /* FspInit API function prototype */
> -typedef u32 (*fsp_init_f)(struct fsp_init_params_t *param);
> +typedef u32 (*fsp_init_f)(struct fsp_init_params *params);
>
>  /* FspNotify API function prototype */
> -typedef u32 (*fsp_notify_f)(struct fsp_notify_params_t *param);
> +typedef u32 (*fsp_notify_f)(struct fsp_notify_params *params);
>
>  #endif
> diff --git a/arch/x86/include/asm/arch-queensbay/fsp/fsp_ffs.h b/arch/x86/include/asm/arch-queensbay/fsp/fsp_ffs.h
> index 1f73680..7fef3e7 100644
> --- a/arch/x86/include/asm/arch-queensbay/fsp/fsp_ffs.h
> +++ b/arch/x86/include/asm/arch-queensbay/fsp/fsp_ffs.h
> @@ -11,7 +11,7 @@
>  #pragma pack(1)
>
>  /* Used to verify the integrity of the file */
> -union ffs_integrity_t {
> +union ffs_integrity {
>         struct {
>                 /*
>                  * The IntegrityCheck.checksum.header field is an 8-bit
> @@ -43,14 +43,14 @@ union ffs_integrity_t {
>   * Each file begins with the header that describe the
>   * contents and state of the files.
>   */
> -struct ffs_file_header_t {
> +struct ffs_file_header {
>         /*
>          * This GUID is the file name.
>          * It is used to uniquely identify the file.
>          */
> -       struct efi_guid_t       name;
> +       struct efi_guid         name;
>         /* Used to verify the integrity of the file */
> -       union ffs_integrity_t   integrity;
> +       union ffs_integrity     integrity;
>         /* Identifies the type of file */
>         u8                      type;
>         /* Declares various file attribute bits */
> @@ -64,16 +64,16 @@ struct ffs_file_header_t {
>         u8                      state;
>  };
>
> -struct ffs_file_header2_t {
> +struct ffs_file_header2 {
>         /*
>          * This GUID is the file name. It is used to uniquely identify the file.
>          * There may be only one instance of a file with the file name GUID of
>          * Name in any given firmware volume, except if the file type is
>          * EFI_FV_FILE_TYPE_FFS_PAD.
>          */
> -       struct efi_guid_t       name;
> +       struct efi_guid         name;
>         /* Used to verify the integrity of the file */
> -       union ffs_integrity_t   integrity;
> +       union ffs_integrity     integrity;
>         /* Identifies the type of file */
>         u8                      type;
>         /* Declares various file attribute bits */
> @@ -81,9 +81,9 @@ struct ffs_file_header2_t {
>         /*
>          * The length of the file in bytes, including the FFS header.
>          * The length of the file data is either
> -        * (size - sizeof(struct ffs_file_header_t)). This calculation means a
> +        * (size - sizeof(struct ffs_file_header)). This calculation means a
>          * zero-length file has a size of 24 bytes, which is
> -        * sizeof(struct ffs_file_header_t). Size is not required to be a
> +        * sizeof(struct ffs_file_header). Size is not required to be a
>          * multiple of 8 bytes. Given a file F, the next file header is located
>          * at the next 8-byte aligned firmware volume offset following the last
>          * byte of the file F.
> @@ -98,7 +98,7 @@ struct ffs_file_header2_t {
>          * If FFS_ATTRIB_LARGE_FILE is set in attr, then ext_size exists
>          * and size must be set to zero.
>          * If FFS_ATTRIB_LARGE_FILE is not set then
> -        * struct ffs_file_header_t is used.
> +        * struct ffs_file_header is used.
>          */
>         u32                     ext_size;
>  };
> @@ -129,7 +129,7 @@ struct ffs_file_header2_t {
>  #define EFI_SECTION_SMM_DEPEX                  0x1C
>
>  /* Common section header */
> -struct raw_section_t {
> +struct raw_section {
>         /*
>          * A 24-bit unsigned integer that contains the total size of
>          * the section in bytes, including the EFI_COMMON_SECTION_HEADER.
> @@ -138,7 +138,7 @@ struct raw_section_t {
>         u8      type;
>  };
>
> -struct raw_section2_t {
> +struct raw_section2 {
>         /*
>          * A 24-bit unsigned integer that contains the total size of
>          * the section in bytes, including the EFI_COMMON_SECTION_HEADER.
> diff --git a/arch/x86/include/asm/arch-queensbay/fsp/fsp_fv.h b/arch/x86/include/asm/arch-queensbay/fsp/fsp_fv.h
> index 01300db..a024451 100644
> --- a/arch/x86/include/asm/arch-queensbay/fsp/fsp_fv.h
> +++ b/arch/x86/include/asm/arch-queensbay/fsp/fsp_fv.h
> @@ -63,7 +63,7 @@
>  #define EFI_FVB2_ALIGNMENT_1G          0x001E0000
>  #define EFI_FVB2_ALIGNMENT_2G          0x001F0000
>
> -struct fv_blkmap_entry_t {
> +struct fv_blkmap_entry {
>         /* The number of sequential blocks which are of the same size */
>         u32     num_blocks;
>         /* The size of the blocks */
> @@ -71,7 +71,7 @@ struct fv_blkmap_entry_t {
>  };
>
>  /* Describes the features and layout of the firmware volume */
> -struct fv_header_t {
> +struct fv_header {
>         /*
>          * The first 16 bytes are reserved to allow for the reset vector of
>          * processors whose reset vector is at address 0.
> @@ -81,7 +81,7 @@ struct fv_header_t {
>          * Declares the file system with which the firmware volume
>          * is formatted.
>          */
> -       struct efi_guid_t       fs_guid;
> +       struct efi_guid         fs_guid;
>         /*
>          * Length in bytes of the complete firmware volume, including
>          * the header.
> @@ -118,18 +118,18 @@ struct fv_header_t {
>          * An array of run-length encoded FvBlockMapEntry structures.
>          * The array is terminated with an entry of {0,0}.
>          */
> -       struct fv_blkmap_entry_t        block_map[1];
> +       struct fv_blkmap_entry  block_map[1];
>  };
>
> -#define EFI_FVH_SIGNATURE SIGNATURE_32('_', 'F', 'V', 'H')
> +#define EFI_FVH_SIGNATURE      SIGNATURE_32('_', 'F', 'V', 'H')
>
>  /* Firmware Volume Header Revision definition */
>  #define EFI_FVH_REVISION       0x02
>
>  /* Extension header pointed by ExtHeaderOffset of volume header */
> -struct fv_ext_header_t {
> +struct fv_ext_header {
>         /* firmware volume name */
> -       struct efi_guid_t       fv_name;
> +       struct efi_guid         fv_name;
>         /* Size of the rest of the extension header including this structure */
>         u32                     ext_hdr_size;
>  };
> diff --git a/arch/x86/include/asm/arch-queensbay/fsp/fsp_hob.h b/arch/x86/include/asm/arch-queensbay/fsp/fsp_hob.h
> index 44c0f90..b4fc11b 100644
> --- a/arch/x86/include/asm/arch-queensbay/fsp/fsp_hob.h
> +++ b/arch/x86/include/asm/arch-queensbay/fsp/fsp_hob.h
> @@ -19,14 +19,14 @@
>   * Describes the format and size of the data inside the HOB.
>   * All HOBs must contain this generic HOB header.
>   */
> -struct hob_header_t {
> +struct hob_header {
>         u16     type;           /* HOB type */
>         u16     len;            /* HOB length */
>         u32     reserved;       /* always zero */
>  };
>
>  /* Enumeration of memory types introduced in UEFI */
> -enum efi_mem_type_t {
> +enum efi_mem_type {
>         EFI_RESERVED_MEMORY_TYPE,
>         /*
>          * The code portions of a loaded application.
> @@ -87,16 +87,16 @@ enum efi_mem_type_t {
>   * exist outside the HOB list. This HOB type describes how memory is used,
>   * not the physical attributes of memory.
>   */
> -struct hob_mem_alloc_t {
> -       struct hob_header_t     hdr;
> +struct hob_mem_alloc {
> +       struct hob_header       hdr;
>         /*
>          * A GUID that defines the memory allocation region's type and purpose,
>          * as well as other fields within the memory allocation HOB. This GUID
>          * is used to define the additional data within the HOB that may be
> -        * present for the memory allocation HOB. Type efi_guid_t is defined in
> +        * present for the memory allocation HOB. Type efi_guid is defined in
>          * InstallProtocolInterface() in the UEFI 2.0 specification.
>          */
> -       struct efi_guid_t       name;
> +       struct efi_guid         name;
>         /*
>          * The base address of memory allocated by this HOB.
>          * Type phys_addr_t is defined in AllocatePages() in the UEFI 2.0
> @@ -111,7 +111,7 @@ struct hob_mem_alloc_t {
>          * Type EFI_MEMORY_TYPE is defined in AllocatePages() in the UEFI 2.0
>          * specification.
>          */
> -       enum efi_mem_type_t     mem_type;
> +       enum efi_mem_type       mem_type;
>         /* padding */
>         u8                      reserved[4];
>  };
> @@ -155,14 +155,14 @@ struct hob_mem_alloc_t {
>   * Describes the resource properties of all fixed, nonrelocatable resource
>   * ranges found on the processor host bus during the HOB producer phase.
>   */
> -struct hob_res_desc_t {
> -       struct hob_header_t     hdr;
> +struct hob_res_desc {
> +       struct hob_header       hdr;
>         /*
>          * A GUID representing the owner of the resource. This GUID is
>          * used by HOB consumer phase components to correlate device
>          * ownership of a resource.
>          */
> -       struct efi_guid_t       owner;
> +       struct efi_guid         owner;
>         u32                     type;
>         u32                     attr;
>         /* The physical start address of the resource region */
> @@ -175,19 +175,19 @@ struct hob_res_desc_t {
>   * Allows writers of executable content in the HOB producer phase to
>   * maintain and manage HOBs with specific GUID.
>   */
> -struct hob_guid_t {
> -       struct hob_header_t     hdr;
> +struct hob_guid {
> +       struct hob_header       hdr;
>         /* A GUID that defines the contents of this HOB */
> -       struct efi_guid_t       name;
> +       struct efi_guid         name;
>         /* GUID specific data goes here */
>  };
>
>  /* Union of all the possible HOB Types */
> -union hob_pointers_t {
> -       struct hob_header_t     *hdr;
> -       struct hob_mem_alloc_t  *mem_alloc;
> -       struct hob_res_desc_t   *res_desc;
> -       struct hob_guid_t       *guid;
> +union hob_pointers {
> +       struct hob_header       *hdr;
> +       struct hob_mem_alloc    *mem_alloc;
> +       struct hob_res_desc     *res_desc;
> +       struct hob_guid         *guid;
>         u8                      *raw;
>  };
>
> @@ -202,7 +202,7 @@ union hob_pointers_t {
>   * @return: HOB type.
>   */
>  #define GET_HOB_TYPE(hob) \
> -       ((*(struct hob_header_t **)&(hob))->type)
> +       ((*(struct hob_header **)&(hob))->type)

Can we remove all these casts?

How about hob->hdr.type? Also maybe convert these to static inline
functions? (lower case)?

>
>  /**
>   * Returns the length, in bytes, of a HOB.
> @@ -215,7 +215,7 @@ union hob_pointers_t {
>   * @return: HOB length.
>   */
>  #define GET_HOB_LENGTH(hob) \
> -       ((*(struct hob_header_t **)&(hob))->len)
> +       ((*(struct hob_header **)&(hob))->len)

hob->hdr.len?

>
>  /**
>   * Returns a pointer to the next HOB in the HOB list.
> @@ -234,13 +234,13 @@ union hob_pointers_t {
>   * Determines if a HOB is the last HOB in the HOB list.
>   *
>   * This macro determine if the HOB specified by hob is the last HOB in the
> - * HOB list.  If hob is last HOB in the HOB list, then TRUE is returned.
> - * Otherwise, FALSE is returned.
> + * HOB list.  If hob is last HOB in the HOB list, then true is returned.
> + * Otherwise, false is returned.
>   *
>   * @hob:          A pointer to a HOB.
>   *
> - * @retval TRUE:  The HOB specified by hob is the last HOB in the HOB list.
> - * @retval FALSE: The HOB specified by hob is not the last HOB in the HOB list.
> + * @retval true:  The HOB specified by hob is the last HOB in the HOB list.
> + * @retval false: The HOB specified by hob is not the last HOB in the HOB list.
>   */
>  #define END_OF_HOB(hob)        (GET_HOB_TYPE(hob) == (u16)HOB_TYPE_EOH)

hob->hdr.type == HOB_TYPE_EOH

>
> @@ -255,7 +255,7 @@ union hob_pointers_t {
>   * @return: A pointer to the data buffer in a HOB.
>   */
>  #define GET_GUID_HOB_DATA(hob) \
> -       (void *)(*(u8 **)&(hob) + sizeof(struct hob_guid_t))
> +       (void *)(*(u8 **)&(hob) + sizeof(struct hob_guid))

Similarly can we use the guid member?

>
>  /**
>   * Returns the size of the data buffer from a HOB of type HOB_TYPE_GUID_EXT.
> @@ -268,7 +268,7 @@ union hob_pointers_t {
>   * @return: The size of the data buffer.
>   */
>  #define GET_GUID_HOB_DATA_SIZE(hob)    \
> -       (u16)(GET_HOB_LENGTH(hob) - sizeof(struct hob_guid_t))
> +       (u16)(GET_HOB_LENGTH(hob) - sizeof(struct hob_guid))
>

Would be good to simplify this one also.


>  /* FSP specific GUID HOB definitions */
>  #define FSP_HEADER_GUID \
> diff --git a/arch/x86/include/asm/arch-queensbay/fsp/fsp_infoheader.h b/arch/x86/include/asm/arch-queensbay/fsp/fsp_infoheader.h
> index ad78bcd..a5edfef 100644
> --- a/arch/x86/include/asm/arch-queensbay/fsp/fsp_infoheader.h
> +++ b/arch/x86/include/asm/arch-queensbay/fsp/fsp_infoheader.h
> @@ -12,7 +12,7 @@
>
>  #pragma pack(1)
>
> -struct fsp_header_t {
> +struct fsp_header {
>         u32     sign;                   /* 'FSPH' */
>         u32     hdr_len;                /* header length */
>         u8      reserved1[3];
> diff --git a/arch/x86/include/asm/arch-queensbay/fsp/fsp_platform.h b/arch/x86/include/asm/arch-queensbay/fsp/fsp_platform.h
> index a7b6e6b..67e4b1e 100644
> --- a/arch/x86/include/asm/arch-queensbay/fsp/fsp_platform.h
> +++ b/arch/x86/include/asm/arch-queensbay/fsp/fsp_platform.h
> @@ -10,8 +10,8 @@
>
>  #pragma pack(1)
>
> -struct fspinit_rtbuf_t {
> -       struct common_buf_t     common; /* FSP common runtime data structure */
> +struct fspinit_rtbuf {
> +       struct common_buf       common; /* FSP common runtime data structure */
>  };
>
>  #pragma pack()
> diff --git a/arch/x86/include/asm/arch-queensbay/fsp/fsp_support.h b/arch/x86/include/asm/arch-queensbay/fsp/fsp_support.h
> index 3296a2b..3ae1b66 100644
> --- a/arch/x86/include/asm/arch-queensbay/fsp/fsp_support.h
> +++ b/arch/x86/include/asm/arch-queensbay/fsp/fsp_support.h
> @@ -18,14 +18,30 @@
>  #include "fsp_bootmode.h"
>  #include "fsp_vpd.h"
>
> -struct shared_data_t {
> -       struct fsp_header_t     *fsp_hdr;
> +struct shared_data {
> +       struct fsp_header       *fsp_hdr;
>         u32                     *stack_top;
> -       struct upd_region_t     fsp_upd;
> +       struct upd_region       fsp_upd;
>  };
>
> +#define FSP_LOWMEM_BASE                0x100000UL
> +#define FSP_HIGHMEM_BASE       0x100000000ULL
> +
> +/**
> + * FSP Continuation assembly helper routine
> + *
> + * This routine jumps to the C version of FSP continuation function
> + */
>  void asm_continuation(void);
>
> +/**
> + * FSP initialization complete
> + *
> + * This is the function that indicates FSP initialization is complete and jumps
> + * back to the bootloader with HOB list pointer as the parameter.
> + *
> + * @hob_list:    HOB list pointer
> + */
>  void fsp_init_done(void *hob_list);
>
>  /**
> @@ -37,19 +53,12 @@ void fsp_init_done(void *hob_list);
>   *
>   * @retval:      Never returns
>   */
> -void fsp_continue(struct shared_data_t *shared_data, u32 status,
> +void fsp_continue(struct shared_data *shared_data, u32 status,
>                   void *hob_list);
>
>  /**
>   * Find FSP header offset in FSP image
>   *
> - * If this function is called before the a stack is established, special care
> - * must be taken. First, it cannot declare any local variable using stack.
> - * Only register variable can be used here. Secondly, some compiler version
> - * will add prolog or epilog code for the C function. If so the function call
> - * may not work before stack is ready. GCC 4.8.1 has been verified to be
> - * working for the following code.
> - *
>   * @retval: the offset of FSP header. If signature is invalid, returns 0.
>   */
>  u32 find_fsp_header(void);
> @@ -67,11 +76,11 @@ void fsp_init(u32 stack_top, u32 boot_mode, void *nvs_buf);
>   * FSP notification wrapper function
>   *
>   * @fsp_hdr: Pointer to FSP information header
> - * @phase:   FSP initialization phase defined in enum fsp_phase_t
> + * @phase:   FSP initialization phase defined in enum fsp_phase
>   *
>   * @retval:  compatible status code with EFI_STATUS defined in PI spec
>   */
> -u32 fsp_notify(struct fsp_header_t *fsp_hdr, u32 phase);
> +u32 fsp_notify(struct fsp_header *fsp_hdr, u32 phase);
>
>  /**
>   * This function retrieves the top of usable low memory.
> @@ -80,7 +89,7 @@ u32 fsp_notify(struct fsp_header_t *fsp_hdr, u32 phase);
>   *
>   * @retval:   Usable low memory top.
>   */
> -u32 get_usable_lowmem_top(const void *hob_list);
> +u32 fsp_get_usable_lowmem_top(const void *hob_list);
>
>  /**
>   * This function retrieves the top of usable high memory.
> @@ -89,7 +98,7 @@ u32 get_usable_lowmem_top(const void *hob_list);
>   *
>   * @retval:   Usable high memory top.
>   */
> -u64 get_usable_highmem_top(const void *hob_list);
> +u64 fsp_get_usable_highmem_top(const void *hob_list);
>
>  /**
>   * This function retrieves a special reserved memory region.
> @@ -102,8 +111,8 @@ u64 get_usable_highmem_top(const void *hob_list);
>   * @retval:   Reserved region start address.
>   *            0 if this region does not exist.
>   */
> -u64 get_fsp_reserved_mem_from_guid(const void *hob_list,
> -                                  u64 *len, struct efi_guid_t *guid);
> +u64 fsp_get_reserved_mem_from_guid(const void *hob_list,
> +                                  u64 *len, struct efi_guid *guid);
>
>  /**
>   * This function retrieves the FSP reserved normal memory.
> @@ -114,7 +123,7 @@ u64 get_fsp_reserved_mem_from_guid(const void *hob_list,
>   * @retval:   FSP reserved memory base
>   *            0 if this region does not exist.
>   */
> -u32 get_fsp_reserved_mem(const void *hob_list, u32 *len);
> +u32 fsp_get_fsp_reserved_mem(const void *hob_list, u32 *len);
>
>  /**
>   * This function retrieves the TSEG reserved normal memory.
> @@ -126,7 +135,7 @@ u32 get_fsp_reserved_mem(const void *hob_list, u32 *len);
>   * @retval NULL:   Failed to find the TSEG reserved memory.
>   * @retval others: TSEG reserved memory base.
>   */
> -u32 get_tseg_reserved_mem(const void *hob_list, u32 *len);
> +u32 fsp_get_tseg_reserved_mem(const void *hob_list, u32 *len);
>
>  /**
>   * Returns the next instance of a HOB type from the starting HOB.
> @@ -136,7 +145,7 @@ u32 get_tseg_reserved_mem(const void *hob_list, u32 *len);
>   *
>   * @retval:   A HOB object with matching type; Otherwise NULL.
>   */
> -void *get_next_hob(u16 type, const void *hob_list);
> +void *fsp_get_next_hob(u16 type, const void *hob_list);
>
>  /**
>   * Returns the next instance of the matched GUID HOB from the starting HOB.
> @@ -146,7 +155,7 @@ void *get_next_hob(u16 type, const void *hob_list);
>   *
>   * @retval:   A HOB object with matching GUID; Otherwise NULL.
>   */
> -void *get_next_guid_hob(const struct efi_guid_t *guid, const void *hob_list);
> +void *fsp_get_next_guid_hob(const struct efi_guid *guid, const void *hob_list);
>
>  /**
>   * This function retrieves a GUID HOB data buffer and size.
> @@ -159,8 +168,8 @@ void *get_next_guid_hob(const struct efi_guid_t *guid, const void *hob_list);
>   * @retval NULL:   Failed to find the GUID HOB.
>   * @retval others: GUID HOB data buffer pointer.
>   */
> -void *get_guid_hob_data(const void *hob_list, u32 *len,
> -                       struct efi_guid_t *guid);
> +void *fsp_get_guid_hob_data(const void *hob_list, u32 *len,
> +                           struct efi_guid *guid);
>
>  /**
>   * This function retrieves FSP Non-volatile Storage HOB buffer and size.
> @@ -172,7 +181,7 @@ void *get_guid_hob_data(const void *hob_list, u32 *len,
>   * @retval NULL:   Failed to find the NVS HOB.
>   * @retval others: FSP NVS data buffer pointer.
>   */
> -void *get_fsp_nvs_data(const void *hob_list, u32 *len);
> +void *fsp_get_nvs_data(const void *hob_list, u32 *len);
>
>  /**
>   * This function retrieves Bootloader temporary stack buffer and size.
> @@ -184,15 +193,15 @@ void *get_fsp_nvs_data(const void *hob_list, u32 *len);
>   * @retval NULL:   Failed to find the bootloader temporary stack HOB.
>   * @retval others: Bootloader temporary stackbuffer pointer.
>   */
> -void *get_bootloader_tmp_mem(const void *hob_list, u32 *len);
> +void *fsp_get_bootloader_tmp_mem(const void *hob_list, u32 *len);
>
>  /**
>   * This function overrides the default configurations in the UPD data region.
>   *
> - * @fsp_upd: A pointer to the upd_region_t data strcture
> + * @fsp_upd: A pointer to the upd_region data strcture
>   *
>   * @return:  None
>   */
> -void update_fsp_upd(struct upd_region_t *fsp_upd);
> +void update_fsp_upd(struct upd_region *fsp_upd);
>
>  #endif
> diff --git a/arch/x86/include/asm/arch-queensbay/fsp/fsp_types.h b/arch/x86/include/asm/arch-queensbay/fsp/fsp_types.h
> index 12ebbfd..f32d827 100644
> --- a/arch/x86/include/asm/arch-queensbay/fsp/fsp_types.h
> +++ b/arch/x86/include/asm/arch-queensbay/fsp/fsp_types.h
> @@ -8,20 +8,8 @@
>  #ifndef __FSP_TYPES_H__
>  #define __FSP_TYPES_H__
>
> -/*
> - * Boolean true value.  UEFI Specification defines this value to be 1,
> - * but this form is more portable.
> - */
> -#define TRUE                   ((unsigned char)(1 == 1))
> -
> -/*
> - * Boolean false value.  UEFI Specification defines this value to be 0,
> - * but this form is more portable.
> - */
> -#define FALSE                  ((unsigned char)(0 == 1))
> -
>  /* 128 bit buffer containing a unique identifier value */
> -struct efi_guid_t {
> +struct efi_guid {
>         u32     data1;
>         u16     data2;
>         u16     data3;
> @@ -80,9 +68,6 @@ struct efi_guid_t {
>  #define SIGNATURE_64(A, B, C, D, E, F, G, H)   \
>         (SIGNATURE_32(A, B, C, D) | ((u64)(SIGNATURE_32(E, F, G, H)) << 32))
>
> -/* Assertion for debug */
> -#define ASSERT(exp)    do { if (!(exp)) for (;;); } while (FALSE)
> -
>  /*
>   * Define FSP API return status code.
>   * Compatiable with EFI_STATUS defined in PI Spec.
> diff --git a/arch/x86/include/asm/arch-queensbay/fsp/fsp_vpd.h b/arch/x86/include/asm/arch-queensbay/fsp/fsp_vpd.h
> index 11cc32f..ff2bb63 100644
> --- a/arch/x86/include/asm/arch-queensbay/fsp/fsp_vpd.h
> +++ b/arch/x86/include/asm/arch-queensbay/fsp/fsp_vpd.h
> @@ -12,7 +12,9 @@
>
>  #pragma pack(1)
>
> -struct upd_region_t {
> +#define UPD_TERMINATOR 0x55AA
> +
> +struct upd_region {
>         u64     sign;                   /* Offset 0x0000 */
>         u64     reserved;               /* Offset 0x0008 */
>         u8      dummy[240];             /* Offset 0x0010 */
> @@ -39,7 +41,7 @@ struct upd_region_t {
>  #define VPD_IMAGE_ID   0x445056574F4E4E4D      /* 'MNNOWVPD' */
>  #define VPD_IMAGE_REV  0x00000301
>
> -struct vpd_region_t {
> +struct vpd_region {
>         u64     sign;                   /* Offset 0x0000 */
>         u32     img_rev;                /* Offset 0x0008 */
>         u32     upd_offset;             /* Offset 0x000C */
> diff --git a/arch/x86/lib/cmd_hob.c b/arch/x86/lib/cmd_hob.c
> index 2fdff2b..075b46f 100644
> --- a/arch/x86/lib/cmd_hob.c
> +++ b/arch/x86/lib/cmd_hob.c
> @@ -28,7 +28,7 @@ static char *hob_type[] = {
>
>  int do_hob(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>  {
> -       union hob_pointers_t hob;
> +       union hob_pointers hob;
>         u16 type;
>         char *desc;
>         int i = 0;
> --
> 1.8.2.1
>

Regards,
Simon
Bin Meng Dec. 15, 2014, 5:42 a.m. UTC | #2
Hi Simon,

On Sun, Dec 14, 2014 at 2:02 PM, Simon Glass <sjg@chromium.org> wrote:
> Hi Bin,
>
> On 13 December 2014 at 21:15, Bin Meng <bmeng.cn@gmail.com> wrote:
>> This is the follow-on patch to clean up the FSP support codes:
>>
>> - Remove the _t suffix on the structures defines
>> - Use U-Boot's assert()
>> - Use standard bool true/false
>> - Remove read_unaligned64()
>> - Use memcmp() in the compare_guid()
>> - Remove the cast in the memset() call
>> - Replace some magic numbers with macros
>> - Use panic() when no valid FSP image header is found
>> - Change some FSP utility routines to use an fsp_ prefix
>> - Add comment blocks for asm_continuation and fsp_init_done
>> - Add comments to mention find_fsp_header() may be called in a
>>   stackless environment
>> - Add comments to mention init(&params) in fsp_init() cannot
>>   be removed
>>
>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>> ---
>>
>>  arch/x86/cpu/queensbay/fsp_configs.c               |   2 +-
>>  arch/x86/cpu/queensbay/fsp_support.c               | 216 +++++++++------------
>>  arch/x86/cpu/queensbay/tnc_dram.c                  |   6 +-
>>  arch/x86/include/asm/arch-queensbay/fsp/fsp_api.h  |  14 +-
>>  arch/x86/include/asm/arch-queensbay/fsp/fsp_ffs.h  |  24 +--
>>  arch/x86/include/asm/arch-queensbay/fsp/fsp_fv.h   |  14 +-
>>  arch/x86/include/asm/arch-queensbay/fsp/fsp_hob.h  |  52 ++---
>>  .../asm/arch-queensbay/fsp/fsp_infoheader.h        |   2 +-
>>  .../include/asm/arch-queensbay/fsp/fsp_platform.h  |   4 +-
>>  .../include/asm/arch-queensbay/fsp/fsp_support.h   |  63 +++---
>>  .../x86/include/asm/arch-queensbay/fsp/fsp_types.h |  17 +-
>>  arch/x86/include/asm/arch-queensbay/fsp/fsp_vpd.h  |   6 +-
>>  arch/x86/lib/cmd_hob.c                             |   2 +-
>>  13 files changed, 198 insertions(+), 224 deletions(-)
>
> Yes this looks good. I will take this patch once we figure out the
> microcode problem as emailed earlier.
>
> I have a few more comments below also - hoping to simplify a little further.
>
>>
>> diff --git a/arch/x86/cpu/queensbay/fsp_configs.c b/arch/x86/cpu/queensbay/fsp_configs.c
>> index aef18fc..af28e45 100644
>> --- a/arch/x86/cpu/queensbay/fsp_configs.c
>> +++ b/arch/x86/cpu/queensbay/fsp_configs.c
>> @@ -8,7 +8,7 @@
>>  #include <common.h>
>>  #include <asm/arch/fsp/fsp_support.h>
>>
>> -void update_fsp_upd(struct upd_region_t *fsp_upd)
>> +void update_fsp_upd(struct upd_region *fsp_upd)
>>  {
>>         /* Override any UPD setting if required */
>>
>> diff --git a/arch/x86/cpu/queensbay/fsp_support.c b/arch/x86/cpu/queensbay/fsp_support.c
>> index f830eeb..dc2de71 100644
>> --- a/arch/x86/cpu/queensbay/fsp_support.c
>> +++ b/arch/x86/cpu/queensbay/fsp_support.c
>> @@ -10,67 +10,48 @@
>>  #include <asm/post.h>
>>
>>  /**
>> - * Reads a 64-bit value from memory that may be unaligned.
>> - *
>> - * This function returns the 64-bit value pointed to by buf. The function
>> - * guarantees that the read operation does not produce an alignment fault.
>> - *
>> - * If the buf is NULL, then ASSERT().
>> - *
>> - * @buf: Pointer to a 64-bit value that may be unaligned.
>> - *
>> - * @return: The 64-bit value read from buf.
>> - */
>> -static u64 read_unaligned64(const u64 *buf)
>> -{
>> -       ASSERT(buf != NULL);
>> -
>> -       return *buf;
>> -}
>> -
>> -/**
>>   * Compares two GUIDs
>>   *
>> - * If the GUIDs are identical then TRUE is returned.
>> - * If there are any bit differences in the two GUIDs, then FALSE is returned.
>> - *
>> - * If guid1 is NULL, then ASSERT().
>> - * If guid2 is NULL, then ASSERT().
>> + * If the GUIDs are identical then true is returned.
>> + * If there are any bit differences in the two GUIDs, then false is returned.
>>   *
>>   * @guid1:        A pointer to a 128 bit GUID.
>>   * @guid2:        A pointer to a 128 bit GUID.
>>   *
>> - * @retval TRUE:  guid1 and guid2 are identical.
>> - * @retval FALSE: guid1 and guid2 are not identical.
>> + * @retval true:  guid1 and guid2 are identical.
>> + * @retval false: guid1 and guid2 are not identical.
>>   */
>> -static unsigned char compare_guid(const struct efi_guid_t *guid1,
>> -                                 const struct efi_guid_t *guid2)
>> +static bool compare_guid(const struct efi_guid *guid1,
>> +                        const struct efi_guid *guid2)
>>  {
>> -       u64 guid1_low;
>> -       u64 guid2_low;
>> -       u64 guid1_high;
>> -       u64 guid2_high;
>> -
>> -       guid1_low  = read_unaligned64((const u64 *)guid1);
>> -       guid2_low  = read_unaligned64((const u64 *)guid2);
>> -       guid1_high = read_unaligned64((const u64 *)guid1 + 1);
>> -       guid2_high = read_unaligned64((const u64 *)guid2 + 1);
>> -
>> -       return (unsigned char)(guid1_low == guid2_low && guid1_high == guid2_high);
>> +       if (memcmp(guid1, guid2, sizeof(struct efi_guid)) == 0)
>> +               return true;
>> +       else
>> +               return false;
>>  }
>>
>>  u32 __attribute__((optimize("O0"))) find_fsp_header(void)
>>  {
>> +       /*
>> +        * This function may be called before the a stack is established,
>> +        * so special care must be taken. First, it cannot declare any local
>> +        * variable using stack. Only register variable can be used here.
>> +        * Secondly, some compiler version will add prolog or epilog code
>> +        * for the C function. If so the function call may not work before
>> +        * stack is ready.
>
> OK, understood. This is supremely ugly - from what I understand this
> function will write to the ROM stack, fail, and it doesn't matter
> since the return address is already there?

No, this function will not write anything to the ROM stack. It only
declares a register variable fsp which is used as a pointer to move
within the FSP image header.

>> +        *
>> +        * GCC 4.8.1 has been verified to be working for the following codes.
>> +        */
>>         volatile register u8 *fsp asm("eax");
>>
>>         /* Initalize the FSP base */
>>         fsp = (u8 *)CONFIG_FSP_ADDR;
>>
>>         /* Check the FV signature, _FVH */
>> -       if (((struct fv_header_t *)fsp)->sign == 0x4856465F) {
>> +       if (((struct fv_header *)fsp)->sign == EFI_FVH_SIGNATURE) {
>>                 /* Go to the end of the FV header and align the address */
>> -               fsp += ((struct fv_header_t *)fsp)->ext_hdr_off;
>> -               fsp += ((struct fv_ext_header_t *)fsp)->ext_hdr_size;
>> +               fsp += ((struct fv_header *)fsp)->ext_hdr_off;
>> +               fsp += ((struct fv_ext_header *)fsp)->ext_hdr_size;
>>                 fsp  = (u8 *)(((u32)fsp + 7) & 0xFFFFFFF8);
>>         } else {
>>                 fsp  = 0;
>> @@ -78,20 +59,20 @@ u32 __attribute__((optimize("O0"))) find_fsp_header(void)
>>
>>         /* Check the FFS GUID */
>>         if (fsp &&
>> -           (((u32 *)&(((struct ffs_file_header_t *)fsp)->name))[0] == 0x912740BE) &&
>> -           (((u32 *)&(((struct ffs_file_header_t *)fsp)->name))[1] == 0x47342284) &&
>> -           (((u32 *)&(((struct ffs_file_header_t *)fsp)->name))[2] == 0xB08471B9) &&
>> -           (((u32 *)&(((struct ffs_file_header_t *)fsp)->name))[3] == 0x0C3F3527)) {
>> +           (((u32 *)&(((struct ffs_file_header *)fsp)->name))[0] == 0x912740BE) &&
>> +           (((u32 *)&(((struct ffs_file_header *)fsp)->name))[1] == 0x47342284) &&
>> +           (((u32 *)&(((struct ffs_file_header *)fsp)->name))[2] == 0xB08471B9) &&
>> +           (((u32 *)&(((struct ffs_file_header *)fsp)->name))[3] == 0x0C3F3527)) {
>
> Perhaps another way to fix this code. Since you have gone to the
> trouble of creating struct efi_guid, we could compare each field
> against FSP_GUID_DATA1, FSP_GUID_DATA2, etc. Also so far as I can see
> the data4 field is never used. Perhaps it could be changed to two
> 32-bit fields to simplify this?

I may have a try. I think the efi_guid is defined this way per the EFI
spec, so we'd better keep it as is. The data4 field is referenced as 8
bytes which you can see from those GUID macros like
FSP_HOB_RESOURCE_OWNER_GRAPHICS_GUID.

>
>>                 /* Add the FFS header size to find the raw section header */
>> -               fsp += sizeof(struct ffs_file_header_t);
>> +               fsp += sizeof(struct ffs_file_header);
>>         } else {
>>                 fsp = 0;
>>         }
>>
>>         if (fsp &&
>> -           ((struct raw_section_t *)fsp)->type == EFI_SECTION_RAW) {
>> +           ((struct raw_section *)fsp)->type == EFI_SECTION_RAW) {
>>                 /* Add the raw section header size to find the FSP header */
>> -               fsp += sizeof(struct raw_section_t);
>> +               fsp += sizeof(struct raw_section);
>>         } else {
>>                 fsp = 0;
>>         }
>> @@ -99,7 +80,7 @@ u32 __attribute__((optimize("O0"))) find_fsp_header(void)
>>         return (u32)fsp;
>>  }
>>
>> -void fsp_continue(struct shared_data_t *shared_data, u32 status, void *hob_list)
>> +void fsp_continue(struct shared_data *shared_data, u32 status, void *hob_list)
>>  {
>>         u32 stack_len;
>>         u32 stack_base;
>> @@ -107,18 +88,18 @@ void fsp_continue(struct shared_data_t *shared_data, u32 status, void *hob_list)
>>
>>         post_code(POST_MRC);
>>
>> -       ASSERT(status == 0);
>> +       assert(status == 0);
>>
>>         /* Get the migrated stack in normal memory */
>> -       stack_base = (u32)get_bootloader_tmp_mem(hob_list, &stack_len);
>> -       ASSERT(stack_base != 0);
>> +       stack_base = (u32)fsp_get_bootloader_tmp_mem(hob_list, &stack_len);
>> +       assert(stack_base != 0);
>>         stack_top  = stack_base + stack_len - sizeof(u32);
>>
>>         /*
>>          * Old stack base is stored at the very end of the stack top,
>>          * use it to calculate the migrated shared data base
>>          */
>> -       shared_data = (struct shared_data_t *)(stack_base +
>> +       shared_data = (struct shared_data *)(stack_base +
>>                         ((u32)shared_data - *(u32 *)stack_top));
>>
>>         /* The boot loader main function entry */
>> @@ -127,50 +108,50 @@ void fsp_continue(struct shared_data_t *shared_data, u32 status, void *hob_list)
>>
>>  void fsp_init(u32 stack_top, u32 boot_mode, void *nvs_buf)
>>  {
>> -       struct shared_data_t shared_data;
>> +       struct shared_data shared_data;
>>         fsp_init_f init;
>> -       struct fsp_init_params_t params;
>> -       struct fspinit_rtbuf_t rt_buf;
>> -       struct vpd_region_t *fsp_vpd;
>> -       struct fsp_header_t *fsp_hdr;
>> -       struct fsp_init_params_t *params_ptr;
>> -       struct upd_region_t *fsp_upd;
>> -
>> -       fsp_hdr = (struct fsp_header_t *)find_fsp_header();
>> +       struct fsp_init_params params;
>> +       struct fspinit_rtbuf rt_buf;
>> +       struct vpd_region *fsp_vpd;
>> +       struct fsp_header *fsp_hdr;
>> +       struct fsp_init_params *params_ptr;
>> +       struct upd_region *fsp_upd;
>> +
>> +       fsp_hdr = (struct fsp_header *)find_fsp_header();
>>         if (fsp_hdr == NULL) {
>>                 /* No valid FSP info header was found */
>> -               ASSERT(FALSE);
>> +               panic("Invalid FSP header");
>>         }
>>
>> -       fsp_upd = (struct upd_region_t *)&shared_data.fsp_upd;
>> -       memset((void *)&rt_buf, 0, sizeof(struct fspinit_rtbuf_t));
>> +       fsp_upd = (struct upd_region *)&shared_data.fsp_upd;
>> +       memset(&rt_buf, 0, sizeof(struct fspinit_rtbuf));
>>
>>         /* Reserve a gap in stack top */
>>         rt_buf.common.stack_top = (u32 *)stack_top - 32;
>>         rt_buf.common.boot_mode = boot_mode;
>> -       rt_buf.common.upd_data = (struct upd_region_t *)fsp_upd;
>> +       rt_buf.common.upd_data = (struct upd_region *)fsp_upd;
>>
>>         /* Get VPD region start */
>> -       fsp_vpd = (struct vpd_region_t *)(fsp_hdr->img_base +
>> +       fsp_vpd = (struct vpd_region *)(fsp_hdr->img_base +
>>                         fsp_hdr->cfg_region_off);
>>
>>         /* Verifify the VPD data region is valid */
>> -       ASSERT((fsp_vpd->img_rev == VPD_IMAGE_REV) &&
>> +       assert((fsp_vpd->img_rev == VPD_IMAGE_REV) &&
>>                (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_t));
>> +              sizeof(struct upd_region));
>>
>>         /* Verifify the UPD data region is valid */
>> -       ASSERT(fsp_upd->terminator == 0x55AA);
>> +       assert(fsp_upd->terminator == UPD_TERMINATOR);
>>
>>         /* Override any UPD setting if required */
>>         update_fsp_upd(fsp_upd);
>>
>> -       memset((void *)&params, 0, sizeof(struct fsp_init_params_t));
>> +       memset(&params, 0, sizeof(struct fsp_init_params));
>>         params.nvs_buf = nvs_buf;
>> -       params.rt_buf = (struct fspinit_rtbuf_t *)&rt_buf;
>> +       params.rt_buf = (struct fspinit_rtbuf *)&rt_buf;
>>         params.continuation = (fsp_continuation_f)asm_continuation;
>>
>>         init = (fsp_init_f)(fsp_hdr->img_base + fsp_hdr->fsp_init);
>> @@ -199,32 +180,28 @@ void fsp_init(u32 stack_top, u32 boot_mode, void *nvs_buf)
>>
>>         /*
>>          * Should never get here.
>> -        * Control will continue from romstage_main_continue_asm.
>> +        * Control will continue from fsp_continue.
>>          * This line below is to prevent the compiler from optimizing
>>          * structure intialization.
>> +        *
>> +        * DO NOT REMOVE!
>>          */
>>         init(&params);
>> -
>> -       /*
>> -        * Should never return.
>> -        * Control will continue from ContinuationFunc
>> -        */
>> -       ASSERT(FALSE);
>>  }
>>
>> -u32 fsp_notify(struct fsp_header_t *fsp_hdr, u32 phase)
>> +u32 fsp_notify(struct fsp_header *fsp_hdr, u32 phase)
>>  {
>>         fsp_notify_f notify;
>> -       struct fsp_notify_params_t params;
>> -       struct fsp_notify_params_t *params_ptr;
>> +       struct fsp_notify_params params;
>> +       struct fsp_notify_params *params_ptr;
>>         u32 status;
>>
>>         if (!fsp_hdr)
>> -               fsp_hdr = (struct fsp_header_t *)find_fsp_header();
>> +               fsp_hdr = (struct fsp_header *)find_fsp_header();
>>
>>         if (fsp_hdr == NULL) {
>>                 /* No valid FSP info header */
>> -               ASSERT(FALSE);
>> +               panic("Invalid FSP header");
>>         }
>>
>>         notify = (fsp_notify_f)(fsp_hdr->img_base + fsp_hdr->fsp_notify);
>> @@ -245,9 +222,9 @@ u32 fsp_notify(struct fsp_header_t *fsp_hdr, u32 phase)
>>         return status;
>>  }
>>
>> -u32 get_usable_lowmem_top(const void *hob_list)
>> +u32 fsp_get_usable_lowmem_top(const void *hob_list)
>>  {
>> -       union hob_pointers_t hob;
>> +       union hob_pointers hob;
>>         phys_addr_t phys_start;
>>         u32 top;
>>
>> @@ -255,14 +232,14 @@ u32 get_usable_lowmem_top(const void *hob_list)
>>         hob.raw = (void *)hob_list;
>>
>>         /* * Collect memory ranges */
>> -       top = 0x100000;
>> +       top = FSP_LOWMEM_BASE;
>>         while (!END_OF_HOB(hob)) {
>>                 if (hob.hdr->type == HOB_TYPE_RES_DESC) {
>>                         if (hob.res_desc->type == RES_SYS_MEM) {
>>                                 phys_start = hob.res_desc->phys_start;
>>                                 /* Need memory above 1MB to be collected here */
>> -                               if (phys_start >= 0x100000 &&
>> -                                   phys_start < (phys_addr_t)0x100000000)
>> +                               if (phys_start >= FSP_LOWMEM_BASE &&
>> +                                   phys_start < (phys_addr_t)FSP_HIGHMEM_BASE)
>>                                         top += (u32)(hob.res_desc->len);
>>                         }
>>                 }
>> @@ -272,9 +249,9 @@ u32 get_usable_lowmem_top(const void *hob_list)
>>         return top;
>>  }
>>
>> -u64 get_usable_highmem_top(const void *hob_list)
>> +u64 fsp_get_usable_highmem_top(const void *hob_list)
>>  {
>> -       union hob_pointers_t hob;
>> +       union hob_pointers hob;
>>         phys_addr_t phys_start;
>>         u64 top;
>>
>> @@ -282,13 +259,13 @@ u64 get_usable_highmem_top(const void *hob_list)
>>         hob.raw = (void *)hob_list;
>>
>>         /* Collect memory ranges */
>> -       top = 0x100000000;
>> +       top = FSP_HIGHMEM_BASE;
>>         while (!END_OF_HOB(hob)) {
>>                 if (hob.hdr->type == HOB_TYPE_RES_DESC) {
>>                         if (hob.res_desc->type == RES_SYS_MEM) {
>>                                 phys_start = hob.res_desc->phys_start;
>>                                 /* Need memory above 1MB to be collected here */
>> -                               if (phys_start >= (phys_addr_t)0x100000000)
>> +                               if (phys_start >= (phys_addr_t)FSP_HIGHMEM_BASE)
>>                                         top += (u32)(hob.res_desc->len);
>>                         }
>>                 }
>> @@ -298,10 +275,10 @@ u64 get_usable_highmem_top(const void *hob_list)
>>         return top;
>>  }
>>
>> -u64 get_fsp_reserved_mem_from_guid(const void *hob_list, u64 *len,
>> -                                  struct efi_guid_t *guid)
>> +u64 fsp_get_reserved_mem_from_guid(const void *hob_list, u64 *len,
>> +                                  struct efi_guid *guid)
>>  {
>> -       union hob_pointers_t hob;
>> +       union hob_pointers hob;
>>
>>         /* Get the HOB list for processing */
>>         hob.raw = (void *)hob_list;
>> @@ -324,39 +301,39 @@ u64 get_fsp_reserved_mem_from_guid(const void *hob_list, u64 *len,
>>         return 0;
>>  }
>>
>> -u32 get_fsp_reserved_mem(const void *hob_list, u32 *len)
>> +u32 fsp_get_fsp_reserved_mem(const void *hob_list, u32 *len)
>>  {
>> -       const struct efi_guid_t guid = FSP_HOB_RESOURCE_OWNER_FSP_GUID;
>> +       const struct efi_guid guid = FSP_HOB_RESOURCE_OWNER_FSP_GUID;
>>         u64 length;
>>         u32 base;
>>
>> -       base = (u32)get_fsp_reserved_mem_from_guid(hob_list,
>> -                       &length, (struct efi_guid_t *)&guid);
>> +       base = (u32)fsp_get_reserved_mem_from_guid(hob_list,
>> +                       &length, (struct efi_guid *)&guid);
>>         if ((len != 0) && (base != 0))
>>                 *len = (u32)length;
>>
>>         return base;
>>  }
>>
>> -u32 get_tseg_reserved_mem(const void *hob_list, u32 *len)
>> +u32 fsp_get_tseg_reserved_mem(const void *hob_list, u32 *len)
>>  {
>> -       const struct efi_guid_t guid = FSP_HOB_RESOURCE_OWNER_TSEG_GUID;
>> +       const struct efi_guid guid = FSP_HOB_RESOURCE_OWNER_TSEG_GUID;
>>         u64 length;
>>         u32 base;
>>
>> -       base = (u32)get_fsp_reserved_mem_from_guid(hob_list,
>> -                       &length, (struct efi_guid_t *)&guid);
>> +       base = (u32)fsp_get_reserved_mem_from_guid(hob_list,
>> +                       &length, (struct efi_guid *)&guid);
>>         if ((len != 0) && (base != 0))
>>                 *len = (u32)length;
>>
>>         return base;
>>  }
>>
>> -void *get_next_hob(u16 type, const void *hob_list)
>> +void *fsp_get_next_hob(u16 type, const void *hob_list)
>>  {
>> -       union hob_pointers_t hob;
>> +       union hob_pointers hob;
>>
>> -       ASSERT(hob_list != NULL);
>> +       assert(hob_list != NULL);
>>
>>         hob.raw = (u8 *)hob_list;
>>
>> @@ -371,12 +348,12 @@ void *get_next_hob(u16 type, const void *hob_list)
>>         return NULL;
>>  }
>>
>> -void *get_next_guid_hob(const struct efi_guid_t *guid, const void *hob_list)
>> +void *fsp_get_next_guid_hob(const struct efi_guid *guid, const void *hob_list)
>>  {
>> -       union hob_pointers_t hob;
>> +       union hob_pointers hob;
>>
>>         hob.raw = (u8 *)hob_list;
>> -       while ((hob.raw = get_next_hob(HOB_TYPE_GUID_EXT,
>> +       while ((hob.raw = fsp_get_next_hob(HOB_TYPE_GUID_EXT,
>>                         hob.raw)) != NULL) {
>>                 if (compare_guid(guid, &hob.guid->name))
>>                         break;
>> @@ -386,11 +363,12 @@ void *get_next_guid_hob(const struct efi_guid_t *guid, const void *hob_list)
>>         return hob.raw;
>>  }
>>
>> -void *get_guid_hob_data(const void *hob_list, u32 *len, struct efi_guid_t *guid)
>> +void *fsp_get_guid_hob_data(const void *hob_list, u32 *len,
>> +                           struct efi_guid *guid)
>>  {
>>         u8 *guid_hob;
>>
>> -       guid_hob = get_next_guid_hob(guid, hob_list);
>> +       guid_hob = fsp_get_next_guid_hob(guid, hob_list);
>>         if (guid_hob == NULL) {
>>                 return NULL;
>>         } else {
>> @@ -401,16 +379,16 @@ void *get_guid_hob_data(const void *hob_list, u32 *len, struct efi_guid_t *guid)
>>         }
>>  }
>>
>> -void *get_fsp_nvs_data(const void *hob_list, u32 *len)
>> +void *fsp_get_nvs_data(const void *hob_list, u32 *len)
>>  {
>> -       const struct efi_guid_t guid = FSP_NON_VOLATILE_STORAGE_HOB_GUID;
>> +       const struct efi_guid guid = FSP_NON_VOLATILE_STORAGE_HOB_GUID;
>>
>> -       return get_guid_hob_data(hob_list, len, (struct efi_guid_t *)&guid);
>> +       return fsp_get_guid_hob_data(hob_list, len, (struct efi_guid *)&guid);
>>  }
>>
>> -void *get_bootloader_tmp_mem(const void *hob_list, u32 *len)
>> +void *fsp_get_bootloader_tmp_mem(const void *hob_list, u32 *len)
>>  {
>> -       const struct efi_guid_t guid = FSP_BOOTLOADER_TEMP_MEM_HOB_GUID;
>> +       const struct efi_guid guid = FSP_BOOTLOADER_TEMP_MEM_HOB_GUID;
>>
>> -       return get_guid_hob_data(hob_list, len, (struct efi_guid_t *)&guid);
>> +       return fsp_get_guid_hob_data(hob_list, len, (struct efi_guid *)&guid);
>>  }
>> diff --git a/arch/x86/cpu/queensbay/tnc_dram.c b/arch/x86/cpu/queensbay/tnc_dram.c
>> index dbc1710..378af01 100644
>> --- a/arch/x86/cpu/queensbay/tnc_dram.c
>> +++ b/arch/x86/cpu/queensbay/tnc_dram.c
>> @@ -14,7 +14,7 @@ DECLARE_GLOBAL_DATA_PTR;
>>  int dram_init(void)
>>  {
>>         phys_size_t ram_size = 0;
>> -       union hob_pointers_t hob;
>> +       union hob_pointers hob;
>>
>>         hob.raw = gd->arch.hob_list;
>>         while (!END_OF_HOB(hob)) {
>> @@ -49,14 +49,14 @@ void dram_init_banksize(void)
>>   */
>>  ulong board_get_usable_ram_top(ulong total_size)
>>  {
>> -       return get_usable_lowmem_top(gd->arch.hob_list);
>> +       return fsp_get_usable_lowmem_top(gd->arch.hob_list);
>>  }
>>
>>  unsigned install_e820_map(unsigned max_entries, struct e820entry *entries)
>>  {
>>         unsigned num_entries = 0;
>>
>> -       union hob_pointers_t hob;
>> +       union hob_pointers hob;
>>
>>         hob.raw = gd->arch.hob_list;
>>
>> diff --git a/arch/x86/include/asm/arch-queensbay/fsp/fsp_api.h b/arch/x86/include/asm/arch-queensbay/fsp/fsp_api.h
>> index 25b938f..95f0e06 100644
>> --- a/arch/x86/include/asm/arch-queensbay/fsp/fsp_api.h
>> +++ b/arch/x86/include/asm/arch-queensbay/fsp/fsp_api.h
>> @@ -16,7 +16,7 @@ typedef void (*fsp_continuation_f)(u32 status, void *hob_list);
>>
>>  #pragma pack(1)
>>
>> -struct fsp_init_params_t {
>> +struct fsp_init_params {
>>         /* Non-volatile storage buffer pointer */
>>         void                    *nvs_buf;
>>         /* Runtime buffer pointer */
>> @@ -25,7 +25,7 @@ struct fsp_init_params_t {
>>         fsp_continuation_f      continuation;
>>  };
>>
>> -struct common_buf_t {
>> +struct common_buf {
>>         /*
>>          * Stack top pointer used by the bootloader. The new stack frame will be
>>          * set up at this location after FspInit API call.
>> @@ -36,24 +36,24 @@ struct common_buf_t {
>>         u32     reserved[7];    /* Reserved */
>>  };
>>
>> -enum fsp_phase_t {
>> +enum fsp_phase {
>>         /* Notification code for post PCI enuermation */
>>         INIT_PHASE_PCI  = 0x20,
>>         /* Notification code before transfering control to the payload */
>>         INIT_PHASE_BOOT = 0x40
>>  };
>>
>> -struct fsp_notify_params_t {
>> +struct fsp_notify_params {
>>         /* Notification phase used for NotifyPhase API */
>> -       enum fsp_phase_t        phase;
>> +       enum fsp_phase  phase;
>>  };
>>
>>  #pragma pack()
>>
>>  /* FspInit API function prototype */
>> -typedef u32 (*fsp_init_f)(struct fsp_init_params_t *param);
>> +typedef u32 (*fsp_init_f)(struct fsp_init_params *params);
>>
>>  /* FspNotify API function prototype */
>> -typedef u32 (*fsp_notify_f)(struct fsp_notify_params_t *param);
>> +typedef u32 (*fsp_notify_f)(struct fsp_notify_params *params);
>>
>>  #endif
>> diff --git a/arch/x86/include/asm/arch-queensbay/fsp/fsp_ffs.h b/arch/x86/include/asm/arch-queensbay/fsp/fsp_ffs.h
>> index 1f73680..7fef3e7 100644
>> --- a/arch/x86/include/asm/arch-queensbay/fsp/fsp_ffs.h
>> +++ b/arch/x86/include/asm/arch-queensbay/fsp/fsp_ffs.h
>> @@ -11,7 +11,7 @@
>>  #pragma pack(1)
>>
>>  /* Used to verify the integrity of the file */
>> -union ffs_integrity_t {
>> +union ffs_integrity {
>>         struct {
>>                 /*
>>                  * The IntegrityCheck.checksum.header field is an 8-bit
>> @@ -43,14 +43,14 @@ union ffs_integrity_t {
>>   * Each file begins with the header that describe the
>>   * contents and state of the files.
>>   */
>> -struct ffs_file_header_t {
>> +struct ffs_file_header {
>>         /*
>>          * This GUID is the file name.
>>          * It is used to uniquely identify the file.
>>          */
>> -       struct efi_guid_t       name;
>> +       struct efi_guid         name;
>>         /* Used to verify the integrity of the file */
>> -       union ffs_integrity_t   integrity;
>> +       union ffs_integrity     integrity;
>>         /* Identifies the type of file */
>>         u8                      type;
>>         /* Declares various file attribute bits */
>> @@ -64,16 +64,16 @@ struct ffs_file_header_t {
>>         u8                      state;
>>  };
>>
>> -struct ffs_file_header2_t {
>> +struct ffs_file_header2 {
>>         /*
>>          * This GUID is the file name. It is used to uniquely identify the file.
>>          * There may be only one instance of a file with the file name GUID of
>>          * Name in any given firmware volume, except if the file type is
>>          * EFI_FV_FILE_TYPE_FFS_PAD.
>>          */
>> -       struct efi_guid_t       name;
>> +       struct efi_guid         name;
>>         /* Used to verify the integrity of the file */
>> -       union ffs_integrity_t   integrity;
>> +       union ffs_integrity     integrity;
>>         /* Identifies the type of file */
>>         u8                      type;
>>         /* Declares various file attribute bits */
>> @@ -81,9 +81,9 @@ struct ffs_file_header2_t {
>>         /*
>>          * The length of the file in bytes, including the FFS header.
>>          * The length of the file data is either
>> -        * (size - sizeof(struct ffs_file_header_t)). This calculation means a
>> +        * (size - sizeof(struct ffs_file_header)). This calculation means a
>>          * zero-length file has a size of 24 bytes, which is
>> -        * sizeof(struct ffs_file_header_t). Size is not required to be a
>> +        * sizeof(struct ffs_file_header). Size is not required to be a
>>          * multiple of 8 bytes. Given a file F, the next file header is located
>>          * at the next 8-byte aligned firmware volume offset following the last
>>          * byte of the file F.
>> @@ -98,7 +98,7 @@ struct ffs_file_header2_t {
>>          * If FFS_ATTRIB_LARGE_FILE is set in attr, then ext_size exists
>>          * and size must be set to zero.
>>          * If FFS_ATTRIB_LARGE_FILE is not set then
>> -        * struct ffs_file_header_t is used.
>> +        * struct ffs_file_header is used.
>>          */
>>         u32                     ext_size;
>>  };
>> @@ -129,7 +129,7 @@ struct ffs_file_header2_t {
>>  #define EFI_SECTION_SMM_DEPEX                  0x1C
>>
>>  /* Common section header */
>> -struct raw_section_t {
>> +struct raw_section {
>>         /*
>>          * A 24-bit unsigned integer that contains the total size of
>>          * the section in bytes, including the EFI_COMMON_SECTION_HEADER.
>> @@ -138,7 +138,7 @@ struct raw_section_t {
>>         u8      type;
>>  };
>>
>> -struct raw_section2_t {
>> +struct raw_section2 {
>>         /*
>>          * A 24-bit unsigned integer that contains the total size of
>>          * the section in bytes, including the EFI_COMMON_SECTION_HEADER.
>> diff --git a/arch/x86/include/asm/arch-queensbay/fsp/fsp_fv.h b/arch/x86/include/asm/arch-queensbay/fsp/fsp_fv.h
>> index 01300db..a024451 100644
>> --- a/arch/x86/include/asm/arch-queensbay/fsp/fsp_fv.h
>> +++ b/arch/x86/include/asm/arch-queensbay/fsp/fsp_fv.h
>> @@ -63,7 +63,7 @@
>>  #define EFI_FVB2_ALIGNMENT_1G          0x001E0000
>>  #define EFI_FVB2_ALIGNMENT_2G          0x001F0000
>>
>> -struct fv_blkmap_entry_t {
>> +struct fv_blkmap_entry {
>>         /* The number of sequential blocks which are of the same size */
>>         u32     num_blocks;
>>         /* The size of the blocks */
>> @@ -71,7 +71,7 @@ struct fv_blkmap_entry_t {
>>  };
>>
>>  /* Describes the features and layout of the firmware volume */
>> -struct fv_header_t {
>> +struct fv_header {
>>         /*
>>          * The first 16 bytes are reserved to allow for the reset vector of
>>          * processors whose reset vector is at address 0.
>> @@ -81,7 +81,7 @@ struct fv_header_t {
>>          * Declares the file system with which the firmware volume
>>          * is formatted.
>>          */
>> -       struct efi_guid_t       fs_guid;
>> +       struct efi_guid         fs_guid;
>>         /*
>>          * Length in bytes of the complete firmware volume, including
>>          * the header.
>> @@ -118,18 +118,18 @@ struct fv_header_t {
>>          * An array of run-length encoded FvBlockMapEntry structures.
>>          * The array is terminated with an entry of {0,0}.
>>          */
>> -       struct fv_blkmap_entry_t        block_map[1];
>> +       struct fv_blkmap_entry  block_map[1];
>>  };
>>
>> -#define EFI_FVH_SIGNATURE SIGNATURE_32('_', 'F', 'V', 'H')
>> +#define EFI_FVH_SIGNATURE      SIGNATURE_32('_', 'F', 'V', 'H')
>>
>>  /* Firmware Volume Header Revision definition */
>>  #define EFI_FVH_REVISION       0x02
>>
>>  /* Extension header pointed by ExtHeaderOffset of volume header */
>> -struct fv_ext_header_t {
>> +struct fv_ext_header {
>>         /* firmware volume name */
>> -       struct efi_guid_t       fv_name;
>> +       struct efi_guid         fv_name;
>>         /* Size of the rest of the extension header including this structure */
>>         u32                     ext_hdr_size;
>>  };
>> diff --git a/arch/x86/include/asm/arch-queensbay/fsp/fsp_hob.h b/arch/x86/include/asm/arch-queensbay/fsp/fsp_hob.h
>> index 44c0f90..b4fc11b 100644
>> --- a/arch/x86/include/asm/arch-queensbay/fsp/fsp_hob.h
>> +++ b/arch/x86/include/asm/arch-queensbay/fsp/fsp_hob.h
>> @@ -19,14 +19,14 @@
>>   * Describes the format and size of the data inside the HOB.
>>   * All HOBs must contain this generic HOB header.
>>   */
>> -struct hob_header_t {
>> +struct hob_header {
>>         u16     type;           /* HOB type */
>>         u16     len;            /* HOB length */
>>         u32     reserved;       /* always zero */
>>  };
>>
>>  /* Enumeration of memory types introduced in UEFI */
>> -enum efi_mem_type_t {
>> +enum efi_mem_type {
>>         EFI_RESERVED_MEMORY_TYPE,
>>         /*
>>          * The code portions of a loaded application.
>> @@ -87,16 +87,16 @@ enum efi_mem_type_t {
>>   * exist outside the HOB list. This HOB type describes how memory is used,
>>   * not the physical attributes of memory.
>>   */
>> -struct hob_mem_alloc_t {
>> -       struct hob_header_t     hdr;
>> +struct hob_mem_alloc {
>> +       struct hob_header       hdr;
>>         /*
>>          * A GUID that defines the memory allocation region's type and purpose,
>>          * as well as other fields within the memory allocation HOB. This GUID
>>          * is used to define the additional data within the HOB that may be
>> -        * present for the memory allocation HOB. Type efi_guid_t is defined in
>> +        * present for the memory allocation HOB. Type efi_guid is defined in
>>          * InstallProtocolInterface() in the UEFI 2.0 specification.
>>          */
>> -       struct efi_guid_t       name;
>> +       struct efi_guid         name;
>>         /*
>>          * The base address of memory allocated by this HOB.
>>          * Type phys_addr_t is defined in AllocatePages() in the UEFI 2.0
>> @@ -111,7 +111,7 @@ struct hob_mem_alloc_t {
>>          * Type EFI_MEMORY_TYPE is defined in AllocatePages() in the UEFI 2.0
>>          * specification.
>>          */
>> -       enum efi_mem_type_t     mem_type;
>> +       enum efi_mem_type       mem_type;
>>         /* padding */
>>         u8                      reserved[4];
>>  };
>> @@ -155,14 +155,14 @@ struct hob_mem_alloc_t {
>>   * Describes the resource properties of all fixed, nonrelocatable resource
>>   * ranges found on the processor host bus during the HOB producer phase.
>>   */
>> -struct hob_res_desc_t {
>> -       struct hob_header_t     hdr;
>> +struct hob_res_desc {
>> +       struct hob_header       hdr;
>>         /*
>>          * A GUID representing the owner of the resource. This GUID is
>>          * used by HOB consumer phase components to correlate device
>>          * ownership of a resource.
>>          */
>> -       struct efi_guid_t       owner;
>> +       struct efi_guid         owner;
>>         u32                     type;
>>         u32                     attr;
>>         /* The physical start address of the resource region */
>> @@ -175,19 +175,19 @@ struct hob_res_desc_t {
>>   * Allows writers of executable content in the HOB producer phase to
>>   * maintain and manage HOBs with specific GUID.
>>   */
>> -struct hob_guid_t {
>> -       struct hob_header_t     hdr;
>> +struct hob_guid {
>> +       struct hob_header       hdr;
>>         /* A GUID that defines the contents of this HOB */
>> -       struct efi_guid_t       name;
>> +       struct efi_guid         name;
>>         /* GUID specific data goes here */
>>  };
>>
>>  /* Union of all the possible HOB Types */
>> -union hob_pointers_t {
>> -       struct hob_header_t     *hdr;
>> -       struct hob_mem_alloc_t  *mem_alloc;
>> -       struct hob_res_desc_t   *res_desc;
>> -       struct hob_guid_t       *guid;
>> +union hob_pointers {
>> +       struct hob_header       *hdr;
>> +       struct hob_mem_alloc    *mem_alloc;
>> +       struct hob_res_desc     *res_desc;
>> +       struct hob_guid         *guid;
>>         u8                      *raw;
>>  };
>>
>> @@ -202,7 +202,7 @@ union hob_pointers_t {
>>   * @return: HOB type.
>>   */
>>  #define GET_HOB_TYPE(hob) \
>> -       ((*(struct hob_header_t **)&(hob))->type)
>> +       ((*(struct hob_header **)&(hob))->type)
>
> Can we remove all these casts?
>
> How about hob->hdr.type? Also maybe convert these to static inline
> functions? (lower case)?

I think I can simplify this.

>>
>>  /**
>>   * Returns the length, in bytes, of a HOB.
>> @@ -215,7 +215,7 @@ union hob_pointers_t {
>>   * @return: HOB length.
>>   */
>>  #define GET_HOB_LENGTH(hob) \
>> -       ((*(struct hob_header_t **)&(hob))->len)
>> +       ((*(struct hob_header **)&(hob))->len)
>
> hob->hdr.len?

This too.

>>
>>  /**
>>   * Returns a pointer to the next HOB in the HOB list.
>> @@ -234,13 +234,13 @@ union hob_pointers_t {
>>   * Determines if a HOB is the last HOB in the HOB list.
>>   *
>>   * This macro determine if the HOB specified by hob is the last HOB in the
>> - * HOB list.  If hob is last HOB in the HOB list, then TRUE is returned.
>> - * Otherwise, FALSE is returned.
>> + * HOB list.  If hob is last HOB in the HOB list, then true is returned.
>> + * Otherwise, false is returned.
>>   *
>>   * @hob:          A pointer to a HOB.
>>   *
>> - * @retval TRUE:  The HOB specified by hob is the last HOB in the HOB list.
>> - * @retval FALSE: The HOB specified by hob is not the last HOB in the HOB list.
>> + * @retval true:  The HOB specified by hob is the last HOB in the HOB list.
>> + * @retval false: The HOB specified by hob is not the last HOB in the HOB list.
>>   */
>>  #define END_OF_HOB(hob)        (GET_HOB_TYPE(hob) == (u16)HOB_TYPE_EOH)
>
> hob->hdr.type == HOB_TYPE_EOH
>
>>
>> @@ -255,7 +255,7 @@ union hob_pointers_t {
>>   * @return: A pointer to the data buffer in a HOB.
>>   */
>>  #define GET_GUID_HOB_DATA(hob) \
>> -       (void *)(*(u8 **)&(hob) + sizeof(struct hob_guid_t))
>> +       (void *)(*(u8 **)&(hob) + sizeof(struct hob_guid))
>
> Similarly can we use the guid member?
>
>>
>>  /**
>>   * Returns the size of the data buffer from a HOB of type HOB_TYPE_GUID_EXT.
>> @@ -268,7 +268,7 @@ union hob_pointers_t {
>>   * @return: The size of the data buffer.
>>   */
>>  #define GET_GUID_HOB_DATA_SIZE(hob)    \
>> -       (u16)(GET_HOB_LENGTH(hob) - sizeof(struct hob_guid_t))
>> +       (u16)(GET_HOB_LENGTH(hob) - sizeof(struct hob_guid))
>>
>
> Would be good to simplify this one also.
>

Yes.

I will send a v2 later.

[snip]

Regards,
Bin
diff mbox

Patch

diff --git a/arch/x86/cpu/queensbay/fsp_configs.c b/arch/x86/cpu/queensbay/fsp_configs.c
index aef18fc..af28e45 100644
--- a/arch/x86/cpu/queensbay/fsp_configs.c
+++ b/arch/x86/cpu/queensbay/fsp_configs.c
@@ -8,7 +8,7 @@ 
 #include <common.h>
 #include <asm/arch/fsp/fsp_support.h>
 
-void update_fsp_upd(struct upd_region_t *fsp_upd)
+void update_fsp_upd(struct upd_region *fsp_upd)
 {
 	/* Override any UPD setting if required */
 
diff --git a/arch/x86/cpu/queensbay/fsp_support.c b/arch/x86/cpu/queensbay/fsp_support.c
index f830eeb..dc2de71 100644
--- a/arch/x86/cpu/queensbay/fsp_support.c
+++ b/arch/x86/cpu/queensbay/fsp_support.c
@@ -10,67 +10,48 @@ 
 #include <asm/post.h>
 
 /**
- * Reads a 64-bit value from memory that may be unaligned.
- *
- * This function returns the 64-bit value pointed to by buf. The function
- * guarantees that the read operation does not produce an alignment fault.
- *
- * If the buf is NULL, then ASSERT().
- *
- * @buf: Pointer to a 64-bit value that may be unaligned.
- *
- * @return: The 64-bit value read from buf.
- */
-static u64 read_unaligned64(const u64 *buf)
-{
-	ASSERT(buf != NULL);
-
-	return *buf;
-}
-
-/**
  * Compares two GUIDs
  *
- * If the GUIDs are identical then TRUE is returned.
- * If there are any bit differences in the two GUIDs, then FALSE is returned.
- *
- * If guid1 is NULL, then ASSERT().
- * If guid2 is NULL, then ASSERT().
+ * If the GUIDs are identical then true is returned.
+ * If there are any bit differences in the two GUIDs, then false is returned.
  *
  * @guid1:        A pointer to a 128 bit GUID.
  * @guid2:        A pointer to a 128 bit GUID.
  *
- * @retval TRUE:  guid1 and guid2 are identical.
- * @retval FALSE: guid1 and guid2 are not identical.
+ * @retval true:  guid1 and guid2 are identical.
+ * @retval false: guid1 and guid2 are not identical.
  */
-static unsigned char compare_guid(const struct efi_guid_t *guid1,
-				  const struct efi_guid_t *guid2)
+static bool compare_guid(const struct efi_guid *guid1,
+			 const struct efi_guid *guid2)
 {
-	u64 guid1_low;
-	u64 guid2_low;
-	u64 guid1_high;
-	u64 guid2_high;
-
-	guid1_low  = read_unaligned64((const u64 *)guid1);
-	guid2_low  = read_unaligned64((const u64 *)guid2);
-	guid1_high = read_unaligned64((const u64 *)guid1 + 1);
-	guid2_high = read_unaligned64((const u64 *)guid2 + 1);
-
-	return (unsigned char)(guid1_low == guid2_low && guid1_high == guid2_high);
+	if (memcmp(guid1, guid2, sizeof(struct efi_guid)) == 0)
+		return true;
+	else
+		return false;
 }
 
 u32 __attribute__((optimize("O0"))) find_fsp_header(void)
 {
+	/*
+	 * This function may be called before the a stack is established,
+	 * so special care must be taken. First, it cannot declare any local
+	 * variable using stack. Only register variable can be used here.
+	 * Secondly, some compiler version will add prolog or epilog code
+	 * for the C function. If so the function call may not work before
+	 * stack is ready.
+	 *
+	 * GCC 4.8.1 has been verified to be working for the following codes.
+	 */
 	volatile register u8 *fsp asm("eax");
 
 	/* Initalize the FSP base */
 	fsp = (u8 *)CONFIG_FSP_ADDR;
 
 	/* Check the FV signature, _FVH */
-	if (((struct fv_header_t *)fsp)->sign == 0x4856465F) {
+	if (((struct fv_header *)fsp)->sign == EFI_FVH_SIGNATURE) {
 		/* Go to the end of the FV header and align the address */
-		fsp += ((struct fv_header_t *)fsp)->ext_hdr_off;
-		fsp += ((struct fv_ext_header_t *)fsp)->ext_hdr_size;
+		fsp += ((struct fv_header *)fsp)->ext_hdr_off;
+		fsp += ((struct fv_ext_header *)fsp)->ext_hdr_size;
 		fsp  = (u8 *)(((u32)fsp + 7) & 0xFFFFFFF8);
 	} else {
 		fsp  = 0;
@@ -78,20 +59,20 @@  u32 __attribute__((optimize("O0"))) find_fsp_header(void)
 
 	/* Check the FFS GUID */
 	if (fsp &&
-	    (((u32 *)&(((struct ffs_file_header_t *)fsp)->name))[0] == 0x912740BE) &&
-	    (((u32 *)&(((struct ffs_file_header_t *)fsp)->name))[1] == 0x47342284) &&
-	    (((u32 *)&(((struct ffs_file_header_t *)fsp)->name))[2] == 0xB08471B9) &&
-	    (((u32 *)&(((struct ffs_file_header_t *)fsp)->name))[3] == 0x0C3F3527)) {
+	    (((u32 *)&(((struct ffs_file_header *)fsp)->name))[0] == 0x912740BE) &&
+	    (((u32 *)&(((struct ffs_file_header *)fsp)->name))[1] == 0x47342284) &&
+	    (((u32 *)&(((struct ffs_file_header *)fsp)->name))[2] == 0xB08471B9) &&
+	    (((u32 *)&(((struct ffs_file_header *)fsp)->name))[3] == 0x0C3F3527)) {
 		/* Add the FFS header size to find the raw section header */
-		fsp += sizeof(struct ffs_file_header_t);
+		fsp += sizeof(struct ffs_file_header);
 	} else {
 		fsp = 0;
 	}
 
 	if (fsp &&
-	    ((struct raw_section_t *)fsp)->type == EFI_SECTION_RAW) {
+	    ((struct raw_section *)fsp)->type == EFI_SECTION_RAW) {
 		/* Add the raw section header size to find the FSP header */
-		fsp += sizeof(struct raw_section_t);
+		fsp += sizeof(struct raw_section);
 	} else {
 		fsp = 0;
 	}
@@ -99,7 +80,7 @@  u32 __attribute__((optimize("O0"))) find_fsp_header(void)
 	return (u32)fsp;
 }
 
-void fsp_continue(struct shared_data_t *shared_data, u32 status, void *hob_list)
+void fsp_continue(struct shared_data *shared_data, u32 status, void *hob_list)
 {
 	u32 stack_len;
 	u32 stack_base;
@@ -107,18 +88,18 @@  void fsp_continue(struct shared_data_t *shared_data, u32 status, void *hob_list)
 
 	post_code(POST_MRC);
 
-	ASSERT(status == 0);
+	assert(status == 0);
 
 	/* Get the migrated stack in normal memory */
-	stack_base = (u32)get_bootloader_tmp_mem(hob_list, &stack_len);
-	ASSERT(stack_base != 0);
+	stack_base = (u32)fsp_get_bootloader_tmp_mem(hob_list, &stack_len);
+	assert(stack_base != 0);
 	stack_top  = stack_base + stack_len - sizeof(u32);
 
 	/*
 	 * Old stack base is stored at the very end of the stack top,
 	 * use it to calculate the migrated shared data base
 	 */
-	shared_data = (struct shared_data_t *)(stack_base +
+	shared_data = (struct shared_data *)(stack_base +
 			((u32)shared_data - *(u32 *)stack_top));
 
 	/* The boot loader main function entry */
@@ -127,50 +108,50 @@  void fsp_continue(struct shared_data_t *shared_data, u32 status, void *hob_list)
 
 void fsp_init(u32 stack_top, u32 boot_mode, void *nvs_buf)
 {
-	struct shared_data_t shared_data;
+	struct shared_data shared_data;
 	fsp_init_f init;
-	struct fsp_init_params_t params;
-	struct fspinit_rtbuf_t rt_buf;
-	struct vpd_region_t *fsp_vpd;
-	struct fsp_header_t *fsp_hdr;
-	struct fsp_init_params_t *params_ptr;
-	struct upd_region_t *fsp_upd;
-
-	fsp_hdr = (struct fsp_header_t *)find_fsp_header();
+	struct fsp_init_params params;
+	struct fspinit_rtbuf rt_buf;
+	struct vpd_region *fsp_vpd;
+	struct fsp_header *fsp_hdr;
+	struct fsp_init_params *params_ptr;
+	struct upd_region *fsp_upd;
+
+	fsp_hdr = (struct fsp_header *)find_fsp_header();
 	if (fsp_hdr == NULL) {
 		/* No valid FSP info header was found */
-		ASSERT(FALSE);
+		panic("Invalid FSP header");
 	}
 
-	fsp_upd = (struct upd_region_t *)&shared_data.fsp_upd;
-	memset((void *)&rt_buf, 0, sizeof(struct fspinit_rtbuf_t));
+	fsp_upd = (struct upd_region *)&shared_data.fsp_upd;
+	memset(&rt_buf, 0, sizeof(struct fspinit_rtbuf));
 
 	/* Reserve a gap in stack top */
 	rt_buf.common.stack_top = (u32 *)stack_top - 32;
 	rt_buf.common.boot_mode = boot_mode;
-	rt_buf.common.upd_data = (struct upd_region_t *)fsp_upd;
+	rt_buf.common.upd_data = (struct upd_region *)fsp_upd;
 
 	/* Get VPD region start */
-	fsp_vpd = (struct vpd_region_t *)(fsp_hdr->img_base +
+	fsp_vpd = (struct vpd_region *)(fsp_hdr->img_base +
 			fsp_hdr->cfg_region_off);
 
 	/* Verifify the VPD data region is valid */
-	ASSERT((fsp_vpd->img_rev == VPD_IMAGE_REV) &&
+	assert((fsp_vpd->img_rev == VPD_IMAGE_REV) &&
 	       (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_t));
+	       sizeof(struct upd_region));
 
 	/* Verifify the UPD data region is valid */
-	ASSERT(fsp_upd->terminator == 0x55AA);
+	assert(fsp_upd->terminator == UPD_TERMINATOR);
 
 	/* Override any UPD setting if required */
 	update_fsp_upd(fsp_upd);
 
-	memset((void *)&params, 0, sizeof(struct fsp_init_params_t));
+	memset(&params, 0, sizeof(struct fsp_init_params));
 	params.nvs_buf = nvs_buf;
-	params.rt_buf = (struct fspinit_rtbuf_t *)&rt_buf;
+	params.rt_buf = (struct fspinit_rtbuf *)&rt_buf;
 	params.continuation = (fsp_continuation_f)asm_continuation;
 
 	init = (fsp_init_f)(fsp_hdr->img_base + fsp_hdr->fsp_init);
@@ -199,32 +180,28 @@  void fsp_init(u32 stack_top, u32 boot_mode, void *nvs_buf)
 
 	/*
 	 * Should never get here.
-	 * Control will continue from romstage_main_continue_asm.
+	 * Control will continue from fsp_continue.
 	 * This line below is to prevent the compiler from optimizing
 	 * structure intialization.
+	 *
+	 * DO NOT REMOVE!
 	 */
 	init(&params);
-
-	/*
-	 * Should never return.
-	 * Control will continue from ContinuationFunc
-	 */
-	ASSERT(FALSE);
 }
 
-u32 fsp_notify(struct fsp_header_t *fsp_hdr, u32 phase)
+u32 fsp_notify(struct fsp_header *fsp_hdr, u32 phase)
 {
 	fsp_notify_f notify;
-	struct fsp_notify_params_t params;
-	struct fsp_notify_params_t *params_ptr;
+	struct fsp_notify_params params;
+	struct fsp_notify_params *params_ptr;
 	u32 status;
 
 	if (!fsp_hdr)
-		fsp_hdr = (struct fsp_header_t *)find_fsp_header();
+		fsp_hdr = (struct fsp_header *)find_fsp_header();
 
 	if (fsp_hdr == NULL) {
 		/* No valid FSP info header */
-		ASSERT(FALSE);
+		panic("Invalid FSP header");
 	}
 
 	notify = (fsp_notify_f)(fsp_hdr->img_base + fsp_hdr->fsp_notify);
@@ -245,9 +222,9 @@  u32 fsp_notify(struct fsp_header_t *fsp_hdr, u32 phase)
 	return status;
 }
 
-u32 get_usable_lowmem_top(const void *hob_list)
+u32 fsp_get_usable_lowmem_top(const void *hob_list)
 {
-	union hob_pointers_t hob;
+	union hob_pointers hob;
 	phys_addr_t phys_start;
 	u32 top;
 
@@ -255,14 +232,14 @@  u32 get_usable_lowmem_top(const void *hob_list)
 	hob.raw = (void *)hob_list;
 
 	/* * Collect memory ranges */
-	top = 0x100000;
+	top = FSP_LOWMEM_BASE;
 	while (!END_OF_HOB(hob)) {
 		if (hob.hdr->type == HOB_TYPE_RES_DESC) {
 			if (hob.res_desc->type == RES_SYS_MEM) {
 				phys_start = hob.res_desc->phys_start;
 				/* Need memory above 1MB to be collected here */
-				if (phys_start >= 0x100000 &&
-				    phys_start < (phys_addr_t)0x100000000)
+				if (phys_start >= FSP_LOWMEM_BASE &&
+				    phys_start < (phys_addr_t)FSP_HIGHMEM_BASE)
 					top += (u32)(hob.res_desc->len);
 			}
 		}
@@ -272,9 +249,9 @@  u32 get_usable_lowmem_top(const void *hob_list)
 	return top;
 }
 
-u64 get_usable_highmem_top(const void *hob_list)
+u64 fsp_get_usable_highmem_top(const void *hob_list)
 {
-	union hob_pointers_t hob;
+	union hob_pointers hob;
 	phys_addr_t phys_start;
 	u64 top;
 
@@ -282,13 +259,13 @@  u64 get_usable_highmem_top(const void *hob_list)
 	hob.raw = (void *)hob_list;
 
 	/* Collect memory ranges */
-	top = 0x100000000;
+	top = FSP_HIGHMEM_BASE;
 	while (!END_OF_HOB(hob)) {
 		if (hob.hdr->type == HOB_TYPE_RES_DESC) {
 			if (hob.res_desc->type == RES_SYS_MEM) {
 				phys_start = hob.res_desc->phys_start;
 				/* Need memory above 1MB to be collected here */
-				if (phys_start >= (phys_addr_t)0x100000000)
+				if (phys_start >= (phys_addr_t)FSP_HIGHMEM_BASE)
 					top += (u32)(hob.res_desc->len);
 			}
 		}
@@ -298,10 +275,10 @@  u64 get_usable_highmem_top(const void *hob_list)
 	return top;
 }
 
-u64 get_fsp_reserved_mem_from_guid(const void *hob_list, u64 *len,
-				   struct efi_guid_t *guid)
+u64 fsp_get_reserved_mem_from_guid(const void *hob_list, u64 *len,
+				   struct efi_guid *guid)
 {
-	union hob_pointers_t hob;
+	union hob_pointers hob;
 
 	/* Get the HOB list for processing */
 	hob.raw = (void *)hob_list;
@@ -324,39 +301,39 @@  u64 get_fsp_reserved_mem_from_guid(const void *hob_list, u64 *len,
 	return 0;
 }
 
-u32 get_fsp_reserved_mem(const void *hob_list, u32 *len)
+u32 fsp_get_fsp_reserved_mem(const void *hob_list, u32 *len)
 {
-	const struct efi_guid_t guid = FSP_HOB_RESOURCE_OWNER_FSP_GUID;
+	const struct efi_guid guid = FSP_HOB_RESOURCE_OWNER_FSP_GUID;
 	u64 length;
 	u32 base;
 
-	base = (u32)get_fsp_reserved_mem_from_guid(hob_list,
-			&length, (struct efi_guid_t *)&guid);
+	base = (u32)fsp_get_reserved_mem_from_guid(hob_list,
+			&length, (struct efi_guid *)&guid);
 	if ((len != 0) && (base != 0))
 		*len = (u32)length;
 
 	return base;
 }
 
-u32 get_tseg_reserved_mem(const void *hob_list, u32 *len)
+u32 fsp_get_tseg_reserved_mem(const void *hob_list, u32 *len)
 {
-	const struct efi_guid_t guid = FSP_HOB_RESOURCE_OWNER_TSEG_GUID;
+	const struct efi_guid guid = FSP_HOB_RESOURCE_OWNER_TSEG_GUID;
 	u64 length;
 	u32 base;
 
-	base = (u32)get_fsp_reserved_mem_from_guid(hob_list,
-			&length, (struct efi_guid_t *)&guid);
+	base = (u32)fsp_get_reserved_mem_from_guid(hob_list,
+			&length, (struct efi_guid *)&guid);
 	if ((len != 0) && (base != 0))
 		*len = (u32)length;
 
 	return base;
 }
 
-void *get_next_hob(u16 type, const void *hob_list)
+void *fsp_get_next_hob(u16 type, const void *hob_list)
 {
-	union hob_pointers_t hob;
+	union hob_pointers hob;
 
-	ASSERT(hob_list != NULL);
+	assert(hob_list != NULL);
 
 	hob.raw = (u8 *)hob_list;
 
@@ -371,12 +348,12 @@  void *get_next_hob(u16 type, const void *hob_list)
 	return NULL;
 }
 
-void *get_next_guid_hob(const struct efi_guid_t *guid, const void *hob_list)
+void *fsp_get_next_guid_hob(const struct efi_guid *guid, const void *hob_list)
 {
-	union hob_pointers_t hob;
+	union hob_pointers hob;
 
 	hob.raw = (u8 *)hob_list;
-	while ((hob.raw = get_next_hob(HOB_TYPE_GUID_EXT,
+	while ((hob.raw = fsp_get_next_hob(HOB_TYPE_GUID_EXT,
 			hob.raw)) != NULL) {
 		if (compare_guid(guid, &hob.guid->name))
 			break;
@@ -386,11 +363,12 @@  void *get_next_guid_hob(const struct efi_guid_t *guid, const void *hob_list)
 	return hob.raw;
 }
 
-void *get_guid_hob_data(const void *hob_list, u32 *len, struct efi_guid_t *guid)
+void *fsp_get_guid_hob_data(const void *hob_list, u32 *len,
+			    struct efi_guid *guid)
 {
 	u8 *guid_hob;
 
-	guid_hob = get_next_guid_hob(guid, hob_list);
+	guid_hob = fsp_get_next_guid_hob(guid, hob_list);
 	if (guid_hob == NULL) {
 		return NULL;
 	} else {
@@ -401,16 +379,16 @@  void *get_guid_hob_data(const void *hob_list, u32 *len, struct efi_guid_t *guid)
 	}
 }
 
-void *get_fsp_nvs_data(const void *hob_list, u32 *len)
+void *fsp_get_nvs_data(const void *hob_list, u32 *len)
 {
-	const struct efi_guid_t guid = FSP_NON_VOLATILE_STORAGE_HOB_GUID;
+	const struct efi_guid guid = FSP_NON_VOLATILE_STORAGE_HOB_GUID;
 
-	return get_guid_hob_data(hob_list, len, (struct efi_guid_t *)&guid);
+	return fsp_get_guid_hob_data(hob_list, len, (struct efi_guid *)&guid);
 }
 
-void *get_bootloader_tmp_mem(const void *hob_list, u32 *len)
+void *fsp_get_bootloader_tmp_mem(const void *hob_list, u32 *len)
 {
-	const struct efi_guid_t guid = FSP_BOOTLOADER_TEMP_MEM_HOB_GUID;
+	const struct efi_guid guid = FSP_BOOTLOADER_TEMP_MEM_HOB_GUID;
 
-	return get_guid_hob_data(hob_list, len, (struct efi_guid_t *)&guid);
+	return fsp_get_guid_hob_data(hob_list, len, (struct efi_guid *)&guid);
 }
diff --git a/arch/x86/cpu/queensbay/tnc_dram.c b/arch/x86/cpu/queensbay/tnc_dram.c
index dbc1710..378af01 100644
--- a/arch/x86/cpu/queensbay/tnc_dram.c
+++ b/arch/x86/cpu/queensbay/tnc_dram.c
@@ -14,7 +14,7 @@  DECLARE_GLOBAL_DATA_PTR;
 int dram_init(void)
 {
 	phys_size_t ram_size = 0;
-	union hob_pointers_t hob;
+	union hob_pointers hob;
 
 	hob.raw = gd->arch.hob_list;
 	while (!END_OF_HOB(hob)) {
@@ -49,14 +49,14 @@  void dram_init_banksize(void)
  */
 ulong board_get_usable_ram_top(ulong total_size)
 {
-	return get_usable_lowmem_top(gd->arch.hob_list);
+	return fsp_get_usable_lowmem_top(gd->arch.hob_list);
 }
 
 unsigned install_e820_map(unsigned max_entries, struct e820entry *entries)
 {
 	unsigned num_entries = 0;
 
-	union hob_pointers_t hob;
+	union hob_pointers hob;
 
 	hob.raw = gd->arch.hob_list;
 
diff --git a/arch/x86/include/asm/arch-queensbay/fsp/fsp_api.h b/arch/x86/include/asm/arch-queensbay/fsp/fsp_api.h
index 25b938f..95f0e06 100644
--- a/arch/x86/include/asm/arch-queensbay/fsp/fsp_api.h
+++ b/arch/x86/include/asm/arch-queensbay/fsp/fsp_api.h
@@ -16,7 +16,7 @@  typedef void (*fsp_continuation_f)(u32 status, void *hob_list);
 
 #pragma pack(1)
 
-struct fsp_init_params_t {
+struct fsp_init_params {
 	/* Non-volatile storage buffer pointer */
 	void			*nvs_buf;
 	/* Runtime buffer pointer */
@@ -25,7 +25,7 @@  struct fsp_init_params_t {
 	fsp_continuation_f	continuation;
 };
 
-struct common_buf_t {
+struct common_buf {
 	/*
 	 * Stack top pointer used by the bootloader. The new stack frame will be
 	 * set up at this location after FspInit API call.
@@ -36,24 +36,24 @@  struct common_buf_t {
 	u32	reserved[7];	/* Reserved */
 };
 
-enum fsp_phase_t {
+enum fsp_phase {
 	/* Notification code for post PCI enuermation */
 	INIT_PHASE_PCI	= 0x20,
 	/* Notification code before transfering control to the payload */
 	INIT_PHASE_BOOT	= 0x40
 };
 
-struct fsp_notify_params_t {
+struct fsp_notify_params {
 	/* Notification phase used for NotifyPhase API */
-	enum fsp_phase_t	phase;
+	enum fsp_phase	phase;
 };
 
 #pragma pack()
 
 /* FspInit API function prototype */
-typedef u32 (*fsp_init_f)(struct fsp_init_params_t *param);
+typedef u32 (*fsp_init_f)(struct fsp_init_params *params);
 
 /* FspNotify API function prototype */
-typedef u32 (*fsp_notify_f)(struct fsp_notify_params_t *param);
+typedef u32 (*fsp_notify_f)(struct fsp_notify_params *params);
 
 #endif
diff --git a/arch/x86/include/asm/arch-queensbay/fsp/fsp_ffs.h b/arch/x86/include/asm/arch-queensbay/fsp/fsp_ffs.h
index 1f73680..7fef3e7 100644
--- a/arch/x86/include/asm/arch-queensbay/fsp/fsp_ffs.h
+++ b/arch/x86/include/asm/arch-queensbay/fsp/fsp_ffs.h
@@ -11,7 +11,7 @@ 
 #pragma pack(1)
 
 /* Used to verify the integrity of the file */
-union ffs_integrity_t {
+union ffs_integrity {
 	struct {
 		/*
 		 * The IntegrityCheck.checksum.header field is an 8-bit
@@ -43,14 +43,14 @@  union ffs_integrity_t {
  * Each file begins with the header that describe the
  * contents and state of the files.
  */
-struct ffs_file_header_t {
+struct ffs_file_header {
 	/*
 	 * This GUID is the file name.
 	 * It is used to uniquely identify the file.
 	 */
-	struct efi_guid_t	name;
+	struct efi_guid		name;
 	/* Used to verify the integrity of the file */
-	union ffs_integrity_t	integrity;
+	union ffs_integrity	integrity;
 	/* Identifies the type of file */
 	u8			type;
 	/* Declares various file attribute bits */
@@ -64,16 +64,16 @@  struct ffs_file_header_t {
 	u8			state;
 };
 
-struct ffs_file_header2_t {
+struct ffs_file_header2 {
 	/*
 	 * This GUID is the file name. It is used to uniquely identify the file.
 	 * There may be only one instance of a file with the file name GUID of
 	 * Name in any given firmware volume, except if the file type is
 	 * EFI_FV_FILE_TYPE_FFS_PAD.
 	 */
-	struct efi_guid_t	name;
+	struct efi_guid		name;
 	/* Used to verify the integrity of the file */
-	union ffs_integrity_t	integrity;
+	union ffs_integrity	integrity;
 	/* Identifies the type of file */
 	u8			type;
 	/* Declares various file attribute bits */
@@ -81,9 +81,9 @@  struct ffs_file_header2_t {
 	/*
 	 * The length of the file in bytes, including the FFS header.
 	 * The length of the file data is either
-	 * (size - sizeof(struct ffs_file_header_t)). This calculation means a
+	 * (size - sizeof(struct ffs_file_header)). This calculation means a
 	 * zero-length file has a size of 24 bytes, which is
-	 * sizeof(struct ffs_file_header_t). Size is not required to be a
+	 * sizeof(struct ffs_file_header). Size is not required to be a
 	 * multiple of 8 bytes. Given a file F, the next file header is located
 	 * at the next 8-byte aligned firmware volume offset following the last
 	 * byte of the file F.
@@ -98,7 +98,7 @@  struct ffs_file_header2_t {
 	 * If FFS_ATTRIB_LARGE_FILE is set in attr, then ext_size exists
 	 * and size must be set to zero.
 	 * If FFS_ATTRIB_LARGE_FILE is not set then
-	 * struct ffs_file_header_t is used.
+	 * struct ffs_file_header is used.
 	 */
 	u32			ext_size;
 };
@@ -129,7 +129,7 @@  struct ffs_file_header2_t {
 #define EFI_SECTION_SMM_DEPEX			0x1C
 
 /* Common section header */
-struct raw_section_t {
+struct raw_section {
 	/*
 	 * A 24-bit unsigned integer that contains the total size of
 	 * the section in bytes, including the EFI_COMMON_SECTION_HEADER.
@@ -138,7 +138,7 @@  struct raw_section_t {
 	u8	type;
 };
 
-struct raw_section2_t {
+struct raw_section2 {
 	/*
 	 * A 24-bit unsigned integer that contains the total size of
 	 * the section in bytes, including the EFI_COMMON_SECTION_HEADER.
diff --git a/arch/x86/include/asm/arch-queensbay/fsp/fsp_fv.h b/arch/x86/include/asm/arch-queensbay/fsp/fsp_fv.h
index 01300db..a024451 100644
--- a/arch/x86/include/asm/arch-queensbay/fsp/fsp_fv.h
+++ b/arch/x86/include/asm/arch-queensbay/fsp/fsp_fv.h
@@ -63,7 +63,7 @@ 
 #define EFI_FVB2_ALIGNMENT_1G		0x001E0000
 #define EFI_FVB2_ALIGNMENT_2G		0x001F0000
 
-struct fv_blkmap_entry_t {
+struct fv_blkmap_entry {
 	/* The number of sequential blocks which are of the same size */
 	u32	num_blocks;
 	/* The size of the blocks */
@@ -71,7 +71,7 @@  struct fv_blkmap_entry_t {
 };
 
 /* Describes the features and layout of the firmware volume */
-struct fv_header_t {
+struct fv_header {
 	/*
 	 * The first 16 bytes are reserved to allow for the reset vector of
 	 * processors whose reset vector is at address 0.
@@ -81,7 +81,7 @@  struct fv_header_t {
 	 * Declares the file system with which the firmware volume
 	 * is formatted.
 	 */
-	struct efi_guid_t	fs_guid;
+	struct efi_guid		fs_guid;
 	/*
 	 * Length in bytes of the complete firmware volume, including
 	 * the header.
@@ -118,18 +118,18 @@  struct fv_header_t {
 	 * An array of run-length encoded FvBlockMapEntry structures.
 	 * The array is terminated with an entry of {0,0}.
 	 */
-	struct fv_blkmap_entry_t	block_map[1];
+	struct fv_blkmap_entry	block_map[1];
 };
 
-#define EFI_FVH_SIGNATURE SIGNATURE_32('_', 'F', 'V', 'H')
+#define EFI_FVH_SIGNATURE	SIGNATURE_32('_', 'F', 'V', 'H')
 
 /* Firmware Volume Header Revision definition */
 #define EFI_FVH_REVISION	0x02
 
 /* Extension header pointed by ExtHeaderOffset of volume header */
-struct fv_ext_header_t {
+struct fv_ext_header {
 	/* firmware volume name */
-	struct efi_guid_t	fv_name;
+	struct efi_guid		fv_name;
 	/* Size of the rest of the extension header including this structure */
 	u32			ext_hdr_size;
 };
diff --git a/arch/x86/include/asm/arch-queensbay/fsp/fsp_hob.h b/arch/x86/include/asm/arch-queensbay/fsp/fsp_hob.h
index 44c0f90..b4fc11b 100644
--- a/arch/x86/include/asm/arch-queensbay/fsp/fsp_hob.h
+++ b/arch/x86/include/asm/arch-queensbay/fsp/fsp_hob.h
@@ -19,14 +19,14 @@ 
  * Describes the format and size of the data inside the HOB.
  * All HOBs must contain this generic HOB header.
  */
-struct hob_header_t {
+struct hob_header {
 	u16	type;		/* HOB type */
 	u16	len;		/* HOB length */
 	u32	reserved;	/* always zero */
 };
 
 /* Enumeration of memory types introduced in UEFI */
-enum efi_mem_type_t {
+enum efi_mem_type {
 	EFI_RESERVED_MEMORY_TYPE,
 	/*
 	 * The code portions of a loaded application.
@@ -87,16 +87,16 @@  enum efi_mem_type_t {
  * exist outside the HOB list. This HOB type describes how memory is used,
  * not the physical attributes of memory.
  */
-struct hob_mem_alloc_t {
-	struct hob_header_t	hdr;
+struct hob_mem_alloc {
+	struct hob_header	hdr;
 	/*
 	 * A GUID that defines the memory allocation region's type and purpose,
 	 * as well as other fields within the memory allocation HOB. This GUID
 	 * is used to define the additional data within the HOB that may be
-	 * present for the memory allocation HOB. Type efi_guid_t is defined in
+	 * present for the memory allocation HOB. Type efi_guid is defined in
 	 * InstallProtocolInterface() in the UEFI 2.0 specification.
 	 */
-	struct efi_guid_t	name;
+	struct efi_guid		name;
 	/*
 	 * The base address of memory allocated by this HOB.
 	 * Type phys_addr_t is defined in AllocatePages() in the UEFI 2.0
@@ -111,7 +111,7 @@  struct hob_mem_alloc_t {
 	 * Type EFI_MEMORY_TYPE is defined in AllocatePages() in the UEFI 2.0
 	 * specification.
 	 */
-	enum efi_mem_type_t	mem_type;
+	enum efi_mem_type	mem_type;
 	/* padding */
 	u8			reserved[4];
 };
@@ -155,14 +155,14 @@  struct hob_mem_alloc_t {
  * Describes the resource properties of all fixed, nonrelocatable resource
  * ranges found on the processor host bus during the HOB producer phase.
  */
-struct hob_res_desc_t {
-	struct hob_header_t	hdr;
+struct hob_res_desc {
+	struct hob_header	hdr;
 	/*
 	 * A GUID representing the owner of the resource. This GUID is
 	 * used by HOB consumer phase components to correlate device
 	 * ownership of a resource.
 	 */
-	struct efi_guid_t	owner;
+	struct efi_guid		owner;
 	u32			type;
 	u32			attr;
 	/* The physical start address of the resource region */
@@ -175,19 +175,19 @@  struct hob_res_desc_t {
  * Allows writers of executable content in the HOB producer phase to
  * maintain and manage HOBs with specific GUID.
  */
-struct hob_guid_t {
-	struct hob_header_t	hdr;
+struct hob_guid {
+	struct hob_header	hdr;
 	/* A GUID that defines the contents of this HOB */
-	struct efi_guid_t	name;
+	struct efi_guid		name;
 	/* GUID specific data goes here */
 };
 
 /* Union of all the possible HOB Types */
-union hob_pointers_t {
-	struct hob_header_t	*hdr;
-	struct hob_mem_alloc_t	*mem_alloc;
-	struct hob_res_desc_t	*res_desc;
-	struct hob_guid_t	*guid;
+union hob_pointers {
+	struct hob_header	*hdr;
+	struct hob_mem_alloc	*mem_alloc;
+	struct hob_res_desc	*res_desc;
+	struct hob_guid		*guid;
 	u8			*raw;
 };
 
@@ -202,7 +202,7 @@  union hob_pointers_t {
  * @return: HOB type.
  */
 #define GET_HOB_TYPE(hob) \
-	((*(struct hob_header_t **)&(hob))->type)
+	((*(struct hob_header **)&(hob))->type)
 
 /**
  * Returns the length, in bytes, of a HOB.
@@ -215,7 +215,7 @@  union hob_pointers_t {
  * @return: HOB length.
  */
 #define GET_HOB_LENGTH(hob) \
-	((*(struct hob_header_t **)&(hob))->len)
+	((*(struct hob_header **)&(hob))->len)
 
 /**
  * Returns a pointer to the next HOB in the HOB list.
@@ -234,13 +234,13 @@  union hob_pointers_t {
  * Determines if a HOB is the last HOB in the HOB list.
  *
  * This macro determine if the HOB specified by hob is the last HOB in the
- * HOB list.  If hob is last HOB in the HOB list, then TRUE is returned.
- * Otherwise, FALSE is returned.
+ * HOB list.  If hob is last HOB in the HOB list, then true is returned.
+ * Otherwise, false is returned.
  *
  * @hob:          A pointer to a HOB.
  *
- * @retval TRUE:  The HOB specified by hob is the last HOB in the HOB list.
- * @retval FALSE: The HOB specified by hob is not the last HOB in the HOB list.
+ * @retval true:  The HOB specified by hob is the last HOB in the HOB list.
+ * @retval false: The HOB specified by hob is not the last HOB in the HOB list.
  */
 #define END_OF_HOB(hob)	(GET_HOB_TYPE(hob) == (u16)HOB_TYPE_EOH)
 
@@ -255,7 +255,7 @@  union hob_pointers_t {
  * @return: A pointer to the data buffer in a HOB.
  */
 #define GET_GUID_HOB_DATA(hob)	\
-	(void *)(*(u8 **)&(hob) + sizeof(struct hob_guid_t))
+	(void *)(*(u8 **)&(hob) + sizeof(struct hob_guid))
 
 /**
  * Returns the size of the data buffer from a HOB of type HOB_TYPE_GUID_EXT.
@@ -268,7 +268,7 @@  union hob_pointers_t {
  * @return: The size of the data buffer.
  */
 #define GET_GUID_HOB_DATA_SIZE(hob)	\
-	(u16)(GET_HOB_LENGTH(hob) - sizeof(struct hob_guid_t))
+	(u16)(GET_HOB_LENGTH(hob) - sizeof(struct hob_guid))
 
 /* FSP specific GUID HOB definitions */
 #define FSP_HEADER_GUID \
diff --git a/arch/x86/include/asm/arch-queensbay/fsp/fsp_infoheader.h b/arch/x86/include/asm/arch-queensbay/fsp/fsp_infoheader.h
index ad78bcd..a5edfef 100644
--- a/arch/x86/include/asm/arch-queensbay/fsp/fsp_infoheader.h
+++ b/arch/x86/include/asm/arch-queensbay/fsp/fsp_infoheader.h
@@ -12,7 +12,7 @@ 
 
 #pragma pack(1)
 
-struct fsp_header_t {
+struct fsp_header {
 	u32	sign;			/* 'FSPH' */
 	u32	hdr_len;		/* header length */
 	u8	reserved1[3];
diff --git a/arch/x86/include/asm/arch-queensbay/fsp/fsp_platform.h b/arch/x86/include/asm/arch-queensbay/fsp/fsp_platform.h
index a7b6e6b..67e4b1e 100644
--- a/arch/x86/include/asm/arch-queensbay/fsp/fsp_platform.h
+++ b/arch/x86/include/asm/arch-queensbay/fsp/fsp_platform.h
@@ -10,8 +10,8 @@ 
 
 #pragma pack(1)
 
-struct fspinit_rtbuf_t {
-	struct common_buf_t	common;	/* FSP common runtime data structure */
+struct fspinit_rtbuf {
+	struct common_buf	common;	/* FSP common runtime data structure */
 };
 
 #pragma pack()
diff --git a/arch/x86/include/asm/arch-queensbay/fsp/fsp_support.h b/arch/x86/include/asm/arch-queensbay/fsp/fsp_support.h
index 3296a2b..3ae1b66 100644
--- a/arch/x86/include/asm/arch-queensbay/fsp/fsp_support.h
+++ b/arch/x86/include/asm/arch-queensbay/fsp/fsp_support.h
@@ -18,14 +18,30 @@ 
 #include "fsp_bootmode.h"
 #include "fsp_vpd.h"
 
-struct shared_data_t {
-	struct fsp_header_t	*fsp_hdr;
+struct shared_data {
+	struct fsp_header	*fsp_hdr;
 	u32			*stack_top;
-	struct upd_region_t	fsp_upd;
+	struct upd_region	fsp_upd;
 };
 
+#define FSP_LOWMEM_BASE		0x100000UL
+#define FSP_HIGHMEM_BASE	0x100000000ULL
+
+/**
+ * FSP Continuation assembly helper routine
+ *
+ * This routine jumps to the C version of FSP continuation function
+ */
 void asm_continuation(void);
 
+/**
+ * FSP initialization complete
+ *
+ * This is the function that indicates FSP initialization is complete and jumps
+ * back to the bootloader with HOB list pointer as the parameter.
+ *
+ * @hob_list:    HOB list pointer
+ */
 void fsp_init_done(void *hob_list);
 
 /**
@@ -37,19 +53,12 @@  void fsp_init_done(void *hob_list);
  *
  * @retval:      Never returns
  */
-void fsp_continue(struct shared_data_t *shared_data, u32 status,
+void fsp_continue(struct shared_data *shared_data, u32 status,
 		  void *hob_list);
 
 /**
  * Find FSP header offset in FSP image
  *
- * If this function is called before the a stack is established, special care
- * must be taken. First, it cannot declare any local variable using stack.
- * Only register variable can be used here. Secondly, some compiler version
- * will add prolog or epilog code for the C function. If so the function call
- * may not work before stack is ready. GCC 4.8.1 has been verified to be
- * working for the following code.
- *
  * @retval: the offset of FSP header. If signature is invalid, returns 0.
  */
 u32 find_fsp_header(void);
@@ -67,11 +76,11 @@  void fsp_init(u32 stack_top, u32 boot_mode, void *nvs_buf);
  * FSP notification wrapper function
  *
  * @fsp_hdr: Pointer to FSP information header
- * @phase:   FSP initialization phase defined in enum fsp_phase_t
+ * @phase:   FSP initialization phase defined in enum fsp_phase
  *
  * @retval:  compatible status code with EFI_STATUS defined in PI spec
  */
-u32 fsp_notify(struct fsp_header_t *fsp_hdr, u32 phase);
+u32 fsp_notify(struct fsp_header *fsp_hdr, u32 phase);
 
 /**
  * This function retrieves the top of usable low memory.
@@ -80,7 +89,7 @@  u32 fsp_notify(struct fsp_header_t *fsp_hdr, u32 phase);
  *
  * @retval:   Usable low memory top.
  */
-u32 get_usable_lowmem_top(const void *hob_list);
+u32 fsp_get_usable_lowmem_top(const void *hob_list);
 
 /**
  * This function retrieves the top of usable high memory.
@@ -89,7 +98,7 @@  u32 get_usable_lowmem_top(const void *hob_list);
  *
  * @retval:   Usable high memory top.
  */
-u64 get_usable_highmem_top(const void *hob_list);
+u64 fsp_get_usable_highmem_top(const void *hob_list);
 
 /**
  * This function retrieves a special reserved memory region.
@@ -102,8 +111,8 @@  u64 get_usable_highmem_top(const void *hob_list);
  * @retval:   Reserved region start address.
  *            0 if this region does not exist.
  */
-u64 get_fsp_reserved_mem_from_guid(const void *hob_list,
-				   u64 *len, struct efi_guid_t *guid);
+u64 fsp_get_reserved_mem_from_guid(const void *hob_list,
+				   u64 *len, struct efi_guid *guid);
 
 /**
  * This function retrieves the FSP reserved normal memory.
@@ -114,7 +123,7 @@  u64 get_fsp_reserved_mem_from_guid(const void *hob_list,
  * @retval:   FSP reserved memory base
  *            0 if this region does not exist.
  */
-u32 get_fsp_reserved_mem(const void *hob_list, u32 *len);
+u32 fsp_get_fsp_reserved_mem(const void *hob_list, u32 *len);
 
 /**
  * This function retrieves the TSEG reserved normal memory.
@@ -126,7 +135,7 @@  u32 get_fsp_reserved_mem(const void *hob_list, u32 *len);
  * @retval NULL:   Failed to find the TSEG reserved memory.
  * @retval others: TSEG reserved memory base.
  */
-u32 get_tseg_reserved_mem(const void *hob_list, u32 *len);
+u32 fsp_get_tseg_reserved_mem(const void *hob_list, u32 *len);
 
 /**
  * Returns the next instance of a HOB type from the starting HOB.
@@ -136,7 +145,7 @@  u32 get_tseg_reserved_mem(const void *hob_list, u32 *len);
  *
  * @retval:   A HOB object with matching type; Otherwise NULL.
  */
-void *get_next_hob(u16 type, const void *hob_list);
+void *fsp_get_next_hob(u16 type, const void *hob_list);
 
 /**
  * Returns the next instance of the matched GUID HOB from the starting HOB.
@@ -146,7 +155,7 @@  void *get_next_hob(u16 type, const void *hob_list);
  *
  * @retval:   A HOB object with matching GUID; Otherwise NULL.
  */
-void *get_next_guid_hob(const struct efi_guid_t *guid, const void *hob_list);
+void *fsp_get_next_guid_hob(const struct efi_guid *guid, const void *hob_list);
 
 /**
  * This function retrieves a GUID HOB data buffer and size.
@@ -159,8 +168,8 @@  void *get_next_guid_hob(const struct efi_guid_t *guid, const void *hob_list);
  * @retval NULL:   Failed to find the GUID HOB.
  * @retval others: GUID HOB data buffer pointer.
  */
-void *get_guid_hob_data(const void *hob_list, u32 *len,
-			struct efi_guid_t *guid);
+void *fsp_get_guid_hob_data(const void *hob_list, u32 *len,
+			    struct efi_guid *guid);
 
 /**
  * This function retrieves FSP Non-volatile Storage HOB buffer and size.
@@ -172,7 +181,7 @@  void *get_guid_hob_data(const void *hob_list, u32 *len,
  * @retval NULL:   Failed to find the NVS HOB.
  * @retval others: FSP NVS data buffer pointer.
  */
-void *get_fsp_nvs_data(const void *hob_list, u32 *len);
+void *fsp_get_nvs_data(const void *hob_list, u32 *len);
 
 /**
  * This function retrieves Bootloader temporary stack buffer and size.
@@ -184,15 +193,15 @@  void *get_fsp_nvs_data(const void *hob_list, u32 *len);
  * @retval NULL:   Failed to find the bootloader temporary stack HOB.
  * @retval others: Bootloader temporary stackbuffer pointer.
  */
-void *get_bootloader_tmp_mem(const void *hob_list, u32 *len);
+void *fsp_get_bootloader_tmp_mem(const void *hob_list, u32 *len);
 
 /**
  * This function overrides the default configurations in the UPD data region.
  *
- * @fsp_upd: A pointer to the upd_region_t data strcture
+ * @fsp_upd: A pointer to the upd_region data strcture
  *
  * @return:  None
  */
-void update_fsp_upd(struct upd_region_t *fsp_upd);
+void update_fsp_upd(struct upd_region *fsp_upd);
 
 #endif
diff --git a/arch/x86/include/asm/arch-queensbay/fsp/fsp_types.h b/arch/x86/include/asm/arch-queensbay/fsp/fsp_types.h
index 12ebbfd..f32d827 100644
--- a/arch/x86/include/asm/arch-queensbay/fsp/fsp_types.h
+++ b/arch/x86/include/asm/arch-queensbay/fsp/fsp_types.h
@@ -8,20 +8,8 @@ 
 #ifndef __FSP_TYPES_H__
 #define __FSP_TYPES_H__
 
-/*
- * Boolean true value.  UEFI Specification defines this value to be 1,
- * but this form is more portable.
- */
-#define TRUE			((unsigned char)(1 == 1))
-
-/*
- * Boolean false value.  UEFI Specification defines this value to be 0,
- * but this form is more portable.
- */
-#define FALSE			((unsigned char)(0 == 1))
-
 /* 128 bit buffer containing a unique identifier value */
-struct efi_guid_t {
+struct efi_guid {
 	u32	data1;
 	u16	data2;
 	u16	data3;
@@ -80,9 +68,6 @@  struct efi_guid_t {
 #define SIGNATURE_64(A, B, C, D, E, F, G, H)	\
 	(SIGNATURE_32(A, B, C, D) | ((u64)(SIGNATURE_32(E, F, G, H)) << 32))
 
-/* Assertion for debug */
-#define ASSERT(exp)	do { if (!(exp)) for (;;); } while (FALSE)
-
 /*
  * Define FSP API return status code.
  * Compatiable with EFI_STATUS defined in PI Spec.
diff --git a/arch/x86/include/asm/arch-queensbay/fsp/fsp_vpd.h b/arch/x86/include/asm/arch-queensbay/fsp/fsp_vpd.h
index 11cc32f..ff2bb63 100644
--- a/arch/x86/include/asm/arch-queensbay/fsp/fsp_vpd.h
+++ b/arch/x86/include/asm/arch-queensbay/fsp/fsp_vpd.h
@@ -12,7 +12,9 @@ 
 
 #pragma pack(1)
 
-struct upd_region_t {
+#define UPD_TERMINATOR	0x55AA
+
+struct upd_region {
 	u64	sign;			/* Offset 0x0000 */
 	u64	reserved;		/* Offset 0x0008 */
 	u8	dummy[240];		/* Offset 0x0010 */
@@ -39,7 +41,7 @@  struct upd_region_t {
 #define VPD_IMAGE_ID	0x445056574F4E4E4D	/* 'MNNOWVPD' */
 #define VPD_IMAGE_REV	0x00000301
 
-struct vpd_region_t {
+struct vpd_region {
 	u64	sign;			/* Offset 0x0000 */
 	u32	img_rev;		/* Offset 0x0008 */
 	u32	upd_offset;		/* Offset 0x000C */
diff --git a/arch/x86/lib/cmd_hob.c b/arch/x86/lib/cmd_hob.c
index 2fdff2b..075b46f 100644
--- a/arch/x86/lib/cmd_hob.c
+++ b/arch/x86/lib/cmd_hob.c
@@ -28,7 +28,7 @@  static char *hob_type[] = {
 
 int do_hob(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
-	union hob_pointers_t hob;
+	union hob_pointers hob;
 	u16 type;
 	char *desc;
 	int i = 0;