Attention is currently required from: Frank Wu, John Su, Alan Lee, Martin Roth, Ivy Jian. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49783 )
Change subject: drivers/generic/bayhub_lv2: Add driver for BayHub lv2 ......................................................................
Patch Set 2: Code-Review+1
(7 comments)
File src/drivers/generic/bayhub_lv2/Kconfig:
https://review.coreboot.org/c/coreboot/+/49783/comment/a5b5a0f6_c6232693 PS2, Line 1: DRIVERS_GENERIC_LV2 DRIVERS_GENERIC_BAYHUB_LV2
File src/drivers/generic/bayhub_lv2/chip.h:
https://review.coreboot.org/c/coreboot/+/49783/comment/b7c3a0ed_86ce02b6 PS2, Line 9: //struct drivers_generic_bayhub_config { hm?
https://review.coreboot.org/c/coreboot/+/49783/comment/2bcb89a6_4da3c916 PS2, Line 11: unsigned int `bool` should be enough (need to #include stdbool.h)
File src/drivers/generic/bayhub_lv2/lv2.c:
https://review.coreboot.org/c/coreboot/+/49783/comment/4376efc6_dcc4a482 PS2, Line 14: __attribute__((weak)) __weak
https://review.coreboot.org/c/coreboot/+/49783/comment/b5f8122f_5b442e6d PS2, Line 24: /* : * This procedure for enabling power-saving mode is from the : * BayHub BIOS Implementation Guideline document. : */ : pci_write_config32(dev, LV2_PROTECT, : LV2_PROTECT_OFF | LV2_PROTECT_LOCK_OFF); : pci_or_config32(dev, LV2_PCR_HEX_FC, LV2_PCIE_PHY_L1_ENABLE); : pci_update_config32(dev, LV2_PCR_HEX_E0, : 0x0FFFFFFF, 0x30000000); : pci_update_config32(dev, LV2_PCR_HEX_FC, : 0xFFF0FFFF, 0x000E0000); : pci_or_config32(dev, LV2_PCR_HEX_A8, LV2_LTR_ENABLE); : pci_write_config32(dev, LV2_PCR_HEX_234, 0x10011001); : pci_update_config32(dev, LV2_PCR_HEX_248, : 0xFFFFFFF0, 0x0000000A); : pci_update_config32(dev, LV2_PCR_HEX_3F4, : 0xFFFFFFF0, 0x0000000A); : pci_or_config32(dev, LV2_LINK_CTRL, LV2_LINK_CTRL_CLKREQ); : pci_update_config32(dev, LV2_PCR_HEX_300, : 0xFFFF0F00, 0x00006055); : pci_update_config32(dev, LV2_PCR_HEX_304, : 0xFFFF0000, 0x0000224B); : pci_update_config32(dev, LV2_LINK_CTRL, : 0xFFFFFFFC, 0x00000002); : pci_write_config32(dev, LV2_PROTECT, : LV2_PROTECT_ON | LV2_PROTECT_LOCK_ON); : printk(BIOS_INFO, "BayHub LV2: Power-saving enabled (link_ctrl=%#x)\n", : pci_read_config32(dev, LV2_LINK_CTRL)); Maybe put this into a separate function? Then most of the lines should fit in 96 characters
https://review.coreboot.org/c/coreboot/+/49783/comment/ab2e2162_b60f5a2b PS2, Line 61: & nit: `&` not required
https://review.coreboot.org/c/coreboot/+/49783/comment/34488e9d_6416490e PS2, Line 77: PCI nit: PCIe ?