[U-Boot] [PATCH 05/18] dm: pci: Add support for PCI driver matching

Bin Meng bmeng.cn at gmail.com
Tue Jul 21 18:12:08 CEST 2015


Hi Simon,

On Tue, Jul 7, 2015 at 6:47 AM, Simon Glass <sjg at chromium.org> wrote:
> At present all PCI devices must be present in the device tree in order to
> be used. Many or most PCI devices don't require any configuration other than
> that which is done automatically by U-Boot. It is inefficent to add a node
> with nothing but a compatible string in order to get a device working.
>
> Add a mechanism whereby PCI drivers can be declared along with the device
> parameters they support (vendor/device/class). When no suitable driver is
> found in the device tree the list of such devices is consulted to determine
> the correct driver. If this also fails, then a generic driver is used as
> before.
>
> The mechanism used is very similar to that provided by Linux and the header
> file defintions are copied from Linux 4.1.
>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
>
>  drivers/pci/pci-uclass.c | 129 ++++++++++++++++++++++++++++++++++++++++++-----
>  include/pci.h            |  79 ++++++++++++++++++++++++++++-
>  2 files changed, 193 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
> index 5b91fe3..41daa0d 100644
> --- a/drivers/pci/pci-uclass.c
> +++ b/drivers/pci/pci-uclass.c
> @@ -353,6 +353,101 @@ int dm_pci_hose_probe_bus(struct pci_controller *hose, pci_dev_t bdf)
>         return sub_bus;
>  }
>
> +/**
> + * pci_match_one_device - Tell if a PCI device structure has a matching
> + *                        PCI device id structure
> + * @id: single PCI device id structure to match
> + * @dev: the PCI device structure to match against
> + *
> + * Returns the matching pci_device_id structure or %NULL if there is no match.
> + */
> +static bool pci_match_one_id(const struct pci_device_id *id,
> +                            const struct pci_device_id *find)
> +{
> +       if ((id->vendor == PCI_ANY_ID || id->vendor == find->vendor) &&
> +           (id->device == PCI_ANY_ID || id->device == find->device) &&
> +           (id->subvendor == PCI_ANY_ID || id->subvendor == find->subvendor) &&
> +           (id->subdevice == PCI_ANY_ID || id->subdevice == find->subdevice) &&
> +           !((id->class ^ find->class) & id->class_mask))
> +               return true;
> +
> +       return false;
> +}
> +
> +/**
> + * pci_find_and_bind_driver() - Find and bind the right PCI driver
> + *
> + * This only looks at certain fields in the descriptor.
> + */
> +static int pci_find_and_bind_driver(struct udevice *parent,
> +                                   struct pci_device_id *find_id, int devfn,
> +                                   struct udevice **devp)
> +{
> +       struct pci_driver_entry *start, *entry;
> +       const char *drv;
> +       int n_ents;
> +       int ret;
> +       char name[30], *str;
> +
> +       *devp = NULL;
> +
> +       debug("%s: Searching for driver: vendor=%x, device=%x\n", __func__,
> +             find_id->vendor, find_id->device);
> +       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++) {
> +               const struct pci_device_id *id;
> +               struct udevice *dev;
> +               const struct driver *drv;
> +
> +               for (id = entry->match;
> +                    id->vendor || id->subvendor || id->class_mask;
> +                    id++) {
> +                       if (!pci_match_one_id(id, find_id))
> +                               continue;
> +
> +                       drv = entry->driver;
> +                       /*
> +                        * We could pass the descriptor to the driver as
> +                        * platdata (instead of NULL) and allow its bind()
> +                        * method to return -ENOENT if it doesn't support this
> +                        * device. That way we could continue the search to
> +                        * 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);
> +                       if (ret)
> +                               goto error;
> +                       debug("%s: Match found: %s\n", __func__, drv->name);
> +                       dev->driver_data = find_id->driver_data;
> +                       *devp = dev;
> +                       return 0;
> +               }
> +       }
> +
> +       /* Bind a generic driver so that the device can be used */
> +       sprintf(name, "pci_%x:%x.%x", parent->seq, PCI_DEV(devfn),
> +               PCI_FUNC(devfn));
> +       str = strdup(name);
> +       if (!str)
> +               return -ENOMEM;
> +       drv = (find_id->class >> 8) == PCI_CLASS_BRIDGE_PCI ? "pci_bridge_drv" :
> +                       "pci_generic_drv";
> +       ret = device_bind_driver(parent, drv, str, devp);
> +       if (ret) {
> +               debug("%s: Failed to bind generic driver: %d", __func__, ret);
> +               return ret;
> +       }
> +       debug("%s: No match found: bound generic driver instead\n", __func__);
> +
> +       return 0;
> +
> +error:
> +       debug("%s: No match found: error %d\n", __func__, ret);
> +       return ret;
> +}
> +
>  int pci_bind_bus_devices(struct udevice *bus)
>  {
>         ulong vendor, device;
> @@ -387,25 +482,33 @@ int pci_bind_bus_devices(struct udevice *bus)
>                       bus->seq, bus->name, PCI_DEV(devfn), PCI_FUNC(devfn));
>                 pci_bus_read_config(bus, devfn, PCI_DEVICE_ID, &device,
>                                     PCI_SIZE_16);
> -               pci_bus_read_config(bus, devfn, PCI_CLASS_DEVICE, &class,
> -                                   PCI_SIZE_16);
> +               pci_bus_read_config(bus, devfn, PCI_CLASS_REVISION, &class,
> +                                   PCI_SIZE_32);
> +               class >>= 8;
>
>                 /* Find this device in the device tree */
>                 ret = pci_bus_find_devfn(bus, devfn, &dev);
>
> +               /* Search for a driver */
> +
>                 /* If nothing in the device tree, bind a generic device */
>                 if (ret == -ENODEV) {
> -                       char name[30], *str;
> -                       const char *drv;
> -
> -                       sprintf(name, "pci_%x:%x.%x", bus->seq,
> -                               PCI_DEV(devfn), PCI_FUNC(devfn));
> -                       str = strdup(name);
> -                       if (!str)
> -                               return -ENOMEM;
> -                       drv = class == PCI_CLASS_BRIDGE_PCI ?
> -                               "pci_bridge_drv" : "pci_generic_drv";
> -                       ret = device_bind_driver(bus, drv, str, &dev);
> +                       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, devfn,
> +                                                   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, devfn,
> +                                                      &dev);
>                 }
>                 if (ret)
>                         return ret;
> diff --git a/include/pci.h b/include/pci.h
> index 3af511b..d21fa8b 100644
> --- a/include/pci.h
> +++ b/include/pci.h
> @@ -468,7 +468,10 @@ typedef int pci_dev_t;
>  #define PCI_ANY_ID             (~0)
>
>  struct pci_device_id {
> -       unsigned int vendor, device;            /* Vendor and device ID or PCI_ANY_ID */
> +       unsigned int vendor, device;    /* Vendor and device ID or PCI_ANY_ID */
> +       unsigned int subvendor, subdevice; /* Subsystem ID's or PCI_ANY_ID */
> +       unsigned int class, class_mask; /* (class,subclass,prog-if) triplet */
> +       unsigned long driver_data;      /* Data private to the driver */
>  };

Can we create another structure for dm? There are lots of codes which
only defines vendor id and device id like below:

static struct pci_device_id mmc_supported[] = {
        { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_TCF_SDIO_0 },
        { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_TCF_SDIO_1 },
};

Although compiler does not generate any error or warning, it is confusing.

>
>  struct pci_controller;
> @@ -1110,7 +1113,79 @@ struct dm_pci_emul_ops {
>  int sandbox_pci_get_emul(struct udevice *bus, pci_dev_t find_devfn,
>                          struct udevice **emulp);
>
> -#endif
> +/**
> + * PCI_DEVICE - macro used to describe a specific pci device
> + * @vend: the 16 bit PCI Vendor ID
> + * @dev: the 16 bit PCI Device ID
> + *
> + * This macro is used to create a struct pci_device_id that matches a
> + * specific device.  The subvendor and subdevice fields will be set to
> + * PCI_ANY_ID.
> + */
> +#define PCI_DEVICE(vend, dev) \
> +       .vendor = (vend), .device = (dev), \
> +       .subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID
> +
> +/**
> + * PCI_DEVICE_SUB - macro used to describe a specific pci device with subsystem
> + * @vend: the 16 bit PCI Vendor ID
> + * @dev: the 16 bit PCI Device ID
> + * @subvend: the 16 bit PCI Subvendor ID
> + * @subdev: the 16 bit PCI Subdevice ID
> + *
> + * This macro is used to create a struct pci_device_id that matches a
> + * specific device with subsystem information.
> + */
> +#define PCI_DEVICE_SUB(vend, dev, subvend, subdev) \
> +       .vendor = (vend), .device = (dev), \
> +       .subvendor = (subvend), .subdevice = (subdev)
> +
> +/**
> + * PCI_DEVICE_CLASS - macro used to describe a specific pci device class
> + * @dev_class: the class, subclass, prog-if triple for this device
> + * @dev_class_mask: the class mask for this device
> + *
> + * This macro is used to create a struct pci_device_id that matches a
> + * specific PCI class.  The vendor, device, subvendor, and subdevice
> + * fields will be set to PCI_ANY_ID.
> + */
> +#define PCI_DEVICE_CLASS(dev_class, dev_class_mask) \
> +       .class = (dev_class), .class_mask = (dev_class_mask), \
> +       .vendor = PCI_ANY_ID, .device = PCI_ANY_ID, \
> +       .subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID
> +
> +/**
> + * PCI_VDEVICE - macro used to describe a specific pci device in short form
> + * @vend: the vendor name
> + * @dev: the 16 bit PCI Device ID
> + *
> + * This macro is used to create a struct pci_device_id that matches a
> + * specific PCI device.  The subvendor, and subdevice fields will be set
> + * to PCI_ANY_ID. The macro allows the next field to follow as the device
> + * private data.
> + */
> +
> +#define PCI_VDEVICE(vend, dev) \
> +       .vendor = PCI_VENDOR_ID_##vend, .device = (dev), \
> +       .subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID, 0, 0
> +
> +/**
> + * struct pci_driver_entry - Matches a driver to its pci_device_id list
> + * @driver: Driver to use
> + * @match: List of match records for this driver, terminated by {}
> + */
> +struct pci_driver_entry {
> +       struct driver *driver;
> +       const struct pci_device_id *match;
> +};
> +
> +#define U_BOOT_PCI_DEVICE(__name, __match)                             \
> +       ll_entry_declare(struct pci_driver_entry, __name, pci_driver_entry) = {\
> +               .driver = llsym(struct driver, __name, driver), \
> +               .match = __match, \
> +               }
> +
> +#endif /* CONFIG_DM_PCI */
>
>  #endif /* __ASSEMBLY__ */
>  #endif /* _PCI_H */
> --

Do you have plan to address the issue that dm pci cannot be used in
the pre-reloc phase? Like we need pci uart as the U-Boot console.

Regards,
Bin


More information about the U-Boot mailing list