staging: comedi: addi_apci_3120: fix timer (*insn_config)

The timer subdevice in this driver does not follow the comedi API.

Fix the (*insn_config) to correctly arm, disarm, set the mode, and
get the status of the timer.

Remove the unnecessary, and broken, (*insn_write).

The new (*insn_config) does not enable the interrupt for timer 2.
Remove the interrupt support code for the timer.

Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
Reviewed-by: Ian Abbott <abbotti@mev.co.uk>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

drivers/staging/comedi/drivers/addi-data/hwdrv_apci3120.c

@@ -40,19 +40,6 @@ This program is distributed in the hope that it will be useful, but WITHOUT ANY
   +----------+-----------+------------------------------------------------+
 */
 
-#define APCI3120_START			1
-#define APCI3120_STOP			0
-
-/* TIMER DEFINE */
-#define APCI3120_QUARTZ_A		70
-#define APCI3120_QUARTZ_B		50
-#define APCI3120_TIMER			1
-#define APCI3120_WATCHDOG		2
-#define APCI3120_TIMER_DISABLE		0
-#define APCI3120_TIMER_ENABLE		1
-
-#define APCI3120_COUNTER		3
-
 static void apci3120_addon_write(struct comedi_device *dev,
 				 unsigned int val, unsigned int reg)
 {
@@ -385,8 +372,6 @@ static irqreturn_t apci3120_interrupt(int irq, void *d)
 	if (devpriv->ctrl & APCI3120_CTRL_EXT_TRIG)
 		apci3120_exttrig_enable(dev, false);
 
-	apci3120_clr_timer2_interrupt(dev);
-
 	if (int_amcc & MASTER_ABORT_INT)
 		dev_err(dev->class_dev, "AMCC IRQ - MASTER DMA ABORT!\n");
 	if (int_amcc & TARGET_ABORT_INT)
@@ -412,28 +397,10 @@ static irqreturn_t apci3120_interrupt(int irq, void *d)
 	}
 
 	if (status & APCI3120_STATUS_TIMER2_INT) {
-		switch (devpriv->b_Timer2Mode) {
-		case APCI3120_COUNTER:
-			break;
-
-		case APCI3120_TIMER:
-
-			/* Send a signal to from kernel to user space */
-			send_sig(SIGIO, devpriv->tsk_Current, 0);
-			break;
-
-		case APCI3120_WATCHDOG:
-
-			/* Send a signal to from kernel to user space */
-			send_sig(SIGIO, devpriv->tsk_Current, 0);
-			break;
-
-		default:
-			/*  disable Timer Interrupt */
-			devpriv->mode &= ~APCI3120_MODE_TIMER2_IRQ_ENA;
-			outb(devpriv->mode, dev->iobase + APCI3120_MODE_REG);
-		}
-
+		/*
+		 * for safety...
+		 * timer2 interrupts are not enabled in the driver
+		 */
 		apci3120_clr_timer2_interrupt(dev);
 	}
 
@@ -441,8 +408,6 @@ static irqreturn_t apci3120_interrupt(int irq, void *d)
 		/* AMCC- Clear write complete interrupt (DMA) */
 		outl(AINT_WT_COMPLETE, devpriv->amcc + AMCC_OP_REG_INTCSR);
 
-		apci3120_clr_timer2_interrupt(dev);
-
 		/* do some data transfer */
 		apci3120_interrupt_dma(irq, d);
 	}
@@ -454,157 +419,3 @@ static irqreturn_t apci3120_interrupt(int irq, void *d)
 
 	return IRQ_HANDLED;
 }
-
-/*
- * Configure Timer 2
- *
- * data[0] = TIMER configure as timer
- *	   = WATCHDOG configure as watchdog
- * data[1] = Timer constant
- * data[2] = Timer2 interrupt (1)enable or(0) disable
- */
-static int apci3120_config_insn_timer(struct comedi_device *dev,
-				      struct comedi_subdevice *s,
-				      struct comedi_insn *insn,
-				      unsigned int *data)
-{
-	struct apci3120_private *devpriv = dev->private;
-	unsigned int divisor;
-
-	if (!data[1])
-		dev_err(dev->class_dev, "No timer constant!\n");
-
-	devpriv->b_Timer2Interrupt = (unsigned char) data[2];	/*  save info whether to enable or disable interrupt */
-
-	divisor = apci3120_ns_to_timer(dev, 2, data[1], CMDF_ROUND_DOWN);
-
-	apci3120_timer_enable(dev, 2, false);
-
-	/* disable timer 2 interrupt and reset operation mode (timer) */
-	devpriv->mode &= ~APCI3120_MODE_TIMER2_IRQ_ENA &
-			 ~APCI3120_MODE_TIMER2_AS_MASK;
-
-	/*  Disable Eoc and Eos Interrupts */
-	devpriv->mode &= ~APCI3120_MODE_EOC_IRQ_ENA &
-			 ~APCI3120_MODE_EOS_IRQ_ENA;
-	outb(devpriv->mode, dev->iobase + APCI3120_MODE_REG);
-
-	if (data[0] == APCI3120_TIMER) {	/* initialize timer */
-		/* Set the Timer 2 in mode 2(Timer) */
-		apci3120_timer_set_mode(dev, 2, APCI3120_TIMER_MODE2);
-
-		/* Set timer 2 delay */
-		apci3120_timer_write(dev, 2, divisor);
-
-		/*  timer2 in Timer mode enabled */
-		devpriv->b_Timer2Mode = APCI3120_TIMER;
-	} else {			/*  Initialize Watch dog */
-		/* Set the Timer 2 in mode 5(Watchdog) */
-		apci3120_timer_set_mode(dev, 2, APCI3120_TIMER_MODE5);
-
-		/* Set timer 2 delay */
-		apci3120_timer_write(dev, 2, divisor);
-
-		/* watchdog enabled */
-		devpriv->b_Timer2Mode = APCI3120_WATCHDOG;
-
-	}
-
-	return insn->n;
-
-}
-
-/*
- * To start and stop the timer
- *
- * data[0] = 1 (start)
- *	   = 0 (stop)
- *	   = 2 (write new value)
- * data[1] = new value
- *
- * devpriv->b_Timer2Mode = 0 DISABLE
- *			 = 1 Timer
- *			 = 2 Watch dog
- */
-static int apci3120_write_insn_timer(struct comedi_device *dev,
-				     struct comedi_subdevice *s,
-				     struct comedi_insn *insn,
-				     unsigned int *data)
-{
-	struct apci3120_private *devpriv = dev->private;
-	unsigned int divisor;
-
-	if ((devpriv->b_Timer2Mode != APCI3120_WATCHDOG)
-		&& (devpriv->b_Timer2Mode != APCI3120_TIMER)) {
-		dev_err(dev->class_dev, "timer2 not configured\n");
-		return -EINVAL;
-	}
-
-	if (data[0] == 2) {	/*  write new value */
-		if (devpriv->b_Timer2Mode != APCI3120_TIMER) {
-			dev_err(dev->class_dev,
-				"timer2 not configured in TIMER MODE\n");
-			return -EINVAL;
-		}
-	}
-
-	switch (data[0]) {
-	case APCI3120_START:
-		apci3120_clr_timer2_interrupt(dev);
-
-		if (devpriv->b_Timer2Mode == APCI3120_TIMER) {	/* start timer */
-			/* Enable Timer */
-			devpriv->mode &= 0x0b;
-			devpriv->mode |= APCI3120_MODE_TIMER2_AS_TIMER;
-		} else {		/* start watch dog */
-			/* Enable WatchDog */
-			devpriv->mode &= 0x0b;
-			devpriv->mode |= APCI3120_MODE_TIMER2_AS_WDOG;
-		}
-
-		/* enable disable interrupt */
-		if (devpriv->b_Timer2Interrupt) {
-			devpriv->mode |= APCI3120_MODE_TIMER2_IRQ_ENA;
-
-			/*  save the task structure to pass info to user */
-			devpriv->tsk_Current = current;
-		} else {
-			devpriv->mode &= ~APCI3120_MODE_TIMER2_IRQ_ENA;
-		}
-		outb(devpriv->mode, dev->iobase + APCI3120_MODE_REG);
-
-		/* start timer */
-		if (devpriv->b_Timer2Mode == APCI3120_TIMER)
-			apci3120_timer_enable(dev, 2, true);
-		break;
-
-	case APCI3120_STOP:
-		/* disable timer 2 interrupt and reset operation mode (timer) */
-		devpriv->mode &= ~APCI3120_MODE_TIMER2_IRQ_ENA &
-				 ~APCI3120_MODE_TIMER2_AS_MASK;
-		outb(devpriv->mode, dev->iobase + APCI3120_MODE_REG);
-
-		apci3120_timer_enable(dev, 2, false);
-
-		apci3120_clr_timer2_interrupt(dev);
-		break;
-
-	case 2:		/* write new value to Timer */
-		if (devpriv->b_Timer2Mode != APCI3120_TIMER) {
-			dev_err(dev->class_dev,
-				"timer2 not configured in TIMER MODE\n");
-			return -EINVAL;
-		}
-
-		divisor = apci3120_ns_to_timer(dev, 2, data[1],
-					       CMDF_ROUND_DOWN);
-
-		/* Set timer 2 delay */
-		apci3120_timer_write(dev, 2, divisor);
-		break;
-	default:
-		return -EINVAL;	/*  Not a valid input */
-	}
-
-	return insn->n;
-}

drivers/staging/comedi/drivers/addi_apci_3120.c

@@ -139,9 +139,6 @@ struct apci3120_private {
 	unsigned char timer_mode;
 	unsigned char mode;
 	unsigned short ctrl;
-	unsigned char b_Timer2Mode;
-	unsigned char b_Timer2Interrupt;
-	struct task_struct *tsk_Current;
 };
 
 /*
@@ -447,6 +444,83 @@ static int apci3120_do_insn_bits(struct comedi_device *dev,
 	return insn->n;
 }
 
+static int apci3120_timer_insn_config(struct comedi_device *dev,
+				      struct comedi_subdevice *s,
+				      struct comedi_insn *insn,
+				      unsigned int *data)
+{
+	struct apci3120_private *devpriv = dev->private;
+	unsigned int divisor;
+	unsigned int status;
+	unsigned int mode;
+	unsigned int timer_mode;
+
+	switch (data[0]) {
+	case INSN_CONFIG_ARM:
+		apci3120_clr_timer2_interrupt(dev);
+		divisor = apci3120_ns_to_timer(dev, 2, data[1],
+					       CMDF_ROUND_DOWN);
+		apci3120_timer_write(dev, 2, divisor);
+		apci3120_timer_enable(dev, 2, true);
+		break;
+
+	case INSN_CONFIG_DISARM:
+		apci3120_timer_enable(dev, 2, false);
+		apci3120_clr_timer2_interrupt(dev);
+		break;
+
+	case INSN_CONFIG_GET_COUNTER_STATUS:
+		data[1] = 0;
+		data[2] = COMEDI_COUNTER_ARMED | COMEDI_COUNTER_COUNTING |
+			  COMEDI_COUNTER_TERMINAL_COUNT;
+
+		if (devpriv->ctrl & APCI3120_CTRL_GATE(2)) {
+			data[1] |= COMEDI_COUNTER_ARMED;
+			data[1] |= COMEDI_COUNTER_COUNTING;
+		}
+		status = inw(dev->iobase + APCI3120_STATUS_REG);
+		if (status & APCI3120_STATUS_TIMER2_INT) {
+			data[1] &= ~COMEDI_COUNTER_COUNTING;
+			data[1] |= COMEDI_COUNTER_TERMINAL_COUNT;
+		}
+		break;
+
+	case INSN_CONFIG_SET_COUNTER_MODE:
+		switch (data[1]) {
+		case I8254_MODE0:
+			mode = APCI3120_MODE_TIMER2_AS_COUNTER;
+			timer_mode = APCI3120_TIMER_MODE0;
+			break;
+		case I8254_MODE2:
+			mode = APCI3120_MODE_TIMER2_AS_TIMER;
+			timer_mode = APCI3120_TIMER_MODE2;
+			break;
+		case I8254_MODE4:
+			mode = APCI3120_MODE_TIMER2_AS_TIMER;
+			timer_mode = APCI3120_TIMER_MODE4;
+			break;
+		case I8254_MODE5:
+			mode = APCI3120_MODE_TIMER2_AS_WDOG;
+			timer_mode = APCI3120_TIMER_MODE5;
+			break;
+		default:
+			return -EINVAL;
+		}
+		apci3120_timer_enable(dev, 2, false);
+		apci3120_clr_timer2_interrupt(dev);
+		apci3120_timer_set_mode(dev, 2, timer_mode);
+		devpriv->mode &= ~APCI3120_MODE_TIMER2_AS_MASK;
+		devpriv->mode |= mode;
+		outb(devpriv->mode, dev->iobase + APCI3120_MODE_REG);
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	return insn->n;
+}
+
 static int apci3120_timer_insn_read(struct comedi_device *dev,
 				    struct comedi_subdevice *s,
 				    struct comedi_insn *insn,
@@ -627,11 +701,10 @@ static int apci3120_auto_attach(struct comedi_device *dev,
 	/* Timer subdevice */
 	s = &dev->subdevices[4];
 	s->type		= COMEDI_SUBD_TIMER;
-	s->subdev_flags	= SDF_WRITABLE | SDF_READABLE;
+	s->subdev_flags	= SDF_READABLE;
 	s->n_chan	= 1;
 	s->maxdata	= 0x00ffffff;
-	s->insn_write	= apci3120_write_insn_timer;
-	s->insn_config	= apci3120_config_insn_timer;
+	s->insn_config	= apci3120_timer_insn_config;
 	s->insn_read	= apci3120_timer_insn_read;
 
 	return 0;