Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35290 )
Change subject: timestamps: Mostly remove struct timestamp_cache ......................................................................
timestamps: Mostly remove struct timestamp_cache
Change-Id: Ifcd75630e562af302312f93bdf180aa90f18d21d Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/lib/timestamp.c 1 file changed, 13 insertions(+), 16 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/35290/1
diff --git a/src/lib/timestamp.c b/src/lib/timestamp.c index 4983885..db73f15 100644 --- a/src/lib/timestamp.c +++ b/src/lib/timestamp.c @@ -58,25 +58,25 @@ as CBMEM comes available. */ static struct timestamp_table *glob_ts_table CAR_GLOBAL;
-static void timestamp_cache_init(struct timestamp_cache *ts_cache, +static void timestamp_cache_init(struct timestamp_table *ts_cache, uint64_t base) { - ts_cache->table.num_entries = 0; - ts_cache->table.max_entries = MAX_BSS_TIMESTAMP_CACHE; - ts_cache->table.base_time = base; + ts_cache->num_entries = 0; + ts_cache->max_entries = MAX_BSS_TIMESTAMP_CACHE; + ts_cache->base_time = base;
if (USE_TIMESTAMP_REGION) - ts_cache->table.max_entries = (REGION_SIZE(timestamp) - - offsetof(struct timestamp_cache, entries)) + ts_cache->max_entries = (REGION_SIZE(timestamp) - + offsetof(struct timestamp_table, entries)) / sizeof(struct timestamp_entry); }
-static struct timestamp_cache *timestamp_cache_get(void) +static struct timestamp_table *timestamp_cache_get(void) { - struct timestamp_cache *ts_cache = NULL; + struct timestamp_table *ts_cache = NULL;
if (TIMESTAMP_CACHE_IN_BSS) { - ts_cache = ×tamp_cache; + ts_cache = ×tamp_cache.table; } else if (USE_TIMESTAMP_REGION) { if (REGION_SIZE(timestamp) < sizeof(*ts_cache)) BUG(); @@ -122,17 +122,14 @@ static struct timestamp_table *timestamp_table_get(void) { struct timestamp_table *ts_table; - struct timestamp_cache *ts_cache;
ts_table = car_get_ptr(glob_ts_table); if (ts_table) return ts_table;
- ts_cache = timestamp_cache_get(); - if (ts_cache) - ts_table = &ts_cache->table; - + ts_table = timestamp_cache_get(); car_set_ptr(glob_ts_table, ts_table); + return ts_table; }
@@ -196,7 +193,7 @@
void timestamp_init(uint64_t base) { - struct timestamp_cache *ts_cache; + struct timestamp_table *ts_cache;
assert(ENV_ROMSTAGE_OR_BEFORE);
@@ -211,7 +208,7 @@ }
timestamp_cache_init(ts_cache, base); - timestamp_table_set(&ts_cache->table); + timestamp_table_set(ts_cache); }
static void timestamp_sync_cache_to_cbmem(struct timestamp_table *ts_cbmem_table)
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35290 )
Change subject: timestamps: Mostly remove struct timestamp_cache ......................................................................
Patch Set 1:
Can we get a decision how amd/picasso will handle CBMEM console and timestamps prior to CBMEM becoming online?
I would rather remove TIMESTAMP_CACHE_IN_BSS. That would keep REGION(timestamp) somewhere in the linker scripts, even though PSP is likely not able to push any useful there.
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35290 )
Change subject: timestamps: Mostly remove struct timestamp_cache ......................................................................
Patch Set 1:
Patch Set 1:
Can we get a decision how amd/picasso will handle CBMEM console and timestamps prior to CBMEM becoming online?
I would rather remove TIMESTAMP_CACHE_IN_BSS. That would keep REGION(timestamp) somewhere in the linker scripts, even though PSP is likely not able to push any useful there.
Removing TIMESTAMP_CACHE_IN_BSS seems OK to me. I'm currently running w/timestamps and console outside of BSS before cbmem. And since I already need to reserve DRAM for early code and data, I don't see the downside of keeping it out of BSS.
As far as the PSP's concerned, it only provides data by writing to the AGESA PSP Output Block (APOB). That's a location we set at build time and the PSP reads it from the BIOS Directory Table we generate. (hmm, I should change that address to make it contiguous to my romstage & data in DRAM...) At the moment, the only plan is for the APOB to be consumed by AGESA.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35290 )
Change subject: timestamps: Mostly remove struct timestamp_cache ......................................................................
Patch Set 2: Code-Review+2
Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35290 )
Change subject: timestamps: Mostly remove struct timestamp_cache ......................................................................
Patch Set 2: Code-Review+2
Kyösti Mälkki has submitted this change. ( https://review.coreboot.org/c/coreboot/+/35290 )
Change subject: timestamps: Mostly remove struct timestamp_cache ......................................................................
timestamps: Mostly remove struct timestamp_cache
Change-Id: Ifcd75630e562af302312f93bdf180aa90f18d21d Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/35290 Reviewed-by: Julius Werner jwerner@chromium.org Reviewed-by: Werner Zeh werner.zeh@siemens.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/lib/timestamp.c 1 file changed, 13 insertions(+), 16 deletions(-)
Approvals: build bot (Jenkins): Verified Werner Zeh: Looks good to me, approved Julius Werner: Looks good to me, approved
diff --git a/src/lib/timestamp.c b/src/lib/timestamp.c index 4983885..db73f15 100644 --- a/src/lib/timestamp.c +++ b/src/lib/timestamp.c @@ -58,25 +58,25 @@ as CBMEM comes available. */ static struct timestamp_table *glob_ts_table CAR_GLOBAL;
-static void timestamp_cache_init(struct timestamp_cache *ts_cache, +static void timestamp_cache_init(struct timestamp_table *ts_cache, uint64_t base) { - ts_cache->table.num_entries = 0; - ts_cache->table.max_entries = MAX_BSS_TIMESTAMP_CACHE; - ts_cache->table.base_time = base; + ts_cache->num_entries = 0; + ts_cache->max_entries = MAX_BSS_TIMESTAMP_CACHE; + ts_cache->base_time = base;
if (USE_TIMESTAMP_REGION) - ts_cache->table.max_entries = (REGION_SIZE(timestamp) - - offsetof(struct timestamp_cache, entries)) + ts_cache->max_entries = (REGION_SIZE(timestamp) - + offsetof(struct timestamp_table, entries)) / sizeof(struct timestamp_entry); }
-static struct timestamp_cache *timestamp_cache_get(void) +static struct timestamp_table *timestamp_cache_get(void) { - struct timestamp_cache *ts_cache = NULL; + struct timestamp_table *ts_cache = NULL;
if (TIMESTAMP_CACHE_IN_BSS) { - ts_cache = ×tamp_cache; + ts_cache = ×tamp_cache.table; } else if (USE_TIMESTAMP_REGION) { if (REGION_SIZE(timestamp) < sizeof(*ts_cache)) BUG(); @@ -122,17 +122,14 @@ static struct timestamp_table *timestamp_table_get(void) { struct timestamp_table *ts_table; - struct timestamp_cache *ts_cache;
ts_table = car_get_ptr(glob_ts_table); if (ts_table) return ts_table;
- ts_cache = timestamp_cache_get(); - if (ts_cache) - ts_table = &ts_cache->table; - + ts_table = timestamp_cache_get(); car_set_ptr(glob_ts_table, ts_table); + return ts_table; }
@@ -196,7 +193,7 @@
void timestamp_init(uint64_t base) { - struct timestamp_cache *ts_cache; + struct timestamp_table *ts_cache;
assert(ENV_ROMSTAGE_OR_BEFORE);
@@ -211,7 +208,7 @@ }
timestamp_cache_init(ts_cache, base); - timestamp_table_set(&ts_cache->table); + timestamp_table_set(ts_cache); }
static void timestamp_sync_cache_to_cbmem(struct timestamp_table *ts_cbmem_table)
-
Julius Werner (Code Review) -
Kyösti Mälkki (Code Review)
-
Marshall Dawson (Code Review)
-
Werner Zeh (Code Review)