Change in ...coreboot[master]: device/pci: Declare pci_root_bus()

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

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

Change subject: device/pci: Declare pci_root_bus()
......................................................................

device/pci: Declare pci_root_bus()

Change-Id: If675abb632fe68acd59ba0bdfef854da3e0839a9
Signed-off-by: Kyösti Mälkki 
---
M src/device/device_const.c
M src/include/device/device.h
M src/include/device/pci.h
3 files changed, 15 insertions(+), 7 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/34004/1

diff --git a/src/device/device_const.c b/src/device/device_const.c
index 6ce1d18..7fdb3dc 100644
--- a/src/device/device_const.c
+++ b/src/device/device_const.c
@@ -166,6 +166,8 @@
    const struct bus *parent, const struct device_path *path)
 {
    DEVTREE_CONST struct device *child;
+	if (!parent)
+		return NULL;
    for (child = parent->children; child; child = child->sibling) {
    	if (path_eq(path, &child->path))
    		break;
@@ -183,10 +185,19 @@
    return find_dev_path(parent, &path);
 }

-DEVTREE_CONST struct device *pcidev_path_on_root(pci_devfn_t devfn)
+DEVTREE_CONST struct bus *pci_root_bus(void)
 {
    DEVTREE_CONST struct device *pci_domain;

+	pci_domain = dev_find_path(NULL, DEVICE_PATH_DOMAIN);
+	if (!pci_domain)
+		 return NULL;
+
+	return pci_domain->link_list;
+}
+
+DEVTREE_CONST struct device *pcidev_path_on_root(pci_devfn_t devfn)
+{
    /* Work around pcidev_path_behind() below failing
     * due tue complicated devicetree with topology
     * being manipulated on-the-fly.
@@ -194,11 +205,7 @@
    if (CONFIG(NORTHBRIDGE_AMD_AMDFAM10))
    	return dev_find_slot(0, devfn);

-	pci_domain = dev_find_path(NULL, DEVICE_PATH_DOMAIN);
-	if (!pci_domain)
-		return NULL;
-
-	return pcidev_path_behind(pci_domain->link_list, devfn);
+	return pcidev_path_behind(pci_root_bus(), devfn);
 }

DEVTREE_CONST struct device *pcidev_on_root(uint8_t dev, uint8_t fn)
diff --git a/src/include/device/device.h b/src/include/device/device.h
index 8e9454c..a200de0 100644
--- a/src/include/device/device.h
+++ b/src/include/device/device.h
@@ -289,6 +289,7 @@
    	pci_devfn_t devfn);
 DEVTREE_CONST struct device *pcidev_path_on_root(pci_devfn_t devfn);
 DEVTREE_CONST struct device *pcidev_on_root(uint8_t dev, uint8_t fn);
+DEVTREE_CONST struct bus *pci_root_bus(void);

void scan_smbus(struct device *bus);
 void scan_generic_bus(struct device *bus);
diff --git a/src/include/device/pci.h b/src/include/device/pci.h
index f1de7bf..6f9093e 100644
--- a/src/include/device/pci.h
+++ b/src/include/device/pci.h
@@ -98,7 +98,7 @@

const char *pin_to_str(int pin);
 int get_pci_irq_pins(struct device *dev, struct device **parent_bdg);
-void pci_assign_irqs(unsigned int bus, unsigned int slot,
+void pci_assign_irqs(struct bus *parent, unsigned int slot,
    	     const unsigned char pIntAtoD[4]);
 const char *get_pci_class_name(struct device *dev);
 const char *get_pci_subclass_name(struct device *dev);

[build bot (Jenkins) (Code Review)]

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

Change subject: device/pci: Declare pci_root_bus()
......................................................................

Patch Set 1:

(2 comments)

https://review.coreboot.org/#/c/34004/1/src/device/device_const.c 
File src/device/device_const.c:

https://review.coreboot.org/#/c/34004/1/src/device/device_const.c@193 
PS1, Line 193: 	if (!pci_domain)
suspect code indent for conditional statements (8, 17)

https://review.coreboot.org/#/c/34004/1/src/device/device_const.c@194 
PS1, Line 194: 		 return NULL;
Statements should start on a tabstop

Change in coreboot[master]: device/pci: Declare pci_root_bus()

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

Hello build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/34004

to look at the new patch set (#2).

Change subject: device/pci: Declare pci_root_bus()
......................................................................

device/pci: Declare pci_root_bus()

Change-Id: If675abb632fe68acd59ba0bdfef854da3e0839a9
Signed-off-by: Kyösti Mälkki 
---
M src/device/device_const.c
M src/include/device/device.h
2 files changed, 17 insertions(+), 6 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/34004/2

Change in coreboot[master]: device/pci: Declare pci_root_bus()

[Paul Menzel (Code Review)]

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

Change subject: device/pci: Declare pci_root_bus()
......................................................................

Patch Set 2: Code-Review+1

Maybe add a short motivation for this function in the commit message.

Change in coreboot[master]: device/pci: Declare pci_root_bus()

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

Hello Paul Menzel, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/34004

to look at the new patch set (#3).

Change subject: device/pci: Declare pci_root_bus()
......................................................................

device/pci: Declare pci_root_bus()

This is used a lot, cache the result so search
of domain from devicetree is only done once.

Improvement only applies when MAYBE_STATIC evaluates
to static.

Change-Id: If675abb632fe68acd59ba0bdfef854da3e0839a9
Signed-off-by: Kyösti Mälkki 
---
M src/device/device_const.c
M src/include/device/device.h
2 files changed, 17 insertions(+), 6 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/34004/3

Change in coreboot[master]: device/pci: Declare pci_root_bus()

[Paul Menzel (Code Review)]

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

Change subject: device/pci: Declare pci_root_bus()
......................................................................

Patch Set 3: Code-Review+1

Change in coreboot[master]: device/pci: Declare pci_root_bus()

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34004 )

Change subject: device/pci: Declare pci_root_bus()
......................................................................

Patch Set 3: Code-Review+2

nice one

Change in coreboot[master]: device/pci: Declare pci_root_bus()

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34004 )

Change subject: device/pci: Declare pci_root_bus()
......................................................................

Patch Set 3: Code-Review+2
> Patch Set 3: Code-Review+2
>
> nice one

indeed

Change in coreboot[master]: device/pci: Declare pci_root_bus()

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

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

Change subject: device/pci: Declare pci_root_bus()
......................................................................

device/pci: Declare pci_root_bus()

This is used a lot, cache the result so search
of domain from devicetree is only done once.

Improvement only applies when MAYBE_STATIC evaluates
to static.

Change-Id: If675abb632fe68acd59ba0bdfef854da3e0839a9
Signed-off-by: Kyösti Mälkki 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/34004
Tested-by: build bot (Jenkins) 
Reviewed-by: Paul Menzel 
Reviewed-by: Nico Huber 
Reviewed-by: Angel Pons 
---
M src/device/device_const.c
M src/include/device/device.h
2 files changed, 17 insertions(+), 6 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Nico Huber: Looks good to me, approved
  Paul Menzel: Looks good to me, but someone else must approve
  Angel Pons: Looks good to me, approved

diff --git a/src/device/device_const.c b/src/device/device_const.c
index 6ce1d18..1853b4c 100644
--- a/src/device/device_const.c
+++ b/src/device/device_const.c
@@ -183,10 +183,24 @@
    return find_dev_path(parent, &path);
 }

-DEVTREE_CONST struct device *pcidev_path_on_root(pci_devfn_t devfn)
+DEVTREE_CONST struct bus *pci_root_bus(void)
 {
    DEVTREE_CONST struct device *pci_domain;
+	MAYBE_STATIC DEVTREE_CONST struct bus *pci_root = NULL;

+	if (pci_root)
+		return pci_root;
+
+	pci_domain = dev_find_path(NULL, DEVICE_PATH_DOMAIN);
+	if (!pci_domain)
+		return NULL;
+
+	pci_root = pci_domain->link_list;
+	return pci_root;
+}
+
+DEVTREE_CONST struct device *pcidev_path_on_root(pci_devfn_t devfn)
+{
    /* Work around pcidev_path_behind() below failing
     * due tue complicated devicetree with topology
     * being manipulated on-the-fly.
@@ -194,11 +208,7 @@
    if (CONFIG(NORTHBRIDGE_AMD_AMDFAM10))
    	return dev_find_slot(0, devfn);

-	pci_domain = dev_find_path(NULL, DEVICE_PATH_DOMAIN);
-	if (!pci_domain)
-		return NULL;
-
-	return pcidev_path_behind(pci_domain->link_list, devfn);
+	return pcidev_path_behind(pci_root_bus(), devfn);
 }

DEVTREE_CONST struct device *pcidev_on_root(uint8_t dev, uint8_t fn)
diff --git a/src/include/device/device.h b/src/include/device/device.h
index 8e9454c..a200de0 100644
--- a/src/include/device/device.h
+++ b/src/include/device/device.h
@@ -289,6 +289,7 @@
    	pci_devfn_t devfn);
 DEVTREE_CONST struct device *pcidev_path_on_root(pci_devfn_t devfn);
 DEVTREE_CONST struct device *pcidev_on_root(uint8_t dev, uint8_t fn);
+DEVTREE_CONST struct bus *pci_root_bus(void);

void scan_smbus(struct device *bus);
 void scan_generic_bus(struct device *bus);