Marco Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39424 )
Change subject: mb/google/octopus: Add custom SAR values for Foob360 ......................................................................
Patch Set 6:
(2 comments)
https://review.coreboot.org/c/coreboot/+/39424/6/src/mainboard/google/octopu... File src/mainboard/google/octopus/variants/foob/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/39424/6/src/mainboard/google/octopu... PS6, Line 5: ramstage-y += mainboard.c Could we just put SAR function into variant.c as I mentioned earlier? I understand phaser is in mainboard.c but almost others are in variant.c actually?
https://review.coreboot.org/c/coreboot/+/39424/6/src/mainboard/google/octopu... File src/mainboard/google/octopus/variants/foob/mainboard.c:
https://review.coreboot.org/c/coreboot/+/39424/6/src/mainboard/google/octopu... PS6, Line 24: uint32_t sku_id = google_chromeec_get_board_sku(); Please call get_board_sku() intead. The reason is because there is a cache so no necessary to communicate to EC every time.