Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31513
Change subject: AGESA: Move debug helper to eventlog file ......................................................................
AGESA: Move debug helper to eventlog file
Change-Id: I2d74f934936e250886526b9c8482f500628a1158 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/drivers/amd/agesa/eventlog.c M src/drivers/amd/agesa/state_machine.c M src/northbridge/amd/agesa/state_machine.h 3 files changed, 40 insertions(+), 39 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/31513/1
diff --git a/src/drivers/amd/agesa/eventlog.c b/src/drivers/amd/agesa/eventlog.c index b80c220..303af53 100644 --- a/src/drivers/amd/agesa/eventlog.c +++ b/src/drivers/amd/agesa/eventlog.c @@ -1,6 +1,8 @@ /* * This file is part of the coreboot project. * + * Copyright (C) 2016 Kyösti Mälkki + * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by * the Free Software Foundation; version 2 of the License. @@ -18,6 +20,7 @@ #include <northbridge/amd/agesa/state_machine.h> #include <northbridge/amd/agesa/BiosCallOuts.h> #include <amdlib.h> +#include <debug_util.h> #include <AGESA.h> #include <AMD.h>
@@ -40,7 +43,7 @@ * under vendorcode/ tree. */
-const char *agesa_struct_name(int state) +static const char *agesa_struct_name(AGESA_STRUCT_NAME state) { #if IS_ENABLED(CONFIG_CPU_AMD_AGESA_OPENSOURCE) if ((state < AMD_INIT_RECOVERY) || (state > AMD_IDENTIFY_DIMMS)) @@ -57,6 +60,27 @@ return AgesaFunctionNameStr[index]; }
+void agesa_state_on_entry(struct agesa_state *task, AGESA_STRUCT_NAME func) +{ + task->apic_id = (u8) (cpuid_ebx(1) >> 24); + task->func = func; + task->function_name = agesa_struct_name(func); + + printk(BIOS_DEBUG, "\nAPIC %02d: ** Enter %s [%08x]\n", + task->apic_id, task->function_name, task->func); +} + +void agesa_state_on_exit(struct agesa_state *task, + AMD_CONFIG_PARAMS *StdHeader) +{ + printk(BIOS_DEBUG, "APIC %02d: Heap in %s (%d) at 0x%08x\n", + task->apic_id, heap_status_name(StdHeader->HeapStatus), + StdHeader->HeapStatus, (u32)StdHeader->HeapBasePtr); + + printk(BIOS_DEBUG, "APIC %02d: ** Exit %s [%08x]\n", + task->apic_id, task->function_name, task->func); +} + /* * Possible AGESA_STATUS values: * diff --git a/src/drivers/amd/agesa/state_machine.c b/src/drivers/amd/agesa/state_machine.c index 672bca1..f676192 100644 --- a/src/drivers/amd/agesa/state_machine.c +++ b/src/drivers/amd/agesa/state_machine.c @@ -27,7 +27,6 @@ #include <northbridge/amd/agesa/BiosCallOuts.h> #include <amdlib.h>
-#include <debug_util.h> #include <AMD.h>
#if IS_ENABLED(CONFIG_CPU_AMD_AGESA_OPENSOURCE) @@ -232,38 +231,6 @@ return status; }
-/* DEBUG trace helper */ - -struct agesa_state -{ - u8 apic_id; - - AGESA_STRUCT_NAME func; - const char *function_name; -}; - -static void state_on_entry(struct agesa_state *task, AGESA_STRUCT_NAME func, - const char *struct_name) -{ - task->apic_id = (u8) (cpuid_ebx(1) >> 24); - task->func = func; - task->function_name = struct_name; - - printk(BIOS_DEBUG, "\nAPIC %02d: ** Enter %s [%08x]\n", - task->apic_id, task->function_name, task->func); -} - -static void state_on_exit(struct agesa_state *task, - AMD_CONFIG_PARAMS *StdHeader) -{ - printk(BIOS_DEBUG, "APIC %02d: Heap in %s (%d) at 0x%08x\n", - task->apic_id, heap_status_name(StdHeader->HeapStatus), - StdHeader->HeapStatus, (u32)StdHeader->HeapBasePtr); - - printk(BIOS_DEBUG, "APIC %02d: ** Exit %s [%08x]\n", - task->apic_id, task->function_name, task->func); -} - int agesa_execute_state(struct sysinfo *cb, AGESA_STRUCT_NAME func) { AMD_INTERFACE_PARAMS aip; @@ -273,13 +240,12 @@ } agesa_params; void *buf = NULL; size_t len = 0; - const char *state_name = agesa_struct_name(func);
AGESA_STATUS status, final;
struct agesa_state task; memset(&task, 0, sizeof(task)); - state_on_entry(&task, func, state_name); + agesa_state_on_entry(&task, func);
aip.StdHeader = cb->StdHeader;
@@ -303,13 +269,13 @@ if (ENV_RAMSTAGE) final = ramstage_dispatch(cb, func, StdHeader);
- agesawrapper_trace(final, StdHeader, state_name); + agesawrapper_trace(final, StdHeader, task.function_name); ASSERT(final < AGESA_FATAL);
status = amd_release_struct(&aip); ASSERT(status == AGESA_SUCCESS);
- state_on_exit(&task, &aip.StdHeader); + agesa_state_on_exit(&task, &aip.StdHeader);
return (final < AGESA_FATAL) ? 0 : -1; } diff --git a/src/northbridge/amd/agesa/state_machine.h b/src/northbridge/amd/agesa/state_machine.h index c7d654d..45d57c2 100644 --- a/src/northbridge/amd/agesa/state_machine.h +++ b/src/northbridge/amd/agesa/state_machine.h @@ -23,7 +23,6 @@ #define HAS_LEGACY_WRAPPER IS_ENABLED(CONFIG_BINARYPI_LEGACY_WRAPPER)
/* eventlog */ -const char *agesa_struct_name(int state); void agesawrapper_trace(AGESA_STATUS ret, AMD_CONFIG_PARAMS *StdHeader, const char *func); AGESA_STATUS agesawrapper_amdreadeventlog(UINT8 HeapStatus);
@@ -54,6 +53,18 @@ void platform_once(struct sysinfo *cb);
void agesa_set_interface(struct sysinfo *cb); + +struct agesa_state +{ + u8 apic_id; + + AGESA_STRUCT_NAME func; + const char *function_name; +}; + +void agesa_state_on_entry(struct agesa_state *task, AGESA_STRUCT_NAME func); +void agesa_state_on_exit(struct agesa_state *task, + AMD_CONFIG_PARAMS *StdHeader); int agesa_execute_state(struct sysinfo *cb, AGESA_STRUCT_NAME func);
/* AGESA dispatchers */
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31513
to look at the new patch set (#2).
Change subject: AGESA: Move debug helper to eventlog file ......................................................................
AGESA: Move debug helper to eventlog file
Change-Id: I2d74f934936e250886526b9c8482f500628a1158 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/drivers/amd/agesa/eventlog.c M src/drivers/amd/agesa/state_machine.c M src/northbridge/amd/agesa/state_machine.h 3 files changed, 39 insertions(+), 39 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/31513/2
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31513 )
Change subject: AGESA: Move debug helper to eventlog file ......................................................................
Patch Set 4: Code-Review+2
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31513 )
Change subject: AGESA: Move debug helper to eventlog file ......................................................................
AGESA: Move debug helper to eventlog file
Change-Id: I2d74f934936e250886526b9c8482f500628a1158 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/31513 Reviewed-by: Marshall Dawson marshalldawson3rd@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/drivers/amd/agesa/eventlog.c M src/drivers/amd/agesa/state_machine.c M src/northbridge/amd/agesa/state_machine.h 3 files changed, 39 insertions(+), 39 deletions(-)
Approvals: build bot (Jenkins): Verified Marshall Dawson: Looks good to me, approved
diff --git a/src/drivers/amd/agesa/eventlog.c b/src/drivers/amd/agesa/eventlog.c index df3759c..03cb64a 100644 --- a/src/drivers/amd/agesa/eventlog.c +++ b/src/drivers/amd/agesa/eventlog.c @@ -1,6 +1,8 @@ /* * This file is part of the coreboot project. * + * Copyright (C) 2016 Kyösti Mälkki + * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by * the Free Software Foundation; version 2 of the License. @@ -18,6 +20,7 @@ #include <northbridge/amd/agesa/state_machine.h> #include <northbridge/amd/agesa/BiosCallOuts.h> #include <amdlib.h> +#include <debug_util.h> #include <AGESA.h> #include <AMD.h>
@@ -40,7 +43,7 @@ * under vendorcode/ tree. */
-const char *agesa_struct_name(int state) +static const char *agesa_struct_name(AGESA_STRUCT_NAME state) { #if CONFIG(CPU_AMD_AGESA_OPENSOURCE) if ((state < AMD_INIT_RECOVERY) || (state > AMD_IDENTIFY_DIMMS)) @@ -57,6 +60,27 @@ return AgesaFunctionNameStr[index]; }
+void agesa_state_on_entry(struct agesa_state *task, AGESA_STRUCT_NAME func) +{ + task->apic_id = (u8) (cpuid_ebx(1) >> 24); + task->func = func; + task->function_name = agesa_struct_name(func); + + printk(BIOS_DEBUG, "\nAPIC %02d: ** Enter %s [%08x]\n", + task->apic_id, task->function_name, task->func); +} + +void agesa_state_on_exit(struct agesa_state *task, + AMD_CONFIG_PARAMS *StdHeader) +{ + printk(BIOS_DEBUG, "APIC %02d: Heap in %s (%d) at 0x%08x\n", + task->apic_id, heap_status_name(StdHeader->HeapStatus), + StdHeader->HeapStatus, (u32)StdHeader->HeapBasePtr); + + printk(BIOS_DEBUG, "APIC %02d: ** Exit %s [%08x]\n", + task->apic_id, task->function_name, task->func); +} + /* * Possible AGESA_STATUS values: * diff --git a/src/drivers/amd/agesa/state_machine.c b/src/drivers/amd/agesa/state_machine.c index 90bf038..03c6582 100644 --- a/src/drivers/amd/agesa/state_machine.c +++ b/src/drivers/amd/agesa/state_machine.c @@ -26,7 +26,6 @@ #include <northbridge/amd/agesa/BiosCallOuts.h> #include <amdlib.h>
-#include <debug_util.h> #include <AMD.h>
#if CONFIG(CPU_AMD_AGESA_OPENSOURCE) @@ -231,38 +230,6 @@ return status; }
-/* DEBUG trace helper */ - -struct agesa_state -{ - u8 apic_id; - - AGESA_STRUCT_NAME func; - const char *function_name; -}; - -static void state_on_entry(struct agesa_state *task, AGESA_STRUCT_NAME func, - const char *struct_name) -{ - task->apic_id = (u8) (cpuid_ebx(1) >> 24); - task->func = func; - task->function_name = struct_name; - - printk(BIOS_DEBUG, "\nAPIC %02d: ** Enter %s [%08x]\n", - task->apic_id, task->function_name, task->func); -} - -static void state_on_exit(struct agesa_state *task, - AMD_CONFIG_PARAMS *StdHeader) -{ - printk(BIOS_DEBUG, "APIC %02d: Heap in %s (%d) at 0x%08x\n", - task->apic_id, heap_status_name(StdHeader->HeapStatus), - StdHeader->HeapStatus, (u32)StdHeader->HeapBasePtr); - - printk(BIOS_DEBUG, "APIC %02d: ** Exit %s [%08x]\n", - task->apic_id, task->function_name, task->func); -} - int agesa_execute_state(struct sysinfo *cb, AGESA_STRUCT_NAME func) { AMD_INTERFACE_PARAMS aip; @@ -272,13 +239,12 @@ } agesa_params; void *buf = NULL; size_t len = 0; - const char *state_name = agesa_struct_name(func);
AGESA_STATUS status, final;
struct agesa_state task; memset(&task, 0, sizeof(task)); - state_on_entry(&task, func, state_name); + agesa_state_on_entry(&task, func);
aip.StdHeader = cb->StdHeader;
@@ -302,13 +268,13 @@ if (ENV_RAMSTAGE) final = ramstage_dispatch(cb, func, StdHeader);
- agesawrapper_trace(final, StdHeader, state_name); + agesawrapper_trace(final, StdHeader, task.function_name); ASSERT(final < AGESA_FATAL);
status = amd_release_struct(&aip); ASSERT(status == AGESA_SUCCESS);
- state_on_exit(&task, &aip.StdHeader); + agesa_state_on_exit(&task, &aip.StdHeader);
return (final < AGESA_FATAL) ? 0 : -1; } diff --git a/src/northbridge/amd/agesa/state_machine.h b/src/northbridge/amd/agesa/state_machine.h index d05ae47..74c3f61 100644 --- a/src/northbridge/amd/agesa/state_machine.h +++ b/src/northbridge/amd/agesa/state_machine.h @@ -23,7 +23,6 @@ #define HAS_LEGACY_WRAPPER CONFIG(BINARYPI_LEGACY_WRAPPER)
/* eventlog */ -const char *agesa_struct_name(int state); void agesawrapper_trace(AGESA_STATUS ret, AMD_CONFIG_PARAMS *StdHeader, const char *func); AGESA_STATUS agesawrapper_amdreadeventlog(UINT8 HeapStatus);
@@ -54,6 +53,17 @@ void platform_once(struct sysinfo *cb);
void agesa_set_interface(struct sysinfo *cb); + +struct agesa_state { + u8 apic_id; + + AGESA_STRUCT_NAME func; + const char *function_name; +}; + +void agesa_state_on_entry(struct agesa_state *task, AGESA_STRUCT_NAME func); +void agesa_state_on_exit(struct agesa_state *task, + AMD_CONFIG_PARAMS *StdHeader); int agesa_execute_state(struct sysinfo *cb, AGESA_STRUCT_NAME func);
/* AGESA dispatchers */
-
Kyösti Mälkki (Code Review) -
Marshall Dawson (Code Review)
-
Patrick Georgi (Code Review)