Attention is currently required from: Angel Pons, Arthur Heymans.
Hello Felix Singer, build bot (Jenkins), Angel Pons, Arthur Heymans,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/61540
to look at the new patch set (#5).
Change subject: mb/asus/p8x7x-series: Refactor mainboard_get_spd()
......................................................................
mb/asus/p8x7x-series: Refactor mainboard_get_spd()
This mainboard-specific hook of Sandy/Ivy native RAM init sequence,
likely generated by autoport, is the same for all variants.
Consolidate them into a separate unit at the baseboard level.
The refactored routine is declared __weak, allowing future variants to
override it should they deviate from the norm.
Build tested on all variants.
Change-Id: I3b9c616a6c722e1f0fc124ced225efdcadb46b25
Signed-off-by: Keith Hui <buurin(a)gmail.com>
---
M src/mainboard/asus/p8x7x-series/Makefile.inc
A src/mainboard/asus/p8x7x-series/early_init.c
M src/mainboard/asus/p8x7x-series/variants/p8c_ws/early_init.c
M src/mainboard/asus/p8x7x-series/variants/p8h77-v/early_init.c
M src/mainboard/asus/p8x7x-series/variants/p8z77-m/early_init.c
M src/mainboard/asus/p8x7x-series/variants/p8z77-m_pro/early_init.c
M src/mainboard/asus/p8x7x-series/variants/p8z77-v/early_init.c
M src/mainboard/asus/p8x7x-series/variants/p8z77-v_lx2/early_init.c
8 files changed, 31 insertions(+), 54 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/61540/5
--
To view, visit https://review.coreboot.org/c/coreboot/+/61540
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3b9c616a6c722e1f0fc124ced225efdcadb46b25
Gerrit-Change-Number: 61540
Gerrit-PatchSet: 5
Gerrit-Owner: Keith Hui <buurin(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: newpatchset
Attention is currently required from: Zheng Bao, Felix Held.
Bao Zheng has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/66852 )
Change subject: amdfwtool: Move the filling of table headers into functions
......................................................................
Patch Set 10:
(2 comments)
File util/amdfwtool/amdfwtool.c:
https://review.coreboot.org/c/coreboot/+/66852/comment/a38592f8_ba7c4af7
PS10, Line 690: fill_psp_directory
fill_psp_directory_efs
https://review.coreboot.org/c/coreboot/+/66852/comment/d2ab790d_b23f7022
PS10, Line 716: fill_bios_directory
fill_bios_directory_to_efs
--
To view, visit https://review.coreboot.org/c/coreboot/+/66852
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib02c68c9f2ea84020b12682c41fb1a6f8f93d725
Gerrit-Change-Number: 66852
Gerrit-PatchSet: 10
Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Zheng Bao
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Zheng Bao
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Thu, 02 Mar 2023 04:47:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Angel Pons, Arthur Heymans.
Keith Hui has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61540 )
Change subject: mb/asus/p8x7x-series: Refactor mainboard_get_spd()
......................................................................
Patch Set 4:
(1 comment)
Patchset:
PS4:
All, let's start that weakness discussion right now.
As far as precedents go, it has already been set. 254 C files in the tree (including those being discussed here) defined weak functions. Plus i945 northbridge also has 3 weak functions, one of them about SPD, almost exactly the use case here. So does i440bx, with weak hooks to enable/disable SPD, with the only user being asus/p3b-f because it does need them. (I did for a time try to deprecate this one and move the offending code elsewhere.) I have another patch train up consolidating a MRC raminit hook on SNB/IVB as well with only one board overriding, and it was Angel who said it benefits from having a weak default. These are but just a few examples of weak functions.
If weak functions are not to be used, let's look at a few alternatives:
1. Refactor code of concern into separate compilation units and switch between them with Kconfig options and Makefile.inc. This in essence is how board variants work. Does add some complexity to the makefiles. Might as well abandon this patch.
2. Conditionals in the function by board variant or # of RAM slots (which still needs to be defined somewhere). Compiler and the next programmer looking will have to sort through the inevitable rats nest.
3. Leave out the __weak and leave it to the guy porting a P8x7x with less than 4 RAM slots (eg. P8H77-I, ITX with 2 slots) to patch the override. This could just be kicking the ball down the road, and his patch could be more ugly.
4. Leave out the __weak and force all current and future P8x7x variants to try reading 4 sets of SPD data. Current variants are not the problem (otherwise this patch would not exist) but future <4 slots boards would have useless work done in runtime.
--
To view, visit https://review.coreboot.org/c/coreboot/+/61540
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3b9c616a6c722e1f0fc124ced225efdcadb46b25
Gerrit-Change-Number: 61540
Gerrit-PatchSet: 4
Gerrit-Owner: Keith Hui <buurin(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Thu, 02 Mar 2023 04:39:11 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Zheng Bao, Fred Reitberger, Felix Held.
Bao Zheng has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/66856 )
Change subject: amdfwtool:combo: Add combo feature for BIOS table
......................................................................
Patch Set 7:
(1 comment)
File util/amdfwtool/amdfwtool.c:
https://review.coreboot.org/c/coreboot/+/66856/comment/e2b4a12d_981af149
PS6, Line 1980: BUFF_TO_RUN(
> should be rebased on […]
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/66856
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If0523a4a0e1f31969e4bbaa6062dcc0f2d6da420
Gerrit-Change-Number: 66856
Gerrit-PatchSet: 7
Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Zheng Bao
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Zheng Bao
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Thu, 02 Mar 2023 04:01:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Bao Zheng <fishbaozi(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Paul Menzel, Zheng Bao.
Bao Zheng has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/66855 )
Change subject: amdfwtool: Separate two cases of combo and non-combo clearly
......................................................................
Patch Set 9:
(1 comment)
Patchset:
PS8:
> hold
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/66855
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I0057165aea553d9dc8e4e719e2804557229a0002
Gerrit-Change-Number: 66855
Gerrit-PatchSet: 9
Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Zheng Bao
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Zheng Bao
Gerrit-Comment-Date: Thu, 02 Mar 2023 03:59:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Bao Zheng <fishbaozi(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Zheng Bao, Fred Reitberger, Felix Held.
Hello build bot (Jenkins), Zheng Bao, Fred Reitberger, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/66856
to look at the new patch set (#7).
Change subject: amdfwtool:combo: Add combo feature for BIOS table
......................................................................
amdfwtool:combo: Add combo feature for BIOS table
It is similar to PSP combo.
Change-Id: If0523a4a0e1f31969e4bbaa6062dcc0f2d6da420
Signed-off-by: Zheng Bao <fishbaozi(a)gmail.com>
---
M util/amdfwtool/amdfwtool.c
M util/amdfwtool/amdfwtool.h
2 files changed, 30 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/56/66856/7
--
To view, visit https://review.coreboot.org/c/coreboot/+/66856
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If0523a4a0e1f31969e4bbaa6062dcc0f2d6da420
Gerrit-Change-Number: 66856
Gerrit-PatchSet: 7
Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Zheng Bao
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Zheng Bao
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: newpatchset