[PATCH v2 34/36] staging: comedi: amplc_dio200_common: convert driver to use the comedi_8254 module

[Ian Abbott]

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 at visionengravers.com>
> Cc: Ian Abbott <abbotti at mev.co.uk>
> Cc: Greg Kroah-Hartman <gregkh at 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: <abbotti at mev.co.uk> )=-
-=(                          Web: http://www.mev.co.uk/  )=-