Frans Hendriks 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)
Will upload a new patch set
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?
No, this file required. (Included by src/soc/intel/braswell/acpi/lpc.asl)
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 […]
cbfs_boot_map_with_leak() does not work on compressed logo. Need to uncompress the image. (The used code is similar to vbt.bin handling)
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 […]
Will remove this file. Weak function will be used return 0.
-
Frans Hendriks (Code Review)