[PATCH 0/4] staging: comedi: addi_apci_1564: Further cleanups

[PATCH 0/4] staging: comedi: addi_apci_1564: Further cleanups

[Chase Southwood]

Latest patchset in a continuing cleanup of addi_apci_1564.c.  Main focus
is on the auto_attach() function, but also cleaning up some of the old addi
common code wrappers.

To be applied on top of my prior patchset to this file (the one beginning
with [PATCH 1/6] staging: comedi: addi_apci_1564: remove eeprom support code)

Chase Southwood (4):
  staging: comedi: addi_apci_1564: board has 32 digital outputs
  staging: comedi: addi_apci_1564: don't allocate unused subdevices
  staging: comedi: addi_apci_1564: absorb apci1564_reset()
  staging: comedi: addi_apci_1564: call apci1564_interrupt() directly

 .../comedi/drivers/addi-data/hwdrv_apci1564.c      | 32 -------
 drivers/staging/comedi/drivers/addi_apci_1564.c    | 98 ++++++++++------------
 2 files changed, 45 insertions(+), 85 deletions(-)

-- 
1.9.0

[PATCH 1/4] staging: comedi: addi_apci_1564: board has 32 digital outputs

[Chase Southwood]

This board always has 32 digital outputs.  Remove the test when
initializing the subdevice.

Also, since this board is the only one supported by this driver, remove
the boardinfo about the digital outputs and just use the data directly in
the subdevice init.

Signed-off-by: Chase Southwood <chase.southwood@gmail.com>
Cc: Ian Abbott <abbotti@mev.co.uk>
Cc: H Hartley Sweeten <hsweeten@visionengravers.com>
---
 drivers/staging/comedi/drivers/addi_apci_1564.c | 29 ++++++++-----------------
 1 file changed, 9 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c b/drivers/staging/comedi/drivers/addi_apci_1564.c
index ea22530..25149b2 100644
--- a/drivers/staging/comedi/drivers/addi_apci_1564.c
+++ b/drivers/staging/comedi/drivers/addi_apci_1564.c
@@ -17,9 +17,6 @@ static const struct addi_board apci1564_boardtypes[] = {
 		.i_Timer		= 1,
 		.interrupt		= apci1564_interrupt,
 		.reset			= apci1564_reset,
-		.do_config		= apci1564_do_config,
-		.do_bits		= apci1564_do_insn_bits,
-		.do_read		= apci1564_do_read,
 		.timer_config		= apci1564_timer_config,
 		.timer_write		= apci1564_timer_write,
 		.timer_read		= apci1564_timer_read,
@@ -98,23 +95,15 @@ static int apci1564_auto_attach(struct comedi_device *dev,
 
 	/*  Allocate and Initialise DO Subdevice Structures */
 	s = &dev->subdevices[3];
-	if (this_board->i_NbrDoChannel) {
-		s->type = COMEDI_SUBD_DO;
-		s->subdev_flags =
-			SDF_READABLE | SDF_WRITEABLE | SDF_GROUND | SDF_COMMON;
-		s->n_chan = this_board->i_NbrDoChannel;
-		s->maxdata = this_board->i_DoMaxdata;
-		s->len_chanlist = this_board->i_NbrDoChannel;
-		s->range_table = &range_digital;
-
-		/* insn_config - for digital output memory */
-		s->insn_config = this_board->do_config;
-		s->insn_write = this_board->do_write;
-		s->insn_bits = this_board->do_bits;
-		s->insn_read = this_board->do_read;
-	} else {
-		s->type = COMEDI_SUBD_UNUSED;
-	}
+	s->type = COMEDI_SUBD_DO;
+	s->subdev_flags = SDF_WRITEABLE;
+	s->n_chan = 32;
+	s->maxdata = 0xffffffff;
+	s->len_chanlist = 32;
+	s->range_table = &range_digital;
+	s->insn_config = apci1564_do_config;
+	s->insn_bits = apci1564_do_insn_bits;
+	s->insn_read = apci1564_do_read;
 
 	/*  Allocate and Initialise Timer Subdevice Structures */
 	s = &dev->subdevices[4];
-- 
1.9.0

[PATCH 2/4] staging: comedi: addi_apci_1564: don't allocate unused subdevices

[Chase Southwood]

The addi-data common code always allocates 7 subdevices.  This driver
cannot or will not use the ones we are currently allocating for analog
input and output or EEPROM, so just don't allocate these subdevices at
all.

Signed-off-by: Chase Southwood <chase.southwood@gmail.com>
Cc: Ian Abbott <abbotti@mev.co.uk>
Cc: H Hartley Sweeten <hsweeten@visionengravers.com>
---
 drivers/staging/comedi/drivers/addi_apci_1564.c | 27 +++++--------------------
 1 file changed, 5 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c b/drivers/staging/comedi/drivers/addi_apci_1564.c
index 25149b2..31b687c 100644
--- a/drivers/staging/comedi/drivers/addi_apci_1564.c
+++ b/drivers/staging/comedi/drivers/addi_apci_1564.c
@@ -47,7 +47,7 @@ static int apci1564_auto_attach(struct comedi_device *dev,
 	const struct addi_board *this_board = comedi_board(dev);
 	struct addi_private *devpriv;
 	struct comedi_subdevice *s;
-	int ret, n_subdevices;
+	int ret;
 
 	dev->board_name = this_board->pc_DriverName;
 
@@ -69,21 +69,12 @@ static int apci1564_auto_attach(struct comedi_device *dev,
 			dev->irq = pcidev->irq;
 	}
 
-	n_subdevices = 7;
-	ret = comedi_alloc_subdevices(dev, n_subdevices);
+	ret = comedi_alloc_subdevices(dev, 3);
 	if (ret)
 		return ret;
 
-	/*  Allocate and Initialise AI Subdevice Structures */
-	s = &dev->subdevices[0];
-	s->type = COMEDI_SUBD_UNUSED;
-
-	/*  Allocate and Initialise AO Subdevice Structures */
-	s = &dev->subdevices[1];
-	s->type = COMEDI_SUBD_UNUSED;
-
 	/*  Allocate and Initialise DI Subdevice Structures */
-	s = &dev->subdevices[2];
+	s = &dev->subdevices[0];
 	s->type = COMEDI_SUBD_DI;
 	s->subdev_flags = SDF_READABLE;
 	s->n_chan = 32;
@@ -94,7 +85,7 @@ static int apci1564_auto_attach(struct comedi_device *dev,
 	s->insn_bits = apci1564_di_insn_bits;
 
 	/*  Allocate and Initialise DO Subdevice Structures */
-	s = &dev->subdevices[3];
+	s = &dev->subdevices[1];
 	s->type = COMEDI_SUBD_DO;
 	s->subdev_flags = SDF_WRITEABLE;
 	s->n_chan = 32;
@@ -106,7 +97,7 @@ static int apci1564_auto_attach(struct comedi_device *dev,
 	s->insn_read = apci1564_do_read;
 
 	/*  Allocate and Initialise Timer Subdevice Structures */
-	s = &dev->subdevices[4];
+	s = &dev->subdevices[2];
 	if (this_board->i_Timer) {
 		s->type = COMEDI_SUBD_TIMER;
 		s->subdev_flags = SDF_WRITEABLE | SDF_GROUND | SDF_COMMON;
@@ -123,14 +114,6 @@ static int apci1564_auto_attach(struct comedi_device *dev,
 		s->type = COMEDI_SUBD_UNUSED;
 	}
 
-	/*  Allocate and Initialise TTL */
-	s = &dev->subdevices[5];
-	s->type = COMEDI_SUBD_UNUSED;
-
-	/* EEPROM */
-	s = &dev->subdevices[6];
-	s->type = COMEDI_SUBD_UNUSED;
-
 	i_ADDI_Reset(dev);
 	return 0;
 }
-- 
1.9.0

[PATCH 3/4] staging: comedi: addi_apci_1564: absorb apci1564_reset()

[Chase Southwood]

This is the only reset fuction used by this driver.  We can remove it from
the boardinfo and absorb the code from hwdrv_apci1564.c into the driver.

Signed-off-by: Chase Southwood <chase.southwood@gmail.com>
Cc: Ian Abbott <abbotti@mev.co.uk>
Cc: H Hartley Sweeten <hsweeten@visionengravers.com>
---
 .../comedi/drivers/addi-data/hwdrv_apci1564.c      | 32 -------------------
 drivers/staging/comedi/drivers/addi_apci_1564.c    | 36 ++++++++++++++++++----
 2 files changed, 30 insertions(+), 38 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
index 7976a22a..b3c3f60 100644
--- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
+++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
@@ -578,35 +578,3 @@ static void apci1564_interrupt(int irq, void *d)
 	}
 	return;
 }
-
-static int apci1564_reset(struct comedi_device *dev)
-{
-	struct addi_private *devpriv = dev->private;
-
-	ui_Type = 0;
-
-	/* Disable the input interrupts and reset status register */
-	outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DI_IRQ_REG);
-	inl(devpriv->i_IobaseAmcc + APCI1564_DI_INT_STATUS_REG);
-	outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DI_INT_MODE1_REG);
-	outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DI_INT_MODE2_REG);
-
-	/* Reset the output channels and disable interrupts */
-	outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DO_REG);
-	outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DO_INT_CTRL_REG);
-
-	/* Reset the watchdog registers */
-	addi_watchdog_reset(devpriv->i_IobaseAmcc + APCI1564_WDOG_REG);
-
-	/* Reset the timer registers */
-	outl(0x0, devpriv->i_IobaseAmcc + APCI1564_TIMER_CTRL_REG);
-	outl(0x0, devpriv->i_IobaseAmcc + APCI1564_TIMER_RELOAD_REG);
-
-	/* Reset the counter registers */
-	outl(0x0, dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER1));
-	outl(0x0, dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER2));
-	outl(0x0, dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER3));
-	outl(0x0, dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER4));
-
-	return 0;
-}
diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c b/drivers/staging/comedi/drivers/addi_apci_1564.c
index 31b687c..254889a 100644
--- a/drivers/staging/comedi/drivers/addi_apci_1564.c
+++ b/drivers/staging/comedi/drivers/addi_apci_1564.c
@@ -16,7 +16,6 @@ static const struct addi_board apci1564_boardtypes[] = {
 		.i_DoMaxdata		= 0xffffffff,
 		.i_Timer		= 1,
 		.interrupt		= apci1564_interrupt,
-		.reset			= apci1564_reset,
 		.timer_config		= apci1564_timer_config,
 		.timer_write		= apci1564_timer_write,
 		.timer_read		= apci1564_timer_read,
@@ -32,11 +31,35 @@ static irqreturn_t v_ADDI_Interrupt(int irq, void *d)
 	return IRQ_RETVAL(1);
 }
 
-static int i_ADDI_Reset(struct comedi_device *dev)
+static int apci1564_reset(struct comedi_device *dev)
 {
-	const struct addi_board *this_board = comedi_board(dev);
+	struct addi_private *devpriv = dev->private;
+
+	ui_Type = 0;
+
+	/* Disable the input interrupts and reset status register */
+	outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DI_IRQ_REG);
+	inl(devpriv->i_IobaseAmcc + APCI1564_DI_INT_STATUS_REG);
+	outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DI_INT_MODE1_REG);
+	outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DI_INT_MODE2_REG);
+
+	/* Reset the output channels and disable interrupts */
+	outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DO_REG);
+	outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DO_INT_CTRL_REG);
+
+	/* Reset the watchdog registers */
+	addi_watchdog_reset(devpriv->i_IobaseAmcc + APCI1564_WDOG_REG);
+
+	/* Reset the timer registers */
+	outl(0x0, devpriv->i_IobaseAmcc + APCI1564_TIMER_CTRL_REG);
+	outl(0x0, devpriv->i_IobaseAmcc + APCI1564_TIMER_RELOAD_REG);
+
+	/* Reset the counter registers */
+	outl(0x0, dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER1));
+	outl(0x0, dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER2));
+	outl(0x0, dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER3));
+	outl(0x0, dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER4));
 
-	this_board->reset(dev);
 	return 0;
 }
 
@@ -62,6 +85,8 @@ static int apci1564_auto_attach(struct comedi_device *dev,
 	dev->iobase = pci_resource_start(pcidev, 1);
 	devpriv->i_IobaseAmcc = pci_resource_start(pcidev, 0);
 
+	apci1564_reset(dev);
+
 	if (pcidev->irq > 0) {
 		ret = request_irq(pcidev->irq, v_ADDI_Interrupt, IRQF_SHARED,
 				  dev->board_name, dev);
@@ -114,7 +139,6 @@ static int apci1564_auto_attach(struct comedi_device *dev,
 		s->type = COMEDI_SUBD_UNUSED;
 	}
 
-	i_ADDI_Reset(dev);
 	return 0;
 }
 
@@ -124,7 +148,7 @@ static void apci1564_detach(struct comedi_device *dev)
 
 	if (devpriv) {
 		if (dev->iobase)
-			i_ADDI_Reset(dev);
+			apci1564_reset(dev);
 		if (dev->irq)
 			free_irq(dev->irq, dev);
 	}
-- 
1.9.0

[PATCH 4/4] staging: comedi: addi_apci_1564: call apci1564_interrupt() directly

[Chase Southwood]

Remove the boardinfo about the interrupt function and just call it
directly.

Signed-off-by: Chase Southwood <chase.southwood@gmail.com>
Cc: Ian Abbott <abbotti@mev.co.uk>
Cc: H Hartley Sweeten <hsweeten@visionengravers.com>
---
 drivers/staging/comedi/drivers/addi_apci_1564.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c b/drivers/staging/comedi/drivers/addi_apci_1564.c
index 254889a..df1a16e 100644
--- a/drivers/staging/comedi/drivers/addi_apci_1564.c
+++ b/drivers/staging/comedi/drivers/addi_apci_1564.c
@@ -15,7 +15,6 @@ static const struct addi_board apci1564_boardtypes[] = {
 		.i_NbrDoChannel		= 32,
 		.i_DoMaxdata		= 0xffffffff,
 		.i_Timer		= 1,
-		.interrupt		= apci1564_interrupt,
 		.timer_config		= apci1564_timer_config,
 		.timer_write		= apci1564_timer_write,
 		.timer_read		= apci1564_timer_read,
@@ -24,10 +23,7 @@ static const struct addi_board apci1564_boardtypes[] = {
 
 static irqreturn_t v_ADDI_Interrupt(int irq, void *d)
 {
-	struct comedi_device *dev = d;
-	const struct addi_board *this_board = comedi_board(dev);
-
-	this_board->interrupt(irq, d);
+	apci1564_interrupt(irq, d);
 	return IRQ_RETVAL(1);
 }
 
-- 
1.9.0

RE: [PATCH 0/4] staging: comedi: addi_apci_1564: Further cleanups

[Hartley Sweeten]

On Saturday, May 03, 2014 12:54 AM, Chase Southwood wrote:
> Latest patchset in a continuing cleanup of addi_apci_1564.c.  Main focus
> is on the auto_attach() function, but also cleaning up some of the old addi
> common code wrappers.
>
> To be applied on top of my prior patchset to this file (the one beginning
> with [PATCH 1/6] staging: comedi: addi_apci_1564: remove eeprom support code)
>
> Chase Southwood (4):
>   staging: comedi: addi_apci_1564: board has 32 digital outputs
>   staging: comedi: addi_apci_1564: don't allocate unused subdevices
>   staging: comedi: addi_apci_1564: absorb apci1564_reset()
>   staging: comedi: addi_apci_1564: call apci1564_interrupt() directly
>
>  .../comedi/drivers/addi-data/hwdrv_apci1564.c      | 32 -------
>  drivers/staging/comedi/drivers/addi_apci_1564.c    | 98 ++++++++++------------
>  2 files changed, 45 insertions(+), 85 deletions(-)

Looks good.

Reviewed-by: H Hartley Sweeten <hsweeten@visionengravers.com>

Re: [PATCH 0/4] staging: comedi: addi_apci_1564: Further cleanups

[Ian Abbott]

On 2014-05-03 08:54, Chase Southwood wrote:
> Latest patchset in a continuing cleanup of addi_apci_1564.c.  Main focus
> is on the auto_attach() function, but also cleaning up some of the old addi
> common code wrappers.
>
> To be applied on top of my prior patchset to this file (the one beginning
> with [PATCH 1/6] staging: comedi: addi_apci_1564: remove eeprom support code)
>
> Chase Southwood (4):
>    staging: comedi: addi_apci_1564: board has 32 digital outputs
>    staging: comedi: addi_apci_1564: don't allocate unused subdevices
>    staging: comedi: addi_apci_1564: absorb apci1564_reset()
>    staging: comedi: addi_apci_1564: call apci1564_interrupt() directly
>
>   .../comedi/drivers/addi-data/hwdrv_apci1564.c      | 32 -------
>   drivers/staging/comedi/drivers/addi_apci_1564.c    | 98 ++++++++++------------
>   2 files changed, 45 insertions(+), 85 deletions(-)

Thumbs up from me too!

Reviewed-by: Ian Abbott <abbotti@mev.co.uk>

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-