Change in coreboot[master]: device/oprom: Replace uses of dev_find_slot()

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34083 )

Change subject: device/oprom: Replace uses of dev_find_slot()
......................................................................

device/oprom: Replace uses of dev_find_slot()

The call to dev_find_slot() may return PCI devices that
are disabled or unaccessible, as PCI enumeration does
not remove nodes from all_devices linked list.

Use PCI topology search instead.

Change-Id: I00233177e5572ca79002a7d141cda1b94b966330
Signed-off-by: Kyösti Mälkki 
---
M src/device/oprom/realmode/x86_interrupts.c
M src/device/oprom/yabel/interrupt.c
M src/device/oprom/yabel/io.c
3 files changed, 5 insertions(+), 5 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/34083/1

diff --git a/src/device/oprom/realmode/x86_interrupts.c b/src/device/oprom/realmode/x86_interrupts.c
index 2629ab9..8e3a51e 100644
--- a/src/device/oprom/realmode/x86_interrupts.c
+++ b/src/device/oprom/realmode/x86_interrupts.c
@@ -174,7 +174,7 @@
    	devfn = X86_EBX & 0xff;
    	bus = X86_EBX >> 8;
    	reg = X86_EDI;
-		dev = dev_find_slot(bus, devfn);
+		dev = pcidev_path_on_bus(bus, devfn);
    	if (!dev) {
    		printk(BIOS_DEBUG, "0x%x: BAD DEVICE bus %d devfn 0x%x\n", func, bus, devfn);
    		// Or are we supposed to return PCIBIOS_NODEV?
diff --git a/src/device/oprom/yabel/interrupt.c b/src/device/oprom/yabel/interrupt.c
index 2bc238e..338156f 100644
--- a/src/device/oprom/yabel/interrupt.c
+++ b/src/device/oprom/yabel/interrupt.c
@@ -409,8 +409,8 @@
    	if ((bus == bios_device.bus) && (devfn == bios_device.devfn)) {
    		dev = bios_device.dev;
    	} else if (CONFIG(YABEL_PCI_ACCESS_OTHER_DEVICES)) {
-			dev = dev_find_slot(bus, devfn);
-			DEBUG_PRINTF_INTR("%s(): function: %x: dev_find_slot() returned: %s\n",
+			dev = pcidev_path_on_bus(bus, devfn);
+			DEBUG_PRINTF_INTR("%s(): function: %x: pcidev_path_on_bus() returned: %s\n",
    			  __func__, M.x86.R_AX, dev_path(dev));
    	}

diff --git a/src/device/oprom/yabel/io.c b/src/device/oprom/yabel/io.c
index 96c9224..94cbe8f 100644
--- a/src/device/oprom/yabel/io.c
+++ b/src/device/oprom/yabel/io.c
@@ -435,8 +435,8 @@
    if ((bus == bios_device.bus) && (devfn == bios_device.devfn)) {
    	dev = bios_device.dev;
    } else if (CONFIG(YABEL_PCI_ACCESS_OTHER_DEVICES)) {
-		dev = dev_find_slot(bus, devfn);
-		DEBUG_PRINTF_INTR("%s(): dev_find_slot() returned: %s\n",
+		dev = pcidev_path_on_bus(bus, devfn);
+		DEBUG_PRINTF_INTR("%s(): pcidev_path_on_bus() returned: %s\n",
    		__func__, dev_path(dev));
    }

[build bot (Jenkins) (Code Review)]

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34083 )

Change subject: device/oprom: Replace uses of dev_find_slot()
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/34083/1/src/device/oprom/yabel/inte... 
File src/device/oprom/yabel/interrupt.c:

https://review.coreboot.org/c/coreboot/+/34083/1/src/device/oprom/yabel/inte... 
PS1, Line 413: 			DEBUG_PRINTF_INTR("%s(): function: %x: pcidev_path_on_bus() returned: %s\n",
line over 96 characters

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34083 )

Change subject: device/oprom: Replace uses of dev_find_slot()
......................................................................

Patch Set 1: Code-Review+1

[build bot (Jenkins) (Code Review)]

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34083 )

Change subject: device/oprom: Replace uses of dev_find_slot()
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/34083/2/src/device/oprom/yabel/inte... 
File src/device/oprom/yabel/interrupt.c:

https://review.coreboot.org/c/coreboot/+/34083/2/src/device/oprom/yabel/inte... 
PS2, Line 413: 			DEBUG_PRINTF_INTR("%s(): function: %x: pcidev_path_on_bus() returned: %s\n",
line over 96 characters

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34083 )

Change subject: device/oprom: Replace uses of dev_find_slot()
......................................................................

Patch Set 2:

Mike, could you test running the oproms you have in secure mode?

[Mike Banon (Code Review)]

Mike Banon has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34083 )

Change subject: device/oprom: Replace uses of dev_find_slot()
......................................................................

Patch Set 2:
> Patch Set 2:
>
> Mike, could you test running the oproms you have in secure mode?

Secure mode is already in my default config (shared as CB:32352), and very recently the OpROMs were working fine. LWeekends are coming and I'll be happy to test your this and some other of your recent patches to see if the things still work. Meanwhile I'll be happy if you could share more ideas about fixing the AMD 16h speeds (please see an e-mail), and will also try them as soon as possible.

[Mike Banon (Code Review)]

Mike Banon has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34083 )

Change subject: device/oprom: Replace uses of dev_find_slot()
......................................................................

Patch Set 2: Code-Review+2

[Mike Banon (Code Review)]

Mike Banon has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34083 )

Change subject: device/oprom: Replace uses of dev_find_slot()
......................................................................

Patch Set 2:

Today I tested this change on G505S using the same CB:32352 config (has a Secure Mode aka YABEL for oproms) - together with G505S dGPU patches from CB:31929. Everything is still working good.

[Mike Banon (Code Review)]

Mike Banon has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34083 )

Change subject: device/oprom: Replace uses of dev_find_slot()
......................................................................

Patch Set 2:

Maybe it's the time to merge this one as well...

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34083 )

Change subject: device/oprom: Replace uses of dev_find_slot()
......................................................................

Patch Set 2:
> Patch Set 2:
>
> Maybe it's the time to merge this one as well...

Dependency on CB:34077 with a TBD comment I am not happy about.

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34083 )

Change subject: device/oprom: Replace uses of dev_find_slot()
......................................................................

device/oprom: Replace uses of dev_find_slot()

The call to dev_find_slot() may return PCI devices that
are disabled or unaccessible, as PCI enumeration does
not remove nodes from all_devices linked list.

Use PCI topology search instead.

Change-Id: I00233177e5572ca79002a7d141cda1b94b966330
Signed-off-by: Kyösti Mälkki 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/34083
Reviewed-by: Mike Banon 
Reviewed-by: Paul Menzel 
Tested-by: build bot (Jenkins) 
---
M src/device/oprom/realmode/x86_interrupts.c
M src/device/oprom/yabel/interrupt.c
M src/device/oprom/yabel/io.c
3 files changed, 5 insertions(+), 5 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Paul Menzel: Looks good to me, but someone else must approve
  Mike Banon: Looks good to me, approved

diff --git a/src/device/oprom/realmode/x86_interrupts.c b/src/device/oprom/realmode/x86_interrupts.c
index 2629ab9..8e3a51e 100644
--- a/src/device/oprom/realmode/x86_interrupts.c
+++ b/src/device/oprom/realmode/x86_interrupts.c
@@ -174,7 +174,7 @@
    	devfn = X86_EBX & 0xff;
    	bus = X86_EBX >> 8;
    	reg = X86_EDI;
-		dev = dev_find_slot(bus, devfn);
+		dev = pcidev_path_on_bus(bus, devfn);
    	if (!dev) {
    		printk(BIOS_DEBUG, "0x%x: BAD DEVICE bus %d devfn 0x%x\n", func, bus, devfn);
    		// Or are we supposed to return PCIBIOS_NODEV?
diff --git a/src/device/oprom/yabel/interrupt.c b/src/device/oprom/yabel/interrupt.c
index 2bc238e..338156f 100644
--- a/src/device/oprom/yabel/interrupt.c
+++ b/src/device/oprom/yabel/interrupt.c
@@ -409,8 +409,8 @@
    	if ((bus == bios_device.bus) && (devfn == bios_device.devfn)) {
    		dev = bios_device.dev;
    	} else if (CONFIG(YABEL_PCI_ACCESS_OTHER_DEVICES)) {
-			dev = dev_find_slot(bus, devfn);
-			DEBUG_PRINTF_INTR("%s(): function: %x: dev_find_slot() returned: %s\n",
+			dev = pcidev_path_on_bus(bus, devfn);
+			DEBUG_PRINTF_INTR("%s(): function: %x: pcidev_path_on_bus() returned: %s\n",
    			  __func__, M.x86.R_AX, dev_path(dev));
    	}

diff --git a/src/device/oprom/yabel/io.c b/src/device/oprom/yabel/io.c
index 8ae91d4..e8c41ce 100644
--- a/src/device/oprom/yabel/io.c
+++ b/src/device/oprom/yabel/io.c
@@ -435,8 +435,8 @@
    if ((bus == bios_device.bus) && (devfn == bios_device.devfn)) {
    	dev = bios_device.dev;
    } else if (CONFIG(YABEL_PCI_ACCESS_OTHER_DEVICES)) {
-		dev = dev_find_slot(bus, devfn);
-		DEBUG_PRINTF_INTR("%s(): dev_find_slot() returned: %s\n",
+		dev = pcidev_path_on_bus(bus, devfn);
+		DEBUG_PRINTF_INTR("%s(): pcidev_path_on_bus() returned: %s\n",
    		__func__, dev_path(dev));
    }