Change in coreboot[master]: lib/trace: Replace __PRE_RAM__ use

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

Change in ...coreboot[master]:...

Kyösti Mälkki (Code Review)

13 Sep 2019 13 Sep '19

11:10 a.m.

Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35399 )

Change subject: lib/trace: Replace __PRE_RAM__ use ......................................................................

lib/trace: Replace __PRE_RAM__ use

Change-Id: I957be92594aced2e8465e7f94d8d42e44c3418d7 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/include/trace.h 1 file changed, 2 insertions(+), 11 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/99/35399/1

diff --git a/src/include/trace.h b/src/include/trace.h index 4f5b8c4..aed69a8 100644 --- a/src/include/trace.h +++ b/src/include/trace.h @@ -16,16 +16,7 @@ #ifndef __TRACE_H #define __TRACE_H

- -#ifdef __PRE_RAM__ - -#define DISABLE_TRACE -#define ENABLE_TRACE -#define DISABLE_TRACE_ON_FUNCTION - -#else /* !__PRE_RAM__ */ - -#if CONFIG(TRACE) && !defined(__SMM__) +#if !ENV_ROMSTAGE_OR_BEFORE && CONFIG(TRACE)

void __cyg_profile_func_enter(void *, void *) __attribute__((no_instrument_function)); @@ -46,5 +37,5 @@ #define DISABLE_TRACE_ON_FUNCTION

#endif -#endif + #endif

Attachments:

attachment.htm (text/html — 3.1 KB)

Show replies by date

Aaron Durbin (Code Review)

23 Sep 23 Sep

5:08 p.m.

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35399 )

Change subject: lib/trace: Replace __PRE_RAM__ use ......................................................................

Patch Set 3: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/35399/3/src/include/trace.h File src/include/trace.h:

https://review.coreboot.org/c/coreboot/+/35399/3/src/include/trace.h@19 PS3, Line 19: ENV_ROMSTAGE_OR_BEFORE Are we just wanting to know if its RAMSTAGE? I think it's cleaner to use ENV_RAMSTAGE. Unless you want postcar in there as well?

-- To view, visit https://review.coreboot.org/c/coreboot/+/35399 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I957be92594aced2e8465e7f94d8d42e44c3418d7 Gerrit-Change-Number: 35399 Gerrit-PatchSet: 3 Gerrit-Owner: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Mon, 23 Sep 2019 15:08:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Kyösti Mälkki (Code Review)

5:23 p.m.

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35399 )

Change subject: lib/trace: Replace __PRE_RAM__ use ......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/35399/3/src/include/trace.h File src/include/trace.h:

https://review.coreboot.org/c/coreboot/+/35399/3/src/include/trace.h@19 PS3, Line 19: ENV_ROMSTAGE_OR_BEFORE

...

Are we just wanting to know if its RAMSTAGE? I think it's cleaner to use ENV_RAMSTAGE. […]

We build lib/trace.c for postcar and ramstage, so I chose this. Maybe lack of .bss was the reason why it was excluded from romstage?

With relocatable modules, I think we should normalize the printed addresses first? Like subtract _program perhaps? Maybe there is more that should be fixed, but maybe not in this commit scope...

-- To view, visit https://review.coreboot.org/c/coreboot/+/35399 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I957be92594aced2e8465e7f94d8d42e44c3418d7 Gerrit-Change-Number: 35399 Gerrit-PatchSet: 3 Gerrit-Owner: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Mon, 23 Sep 2019 15:23:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Aaron Durbin adurbin@chromium.org Gerrit-MessageType: comment

Aaron Durbin (Code Review)

5:31 p.m.

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35399 )

Change subject: lib/trace: Replace __PRE_RAM__ use ......................................................................

Patch Set 3: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/35399/3/src/include/trace.h File src/include/trace.h:

https://review.coreboot.org/c/coreboot/+/35399/3/src/include/trace.h@19 PS3, Line 19: ENV_ROMSTAGE_OR_BEFORE

...

We build lib/trace.c for postcar and ramstage, so I chose this. Maybe lack of . […]

Ok. I just wanted to confirm.

-- To view, visit https://review.coreboot.org/c/coreboot/+/35399 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I957be92594aced2e8465e7f94d8d42e44c3418d7 Gerrit-Change-Number: 35399 Gerrit-PatchSet: 3 Gerrit-Owner: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Mon, 23 Sep 2019 15:31:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Kyösti Mälkki kyosti.malkki@gmail.com Comment-In-Reply-To: Aaron Durbin adurbin@chromium.org Gerrit-MessageType: comment

Kyösti Mälkki (Code Review)

5:45 p.m.

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35399 )

Change subject: lib/trace: Replace __PRE_RAM__ use ......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/35399/3/src/include/trace.h File src/include/trace.h:

https://review.coreboot.org/c/coreboot/+/35399/3/src/include/trace.h@19 PS3, Line 19: ENV_ROMSTAGE_OR_BEFORE

...

Ok. I just wanted to confirm.

Ack

-- To view, visit https://review.coreboot.org/c/coreboot/+/35399 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I957be92594aced2e8465e7f94d8d42e44c3418d7 Gerrit-Change-Number: 35399 Gerrit-PatchSet: 3 Gerrit-Owner: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Mon, 23 Sep 2019 15:45:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Kyösti Mälkki kyosti.malkki@gmail.com Comment-In-Reply-To: Aaron Durbin adurbin@chromium.org Gerrit-MessageType: comment

Kyösti Mälkki (Code Review)

11:38 p.m.

Kyösti Mälkki has submitted this change. ( https://review.coreboot.org/c/coreboot/+/35399 )

Change subject: lib/trace: Replace __PRE_RAM__ use ......................................................................

lib/trace: Replace __PRE_RAM__ use

Change-Id: I957be92594aced2e8465e7f94d8d42e44c3418d7 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/35399 Reviewed-by: Aaron Durbin adurbin@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/include/trace.h 1 file changed, 2 insertions(+), 11 deletions(-)

Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved

diff --git a/src/include/trace.h b/src/include/trace.h index 4f5b8c4..aed69a8 100644 --- a/src/include/trace.h +++ b/src/include/trace.h @@ -16,16 +16,7 @@ #ifndef __TRACE_H #define __TRACE_H

void __cyg_profile_func_enter(void *, void *) __attribute__((no_instrument_function)); @@ -46,5 +37,5 @@ #define DISABLE_TRACE_ON_FUNCTION

#endif -#endif + #endif

-- To view, visit https://review.coreboot.org/c/coreboot/+/35399 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I957be92594aced2e8465e7f94d8d42e44c3418d7 Gerrit-Change-Number: 35399 Gerrit-PatchSet: 4 Gerrit-Owner: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: merged

2046

days inactive

2056

days old

coreboot-gerrit@coreboot.org

5 comments

2 participants

tags (0)

participants (2)

Aaron Durbin (Code Review)
Kyösti Mälkki (Code Review)