Attention is currently required from: Zheng Bao. Hello Zheng Bao,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/59175
to review the following change.
Change subject: amdfwtool: Pack the PSP/BHD directory filling to functions ......................................................................
amdfwtool: Pack the PSP/BHD directory filling to functions
This two steps would be more complicated to handle more types of APU. Extract them to functions and then we can treat independently. More future cases can easily to be added.
Change-Id: I2625c8858605c990da2225a16036b80ae31032f6 Signed-off-by: Zheng Bao fishbaozi@gmail.com --- M util/amdfwtool/amdfwtool.c M util/amdfwtool/amdfwtool.h 2 files changed, 45 insertions(+), 31 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/59175/1
diff --git a/util/amdfwtool/amdfwtool.c b/util/amdfwtool/amdfwtool.c index 049755b..f054d7e 100644 --- a/util/amdfwtool/amdfwtool.c +++ b/util/amdfwtool/amdfwtool.c @@ -455,6 +455,37 @@
}
+static void fill_psp_dirctory(embedded_firmware *amd_romsig, void *pspdir, + context *ctx, uint8_t comboable, uint8_t soc_id) +{ + /* TODO: Change the flag comboable to specifi SOC ID. */ + (void) (&soc_id); + if (comboable) + amd_romsig->combo_psp_directory = BUFF_TO_RUN(*ctx, pspdir); + else + amd_romsig->psp_directory = BUFF_TO_RUN(*ctx, pspdir); +} + +static void fill_bios_dirctory(embedded_firmware *amd_romsig, void *biosdir, + context *ctx, uint8_t soc_id) +{ + switch (soc_id) { + case PLATFORM_RENOIR: + case PLATFORM_LUCIENNE: + case PLATFORM_CEZANNE: + amd_romsig->bios3_entry = BUFF_TO_RUN(*ctx, biosdir); + break; + case PLATFORM_MENDOCINO: + break; + case PLATFORM_STONEYRIDGE: + case PLATFORM_RAVEN: + case PLATFORM_PICASSO: + default: + amd_romsig->bios1_entry = BUFF_TO_RUN(*ctx, biosdir); + break; + } +} + static ssize_t copy_blob(void *dest, const char *src_file, size_t room) { int fd; @@ -490,17 +521,6 @@ return bytes; }
-enum platform { - PLATFORM_UNKNOWN, - PLATFORM_STONEYRIDGE, - PLATFORM_RAVEN, - PLATFORM_PICASSO, - PLATFORM_RENOIR, - PLATFORM_CEZANNE, - PLATFORM_MENDOCINO, - PLATFORM_LUCIENNE, -}; - static uint32_t get_psp_id(enum platform soc_id) { uint32_t psp_id; @@ -1613,14 +1633,11 @@ amd_psp_fw_table, PSP_COOKIE, &cb_config); }
- if (comboable) - amd_romsig->combo_psp_directory = BUFF_TO_RUN(ctx, pspdir); - else - amd_romsig->psp_directory = BUFF_TO_RUN(ctx, pspdir); + fill_psp_dirctory(amd_romsig, pspdir, &ctx, comboable, soc_id);
#if PSP_COMBO psp_combo_directory *combo_dir = new_combo_dir(&ctx); - amd_romsig->combo_psp_directory = BUFF_TO_RUN(ctx, combo_dir); + fill_psp_dirctory(amd_romsig, combo_dir, &ctx, comboable, soc_id); /* 0 -Compare PSP ID, 1 -Compare chip family ID */ combo_dir->entries[0].id_sel = 0; combo_dir->entries[0].id = get_psp_id(soc_id); @@ -1648,21 +1665,7 @@ integrate_bios_firmwares(&ctx, biosdir, NULL, amd_bios_table, BDT1_COOKIE, &cb_config); } - switch (soc_id) { - case PLATFORM_RENOIR: - case PLATFORM_LUCIENNE: - case PLATFORM_CEZANNE: - amd_romsig->bios3_entry = BUFF_TO_RUN(ctx, biosdir); - break; - case PLATFORM_MENDOCINO: - break; - case PLATFORM_STONEYRIDGE: - case PLATFORM_RAVEN: - case PLATFORM_PICASSO: - default: - amd_romsig->bios1_entry = BUFF_TO_RUN(ctx, biosdir); - break; - } + fill_bios_dirctory(amd_romsig, biosdir, &ctx, soc_id); }
/* Free the filename. */ diff --git a/util/amdfwtool/amdfwtool.h b/util/amdfwtool/amdfwtool.h index a7ac7c1..d184f20 100644 --- a/util/amdfwtool/amdfwtool.h +++ b/util/amdfwtool/amdfwtool.h @@ -75,6 +75,17 @@ AMD_BIOS_SKIP } amd_bios_type;
+enum platform { + PLATFORM_UNKNOWN, + PLATFORM_STONEYRIDGE, + PLATFORM_RAVEN, + PLATFORM_PICASSO, + PLATFORM_RENOIR, + PLATFORM_CEZANNE, + PLATFORM_MENDOCINO, + PLATFORM_LUCIENNE, +}; + struct second_gen_efs { /* todo: expand for Server products */ int gen:1; /* Client products only use bit 0 */ int reserved:31;