Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Fred Reitberger.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61558 )
Change subject: soc/amd/common/block/psp: introduce AMD_SOC_SEPARATE_EFS_SECTION
......................................................................
Patch Set 1:
(1 comment)
File src/soc/amd/common/block/psp/efs_fmap_check.c:
https://review.coreboot.org/c/coreboot/+/61558/comment/ce9799a2_30cd7751
PS1, Line 7: FMAP_SECTION_FLASH_START
hmm, or should we use FLASH_BASE_ADDR here? i'm not sure if it's guaranteed that both are equivalent and if not if we should also have an assert for that here
--
To view, visit https://review.coreboot.org/c/coreboot/+/61558
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I5ed0f76c9c9c9c180ee5f1b96f88689d0979bb5e
Gerrit-Change-Number: 61558
Gerrit-PatchSet: 1
Gerrit-Owner: Fred Reitberger <reitbergerfred(a)gmail.com>
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-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: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Comment-Date: Mon, 07 Feb 2022 17:54:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Subrata Banik, Nick Vaccaro.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61217 )
Change subject: mb/google/brya/var/agah: Select PCIEXP_SUPPORT_RESIZABLE_BARS
......................................................................
Patch Set 1:
(1 comment)
File src/mainboard/google/brya/Kconfig.name:
https://review.coreboot.org/c/coreboot/+/61217/comment/b44b4c90_67e07905
PS1, Line 10: PCIEXP_SUPPORT_RESIZABLE_BARS
> NIT - might as well alphabetize this one like you did with DRIVERS_I2C_MAX98390.
Oops, I know my alphabet!
--
To view, visit https://review.coreboot.org/c/coreboot/+/61217
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9cf86ba3160ae5018655b5d366e89f4273b30b94
Gerrit-Change-Number: 61217
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Comment-Date: Mon, 07 Feb 2022 17:35:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Subrata Banik, Tim Wawrzynczak.
Hello build bot (Jenkins), Subrata Banik, Nick Vaccaro,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/61217
to look at the new patch set (#2).
Change subject: mb/google/brya/var/agah: Select PCIEXP_SUPPORT_RESIZABLE_BARS
......................................................................
mb/google/brya/var/agah: Select PCIEXP_SUPPORT_RESIZABLE_BARS
The google/agah variant will use a peripheral that will require the use
of the PCIe Resizable BAR feature from the PCIe spec. Thus, select
the new Kconfig option to enable it. The appropriate Resizable BAR size
will be updated later.
BUG=b:214443809
TEST=build
Change-Id: I9cf86ba3160ae5018655b5d366e89f4273b30b94
Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
---
M src/mainboard/google/brya/Kconfig.name
1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/61217/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/61217
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9cf86ba3160ae5018655b5d366e89f4273b30b94
Gerrit-Change-Number: 61217
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Nico Huber.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61682 )
Change subject: nb/amd/common: Move RAM calcualtion to common code
......................................................................
Patch Set 1:
(4 comments)
File src/northbridge/amd/common/ram_calc.c:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-140189):
https://review.coreboot.org/c/coreboot/+/61682/comment/6fe2f82a_e28951dd
PS1, Line 25: /* [N-1:24] at [N-1-8:16], if N < 40 addres_mask will clear unused high bits */
'addres' may be misspelled - perhaps 'address'?
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-140189):
https://review.coreboot.org/c/coreboot/+/61682/comment/a9777811_58c8b1f8
PS1, Line 29: /* [N-1:40] at [N-1-40:0], addres_mask will clear unused high bits */
'addres' may be misspelled - perhaps 'address'?
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-140189):
https://review.coreboot.org/c/coreboot/+/61682/comment/890e6413_565fdcb2
PS1, Line 40: /* [N-1:24] at [N-1-8:16], if N < 40 addres_mask will clear unused high bits */
'addres' may be misspelled - perhaps 'address'?
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-140189):
https://review.coreboot.org/c/coreboot/+/61682/comment/8713f7db_81530f59
PS1, Line 45: /* [N-1:40] at [N-1-40:0] addres_mask will clear unused high bits */
'addres' may be misspelled - perhaps 'address'?
--
To view, visit https://review.coreboot.org/c/coreboot/+/61682
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3d222f6cbd26fe78d968d2883984f25e8b62ab5c
Gerrit-Change-Number: 61682
Gerrit-PatchSet: 1
Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Comment-Date: Mon, 07 Feb 2022 17:34:43 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Reka Norman, Nick Vaccaro, Tim Wawrzynczak, Karthik Ramasubramanian.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61639 )
Change subject: util/spd_tools/spd_gen/lp5: Update BusWidth Encoding
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/61639
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ia12a5bd8f70a70ca8a510ecf00f6268c6904ec25
Gerrit-Change-Number: 61639
Gerrit-PatchSet: 1
Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Reka Norman <rekanorman(a)chromium.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Karthikeyan Ramasubramanian <kramasub(a)chromium.org>
Gerrit-Attention: Reka Norman <rekanorman(a)chromium.org>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)google.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Comment-Date: Mon, 07 Feb 2022 17:32:49 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Jason Glenesk, Marshall Dawson, Felix Held.
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61681 )
Change subject: soc/amd/sabrina/Kconfig: remove SOC_AMD_COMMON_BLOCK_PCI_MMCONF TODO
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/61681
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7e3064bab5ca1e277b04f9aae98f9adabce75399
Gerrit-Change-Number: 61681
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-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: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Mon, 07 Feb 2022 17:31:37 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Rory Liu, Zhuohao Lee.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61267 )
Change subject: drivers/net/r8168: Add ASPM control mechanism
......................................................................
Patch Set 3:
(3 comments)
File src/drivers/net/r8168.c:
https://review.coreboot.org/c/coreboot/+/61267/comment/7e38dde7_fc15bbee
PS2, Line 252: struct drivers_net_config *config = dev->chip_info;
: if (!config || !config->enable_aspm)
: return;
> Should we just change this to […]
Done
File src/drivers/net/r8168.c:
https://review.coreboot.org/c/coreboot/+/61267/comment/f05d925e_107b8341
PS3, Line 40: ASPM_L1_2 0xe059000f
nit:
```
#define ASPM_L1_2_MASK 0xe059000f
```
so it is indented one space after CMD_REG_ASPM
https://review.coreboot.org/c/coreboot/+/61267/comment/f4ba8aeb_3502d42c
PS3, Line 359: /* Enable ASPM_L1.2 */
: if (CONFIG(PCIEXP_ASPM))
: enable_aspm_l1_2(io_base);
Is this applicable to both the 8168 and 8125 ?
--
To view, visit https://review.coreboot.org/c/coreboot/+/61267
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I944dbf04d3ca19df4de224540bee538bff4d1f12
Gerrit-Change-Number: 61267
Gerrit-PatchSet: 3
Gerrit-Owner: Rory Liu <rory.liu(a)quanta.corp-partner.google.com>
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: Alan Huang <alan-huang(a)quanta.corp-partner.google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Rory Liu <rory.liu(a)quanta.corp-partner.google.com>
Gerrit-Attention: Zhuohao Lee <zhuohao(a)google.com>
Gerrit-Comment-Date: Mon, 07 Feb 2022 17:31:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Jason Glenesk, Marshall Dawson, Felix Held.
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61680 )
Change subject: soc/amd/common/block/pci/amd_pci_mmconf: add assert for MMCONF region
......................................................................
Patch Set 1:
(1 comment)
File src/soc/amd/common/block/pci/amd_pci_mmconf.c:
https://review.coreboot.org/c/coreboot/+/61680/comment/db13aa7e_e4355742
PS1, Line 15: CONFIG_ECAM_MMCONF_BASE_ADDRESS
Is it even possible for this #define to contain a 64 bit number? Doesn't the constant need the LL suffix to be considered a 64-bit number?
--
To view, visit https://review.coreboot.org/c/coreboot/+/61680
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib0b4443d7a86f20c4c2508a88db5d731560f8312
Gerrit-Change-Number: 61680
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-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: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Mon, 07 Feb 2022 17:31:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans.
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52922 )
Change subject: nb/amd/{agesa,pi}: Avoid overflows during DRAM calculation
......................................................................
Patch Set 4:
(9 comments)
File src/northbridge/amd/agesa/family14/northbridge.c:
https://review.coreboot.org/c/coreboot/+/52922/comment/803722c9_b81c643c
PS3, Line 573: sizek = limitk - 768;
> I think this was missing the `+ 1`. Which is implicitly fixed by making […]
Most likely
https://review.coreboot.org/c/coreboot/+/52922/comment/b126bad6_69b6d469
PS3, Line 115: [39:24] at [31:16]
> BKDG says [35:24] at [27:16]. This would also be what the code masks below.
Changed the comments in the new patch.
https://review.coreboot.org/c/coreboot/+/52922/comment/91af4f57_ca379c94
PS3, Line 119: {DramBase[35:24], 00_0000h} <= address[35:0]
> How to calculate an 8 from this? I'm guessing by taking the masked bits into […]
Please see if the new patch is more clear about it
https://review.coreboot.org/c/coreboot/+/52922/comment/338766a3_0ff39eca
PS3, Line 120: convert to KiB by shifting 10 bits left
> Please use `/ KiB` (commonlib/helpers.h) instead.
Added
https://review.coreboot.org/c/coreboot/+/52922/comment/2484a881_7896ceb0
PS3, Line 127: [39:24] at [31:16]
> BKDG mentions [39:36] as 0, and the mask below only covers the other bits.
Added a full 64bit mask based on cpu_phys_address_size which should mask unused bit appropriately
File src/northbridge/amd/agesa/family15tn/northbridge.c:
https://review.coreboot.org/c/coreboot/+/52922/comment/a497f221_0500f176
PS3, Line 47: mask out DramMask [26:24] too
> Any idea why the original code did this? It was basically forcing 128MiB granularity.
I have no idea. This was one of the comments which led me to think something is wrong here.
https://review.coreboot.org/c/coreboot/+/52922/comment/72ca5bda_581b7f0c
PS3, Line 710: sizek = limitk - basek;
> This seems off by one granularity step (128MiB). […]
It seems so.
https://review.coreboot.org/c/coreboot/+/52922/comment/60deacf2_3415948f
PS3, Line 123: = *basek |
> Nit, `|=`.
Done in new patch
File src/northbridge/amd/pi/00730F01/northbridge.c:
https://review.coreboot.org/c/coreboot/+/52922/comment/c4481424_30d0ec61
PS3, Line 135: }
> This looks familiar. If it's the same function again, please consider to not […]
Moved to common code in new patchset
--
To view, visit https://review.coreboot.org/c/coreboot/+/52922
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3b5c1df96c308ff50c8de104e213219a98f25e10
Gerrit-Change-Number: 52922
Gerrit-PatchSet: 4
Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Mon, 07 Feb 2022 17:21:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment