Re: [PATCH v2 30/31] staging: comedi: ni_pcidio: request_irq() before seting up subdevices

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

 



On 2013-12-05 20:43, H Hartley Sweeten wrote:
Do the request_irq() before setting up the subdevices. Only hook up
the command support of the irq was sucessfully requested.

Note that, because of the IRQF_SHARED flag, nidio_interrupt() _may_
be called before the device is ready and the subdevices are setup.
This condition is handled by the (!dev->attached) sanity check. The
initialzation of the local variable pointers before this test is
still safe since we are just getting addresses and not dereferencing
them.

Signed-off-by: H Hartley Sweeten <hsweeten@xxxxxxxxxxxxxxxxxxx>
Cc: Ian Abbott <abbotti@xxxxxxxxx>
Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
  drivers/staging/comedi/drivers/ni_pcidio.c | 38 +++++++++++++++---------------
  1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/comedi/drivers/ni_pcidio.c b/drivers/staging/comedi/drivers/ni_pcidio.c
index 30c46a3..8ab8a87 100644
--- a/drivers/staging/comedi/drivers/ni_pcidio.c
+++ b/drivers/staging/comedi/drivers/ni_pcidio.c
@@ -1011,6 +1011,14 @@ static int nidio_auto_attach(struct comedi_device *dev,

  	nidio_reset_board(dev);

+	irq = mite_irq(devpriv->mite);
+	if (irq) {
+		ret = request_irq(irq, nidio_interrupt, IRQF_SHARED,
+				  dev->board_name, dev);
+		if (ret == 0)
+			dev->irq = irq;
+	}
+
  	ret = comedi_alloc_subdevices(dev, 1);

Unfortunately, the interrupt handler `nidio_interrupt()` still dereferences the subdevice pointer before it tests `dev->attached`:

	struct comedi_subdevice *s = dev->read_subdev;
	struct comedi_async *async = s->async;
	/* ... */
	if (!dev->attached) {
		return IRQ_NONE;
	}

so this can still fail if `dev->read_subdev` hasn't been set yet.

  	if (ret)
  		return ret;
@@ -1019,31 +1027,23 @@ static int nidio_auto_attach(struct comedi_device *dev,
  		 readb(devpriv->mite->daq_io_addr + Chip_Version));

  	s = &dev->subdevices[0];
-
-	dev->read_subdev = s;
  	s->type = COMEDI_SUBD_DIO;
-	s->subdev_flags =
-		SDF_READABLE | SDF_WRITABLE | SDF_LSAMPL | SDF_PACKED |
-		SDF_CMD_READ;
+	s->subdev_flags = SDF_READABLE | SDF_WRITABLE | SDF_LSAMPL | SDF_PACKED;
  	s->n_chan = 32;
  	s->range_table = &range_digital;
  	s->maxdata = 1;
  	s->insn_config = &ni_pcidio_insn_config;
  	s->insn_bits = &ni_pcidio_insn_bits;
-	s->do_cmd = &ni_pcidio_cmd;
-	s->do_cmdtest = &ni_pcidio_cmdtest;
-	s->cancel = &ni_pcidio_cancel;
-	s->len_chanlist = 32;	/* XXX */
-	s->buf_change = &ni_pcidio_change;
-	s->async_dma_dir = DMA_BIDIRECTIONAL;
-	s->poll = &ni_pcidio_poll;
-
-	irq = mite_irq(devpriv->mite);
-	if (irq) {
-		ret = request_irq(irq, nidio_interrupt, IRQF_SHARED,
-				  dev->board_name, dev);
-		if (ret == 0)
-			dev->irq = irq;
+	if (dev->irq) {
+		dev->read_subdev = s;
+		s->subdev_flags |= SDF_CMD_READ;
+		s->async_dma_dir = DMA_BIDIRECTIONAL;
+		s->len_chanlist = s->n_chan;
+		s->do_cmd = ni_pcidio_cmd;
+		s->do_cmdtest = ni_pcidio_cmdtest;
+		s->cancel = ni_pcidio_cancel;
+		s->poll = ni_pcidio_poll;
+		s->buf_change = ni_pcidio_change;
  	}

  	return 0;



--
-=( 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