Change in coreboot[master]: util/inteltool: fix Ie00fef5a5ba2779e0ff45640cff5cc9f1d096dc1 on FreeBSD
Idwer Vollering has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37767 )
Change subject: util/inteltool: fix Ie00fef5a5ba2779e0ff45640cff5cc9f1d096dc1 on FreeBSD ......................................................................
util/inteltool: fix Ie00fef5a5ba2779e0ff45640cff5cc9f1d096dc1 on FreeBSD
Change-Id: I4aba72f39b528fd70451a4656fd6c835ff766e49 Signed-off-by: Idwer Vollering vidwer@gmail.com --- M util/inteltool/Makefile M util/inteltool/inteltool.h 2 files changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/37767/1
diff --git a/util/inteltool/Makefile b/util/inteltool/Makefile index cd02fa8..918b6ec 100644 --- a/util/inteltool/Makefile +++ b/util/inteltool/Makefile @@ -23,6 +23,7 @@ INSTALL ?= /usr/bin/env install PREFIX ?= /usr/local CFLAGS ?= -O2 -g -Wall -Wextra -Wmissing-prototypes +CFLAGS += -Wno-unused-function LDFLAGS += -lpci -lz
CPPFLAGS += -I$(top)/src/commonlib/include diff --git a/util/inteltool/inteltool.h b/util/inteltool/inteltool.h index fc6dc4b..81ebc72 100644 --- a/util/inteltool/inteltool.h +++ b/util/inteltool/inteltool.h @@ -370,7 +370,7 @@ #define rdmsr freebsd_rdmsr #define wrmsr freebsd_wrmsr typedef struct { uint32_t hi, lo; } msr_t; -msr_t freebsd_rdmsr(int addr); +static msr_t freebsd_rdmsr(int addr); int freebsd_wrmsr(int addr, msr_t msr); #endif typedef struct { uint16_t addr; int size; char *name; } io_register_t;
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37767 )
Change subject: util/inteltool: fix Ie00fef5a5ba2779e0ff45640cff5cc9f1d096dc1 on FreeBSD ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/37767/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37767/1//COMMIT_MSG@7 PS1, Line 7: util/inteltool: fix Ie00fef5a5ba2779e0ff45640cff5cc9f1d096dc1 on FreeBSD Please use something like below.
Make local function static for FreeBSD
https://review.coreboot.org/c/coreboot/+/37767/1/util/inteltool/Makefile File util/inteltool/Makefile:
https://review.coreboot.org/c/coreboot/+/37767/1/util/inteltool/Makefile@26 PS1, Line 26: CFLAGS += -Wno-unused-function Please elaborate, why this is needed.
Idwer Vollering has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37767 )
Change subject: util/inteltool: fix Ie00fef5a5ba2779e0ff45640cff5cc9f1d096dc1 on FreeBSD ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37767/1/util/inteltool/Makefile File util/inteltool/Makefile:
https://review.coreboot.org/c/coreboot/+/37767/1/util/inteltool/Makefile@26 PS1, Line 26: CFLAGS += -Wno-unused-function
Please elaborate, why this is needed.
This is a cosmetic solution to hide useless output like ./inteltool.h:373:14: warning: unused function 'freebsd_rdmsr' [-Wunused-function] With or without this line, a binary will be built.
Hello Stefan Reinauer, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37767
to look at the new patch set (#2).
Change subject: util/inteltool: correct typedef'ed rdmsr function for FreeBSD ......................................................................
util/inteltool: correct typedef'ed rdmsr function for FreeBSD
freebsd_msr() needs to be static. This was overlooked in commit Ie00fef5a5ba2779e0ff45640cff5cc9f1d096dc1
Change-Id: I4aba72f39b528fd70451a4656fd6c835ff766e49 Signed-off-by: Idwer Vollering vidwer@gmail.com --- M util/inteltool/Makefile M util/inteltool/inteltool.h 2 files changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/37767/2
Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37767 )
Change subject: util/inteltool: correct typedef'ed rdmsr function for FreeBSD ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/37767/1/util/inteltool/Makefile File util/inteltool/Makefile:
https://review.coreboot.org/c/coreboot/+/37767/1/util/inteltool/Makefile@26 PS1, Line 26: CFLAGS += -Wno-unused-function
This is a cosmetic solution to hide useless output like ./inteltool. […]
I'm a little puzzled by why this is happening. As far as I can tell, this freebsd_rdmsr/rdmsr function is called in cpu.c, so there shouldn't be a warning...
https://review.coreboot.org/c/coreboot/+/37767/1/util/inteltool/inteltool.h File util/inteltool/inteltool.h:
https://review.coreboot.org/c/coreboot/+/37767/1/util/inteltool/inteltool.h@... PS1, Line 374: int freebsd_wrmsr(int addr, msr_t msr); I think these two prototypes can just be removed instead, since they aren't used outside of cpu.c (it doesn't make sense to declare a static function in a header anyway).
Hello Stefan Reinauer, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37767
to look at the new patch set (#3).
Change subject: util/inteltool: drop OS-specific rdmsr functions ......................................................................
util/inteltool: drop OS-specific rdmsr functions
Tested on: FreeBSD
The previous commit is Ie00fef5a5ba2779e0ff45640cff5cc9f1d096dc1
Change-Id: I4aba72f39b528fd70451a4656fd6c835ff766e49 Signed-off-by: Idwer Vollering vidwer@gmail.com --- M util/inteltool/inteltool.h 1 file changed, 0 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/37767/3
Idwer Vollering has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37767 )
Change subject: util/inteltool: drop OS-specific rdmsr functions ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/37767/1/util/inteltool/Makefile File util/inteltool/Makefile:
https://review.coreboot.org/c/coreboot/+/37767/1/util/inteltool/Makefile@26 PS1, Line 26: CFLAGS += -Wno-unused-function
I'm a little puzzled by why this is happening. […]
Done
https://review.coreboot.org/c/coreboot/+/37767/1/util/inteltool/inteltool.h File util/inteltool/inteltool.h:
https://review.coreboot.org/c/coreboot/+/37767/1/util/inteltool/inteltool.h@... PS1, Line 374: int freebsd_wrmsr(int addr, msr_t msr);
I think these two prototypes can just be removed instead, since they aren't used outside of cpu. […]
Done
Hello Stefan Reinauer, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37767
to look at the new patch set (#4).
Change subject: util/inteltool: drop OS-specific rdmsr functions ......................................................................
util/inteltool: drop OS-specific rdmsr functions
Tested on: FreeBSD 13.0-CURRENT r355582
The previous commit is Ie00fef5a5ba2779e0ff45640cff5cc9f1d096dc1
Change-Id: I4aba72f39b528fd70451a4656fd6c835ff766e49 Signed-off-by: Idwer Vollering vidwer@gmail.com --- M util/inteltool/inteltool.h 1 file changed, 0 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/37767/4
Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37767 )
Change subject: util/inteltool: drop OS-specific rdmsr functions ......................................................................
Patch Set 4:
(3 comments)
https://review.coreboot.org/c/coreboot/+/37767/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37767/4//COMMIT_MSG@7 PS4, Line 7: rdmsr functions nit: rdmsr/wrmsr prototypes
https://review.coreboot.org/c/coreboot/+/37767/4//COMMIT_MSG@9 PS4, Line 9: Tested on: FreeBSD 13.0-CURRENT r355582 This should go after the commit message.
https://review.coreboot.org/c/coreboot/+/37767/4//COMMIT_MSG@12 PS4, Line 12: Maybe add that these prototypes need to be removed since that commit made rdmsr static. Also, it would be better to link to the SHA hash and commit summary, eg. 6faccd1f00 (util/inteltool: Make internal functions static), rather than the change id.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37767 )
Change subject: util/inteltool: drop OS-specific rdmsr functions ......................................................................
Patch Set 4: Code-Review+1
Hello Angel Pons, Stefan Reinauer, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37767
to look at the new patch set (#5).
Change subject: util/inteltool: drop OS-specific rdmsr/wrmsr prototypes ......................................................................
util/inteltool: drop OS-specific rdmsr/wrmsr prototypes
The previous commit (that was not touching inteltool.h) marking internal functions as static is 6faccd1f00
Tested on: FreeBSD 13.0-CURRENT r355582
Change-Id: I4aba72f39b528fd70451a4656fd6c835ff766e49 Signed-off-by: Idwer Vollering vidwer@gmail.com --- M util/inteltool/inteltool.h 1 file changed, 0 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/37767/5
Idwer Vollering has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37767 )
Change subject: util/inteltool: drop OS-specific rdmsr/wrmsr prototypes ......................................................................
Patch Set 5:
(3 comments)
https://review.coreboot.org/c/coreboot/+/37767/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37767/4//COMMIT_MSG@7 PS4, Line 7: rdmsr functions
nit: rdmsr/wrmsr prototypes
Done
https://review.coreboot.org/c/coreboot/+/37767/4//COMMIT_MSG@9 PS4, Line 9: Tested on: FreeBSD 13.0-CURRENT r355582
This should go after the commit message.
Done
https://review.coreboot.org/c/coreboot/+/37767/4//COMMIT_MSG@12 PS4, Line 12:
Maybe add that these prototypes need to be removed since that commit made rdmsr static. […]
Done
Idwer Vollering has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37767 )
Change subject: util/inteltool: drop OS-specific rdmsr/wrmsr prototypes ......................................................................
Patch Set 5:
Ping!
Hello build bot (Jenkins), Stefan Reinauer, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37767
to look at the new patch set (#6).
Change subject: util/inteltool: drop OS-specific rdmsr/wrmsr prototypes ......................................................................
util/inteltool: drop OS-specific rdmsr/wrmsr prototypes
The previous commit (that was not touching inteltool.h) marking internal functions as static is commit 6faccd1f00
Tested on: FreeBSD 13.0-CURRENT r355582
Change-Id: I4aba72f39b528fd70451a4656fd6c835ff766e49 Signed-off-by: Idwer Vollering vidwer@gmail.com --- M util/inteltool/inteltool.h 1 file changed, 0 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/37767/6
Hello build bot (Jenkins), Stefan Reinauer, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37767
to look at the new patch set (#7).
Change subject: util/inteltool: drop OS-specific rdmsr/wrmsr prototypes ......................................................................
util/inteltool: drop OS-specific rdmsr/wrmsr prototypes
The previous commit (that was not touching inteltool.h) marking internal functions as static is commit 6faccd1f00
Tested on: FreeBSD 13.0-CURRENT r355582
Change-Id: I4aba72f39b528fd70451a4656fd6c835ff766e49 Signed-off-by: Idwer Vollering vidwer@gmail.com --- M util/inteltool/inteltool.h 1 file changed, 0 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/37767/7
Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37767 )
Change subject: util/inteltool: drop OS-specific rdmsr/wrmsr prototypes ......................................................................
Patch Set 8: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/37767/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37767/1//COMMIT_MSG@7 PS1, Line 7: util/inteltool: fix Ie00fef5a5ba2779e0ff45640cff5cc9f1d096dc1 on FreeBSD
Please use something like below. […]
Done
Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37767 )
Change subject: util/inteltool: drop OS-specific rdmsr/wrmsr prototypes ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37767/8//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37767/8//COMMIT_MSG@7 PS8, Line 7: OS- FreeBSD specific would be more accurate, as this is a FreeBSD specific patch
Idwer Vollering has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37767 )
Change subject: util/inteltool: drop OS-specific rdmsr/wrmsr prototypes ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37767/8//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37767/8//COMMIT_MSG@7 PS8, Line 7: OS-
FreeBSD specific would be more accurate, as this is a FreeBSD specific patch
Done
Idwer Vollering has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37767 )
Change subject: util/inteltool: drop OS-specific rdmsr/wrmsr prototypes ......................................................................
util/inteltool: drop OS-specific rdmsr/wrmsr prototypes
The previous commit (that was not touching inteltool.h) marking internal functions as static is commit 6faccd1f00
Tested on: FreeBSD 13.0-CURRENT r355582
Change-Id: I4aba72f39b528fd70451a4656fd6c835ff766e49 Signed-off-by: Idwer Vollering vidwer@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/37767 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Jacob Garber jgarber1@ualberta.ca Reviewed-by: Angel Pons th3fanbus@gmail.com --- M util/inteltool/inteltool.h 1 file changed, 0 insertions(+), 2 deletions(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, but someone else must approve Jacob Garber: Looks good to me, approved
diff --git a/util/inteltool/inteltool.h b/util/inteltool/inteltool.h index 77ad61f..b3253e7 100644 --- a/util/inteltool/inteltool.h +++ b/util/inteltool/inteltool.h @@ -380,8 +380,6 @@ #define rdmsr freebsd_rdmsr #define wrmsr freebsd_wrmsr typedef struct { uint32_t hi, lo; } msr_t; -msr_t freebsd_rdmsr(int addr); -int freebsd_wrmsr(int addr, msr_t msr); #endif typedef struct { uint16_t addr; int size; char *name; } io_register_t; typedef struct {
-
Angel Pons (Code Review) -
Idwer Vollering (Code Review)
-
Jacob Garber (Code Review)
-
Paul Menzel (Code Review)
-
Stefan Reinauer (Code Review)