Attention is currently required from: Hou-hsun Lee, Paul Menzel, Tim Wawrzynczak, Nick Vaccaro, Zhuohao Lee. Alan Huang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58241 )
Change subject: mb/google/brya/var/baseboard/brask: Add power limits functions ......................................................................
Patch Set 14:
(8 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/58241/comment/27bb98dc_f2abc3ea PS7, Line 9: Set PL and PsysPL for Brask.
Where did you get the values from?
Fixed
File src/mainboard/google/brya/variants/baseboard/brask/ramstage.c:
https://review.coreboot.org/c/coreboot/+/58241/comment/8885e769_5b5f7962 PS12, Line 20: bool get_sku_index(const struct cpu_power_limits *limits, size_t num_entries,
if you make this a static function then you don't need the prototype above.
Fixed.
https://review.coreboot.org/c/coreboot/+/58241/comment/06cb94f7_99daba74 PS12, Line 121:
nit: extra blank line
Fixed.
File src/mainboard/google/brya/variants/brask/ramstage.c:
https://review.coreboot.org/c/coreboot/+/58241/comment/e919d967_b279efe5 PS7, Line 20: /* Psys_pmax considerations. : *
Please use the recommented multi-line comment styles [1]. […]
Done
File src/mainboard/google/brya/variants/brask/ramstage.c:
PS12:
Adding this file should be a separate change on top of the rest
Separate to here https://review.coreboot.org/c/coreboot/+/59576
https://review.coreboot.org/c/coreboot/+/58241/comment/460e5845_30ebed81 PS12, Line 21: *
nit: space after `)`
Fixed.
https://review.coreboot.org/c/coreboot/+/58241/comment/efd62da5_42940bc3 PS12, Line 23: { PCI_DEVICE_ID_INTEL_ADL_P_ID_6, 15, 135, 135 },
The psys_pmax_power should be same (257W) for all SKUs. […]
Updated to 257. Yes, we didn't use it. Should I use these values somewhere?
https://review.coreboot.org/c/coreboot/+/58241/comment/67e4f53c_8124cf87 PS12, Line 41: barral
`barrel`
Fixed.