Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/56705 )
Change subject: soc/amd/common/block/gpio_banks: use unsigned int for gevent parameter
......................................................................
soc/amd/common/block/gpio_banks: use unsigned int for gevent parameter
A valid GEVENT number is never negative. The local variable in
set_single_gpio still needs to be a signed integer, since the return
value of get_gpio_gevent being -1 indicates that the GPIO can't generate
a GEVENT. The check for that makes the function return before calling
program_smi of program_sci, so the parameter of those functions can be
changed to unsigned.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: I6ce23ceed1585589932824b8cab2a138328672a9
---
M src/soc/amd/common/block/gpio_banks/gpio.c
1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/56705/1
diff --git a/src/soc/amd/common/block/gpio_banks/gpio.c b/src/soc/amd/common/block/gpio_banks/gpio.c
index 6e141f6..e897371 100644
--- a/src/soc/amd/common/block/gpio_banks/gpio.c
+++ b/src/soc/amd/common/block/gpio_banks/gpio.c
@@ -27,7 +27,7 @@
return -1;
}
-static void program_smi(uint32_t flags, int gevent_num)
+static void program_smi(uint32_t flags, unsigned int gevent_num)
{
uint8_t level;
@@ -52,7 +52,7 @@
* In a similar fashion, polarity (rising/falling, hi/lo) of each GPE is
* represented as a single bit in SMI_SCI_TRIG register.
*/
-static void program_sci(uint32_t flags, int gevent_num)
+static void program_sci(uint32_t flags, unsigned int gevent_num)
{
struct sci_source sci;
--
To view, visit https://review.coreboot.org/c/coreboot/+/56705
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I6ce23ceed1585589932824b8cab2a138328672a9
Gerrit-Change-Number: 56705
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: newchange
Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson.
Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/56703 )
Change subject: soc/amd/common/block/gpio_banks/gpio: add comment in check_gpios
......................................................................
soc/amd/common/block/gpio_banks/gpio: add comment in check_gpios
Each bit in the GPIO wake status index registers is set to 1 when at
least one of 4 corresponding GPIO pins has its wake status register set.
Added the comment since the gpio_base + i * 4 in the next line looked as
if it calculates some absolute register value which is not what the code
does or should be doing.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: I2fc8e9c5bd7c1b011f364b05d0cfdeb0df88ada6
---
M src/soc/amd/common/block/gpio_banks/gpio.c
1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/56703/1
diff --git a/src/soc/amd/common/block/gpio_banks/gpio.c b/src/soc/amd/common/block/gpio_banks/gpio.c
index 8f6f195..7d725bd 100644
--- a/src/soc/amd/common/block/gpio_banks/gpio.c
+++ b/src/soc/amd/common/block/gpio_banks/gpio.c
@@ -308,6 +308,7 @@
for (i = 0; i < bit_limit; i++) {
if (!(wake_stat & BIT(i)))
continue;
+ /* Each wake status register bit is for 4 GPIOs that then will be checked */
begin = gpio_base + i * 4;
end = begin + 4;
/* There is no gpio 63. */
--
To view, visit https://review.coreboot.org/c/coreboot/+/56703
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I2fc8e9c5bd7c1b011f364b05d0cfdeb0df88ada6
Gerrit-Change-Number: 56703
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-MessageType: newchange
Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson.
Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/56702 )
Change subject: soc/amd/common/block/gpio_banks/gpio: use size_t where needed
......................................................................
soc/amd/common/block/gpio_banks/gpio: use size_t where needed
Since the parameter the variable gets compared with is size_t type, use
size_t as type for that variable too.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: If82a948bf71079d456616f4438f4b754e0d7262d
---
M src/soc/amd/common/block/gpio_banks/gpio.c
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/56702/1
diff --git a/src/soc/amd/common/block/gpio_banks/gpio.c b/src/soc/amd/common/block/gpio_banks/gpio.c
index 08621c0..8f6f195 100644
--- a/src/soc/amd/common/block/gpio_banks/gpio.c
+++ b/src/soc/amd/common/block/gpio_banks/gpio.c
@@ -18,7 +18,7 @@
static int get_gpio_gevent(gpio_t gpio, const struct soc_amd_event *table,
size_t items)
{
- int i;
+ size_t i;
for (i = 0; i < items; i++) {
if ((table + i)->gpio == gpio)
--
To view, visit https://review.coreboot.org/c/coreboot/+/56702
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If82a948bf71079d456616f4438f4b754e0d7262d
Gerrit-Change-Number: 56702
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-MessageType: newchange
Attention is currently required from: Tim Wawrzynczak, Yu-Ping Wu.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56698 )
Change subject: MAINTAINERS: Add myself for src/security/vboot
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
Do you have much experience with this code, Tim? I don't mind adding extra people to the list (or if you just want to take care of specific stuff like EC sync, that's fine too), but I feel that primarily this should be handled by someone who is also maintaining the vboot library code itself (what used to be kitching@ and is now supposed to transition to yupingso@). (And I should probably also be on the list, I'm basically reviewing all of these already anyway.)
Or if you have free cycles and want to take a more active role in vboot itself, let me know! We have a ton of current and backlogged issues that could use extra help.
--
To view, visit https://review.coreboot.org/c/coreboot/+/56698
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I560a333c807ad5e055a0e035e675195d9cbbf954
Gerrit-Change-Number: 56698
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Comment-Date: Fri, 30 Jul 2021 00:35:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56697 )
Change subject: MAINTAINERS: Remove Aaron Durbin
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/56697
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ife7c75ab8290873da1e02332609482c407373c45
Gerrit-Change-Number: 56697
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Comment-Date: Fri, 30 Jul 2021 00:07:39 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Jakub Czapiga, Jan Dabros.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56601 )
Change subject: tests: Add lib/cbfs-verification-test test case
......................................................................
Patch Set 2:
(3 comments)
File tests/lib/cbfs-verification-test.c:
https://review.coreboot.org/c/coreboot/+/56601/comment/6d8f81ec_56131ac6
PS2, Line 4: #define __noreturn
> This is here to block `__noreturn` effect on `die()` function, which falls `failed()` to signal an e […]
You're only changing the prototype of the function for this compilation unit though, other compilation units (e.g. lib/cbfs.c which I assume is what you actually care about?) will still assume it is noreturn. Having code that assumes a function is noreturn call a function implementation that is not actually noreturn is a very bad thing. I guess since we know fail() doesn't actually return it wouldn't blow up, but still, the more correct solution would be to just mark _fail() (and mock_assert(), I guess) in Cmocka as __attribute__((__noreturn__)). (Maybe you could try upstreaming a change to Cmocka for that? Unfortunately the maintainer didn't seem to be very active last time I reached out to him, but doesn't hurt to try...)
https://review.coreboot.org/c/coreboot/+/56601/comment/436e81fc_f60f3f96
PS2, Line 121: {
> `ulzman()` and `ulz4fn()` will be called only for files with compression enabled and when the `cbfs_ […]
Agreed.
https://review.coreboot.org/c/coreboot/+/56601/comment/cd761b98_9a51f76a
PS2, Line 167: assert_null(mapping);
> The problem with --add-symbol is, we have to know, what address of <functionname> is. To do this, we have to scan all produced object files to find the exact file and address of that symbol.
Well, I mean, the objcopy is already done on each .o file individually anyway. At that point you could iterate through $(1)-mocks, check if each of those names is defined as a symbol (type F) in the input .o with `objdump -t`, and if so use the address and section name parsed out of there to `--add-symbol __real_functionname=section:address,F` an alias during the objcopy. Then I believe the added symbol should keep pointing to the function during linking (because it's relative to that section) and prevent the section from being garbage collected if it is referenced during the final link, even if the function symbol itself has been overridden by another non-weak definition elsewhere.
But okay, I admit that's maybe going a bit far. I just like having cool things...
--
To view, visit https://review.coreboot.org/c/coreboot/+/56601
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I1d8cbb1c2d0a9db3236de065428b70a9c2a66330
Gerrit-Change-Number: 56601
Gerrit-PatchSet: 2
Gerrit-Owner: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Reviewer: Jan Dabros <jsd(a)semihalf.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Paul Fagerburg <pfagerburg(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Attention: Jan Dabros <jsd(a)semihalf.com>
Gerrit-Comment-Date: Fri, 30 Jul 2021 00:06:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Jakub Czapiga <jacz(a)semihalf.com>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak, Julius Werner.
Aaron Durbin has removed Aaron Durbin from this change. ( https://review.coreboot.org/c/coreboot/+/56697 )
Change subject: MAINTAINERS: Remove Aaron Durbin
......................................................................
Removed reviewer Aaron Durbin.
--
To view, visit https://review.coreboot.org/c/coreboot/+/56697
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ife7c75ab8290873da1e02332609482c407373c45
Gerrit-Change-Number: 56697
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: deleteReviewer