Attention is currently required from: Paul Menzel, Shelley Chen.
Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78009?usp=email )
Change subject: mb/google/brox: Create new Brox baseboard ......................................................................
Patch Set 11:
(19 comments)
File src/mainboard/google/brox/Kconfig:
https://review.coreboot.org/c/coreboot/+/78009/comment/ad19cb6a_40d7142f : PS11, Line 145: config INCLUDE_NVIDIA_GPU_ASL : def_bool n : help : Select this if the variant has an Nvidia GN20 GPU attached to PEG1 Is Nvidia GN20 GPU attached to PEG1 an option for Brox?
File src/mainboard/google/brox/Kconfig.name:
https://review.coreboot.org/c/coreboot/+/78009/comment/d0f1f10b_3d3548f1 : PS11, Line 5: select BOARD_GOOGLE_BASEBOARD_BROX selects now go in Kconfig
File src/mainboard/google/brox/acpi/power.asl:
https://review.coreboot.org/c/coreboot/+/78009/comment/2e347aae_af20954e : PS11, Line 9: if CONFIG(BOARD_GOOGLE_AGAH) : #define GPIO_1V8_PWR_EN GPP_F12 : : #define GPIO_NV33_PWR_EN GPP_A21 : #define GPIO_NV33_PG GPP_A22 : #else : remove
https://review.coreboot.org/c/coreboot/+/78009/comment/780ae919_aaff6ca3 : PS11, Line 35: NIT - extra tab
https://review.coreboot.org/c/coreboot/+/78009/comment/07ccb788_36f335db : PS11, Line 47: For board revs 3 and later, two pins moved: This comment is not really valid as written.
https://review.coreboot.org/c/coreboot/+/78009/comment/8a4581f5_a2de742b : PS11, Line 132: #if CONFIG(BOARD_GOOGLE_AGAH) : STXS (GPIO_FBVDD_PWR_EN) : #else : remove
https://review.coreboot.org/c/coreboot/+/78009/comment/c229189f_fff8fc97 : PS11, Line 140: #if CONFIG(BOARD_GOOGLE_AGAH) : Sleep (10) : #else : remove
https://review.coreboot.org/c/coreboot/+/78009/comment/d611c45a_8f0077f8 : PS11, Line 182: #if CONFIG(BOARD_GOOGLE_AGAH) : CTXS (GPIO_FBVDD_PWR_EN) : #else : remove
https://review.coreboot.org/c/coreboot/+/78009/comment/3c787e25_a5f818d9 : PS11, Line 251: #if CONFIG(BOARD_GOOGLE_AGAH) : CTXS (GPIO_FBVDD_PWR_EN) : #else : remove
https://review.coreboot.org/c/coreboot/+/78009/comment/6740f678_7de68aca : PS11, Line 282: #if CONFIG(BOARD_GOOGLE_AGAH) : STXS (GPIO_FBVDD_PWR_EN) : #else : remove
https://review.coreboot.org/c/coreboot/+/78009/comment/f2c86548_a6dd2e86 : PS11, Line 290: #if CONFIG(BOARD_GOOGLE_AGAH) : Sleep (10) : #else : remove
File src/mainboard/google/brox/chromeos.fmd:
https://review.coreboot.org/c/coreboot/+/78009/comment/efb3d522_3318c77a : PS11, Line 30: brya brox
https://review.coreboot.org/c/coreboot/+/78009/comment/c9dd3726_33bdc2ec : PS11, Line 39: ADL Still relevant for RPL?
File src/mainboard/google/brox/dsdt.asl:
https://review.coreboot.org/c/coreboot/+/78009/comment/16ff4893_067aa2db : PS11, Line 18: #include <cpu/intel/common/acpi/cpu.asl> NIT - alphabetize
File src/mainboard/google/brox/variants/baseboard/brox/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/78009/comment/82fc5eaa_568ce027 : PS11, Line 76: disable_c1_state_auto_demotion Is this needed for RPL-UR?
File src/mainboard/google/brox/variants/baseboard/brox/include/baseboard/ec.h:
https://review.coreboot.org/c/coreboot/+/78009/comment/dc866dac_af1601b1 : PS11, Line 8: #include <baseboard/gpio.h> NIT - alphabetize
https://review.coreboot.org/c/coreboot/+/78009/comment/0dba67d9_53f0e937 : PS11, Line 10: INCLUDE_NVIDIA_GPU_ASL Will this be an option for Brox?
File src/mainboard/google/brox/variants/baseboard/brox/ramstage.c:
https://review.coreboot.org/c/coreboot/+/78009/comment/e11e0ddb_a1ffe11a : PS11, Line 8: : #include <drivers/intel/dptf/chip.h> : #include <intelblocks/power_limit.h> NIT - alphabetize into system includes block above
File src/mainboard/google/brox/variants/brox/fw_config.c:
https://review.coreboot.org/c/coreboot/+/78009/comment/26441dde_dd3c9392 : PS11, Line 4: #include <console/console.h> : #include <fw_config.h> : #include <gpio.h> These aren't needed, remove.