Attention is currently required from: Angel Pons, Arthur Heymans, Christian Walter, Felix Held, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Nico Huber, Patrick Rudolph, Tim Chu.
Shuo Liu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81958?usp=email )
Change subject: soc/intel/xeon_sp: Add soc_add_dram_resources
......................................................................
Patch Set 2:
(2 comments)
File src/soc/intel/xeon_sp/chip_gen1.c:
https://review.coreboot.org/c/coreboot/+/81958/comment/e98944d6_ea61d7e3 :
PS1, Line 234: * @param mc_values List of system memory map variables.
This would mean that you'd move the entire enum into the API only to move […]
The list (a.k.a. mc_values[]) is initially written/loaded by common codes at the beginning of mc_add_dram_resources, and read/used in subsequential logics (include soc_add_dram_resources).
Hence, TOHM is just referred to instead of being modified. If we further declares mc_value[] as a read-only parameter for soc_add_dram_resources, should this be adequate?
P.S. IMO, mc_values[] as a set of system memory map registers, it is beneficial to be exposed to SoC codes.
File src/soc/intel/xeon_sp/uncore.c:
https://review.coreboot.org/c/coreboot/+/81958/comment/ec74936e_90235410 :
PS1, Line 306: } else {
Sorry, I guess this was misunderstood. Your gen1 soc_add_dram_resources() […]
Got your points, personally I think we could still put into chip_gen1.c but use below patterns,
#if CONFIG(SOC_INTEL_HAS_CXL)
the CXL version
#else
the non-CXL version
#endif
Or remove this function totally from gen1 layer to SoC layers (SKX/CPX/SPR has its dedicated implementations, with only SPR supports CXL).
Your opinion?
--
To view, visit
https://review.coreboot.org/c/coreboot/+/81958?usp=email
To unsubscribe, or for help writing mail filters, visit
https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ie15b11e1f4cdc861324286b1397f9c5e431b74ab
Gerrit-Change-Number: 81958
Gerrit-PatchSet: 2
Gerrit-Owner: Shuo Liu
shuo.liu@intel.com
Gerrit-Reviewer: Angel Pons
th3fanbus@gmail.com
Gerrit-Reviewer: Arthur Heymans
arthur@aheymans.xyz
Gerrit-Reviewer: Christian Walter
christian.walter@9elements.com
Gerrit-Reviewer: Felix Held
felix-coreboot@felixheld.de
Gerrit-Reviewer: Johnny Lin
Johnny_Lin@wiwynn.com
Gerrit-Reviewer: Jonathan Zhang
jon.zhixiong.zhang@gmail.com
Gerrit-Reviewer: Lean Sheng Tan
sheng.tan@9elements.com
Gerrit-Reviewer: Nico Huber
nico.h@gmx.de
Gerrit-Reviewer: Patrick Rudolph
patrick.rudolph@9elements.com
Gerrit-Reviewer: Tim Chu
Tim.Chu@quantatw.com
Gerrit-Reviewer: build bot (Jenkins)
no-reply@coreboot.org
Gerrit-Attention: Nico Huber
nico.h@gmx.de
Gerrit-Attention: Patrick Rudolph
patrick.rudolph@9elements.com
Gerrit-Attention: Jonathan Zhang
jon.zhixiong.zhang@gmail.com
Gerrit-Attention: Johnny Lin
Johnny_Lin@wiwynn.com
Gerrit-Attention: Christian Walter
christian.walter@9elements.com
Gerrit-Attention: Angel Pons
th3fanbus@gmail.com
Gerrit-Attention: Arthur Heymans
arthur@aheymans.xyz
Gerrit-Attention: Lean Sheng Tan
sheng.tan@9elements.com
Gerrit-Attention: Felix Held
felix-coreboot@felixheld.de
Gerrit-Attention: Tim Chu
Tim.Chu@quantatw.com
Gerrit-Comment-Date: Fri, 19 Apr 2024 04:49:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber
nico.h@gmx.de
Comment-In-Reply-To: Shuo Liu
shuo.liu@intel.com
Comment-In-Reply-To: Angel Pons
th3fanbus@gmail.com
Gerrit-MessageType: comment