Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40385 )
Change subject: soc/intel/xeon_sp/cpx: configure FSP-M UPD parameters
......................................................................
Patch Set 22:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40385/21/src/soc/intel/xeon_sp/cpx…
File src/soc/intel/xeon_sp/cpx/romstage.c:
https://review.coreboot.org/c/coreboot/+/40385/21/src/soc/intel/xeon_sp/cpx…
PS21, Line 24: arch_upd->StackBase = (void *) 0xfe930000;
> Can we do this as part of the device tree and not hardcode the configuration?
We have an IPS ticket with Intel. Intel agreed that this is a violation of FSP spec, and thus it will be fixed in near future (once we clear the critical issues). At that time, this part of code (hardcoding StackBase and StackSize) will be reverted, to use FSP_USES_CB_STACK instead.
--
To view, visit https://review.coreboot.org/c/coreboot/+/40385
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I2d0762a742d8803c7396034e3244120c1e8ece67
Gerrit-Change-Number: 40385
Gerrit-PatchSet: 22
Gerrit-Owner: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Reviewer: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com>
Gerrit-Reviewer: Jingle Hsu <jingle_hsu(a)wiwynn.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Morgan Jang
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Fri, 05 Jun 2020 00:56:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-MessageType: comment
Hello Philipp Deppenwiese, build bot (Jenkins), Anjaneya "Reddy" Chagam, Johnny Lin, Jingle Hsu, Morgan Jang, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40383
to look at the new patch set (#21).
Change subject: soc/intel/xeon_sp/cpx: fix MADT ACPI table
......................................................................
soc/intel/xeon_sp/cpx: fix MADT ACPI table
Fix MADT table generation to keep IIO stack design in consideration.
Signed-off-by: Jonathan Zhang <jonzhang(a)fb.com>
Signed-off-by: Reddy Chagam <anjaneya.chagam(a)intel.com>
Change-Id: If1bf6e39db545e227e9867aa8d24f7db1d820216
---
M src/soc/intel/xeon_sp/cpx/acpi.c
M src/soc/intel/xeon_sp/cpx/include/soc/acpi.h
M src/soc/intel/xeon_sp/cpx/include/soc/pci_devs.h
3 files changed, 106 insertions(+), 20 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/40383/21
--
To view, visit https://review.coreboot.org/c/coreboot/+/40383
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If1bf6e39db545e227e9867aa8d24f7db1d820216
Gerrit-Change-Number: 40383
Gerrit-PatchSet: 21
Gerrit-Owner: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Reviewer: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com>
Gerrit-Reviewer: Jingle Hsu <jingle_hsu(a)wiwynn.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Morgan Jang
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-MessageType: newpatchset
Nathaniel L Desimone has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41728 )
Change subject: drivers/intel/fsp2_0: Add FSP 2.2 specific support
......................................................................
Patch Set 9: -Code-Review
(1 comment)
https://review.coreboot.org/c/coreboot/+/41728/9/src/drivers/intel/fsp2_0/s…
File src/drivers/intel/fsp2_0/silicon_init.c:
https://review.coreboot.org/c/coreboot/+/41728/9/src/drivers/intel/fsp2_0/s…
PS9, Line 46: !=
> if you made this == and returned here the indent could be removed and strings wouldn't need to be aw […]
This isn't the correct way to test for success anyway. It should be if(!EFI_ERROR(status)). It is possible to have "successful" error codes other than EFI_SUCCESS. The MSB indicates error.
--
To view, visit https://review.coreboot.org/c/coreboot/+/41728
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If7177a267f3a9b4cbb60a639f1c737b9a3341913
Gerrit-Change-Number: 41728
Gerrit-PatchSet: 9
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Reviewer: Nathaniel L Desimone <nathaniel.l.desimone(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Reviewer: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com>
Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-CC: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Comment-Date: Thu, 04 Jun 2020 22:03:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Duncan Laurie <dlaurie(a)chromium.org>
Gerrit-MessageType: comment
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42072 )
Change subject: libpayload: drivers/usb: add a USB pre-poll hook
......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42072/2/payloads/libpayload/includ…
File payloads/libpayload/include/usb/usb.h:
https://review.coreboot.org/c/coreboot/+/42072/2/payloads/libpayload/includ…
PS2, Line 352: void __attribute__((weak)) usb_poll_prepare (void);
> space prohibited between function name and open parenthesis '('
Ack
--
To view, visit https://review.coreboot.org/c/coreboot/+/42072
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic6c23756f64b891b3c5683cd650c605b8630b0fb
Gerrit-Change-Number: 42072
Gerrit-PatchSet: 2
Gerrit-Owner: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Thu, 04 Jun 2020 21:50:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: comment
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42072 )
Change subject: libpayload: drivers/usb: add a USB pre-poll hook
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/42072
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic6c23756f64b891b3c5683cd650c605b8630b0fb
Gerrit-Change-Number: 42072
Gerrit-PatchSet: 2
Gerrit-Owner: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Thu, 04 Jun 2020 21:46:45 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42072 )
Change subject: libpayload: drivers/usb: add a USB pre-poll hook
......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42072/2/payloads/libpayload/includ…
File payloads/libpayload/include/usb/usb.h:
https://review.coreboot.org/c/coreboot/+/42072/2/payloads/libpayload/includ…
PS2, Line 352: void __attribute__((weak)) usb_poll_prepare (void);
space prohibited between function name and open parenthesis '('
--
To view, visit https://review.coreboot.org/c/coreboot/+/42072
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic6c23756f64b891b3c5683cd650c605b8630b0fb
Gerrit-Change-Number: 42072
Gerrit-PatchSet: 2
Gerrit-Owner: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Thu, 04 Jun 2020 21:38:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42072 )
Change subject: libpayload: drivers/usb: add a USB pre-poll hook
......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/42072/1//COMMIT_MSG
Commit Message:
https://review.coreboot.org/c/coreboot/+/42072/1//COMMIT_MSG@7
PS1, Line 7: drivers/usb: add a USB pre-poll hook
> libpayload should be in the prefix. Maybe: […]
Done
https://review.coreboot.org/c/coreboot/+/42072/1//COMMIT_MSG@14
PS1, Line 14: hookink
> hooking?
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/42072
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic6c23756f64b891b3c5683cd650c605b8630b0fb
Gerrit-Change-Number: 42072
Gerrit-PatchSet: 2
Gerrit-Owner: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Thu, 04 Jun 2020 21:37:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-MessageType: comment
Hello build bot (Jenkins), Nico Huber, Tim Wawrzynczak, Julius Werner,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42072
to look at the new patch set (#2).
Change subject: libpayload: drivers/usb: add a USB pre-poll hook
......................................................................
libpayload: drivers/usb: add a USB pre-poll hook
This adds a hook so that a payload can optionally perform USB service
functions in conjunction with regular USB port status polling. In
particular, this allows depthcharge to control the state of an
external USB mux. Some SoCs like Tiger Lake have a USB mux for Type-C
ports that must be kept in sync with the state of the port as reported
by the TCPC. This can be achieved by hooking into the poll routine to
refresh the state of the USB mux.
BUG=b:149883933
TEST=booted into recovery from Type-C flash drive on volteer
Change-Id: Ic6c23756f64b891b3c5683cd650c605b8630b0fb
Signed-off-by: Caveh Jalali <caveh(a)chromium.org>
---
M payloads/libpayload/drivers/usb/usb.c
M payloads/libpayload/include/usb/usb.h
2 files changed, 11 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/42072/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/42072
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic6c23756f64b891b3c5683cd650c605b8630b0fb
Gerrit-Change-Number: 42072
Gerrit-PatchSet: 2
Gerrit-Owner: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-MessageType: newpatchset
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42072 )
Change subject: drivers/usb: add a USB pre-poll hook
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/42072/1//COMMIT_MSG
Commit Message:
https://review.coreboot.org/c/coreboot/+/42072/1//COMMIT_MSG@10
PS1, Line 10: In
: particular, this allows depthcharge to control the state of an
: external USB mux. Some SoCs like Tiger Lake have a USB mux for Type-C
: ports that must be kept in sync with the state of the port as reported
: by the TCPC. This can be achieved by hookink into the poll routine to
: refresh the state of the USB mux.
> This explains the problem to solve. But not why it's done in […]
I think the short answer is that the mux is controlled by the Chrome EC on the board, and the driver to communicate with that is in depthcharge. And I think the reason that driver is in depthcharge originally was that it was based off GPL code that couldn't be added to libpayload.
I agree that the general situation of these driver splits is far from great (also in other cases, like SPI flash and the whole libpayload_init_default_cbfs_media() mess) and it would be much better to have most of it in libpayload. But coreboot kinda forced this situation by insisting that libpayload can only be BSD code. We have CONFIG_LP_GPL now (which didn't exist back then) which could be used to move more GPL drivers into libpayload, but this is all tied up in 10 years worth of glue code and frameworks in depthcharge by now and I don't think anyone has time to migrate the world into another repository. So we are kinda stuck where we are with this.
--
To view, visit https://review.coreboot.org/c/coreboot/+/42072
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic6c23756f64b891b3c5683cd650c605b8630b0fb
Gerrit-Change-Number: 42072
Gerrit-PatchSet: 1
Gerrit-Owner: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Thu, 04 Jun 2020 20:51:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment