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.
>+ 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?
Regards,
Hartley
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
