On Tuesday, February 24, 2015 6:20 AM, Ian Abbott wrote: > 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 <hsweeten@xxxxxxxxxxxxxxxxxxx> >> Cc: Ian Abbott <abbotti@xxxxxxxxx> >> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> >> --- >> 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. Ah.. Now I see what you mean. Thanks for the explanation. I'll post a v3 of this patch shortly. Thanks, Hartley _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel