[PATCH v2] staging: comedi: adv_pci_dio: restore PCI-1753E support

H Hartley Sweeten hartleys at visionengravers.com
Fri Mar 15 17:48:45 UTC 2013


On Friday, March 15, 2013 10:44 AM, Ian Abbott wrote:
> On 2013-03-15 17:06, H Hartley Sweeten wrote:
>> On Friday, March 15, 2013 3:32 AM, Ian Abbott wrote:
>>> +	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.

Fair enough.

>>> +	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.

Like I said, it's just a pitpick.

I still would use pdidev->driver->name instead of the open coded string.
It saves a couple bytes and if the driver name ever changes they would
still match. Call it a pet peeve of mine... ;-)

Your call.

Regards,
Hartley



More information about the devel mailing list