Skip to contentSkip to conversation

staging: comedi: adv_pci1723: fix initial dio subdevice state and io_bits

PATCH In review H Hartley Sweeten 2 participants 4 messages Aug 18, 2012 – Aug 21, 2012 Staging

lore
Share & tools
lore

H Hartley Sweeten

author
#0 lore raw

PATCH 9/20 staging: comedi: adv_pci1723: fix initial dio subdevice state and io_bits

The initial state and io_bits for the dio subdevice is determined in the pci1723_attach() but it's being saved in the wrong subdevice. Move the code so it gets saved correctly.

Signed-off-by H Hartley Sweeten <[email protected]>
Cc Ian Abbott <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
---
 drivers/staging/comedi/drivers/adv_pci1723.c | 29 ++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)
drivers/staging/comedi/drivers/adv_pci1723.c +15 -14
index a790bb7..68ca075 100644
@@ -350,6 +350,21 @@ static int pci1723_attach(struct comedi_device *dev,
 		s->insn_write = pci1723_ao_write_winsn;
 		s->insn_read = pci1723_insn_read_ao;
 
+		subdev++;
+	}
+
+	if (this_board->n_diochan) {
+		s = dev->subdevices + subdev;
+		s->type = COMEDI_SUBD_DIO;
+		s->subdev_flags =
+		    SDF_READABLE | SDF_WRITABLE | SDF_GROUND | SDF_COMMON;
+		s->n_chan = this_board->n_diochan;
+		s->maxdata = 1;
+		s->len_chanlist = this_board->n_diochan;
+		s->range_table = &range_digital;
+		s->insn_config = pci1723_dio_insn_config;
+		s->insn_bits = pci1723_dio_insn_bits;
+
 		/* read DIO config */
 		switch (inw(dev->iobase + PCI1723_DIGITAL_IO_PORT_MODE)
 								       & 0x03) {

@@ -372,20 +387,6 @@ static int pci1723_attach(struct comedi_device *dev,
 		subdev++;
 	}
 
-	if (this_board->n_diochan) {
-		s = dev->subdevices + subdev;
-		s->type = COMEDI_SUBD_DIO;
-		s->subdev_flags =
-		    SDF_READABLE | SDF_WRITABLE | SDF_GROUND | SDF_COMMON;
-		s->n_chan = this_board->n_diochan;
-		s->maxdata = 1;
-		s->len_chanlist = this_board->n_diochan;
-		s->range_table = &range_digital;
-		s->insn_config = pci1723_dio_insn_config;
-		s->insn_bits = pci1723_dio_insn_bits;
-		subdev++;
-	}
-
 	devpriv->valid = 1;
 
 	pci1723_reset(dev);
Signature · 1 line
1.7.11
#1 lore raw

drivers/staging/comedi/drivers/adv_pci1723.c static int pci1723_attach(struct comedi_device *dev, · line 357

Quoted 2 lines · H Hartley Sweeten
drivers/staging/comedi/drivers/adv_pci1723.c +2 -0
+	if (this_board->n_diochan) {
+		s = dev->subdevices + subdev;

This pointer math sucks still... I feel like the unreadable code is part of what caused this bug.

We don't have to change these all at once. If we fix only one line, then at least one line will be readable and that is one more than before so *improvement*.

regards,
dan carpenter

author
#2 lore raw
Quoted 10 lines · Dan Carpenter

drivers/staging/comedi/drivers/adv_pci1723.c static int pci1723_attach(struct comedi_device *dev, · line 357

Quoted 2 lines · H Hartley Sweeten
drivers/staging/comedi/drivers/adv_pci1723.c +2 -0
+	if (this_board->n_diochan) {
+		s = dev->subdevices + subdev;

This pointer math sucks still... I feel like the unreadable code is part of what caused this bug.

We don't have to change these all at once. If we fix only one line, then at least one line will be readable and that is one more than before so *improvement*.

Hello Dan,

I'll work of changing the pointer math to array access.

I'll try to post a big chunk of them today.

Regards,
Hartley

author
#3 lore raw
Quoted 12 lines · H Hartley Sweeten
Quoted 10 lines · Dan Carpenter

drivers/staging/comedi/drivers/adv_pci1723.c static int pci1723_attach(struct comedi_device *dev, · line 357

Quoted 2 lines · H Hartley Sweeten
drivers/staging/comedi/drivers/adv_pci1723.c +2 -0
+	if (this_board->n_diochan) {
+		s = dev->subdevices + subdev;

This pointer math sucks still... I feel like the unreadable code is part of what caused this bug.

We don't have to change these all at once. If we fix only one line, then at least one line will be readable and that is one more than before so *improvement*.

Quoted 6 lines

Hello Dan,

I'll work of changing the pointer math to array access.

I'll try to post a big chunk of them today.

Dan,

I just posted a patch that changes all the pointer math for the comedi_subdevice to array access.

It's based on top of the changes I have already posted for the comedi subsystem. As long as Greg doesn't find any issues with those, this patch should apply cleanly.

If I need to I will rebase it.

Regards,
Hartley