On 23/02/15 21:58, H Hartley Sweeten wrote:
Convert this driver to use the comedi_8254 module to provide the 8254 timer 
support.

Add 'clock_src' and 'gate_src' members to the comedi_8254 data for convienence.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
v2: no change

  drivers/staging/comedi/Kconfig                     |   1 +
  .../staging/comedi/drivers/amplc_dio200_common.c   | 263 ++++++---------------
  drivers/staging/comedi/drivers/comedi_8254.h       |   4 +
  3 files changed, 72 insertions(+), 196 deletions(-)
[snip]
+static int dio200_subdev_8254_offset(struct comedi_device *dev,
+                                    struct comedi_subdevice *s)
  {
-       const struct dio200_board *board = dev->board_ptr;
-       struct dio200_subdev_8254 *subpriv = s->private;
+       struct comedi_8254 *i8254 = s->private;

-       if (!board->has_clk_gat_sce)
-               return -1;
+       if (dev->mmio)
+               return i8254->mmio - dev->mmio;

-       return subpriv->gate_src[counter_number];
+       return i8254->iobase - dev->iobase;
  }

-static int dio200_subdev_8254_set_clock_src(struct comedi_device *dev,
+static void dio200_subdev_8254_set_gate_src(struct comedi_device *dev,
                                            struct comedi_subdevice *s,
-                                           unsigned int counter_number,
-                                           unsigned int clock_src)
+                                           unsigned int chan,
+                                           unsigned int src)
  {
-       const struct dio200_board *board = dev->board_ptr;
-       struct dio200_subdev_8254 *subpriv = s->private;
-       unsigned char byte;
+       unsigned int offset = dio200_subdev_8254_offset(dev, s);

-       if (!board->has_clk_gat_sce)
-               return -1;
-       if (clock_src > (board->is_pcie ? 31 : 7))
-               return -1;
-
-       subpriv->clock_src[counter_number] = clock_src;
-       byte = clk_sce((subpriv->ofs >> 2) & 1, counter_number, clock_src);
-       dio200_write8(dev, DIO200_CLK_SCE(subpriv->ofs >> 3), byte);
-
-       return 0;
+       dio200_write8(dev, DIO200_GAT_SCE(offset >> 3),
+                     gat_sce((offset >> 2) & 1, chan, src));
  }

-static int dio200_subdev_8254_get_clock_src(struct comedi_device *dev,
-                                           struct comedi_subdevice *s,
-                                           unsigned int counter_number,
-                                           unsigned int *period_ns)
+static void dio200_subdev_8254_set_clock_src(struct comedi_device *dev,
+                                            struct comedi_subdevice *s,
+                                            unsigned int chan,
+                                            unsigned int src)
  {
-       const struct dio200_board *board = dev->board_ptr;
-       struct dio200_subdev_8254 *subpriv = s->private;
-       unsigned clock_src;
-
-       if (!board->has_clk_gat_sce)
-               return -1;
+       unsigned int offset = dio200_subdev_8254_offset(dev, s);

-       clock_src = subpriv->clock_src[counter_number];
-       *period_ns = clock_period[clock_src];
-       return clock_src;
+       dio200_write8(dev, DIO200_CLK_SCE(offset >> 3),
+                     clk_sce((offset >> 2) & 1, chan, src));
  }
[snip]
@@ -686,28 +551,34 @@ static int dio200_subdev_8254_init(struct comedi_device 
*dev,
                                   unsigned int offset)
  {
        const struct dio200_board *board = dev->board_ptr;
-       struct dio200_subdev_8254 *subpriv;
-       unsigned int chan;
+       struct comedi_8254 *i8254;
+       unsigned int regshift;
+       int chan;

-       subpriv = comedi_alloc_spriv(s, sizeof(*subpriv));
-       if (!subpriv)
+       regshift = (board->is_pcie) ? 3 : 0;
+
+       if (dev->mmio)
+               i8254 = comedi_8254_mm_init(dev->mmio + offset,
+                                           0, I8254_IO8, regshift);
+       else
+               i8254 = comedi_8254_init(dev->iobase + offset,
+                                        0, I8254_IO8, regshift);
+       if (!i8254)
                return -ENOMEM;

-       s->type = COMEDI_SUBD_COUNTER;
-       s->subdev_flags = SDF_WRITABLE | SDF_READABLE;
-       s->n_chan = 3;
-       s->maxdata = 0xFFFF;
-       s->insn_read = dio200_subdev_8254_read;
-       s->insn_write = dio200_subdev_8254_write;
-       s->insn_config = dio200_subdev_8254_config;
+       comedi_8254_subdevice_init(s, i8254);

-       subpriv->ofs = offset;
+       i8254->insn_config = dio200_subdev_8254_config;
+
+       /*
+        * Set the runflag bit so that the core will autmatically
+        * kfree(s->private) when the driver is detached.
+        */
+       s->runflags |= COMEDI_SRF_FREE_SPRIV;

        /* Initialize channels. */
-       for (chan = 0; chan < 3; chan++) {
-               dio200_subdev_8254_set_mode(dev, s, chan,
-                                           I8254_MODE0 | I8254_BINARY);
-               if (board->has_clk_gat_sce) {
+       if (board->has_clk_gat_sce) {
+               for (chan = 0; chan < 3; chan++) {
                        /* Gate source 0 is VCC (logic 1). */
                        dio200_subdev_8254_set_gate_src(dev, s, chan, 0);
                        /* Clock source 0 is the dedicated clock input. */

These are still wrong.  As an example, let's use subdevice 4 of pcie215.

board->sd_type[4] is sd_8254, board->sd_info[4] is 0x10. 0x10 is the offset that gets passed to dio200_subdev_8254_init().

Previously, the 0x10 would be stored in subpriv->ofs. Let's say the insn_read handler is called for one of the three channels. dio200_subdev_8254_read() would call dio200_subdev_8254_read_chan() for the specified channel of the subdevice. The first thing that does is write to the 8254 control register (to latch the counter). To do that it calls dio200_write8() with an offset of subpriv->ofs + i8254_control_reg, which is 0x10 + 3, or 0x13. Since this board has a regshift of 3, dio200_write8() changes the offset to 0x13 << 3, or 0x98. The register written to is at dev->mmio + 0x98.

The new code sets up the 8254 by calling comedi_mm_init() with a base address of dev->mmio + 0x10 and a regshift of 3. So the four registers it accesses will be at dev->mmio + 0x10, dev->mmio + 0x18, dev->mmio + 0x20 and dev->mmio + 0x28. These are all 0x70 less than they should be. The correct base address to pass to comedi_mm_init() is dev->mmio + 0x80, i.e. dev->mmio + (offset << regshift). However, that would cause your dio200_subdev_8254_offset() to return an offset 8 times bigger than it needs to be, so dio200_subdev_8254_offset() should shift the offset right by regshift before returning it.

--
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbo...@mev.co.uk> )=-
-=(                          Web: http://www.mev.co.uk/  )=-
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to