Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56700 )
Change subject: soc/amd/common/block/gpio_banks/gpio: use gpio_t for GPIO numbers
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS3:
> Maybe change gpio_read32() and _write32() to accept gpio_t ahead of this patch?
IIRC I didn't do that since gpio_t is defined in gpio_banks.h and including that in acpimmio.h would be a bit unexpected. I do agree that it would be good if the functions use the correct parameter type though. Would moving the register access functions from acpimmio.h to the common gpio.c be a good idea? In general the GPIO part of acpimmio.h seems a bit odd to me, since it does more than just the raw register access. Would probably be a good idea to try to untangle things a bit more there and also make the common i2c code use some helper functions from gpio.c instead of directly accessing the GPIO registers
--
To view, visit https://review.coreboot.org/c/coreboot/+/56700
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7cf9cbd2a287dcfe3a47a8a6b164c2b3d8ae95d6
Gerrit-Change-Number: 56700
Gerrit-PatchSet: 3
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-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.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-Comment-Date: Fri, 30 Jul 2021 23:36:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-MessageType: comment
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:
(2 comments)
Patchset:
PS1:
> Could you explain to me what this file is for? Does Gerrit automatically add people as CL reviewers […]
Yes, it's gonna auto-CC you and it's also meant for people to manually look up who they can ask about a certain file (and the M:, S:, etc. tags are supposed to indicate support level). Moving this to CB:56728
PS1:
> I do have some, not nearly as much as Joel or you 😊 I just figured having someone there was better […]
Okay. Like I said I'm happy to also have you on the list if you want to (for everything or just parts), let me know if you do.
--
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-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Fri, 30 Jul 2021 23:35:12 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Yu-Ping Wu.
Hello Yu-Ping Wu,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/56728
to review the following change.
Change subject: MAINTAINERS: Update vboot maintainers
......................................................................
MAINTAINERS: Update vboot maintainers
Add current maintainers for vboot-related code, and combine the vboot
and vboot2 sections which are not really separate anymore. What's left
in vendorcode/google/chromeos isn't really related to vboot anymore
since the vboot code was moved under src/security, so take it out of
there.
Signed-off-by: Julius Werner <jwerner(a)chromium.org>
Change-Id: I59a2df9c1580291a6d29537868aab0e1b339a2ce
---
M MAINTAINERS
1 file changed, 3 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/28/56728/1
diff --git a/MAINTAINERS b/MAINTAINERS
index 02fe5fe..27fb817 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1,5 +1,4 @@
-
List of upstream coreboot maintainers
and how to submit coreboot changes
@@ -790,8 +789,10 @@
F: 3rdparty/blobs/
VERIFIED BOOT
+M: Julius Werner <jwerner(a)chromium.org>
+M: Yu-Ping Wu <yupingso(a)google.com>
F: 3rdparty/vboot/
-F: src/vendorcode/google/chromeos/
+F: src/security/vboot/
F: src/include/tpm.h
F: src/include/tpm_lite/
@@ -823,9 +824,6 @@
F: src/include/console/
F: src/drivers/uart/
-VERIFIED BOOT 2
-F: src/security/vboot/
-
TPM SUPPORT
M: Christian Walter <christian.walter(a)9elements.com>
S: Supported
--
To view, visit https://review.coreboot.org/c/coreboot/+/56728
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I59a2df9c1580291a6d29537868aab0e1b339a2ce
Gerrit-Change-Number: 56728
Gerrit-PatchSet: 1
Gerrit-Owner: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-MessageType: newchange
Julius Werner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/56697 )
Change subject: MAINTAINERS: Remove Aaron Durbin
......................................................................
MAINTAINERS: Remove Aaron Durbin
Aaron is no longer employed at Google and is not actively working on
coreboot anymore, therefore remove this account from MAINTAINERS.
Change-Id: Ife7c75ab8290873da1e02332609482c407373c45
Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/56697
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Julius Werner <jwerner(a)chromium.org>
Reviewed-by: Paul Menzel <paulepanter(a)mailbox.org>
---
M MAINTAINERS
1 file changed, 0 insertions(+), 1 deletion(-)
Approvals:
build bot (Jenkins): Verified
Paul Menzel: Looks good to me, but someone else must approve
Julius Werner: Looks good to me, approved
diff --git a/MAINTAINERS b/MAINTAINERS
index 74f1854..02fe5fe 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -824,7 +824,6 @@
F: src/drivers/uart/
VERIFIED BOOT 2
-M: Aaron Durbin <adurbin(a)chromium.org>
F: src/security/vboot/
TPM SUPPORT
--
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: 3
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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Attention is currently required from: Tim Wawrzynczak.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56726 )
Change subject: src: Add new Kconfig DEBUG_SOC_HOOKS
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
Does this need to be toplevel? I don't really understand what this debug_hooks thing is supposed to be, but it sounds very Intel-specific. Shouldn't it go into soc/intel/Kconfig?
--
To view, visit https://review.coreboot.org/c/coreboot/+/56726
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If0e7f076bb7e3b630c3ab9d28071312d3e21e927
Gerrit-Change-Number: 56726
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
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-Comment-Date: Fri, 30 Jul 2021 23:25:07 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/56693 )
Change subject: soc/amd/common/block/include/gpio_banks: use gpio_t for gpio numbers
......................................................................
soc/amd/common/block/include/gpio_banks: use gpio_t for gpio numbers
With the addition of the remote GPIO support, the GPIO number won't fit
into 8 bit any more, so use the gpio_t type instead which is an uint32_t
typedef.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: I3de93fd3a2f2af3c1e3b335fef84019c56482051
Reviewed-on: https://review.coreboot.org/c/coreboot/+/56693
Reviewed-by: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
---
M src/soc/amd/common/block/gpio_banks/gpio.c
M src/soc/amd/common/block/include/amdblocks/gpio_banks.h
2 files changed, 6 insertions(+), 6 deletions(-)
Approvals:
build bot (Jenkins): Verified
Marshall Dawson: Looks good to me, approved
diff --git a/src/soc/amd/common/block/gpio_banks/gpio.c b/src/soc/amd/common/block/gpio_banks/gpio.c
index 85f2736..108635a 100644
--- a/src/soc/amd/common/block/gpio_banks/gpio.c
+++ b/src/soc/amd/common/block/gpio_banks/gpio.c
@@ -15,7 +15,7 @@
#include <assert.h>
#include <string.h>
-static int get_gpio_gevent(uint8_t gpio, const struct soc_amd_event *table,
+static int get_gpio_gevent(gpio_t gpio, const struct soc_amd_event *table,
size_t items)
{
int i;
diff --git a/src/soc/amd/common/block/include/amdblocks/gpio_banks.h b/src/soc/amd/common/block/include/amdblocks/gpio_banks.h
index 098208b..d17aa7a 100644
--- a/src/soc/amd/common/block/include/amdblocks/gpio_banks.h
+++ b/src/soc/amd/common/block/include/amdblocks/gpio_banks.h
@@ -6,15 +6,17 @@
#include <types.h>
#include "gpio_defs.h"
+typedef uint32_t gpio_t;
+
struct soc_amd_gpio {
- uint8_t gpio;
+ gpio_t gpio;
uint8_t function;
uint32_t control;
uint32_t flags;
};
struct soc_amd_event {
- uint8_t gpio;
+ gpio_t gpio;
uint8_t event;
};
@@ -24,7 +26,7 @@
/* Number of wake_gpio with a valid setting. */
uint32_t num_valid_wake_gpios;
/* GPIO index number that caused a wake. */
- uint8_t wake_gpios[16];
+ gpio_t wake_gpios[16];
};
/* Fill gpio_wake_state object for future event reporting. */
@@ -52,8 +54,6 @@
return (flags & GPIO_FLAG_EVENT_ACTIVE_MASK) == GPIO_FLAG_EVENT_ACTIVE_LOW;
}
-typedef uint32_t gpio_t;
-
/*
* gpio_configure_pads_with_override accepts as input two GPIO tables:
* 1. Base config
--
To view, visit https://review.coreboot.org/c/coreboot/+/56693
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3de93fd3a2f2af3c1e3b335fef84019c56482051
Gerrit-Change-Number: 56693
Gerrit-PatchSet: 3
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: 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-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: merged
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/56675 )
Change subject: soc/amd/common/block/include/acpimmio_map: drop unused GPIO bank defines
......................................................................
soc/amd/common/block/include/acpimmio_map: drop unused GPIO bank defines
The offsets of all GPIOs in the up to four regular banks are all
calculated relatively to ACPIMMIO_GPIO0_BANK, so we can just drop the
unused defines for ACPIMMIO_GPIO1_BANK and ACPIMMIO_GPIO2_BANK.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: I832ffdca479c1f07219a23b4a7f9be69322dfe03
Reviewed-on: https://review.coreboot.org/c/coreboot/+/56675
Reviewed-by: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
---
M src/soc/amd/common/block/include/amdblocks/acpimmio_map.h
1 file changed, 0 insertions(+), 2 deletions(-)
Approvals:
build bot (Jenkins): Verified
Marshall Dawson: Looks good to me, approved
diff --git a/src/soc/amd/common/block/include/amdblocks/acpimmio_map.h b/src/soc/amd/common/block/include/amdblocks/acpimmio_map.h
index ebfc039..7081dbb 100644
--- a/src/soc/amd/common/block/include/amdblocks/acpimmio_map.h
+++ b/src/soc/amd/common/block/include/amdblocks/acpimmio_map.h
@@ -119,8 +119,6 @@
#define ACPIMMIO_MISC_BANK 0x0e00
#define ACPIMMIO_DPVGA_BANK 0x1400
#define ACPIMMIO_GPIO0_BANK 0x1500
-#define ACPIMMIO_GPIO1_BANK 0x1600
-#define ACPIMMIO_GPIO2_BANK 0x1700
#define ACPIMMIO_XHCIPM_BANK 0x1c00
#define ACPIMMIO_ACDCTMR_BANK 0x1d00
#define ACPIMMIO_AOAC_BANK 0x1e00
--
To view, visit https://review.coreboot.org/c/coreboot/+/56675
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I832ffdca479c1f07219a23b4a7f9be69322dfe03
Gerrit-Change-Number: 56675
Gerrit-PatchSet: 3
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: 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-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: merged
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/56674 )
Change subject: soc/amd/common/block/include/acpimmio_map: add GPIO bank 3 to table
......................................................................
soc/amd/common/block/include/acpimmio_map: add GPIO bank 3 to table
GPIO bank 3 isn't used in coreboot, but the existence is documented in
both the Picasso PPR #55570 Rev 3.16 and Cezanne PPR #56569 Rev 3.01 and
for those two SoCs all 4 banks are covered by the corresponding
Memory32Fixed region in the DSDT.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: Id444a97a398d7e3abfd1f5c4a32e762ee6ff68f1
Reviewed-on: https://review.coreboot.org/c/coreboot/+/56674
Reviewed-by: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
---
M src/soc/amd/common/block/include/amdblocks/acpimmio_map.h
1 file changed, 2 insertions(+), 0 deletions(-)
Approvals:
build bot (Jenkins): Verified
Marshall Dawson: Looks good to me, approved
diff --git a/src/soc/amd/common/block/include/amdblocks/acpimmio_map.h b/src/soc/amd/common/block/include/amdblocks/acpimmio_map.h
index 176dc2b..ebfc039 100644
--- a/src/soc/amd/common/block/include/amdblocks/acpimmio_map.h
+++ b/src/soc/amd/common/block/include/amdblocks/acpimmio_map.h
@@ -72,6 +72,8 @@
* +---------------------------------------------------------------------------+
* |0x1700 GPIO configuration registers bank 2 (following bank 1) |
* +---------------------------------------------------------------------------+
+ * |0x1800 GPIO configuration registers bank 3 (following bank 2) |
+ * +---------------------------------------------------------------------------+
* |0x1c00 xHCI Power Management registers |
* +---------------------------------------------------------------------------+
* |0x1d00 Wake device (AC DC timer) |
--
To view, visit https://review.coreboot.org/c/coreboot/+/56674
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Id444a97a398d7e3abfd1f5c4a32e762ee6ff68f1
Gerrit-Change-Number: 56674
Gerrit-PatchSet: 3
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: 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-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: merged