David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30414 )
Change subject: mainboard/facebook/fbg1701: Do initial mainboard commit ......................................................................
Patch Set 22:
(3 comments)
It looks like almost comments have been addressed since PS19, let's try to get this merged soon.
https://review.coreboot.org/#/c/30414/22/src/mainboard/facebook/fbg1701/acpi... File src/mainboard/facebook/fbg1701/acpi/ec.asl:
https://review.coreboot.org/#/c/30414/22/src/mainboard/facebook/fbg1701/acpi... PS22, Line 1: /* Can this file be removed?
https://review.coreboot.org/#/c/30414/22/src/mainboard/facebook/fbg1701/logo... File src/mainboard/facebook/fbg1701/logo.c:
https://review.coreboot.org/#/c/30414/22/src/mainboard/facebook/fbg1701/logo... PS22, Line 35: size_t file_size = cbfs_boot_load_file(filename, logo_data, As Michal suggested in PS17, you can get rid of logo_array[] and use the return value from cbfs_boot_map_with_leak(), e.g. logo_data = cbfs_boot_map_with_leak(filename, CBFS_TYPE_RAW, *logo_size);
(There are a lot of examples of how to use this with other data blobs, particularly SPD blobs)
https://review.coreboot.org/#/c/30414/22/src/mainboard/facebook/fbg1701/smih... File src/mainboard/facebook/fbg1701/smihandler.c:
https://review.coreboot.org/#/c/30414/22/src/mainboard/facebook/fbg1701/smih... PS22, Line 22: int mainboard_io_trap_handler(int smif) Is any of this needed? If we just need the function definition to be present, would it be better to return 0 immediately instead of going thru the switch() below?
-
David Hendricks (Code Review)