[U-Boot] [PATCH v4 0/4] dm: pci: Support pci devices in the device tree with driver model

Support pci uart devices in the device tree with driver model.
With this series, pci uart now works on Intel Crown Bay with driver model. "dm tree" output below:
=> dm tree Class Probed Name ---------------------------------------- root [ + ] root_driver serial [ ] |-- serial rtc [ ] |-- rtc gpio [ ] |-- gpioa gpio [ ] |-- gpiob spi [ + ] |-- spi spi_flash [ ] | `-- spi-flash@0 pci [ + ] |-- pci pci [ + ] | |-- pcie@17,0 pci [ + ] | | `-- topcliff@0,0 serial [ ] | | |-- uart@a,1 serial [ ] | | |-- uart@a,2 serial [ + ] | | |-- uart@a,3 serial [ ] | | |-- uart@a,4 pci_generic [ ] | | |-- pci_2:0.0 pci_generic [ ] | | |-- pci_2:0.1 pci_generic [ ] | | |-- pci_2:0.2 pci_generic [ ] | | |-- pci_2:2.0 pci_generic [ ] | | |-- pci_2:2.1 pci_generic [ ] | | |-- pci_2:2.2 pci_generic [ ] | | |-- pci_2:2.3 pci_generic [ ] | | |-- pci_2:6.0 pci_generic [ ] | | |-- pci_2:8.0 pci_generic [ ] | | |-- pci_2:8.1 pci_generic [ ] | | |-- pci_2:8.2 pci_generic [ ] | | |-- pci_2:8.3 pci_generic [ ] | | |-- pci_2:a.0 pci_generic [ ] | | |-- pci_2:c.0 pci_generic [ ] | | |-- pci_2:c.2 pci_generic [ ] | | `-- pci_2:c.3 pci_generic [ ] | |-- pci_0:0.0 pci_generic [ ] | |-- pci_0:2.0 pci [ + ] | |-- pci_0:18.0 pci [ + ] | |-- pci_0:19.0 pci [ + ] | |-- pci_0:1a.0 pci_generic [ ] | |-- pci_0:1b.0 pci_generic [ ] | `-- pci_0:1f.0 simple_bus [ + ] `-- cpus cpu [ + ] |-- cpu@0 cpu [ + ] `-- cpu@1
Changes in v4: - Fix memory leak in device_probe_child()
Changes in v3: - Rebase on u-boot-x86/master - Drop v2 patches which were already applied
Bin Meng (4): dm: pci: Optimize pci_uclass_post_bind() dm: core: Fix code reentrancy issue in device_probe_child() x86: crownbay: Support Topcliff integrated pci uart devices with driver model dm: pci: Document binding of pci device drivers
arch/x86/dts/crownbay.dts | 15 ++++++--- doc/driver-model/pci-info.txt | 71 +++++++++++++++++++++++++++++++++++++++++-- drivers/core/device.c | 19 +++++++++--- drivers/pci/pci-uclass.c | 7 +++++ 4 files changed, 99 insertions(+), 13 deletions(-)

If there is no pci device listed in the device tree, don't bother scanning the device tree.
Signed-off-by: Bin Meng bmeng.cn@gmail.com Acked-by: Simon Glass sjg@chromium.org
---
Changes in v4: None Changes in v3: - Rebase on u-boot-x86/master - Drop v2 patches which were already applied
drivers/pci/pci-uclass.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 2d12344..b25298f 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -632,6 +632,13 @@ error: static int pci_uclass_post_bind(struct udevice *bus) { /* + * If there is no pci device listed in the device tree, + * don't bother scanning the device tree. + */ + if (bus->of_offset == -1) + return 0; + + /* * Scan the device tree for devices. This does not probe the PCI bus, * as this is not permitted while binding. It just finds devices * mentioned in the device tree.

On 24 August 2015 at 01:14, Bin Meng bmeng.cn@gmail.com wrote:
If there is no pci device listed in the device tree, don't bother scanning the device tree.
Signed-off-by: Bin Meng bmeng.cn@gmail.com Acked-by: Simon Glass sjg@chromium.org
Changes in v4: None Changes in v3:
- Rebase on u-boot-x86/master
- Drop v2 patches which were already applied
drivers/pci/pci-uclass.c | 7 +++++++ 1 file changed, 7 insertions(+)
I'd like to get this series in to correct your serial problem.
Applied to u-boot-x86, thanks!

The device might have already been probed during the call to device_probe() on its parent device (e.g. PCI bridge devices). In its parent device's probe routine, it might probe all of its child devices via device_probe() thus the codes reenter device_probe_child(). To support code reentrancy, test these allocated memory against NULL to avoid memory leak, and return to the caller if dev->flags has DM_FLAG_ACTIVATED set after device_probe() returns, so that we don't mess up the device.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
---
Changes in v4: - Fix memory leak in device_probe_child()
Changes in v3: None
drivers/core/device.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/core/device.c b/drivers/core/device.c index e23a872..a31e25f 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -226,17 +226,17 @@ int device_probe_child(struct udevice *dev, void *parent_priv) drv = dev->driver; assert(drv);
- /* Allocate private data if requested */ - if (drv->priv_auto_alloc_size) { + /* Allocate private data if requested and not reentered */ + if (drv->priv_auto_alloc_size && !dev->priv) { dev->priv = alloc_priv(drv->priv_auto_alloc_size, drv->flags); if (!dev->priv) { ret = -ENOMEM; goto fail; } } - /* Allocate private data if requested */ + /* Allocate private data if requested and not reentered */ size = dev->uclass->uc_drv->per_device_auto_alloc_size; - if (size) { + if (size && !dev->uclass_priv) { dev->uclass_priv = calloc(1, size); if (!dev->uclass_priv) { ret = -ENOMEM; @@ -251,7 +251,7 @@ int device_probe_child(struct udevice *dev, void *parent_priv) size = dev->parent->uclass->uc_drv-> per_child_auto_alloc_size; } - if (size) { + if (size && !dev->parent_priv) { dev->parent_priv = alloc_priv(size, drv->flags); if (!dev->parent_priv) { ret = -ENOMEM; @@ -264,6 +264,15 @@ int device_probe_child(struct udevice *dev, void *parent_priv) ret = device_probe(dev->parent); if (ret) goto fail; + + /* + * The device might have already been probed during + * the call to device_probe() on its parent device + * (e.g. PCI bridge devices). Test the flags again + * so that we don't mess up the device. + */ + if (dev->flags & DM_FLAG_ACTIVATED) + return 0; }
seq = uclass_resolve_seq(dev);

On 24 August 2015 at 02:14, Bin Meng bmeng.cn@gmail.com wrote:
The device might have already been probed during the call to device_probe() on its parent device (e.g. PCI bridge devices). In its parent device's probe routine, it might probe all of its child devices via device_probe() thus the codes reenter device_probe_child(). To support code reentrancy, test these allocated memory against NULL to avoid memory leak, and return to the caller if dev->flags has DM_FLAG_ACTIVATED set after device_probe() returns, so that we don't mess up the device.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
Changes in v4:
- Fix memory leak in device_probe_child()
Changes in v3: None
drivers/core/device.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-)
Looks like a good solution and it's good to have the comments too.
Acked-by: Simon Glass sjg@chromium.org

On 24 August 2015 at 22:04, Simon Glass sjg@chromium.org wrote:
On 24 August 2015 at 02:14, Bin Meng bmeng.cn@gmail.com wrote:
The device might have already been probed during the call to device_probe() on its parent device (e.g. PCI bridge devices). In its parent device's probe routine, it might probe all of its child devices via device_probe() thus the codes reenter device_probe_child(). To support code reentrancy, test these allocated memory against NULL to avoid memory leak, and return to the caller if dev->flags has DM_FLAG_ACTIVATED set after device_probe() returns, so that we don't mess up the device.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
Changes in v4:
- Fix memory leak in device_probe_child()
Changes in v3: None
drivers/core/device.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-)
Looks like a good solution and it's good to have the comments too.
Acked-by: Simon Glass sjg@chromium.org
Applied to u-boot-x86, thanks!

In order to make a pci uart device node to be properly bound to its driver, we need make sure its parent node has a compatible string which matches a driver that scans all of its child device nodes in the device tree.
Change all pci bridge nodes under root pci node to use "pci-bridge" compatible driver, as well as corresponding <reg> properties to indicate its devfn. At last, adding "u-boot,dm-pre-reloc" to each of these nodes for driver model to initialize them before relocation.
Signed-off-by: Bin Meng bmeng.cn@gmail.com Acked-by: Simon Glass sjg@chromium.org ---
Changes in v4: None Changes in v3: None
arch/x86/dts/crownbay.dts | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/arch/x86/dts/crownbay.dts b/arch/x86/dts/crownbay.dts index 3af9cc3..9f00c67 100644 --- a/arch/x86/dts/crownbay.dts +++ b/arch/x86/dts/crownbay.dts @@ -91,7 +91,6 @@ #address-cells = <3>; #size-cells = <2>; compatible = "pci-x86"; - device_type = "pci"; u-boot,dm-pre-reloc; ranges = <0x02000000 0x0 0x40000000 0x40000000 0 0x80000000 0x42000000 0x0 0xc0000000 0xc0000000 0 0x20000000 @@ -100,14 +99,16 @@ pcie@17,0 { #address-cells = <3>; #size-cells = <2>; - compatible = "intel,pci"; - device_type = "pci"; + compatible = "pci-bridge"; + u-boot,dm-pre-reloc; + reg = <0x0000b800 0x0 0x0 0x0 0x0>;
topcliff@0,0 { #address-cells = <3>; #size-cells = <2>; - compatible = "intel,pci"; - device_type = "pci"; + compatible = "pci-bridge"; + u-boot,dm-pre-reloc; + reg = <0x00010000 0x0 0x0 0x0 0x0>;
pciuart0: uart@a,1 { compatible = "pci8086,8811.00", @@ -115,6 +116,7 @@ "pciclass,070002", "pciclass,0700", "x86-uart"; + u-boot,dm-pre-reloc; reg = <0x00025100 0x0 0x0 0x0 0x0 0x01025110 0x0 0x0 0x0 0x0>; reg-shift = <0>; @@ -128,6 +130,7 @@ "pciclass,070002", "pciclass,0700", "x86-uart"; + u-boot,dm-pre-reloc; reg = <0x00025200 0x0 0x0 0x0 0x0 0x01025210 0x0 0x0 0x0 0x0>; reg-shift = <0>; @@ -141,6 +144,7 @@ "pciclass,070002", "pciclass,0700", "x86-uart"; + u-boot,dm-pre-reloc; reg = <0x00025300 0x0 0x0 0x0 0x0 0x01025310 0x0 0x0 0x0 0x0>; reg-shift = <0>; @@ -154,6 +158,7 @@ "pciclass,070002", "pciclass,0700", "x86-uart"; + u-boot,dm-pre-reloc; reg = <0x00025400 0x0 0x0 0x0 0x0 0x01025410 0x0 0x0 0x0 0x0>; reg-shift = <0>;

On 24 August 2015 at 01:14, Bin Meng bmeng.cn@gmail.com wrote:
In order to make a pci uart device node to be properly bound to its driver, we need make sure its parent node has a compatible string which matches a driver that scans all of its child device nodes in the device tree.
Change all pci bridge nodes under root pci node to use "pci-bridge" compatible driver, as well as corresponding <reg> properties to indicate its devfn. At last, adding "u-boot,dm-pre-reloc" to each of these nodes for driver model to initialize them before relocation.
Signed-off-by: Bin Meng bmeng.cn@gmail.com Acked-by: Simon Glass sjg@chromium.org
Changes in v4: None Changes in v3: None
arch/x86/dts/crownbay.dts | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)
Applied to u-boot-x86, thanks!

Document how pci devices are bound to device drivers.
Signed-off-by: Bin Meng bmeng.cn@gmail.com Acked-by: Simon Glass sjg@chromium.org
---
Changes in v4: None Changes in v3: None
doc/driver-model/pci-info.txt | 71 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 68 insertions(+), 3 deletions(-)
diff --git a/doc/driver-model/pci-info.txt b/doc/driver-model/pci-info.txt index cf69167..52b4389 100644 --- a/doc/driver-model/pci-info.txt +++ b/doc/driver-model/pci-info.txt @@ -34,9 +34,74 @@ 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 device tree. If they do this serves to specify -the driver to use for the device. In this case they will be bound at -start-up. +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 +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: + + pci { + #address-cells = <3>; + #size-cells = <2>; + compatible = "pci-x86"; + u-boot,dm-pre-reloc; + ranges = <0x02000000 0x0 0x40000000 0x40000000 0 0x80000000 + 0x42000000 0x0 0xc0000000 0xc0000000 0 0x20000000 + 0x01000000 0x0 0x2000 0x2000 0 0xe000>; + + pcie@17,0 { + #address-cells = <3>; + #size-cells = <2>; + compatible = "pci-bridge"; + u-boot,dm-pre-reloc; + reg = <0x0000b800 0x0 0x0 0x0 0x0>; + + topcliff@0,0 { + #address-cells = <3>; + #size-cells = <2>; + compatible = "pci-bridge"; + u-boot,dm-pre-reloc; + reg = <0x00010000 0x0 0x0 0x0 0x0>; + + pciuart0: uart@a,1 { + compatible = "pci8086,8811.00", + "pci8086,8811", + "pciclass,070002", + "pciclass,0700", + "x86-uart"; + u-boot,dm-pre-reloc; + reg = <0x00025100 0x0 0x0 0x0 0x0 + 0x01025110 0x0 0x0 0x0 0x0>; + ...... + }; + + ...... + }; + }; + + ...... + }; + +In this example, the root PCI bus node is the "/pci" which matches "pci-x86" +driver. It has a subnode "pcie@17,0" with driver "pci-bridge". "pcie@17,0" +also has subnode "topcliff@0,0" which is a "pci-bridge" too. Under that bridge, +a PCI UART device "uart@a,1" is described. This exactly reflects the hardware +bus hierarchy: on the root PCI bus, there is a PCIe root port which connects +to a downstream device Topcliff chipset. Inside Topcliff chipset, it has a +PCIe-to-PCI bridge and all the chipset integrated devices like the PCI UART +device are on the PCI bus. Like other devices in the device tree, if we want +to bind PCI devices before relocation, "u-boot,dm-pre-reloc" must be declared +in each of these nodes. + +If PCI devices are not listed in the device tree, U_BOOT_PCI_DEVICE can be used +to specify the driver to use for the device. The device tree takes precedence +over U_BOOT_PCI_DEVICE. Plese note with U_BOOT_PCI_DEVICE, only drivers with +DM_FLAG_PRE_RELOC will be bound before relocation. If neither device tree nor +U_BOOT_PCI_DEVICE is provided, the built-in driver (either pci_bridge_drv or +pci_generic_drv) will be used.
Sandbox

On 24 August 2015 at 01:14, Bin Meng bmeng.cn@gmail.com wrote:
Document how pci devices are bound to device drivers.
Signed-off-by: Bin Meng bmeng.cn@gmail.com Acked-by: Simon Glass sjg@chromium.org
Changes in v4: None Changes in v3: None
doc/driver-model/pci-info.txt | 71 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 68 insertions(+), 3 deletions(-)
Applied to u-boot-x86, thanks!
participants (2)
-
Bin Meng
-
Simon Glass