Attention is currently required from: David Wu, Karthik Ramasubramanian, Ren Kuo, Shelley Chen, Subrata Banik, Tyler Wang.
Ren Kuo has posted comments on this change by Ren Kuo. ( https://review.coreboot.org/c/coreboot/+/83212?usp=email )
Change subject: mb/google/brox: Create jubilant variant ......................................................................
Patch Set 6:
(6 comments)
File src/mainboard/google/brox/variants/jubilant/Makefile.mk:
https://review.coreboot.org/c/coreboot/+/83212/comment/a4fd1aa1_d1683c2c?usp... : PS4, Line 3: bootblock-y += gpio.c
Nit: Leave a blank line between each stages for clarity.
Fixed
File src/mainboard/google/brox/variants/jubilant/fw_config.c:
https://review.coreboot.org/c/coreboot/+/83212/comment/5eb62b17_a300724b?usp... : PS4, Line 15: if (fw_config_probe(FW_CONFIG(STORAGE, STORAGE_NVME))) { : printk(BIOS_INFO, "Configure GPIOs, device config for NVME.\n"); : } : : if (fw_config_probe(FW_CONFIG(STORAGE, STORAGE_UNKNOWN))) { : printk(BIOS_INFO, "Configure GPIOs, device config for UFS and NVME.\n"); : }
These things do nothing. Remove them now and add them later when you configure the GPIOs.
fixed.
File src/mainboard/google/brox/variants/jubilant/include/variant/gpio.h:
https://review.coreboot.org/c/coreboot/+/83212/comment/51d23b4f_e3d3dfc0?usp... : PS4, Line 11: #define WWAN_RST GPP_E16 : #define WWAN_PERST GPP_E0
These don't look correct. WWAN_PERST does not exist in GPIO config.
Fixed.
File src/mainboard/google/brox/variants/jubilant/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/83212/comment/b1c3b9a7_53b98808?usp... : PS4, Line 2: field RETIMER 0 1 : option RETIMER_UNKNOWN 0 : option RETIMER_BYPASS 1 : end : field STORAGE 2 3 : option STORAGE_UNKNOWN 0 : option STORAGE_UFS 1 : option STORAGE_NVME 2 : end : field WIFI_BT 4 4 : option WIFI_BT_CNVI 0 : option WIFI_BT_PCIE 1 : end : field AUDIO 5 7 : option AUDIO_UNKNOWN 0 : option AUDIO_REALTEK_ALC256 1 : end : field UFC 8 9 : option UFC_NONE 0 : option UFC_USB 1 : end : field FPMCU 17 18 : option FP_ABSENT 0 : option FP_MCU_NUVOTON 1 : end : field ISH 21 : option ISH_DISABLE 0 : option ISH_ENABLE 1 : end
The entire devicetree seems copypasted from another devicetree. […]
Fixed.
https://review.coreboot.org/c/coreboot/+/83212/comment/57c2bfb1_266b367b?usp... : PS4, Line 39: WWLAN
Nit: WWAN
Fixed.
https://review.coreboot.org/c/coreboot/+/83212/comment/1b0f306d_7ffae045?usp... : PS4, Line 294: DB_USB DB_1C_LTE
This mask is not defined in the FW Config. […]
fixed.