Attention is currently required from: Arthur Heymans, Cliff Huang, Lance Zhao, Naresh Solanki, Patrick Rudolph, Paul Menzel, Tim Wawrzynczak.
Martin L Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77884?usp=email )
Change subject: acpi/acpi.h: Add IO Remapping Table structures ......................................................................
Patch Set 4: Code-Review+2
(3 comments)
File src/acpi/Kconfig:
https://review.coreboot.org/c/coreboot/+/77884/comment/678d92a8_524f97f0 : PS4, Line 89: config ACPI_IORT What you have here is fine. I started thinking that it should also have a "depends on" for ARM, but honestly it doesn't matter, because when a "select" statement is used, it ignores all of the "depends on" attributes.
Maybe add a depends on for ARM platforms as a way of documenting it? Maybe Kconfig will throw an error if it's selected without the dependencies. Your choice.
To actually be effective, it needs to have the "if HAVE_ACPI_TABLES" added to where it's selected, but presumably that's another select. Hopefully nobody ever tries to disable it.
File src/include/acpi/acpi.h:
https://review.coreboot.org/c/coreboot/+/77884/comment/4d726c6b_53970165 : PS4, Line 81: IORT, We get a lot of conflicts here. We might want to make this table vertical and alphabetize it so that we get fewer conflicts between patches. This patch is definitely conflicting with several others in flight right now.
That's probably for another patch, and not really your responsibility, but maybe something we should consider.
https://review.coreboot.org/c/coreboot/+/77884/comment/47284941_05403a29 : PS4, Line 1527: /* IORT - IO Remapping Table */ I commented in https://review.coreboot.org/c/coreboot/+/78071/9/src/include/acpi/acpi.h about adding changing our methodology for this file to put the structures for tables into separate headers and just including them into this file. As with that patch, It's something to consider, but that can be done in a separate commit or not at all.
We could even do a single commit that separates up the file into a bunch of headers, but that's not your responsibility...
-
Martin L Roth (Code Review)