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