Attention is currently required from: Arthur Heymans, Cliff Huang, David Milosevic, Lance Zhao, Maximilian Brune, Nico Huber, Patrick Rudolph, Tim Wawrzynczak.
Martin L Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78071?usp=email )
Change subject: acpi: Add PPTT support ......................................................................
Patch Set 9:
(3 comments)
Patchset:
PS9: Kconfig LGTM.
Please rebase on top of the current main branch to get a full jenkins run.
File src/acpi/acpi_pptt.c:
https://review.coreboot.org/c/coreboot/+/78071/comment/7b26dc4f_8d9b8975 : PS8, Line 127: setup_topology(node->sibling, parent_ref, current);
To be honest, I find it harder to understand/read when we mix recursion with loops. […]
Would separating the recursion into a pair of functions, one for node->sibling and one for node->child, make it easier to understand?
Since it is complex, maybe also add some documentation with an example.
File src/include/acpi/acpi.h:
https://review.coreboot.org/c/coreboot/+/78071/comment/bb4e97cf_08b9a465 : PS9, Line 1392: /* As defined in ACPI 6.4 */ Not really a complaint about this patch, but maybe it's time to start looking at splitting up this header into a bunch of separate headers that get included by acpi.h? It's just getting quite long, so splitting it up into separate files could allow us to work on things a little more easly, and maybe with fewer conflicts.
Obviously the acpi_tables enum would stay in here, but the rest could go into acpi/acpi_pptt.h or something.
If you think that's appropriate, it could be done either in this patch or a follow on. If you don't think it's reasonable, feel free to ack this comment and carry on.