Attention is currently required from: Fred Reitberger, Jason Glenesk, Martin L Roth, Matt DeVillier, Nick Kochlowski, Varshit Pandya.
Felix Held 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/a93d8403_24c1d56b?usp... : PS6, Line 5: depends on SOC_AMD_OPENSIL
I don't think this is needed, or actually even useful. […]
i asked for that one to be put back in some earlier version, but i don't have a super strong opinion on that one. my reasoning was that in order to avoid the build failing when only one option is selected, it might be good to have the dependency there; not 100% convinced that selecting SOC_AMD_OPENSIL here would be a better solution, since selecting SOC_AMD_OPENSIL also requires selecting one of the SOC_AMD_OPENSIL_* options and i don't think that turning the SOC_AMD_OPENSIL_* into a kconfig choice with the stub being the default, since i think we can't easily add another choice option from site-local there
File src/drivers/amd/opensil/opensil.h:
https://review.coreboot.org/c/coreboot/+/85631/comment/58dc7d88_47178e37?usp... : PS6, Line 3: _OPENSIL_DRIVER_H_
Nit: maybe "DRIVER_AMD_OPENSIL", without the underscores? Or not. […]
good point, now that i've thought about that one, i second that
File src/soc/amd/phoenix/chip.c:
https://review.coreboot.org/c/coreboot/+/85631/comment/8a659c10_80617e5b?usp... : PS6, Line 3: /* TODO: Update for Phoenix */
Not related to this patch, but maybe we can get rid of this todo soon?
i probably just forgot to remove that one. but yes, that can be removed; probably something for a separate patch