Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45667 )
Change subject: sconfig: Move exported device names to separate output file ......................................................................
Patch Set 1:
(4 comments)
https://review.coreboot.org/c/coreboot/+/45667/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45667/1//COMMIT_MSG@14 PS1, Line 14: with only the FW_CONFIG_FIELD* macros, making it easily consumable by : payloads. Shouldn't we also add FW_CONFIG_FIELD* macros to something like static_fw_config.h instead of including them directly in static.h: 1. Having the file name include context would be helpful even for payloads when including this file in their codebase. 2. If more things get added to static.h in the future, this helps with clean organization. Basically, static.h will only include other header files -- each created for a specific purpose.
https://review.coreboot.org/c/coreboot/+/45667/1/Makefile.inc File Makefile.inc:
https://review.coreboot.org/c/coreboot/+/45667/1/Makefile.inc@622 PS1, Line 622: DEVICETREE_DEVICENAMES_H := $(obj)/static_devices.h : SCONFIG_OPTIONS += --output_d=$(DEVICETREE_DEVICENAMES_H) Place this with DEVICETREE_STATIC_H above?
https://review.coreboot.org/c/coreboot/+/45667/1/util/sconfig/main.c File util/sconfig/main.c:
https://review.coreboot.org/c/coreboot/+/45667/1/util/sconfig/main.c@1806 PS1, Line 1806: static_devices.h Not a big problem, but we take in the file name as input which gets used to get a pointer to the file descriptor and write to file, but when generating #include, we just hardcode the file name here. I know this is similar to what we did for static.h as well. But if we were to ever change the filename for any reason, we would have to update sconfig as well.
https://review.coreboot.org/c/coreboot/+/45667/1/util/sconfig/main.c@1826 PS1, Line 1826: fprintf(autohead, "\n#endif /* __STATIC_DEVICE_TREE_H */\n"); : fprintf(autodev, "\n#endif /* __STATIC_DEVICE_NAMES_H */\n"); Time to create helper functions to create each header file?