build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/15164 )
Change subject: nb/amd/pi/00730F01: Add initial native IVRS support ......................................................................
Patch Set 18:
(18 comments)
https://review.coreboot.org/#/c/15164/18/src/northbridge/amd/pi/00730F01/nor... File src/northbridge/amd/pi/00730F01/northbridge.c:
https://review.coreboot.org/#/c/15164/18/src/northbridge/amd/pi/00730F01/nor... PS18, Line 457: if ((dev->bus->secondary == 0x0) && (dev->path.pci.devfn == 0x0)) { line over 80 characters
https://review.coreboot.org/#/c/15164/18/src/northbridge/amd/pi/00730F01/nor... PS18, Line 457: if ((dev->bus->secondary == 0x0) && (dev->path.pci.devfn == 0x0)) { braces {} are not necessary for single statement blocks
https://review.coreboot.org/#/c/15164/18/src/northbridge/amd/pi/00730F01/nor... PS18, Line 485: && (dev->path.pci.devfn < (0x3 << 3))) { line over 80 characters
https://review.coreboot.org/#/c/15164/18/src/northbridge/amd/pi/00730F01/nor... PS18, Line 541: if ((dev->hdr_type & 0x7f) == PCI_HEADER_TYPE_NORMAL) { line over 80 characters
https://review.coreboot.org/#/c/15164/18/src/northbridge/amd/pi/00730F01/nor... PS18, Line 543: if (pci_find_capability(dev, PCI_CAP_ID_PCIE)) { line over 80 characters
https://review.coreboot.org/#/c/15164/18/src/northbridge/amd/pi/00730F01/nor... PS18, Line 562: /* Device is legacy PCI or PCI-X */ line over 80 characters
https://review.coreboot.org/#/c/15164/18/src/northbridge/amd/pi/00730F01/nor... PS18, Line 583: } else if ((dev->hdr_type & 0x7f) == PCI_HEADER_TYPE_BRIDGE) { line over 80 characters
https://review.coreboot.org/#/c/15164/18/src/northbridge/amd/pi/00730F01/nor... PS18, Line 584: if (pci_find_capability(dev, PCI_CAP_ID_PCIE)) { line over 80 characters
https://review.coreboot.org/#/c/15164/18/src/northbridge/amd/pi/00730F01/nor... PS18, Line 609: for (sibling = link->children; sibling; sibling = sibling->sibling) line over 80 characters
https://review.coreboot.org/#/c/15164/18/src/northbridge/amd/pi/00730F01/nor... PS18, Line 610: add_ivrs_device_entries(dev, sibling, depth + 1, depth, root_level, current, length); line over 80 characters
https://review.coreboot.org/#/c/15164/18/src/northbridge/amd/pi/00730F01/nor... PS18, Line 615: unsigned long acpi_fill_ivrs_ioapic(acpi_ivrs_t* ivrs, unsigned long current) "foo* bar" should be "foo *bar"
https://review.coreboot.org/#/c/15164/18/src/northbridge/amd/pi/00730F01/nor... PS18, Line 652: static unsigned long acpi_fill_ivrs(acpi_ivrs_t* ivrs, unsigned long current) "foo* bar" should be "foo *bar"
https://review.coreboot.org/#/c/15164/18/src/northbridge/amd/pi/00730F01/nor... PS18, Line 658: printk(BIOS_WARNING, "acpi_fill_ivrs: Unable to locate G-series northbridge device! IVRS table not generated...\n"); Prefer using '"%s...", __func__' to using 'acpi_fill_ivrs', this function's name, in a string
https://review.coreboot.org/#/c/15164/18/src/northbridge/amd/pi/00730F01/nor... PS18, Line 663: ivrs->iv_info |= (0x40 << 15); /* Maximum supported virtual address size */ line over 80 characters
https://review.coreboot.org/#/c/15164/18/src/northbridge/amd/pi/00730F01/nor... PS18, Line 664: ivrs->iv_info |= (0x30 << 8); /* Maximum supported physical address size */ line over 80 characters
https://review.coreboot.org/#/c/15164/18/src/northbridge/amd/pi/00730F01/nor... PS18, Line 669: ivrs->ivhd.flags |= 0x10; /* Enable ATS support */ line over 80 characters
https://review.coreboot.org/#/c/15164/18/src/northbridge/amd/pi/00730F01/nor... PS18, Line 671: ivrs->ivhd.device_id = 0x2 | (nb_dev->bus->secondary << 8); /* BDF <bus>:00.2 */ line over 80 characters
https://review.coreboot.org/#/c/15164/18/src/northbridge/amd/pi/00730F01/nor... PS18, Line 672: ivrs->ivhd.capability_offset = 0x40; /* Capability block 0x40 (type 0xf, "Secure device") */ line over 80 characters