Skip to contentSkip to conversation

pci: Support parsing PCI controller DT subnodes

PATCH New Marek Vasut 3 participants 12 messages Oct 10, 2018 – Oct 22, 2018 PCI

lore
Share & tools
lore
#0 lore raw

PATCH 1/4 pci: Support parsing PCI controller DT subnodes

The PCI controller can have DT subnodes describing extra properties of particular PCI devices, ie. a PHY attached to an EHCI controller on a PCI bus. This patch parses those DT subnodes and assigns a node to the PCI device instance, so that the driver can extract details from that node and ie. configure the PHY using the PHY subsystem.

---
V2: Use ofnode_read_pci_addr() instead of ofnode_get_addr_size()

---
 drivers/pci/pci-uclass.c | 32 +++++++++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)
drivers/pci/pci-uclass.c +29 -3
index eb118f3496..da49c96ed5 100644
@@ -90,6 +90,27 @@ int pci_get_ff(enum pci_size_t size)
 	}
 }
 
+static void pci_dev_find_ofnode(struct udevice *bus, phys_addr_t bdf,
+				ofnode *rnode)
+{
+	struct fdt_pci_addr addr;
+	ofnode node;
+	int ret;
+
+	dev_for_each_subnode(node, bus) {
+		ret = ofnode_read_pci_addr(node, FDT_PCI_SPACE_CONFIG, "reg",
+					   &addr);
+		if (ret)
+			continue;
+
+		if (PCI_MASK_BUS(addr.phys_hi) != PCI_MASK_BUS(bdf))
+			continue;
+
+		*rnode = node;
+		break;
+	}
+};
+
 int pci_bus_find_devfn(struct udevice *bus, pci_dev_t find_devfn,
 		       struct udevice **devp)
 {
@@ -641,6 +662,7 @@ static int pci_find_and_bind_driver(struct udevice *parent,
 				    pci_dev_t bdf, struct udevice **devp)
 {
 	struct pci_driver_entry *start, *entry;
+	ofnode node = ofnode_null();
 	const char *drv;
 	int n_ents;
 	int ret;
@@ -651,6 +673,10 @@ static int pci_find_and_bind_driver(struct udevice *parent,
 
 	debug("%s: Searching for driver: vendor=%x, device=%x\n", __func__,
 	      find_id->vendor, find_id->device);
+
+	/* Determine optional OF node */
+	pci_dev_find_ofnode(parent, bdf, &node);
+
 	start = ll_entry_start(struct pci_driver_entry, pci_driver_entry);
 	n_ents = ll_entry_count(struct pci_driver_entry, pci_driver_entry);
 	for (entry = start; entry != start + n_ents; entry++) {
@@ -684,8 +710,8 @@ static int pci_find_and_bind_driver(struct udevice *parent,
 			 * find another driver. For now this doesn't seem
 			 * necesssary, so just bind the first match.
 			 */
-			ret = device_bind(parent, drv, drv->name, NULL, -1,
-					  &dev);
+			ret = device_bind_ofnode(parent, drv, drv->name, NULL,
+						 node, &dev);
 			if (ret)
 				goto error;
 			debug("%s: Match found: %s\n", __func__, drv->name);
@@ -712,7 +738,7 @@ static int pci_find_and_bind_driver(struct udevice *parent,
 		return -ENOMEM;
 	drv = bridge ? "pci_bridge_drv" : "pci_generic_drv";
 
-	ret = device_bind_driver(parent, drv, str, devp);
+	ret = device_bind_driver_to_node(parent, drv, str, node, devp);
 	if (ret) {
 		debug("%s: Failed to bind generic driver: %d\n", __func__, ret);
 		free(str);
Signature · 1 line
2.18.0
#1 lore raw

PATCH 2/4 pci: Update documentation to make 'compatible' string optional

Reword the documentation to make it clear the compatible string is now optional, yet still matching on it takes precedence over PCI IDs and PCI classes.

---
 doc/driver-model/pci-info.txt | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)
doc/driver-model/pci-info.txt +9 -5
index e1701d1fbc..14364c5c75 100644
@@ -34,11 +34,15 @@ under that bus.
 Note that this is all done on a lazy basis, as needed, so until something is
 touched on PCI (eg: a call to pci_find_devices()) it will not be probed.
 
-PCI devices can appear in the flattened device tree. If they do this serves to
-specify the driver to use for the device. In this case they will be bound at
-first. Each PCI device node must have a compatible string list as well as a
-<reg> property, as defined by the IEEE Std 1275-1994 PCI bus binding document
-v2.1. Note we must describe PCI devices with the same bus hierarchy as the
+PCI devices can appear in the flattened device tree. If they do, their node
+often contains extra information which cannot be derived from the PCI IDs or
+PCI class of the device. Each PCI device node must have a <reg> property, as
+defined by the IEEE Std 1275-1994 PCI bus binding document v2.1. Compatible
+string list is optional and generally not needed, since PCI is discoverable
+bus, albeit there are justified exceptions. If the compatible string is
+present, matching on it takes precedence over PCI IDs and PCI classes.
+
+Note we must describe PCI devices with the same bus hierarchy as the
 hardware, otherwise driver model cannot detect the correct parent/children
 relationship during PCI bus enumeration thus PCI devices won't be bound to
 their drivers accordingly. A working example like below:
Signature · 1 line
2.18.0
#2 lore raw

PATCH 2/4 pci: Update documentation to make 'compatible' string optional

Hi Marek,

Quoted 5 lines · Marek Vasut

Reword the documentation to make it clear the compatible string is now optional, yet still matching on it takes precedence over PCI IDs and PCI classes.

Quoted 7 lines
---
 doc/driver-model/pci-info.txt | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

I think I provided my RB tag to previous version, and if there is no changes, I would appreciate it the RB tag was added in the new version to save some time.

Quoted 16 lines
doc/driver-model/pci-info.txt +3 -5
index e1701d1fbc..14364c5c75 100644
@@ -34,11 +34,15 @@ under that bus.
 Note that this is all done on a lazy basis, as needed, so until something is
 touched on PCI (eg: a call to pci_find_devices()) it will not be probed.
 
-PCI devices can appear in the flattened device tree. If they do this serves to
-specify the driver to use for the device. In this case they will be bound at
-first. Each PCI device node must have a compatible string list as well as a
-<reg> property, as defined by the IEEE Std 1275-1994 PCI bus binding document
-v2.1. Note we must describe PCI devices with the same bus hierarchy as the
+PCI devices can appear in the flattened device tree. If they do, their node
+often contains extra information which cannot be derived from the PCI IDs or
+PCI class of the device. Each PCI device node must have a <reg> property, as

doc/driver-model/pci-info.txt under that bus.

Quoted 10 lines
doc/driver-model/pci-info.txt +6 -1
+defined by the IEEE Std 1275-1994 PCI bus binding document v2.1. Compatible
+string list is optional and generally not needed, since PCI is discoverable
+bus, albeit there are justified exceptions. If the compatible string is
+present, matching on it takes precedence over PCI IDs and PCI classes.
+
+Note we must describe PCI devices with the same bus hierarchy as the
 hardware, otherwise driver model cannot detect the correct parent/children
 relationship during PCI bus enumeration thus PCI devices won't be bound to
 their drivers accordingly. A working example like below:
--

Regards,
Bin

#3 lore raw

PATCH 2/4 pci: Update documentation to make 'compatible' string optional

In reply to Marek Vasut
Quoted 8 lines · Marek Vasut

Reword the documentation to make it clear the compatible string is now optional, yet still matching on it takes precedence over PCI IDs and PCI classes.

Applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20181021/76724d87/attachment.sig>
#4 lore raw

PATCH 3/4 test: Add PCI device entry without compat string and with DT node

In reply to Marek Vasut

Add PCI entry without compatible string and with a DT node only with reg = <...> property into the DT. This is needed for the tests to verify whether such a setup creates an U-Boot PCI device with the DT node associated with it in udevice.node.

---
 arch/sandbox/dts/test.dts | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
arch/sandbox/dts/test.dts +5 -1
index ad94901fa1..0a6b86999d 100644
@@ -363,7 +363,11 @@
 		ranges = <0x02000000 0 0x30000000 0x30000000 0 0x2000
 				0x01000000 0 0x40000000 0x40000000 0 0x2000>;
 		sandbox,dev-info = <0x08 0x00 0x1234 0x5678
-				    0x0c 0x00 0x1234 0x5678>;
+				    0x0c 0x00 0x1234 0x5678
+				    0x10 0x00 0x1234 0x5678>;
+		pci at 10,0 {
+			reg = <0x8000 0 0 0 0>;
+		};
 	};
 
 	pci2: pci-controller2 {
Signature · 1 line
2.18.0
#5 lore raw

PATCH 3/4 test: Add PCI device entry without compat string and with DT node

Quoted 6 lines · Marek Vasut

Add PCI entry without compatible string and with a DT node only with reg = <...> property into the DT. This is needed for the tests to verify whether such a setup creates an U-Boot PCI device with the DT node associated with it in udevice.node.

Quoted 7 lines
---
 arch/sandbox/dts/test.dts | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

nits: I believe this can be squashed to patch [4/4] to make it a complete test case update.

#6 lore raw

PATCH 3/4 test: Add PCI device entry without compat string and with DT node

In reply to Marek Vasut
Quoted 9 lines · Marek Vasut

Add PCI entry without compatible string and with a DT node only with reg = <...> property into the DT. This is needed for the tests to verify whether such a setup creates an U-Boot PCI device with the DT node associated with it in udevice.node.

Applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20181021/6711fb48/attachment.sig>
#7 lore raw

PATCH 4/4 test: Add test for PCI device without compat string and with DT node

In reply to Marek Vasut

Add test which checks if a PCI device described in DT with an entry and reg = <...> property, but without compatible string results in a valid U-Boot PCI udevice with the udevice.node populated with reference to this DT node. Also check if the other PCI device without a DT node does not contain any bogus udevice.node.

---
 test/dm/pci.c | 5 +++++
 1 file changed, 5 insertions(+)
test/dm/pci.c +5 -0
index 869970072d..a1dedd84a7 100644
@@ -119,8 +119,13 @@ static int dm_test_pci_drvdata(struct unit_test_state *uts)
 
 	ut_assertok(dm_pci_bus_find_bdf(PCI_BDF(1, 0x08, 0), &swap));
 	ut_asserteq(SWAP_CASE_DRV_DATA, swap->driver_data);
+	ut_assertok(dev_of_valid(swap));
 	ut_assertok(dm_pci_bus_find_bdf(PCI_BDF(1, 0x0c, 0), &swap));
 	ut_asserteq(SWAP_CASE_DRV_DATA, swap->driver_data);
+	ut_assertok(dev_of_valid(swap));
+	ut_assertok(dm_pci_bus_find_bdf(PCI_BDF(1, 0x10, 0), &swap));
+	ut_asserteq(SWAP_CASE_DRV_DATA, swap->driver_data);
+	ut_assertok(!dev_of_valid(swap));
 
 	return 0;
 }
Signature · 1 line
2.18.0
#8 lore raw

PATCH 4/4 test: Add test for PCI device without compat string and with DT node

Quoted 8 lines · Marek Vasut

Add test which checks if a PCI device described in DT with an entry and reg = <...> property, but without compatible string results in a valid U-Boot PCI udevice with the udevice.node populated with reference to this DT node. Also check if the other PCI device without a DT node does not contain any bogus udevice.node.

Quoted 7 lines
---
 test/dm/pci.c | 5 +++++
 1 file changed, 5 insertions(+)
#9 lore raw

PATCH 4/4 test: Add test for PCI device without compat string and with DT node

In reply to Marek Vasut
Quoted 11 lines · Marek Vasut

Add test which checks if a PCI device described in DT with an entry and reg = <...> property, but without compatible string results in a valid U-Boot PCI udevice with the udevice.node populated with reference to this DT node. Also check if the other PCI device without a DT node does not contain any bogus udevice.node.

Applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20181021/ed26ab54/attachment.sig>
#10 lore raw

PATCH 1/4 pci: Support parsing PCI controller DT subnodes

In reply to Marek Vasut
Quoted 7 lines · Marek Vasut

The PCI controller can have DT subnodes describing extra properties of particular PCI devices, ie. a PHY attached to an EHCI controller on a PCI bus. This patch parses those DT subnodes and assigns a node to the PCI device instance, so that the driver can extract details from that node and ie. configure the PHY using the PHY subsystem.

Quoted 9 lines

---
V2: Use ofnode_read_pci_addr() instead of ofnode_get_addr_size()

---
 drivers/pci/pci-uclass.c | 32 +++++++++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)
#11 lore raw

PATCH 1/4 pci: Support parsing PCI controller DT subnodes

In reply to Marek Vasut
Quoted 10 lines · Marek Vasut

The PCI controller can have DT subnodes describing extra properties of particular PCI devices, ie. a PHY attached to an EHCI controller on a PCI bus. This patch parses those DT subnodes and assigns a node to the PCI device instance, so that the driver can extract details from that node and ie. configure the PHY using the PHY subsystem.

Applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20181021/e779a84a/attachment.sig>