[U-Boot] [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.
Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com --- 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(-)
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index eb118f3496..da49c96ed5 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -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);

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.
Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com --- doc/driver-model/pci-info.txt | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/doc/driver-model/pci-info.txt b/doc/driver-model/pci-info.txt index e1701d1fbc..14364c5c75 100644 --- a/doc/driver-model/pci-info.txt +++ b/doc/driver-model/pci-info.txt @@ -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:

Hi Marek,
On Thu, Oct 11, 2018 at 3:28 AM Marek Vasut marek.vasut@gmail.com wrote:
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.
Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com
doc/driver-model/pci-info.txt | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
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.
diff --git a/doc/driver-model/pci-info.txt b/doc/driver-model/pci-info.txt index e1701d1fbc..14364c5c75 100644 --- a/doc/driver-model/pci-info.txt +++ b/doc/driver-model/pci-info.txt @@ -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: --
Regards, Bin

On Wed, Oct 10, 2018 at 09:27:07PM +0200, Marek Vasut wrote:
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.
Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com Reviewed-by: Bin Meng bmeng.cn@gmail.com
Applied to u-boot/master, thanks!

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.
Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com --- arch/sandbox/dts/test.dts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index ad94901fa1..0a6b86999d 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -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@10,0 { + reg = <0x8000 0 0 0 0>; + }; };
pci2: pci-controller2 {

On Thu, Oct 11, 2018 at 3:28 AM Marek Vasut marek.vasut@gmail.com wrote:
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.
Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com
arch/sandbox/dts/test.dts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
nits: I believe this can be squashed to patch [4/4] to make it a complete test case update.

On Wed, Oct 10, 2018 at 09:27:08PM +0200, Marek Vasut wrote:
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.
Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com Reviewed-by: Bin Meng bmeng.cn@gmail.com
Applied to u-boot/master, thanks!

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.
Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com --- test/dm/pci.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/test/dm/pci.c b/test/dm/pci.c index 869970072d..a1dedd84a7 100644 --- a/test/dm/pci.c +++ b/test/dm/pci.c @@ -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; }

On Thu, Oct 11, 2018 at 3:29 AM Marek Vasut marek.vasut@gmail.com wrote:
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.
Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com
test/dm/pci.c | 5 +++++ 1 file changed, 5 insertions(+)
Reviewed-by: Bin Meng bmeng.cn@gmail.com

On Wed, Oct 10, 2018 at 09:27:09PM +0200, Marek Vasut wrote:
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.
Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com Reviewed-by: Bin Meng bmeng.cn@gmail.com
Applied to u-boot/master, thanks!

On Thu, Oct 11, 2018 at 3:27 AM Marek Vasut marek.vasut@gmail.com wrote:
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.
Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com
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(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com

On Wed, Oct 10, 2018 at 09:27:06PM +0200, Marek Vasut wrote:
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.
Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com Reviewed-by: Bin Meng bmeng.cn@gmail.com
Applied to u-boot/master, thanks!
participants (3)
-
Bin Meng
-
Marek Vasut
-
Tom Rini