Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40278 )
Change subject: mb/purism/librem_whl: Add new board Librem Mini (WHL-U) ......................................................................
Patch Set 8:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40278/7/src/mainboard/purism/librem... File src/mainboard/purism/librem_whl/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/40278/7/src/mainboard/purism/librem... PS7, Line 135: # Misc : register "Device4Enable" = "1" : # HECI must be enabled w/HAP disable else S3 issues : register "HeciEnabled" = "1" : register "Heci3Enabled" = "0"
Please move these down to the devicetree.
Done
https://review.coreboot.org/c/coreboot/+/40278/7/src/mainboard/purism/librem... File src/mainboard/purism/librem_whl/ramstage.c:
https://review.coreboot.org/c/coreboot/+/40278/7/src/mainboard/purism/librem... PS7, Line 8: /* Configure pads prior to SiliconInit() in case there's any : * dependencies during hardware initialization. */
I think this is wrong. GPIOs are configured after silicon init. I would just delete it.
mainboard_silicon_init_params() is called in: src/soc/intel/cannonlake/fsp_params.c platform_fsp_silicon_init_params_cb() ln 274
per src/drivers/intel/fsp2_0/silicon_init.c, this happens before silicon_init is called
-
Matt DeVillier (Code Review)