Attention is currently required from: Kyösti Mälkki, Patrick Rudolph, Elyes Haouas.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61517 )
Change subject: aopen/dxplplusu: Support SMM_ASEG and SMM_TSEG ......................................................................
Patch Set 7: Code-Review+2
(5 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/61517/comment/ada0dab8_116866dc PS7, Line 13:
'occuring' may be misspelled - perhaps 'occurring'?
Please fix, it's line 11.
File src/northbridge/intel/e7505/Kconfig:
https://review.coreboot.org/c/coreboot/+/61517/comment/6570ae9f_5254c2b1 PS7, Line 13: #select SMM_ASEG Should this be selectable?
File src/northbridge/intel/e7505/memmap.c:
https://review.coreboot.org/c/coreboot/+/61517/comment/993a9802_c39e26f8 PS7, Line 42: return 512 *KiB;
need consistent spacing around '*' (ctx:WxV)
Please fix.
https://review.coreboot.org/c/coreboot/+/61517/comment/53060947_bbf29f58 PS7, Line 54: tolm -= northbridge_get_tseg_size(); Since most other places also use TSEG size, would it be a good idea to have a function that returns both values? Or make the `northbridge_get_tseg_base()` function take the size as parameter.
File src/northbridge/intel/e7505/raminit.c:
https://review.coreboot.org/c/coreboot/+/61517/comment/7b9cdbbb_0d1e42ad PS7, Line 1690: #define HOST_BRIDGE PCI_DEV(0, 0, 0) Other Intel northbridges have this define in a common header to avoid redundancy. Hmmm, maybe we should look into making SCONFIG generate these macros from devicetree aliases.
-
Angel Pons (Code Review)