[U-Boot] [PATCH] x86: Simplify the fsp hob access functions
Remove the troublesome union hob_pointers so that some annoying casts are no longer needed in those hob access routines. This also improves the readability.
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
arch/x86/cpu/queensbay/fsp_support.c | 95 ++++++++++++---------- arch/x86/cpu/queensbay/tnc_dram.c | 39 +++++---- arch/x86/include/asm/arch-queensbay/fsp/fsp_hob.h | 46 ++++------- .../include/asm/arch-queensbay/fsp/fsp_support.h | 5 +- arch/x86/lib/cmd_hob.c | 16 ++-- 5 files changed, 101 insertions(+), 100 deletions(-)
diff --git a/arch/x86/cpu/queensbay/fsp_support.c b/arch/x86/cpu/queensbay/fsp_support.c index ef1916b..4764e3c 100644 --- a/arch/x86/cpu/queensbay/fsp_support.c +++ b/arch/x86/cpu/queensbay/fsp_support.c @@ -231,26 +231,28 @@ u32 fsp_notify(struct fsp_header *fsp_hdr, u32 phase)
u32 fsp_get_usable_lowmem_top(const void *hob_list) { - union hob_pointers hob; + const struct hob_header *hdr; + struct hob_res_desc *res_desc; phys_addr_t phys_start; u32 top;
/* Get the HOB list for processing */ - hob.raw = (void *)hob_list; + hdr = hob_list;
/* * Collect memory ranges */ top = FSP_LOWMEM_BASE; - while (!end_of_hob(hob)) { - if (get_hob_type(hob) == HOB_TYPE_RES_DESC) { - if (hob.res_desc->type == RES_SYS_MEM) { - phys_start = hob.res_desc->phys_start; + while (!end_of_hob(hdr)) { + if (get_hob_type(hdr) == HOB_TYPE_RES_DESC) { + res_desc = (struct hob_res_desc *)hdr; + if (res_desc->type == RES_SYS_MEM) { + phys_start = res_desc->phys_start; /* Need memory above 1MB to be collected here */ if (phys_start >= FSP_LOWMEM_BASE && phys_start < (phys_addr_t)FSP_HIGHMEM_BASE) - top += (u32)(hob.res_desc->len); + top += (u32)(res_desc->len); } } - hob.raw = get_next_hob(hob); + hdr = get_next_hob(hdr); }
return top; @@ -258,25 +260,27 @@ u32 fsp_get_usable_lowmem_top(const void *hob_list)
u64 fsp_get_usable_highmem_top(const void *hob_list) { - union hob_pointers hob; + const struct hob_header *hdr; + struct hob_res_desc *res_desc; phys_addr_t phys_start; u64 top;
/* Get the HOB list for processing */ - hob.raw = (void *)hob_list; + hdr = hob_list;
/* Collect memory ranges */ top = FSP_HIGHMEM_BASE; - while (!end_of_hob(hob)) { - if (get_hob_type(hob) == HOB_TYPE_RES_DESC) { - if (hob.res_desc->type == RES_SYS_MEM) { - phys_start = hob.res_desc->phys_start; + while (!end_of_hob(hdr)) { + if (get_hob_type(hdr) == HOB_TYPE_RES_DESC) { + res_desc = (struct hob_res_desc *)hdr; + if (res_desc->type == RES_SYS_MEM) { + phys_start = res_desc->phys_start; /* Need memory above 1MB to be collected here */ if (phys_start >= (phys_addr_t)FSP_HIGHMEM_BASE) - top += (u32)(hob.res_desc->len); + top += (u32)(res_desc->len); } } - hob.raw = get_next_hob(hob); + hdr = get_next_hob(hdr); }
return top; @@ -285,24 +289,26 @@ u64 fsp_get_usable_highmem_top(const void *hob_list) u64 fsp_get_reserved_mem_from_guid(const void *hob_list, u64 *len, struct efi_guid *guid) { - union hob_pointers hob; + const struct hob_header *hdr; + struct hob_res_desc *res_desc;
/* Get the HOB list for processing */ - hob.raw = (void *)hob_list; + hdr = hob_list;
/* Collect memory ranges */ - while (!end_of_hob(hob)) { - if (get_hob_type(hob) == HOB_TYPE_RES_DESC) { - if (hob.res_desc->type == RES_MEM_RESERVED) { - if (compare_guid(&hob.res_desc->owner, guid)) { + while (!end_of_hob(hdr)) { + if (get_hob_type(hdr) == HOB_TYPE_RES_DESC) { + res_desc = (struct hob_res_desc *)hdr; + if (res_desc->type == RES_MEM_RESERVED) { + if (compare_guid(&res_desc->owner, guid)) { if (len) - *len = (u32)(hob.res_desc->len); + *len = (u32)(res_desc->len);
- return (u64)(hob.res_desc->phys_start); + return (u64)(res_desc->phys_start); } } } - hob.raw = get_next_hob(hob); + hdr = get_next_hob(hdr); }
return 0; @@ -336,44 +342,45 @@ u32 fsp_get_tseg_reserved_mem(const void *hob_list, u32 *len) return base; }
-void *fsp_get_next_hob(u16 type, const void *hob_list) +const struct hob_header *fsp_get_next_hob(u16 type, const void *hob_list) { - union hob_pointers hob; + const struct hob_header *hdr;
- assert(hob_list != NULL); - - hob.raw = (u8 *)hob_list; + hdr = hob_list;
/* Parse the HOB list until end of list or matching type is found */ - while (!end_of_hob(hob)) { - if (get_hob_type(hob) == type) - return hob.raw; + while (!end_of_hob(hdr)) { + if (get_hob_type(hdr) == type) + return hdr;
- hob.raw = get_next_hob(hob); + hdr = get_next_hob(hdr); }
return NULL; }
-void *fsp_get_next_guid_hob(const struct efi_guid *guid, const void *hob_list) +const struct hob_header *fsp_get_next_guid_hob(const struct efi_guid *guid, + const void *hob_list) { - union hob_pointers hob; - - hob.raw = (u8 *)hob_list; - while ((hob.raw = fsp_get_next_hob(HOB_TYPE_GUID_EXT, - hob.raw)) != NULL) { - if (compare_guid(guid, &hob.guid->name)) + const struct hob_header *hdr; + struct hob_guid *guid_hob; + + hdr = hob_list; + while ((hdr = fsp_get_next_hob(HOB_TYPE_GUID_EXT, + hdr)) != NULL) { + guid_hob = (struct hob_guid *)hdr; + if (compare_guid(guid, &(guid_hob->name))) break; - hob.raw = get_next_hob(hob); + hdr = get_next_hob(hdr); }
- return hob.raw; + return hdr; }
void *fsp_get_guid_hob_data(const void *hob_list, u32 *len, struct efi_guid *guid) { - u8 *guid_hob; + const struct hob_header *guid_hob;
guid_hob = fsp_get_next_guid_hob(guid, hob_list); if (guid_hob == NULL) { diff --git a/arch/x86/cpu/queensbay/tnc_dram.c b/arch/x86/cpu/queensbay/tnc_dram.c index 8e97c9b..b669dbc 100644 --- a/arch/x86/cpu/queensbay/tnc_dram.c +++ b/arch/x86/cpu/queensbay/tnc_dram.c @@ -14,17 +14,19 @@ DECLARE_GLOBAL_DATA_PTR; int dram_init(void) { phys_size_t ram_size = 0; - union hob_pointers hob; + const struct hob_header *hdr; + struct hob_res_desc *res_desc;
- hob.raw = gd->arch.hob_list; - while (!end_of_hob(hob)) { - if (get_hob_type(hob) == HOB_TYPE_RES_DESC) { - if (hob.res_desc->type == RES_SYS_MEM || - hob.res_desc->type == RES_MEM_RESERVED) { - ram_size += hob.res_desc->len; + hdr = gd->arch.hob_list; + while (!end_of_hob(hdr)) { + if (get_hob_type(hdr) == HOB_TYPE_RES_DESC) { + res_desc = (struct hob_res_desc *)hdr; + if (res_desc->type == RES_SYS_MEM || + res_desc->type == RES_MEM_RESERVED) { + ram_size += res_desc->len; } } - hob.raw = get_next_hob(hob); + hdr = get_next_hob(hdr); }
gd->ram_size = ram_size; @@ -55,22 +57,23 @@ ulong board_get_usable_ram_top(ulong total_size) unsigned install_e820_map(unsigned max_entries, struct e820entry *entries) { unsigned num_entries = 0; + const struct hob_header *hdr; + struct hob_res_desc *res_desc;
- union hob_pointers hob; + hdr = gd->arch.hob_list;
- hob.raw = gd->arch.hob_list; + while (!end_of_hob(hdr)) { + if (get_hob_type(hdr) == HOB_TYPE_RES_DESC) { + res_desc = (struct hob_res_desc *)hdr; + entries[num_entries].addr = res_desc->phys_start; + entries[num_entries].size = res_desc->len;
- while (!end_of_hob(hob)) { - if (get_hob_type(hob) == HOB_TYPE_RES_DESC) { - entries[num_entries].addr = hob.res_desc->phys_start; - entries[num_entries].size = hob.res_desc->len; - - if (hob.res_desc->type == RES_SYS_MEM) + if (res_desc->type == RES_SYS_MEM) entries[num_entries].type = E820_RAM; - else if (hob.res_desc->type == RES_MEM_RESERVED) + else if (res_desc->type == RES_MEM_RESERVED) entries[num_entries].type = E820_RESERVED; } - hob.raw = get_next_hob(hob); + hdr = get_next_hob(hdr); num_entries++; }
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 380b64e..5110361 100644 --- a/arch/x86/include/asm/arch-queensbay/fsp/fsp_hob.h +++ b/arch/x86/include/asm/arch-queensbay/fsp/fsp_hob.h @@ -182,15 +182,6 @@ struct hob_guid { /* GUID specific data goes here */ };
-/* Union of all the possible HOB Types */ -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; -}; - /** * get_hob_type() - return the type of a HOB * @@ -201,9 +192,9 @@ union hob_pointers { * * @return: HOB type. */ -static inline u16 get_hob_type(union hob_pointers hob) +static inline u16 get_hob_type(const struct hob_header *hdr) { - return hob.hdr->type; + return hdr->type; }
/** @@ -216,9 +207,9 @@ static inline u16 get_hob_type(union hob_pointers hob) * * @return: HOB length. */ -static inline u16 get_hob_length(union hob_pointers hob) +static inline u16 get_hob_length(const struct hob_header *hdr) { - return hob.hdr->len; + return hdr->len; }
/** @@ -227,13 +218,13 @@ static inline u16 get_hob_length(union hob_pointers hob) * This macro returns a pointer to HOB that follows the HOB specified by hob * in the HOB List. * - * @hob: A pointer to a HOB. + * @hdr: A pointer to a HOB. * * @return: A pointer to the next HOB in the HOB list. */ -static inline void *get_next_hob(union hob_pointers hob) +static inline const struct hob_header *get_next_hob(const struct hob_header *hdr) { - return (void *)(*(u8 **)&(hob) + get_hob_length(hob)); + return (const struct hob_header *)((u32)hdr + get_hob_length(hdr)); }
/** @@ -243,14 +234,14 @@ static inline void *get_next_hob(union hob_pointers hob) * 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. + * @hdr: 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 hdr is the last HOB in the HOB list. + * @retval false: The HOB specified by hdr is not the last HOB in the HOB list. */ -static inline bool end_of_hob(union hob_pointers hob) +static inline bool end_of_hob(const struct hob_header *hdr) { - return get_hob_type(hob) == HOB_TYPE_EOH; + return get_hob_type(hdr) == HOB_TYPE_EOH; }
/** @@ -260,13 +251,13 @@ static inline bool end_of_hob(union hob_pointers hob) * This macro returns a pointer to the data buffer in a HOB specified by hob. * hob is assumed to be a HOB of type HOB_TYPE_GUID_EXT. * - * @hob: A pointer to a HOB. + * @hdr: A pointer to a HOB. * * @return: A pointer to the data buffer in a HOB. */ -static inline void *get_guid_hob_data(u8 *hob) +static inline void *get_guid_hob_data(const struct hob_header *hdr) { - return (void *)(hob + sizeof(struct hob_guid)); + return (void *)((u32)hdr + sizeof(struct hob_guid)); }
/** @@ -276,14 +267,13 @@ static inline void *get_guid_hob_data(u8 *hob) * This macro returns the size, in bytes, of the data buffer in a HOB * specified by hob. hob is assumed to be a HOB of type HOB_TYPE_GUID_EXT. * - * @hob: A pointer to a HOB. + * @hdr: A pointer to a HOB. * * @return: The size of the data buffer. */ -static inline u16 get_guid_hob_data_size(u8 *hob) +static inline u16 get_guid_hob_data_size(const struct hob_header *hdr) { - union hob_pointers hob_p = *(union hob_pointers *)hob; - return get_hob_length(hob_p) - sizeof(struct hob_guid); + return get_hob_length(hdr) - sizeof(struct hob_guid); }
/* FSP specific GUID HOB definitions */ 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 3ae1b66..2a3e987 100644 --- a/arch/x86/include/asm/arch-queensbay/fsp/fsp_support.h +++ b/arch/x86/include/asm/arch-queensbay/fsp/fsp_support.h @@ -145,7 +145,7 @@ u32 fsp_get_tseg_reserved_mem(const void *hob_list, u32 *len); * * @retval: A HOB object with matching type; Otherwise NULL. */ -void *fsp_get_next_hob(u16 type, const void *hob_list); +const struct hob_header *fsp_get_next_hob(u16 type, const void *hob_list);
/** * Returns the next instance of the matched GUID HOB from the starting HOB. @@ -155,7 +155,8 @@ void *fsp_get_next_hob(u16 type, const void *hob_list); * * @retval: A HOB object with matching GUID; Otherwise NULL. */ -void *fsp_get_next_guid_hob(const struct efi_guid *guid, const void *hob_list); +const struct hob_header *fsp_get_next_guid_hob(const struct efi_guid *guid, + const void *hob_list);
/** * This function retrieves a GUID HOB data buffer and size. diff --git a/arch/x86/lib/cmd_hob.c b/arch/x86/lib/cmd_hob.c index b552fe6..8d1f038 100644 --- a/arch/x86/lib/cmd_hob.c +++ b/arch/x86/lib/cmd_hob.c @@ -28,20 +28,20 @@ static char *hob_type[] = {
int do_hob(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { - union hob_pointers hob; + const struct hob_header *hdr; u16 type; char *desc; int i = 0;
- hob.raw = (u8 *)gd->arch.hob_list; + hdr = gd->arch.hob_list;
- printf("HOB list address: 0x%08x\n\n", (unsigned int)hob.raw); + printf("HOB list address: 0x%08x\n\n", (unsigned int)hdr);
printf("No. | Address | Type | Length in Bytes\n"); printf("----|----------|---------------------|----------------\n"); - while (!end_of_hob(hob)) { - printf("%-3d | %08x | ", i, (unsigned int)hob.raw); - type = get_hob_type(hob); + while (!end_of_hob(hdr)) { + printf("%-3d | %08x | ", i, (unsigned int)hdr); + type = get_hob_type(hdr); if (type == HOB_TYPE_UNUSED) desc = "*Unused*"; else if (type == HOB_TYPE_EOH) @@ -50,8 +50,8 @@ int do_hob(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) desc = hob_type[type]; else desc = "*Invalid Type*"; - printf("%-19s | %-15d\n", desc, get_hob_length(hob)); - hob.raw = get_next_hob(hob); + printf("%-19s | %-15d\n", desc, get_hob_length(hdr)); + hdr = get_next_hob(hdr); i++; }
Hi Bin,
On 30 December 2014 at 01:02, Bin Meng bmeng.cn@gmail.com wrote:
Remove the troublesome union hob_pointers so that some annoying casts are no longer needed in those hob access routines. This also improves the readability.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
arch/x86/cpu/queensbay/fsp_support.c | 95 ++++++++++++---------- arch/x86/cpu/queensbay/tnc_dram.c | 39 +++++---- arch/x86/include/asm/arch-queensbay/fsp/fsp_hob.h | 46 ++++------- .../include/asm/arch-queensbay/fsp/fsp_support.h | 5 +- arch/x86/lib/cmd_hob.c | 16 ++-- 5 files changed, 101 insertions(+), 100 deletions(-)
Yes a big improvement - see a few additional ideas for a follow-on patch below.
Acked-by: Simon Glass sjg@chromium.org
diff --git a/arch/x86/cpu/queensbay/fsp_support.c b/arch/x86/cpu/queensbay/fsp_support.c index ef1916b..4764e3c 100644 --- a/arch/x86/cpu/queensbay/fsp_support.c +++ b/arch/x86/cpu/queensbay/fsp_support.c @@ -231,26 +231,28 @@ u32 fsp_notify(struct fsp_header *fsp_hdr, u32 phase)
u32 fsp_get_usable_lowmem_top(const void *hob_list) {
union hob_pointers hob;
const struct hob_header *hdr;struct hob_res_desc *res_desc; phys_addr_t phys_start; u32 top; /* Get the HOB list for processing */
hob.raw = (void *)hob_list;
hdr = hob_list; /* * Collect memory ranges */ top = FSP_LOWMEM_BASE;
while (!end_of_hob(hob)) {if (get_hob_type(hob) == HOB_TYPE_RES_DESC) {if (hob.res_desc->type == RES_SYS_MEM) {phys_start = hob.res_desc->phys_start;
while (!end_of_hob(hdr)) {if (get_hob_type(hdr) == HOB_TYPE_RES_DESC) {res_desc = (struct hob_res_desc *)hdr;if (res_desc->type == RES_SYS_MEM) {phys_start = res_desc->phys_start; /* Need memory above 1MB to be collected here */ if (phys_start >= FSP_LOWMEM_BASE && phys_start < (phys_addr_t)FSP_HIGHMEM_BASE)
top += (u32)(hob.res_desc->len);
top += (u32)(res_desc->len); } }
hob.raw = get_next_hob(hob);
hdr = get_next_hob(hdr); } return top;@@ -258,25 +260,27 @@ u32 fsp_get_usable_lowmem_top(const void *hob_list)
u64 fsp_get_usable_highmem_top(const void *hob_list) {
union hob_pointers hob;
const struct hob_header *hdr;struct hob_res_desc *res_desc; phys_addr_t phys_start; u64 top; /* Get the HOB list for processing */
hob.raw = (void *)hob_list;
hdr = hob_list; /* Collect memory ranges */ top = FSP_HIGHMEM_BASE;
while (!end_of_hob(hob)) {if (get_hob_type(hob) == HOB_TYPE_RES_DESC) {if (hob.res_desc->type == RES_SYS_MEM) {phys_start = hob.res_desc->phys_start;
while (!end_of_hob(hdr)) {if (get_hob_type(hdr) == HOB_TYPE_RES_DESC) {res_desc = (struct hob_res_desc *)hdr;if (res_desc->type == RES_SYS_MEM) {phys_start = res_desc->phys_start; /* Need memory above 1MB to be collected here */ if (phys_start >= (phys_addr_t)FSP_HIGHMEM_BASE)
top += (u32)(hob.res_desc->len);
top += (u32)(res_desc->len); } }
hob.raw = get_next_hob(hob);
hdr = get_next_hob(hdr); } return top;@@ -285,24 +289,26 @@ u64 fsp_get_usable_highmem_top(const void *hob_list) u64 fsp_get_reserved_mem_from_guid(const void *hob_list, u64 *len, struct efi_guid *guid) {
union hob_pointers hob;
const struct hob_header *hdr;struct hob_res_desc *res_desc; /* Get the HOB list for processing */
hob.raw = (void *)hob_list;
hdr = hob_list; /* Collect memory ranges */
while (!end_of_hob(hob)) {if (get_hob_type(hob) == HOB_TYPE_RES_DESC) {if (hob.res_desc->type == RES_MEM_RESERVED) {if (compare_guid(&hob.res_desc->owner, guid)) {
while (!end_of_hob(hdr)) {if (get_hob_type(hdr) == HOB_TYPE_RES_DESC) {res_desc = (struct hob_res_desc *)hdr;if (res_desc->type == RES_MEM_RESERVED) {if (compare_guid(&res_desc->owner, guid)) { if (len)
*len = (u32)(hob.res_desc->len);
*len = (u32)(res_desc->len);
return (u64)(hob.res_desc->phys_start);
return (u64)(res_desc->phys_start); } } }
hob.raw = get_next_hob(hob);
hdr = get_next_hob(hdr); } return 0;@@ -336,44 +342,45 @@ u32 fsp_get_tseg_reserved_mem(const void *hob_list, u32 *len) return base; }
-void *fsp_get_next_hob(u16 type, const void *hob_list) +const struct hob_header *fsp_get_next_hob(u16 type, const void *hob_list)
I'd suggest just using uint instead of u16 in this sort of situation. To me the u16 doesn't add anything to a parameter context, and it often bloats the code due to the masking that each function needs to implement.
{
union hob_pointers hob;
const struct hob_header *hdr;
assert(hob_list != NULL);hob.raw = (u8 *)hob_list;
hdr = hob_list; /* Parse the HOB list until end of list or matching type is found */
while (!end_of_hob(hob)) {if (get_hob_type(hob) == type)return hob.raw;
while (!end_of_hob(hdr)) {if (get_hob_type(hdr) == type)return hdr;
hob.raw = get_next_hob(hob);
hdr = get_next_hob(hdr); } return NULL;}
-void *fsp_get_next_guid_hob(const struct efi_guid *guid, const void *hob_list) +const struct hob_header *fsp_get_next_guid_hob(const struct efi_guid *guid,
const void *hob_list){
union hob_pointers hob;hob.raw = (u8 *)hob_list;while ((hob.raw = fsp_get_next_hob(HOB_TYPE_GUID_EXT,hob.raw)) != NULL) {if (compare_guid(guid, &hob.guid->name))
const struct hob_header *hdr;struct hob_guid *guid_hob;hdr = hob_list;while ((hdr = fsp_get_next_hob(HOB_TYPE_GUID_EXT,hdr)) != NULL) {guid_hob = (struct hob_guid *)hdr;if (compare_guid(guid, &(guid_hob->name))) break;
hob.raw = get_next_hob(hob);
hdr = get_next_hob(hdr); }
return hob.raw;
return hdr;}
void *fsp_get_guid_hob_data(const void *hob_list, u32 *len, struct efi_guid *guid) {
u8 *guid_hob;
const struct hob_header *guid_hob; guid_hob = fsp_get_next_guid_hob(guid, hob_list); if (guid_hob == NULL) {diff --git a/arch/x86/cpu/queensbay/tnc_dram.c b/arch/x86/cpu/queensbay/tnc_dram.c index 8e97c9b..b669dbc 100644 --- a/arch/x86/cpu/queensbay/tnc_dram.c +++ b/arch/x86/cpu/queensbay/tnc_dram.c @@ -14,17 +14,19 @@ DECLARE_GLOBAL_DATA_PTR; int dram_init(void) { phys_size_t ram_size = 0;
union hob_pointers hob;
const struct hob_header *hdr;struct hob_res_desc *res_desc;
hob.raw = gd->arch.hob_list;while (!end_of_hob(hob)) {if (get_hob_type(hob) == HOB_TYPE_RES_DESC) {if (hob.res_desc->type == RES_SYS_MEM ||hob.res_desc->type == RES_MEM_RESERVED) {ram_size += hob.res_desc->len;
hdr = gd->arch.hob_list;while (!end_of_hob(hdr)) {if (get_hob_type(hdr) == HOB_TYPE_RES_DESC) {res_desc = (struct hob_res_desc *)hdr;if (res_desc->type == RES_SYS_MEM ||res_desc->type == RES_MEM_RESERVED) {ram_size += res_desc->len; } }
hob.raw = get_next_hob(hob);
hdr = get_next_hob(hdr); } gd->ram_size = ram_size;@@ -55,22 +57,23 @@ ulong board_get_usable_ram_top(ulong total_size) unsigned install_e820_map(unsigned max_entries, struct e820entry *entries) { unsigned num_entries = 0;
const struct hob_header *hdr;struct hob_res_desc *res_desc;
union hob_pointers hob;
hdr = gd->arch.hob_list;
hob.raw = gd->arch.hob_list;
while (!end_of_hob(hdr)) {if (get_hob_type(hdr) == HOB_TYPE_RES_DESC) {res_desc = (struct hob_res_desc *)hdr;entries[num_entries].addr = res_desc->phys_start;entries[num_entries].size = res_desc->len;
while (!end_of_hob(hob)) {if (get_hob_type(hob) == HOB_TYPE_RES_DESC) {entries[num_entries].addr = hob.res_desc->phys_start;entries[num_entries].size = hob.res_desc->len;if (hob.res_desc->type == RES_SYS_MEM)
if (res_desc->type == RES_SYS_MEM) entries[num_entries].type = E820_RAM;
else if (hob.res_desc->type == RES_MEM_RESERVED)
else if (res_desc->type == RES_MEM_RESERVED) entries[num_entries].type = E820_RESERVED; }
hob.raw = get_next_hob(hob);
hdr = get_next_hob(hdr); num_entries++; }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 380b64e..5110361 100644 --- a/arch/x86/include/asm/arch-queensbay/fsp/fsp_hob.h +++ b/arch/x86/include/asm/arch-queensbay/fsp/fsp_hob.h @@ -182,15 +182,6 @@ struct hob_guid { /* GUID specific data goes here */ };
-/* Union of all the possible HOB Types */ -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;-};
/**
- get_hob_type() - return the type of a HOB
@@ -201,9 +192,9 @@ union hob_pointers {
- @return: HOB type.
*/ -static inline u16 get_hob_type(union hob_pointers hob) +static inline u16 get_hob_type(const struct hob_header *hdr) {
return hob.hdr->type;
return hdr->type;}
Drop this function?
/** @@ -216,9 +207,9 @@ static inline u16 get_hob_type(union hob_pointers hob)
- @return: HOB length.
*/ -static inline u16 get_hob_length(union hob_pointers hob) +static inline u16 get_hob_length(const struct hob_header *hdr) {
return hob.hdr->len;
return hdr->len;}
Drop this function?
/** @@ -227,13 +218,13 @@ static inline u16 get_hob_length(union hob_pointers hob)
- This macro returns a pointer to HOB that follows the HOB specified by hob
- in the HOB List.
- @hob: A pointer to a HOB.
*/
- @hdr: A pointer to a HOB.
- @return: A pointer to the next HOB in the HOB list.
-static inline void *get_next_hob(union hob_pointers hob) +static inline const struct hob_header *get_next_hob(const struct hob_header *hdr) {
return (void *)(*(u8 **)&(hob) + get_hob_length(hob));
return (const struct hob_header *)((u32)hdr + get_hob_length(hdr));}
/** @@ -243,14 +234,14 @@ static inline void *get_next_hob(union hob_pointers hob)
- 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.
- @hdr: 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 hdr is the last HOB in the HOB list.
*/
- @retval false: The HOB specified by hdr is not the last HOB in the HOB list.
-static inline bool end_of_hob(union hob_pointers hob) +static inline bool end_of_hob(const struct hob_header *hdr) {
return get_hob_type(hob) == HOB_TYPE_EOH;
return get_hob_type(hdr) == HOB_TYPE_EOH;}
I suppose this function is useful.
/** @@ -260,13 +251,13 @@ static inline bool end_of_hob(union hob_pointers hob)
- This macro returns a pointer to the data buffer in a HOB specified by hob.
- hob is assumed to be a HOB of type HOB_TYPE_GUID_EXT.
- @hob: A pointer to a HOB.
*/
- @hdr: A pointer to a HOB.
- @return: A pointer to the data buffer in a HOB.
-static inline void *get_guid_hob_data(u8 *hob) +static inline void *get_guid_hob_data(const struct hob_header *hdr) {
return (void *)(hob + sizeof(struct hob_guid));
return (void *)((u32)hdr + sizeof(struct hob_guid));}
/** @@ -276,14 +267,13 @@ static inline void *get_guid_hob_data(u8 *hob)
- This macro returns the size, in bytes, of the data buffer in a HOB
- specified by hob. hob is assumed to be a HOB of type HOB_TYPE_GUID_EXT.
- @hob: A pointer to a HOB.
*/
- @hdr: A pointer to a HOB.
- @return: The size of the data buffer.
-static inline u16 get_guid_hob_data_size(u8 *hob) +static inline u16 get_guid_hob_data_size(const struct hob_header *hdr) {
union hob_pointers hob_p = *(union hob_pointers *)hob;return get_hob_length(hob_p) - sizeof(struct hob_guid);
return get_hob_length(hdr) - sizeof(struct hob_guid);}
/* FSP specific GUID HOB definitions */ 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 3ae1b66..2a3e987 100644 --- a/arch/x86/include/asm/arch-queensbay/fsp/fsp_support.h +++ b/arch/x86/include/asm/arch-queensbay/fsp/fsp_support.h @@ -145,7 +145,7 @@ u32 fsp_get_tseg_reserved_mem(const void *hob_list, u32 *len);
- @retval: A HOB object with matching type; Otherwise NULL.
*/ -void *fsp_get_next_hob(u16 type, const void *hob_list); +const struct hob_header *fsp_get_next_hob(u16 type, const void *hob_list);
/**
- Returns the next instance of the matched GUID HOB from the starting HOB.
@@ -155,7 +155,8 @@ void *fsp_get_next_hob(u16 type, const void *hob_list);
- @retval: A HOB object with matching GUID; Otherwise NULL.
*/ -void *fsp_get_next_guid_hob(const struct efi_guid *guid, const void *hob_list); +const struct hob_header *fsp_get_next_guid_hob(const struct efi_guid *guid,
const void *hob_list);/**
- This function retrieves a GUID HOB data buffer and size.
diff --git a/arch/x86/lib/cmd_hob.c b/arch/x86/lib/cmd_hob.c index b552fe6..8d1f038 100644 --- a/arch/x86/lib/cmd_hob.c +++ b/arch/x86/lib/cmd_hob.c @@ -28,20 +28,20 @@ static char *hob_type[] = {
int do_hob(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) {
union hob_pointers hob;
const struct hob_header *hdr; u16 type; char *desc; int i = 0;
hob.raw = (u8 *)gd->arch.hob_list;
hdr = gd->arch.hob_list;
printf("HOB list address: 0x%08x\n\n", (unsigned int)hob.raw);
printf("HOB list address: 0x%08x\n\n", (unsigned int)hdr); printf("No. | Address | Type | Length in Bytes\n"); printf("----|----------|---------------------|----------------\n");
while (!end_of_hob(hob)) {printf("%-3d | %08x | ", i, (unsigned int)hob.raw);type = get_hob_type(hob);
while (!end_of_hob(hdr)) {printf("%-3d | %08x | ", i, (unsigned int)hdr);type = get_hob_type(hdr); if (type == HOB_TYPE_UNUSED) desc = "*Unused*"; else if (type == HOB_TYPE_EOH)@@ -50,8 +50,8 @@ int do_hob(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) desc = hob_type[type]; else desc = "*Invalid Type*";
printf("%-19s | %-15d\n", desc, get_hob_length(hob));hob.raw = get_next_hob(hob);
printf("%-19s | %-15d\n", desc, get_hob_length(hdr));hdr = get_next_hob(hdr); i++; }-- 1.8.2.1
Regards, Simon
Hi Simon,
On Wed, Dec 31, 2014 at 7:02 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 30 December 2014 at 01:02, Bin Meng bmeng.cn@gmail.com wrote:
Remove the troublesome union hob_pointers so that some annoying casts are no longer needed in those hob access routines. This also improves the readability.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
arch/x86/cpu/queensbay/fsp_support.c | 95 ++++++++++++---------- arch/x86/cpu/queensbay/tnc_dram.c | 39 +++++---- arch/x86/include/asm/arch-queensbay/fsp/fsp_hob.h | 46 ++++------- .../include/asm/arch-queensbay/fsp/fsp_support.h | 5 +- arch/x86/lib/cmd_hob.c | 16 ++-- 5 files changed, 101 insertions(+), 100 deletions(-)
Yes a big improvement - see a few additional ideas for a follow-on patch below.
Acked-by: Simon Glass sjg@chromium.org
diff --git a/arch/x86/cpu/queensbay/fsp_support.c b/arch/x86/cpu/queensbay/fsp_support.c index ef1916b..4764e3c 100644 --- a/arch/x86/cpu/queensbay/fsp_support.c +++ b/arch/x86/cpu/queensbay/fsp_support.c @@ -231,26 +231,28 @@ u32 fsp_notify(struct fsp_header *fsp_hdr, u32 phase)
u32 fsp_get_usable_lowmem_top(const void *hob_list) {
union hob_pointers hob;
const struct hob_header *hdr;struct hob_res_desc *res_desc; phys_addr_t phys_start; u32 top; /* Get the HOB list for processing */
hob.raw = (void *)hob_list;
hdr = hob_list; /* * Collect memory ranges */ top = FSP_LOWMEM_BASE;
while (!end_of_hob(hob)) {if (get_hob_type(hob) == HOB_TYPE_RES_DESC) {if (hob.res_desc->type == RES_SYS_MEM) {phys_start = hob.res_desc->phys_start;
while (!end_of_hob(hdr)) {if (get_hob_type(hdr) == HOB_TYPE_RES_DESC) {res_desc = (struct hob_res_desc *)hdr;if (res_desc->type == RES_SYS_MEM) {phys_start = res_desc->phys_start; /* Need memory above 1MB to be collected here */ if (phys_start >= FSP_LOWMEM_BASE && phys_start < (phys_addr_t)FSP_HIGHMEM_BASE)
top += (u32)(hob.res_desc->len);
top += (u32)(res_desc->len); } }
hob.raw = get_next_hob(hob);
hdr = get_next_hob(hdr); } return top;@@ -258,25 +260,27 @@ u32 fsp_get_usable_lowmem_top(const void *hob_list)
u64 fsp_get_usable_highmem_top(const void *hob_list) {
union hob_pointers hob;
const struct hob_header *hdr;struct hob_res_desc *res_desc; phys_addr_t phys_start; u64 top; /* Get the HOB list for processing */
hob.raw = (void *)hob_list;
hdr = hob_list; /* Collect memory ranges */ top = FSP_HIGHMEM_BASE;
while (!end_of_hob(hob)) {if (get_hob_type(hob) == HOB_TYPE_RES_DESC) {if (hob.res_desc->type == RES_SYS_MEM) {phys_start = hob.res_desc->phys_start;
while (!end_of_hob(hdr)) {if (get_hob_type(hdr) == HOB_TYPE_RES_DESC) {res_desc = (struct hob_res_desc *)hdr;if (res_desc->type == RES_SYS_MEM) {phys_start = res_desc->phys_start; /* Need memory above 1MB to be collected here */ if (phys_start >= (phys_addr_t)FSP_HIGHMEM_BASE)
top += (u32)(hob.res_desc->len);
top += (u32)(res_desc->len); } }
hob.raw = get_next_hob(hob);
hdr = get_next_hob(hdr); } return top;@@ -285,24 +289,26 @@ u64 fsp_get_usable_highmem_top(const void *hob_list) u64 fsp_get_reserved_mem_from_guid(const void *hob_list, u64 *len, struct efi_guid *guid) {
union hob_pointers hob;
const struct hob_header *hdr;struct hob_res_desc *res_desc; /* Get the HOB list for processing */
hob.raw = (void *)hob_list;
hdr = hob_list; /* Collect memory ranges */
while (!end_of_hob(hob)) {if (get_hob_type(hob) == HOB_TYPE_RES_DESC) {if (hob.res_desc->type == RES_MEM_RESERVED) {if (compare_guid(&hob.res_desc->owner, guid)) {
while (!end_of_hob(hdr)) {if (get_hob_type(hdr) == HOB_TYPE_RES_DESC) {res_desc = (struct hob_res_desc *)hdr;if (res_desc->type == RES_MEM_RESERVED) {if (compare_guid(&res_desc->owner, guid)) { if (len)
*len = (u32)(hob.res_desc->len);
*len = (u32)(res_desc->len);
return (u64)(hob.res_desc->phys_start);
return (u64)(res_desc->phys_start); } } }
hob.raw = get_next_hob(hob);
hdr = get_next_hob(hdr); } return 0;@@ -336,44 +342,45 @@ u32 fsp_get_tseg_reserved_mem(const void *hob_list, u32 *len) return base; }
-void *fsp_get_next_hob(u16 type, const void *hob_list) +const struct hob_header *fsp_get_next_hob(u16 type, const void *hob_list)
I'd suggest just using uint instead of u16 in this sort of situation. To me the u16 doesn't add anything to a parameter context, and it often bloats the code due to the masking that each function needs to implement.
Yes, will do in a follow-on patch.
{
union hob_pointers hob;
const struct hob_header *hdr;
assert(hob_list != NULL);hob.raw = (u8 *)hob_list;
hdr = hob_list; /* Parse the HOB list until end of list or matching type is found */
while (!end_of_hob(hob)) {if (get_hob_type(hob) == type)return hob.raw;
while (!end_of_hob(hdr)) {if (get_hob_type(hdr) == type)return hdr;
hob.raw = get_next_hob(hob);
hdr = get_next_hob(hdr); } return NULL;}
-void *fsp_get_next_guid_hob(const struct efi_guid *guid, const void *hob_list) +const struct hob_header *fsp_get_next_guid_hob(const struct efi_guid *guid,
const void *hob_list){
union hob_pointers hob;hob.raw = (u8 *)hob_list;while ((hob.raw = fsp_get_next_hob(HOB_TYPE_GUID_EXT,hob.raw)) != NULL) {if (compare_guid(guid, &hob.guid->name))
const struct hob_header *hdr;struct hob_guid *guid_hob;hdr = hob_list;while ((hdr = fsp_get_next_hob(HOB_TYPE_GUID_EXT,hdr)) != NULL) {guid_hob = (struct hob_guid *)hdr;if (compare_guid(guid, &(guid_hob->name))) break;
hob.raw = get_next_hob(hob);
hdr = get_next_hob(hdr); }
return hob.raw;
return hdr;}
void *fsp_get_guid_hob_data(const void *hob_list, u32 *len, struct efi_guid *guid) {
u8 *guid_hob;
const struct hob_header *guid_hob; guid_hob = fsp_get_next_guid_hob(guid, hob_list); if (guid_hob == NULL) {diff --git a/arch/x86/cpu/queensbay/tnc_dram.c b/arch/x86/cpu/queensbay/tnc_dram.c index 8e97c9b..b669dbc 100644 --- a/arch/x86/cpu/queensbay/tnc_dram.c +++ b/arch/x86/cpu/queensbay/tnc_dram.c @@ -14,17 +14,19 @@ DECLARE_GLOBAL_DATA_PTR; int dram_init(void) { phys_size_t ram_size = 0;
union hob_pointers hob;
const struct hob_header *hdr;struct hob_res_desc *res_desc;
hob.raw = gd->arch.hob_list;while (!end_of_hob(hob)) {if (get_hob_type(hob) == HOB_TYPE_RES_DESC) {if (hob.res_desc->type == RES_SYS_MEM ||hob.res_desc->type == RES_MEM_RESERVED) {ram_size += hob.res_desc->len;
hdr = gd->arch.hob_list;while (!end_of_hob(hdr)) {if (get_hob_type(hdr) == HOB_TYPE_RES_DESC) {res_desc = (struct hob_res_desc *)hdr;if (res_desc->type == RES_SYS_MEM ||res_desc->type == RES_MEM_RESERVED) {ram_size += res_desc->len; } }
hob.raw = get_next_hob(hob);
hdr = get_next_hob(hdr); } gd->ram_size = ram_size;@@ -55,22 +57,23 @@ ulong board_get_usable_ram_top(ulong total_size) unsigned install_e820_map(unsigned max_entries, struct e820entry *entries) { unsigned num_entries = 0;
const struct hob_header *hdr;struct hob_res_desc *res_desc;
union hob_pointers hob;
hdr = gd->arch.hob_list;
hob.raw = gd->arch.hob_list;
while (!end_of_hob(hdr)) {if (get_hob_type(hdr) == HOB_TYPE_RES_DESC) {res_desc = (struct hob_res_desc *)hdr;entries[num_entries].addr = res_desc->phys_start;entries[num_entries].size = res_desc->len;
while (!end_of_hob(hob)) {if (get_hob_type(hob) == HOB_TYPE_RES_DESC) {entries[num_entries].addr = hob.res_desc->phys_start;entries[num_entries].size = hob.res_desc->len;if (hob.res_desc->type == RES_SYS_MEM)
if (res_desc->type == RES_SYS_MEM) entries[num_entries].type = E820_RAM;
else if (hob.res_desc->type == RES_MEM_RESERVED)
else if (res_desc->type == RES_MEM_RESERVED) entries[num_entries].type = E820_RESERVED; }
hob.raw = get_next_hob(hob);
hdr = get_next_hob(hdr); num_entries++; }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 380b64e..5110361 100644 --- a/arch/x86/include/asm/arch-queensbay/fsp/fsp_hob.h +++ b/arch/x86/include/asm/arch-queensbay/fsp/fsp_hob.h @@ -182,15 +182,6 @@ struct hob_guid { /* GUID specific data goes here */ };
-/* Union of all the possible HOB Types */ -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;-};
/**
- get_hob_type() - return the type of a HOB
@@ -201,9 +192,9 @@ union hob_pointers {
- @return: HOB type.
*/ -static inline u16 get_hob_type(union hob_pointers hob) +static inline u16 get_hob_type(const struct hob_header *hdr) {
return hob.hdr->type;
return hdr->type;}
Drop this function?
Agreed.
/** @@ -216,9 +207,9 @@ static inline u16 get_hob_type(union hob_pointers hob)
- @return: HOB length.
*/ -static inline u16 get_hob_length(union hob_pointers hob) +static inline u16 get_hob_length(const struct hob_header *hdr) {
return hob.hdr->len;
return hdr->len;}
Drop this function?
Agreed.
[snip]
Regards, Bin
On 31 December 2014 at 01:22, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Wed, Dec 31, 2014 at 7:02 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 30 December 2014 at 01:02, Bin Meng bmeng.cn@gmail.com wrote:
Remove the troublesome union hob_pointers so that some annoying casts are no longer needed in those hob access routines. This also improves the readability.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
arch/x86/cpu/queensbay/fsp_support.c | 95 ++++++++++++---------- arch/x86/cpu/queensbay/tnc_dram.c | 39 +++++---- arch/x86/include/asm/arch-queensbay/fsp/fsp_hob.h | 46 ++++------- .../include/asm/arch-queensbay/fsp/fsp_support.h | 5 +- arch/x86/lib/cmd_hob.c | 16 ++-- 5 files changed, 101 insertions(+), 100 deletions(-)
Yes a big improvement - see a few additional ideas for a follow-on patch below.
Acked-by: Simon Glass sjg@chromium.org
Applied to u-boot-x86/next, thanks!
participants (2)
-
Bin Meng -
Simon Glass