[U-Boot] [PATCH v2 08/12] dm: pci: Remove the unnecessary pci_bus_find_devfn() in pci_bind_bus_devices()

[Simon Glass <sjg@chromium.org>]

Hi Bin,

On 20 August 2015 at 07:40, Bin Meng <bmeng.cn@gmail.com> wrote:
> During pci_bind_bus_devices() before finding a proper driver to bind
> the device, pci_bus_find_devfn() is called to find if this device
> already exists. However since device is even not bound, this call
> always returns -ENODEV. It is really unnecessary hence remove it.
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
>
> Changes in v2: None
>
>  drivers/pci/pci-uclass.c | 35 ++++++++++++++---------------------
>  1 file changed, 14 insertions(+), 21 deletions(-)

This doesn't look quite right to me.

The way this is supposed to work is that when the bus or bridge is
bound, pci_uclass_post_bind() should add a device for each node it
finds in the device tree on that bus.

I think the problem in your case is that in crownbay.dts you have:

pcie at 17,0 {
  #address-cells = <3>;
  #size-cells = <2>;
  compatible = "intel,pci";
  device_type = "pci";

and the compatible string for this doesn't match any driver. Thus the
serial ports are never 'seen' during the initial bind and devices are
not created.

If you add a driver for your device 17 and have it call
dm_scan_fdt_node() then the devices should be bound correctly.

I think then your new code may not be needed, or a small smaller
change may be needed.

>
> diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
> index c90e7ac..63e85b9 100644
> --- a/drivers/pci/pci-uclass.c
> +++ b/drivers/pci/pci-uclass.c
> @@ -551,6 +551,8 @@ int pci_bind_bus_devices(struct udevice *bus)
>         ulong header_type;
>         pci_dev_t bdf, end;
>         bool found_multi;
> +       struct pci_device_id find_id;
> +       ulong val;
>         int ret;
>
>         found_multi = false;
> @@ -585,30 +587,21 @@ int pci_bind_bus_devices(struct udevice *bus)
>                                     PCI_SIZE_32);
>                 class >>= 8;
>
> -               /* Find this device in the device tree */
> -               ret = pci_bus_find_devfn(bus, PCI_MASK_BUS(bdf), &dev);
> -
>                 /* Search for a driver */
>
> -               /* If nothing in the device tree, bind a generic device */
> -               if (ret == -ENODEV) {
> -                       struct pci_device_id find_id;
> -                       ulong val;
> -
> -                       memset(&find_id, '\0', sizeof(find_id));
> -                       find_id.vendor = vendor;
> -                       find_id.device = device;
> -                       find_id.class = class;
> -                       if ((header_type & 0x7f) == PCI_HEADER_TYPE_NORMAL) {
> -                               pci_bus_read_config(bus, bdf,
> -                                                   PCI_SUBSYSTEM_VENDOR_ID,
> -                                                   &val, PCI_SIZE_32);
> -                               find_id.subvendor = val & 0xffff;
> -                               find_id.subdevice = val >> 16;
> -                       }
> -                       ret = pci_find_and_bind_driver(bus, &find_id, bdf,
> -                                                      &dev);
> +               memset(&find_id, '\0', sizeof(find_id));
> +               find_id.vendor = vendor;
> +               find_id.device = device;
> +               find_id.class = class;
> +               if ((header_type & 0x7f) == PCI_HEADER_TYPE_NORMAL) {
> +                       pci_bus_read_config(bus, bdf,
> +                                           PCI_SUBSYSTEM_VENDOR_ID,
> +                                           &val, PCI_SIZE_32);
> +                       find_id.subvendor = val & 0xffff;
> +                       find_id.subdevice = val >> 16;
>                 }
> +               ret = pci_find_and_bind_driver(bus, &find_id, bdf,
> +                                              &dev);
>                 if (ret)
>                         return ret;
>
> --
> 1.8.2.1
>

Regards,
Simon