Attention is currently required from: Eric Lai, Kapil Porwal, Nick Vaccaro, Subrata Banik, Varshit Pandya.
Dinesh Gehlot has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81626?usp=email )
Change subject: mb/google/brya: Add new baseboard trulo ......................................................................
Patch Set 3:
(6 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/81626/comment/5550c7e0_d5e598ac : PS2, Line 11:
would be good if you could add the "TEST" info
Acknowledged
File src/mainboard/google/brya/Kconfig:
https://review.coreboot.org/c/coreboot/+/81626/comment/416e9fd6_8cc746ed : PS2, Line 128: config BOARD_GOOGLE_BASEBOARD_TRULO : def_bool n : select BOARD_GOOGLE_BRYA_COMMON : select CHROMEOS_DRAM_PART_NUMBER_IN_CBI if CHROMEOS : select SOC_INTEL_ALDERLAKE_PCH_N : select SYSTEM_TYPE_LAPTOP
all the baseboard related config are in alphabetical order, may be move this after the "BOARD_GOOGLE […]
Acknowledged
File src/mainboard/google/brya/variants/baseboard/trulo/gpio.c:
https://review.coreboot.org/c/coreboot/+/81626/comment/3b4d76ee_e7b80d29 : PS2, Line 1:
empty line after the line containing license information, just like how you did for ec.h file.
Acknowledged
https://review.coreboot.org/c/coreboot/+/81626/comment/7e31b70a_c7cedbad : PS2, Line 5: #include <types.h> : #include <soc/gpio.h>
sort them in alphabetical order ?
Acknowledged
https://review.coreboot.org/c/coreboot/+/81626/comment/341b8712_1a5b3d46 : PS2, Line 8: /* Pad configuration in ramstage */
I would put an empty line after include file list ending (line 8 should be an empty line)
Acknowledged
File src/mainboard/google/brya/variants/baseboard/trulo/include/baseboard/ec.h:
https://review.coreboot.org/c/coreboot/+/81626/comment/29722cc0_d282133e : PS2, Line 6: #include <ec/ec.h> : #include <ec/google/chromeec/ec_commands.h> : #include <baseboard/gpio.h>
sort them alphabetically ..
Acknowledged