Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Martin L Roth, Matt DeVillier, Nick Kochlowski, Varshit Pandya.
Martin Roth has posted comments on this change by Nick Kochlowski. ( https://review.coreboot.org/c/coreboot/+/85631?usp=email )
Change subject: drivers/amd/opensil: Add openSIL timepoint calls ......................................................................
Patch Set 6:
(3 comments)
File src/drivers/amd/opensil/Kconfig:
https://review.coreboot.org/c/coreboot/+/85631/comment/fb859c68_8e52cc87?usp... : PS6, Line 5: depends on SOC_AMD_OPENSIL I don't think this is needed, or actually even useful. Because there's no prompt, the option *must* be selected to be enabled. Using a select will override a "depends on" statement.
File src/drivers/amd/opensil/opensil.h:
https://review.coreboot.org/c/coreboot/+/85631/comment/aa20c667_57dc4f93?usp... : PS6, Line 3: _OPENSIL_DRIVER_H_ Nit: maybe "DRIVER_AMD_OPENSIL", without the underscores? Or not.
It's not critical, but we'd like to get rid of all of the underscores at some point. We thought they were useful, but apparently they violate some spec or another.
File src/soc/amd/phoenix/chip.c:
https://review.coreboot.org/c/coreboot/+/85631/comment/a27e9c4a_d9ca3afc?usp... : PS6, Line 3: /* TODO: Update for Phoenix */ Not related to this patch, but maybe we can get rid of this todo soon?