[PATCH 13/66] staging: comedi: pcl816: interrupt handlers should not busywait

[Ian Abbott]

On 2014-02-28 23:24, H Hartley Sweeten wrote:
> The interrupt is only generated by the hardware at the completion of
> an A/D conversion. Because of this the sanity check to make sure that
> the A/D conversion is complete and data is available is probably
> unnecessary but it doesn't hurt anything.
>
> The busywait loop is a different issue. Interrupt routines should not
> busywait. That's just mean...
>
> Remove the bustwait and use pcl816_ai_eoc() to check for the end-of-
> conversion.
>
> Signed-off-by: H Hartley Sweeten <hsweeten at visionengravers.com>
> Cc: Ian Abbott <abbotti at mev.co.uk>
> Cc: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
> ---
>   drivers/staging/comedi/drivers/pcl816.c | 10 +---------
>   1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/drivers/staging/comedi/drivers/pcl816.c b/drivers/staging/comedi/drivers/pcl816.c
> index 66d3c5a..6000ca91 100644
> --- a/drivers/staging/comedi/drivers/pcl816.c
> +++ b/drivers/staging/comedi/drivers/pcl816.c
> @@ -279,22 +279,14 @@ static irqreturn_t interrupt_pcl816_ai_mode13_int(int irq, void *d)
>   	struct pcl816_private *devpriv = dev->private;
>   	struct comedi_subdevice *s = dev->read_subdev;
>   	struct comedi_cmd *cmd = &s->async->cmd;
> -	int timeout = 50;	/* wait max 50us */
>
> -	while (timeout--) {
> -		if (!(inb(dev->iobase + PCL816_STATUS) &
> -		      PCL816_STATUS_DRDY_MASK))
> -			break;
> -		udelay(1);
> -	}
> -	if (!timeout) {		/*  timeout, bail error */
> +	if (pcl816_ai_eoc(dev, s, NULL, 0)) {
>   		outb(0, dev->iobase + PCL816_CLRINT);	/* clear INT request */
>   		comedi_error(dev, "A/D mode1/3 IRQ without DRDY!");
>   		s->cancel(dev, s);
>   		s->async->events |= COMEDI_CB_EOA | COMEDI_CB_ERROR;
>   		comedi_event(dev, s);
>   		return IRQ_HANDLED;
> -
>   	}
>
>   	comedi_buf_put(s->async, pcl816_ai_get_sample(dev, s));
>

Curiously, the user manual for this card suggests the interrupt occurs 
at the start of the conversion, rather than the end.  On page 42 of the 
user manual:

5.7. A/D Data Transfer
[snip]
2. Interrupt Routine
[...]  
 At the start of each conversion, the A/D Trigger signal 
generates an interrupt that enables the interrupt service routine to 
perform the transfer. [...]



So it seems the interrupt routine needs to wait for the DRDY bit to be 
set, at least for this transfer mode, which isn't actually used by the 
driver!  I think this mode was disabled for reliability reasons in the 
dim and distant past, although it seems it was only half-disabled; 
pcl816_ai_cmd() only uses the DMA modes, but command support is enabled 
in pcl816_attach() even if the DMA channel has not been configured (it 
shouldn't be allowed unless both the IRQ and DMA channel have been 
configured).

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti at mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-