[PATCH 09/20] staging: comedi: adv_pci1723: fix initial dio subdevice state and io_bits

[PATCH 09/20] staging: comedi: adv_pci1723: fix initial dio subdevice state and io_bits

[H Hartley Sweeten]

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 <hsweeten@visionengravers.com>
Cc: Ian Abbott <abbotti@mev.co.uk>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/adv_pci1723.c | 29 ++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/comedi/drivers/adv_pci1723.c b/drivers/staging/comedi/drivers/adv_pci1723.c
index a790bb7..68ca075 100644
--- a/drivers/staging/comedi/drivers/adv_pci1723.c
+++ b/drivers/staging/comedi/drivers/adv_pci1723.c
@@ -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);
-- 
1.7.11

Re: [PATCH 09/20] staging: comedi: adv_pci1723: fix initial dio subdevice state and io_bits

[Dan Carpenter]

On Fri, Aug 17, 2012 at 06:17:38PM -0700, H Hartley Sweeten wrote:
> +	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

RE: [PATCH 09/20] staging: comedi: adv_pci1723: fix initial dio subdevice state and io_bits

[H Hartley Sweeten]

On Monday, August 20, 2012 2:49 PM, Dan Carpenter wrote:
> On Fri, Aug 17, 2012 at 06:17:38PM -0700, H Hartley Sweeten wrote:
>> +	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

RE: [PATCH 09/20] staging: comedi: adv_pci1723: fix initial dio subdevice state and io_bits

[H Hartley Sweeten]

On Monday, August 20, 2012 2:51 PM, H Hartley Sweeten wrote:
> On Monday, August 20, 2012 2:49 PM, Dan Carpenter wrote:
>> On Fri, Aug 17, 2012 at 06:17:38PM -0700, H Hartley Sweeten wrote:
>>> +	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.
>

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