[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