Hello Philippe Mathieu-Daudé, Subrata Banik, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/19467
to look at the new patch set (#6).
Change subject: pci_device: Write vendor ID to subsystem vendor ID
......................................................................
pci_device: Write vendor ID to subsystem vendor ID
Write vendor/device id to subsystem vendor/device id
if they are not provided.
Change-Id: I5027331a6adf9109767415ba22dfcb17b35ef54b
Signed-off-by: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
---
M src/device/pci_device.c
1 file changed, 9 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/19467/6
--
To view, visit https://review.coreboot.org/19467
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5027331a6adf9109767415ba22dfcb17b35ef54b
Gerrit-PatchSet: 6
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Philippe Mathieu-Daudé <philippe.mathieu.daude(a)gmail.com>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins)
Lee Leahy has posted comments on this change. ( https://review.coreboot.org/19300 )
Change subject: arch/x86: Share storage data structures between early stages
......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/#/c/19300/3/src/arch/x86/Kconfig
File src/arch/x86/Kconfig:
PS3, Line 244:
: # Allocate space in CAR to share the driver storage structures between stages
: config CAR_DRIVERS_STORAGE
: bool
: default n
> If we need this, could it go in drivers/storage/kconfig instead of here?
CAR_DRIVERS_STORAGE Kconfig value removed
https://review.coreboot.org/#/c/19300/3/src/arch/x86/car.ld
File src/arch/x86/car.ld:
PS3, Line 47: CONFIG_CAR_DRIVERS_STORAGE
> can this just be CONFIG_DRIVERS_STORAGE, and we always allocate it if we're
Using DRIVERS_STORAGE instead as requested.
--
To view, visit https://review.coreboot.org/19300
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I20a01b850a31df9887a428bf07ca476c8410d33e
Gerrit-PatchSet: 3
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Lee Leahy <leroy.p.leahy(a)intel.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)intel.com>
Gerrit-Reviewer: Lee Leahy <leroy.p.leahy(a)intel.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins)
Gerrit-HasComments: Yes
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/19300
to look at the new patch set (#4).
Change subject: arch/x86: Share storage data structures between early stages
......................................................................
arch/x86: Share storage data structures between early stages
Define a common area in CAR so that the storage data structures can be
shared between stages.
TEST=Build and run on Reef
Change-Id: I20a01b850a31df9887a428bf07ca476c8410d33e
Signed-off-by: Lee Leahy <Leroy.P.Leahy(a)intel.com>
---
M src/arch/x86/car.ld
1 file changed, 5 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/00/19300/4
--
To view, visit https://review.coreboot.org/19300
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I20a01b850a31df9887a428bf07ca476c8410d33e
Gerrit-PatchSet: 4
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Lee Leahy <leroy.p.leahy(a)intel.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)intel.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins)
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/19157 )
Change subject: amd/pi/00670F00: Add AMD PSP support for Stoney Ridge
......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/#/c/19157/1/src/northbridge/amd/pi/00670F00/psp…
File src/northbridge/amd/pi/00670F00/psp.c:
Line 86: ; /* wait for initialized and no command in progress */
> You can use struct stopwatch in timer.h. stopwatch_expired()
Difficult with AMD currently. stopwatch uses timer_monotonic_get() but all AMD use the LAPIC timer and that version of timer_monotonic_get() isn't present in romstage.
https://review.coreboot.org/#/c/19157/12/src/northbridge/amd/pi/00670F00/ps…
File src/northbridge/amd/pi/00670F00/psp.c:
PS12, Line 125:
> This reads better with the parens, I think. I know the precedence has & hig
agree
https://review.coreboot.org/#/c/19157/1/src/northbridge/amd/pi/00670F00/psp…
File src/northbridge/amd/pi/00670F00/psp.h:
PS1, Line 52: typedef struct {
: mbox_buffer_header header;
: } mbox_default_buffer;
:
: typedef union _mbox_buffer {
: mbox_default_buffer default_buff;
: u8 reserved[32];
: } mbox_buffer;
:
: typedef struct {
: mbox_buffer buffer[2];
: } unaligned_mbox_buffer;
> There's no need for these typedefs nor for the indirection. I'm just very c
I sort of like typdefs but OK.
Have changed how the alignment is done. The spec doesn't address alignment, but the reference code does it. Nor does the reference code explain why the buffers are padded to a minimum of 32 bytes. The comment seems intentional but no explanation is given. I'm a little leery about changing that.
> There's no need for struct mbox_default_buffer...
I agree it looks funny the way it is but as new commands and their buffer definitions are added, it may look better. An example that's not implemented yet will look something like:
struct smm_trigger_info {
u64 address;
u32 address_type;
/* more stuff */
};
struct smm_req_buffer {
u64 smm_base;
/* more stuff */
struct smm_trigger_info smm_trig_info;
};
struct smm_info_buffer { /* larger than 32, BTW */
struct mbox_buffer_header header;
struct smm_req_buffer req;
};
In general, I'd prefer the code match the spec even if it adds an extra layer in the structs.
--
To view, visit https://review.coreboot.org/19157
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf111470b261d6516b878e88be471967abe681b3
Gerrit-PatchSet: 1
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Marc Jones <marc(a)marcjonesconsulting.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins)
Gerrit-HasComments: Yes
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/19157
to look at the new patch set (#14).
Change subject: amd/pi/00670F00: Add AMD PSP support for Stoney Ridge
......................................................................
amd/pi/00670F00: Add AMD PSP support for Stoney Ridge
Add PSP communication support that is not covered by AGESA.
The PSP must be notified DRAM is ready after amdinitpost().
Move amd_initcpuio() to the end of amdinitpost() so that the PSP
decode for notify works.
Change-Id: Icf111470b261d6516b878e88be471967abe681b3
Signed-off-by: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Signed-off-by: Marc Jones <marcj303(a)gmail.com>
---
M src/cpu/amd/pi/00670F00/romstage.c
M src/northbridge/amd/pi/00670F00/Makefile.inc
A src/northbridge/amd/pi/00670F00/psp.c
A src/northbridge/amd/pi/00670F00/psp.h
M src/northbridge/amd/pi/agesawrapper.c
5 files changed, 302 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/19157/14
--
To view, visit https://review.coreboot.org/19157
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icf111470b261d6516b878e88be471967abe681b3
Gerrit-PatchSet: 14
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Marc Jones <marc(a)marcjonesconsulting.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins)