Change in coreboot[master]: devicetree: Add accessors for chip_info

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

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

Change subject: devicetree: Add accessors for chip_info
......................................................................

devicetree: Add accessors for chip_info

Apply uniform style of error messages for missing device
nodes and chip_info.

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

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/34298/1

diff --git a/src/device/device_const.c b/src/device/device_const.c
index e9cf5eb..ba12249 100644
--- a/src/device/device_const.c
+++ b/src/device/device_const.c
@@ -244,6 +244,33 @@
    return dev_find_slot(0, devfn);
 }

+DEVTREE_CONST void *config_of(DEVTREE_CONST struct device *dev)
+{
+	if (dev && dev->chip_info)
+		return dev->chip_info;
+
+	printk(BIOS_ERR, "BUG: %s has no chip_info\n", dev_path(dev));
+	return NULL;
+}
+
+DEVTREE_CONST void *config_of_path(pci_devfn_t devfn)
+{
+	DEVTREE_CONST struct device *dev = pcidev_path_on_root(devfn);
+	if (dev)
+		return config_of(dev);
+
+	dev = dev_find_slot(0, devfn);
+	if (dev) {
+		printk(BIOS_ERR, "BUG: config of hidden 00:%02x.%u requested\n",
+		       devfn >> 3, devfn & 7);
+		return config_of(dev);
+	}
+
+	printk(BIOS_ERR, "BUG: config of missing 00:%02x.%u requested\n",
+	       devfn >> 3, devfn & 7);
+	return NULL;
+}
+
 /**
  * Given an SMBus bus and a device number, find the device structure.
  *
diff --git a/src/include/device/device.h b/src/include/device/device.h
index 24b6421..f8485a3 100644
--- a/src/include/device/device.h
+++ b/src/include/device/device.h
@@ -294,6 +294,10 @@
 DEVTREE_CONST struct device *dev_find_slot(unsigned int bus, unsigned int devfn);
 DEVTREE_CONST struct device *pcidev_path_on_root_debug(pci_devfn_t devfn, const char *func);

+/* Safely discover chip_info. */
+DEVTREE_CONST void *config_of(DEVTREE_CONST struct device *dev);
+DEVTREE_CONST void *config_of_path(pci_devfn_t devfn);
+
 void scan_smbus(struct device *bus);
 void scan_generic_bus(struct device *bus);
 void scan_static_bus(struct device *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/+/34298

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

Change subject: devicetree: Add accessors for chip_info
......................................................................

devicetree: Add accessors for chip_info

Apply uniform style of error messages for missing device
nodes and chip_info.

Change-Id: I70def4599509b8193e44ea3f02c4906f865b4469
Signed-off-by: Kyösti Mälkki 
---
M src/device/device_const.c
M src/include/device/device.h
2 files changed, 35 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/34298/2

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34298 )

Change subject: devicetree: Add accessors for chip_info
......................................................................

Patch Set 2: Code-Review+2

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

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

Change subject: devicetree: Add accessors for chip_info
......................................................................

Patch Set 2:

Hold from merge until validated by testing of CB:34299.

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

Hello build bot (Jenkins), Furquan Shaikh,

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

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

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

Change subject: devicetree: Add accessors for chip_info
......................................................................

devicetree: Add accessors for chip_info

Apply uniform style of error messages for missing device
nodes and chip_info.

Change-Id: I70def4599509b8193e44ea3f02c4906f865b4469
Signed-off-by: Kyösti Mälkki 
---
M src/device/device_const.c
M src/include/device/device.h
2 files changed, 35 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/34298/3

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34298 )

Change subject: devicetree: Add accessors for chip_info
......................................................................

Patch Set 3: Code-Review+2

[Angel Pons (Code Review)]

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

Change subject: devicetree: Add accessors for chip_info
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/34298/3/src/include/device/device.h 
File src/include/device/device.h:

https://review.coreboot.org/c/coreboot/+/34298/3/src/include/device/device.h... 
PS3, Line 306: 	devtree_die();
Could this get __func__ passed as parameter? Otherwise, the die() message is rather imprecise.

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

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

Change subject: devicetree: Add accessors for chip_info
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/34298/3/src/include/device/device.h 
File src/include/device/device.h:

https://review.coreboot.org/c/coreboot/+/34298/3/src/include/device/device.h... 
PS3, Line 306: 	devtree_die();
> Could this get __func__ passed as parameter? Otherwise, the die() message is rather imprecise.

We could wrap the call inside preprocessor macro to do that. Maybe adjust the error with advice 'Enable DEBUG_xxx to resolve issue' and leave it conditional. To use __func__ alone may not be a unique identifier and I don't want to pull __file__ in by default.

With such DEBUG_xxx enabled, maybe this should fall through with chip_info pointing to some zero-initialized array and die() only after console_init() so that there is chance to actually display the message.

Also auto-discovered devices that are not present in static devicetree will never have chip_info assigned, so we need some more logic here until we expand the use of this function to drivers/. But current use for soc/intel should be fine.

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

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

Change subject: devicetree: Add accessors for chip_info
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/34298/3/src/include/device/device.h 
File src/include/device/device.h:

https://review.coreboot.org/c/coreboot/+/34298/3/src/include/device/device.h... 
PS3, Line 306: 	devtree_die();
> We could wrap the call inside preprocessor macro to do that. […]

Done

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

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

Change subject: devicetree: Add accessors for chip_info
......................................................................

devicetree: Add accessors for chip_info

Apply uniform style of error messages for missing device
nodes and chip_info.

Change-Id: I70def4599509b8193e44ea3f02c4906f865b4469
Signed-off-by: Kyösti Mälkki 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/34298
Tested-by: build bot (Jenkins) 
Reviewed-by: Furquan Shaikh 
---
M src/device/device_const.c
M src/include/device/device.h
2 files changed, 35 insertions(+), 1 deletion(-)

Approvals:
  build bot (Jenkins): Verified
  Furquan Shaikh: Looks good to me, approved

diff --git a/src/device/device_const.c b/src/device/device_const.c
index f2f0177..c472aea 100644
--- a/src/device/device_const.c
+++ b/src/device/device_const.c
@@ -240,12 +240,22 @@
    if (dev)
    	return dev;

-	printk(BIOS_ERR, "BUG: %s requests hidden 00:%02x.%u\n", func, devfn >> 3, devfn & 7);
+	devtree_bug(func, devfn);

/* FIXME: This can return wrong device. */
    return dev_find_slot(0, devfn);
 }

+void devtree_bug(const char *func, pci_devfn_t devfn)
+{
+	printk(BIOS_ERR, "BUG: %s requests hidden 00:%02x.%u\n", func, devfn >> 3, devfn & 7);
+}
+
+void __noreturn devtree_die(void)
+{
+	die("DEVTREE: dev or chip_info is NULL\n");
+}
+
 /**
  * Given an SMBus bus and a device number, find the device structure.
  *
diff --git a/src/include/device/device.h b/src/include/device/device.h
index 4ffbff4..8e1e62a 100644
--- a/src/include/device/device.h
+++ b/src/include/device/device.h
@@ -294,6 +294,30 @@
 DEVTREE_CONST struct device *dev_find_slot(unsigned int bus, unsigned int devfn);
 DEVTREE_CONST struct device *pcidev_path_on_root_debug(pci_devfn_t devfn, const char *func);

+/* Robust discovery of chip_info. */
+void devtree_bug(const char *func, pci_devfn_t devfn);
+void __noreturn devtree_die(void);
+
+static inline DEVTREE_CONST void *config_of(const struct device *dev)
+{
+	if (dev && dev->chip_info)
+		return dev->chip_info;
+
+	devtree_die();
+}
+
+static inline DEVTREE_CONST void *config_of_path(pci_devfn_t devfn)
+{
+	const struct device *dev = pcidev_path_on_root(devfn);
+	if (dev)
+		return config_of(dev);
+
+	devtree_bug(__func__, devfn);
+
+	dev = dev_find_slot(0, devfn);
+	return config_of(dev);
+}
+
 void scan_smbus(struct device *bus);
 void scan_generic_bus(struct device *bus);
 void scan_static_bus(struct device *bus);