[PATCH v2 01/31] staging: comedi: ni_at_2150: tidy up irq/dma request

Ian Abbott abbotti at mev.co.uk
Fri Dec 6 09:57:19 UTC 2013


On 2013-12-05 20:43, H Hartley Sweeten wrote:
> This driver needs both an irq and dma in order to support async
> commands. If the irq and dma are not available the driver will
> still function for single analog input reads.
>
> Tidy up the code that does the irq and dma requests so that the
> driver will still attach if they are not avaliable. The attach
> will still fail, with -ENOMEM, if the dma buffer cannot be allocated.
>
> Remove the noise about the irq and dma during the attach.
>
> Only hook up the async commands support if the irq and dma are
> available. Remove the then unnecessary sanity check in a2150_ai_cmd().
>
> 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/ni_at_a2150.c | 79 ++++++++++++----------------
>   1 file changed, 33 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/staging/comedi/drivers/ni_at_a2150.c b/drivers/staging/comedi/drivers/ni_at_a2150.c
> index cc69dde..0876bc5 100644
> --- a/drivers/staging/comedi/drivers/ni_at_a2150.c
> +++ b/drivers/staging/comedi/drivers/ni_at_a2150.c
> @@ -395,11 +395,6 @@ static int a2150_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
>   	unsigned int old_config_bits = devpriv->config_bits;
>   	unsigned int trigger_bits;
>
> -	if (!dev->irq || !devpriv->dma) {
> -		comedi_error(dev,
> -			     " irq and dma required, cannot do hardware conversions");
> -		return -1;
> -	}
>   	if (cmd->flags & TRIG_RT) {
>   		comedi_error(dev,
>   			     " dma incompatible with hard real-time interrupt (TRIG_RT), aborting");
> @@ -703,46 +698,35 @@ static int a2150_attach(struct comedi_device *dev, struct comedi_devconfig *it)
>   	if (ret)
>   		return ret;
>
> -	/* grab our IRQ */
> -	if (irq) {
> -		/*  check that irq is supported */
> -		if (irq < 3 || irq == 8 || irq == 13 || irq > 15) {
> -			printk(" invalid irq line %u\n", irq);
> -			return -EINVAL;
> -		}
> -		if (request_irq(irq, a2150_interrupt, 0,
> -				dev->driver->driver_name, dev)) {
> -			printk("unable to allocate irq %u\n", irq);
> -			return -EINVAL;
> +	dev->board_ptr = a2150_boards + a2150_probe(dev);
> +	thisboard = comedi_board(dev);
> +	dev->board_name = thisboard->name;
> +
> +	if ((irq >= 3 && irq <= 7) || (irq >= 9 && irq <= 12) ||
> +	    irq == 14 || irq == 15) {

I'd have gone with `if (irq >= 3 && irq <= 15 && irq != 8 && irq != 
13)`, but as long as it does the same thing it doesn't matter!

> +		ret = request_irq(irq, a2150_interrupt, 0,
> +				  dev->board_name, dev);
> +		if (ret == 0) {
> +			devpriv->irq_dma_bits |= IRQ_LVL_BITS(irq);
> +			dev->irq = irq;
>   		}
> -		devpriv->irq_dma_bits |= IRQ_LVL_BITS(irq);
> -		dev->irq = irq;
>   	}
> -	/*  initialize dma */
> -	if (dma) {
> -		if (dma == 4 || dma > 7) {
> -			printk(" invalid dma channel %u\n", dma);
> -			return -EINVAL;
> -		}
> -		if (request_dma(dma, dev->driver->driver_name)) {
> -			printk(" failed to allocate dma channel %u\n", dma);
> -			return -EINVAL;
> -		}
> -		devpriv->dma = dma;
> -		devpriv->dma_buffer =
> -		    kmalloc(A2150_DMA_BUFFER_SIZE, GFP_KERNEL | GFP_DMA);
> -		if (devpriv->dma_buffer == NULL)
> -			return -ENOMEM;
>
> -		disable_dma(dma);
> -		set_dma_mode(dma, DMA_MODE_READ);
> +	if (dev->irq && ((dma >= 0 && dma <= 4) || (dma >= 5 && dma <= 7))) {

Since `dma` is unsigned, I'd have gone with `if (dev->irq && dma <= 7 && 
dma != 4)`, but it doesn't really matter, although your `dma >= 0` is 
always true.

> +		ret = request_dma(dma, dev->board_name);
> +		if (ret == 0) {
> +			devpriv->dma = dma;
> +			devpriv->dma_buffer = kmalloc(A2150_DMA_BUFFER_SIZE,
> +						      GFP_KERNEL | GFP_DMA);
> +			if (!devpriv->dma_buffer)
> +				return -ENOMEM;
>
> -		devpriv->irq_dma_bits |= DMA_CHAN_BITS(dma);
> -	}
> +			disable_dma(dma);
> +			set_dma_mode(dma, DMA_MODE_READ);
>
> -	dev->board_ptr = a2150_boards + a2150_probe(dev);
> -	thisboard = comedi_board(dev);
> -	dev->board_name = thisboard->name;
> +			devpriv->irq_dma_bits |= DMA_CHAN_BITS(dma);
> +		}
> +	}
>
>   	ret = comedi_alloc_subdevices(dev, 1);
>   	if (ret)
> @@ -750,17 +734,20 @@ static int a2150_attach(struct comedi_device *dev, struct comedi_devconfig *it)
>
>   	/* analog input subdevice */
>   	s = &dev->subdevices[0];
> -	dev->read_subdev = s;
>   	s->type = COMEDI_SUBD_AI;
> -	s->subdev_flags = SDF_READABLE | SDF_GROUND | SDF_OTHER | SDF_CMD_READ;
> +	s->subdev_flags = SDF_READABLE | SDF_GROUND | SDF_OTHER;
>   	s->n_chan = 4;
> -	s->len_chanlist = 4;
>   	s->maxdata = 0xffff;
>   	s->range_table = &range_a2150;
> -	s->do_cmd = a2150_ai_cmd;
> -	s->do_cmdtest = a2150_ai_cmdtest;
>   	s->insn_read = a2150_ai_rinsn;
> -	s->cancel = a2150_cancel;
> +	if (dev->irq && devpriv->dma) {
> +		dev->read_subdev = s;
> +		s->subdev_flags |= SDF_CMD_READ;
> +		s->len_chanlist = s->n_chan;
> +		s->do_cmd = a2150_ai_cmd;
> +		s->do_cmdtest = a2150_ai_cmdtest;
> +		s->cancel = a2150_cancel;
> +	}
>
>   	/* need to do this for software counting of completed conversions, to
>   	 * prevent hardware count from stopping acquisition */
>


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


More information about the devel mailing list