Ronak Kanabar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30774 )
Change subject: mb/google/hatch: Configure miscellaneous features ......................................................................
Patch Set 4:
(7 comments)
https://review.coreboot.org/#/c/30774/1/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/devicetree.cb:
https://review.coreboot.org/#/c/30774/1/src/mainboard/google/hatch/variants/... PS1, Line 52: 3
check for a macro in chip. […]
That SaGV enum was not applicable here because it has 5 values as below 0:Disabled, 1:FixedLow, 2:FixedMid, 3:FixedHigh, 4:Enabled for CML we don't use FixedMid so in CML FSP we have below options 0:Disabled, 1:FixedLow, 2:FixedHigh, 3:Enabled
I pushed following patch to correct SaGv options for all Soc once that change is merged we can use SaGv_Enable in place of 3
https://review.coreboot.org/c/coreboot/+/30917/
https://review.coreboot.org/#/c/30774/1/src/mainboard/google/hatch/variants/... PS1, Line 53: HeciEnabled
This option gives a choice for enabling/disabling Heci function to the mainboard folks. […]
Done
https://review.coreboot.org/#/c/30774/1/src/mainboard/google/hatch/variants/... PS1, Line 54: VmxEnable
+1. […]
Done
https://review.coreboot.org/#/c/30774/1/src/mainboard/google/hatch/variants/... PS1, Line 55: register "speed_shift_enable" = "1"
add a description as below […]
I will add comment
https://review.coreboot.org/#/c/30774/1/src/mainboard/google/hatch/variants/... PS1, Line 56: s0ix_enable
# Enable S0ix
I will add comment
https://review.coreboot.org/#/c/30774/1/src/mainboard/google/hatch/variants/... PS1, Line 57: dptf_enable
don't enable it now without adding proper ASL entries. […]
This is place holder option whenever we will add ASL we will enable this bit
https://review.coreboot.org/#/c/30774/1/src/mainboard/google/hatch/variants/... PS1, Line 58: dmipwroptimize
Looks like this option doesn't have any effect, must be removed.
Done