RE: [PATCH] staging: comedi_pci: make comedi_pci_disable() safe to always call

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel


[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux