Attention is currently required from: Mike Banon.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57319 )
Change subject: src/device/Kconfig: introduce the AMD_DGPU_WITHOUT_EEPROM symbols
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
How is the situation different from that for nvidia?
--
To view, visit https://review.coreboot.org/c/coreboot/+/57319
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I424f93961c9d4a856dd6f5285417597cb117084d
Gerrit-Change-Number: 57319
Gerrit-PatchSet: 2
Gerrit-Owner: Mike Banon <mikebdp2(a)gmail.com>
Gerrit-Reviewer: awokd(a)danwin1210.me
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Mike Banon <mikebdp2(a)gmail.com>
Gerrit-Comment-Date: Fri, 03 Sep 2021 15:42:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Tim Wawrzynczak, Nick Vaccaro.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57069 )
Change subject: coreboot tables: Add type-c port info to coreboot table
......................................................................
Patch Set 16:
(3 comments)
File payloads/libpayload/include/sysinfo.h:
https://review.coreboot.org/c/coreboot/+/57069/comment/35523ee9_08a56639
PS16, Line 49: /* The orientation of the signal follows the orientation of the CC lines. */
: #define SYSINFO_TYPEC_ORIENTATION_FOLLOW_CC 0
:
: /* The orientation of the signal is fixed to follow CC1 */
: #define SYSINFO_TYPEC_ORIENTATION_NORMAL 1
:
: /* The orientation of the signal is fixed to follow CC2 */
: #define SYSINFO_TYPEC_ORIENTATION_REVERSE 2
Shouldn't this be part of `coreboot_tables.h`?
https://review.coreboot.org/c/coreboot/+/57069/comment/c44353e4_3e5f64bd
PS16, Line 172: struct {
: uint8_t usb2_port_number;
: uint8_t usb3_port_number;
: uint8_t sbu_orientation;
: uint8_t data_orientation;
Can we re-use the struct from `coreboot_tables.h`?
File src/lib/coreboot_table.c:
https://review.coreboot.org/c/coreboot/+/57069/comment/d7d7db14_68139065
PS16, Line 263: rec->size = sizeof(*rec);
Doesn't this need to include the ports?
--
To view, visit https://review.coreboot.org/c/coreboot/+/57069
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ice732be2fa634dbf31ec620552b383c4a5b41451
Gerrit-Change-Number: 57069
Gerrit-PatchSet: 16
Gerrit-Owner: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Zhuohao Lee <zhuohao(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Comment-Date: Fri, 03 Sep 2021 15:41:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/56808 )
Change subject: mb/pcengines/apu2/gpio_ftns.h: add comment about GPIO numbers
......................................................................
mb/pcengines/apu2/gpio_ftns.h: add comment about GPIO numbers
The mapping of the package GPIO numbers to the GPIO numbers on the GPIO
controller isn't a 1:1 one, so add a comment about that to avoid
confusion. Also change the comment style to match the style guide.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: Ie30bf5483ea2e2516d7e3fdd21ea9338362e526e
Reviewed-on: https://review.coreboot.org/c/coreboot/+/56808
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Michał Żygowski <michal.zygowski(a)3mdeb.com>
---
M src/mainboard/pcengines/apu2/gpio_ftns.h
1 file changed, 6 insertions(+), 5 deletions(-)
Approvals:
build bot (Jenkins): Verified
Michał Żygowski: Looks good to me, approved
diff --git a/src/mainboard/pcengines/apu2/gpio_ftns.h b/src/mainboard/pcengines/apu2/gpio_ftns.h
index 95c744f..6386220 100644
--- a/src/mainboard/pcengines/apu2/gpio_ftns.h
+++ b/src/mainboard/pcengines/apu2/gpio_ftns.h
@@ -5,11 +5,12 @@
int get_spd_offset(void);
-//
-// Based on PC Engines APU2C and APU3A schematics
-// http://www.pcengines.ch/schema/apu2c.pdf
-// http://www.pcengines.ch/schema/apu3a.pdf
-//
+/*
+ * Based on PC Engines APU2C and APU3A schematics
+ * http://www.pcengines.ch/schema/apu2c.pdf
+ * http://www.pcengines.ch/schema/apu3a.pdf
+ * Beware that the GPIO pin numbers on the package don't match the internal GPIO numbers
+ */
#define GPIO_22 0x09 // MODESW (APU5)
#define GPIO_32 0x59 // MODESW (SIMSWAP2 on APU5)
#define GPIO_33 0x5A // SIMSWAP (SIMSWAP3 on APU5)
--
To view, visit https://review.coreboot.org/c/coreboot/+/56808
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ie30bf5483ea2e2516d7e3fdd21ea9338362e526e
Gerrit-Change-Number: 56808
Gerrit-PatchSet: 2
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: merged
Attention is currently required from: Piotr Król.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56784 )
Change subject: mb/pcengines/apu2/romstage: use proper GPIO configuration API
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS2:
> Works well, no issues detected by our testing suites. Felix you may add […]
thanks for testing this patch!
--
To view, visit https://review.coreboot.org/c/coreboot/+/56784
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If121941c8a6ba88913653192740997aeef426548
Gerrit-Change-Number: 56784
Gerrit-PatchSet: 3
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Piotr Król <piotr.krol(a)3mdeb.com>
Gerrit-Comment-Date: Fri, 03 Sep 2021 15:40:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-MessageType: comment
Attention is currently required from: Felix Held, Piotr Król.
Hello build bot (Jenkins), Michał Żygowski, Piotr Król,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/56784
to look at the new patch set (#3).
Change subject: mb/pcengines/apu2/romstage: use proper GPIO configuration API
......................................................................
mb/pcengines/apu2/romstage: use proper GPIO configuration API
Also remove the unused amdblocks/acpimmio.h include in gpio_ftns.c.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Tested-by: Michał Kopeć <michal.kopec(a)3mdeb.com>
Change-Id: If121941c8a6ba88913653192740997aeef426548
---
M src/mainboard/pcengines/apu2/gpio_ftns.c
M src/mainboard/pcengines/apu2/romstage.c
2 files changed, 44 insertions(+), 54 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/56784/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/56784
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If121941c8a6ba88913653192740997aeef426548
Gerrit-Change-Number: 56784
Gerrit-PatchSet: 3
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Attention: Piotr Król <piotr.krol(a)3mdeb.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Felix Held.
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56779 )
Change subject: soc/amd/common/block/i2c: move raw GPIO access functions to gpio_banks
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/56779
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I84dff381ad86e0c7f879f0f079186aec9cafc604
Gerrit-Change-Number: 56779
Gerrit-PatchSet: 2
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: Martin Roth <martinroth(a)google.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-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Fri, 03 Sep 2021 15:28:17 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Tim Wawrzynczak, Nick Vaccaro, Patrick Rudolph.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57345 )
Change subject: driver/intel/pmc_mux/conn: add conn_get_type_c_list()
......................................................................
Patch Set 3: Code-Review+1
(1 comment)
File src/drivers/intel/pmc_mux/conn/conn.c:
https://review.coreboot.org/c/coreboot/+/57345/comment/4682313d_06ad7163
PS3, Line 117: mux = pmc->link_list->children;
It could be any of the children, right? For the moment there is only
one, I suppose? Maybe at least leave a comment that one could check
for `drivers_intel_pmc_mux_ops` in case they are looking for it.
--
To view, visit https://review.coreboot.org/c/coreboot/+/57345
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic56a1ad1b617e3af000664147d21165e6ea3a742
Gerrit-Change-Number: 57345
Gerrit-PatchSet: 3
Gerrit-Owner: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Fri, 03 Sep 2021 15:26:07 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment