On Wednesday, March 13, 2013 4:10 AM, Ian Abbott wrote:
> On 13/03/2013 00:41, H Hartley Sweeten wrote:
>> --- a/drivers/staging/comedi/comedi_pci.c
>> +++ b/drivers/staging/comedi/comedi_pci.c
>> @@ -57,14 +57,16 @@ EXPORT_SYMBOL_GPL(comedi_pci_enable);
>>
>>   /**
>>    * comedi_pci_disable() - Release the regions and disable the PCI device.
>> - * @pcidev: pci_dev struct
>> - *
>> - * This must be matched with a previous successful call to 
>> comedi_pci_enable().
>> + * @dev: comedi_device struct
>>    */
>> -void comedi_pci_disable(struct pci_dev *pcidev)
>> +void comedi_pci_disable(struct comedi_device *dev)
>>   {
>> -       pci_release_regions(pcidev);
>> -       pci_disable_device(pcidev);
>> +       struct pci_dev *pcidev = comedi_to_pci_dev(dev);
>> +
>> +       if (pcidev && dev->iobase) {
>> +               pci_release_regions(pcidev);
>> +               pci_disable_device(pcidev);
>> +       }
>>   }
>
> Maybe it should set dev->iobase to 0 as well and your reworked 
> comedi_pci_enable() set dev->iobase to a temporary non-zero value. 
> Several drivers only use dev->iobase as a flag anyway.  (Maybe we 
> shouldn't be overloading dev->iobase in this way - it seems kind of 
> yucky.  Perhaps we should use a new member of struct comedi_device.) 
> This can be done in a follow-up patch.

Ian,

I need to respin these two patches to pick up the adv_pci1724 driver.

I don't think setting dev->iobase to 0 is necessary since the comedi core
always calls cleanup_device() after the (*detach). That function sets all
the dev variables to 0 or NULL.

After these two get accepted I plan on reworking comedi_pci_enable()
to take an additional parameter (main_pcibar) and automatically set
dev->iobase for the drivers, i.e.:

int comedi_pci_enable(struct comedi_device *dev, int main_pcibar)
{
        ...
        If (main_pcibar >= 0) {
                dev->iobase = pci_resource_start(pcidev, main_pcibar);
        else
                dev->iobase = 1;        /* for comedi_pci_disable() */

        return 0;
}

Regards,
Hartley

_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel

Reply via email to