On Thursday, July 19, 2012 4:17 PM, gregkh wrote: > On Thu, Jul 19, 2012 at 12:12:02PM -0500, H Hartley Sweeten wrote: >> I was planning on making a comedi_find_pci_dev() function that the >> drivers could call with a "match" callback. This would allow a common >> function for most of the boilerplate code and just require the drivers >> to check the the match against the boardinfo dev/id, etc. as required. >> Something like: >> >> struct pci_dev *comedi_find_pci_dev(struct comedi_device *dev, >> struct comedi_devconfig *it, >> const void *(*match)(struct comedi_device *, >> struct pci_dev *)) >> { >> struct pci_dev *pcidev = NULL; >> int bus = it->options[0]; >> int slot = it->options[1]; >> >> for_each_pci_dev(pcidev) { >> /* pci_is_enabled() test? */ >> if ((bus && bus != pcidev->bus->number) || >> (slot && slot != PCI_SLOT(pcidev->devfn))) >> continue; >> dev->board_ptr = match(dev, pcidev); >> if (dev->board_ptr) { >> comedi_set_hw_dev(dev, &pcidev->dev); >> return pcidev; >> } >> } >> return NULL; >> } >> >> The "match" function for a driver would then just be something like: >> >> const void *match_pci(struct comedi_device *dev, struct pci_dev *pcidev) >> { >> const struct boardinfo *board; >> int i; >> >> for (i = 0; i < ARRAY_SIZE(boardinfo); i++) { >> board = &boardinfo[i]; >> if (pcidev->vendor != board->ven_id || >> pcidev->device != board->dev_id) >> continue; >> return board; >> } >> return NULL; >> } >> >> This would require adding a dummy boardinfo to some of the drivers but >> I think it's cleaner. >> >> Comments? > > Ick. What ever happened to converting these drivers to use the PCI api > correctly and not to search the PCI space for the device, but have the > PCI core call them if the device is found? > > It looks like most of these drivers have already been converted to that > style, so these checks for "do we find a device" can all be removed > entirely now, right? There's no way the functions would be called if > the device wasn't found in the first place. > > Or am I missing something here? If the comedi pci drivers have the "attach_pci" callback defined, the PCI api does correctly probe the driver. The comedi_pci_auto_config() then passes the pci_dev directly to the driver and the search of the PCI space for the device is not required. If the "attach_pci" callback is not defined, the comedi_pci_auto_config() then falls back to passing the bus/slot information to the driver and uses the "attach" callback. In this case we could probably fast-track the search by using pci_get_slot() instead of doing the for_each_pci_dev() loop. I think the problem is the COMEDI_DEVCONFIG ioctl. The userspace utility 'comedi_config' uses that ioctl to link a device node to a comedi driver. That utility allows passing the bus/slot information but it's not required. This means we have to search the PCI space for the pci_dev that matches the driver. Not sure what to do here... Regards, Hartley _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel