Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/24996 )
Change subject: mb/google/poppy : Get SKU_ID from EC for Nami/Vayne ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/#/c/24996/1/src/mainboard/google/poppy/ramstage.... File src/mainboard/google/poppy/ramstage.c:
https://review.coreboot.org/#/c/24996/1/src/mainboard/google/poppy/ramstage.... PS1, Line 28: board_sku_id This should be moved under mainboard/google/poppy/variants/nami/.
https://review.coreboot.org/#/c/24996/1/src/mainboard/google/poppy/ramstage.... PS1, Line 48: mainboard_devtree_update Do you intend to do this before FSP-S is run?
If yes, then you can add a call in mainboard_silicon_init_params to do variant_silicon_init_params(FSP_SIL_UPD *params) and handle enabling/disabling of ports there.
If no, then you can keep the ports enabled in devicetree.cb for nami and handle it in something like mainboard/google/poppy/variants/nami/pl2.c. Though, you will have to rename pl2.c to something like device.c.
https://review.coreboot.org/#/c/24996/1/src/soc/intel/skylake/chip_fsp20.c File src/soc/intel/skylake/chip_fsp20.c:
https://review.coreboot.org/#/c/24996/1/src/soc/intel/skylake/chip_fsp20.c@1... PS1, Line 105: mainboard_devtree_update There is no need to add a callback from SoC code. This can be handled completely in mainboard/variant-specific code.
-
Furquan Shaikh (Code Review)