[coreboot-gerrit] Change in coreboot[master]: nb/amd/pi/00730F01: Add initial native IVRS support

build bot (Jenkins) (Code Review) gerrit at coreboot.org
Mon Jul 23 15:50:17 CEST 2018


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/northbridge.c
File src/northbridge/amd/pi/00730F01/northbridge.c:

https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@441
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/northbridge.c@453
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/northbridge.c@460
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/northbridge.c@460
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/northbridge.c@462
PS16, Line 462: 							p = (uint8_t *) *current;
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@463
PS16, Line 463: 							p[0] = 0x2;			/* Entry type */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@464
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/northbridge.c@465
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/northbridge.c@466
PS16, Line 466: 							p[3] = 0x0;			/* Data */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@467
PS16, Line 467: 							p[4] = 0x0;			/* Padding */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@468
PS16, Line 468: 							p[5] = 0x0;			/* Padding */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@469
PS16, Line 469: 							p[6] = 0x0;			/* Padding */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@470
PS16, Line 470: 							p[7] = 0x0;			/* Padding */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@473
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/northbridge.c@473
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/northbridge.c@475
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/northbridge.c@475
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/northbridge.c@476
PS16, Line 476: 							/* FCH PCIe bridge device */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@477
PS16, Line 477: 							p = (uint8_t *) *current;
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@478
PS16, Line 478: 							p[0] = 0x2;			/* Entry type */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@479
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/northbridge.c@480
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/northbridge.c@481
PS16, Line 481: 							p[3] = 0x0;			/* Data */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@482
PS16, Line 482: 							p[4] = 0x0;			/* Padding */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@483
PS16, Line 483: 							p[5] = 0x0;			/* Padding */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@484
PS16, Line 484: 							p[6] = 0x0;			/* Padding */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@485
PS16, Line 485: 							p[7] = 0x0;			/* Padding */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@488
PS16, Line 488: 						} else {
Too many leading tabs - consider code refactoring


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@489
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/northbridge.c@489
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/northbridge.c@490
PS16, Line 490: 								/* SMBUS controller */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@491
PS16, Line 491: 								p = (uint8_t *) *current;
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@492
PS16, Line 492: 								p[0] = 0x2;			/* Entry type */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@493
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/northbridge.c@494
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/northbridge.c@495
PS16, Line 495: 								p[3] = 0x97;			/* Data */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@496
PS16, Line 496: 								p[4] = 0x0;			/* Padding */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@497
PS16, Line 497: 								p[5] = 0x0;			/* Padding */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@498
PS16, Line 498: 								p[6] = 0x0;			/* Padding */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@499
PS16, Line 499: 								p[7] = 0x0;			/* Padding */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@502
PS16, Line 502: 							} else {
Too many leading tabs - consider code refactoring


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@503
PS16, Line 503: 								/* Other southbridge device */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@504
PS16, Line 504: 								p = (uint8_t *) *current;
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@505
PS16, Line 505: 								p[0] = 0x2;			/* Entry type */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@506
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/northbridge.c@507
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/northbridge.c@508
PS16, Line 508: 								p[3] = 0x0;			/* Data */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@509
PS16, Line 509: 								p[4] = 0x0;			/* Padding */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@510
PS16, Line 510: 								p[5] = 0x0;			/* Padding */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@511
PS16, Line 511: 								p[6] = 0x0;			/* Padding */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@512
PS16, Line 512: 								p[7] = 0x0;			/* Padding */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@518
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/northbridge.c@518
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/northbridge.c@519
PS16, Line 519: 							/* Device behind bridge */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@520
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/northbridge.c@520
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/northbridge.c@521
PS16, Line 521: 								/* Device is PCIe */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@522
PS16, Line 522: 								p = (uint8_t *) *current;
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@523
PS16, Line 523: 								p[0] = 0x2;			/* Entry type */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@524
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/northbridge.c@525
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/northbridge.c@526
PS16, Line 526: 								p[3] = 0x0;			/* Data */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@527
PS16, Line 527: 								p[4] = 0x0;			/* Padding */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@528
PS16, Line 528: 								p[5] = 0x0;			/* Padding */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@529
PS16, Line 529: 								p[6] = 0x0;			/* Padding */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@530
PS16, Line 530: 								p[7] = 0x0;			/* Padding */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@533
PS16, Line 533: 							} else {
Too many leading tabs - consider code refactoring


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@534
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/northbridge.c@535
PS16, Line 535: 								p = (uint8_t *) *current;
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@536
PS16, Line 536: 								p[0] = 0x42;			/* Entry type */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@537
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/northbridge.c@538
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/northbridge.c@539
PS16, Line 539: 								p[3] = 0x0;			/* Data */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@540
PS16, Line 540: 								p[4] = 0x0;			/* Reserved */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@541
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/northbridge.c@542
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/northbridge.c@543
PS16, Line 543: 								p[7] = 0x0;			/* Reserved */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@547
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/northbridge.c@547
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/northbridge.c@548
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/northbridge.c@548
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/northbridge.c@549
PS16, Line 549: 								/* Bridge is PCIe */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@550
PS16, Line 550: 								p = (uint8_t *) *current;
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@551
PS16, Line 551: 								p[0] = 0x2;			/* Entry type */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@552
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/northbridge.c@553
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/northbridge.c@554
PS16, Line 554: 								p[3] = 0x0;			/* Data */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@555
PS16, Line 555: 								p[4] = 0x0;			/* Padding */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@556
PS16, Line 556: 								p[5] = 0x0;			/* Padding */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@557
PS16, Line 557: 								p[6] = 0x0;			/* Padding */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@558
PS16, Line 558: 								p[7] = 0x0;			/* Padding */
line over 80 characters


https://review.coreboot.org/#/c/15164/16/src/northbridge/amd/pi/00730F01/northbridge.c@570
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/northbridge.c@571
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/northbridge.c@576
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/northbridge.c@613
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/northbridge.c@619
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/northbridge.c@624
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/northbridge.c@625
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/northbridge.c@630
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/northbridge.c@632
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/northbridge.c@633
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/northbridge.c@655
PS16, Line 655: 	add_ivrs_device_entries(NULL, all_devices, 0, -1, NULL, &current, &ivrs->ivhd.length);
line over 80 characters



-- 
To view, visit https://review.coreboot.org/15164
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ae789f75363435accd14a1b556e1570f43f94c4
Gerrit-Change-Number: 15164
Gerrit-PatchSet: 16
Gerrit-Owner: Timothy Pearson <tpearson at raptorengineering.com>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki at gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth at google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki at gmail.com>
Gerrit-Reviewer: Timothy Pearson <tpearson at raptorengineering.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply at coreboot.org>
Gerrit-CC: Piotr Król <piotr.krol at 3mdeb.com>
Gerrit-Comment-Date: Mon, 23 Jul 2018 13:50:17 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20180723/59fb6410/attachment.html>


More information about the coreboot-gerrit mailing list