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 16:
(102 comments)
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... File src/northbridge/amd/pi/00730F01/northbridge.c:
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 441: static void add_ivrs_device_entries(struct device *parent, struct device *dev, int depth, int linknum, int8_t *root_level, unsigned long *current, uint16_t *length) line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 453: if ((dev->bus->secondary == 0x0) && (dev->path.pci.devfn == 0x0)) line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 460: if (dev->path.pci.devfn == 0x2) { line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 460: if (dev->path.pci.devfn == 0x2) { Too many leading tabs - consider code refactoring
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 462: p = (uint8_t *) *current; line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 463: p[0] = 0x2; /* Entry type */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 464: p[1] = dev->path.pci.devfn; /* Device */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 465: p[2] = dev->bus->secondary; /* Bus */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 466: p[3] = 0x0; /* Data */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 467: p[4] = 0x0; /* Padding */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 468: p[5] = 0x0; /* Padding */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 469: p[6] = 0x0; /* Padding */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 470: p[7] = 0x0; /* Padding */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 473: } else if (dev->path.pci.devfn < (0x2 << 3)) { line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 473: } else if (dev->path.pci.devfn < (0x2 << 3)) { Too many leading tabs - consider code refactoring
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 475: } else if ((dev->path.pci.devfn >= (0x2 << 3)) && (dev->path.pci.devfn < (0x3 << 3))) { line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 475: } else if ((dev->path.pci.devfn >= (0x2 << 3)) && (dev->path.pci.devfn < (0x3 << 3))) { Too many leading tabs - consider code refactoring
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 476: /* FCH PCIe bridge device */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 477: p = (uint8_t *) *current; line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 478: p[0] = 0x2; /* Entry type */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 479: p[1] = dev->path.pci.devfn; /* Device */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 480: p[2] = dev->bus->secondary; /* Bus */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 481: p[3] = 0x0; /* Data */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 482: p[4] = 0x0; /* Padding */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 483: p[5] = 0x0; /* Padding */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 484: p[6] = 0x0; /* Padding */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 485: p[7] = 0x0; /* Padding */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 488: } else { Too many leading tabs - consider code refactoring
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 489: if (dev->path.pci.devfn == (0x14 << 3)) { line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 489: if (dev->path.pci.devfn == (0x14 << 3)) { Too many leading tabs - consider code refactoring
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 490: /* SMBUS controller */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 491: p = (uint8_t *) *current; line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 492: p[0] = 0x2; /* Entry type */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 493: p[1] = dev->path.pci.devfn; /* Device */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 494: p[2] = dev->bus->secondary; /* Bus */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 495: p[3] = 0x97; /* Data */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 496: p[4] = 0x0; /* Padding */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 497: p[5] = 0x0; /* Padding */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 498: p[6] = 0x0; /* Padding */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 499: p[7] = 0x0; /* Padding */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 502: } else { Too many leading tabs - consider code refactoring
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 503: /* Other southbridge device */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 504: p = (uint8_t *) *current; line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 505: p[0] = 0x2; /* Entry type */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 506: p[1] = dev->path.pci.devfn; /* Device */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 507: p[2] = dev->bus->secondary; /* Bus */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 508: p[3] = 0x0; /* Data */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 509: p[4] = 0x0; /* Padding */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 510: p[5] = 0x0; /* Padding */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 511: p[6] = 0x0; /* Padding */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 512: p[7] = 0x0; /* Padding */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 518: if ((dev->hdr_type & 0x7f) == PCI_HEADER_TYPE_NORMAL) { line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 518: if ((dev->hdr_type & 0x7f) == PCI_HEADER_TYPE_NORMAL) { Too many leading tabs - consider code refactoring
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 519: /* Device behind bridge */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 520: if (pci_find_capability(dev, PCI_CAP_ID_PCIE)) { line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 520: if (pci_find_capability(dev, PCI_CAP_ID_PCIE)) { Too many leading tabs - consider code refactoring
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 521: /* Device is PCIe */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 522: p = (uint8_t *) *current; line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 523: p[0] = 0x2; /* Entry type */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 524: p[1] = dev->path.pci.devfn; /* Device */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 525: p[2] = dev->bus->secondary; /* Bus */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 526: p[3] = 0x0; /* Data */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 527: p[4] = 0x0; /* Padding */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 528: p[5] = 0x0; /* Padding */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 529: p[6] = 0x0; /* Padding */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 530: p[7] = 0x0; /* Padding */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 533: } else { Too many leading tabs - consider code refactoring
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 534: /* Device is legacy PCI or PCI-X */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 535: p = (uint8_t *) *current; line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 536: p[0] = 0x42; /* Entry type */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 537: p[1] = dev->path.pci.devfn; /* Device */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 538: p[2] = dev->bus->secondary; /* Bus */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 539: p[3] = 0x0; /* Data */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 540: p[4] = 0x0; /* Reserved */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 541: p[5] = parent->path.pci.devfn; /* Device */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 542: p[6] = parent->bus->secondary; /* Bus */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 543: p[7] = 0x0; /* Reserved */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 547: } else if ((dev->hdr_type & 0x7f) == PCI_HEADER_TYPE_BRIDGE) { line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 547: } else if ((dev->hdr_type & 0x7f) == PCI_HEADER_TYPE_BRIDGE) { Too many leading tabs - consider code refactoring
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 548: if (pci_find_capability(dev, PCI_CAP_ID_PCIE)) { line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 548: if (pci_find_capability(dev, PCI_CAP_ID_PCIE)) { Too many leading tabs - consider code refactoring
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 549: /* Bridge is PCIe */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 550: p = (uint8_t *) *current; line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 551: p[0] = 0x2; /* Entry type */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 552: p[1] = dev->path.pci.devfn; /* Device */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 553: p[2] = dev->bus->secondary; /* Bus */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 554: p[3] = 0x0; /* Data */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 555: p[4] = 0x0; /* Padding */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 556: p[5] = 0x0; /* Padding */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 557: p[6] = 0x0; /* Padding */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 558: p[7] = 0x0; /* Padding */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 570: for (sibling = link->children; sibling; sibling = sibling->sibling) line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 571: add_ivrs_device_entries(dev, sibling, depth + 1, depth, root_level, current, length); line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 576: 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/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 613: 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/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 619: 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/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 624: ivrs->iv_info |= (0x40 << 15); /* Maximum supported virtual address size */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 625: ivrs->iv_info |= (0x30 << 8); /* Maximum supported physical address size */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 630: ivrs->ivhd.flags |= 0x10; /* Enable ATS support */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 632: ivrs->ivhd.device_id = 0x2 | (nb_dev->bus->secondary << 8); /* BDF <bus>:00.2 */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 633: ivrs->ivhd.capability_offset = 0x40; /* Capability block 0x40 (type 0xf, "Secure device") */ line over 80 characters
https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/nor... PS16, Line 655: add_ivrs_device_entries(NULL, all_devices, 0, -1, NULL, ¤t, &ivrs->ivhd.length); line over 80 characters
-
build bot (Jenkins) (Code Review)