Attention is currently required from: Angel Pons, Arthur Heymans, Christian Walter, Felix Held, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, Shuo Liu, Tim Chu.
2 comments:
File src/soc/intel/xeon_sp/chip_gen1.c:
Patch Set #1, Line 234: * @param mc_values List of system memory map variables.
Personally, I think to have the soc memory map codes to get a full picture of system memory map sett […]
This would mean that you'd move the entire enum into the API only to move
it back in the next commit? I don't think this makes sense, it would create
noise in the git history.
If you want to split the two steps (making it TOHM only, and moving the code
to another file), you could first create a static function in `uncore.c` with
the new interface, and then move the code in a second commit.
You seem to have missed my question above:
> After this patch all the array entries but TOHM are handled by common code. If
> an implementation of soc_add_dram_resources() would try to handle other entries
> as well, wouldn't that cause conflicts?
IMO, this is very important for the API design: "[...], wouldn't that cause
conflicts?". If it would, we should never create such an API that encourages
error-prone code.
File src/soc/intel/xeon_sp/uncore.c:
Patch Set #1, Line 306: } else {
It's hard to review this change without knowing what the implementation of `soc_add_dram_resources` […]
Sorry, I guess this was misunderstood. Your gen1 soc_add_dram_resources()
has all its code in two branches of an `if` that is decided at compile time.
This already makes it two different implementations of the same function.
With splitting I meant to split it into to separate soc_add_dram_resources()
functions one for gen1 w/o CXL (`chip_gen1.c` seems to be a good place for
that) and one for SPR which has CXL (if it's not shared between platforms,
I guess it should reside somewhere in `spr/`).
To view, visit change 81958. To unsubscribe, or for help writing mail filters, visit settings.