Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44359 )
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
Patch Set 8:
(3 comments)
https://review.coreboot.org/c/coreboot/+/44359/8/src/mainboard/google/voltee... File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/44359/8/src/mainboard/google/voltee... PS8, Line 40: tlcl_lib_init(); : if (get_cr50_long_interrupt_pulses()) { : printk(BIOS_INFO, "Enabling S0i3.4\n"); : } else { : /* Disable S03.4, preventing the GPIO block from switching to : * slow clock. */ : printk(BIOS_INFO, "Not enabling S0i3.4\n"); : cfg->LpmStateDisableMask = LPM_S0i3_4; : } nit: Put this in a helper function just to logically keep all changes together. mainboard_update_s0ix_disable_mask().
https://review.coreboot.org/c/coreboot/+/44359/8/src/soc/intel/tigerlake/Kco... File src/soc/intel/tigerlake/Kconfig:
PS8: Can you please push this in a separate patch? We typically separate out SoC changes from mainboard changes.
https://review.coreboot.org/c/coreboot/+/44359/8/src/soc/intel/tigerlake/Kco... PS8, Line 173: config A comment here would be very helpful as to why CR50_USE_LONG_INTERRUPT_PULSES is being selected.