On 2013-03-15 17:06, H Hartley Sweeten wrote:
On Friday, March 15, 2013 3:32 AM, Ian Abbott wrote:Back in the old days (before "staging") when Comedi only supported manual configuration of devices, the "adv_pci_dio" driver supported both PCI-1753 ("pci1753") and PCI-1753E ("pci1753e"). In actual fact, "pci1753e" is just a PCI-1753 connected by a ribbon cable to a PCI-1753E expansion card, which is plugged into a PCI slot but is not a PCI device itself. Now that the "adv_pci_dio" driver only supports automatic configuration of devices and the main "comedi" module no longer allows auto-configuration to be disabled, a PCI-1753 with a PCI-1753E expansion card is always treated as an unexpanded PCI-1753 ("pci1753") and there is no way to override it. (Recently, an undefined macro `USE_PCI1753E_BOARDINFO` was used to make the driver switch to supporting "pci1753e" instead of "pci1753", but this is less than ideal.) Advantech has their own Linux (non-Comedi) driver for the PCI-1753 which detects whether the PCI-1753E expansion card is connected to the PCI-1753 by fiddling with a register at offset 53 from the main registers base. Use Advantech's test in our "adv_pci_dio" driver. If the board appears to be a PCI-1753 ("pci1753"), check if the expansion card appears to be present, and if so, treat the device as a PCI-1753 plus PCI-1753E expansion card ("pci1753e"). Also, get rid of `enum dio_boardid` (`BOARD_...` enum values) which was added recently and just use the older `TYPE_...` enum values from `enum hw_cards_id` instead as the mapping is now 1-to-1. Signed-off-by: Ian Abbott <abbotti@xxxxxxxxx> ---Just a couple comments. <snip>+static unsigned long pci_dio_override_cardtype(struct pci_dev *pcidev, + unsigned long cardtype) +{ + /* + * Change cardtype from TYPE_PCI1753 to TYPE_PCI1753E if expansion + * board available. Need to enable PCI device and request the main + * registers PCI BAR temporarily to perform the test. + */ + if (cardtype != TYPE_PCI1753) + return cardtype; + if (pci_enable_device(pcidev) < 0) + return cardtype;If the pci device cannot be enabled shouldn't the pci probe just return an errno? There doesn't seem to be any reason the continue with the auto attach.
It could return an error here (and pass cardtype by reference), but it should fail later anyway (still within the same PCI probe call) when the auto attach calls comedi_pci_enable(), so it just seemed easiest to pass the buck.
+ if (pci_request_region(pcidev, PCIDIO_MAINREG, "adv_pci_dio") == 0) {Nitpick, is there any way to get the pci_driver name here instead of the open coded string?
Sure, we could use `adv_pci_dio_pci_driver.name` (except it hasn't been declared yet at that point), or `pcidev->driver->name`. It's only used transiently though, so it doesn't matter much what string we use.
-- -=( Ian Abbott @ MEV Ltd. E-mail: <abbotti@xxxxxxxxx> )=- -=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=- _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
