Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30989 )
Change subject: intelblocks/systemagent: Add ACPI table generation hook ......................................................................
Patch Set 2:
(5 comments)
Thanks for having a look Alex
https://review.coreboot.org/#/c/30989/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/30989/2//COMMIT_MSG@9 PS2, Line 9:
I am not a native speaker, but I think that there should be a comma here.
Ack
https://review.coreboot.org/#/c/30989/2//COMMIT_MSG@9 PS2, Line 9: SoC-scope
just `SoC scope` I guess, same for `weak function` and `SoC-code` cause they aren't compound adjecti […]
Ack
https://review.coreboot.org/#/c/30989/2//COMMIT_MSG@13 PS2, Line 13: make
`do no harm` should be the right one
Ack
https://review.coreboot.org/#/c/30989/2/src/soc/intel/common/block/include/i... File src/soc/intel/common/block/include/intelblocks/acpi.h:
https://review.coreboot.org/#/c/30989/2/src/soc/intel/common/block/include/i... PS2, Line 79: const
Don't declare constness for function-local variables, same for the `rsdp` parameter. […]
Agree, will change.
https://review.coreboot.org/#/c/30989/2/src/soc/intel/common/block/systemage... File src/soc/intel/common/block/systemagent/systemagent.c:
https://review.coreboot.org/#/c/30989/2/src/soc/intel/common/block/systemage... PS2, Line 23: #include <intelblocks/acpi.h>
Is there a specific reason to not sort the includes? If so, consider documenting with a comment.
No specific reason, will sort the include.