RE: [PATCH 04/19] staging: comedi: ni_mio_common: tidy up DIO subdevice ifdef'ery

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

 



On Wednesday, April 13, 2016 5:04 AM, Ian Abbott wrote:
> On 12/04/16 22:12, H Hartley Sweeten wrote:
>> This file is is bit of a mess. It's included by the ni_atmio, ni_mio_cs, and
>> ni_pcimio drivers. The ni_pcimio driver is the only one that uses DMA. It
>> defines PCIDMA so that the dma code is compiled it. This causes a bunch
>> of ifdef'ery in the file.
>>
>> The DIO subdevice for the ni_pcidio "is_m_series" boards is quite different
>> from the standard e-series DIO. Mainly it supports async commands that use
>> DMA.
>>
>> Tidy up some of the ifdef'ery by adding ifdef to the subdevice init.
>>
>> Also and ifdef to the interrupt handler and remove the unnecessary if
>> (!devpriv->is_m_series) check in handle_cdio_interrupt().
>>
>> Consolidate the other ifdef's to block out the affected code.
>>
>> Signed-off-by: H Hartley Sweeten <hsweeten@xxxxxxxxxxxxxxxxxxx>
>> Cc: Ian Abbott <abbotti@xxxxxxxxx>
>> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
>> ---

 [snip]


>> @@ -3716,13 +3707,8 @@ static void handle_cdio_interrupt(struct comedi_device *dev)
>>   	struct ni_private *devpriv = dev->private;
>>   	unsigned cdio_status;
>>   	struct comedi_subdevice *s = &dev->subdevices[NI_DIO_SUBDEV];
>> -#ifdef PCIDMA
>>   	unsigned long flags;
>> -#endif
>>
>> -	if (!devpriv->is_m_series)
>> -		return;
>
> Removal of that test causes the function to screw up for E-series cards...

Oops. Overlooked that.

[snip]

> ...by reading a bogus (on E-series) register, possibly setting 
> s->async->events and always calling comedi_handle_events() (which also 
> uses s->async), even though s->async will be NULL for this subdevice on 
> E-series cards.
>
> So it needs some sort of test, and as its using M-series registers, 
> testing devpriv->is_m_series seems reasonable enough.  Alternatively, it 
> could test that s->async is non-NULL.

[snip]

> Alternatively, handle_cdio_interrupt() could be called conditionally here.

Thanks for spotting this. I'll fix it up.

Regards,
Hartley

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