Attention is currently required from: Martin Roth - Personal, Paul Menzel, Arthur Heymans, Keith Hui. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38988 )
Change subject: mainboard/asus/p8x7x-series: Add new variant P8Z77-M ......................................................................
Patch Set 20: Code-Review+1
(17 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/38988/comment/3f9fd3d4_315f2fa0 PS19, Line 9: dumps.
nit: Since CB:60006 the commit message line length limit is 72. This part goes past the limit.
Done
https://review.coreboot.org/c/coreboot/+/38988/comment/5005f96a_61db09a0 PS19, Line 14: tests
nit: Since CB:60006 the commit message line length limit is 72. This part goes past the limit.
Done
https://review.coreboot.org/c/coreboot/+/38988/comment/b1e01dd8_19d9acb2 PS19, Line 27:
Anything not working or untested?
Done
Commit Message:
https://review.coreboot.org/c/coreboot/+/38988/comment/ce50e94d_e803cd0c PS20, Line 32: 6ch analog audio out Interesting, what doesn't work?
File src/mainboard/asus/p8x7x-series/Kconfig.name:
https://review.coreboot.org/c/coreboot/+/38988/comment/558a1c1b_aa3b0107 PS19, Line 47: select USE_NATIVE_RAMINIT
If you select this, it's impossible to use MRC.bin with this board, so all the MRC. […]
Done
File src/mainboard/asus/p8x7x-series/variants/p8z77-m/acpi_tables.c:
https://review.coreboot.org/c/coreboot/+/38988/comment/440e27f1_3b13f755 PS19, Line 4: #include <southbridge/intel/bd82x6x/nvs.h>
Hmmm, this doesn't exist. This file isn't being compiled in, and can be dropped.
Done
https://review.coreboot.org/c/coreboot/+/38988/comment/bb954b43_5c57d500 PS19, Line 8: /* Critical temp: Shut down the PC at 95 degrees C */ : gnvs->tcrt = 95; : : /* Start throttling the CPU at 85 degrees C */ : gnvs->tpsv = 85;
These values aren't used anywhere. The entire file can be dropped.
Done
File src/mainboard/asus/p8x7x-series/variants/p8z77-m/cmos.layout:
https://review.coreboot.org/c/coreboot/+/38988/comment/68d6f94b_b9f6e751 PS19, Line 26: 1
After CB:39828 this should be 2 bits, there's three possible settings for `sata_mode`
Done
https://review.coreboot.org/c/coreboot/+/38988/comment/ad92138e_2a0b9836 PS19, Line 95: 6 0 AHCI : 6 1 Compatible
After CB:39828 this should be expanded accordingly: […]
Done
File src/mainboard/asus/p8x7x-series/variants/p8z77-m/dsdt.asl:
https://review.coreboot.org/c/coreboot/+/38988/comment/4f133764_565d347b PS19, Line 10: /* OEM revision */
autoport adds this comment, but it's not true. I'd remove it.
Done
File src/mainboard/asus/p8x7x-series/variants/p8z77-m/early_init.c:
https://review.coreboot.org/c/coreboot/+/38988/comment/cae31609_419c7381 PS19, Line 1: /* SPDX-License-Identifier: GPL-2.0-or-later */
nit: add a blank line before the #includes
Done
File src/mainboard/asus/p8x7x-series/variants/p8z77-m/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/38988/comment/61d05a90_4f89e9f8 PS19, Line 6: device pci 00.0 on end # Host bridge : device pci 01.0 on end # PCIe Bridge for discrete graphics : device pci 02.0 on end # Internal graphics VGA controller
Already specified in the devicetree, can be dropped.
Done
https://review.coreboot.org/c/coreboot/+/38988/comment/f2fde4df_6af62935 PS19, Line 11: register "pcie_port_coalesce" = "1"
Shouldn't be needed, `device pci 1c.0` is enabled.
Done
https://review.coreboot.org/c/coreboot/+/38988/comment/ca49f8c7_464d48fe PS19, Line 12: register "sata_interface_speed_support" = "0x3" # 0x3=SATAIII : register "sata_port_map" = "0x3f" : register "spi_lvscc" = "0x2005" : register "spi_uvscc" = "0x2005" : register "superspeed_capable_ports" = "0x0000000f" : register "xhci_overcurrent_mapping" = "0x00000c03" : register "xhci_switchable_ports" = "0x0000000f" # the 4 ports
Already specified in the devicetree, can be dropped.
Done
https://review.coreboot.org/c/coreboot/+/38988/comment/8100d1e0_b22c6b17 PS19, Line 20: device pci 14.0 on end # USB 3.0 Controller : device pci 16.0 on end # Management Engine Interface 1 : device pci 16.1 off end # Management Engine Interface 2 : device pci 16.2 off end # Management Engine IDE Redirect : device pci 16.3 off end # Management Engine Keyboard/Text : device pci 1a.0 on end # USB2 EHCI #2 : device pci 1b.0 on end # High Definition Audio controller
Already specified in the devicetree, can be dropped.
Done
https://review.coreboot.org/c/coreboot/+/38988/comment/5aff8bdf_1d54f0c0 PS19, Line 35: device pci 1d.0 on end # USB2 EHCI #1 : device pci 1e.0 off end # PCI bridge
Already specified in the devicetree, can be dropped.
Done
https://review.coreboot.org/c/coreboot/+/38988/comment/5b10773f_5d676a2c PS19, Line 99: device pci 1f.2 on end # SATA Controller (AHCI) : device pci 1f.3 on end # SMBus : device pci 1f.5 off end # SATA Controller (Legacy ports 4-5) : device pci 1f.6 off end # Thermal
Already specified in the devicetree, can be dropped.
Done