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

Hi Simon,

On Tue, Jul 7, 2015 at 6:47 AM, Simon Glass sjg@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@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