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 <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.

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
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to