Attention is currently required from: Jérémy Compostella, Pranava Y N.
Subrata Banik has posted comments on this change by Jérémy Compostella. ( https://review.coreboot.org/c/coreboot/+/84407?usp=email )
Change subject: mb/google/fatcat: Add override tree ......................................................................
Patch Set 1:
(13 comments)
Patchset:
PS1: Could you please add minimal override tree support without dumping all possible IOs at once? We should take devices one at a time and add their required changes when we're ready to enable them.
File src/mainboard/google/fatcat/variants/fatcat/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/84407/comment/3dcc93f7_7ef64e33?usp... : PS1, Line 3: # GPE configuration : # Note that GPE events called out in ASL code rely on this : # route. i.e. If this route changes then the affected GPE : # offset bits also need to be changed. : register "pmc_gpe0_dw0" = "GPP_B" : register "pmc_gpe0_dw1" = "GPP_D" : register "pmc_gpe0_dw2" = "GPP_E" this should do to baseboard devicetree
https://review.coreboot.org/c/coreboot/+/84407/comment/93033073_bb95d40d?usp... : PS1, Line 45: : # EC host command ranges are in 0x800-0x8ff & 0x200-0x20f : register "gen1_dec" = "0x00fc0801" : register "gen2_dec" = "0x000c0201" : # EC memory map range is 0x900-0x9ff : register "gen3_dec" = "0x00fc0901" same as previous comment
https://review.coreboot.org/c/coreboot/+/84407/comment/f841f763_a17c4706?usp... : PS1, Line 52: # This disabled autonomous GPIO power management for early PO : register "gpio_override_pm" = "1" : register "gpio_pm[COMM_0]" = "0" : register "gpio_pm[COMM_1]" = "0" : register "gpio_pm[COMM_3]" = "0" : register "gpio_pm[COMM_4]" = "0" : register "gpio_pm[COMM_5]" = "0" we don't need to override to zero value, unless defined the value should be zero
https://review.coreboot.org/c/coreboot/+/84407/comment/e12e5a8c_714c5691?usp... : PS1, Line 69: # Enable s0ix : register "s0ix_enable" = "1" this should do to baseboard devicetree
https://review.coreboot.org/c/coreboot/+/84407/comment/cd86d616_f9471c94?usp... : PS1, Line 114: # Intel Common SoC Config we generally maintain a table to understand the I2Cx mapping, can you please apply that here as well ?
https://review.coreboot.org/c/coreboot/+/84407/comment/01e0100e_ac498c74?usp... : PS1, Line 223: please use consistent tab
https://review.coreboot.org/c/coreboot/+/84407/comment/48329c87_4c984a6f?usp... : PS1, Line 223: use usb2_port1 as usb2_port : use tcss_usb3_port0 as usb3_port : device generic 0 alias conn0 on end : end : chip drivers/intel/pmc_mux/conn : use usb2_port2 as usb2_port : use tcss_usb3_port1 as usb3_port : device generic 1 alias conn1 on end : end : chip drivers/intel/pmc_mux/conn : use usb2_port3 as usb2_port : use tcss_usb3_port2 as usb3_port : device generic 2 alias conn2 on end : end : chip drivers/intel/pmc_mux/conn : use usb2_port4 as usb2_port : use tcss_usb3_port3 as usb3_port : device generic 3 alias conn3 on end : end : end I also don't believe we need PMC MUX when we are planning to use PDC<->PMC communication in PTL
https://review.coreboot.org/c/coreboot/+/84407/comment/34d089df_01fcb984?usp... : PS1, Line 246: device ref pcie_rp1 off : # register "pcie_rp[PCIE_RP(1)]" = "{ : # .clk_src = 3, : # .clk_req = 3, : # .flags = PCIE_RP_CLK_REQ_DETECT | PCIE_RP_LTR | PCIE_RP_AER, : # }" : end # Gbe please don't add GBE support which is non-POR for Chrome, if you wish to keep RP0 enabled then keep it on in devicetree.cb
https://review.coreboot.org/c/coreboot/+/84407/comment/18475431_ab66d175?usp... : PS1, Line 253: device ref pcie_rp2 on : register "pcie_rp[PCIE_RP(2)]" = "{ : .clk_src = 5, : .clk_req = 5, : .flags = PCIE_RP_CLK_REQ_DETECT | PCIE_RP_LTR | PCIE_RP_AER, : }" : end # WWAN : device ref pcie_rp3 on : # Enable PCH PCIE x1 slot using CLK 3 : register "pcie_rp[PCIE_RP(3)]" = "{ : .clk_src = 2, : .clk_req = 2, : .flags = PCIE_RP_CLK_REQ_DETECT | PCIE_RP_LTR | PCIE_RP_AER, : }" : chip soc/intel/common/block/pcie/rtd3 : register "enable_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPP_A08)" : register "enable_delay_ms" = "100" : register "reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_D19)" : register "reset_delay_ms" = "20" : register "srcclk_pin" = "2" : device generic 0 on end : end : end # PCIE x1 slot : device ref pcie_rp4 on : register "pcie_rp[PCH_RP(4)]" = "{ : .clk_src = 4, : .clk_req = 4, : .flags = PCIE_RP_CLK_REQ_DETECT | PCIE_RP_LTR | PCIE_RP_AER, : }" : chip soc/intel/common/block/pcie/rtd3 : register "reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_A11)" : register "srcclk_pin" = "4" : device pci 00.0 on end : end : end # discrete WLAN all IOs should be part of FW_CONFIG.
https://review.coreboot.org/c/coreboot/+/84407/comment/94c47c5e_bf2539fd?usp... : PS1, Line 305: device ref pcie_rp9 on : register "pcie_rp[PCIE_RP(9)]" = "{ : .clk_src = 1, : .clk_req = 1, : .flags = PCIE_RP_CLK_REQ_DETECT | PCIE_RP_LTR | PCIE_RP_AER, : }" : chip soc/intel/common/block/pcie/rtd3 : register "is_storage" = "true" : register "enable_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPP_B16)" : register "reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_E03)" : register "srcclk_pin" = "1" : device generic 0 on end : end : end # Gen5 M.2 SSD / x4 PCIe slot please don't need Gen5 support. we will add depending on FW config later
https://review.coreboot.org/c/coreboot/+/84407/comment/4c717505_b25f79ca?usp... : PS1, Line 330: lnlrvp_ish please don't enable everything as part of base CL. Kindly keep ISH off for now
https://review.coreboot.org/c/coreboot/+/84407/comment/cc2ca6c1_f67fc015?usp... : PS1, Line 654: device ref hda on : chip drivers/intel/soundwire : device generic 0 on : chip drivers/soundwire/alc711 : # SoundWire Link 1 ID 1 : register "desc" = ""Headset Codec"" : device generic 1.1 on end : end : end : end : chip drivers/generic/max98357a : register "hid" = ""MX98357A"" : register "sdmode_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPP_E19)" : register "sdmode_delay" = "5" : device generic 0 on end : end : chip drivers/generic/max98357a : register "hid" = ""MX98360A"" : register "sdmode_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPP_E19)" : register "sdmode_delay" = "5" : device generic 0 on end : end : end : : end please use consistent spacing
-
Subrata Banik (Code Review)