Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 8:
(7 comments)
Nice work. Can the QEMU Q35 change be factored out?
https://review.coreboot.org/c/coreboot/+/36221/8//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36221/8//COMMIT_MSG@25 PS8, Line 25: keyword to sconfig Strange line break. Could you reformat the whole message for 75 characters, please?
https://review.coreboot.org/c/coreboot/+/36221/8/src/Kconfig File src/Kconfig:
https://review.coreboot.org/c/coreboot/+/36221/8/src/Kconfig@339 PS8, Line 339: bool "Enable a configurable ramstage." Please remove the dot at the end.
https://review.coreboot.org/c/coreboot/+/36221/8/src/Kconfig@344 PS8, Line 344: The minimal PCI scanning will only check those parts that are enabled : in the devicetree.cb. By convention none of those devices should be bridges. Move this to the description of the option below?
https://review.coreboot.org/c/coreboot/+/36221/8/src/include/device/device.h File src/include/device/device.h:
https://review.coreboot.org/c/coreboot/+/36221/8/src/include/device/device.h... PS8, Line 133: * Remove for concise multi-line comments.
https://review.coreboot.org/c/coreboot/+/36221/8/util/sconfig/lex.yy.c_shipp... File util/sconfig/lex.yy.c_shipped:
https://review.coreboot.org/c/coreboot/+/36221/8/util/sconfig/lex.yy.c_shipp... PS8, Line 9: #define YY_FLEX_SUBMINOR_VERSION 4 Why increment it by 3?
https://review.coreboot.org/c/coreboot/+/36221/8/util/sconfig/sconfig.tab.c_... File util/sconfig/sconfig.tab.c_shipped:
https://review.coreboot.org/c/coreboot/+/36221/8/util/sconfig/sconfig.tab.c_... PS8, Line 112: #ifndef YY_YY_HOME_RMINNICH_PROJECTS_LINUXBOOT_COREBOOTNERF_GITHUBCOREBOOT_UTIL_SCONFIG_SCONFIG_TAB_H_SHIPPED_INCLUDED Is this wanted?
https://review.coreboot.org/c/coreboot/+/36221/8/util/sconfig/sconfig.tab.h_... File util/sconfig/sconfig.tab.h_shipped:
https://review.coreboot.org/c/coreboot/+/36221/8/util/sconfig/sconfig.tab.h_... PS8, Line 33: #ifndef YY_YY_HOME_RMINNICH_PROJECTS_LINUXBOOT_COREBOOTNERF_GITHUBCOREBOOT_UTIL_SCONFIG_SCONFIG_TAB_H_SHIPPED_INCLUDED Ditto.