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

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

 



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@xxxxxxxxxxxxxxxxxxx>
Cc: Ian Abbott <abbotti@xxxxxxxxx>
Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
  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@xxxxxxxxx>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-
_______________________________________________
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