Re: [RFC PATCH 01/36] staging: comedi: comedi_8254: introduce module for 8254 timer support

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

 



On 20/02/15 23:04, H Hartley Sweeten wrote:
A 8254 timer/counter is commonly used on data acquisition boards to provide
the internal pacer clock used to acquire analog input samples. Some boards
also to allow the timers to be used externally.

Currently the 8254 timers are supported by comedi using the 8253.h header
and a number of inline functions. This works for the internal pacer clock
but requires the drivers to implement subdevice code necessary to use the
timers externally.

Introduce a new module to support both the internal pacer clock and the
external counter subdevice. This will allow removing a bunch of duplicated
code in the drivers and standardizes the comedi 8254 timer support.

This implementation is based on the 8253.h inline functions and the various
subdevice functionality in the comedi drivers.

Signed-off-by: H Hartley Sweeten <hsweeten@xxxxxxxxxxxxxxxxxxx>
Cc: Ian Abbott <abbotti@xxxxxxxxx>
Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
  drivers/staging/comedi/Kconfig               |   3 +
  drivers/staging/comedi/drivers/Makefile      |   1 +
  drivers/staging/comedi/drivers/comedi_8254.c | 665 +++++++++++++++++++++++++++
  drivers/staging/comedi/drivers/comedi_8254.h | 129 ++++++
  4 files changed, 798 insertions(+)
  create mode 100644 drivers/staging/comedi/drivers/comedi_8254.c
  create mode 100644 drivers/staging/comedi/drivers/comedi_8254.h

[snip]
diff --git a/drivers/staging/comedi/drivers/comedi_8254.c b/drivers/staging/comedi/drivers/comedi_8254.c
new file mode 100644
index 0000000..51fe043
--- /dev/null
+++ b/drivers/staging/comedi/drivers/comedi_8254.c
[snip]
+static unsigned int __i8254_read(struct comedi_8254 *i8254, unsigned int reg)
+{
+	unsigned int reg_offset = (reg * i8254->iosize) << i8254->regshift;

How is the regshift meant to be used when iosize is 2 (I8254_IO16) or 4 (I8254_IO32)? The usage above suggests that regshift should be 0 when there are no gaps in the registers, but the sanity check in __i8254_init() suggests otherwise.

+	unsigned int val;
+
+	switch (i8254->iosize) {
+	default:
+	case I8254_IO8:
+		if (i8254->mmio)
+			val = readb(i8254->mmio + reg_offset);
+		else
+			val = inb(i8254->iobase + reg_offset);
+		break;
+	case I8254_IO16:
+		if (i8254->mmio)
+			val = readw(i8254->mmio + reg_offset);
+		else
+			val = inw(i8254->iobase + reg_offset);
+		break;
+	case I8254_IO32:
+		if (i8254->mmio)
+			val = readl(i8254->mmio + reg_offset);
+		else
+			val = inl(i8254->iobase + reg_offset);
+		break;
+	}
+	return val;

Only the bottom 8 bits of the returned value are valid, so it ought to be ANDed with 0xff.

[snip]
+static struct comedi_8254 *__i8254_init(unsigned long iobase,
+					void __iomem *mmio,
+					unsigned int osc_base,
+					unsigned int iosize,
+					unsigned int regshift)
+{
+	struct comedi_8254 *i8254;
+	int i;
+
+	/* sanity check the iosize and that the regshift is valid */
+	if (!(iosize == I8254_IO8 ||
+	      (iosize == I8254_IO16 && regshift >= 1) ||
+	      (iosize == I8254_IO32 && regshift >= 2)))
+		return NULL;

(This snippet referred to above.)

--
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@xxxxxxxxx> )=-
-=(                          Web: http://www.mev.co.uk/  )=-
_______________________________________________
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