On Tuesday, October 06, 2015 4:06 AM, Ian Abbott wrote: > On 05/10/15 23:33, H Hartley Sweeten wrote: >> The board has an 8254 timer/counter. Add support for this subdevice. >> >> Signed-off-by: H Hartley Sweeten <hsweeten@xxxxxxxxxxxxxxxxxxx> >> Cc: Ian Abbott <abbotti@xxxxxxxxx> >> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> >> --- >> drivers/staging/comedi/drivers/multiq3.c | 147 ++++++++++++++++++++++++++++++- >> 1 file changed, 146 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/staging/comedi/drivers/multiq3.c b/drivers/staging/comedi/drivers/multiq3.c >> index fc743df..50efc7c 100644 >> --- a/drivers/staging/comedi/drivers/multiq3.c >> +++ b/drivers/staging/comedi/drivers/multiq3.c >> @@ -38,6 +38,8 @@ >> >> #include "../comedidev.h" >> >> +#include "comedi_8254.h" /* just for the register map */ >> + >> /* >> * Register map >> */ >> @@ -47,6 +49,8 @@ >> #define MULTIQ3_AI_REG 0x04 >> #define MULTIQ3_AI_CONV_REG 0x04 >> #define MULTIQ3_STATUS_REG 0x06 >> +#define MULTIQ3_STATUS_CT(x) (((x) == 0) ? BIT(0) : \ >> + ((x) == 1) ? BIT(2) : BIT(1)) > > Which manual are you looking at, Hartley? The first copy I found online > was this one: > > http://engineering.nyu.edu/mechatronics/Control_Lab/mq3_manual.pdf Same manual. > I suspect the strange ordering of the CTx bits in the diagram in section > 3.2.4.2 is probably a typo. Underneath the diagram it says, "Bits 0-2 > (CT0,CT1,CT2): Counter timeout states for the three timers." There's a > discrepancy between that and the diagram, so I think I'd stick with the > one that is less stupid! I also thought the order was a bit strange. > These CTx status bits are more than likely connected to the OUT0, OUT1 > and OUT2 pins of the 82C54 and will behave according to the mode of the > respective counter channels. > >> #define MULTIQ3_STATUS_EOC BIT(3) >> #define MULTIQ3_STATUS_EOC_I BIT(4) >> #define MULTIQ3_CTRL_REG 0x06 >> @@ -257,6 +261,126 @@ static int multiq3_encoder_insn_config(struct comedi_device *dev, > return insn->n; > } > > +static unsigned int multiq3_i8254_read(struct comedi_device *dev, > + unsigned int reg) > +{ > + /* select the 8254 register then read the value */ > + multiq3_set_ctrl(dev, MULTIQ3_CTRL_RC(reg)); > + return inb(dev->iobase + MULTIQ3_CLK_REG); > +} > > According to the manual I linked to, sections 3.2, 3.2.5.1 and 3.2.5.2 > indicate that CLK_DATA (your MULTIQ3_CLK_REG) is write-only. This means > the INSN_CONFIG_8254_READ_STATUS config instruction and the insn_read > handler won't work, although INSN_CONFIG_8254_READ_STATUS could be > partially emulated (see below). Ugh.. Missed that part. It does appear that the CLK_DATA register is write only. Kind of a silly design choice... <snip> > As mentioned above, the insn_read handler this won't work, unfortunately! <snip> >> +static int multiq3_8254_insn_config(struct comedi_device *dev, >> + struct comedi_subdevice *s, >> + struct comedi_insn *insn, >> + unsigned int *data) >> +{ >> + unsigned int chan = CR_CHAN(insn->chanspec); >> + unsigned int status; >> + int ret; >> + >> + switch (data[0]) { >> + case INSN_CONFIG_RESET: >> + ret = multiq3_8254_set_mode(dev, chan, >> + I8254_MODE0 | I8254_BINARY); >> + if (ret) >> + return ret; >> + break; >> + case INSN_CONFIG_SET_COUNTER_MODE: >> + ret = multiq3_8254_set_mode(dev, chan, data[1]); >> + if (ret) >> + return ret; >> + break; >> + case INSN_CONFIG_GET_COUNTER_STATUS: >> + data[1] = 0; >> + status = inw(dev->iobase + MULTIQ3_STATUS_REG); >> + if (status & MULTIQ3_STATUS_CT(chan)) >> + data[1] |= COMEDI_COUNTER_TERMINAL_COUNT; >> + data[2] = COMEDI_COUNTER_TERMINAL_COUNT; >> + break; > > I think this would only indicate a terminal count if the counter channel > is in mode 0. Or possibly the state of the 'out' signal for each counter. >> + case INSN_CONFIG_8254_READ_STATUS: >> + multiq3_i8254_write(dev, I8254_CTRL_READBACK_STATUS | >> + I8254_CTRL_READBACK_SEL_CTR(chan), >> + I8254_CTRL_REG); >> + >> + data[1] = multiq3_i8254_read(dev, chan); >> + break; > > As mentioned above, this won't work, unfortunately, due to the register > being write-only. Most of the function can be emulated by remembering > the mode the channel is set to, and by reading the output status of the > channel from MULTIQ3_STATUS_REG. The format of the 8254 status is: I think just dropping this patch is probably the right choice. As you mentioned in the follow up email, the counter/timers don't appear to be very useful on this board. It doesn't appear that any of the signals (clk, gate, out) are available to the user and they can only really be used to generate an interrupt with the output of the timer. Are you ok with the rest of the series if this one is dropped? Thanks, Hartley _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
