Re: [U-Boot] [PATCH 3/6] pci: Support parsing PCI controller DT subnodes

Hi Marek,

On Sat, Sep 22, 2018 at 7:00 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

---

drivers/pci/pci-uclass.c | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index de523a76ad..e274632428 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -90,6 +90,25 @@ int pci_get_ff(enum pci_size_t size) } }

+static void pci_dev_find_ofnode(struct udevice *bus, phys_addr_t bdf,

•                           ofnode *rnode)
  

+{

•   ofnode node;
  

•   dev_for_each_subnode(node, bus) {
  

•           phys_addr_t df, size;
  

•           df = ofnode_get_addr_size(node, "reg", &size);
  

Using API ofnode_get_addr_size() is wrong. It cannot handle PCI-specific address formats. I understand why you added "0x00008000 0 0x00000000 0x00008000 0 0x2000" to the bus ranges property in patch 5, is to make ofnode_get_addr_size() work, but that's the wrong approach. The correct API should be ofnode_read_pci_addr(). To call it like this:

ret = ofnode_read_pci_addr(node, FDT_PCI_SPACE_CONFIG, "reg", &addr); if (!ret) df = addr.phys_hi & 0xff00;

•           if (df == FDT_ADDR_T_NONE)
  

•                   continue;
  

•           if (PCI_MASK_BUS(df) != 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 +660,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 +671,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 +708,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 +736,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);
  

--

Regards, Bin