On Friday, June 03, 2016 2:58 AM, Ian Abbott wrote: > On 02/06/16 22:58, H Hartley Sweeten wrote: >> This board supports change-of-state interrupts on digital inputs 4 to 19 >> not 0 to 15. >> >> The current code "works" but it could set inappropriate bits in the mode1 >> and mode2 registers that setup which channels are enabled. It also doesn't >> return the status of the upper 4 channels (19 to 16). >> >> Fix the comment and mask the mode1/mode2 values so that only the interrupt >> capable channels can be enabled. >> >> Add the SDF_LSAMPL flag to the subdevice so that 32-bit samples are used >> instead of 16-bit ones. This allows returning the upper 4 channels. Use >> the remaining bits in the sample to return "event" flags to the user. >> >> The timer and counter subdevices can also generate interrupts and are a bit >> hacked. They don't currently follow the comedi API and they use send_sig() >> to let the task that know that the interrupt occured. The "event" flags will >> be used instead when these subdevices are fixed. >> >> Signed-off-by: H Hartley Sweeten <hsweeten@xxxxxxxxxxxxxxxxxxx> >> Cc: Ian Abbott <abbotti@xxxxxxxxx> >> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> >> --- >> drivers/staging/comedi/drivers/addi_apci_1564.c | 37 +++++++++++++++++-------- >> 1 file changed, 26 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c b/drivers/staging/comedi/drivers/addi_apci_1564.c >> index f1ccfbd..37e18b3 100644 <snip> >> +#define APCI1564_DI_INT_MODE_MASK 0x000ffff0 /* chans [19:4] */ <snip> >> +#define APCI1564_EVENT_MASK 0xfff0000f /* all but [19:4] */ <snip> >> + s->state &= ~APCI1564_EVENT_MASK; >> + > status = inl(dev->iobase + APCI1564_DI_IRQ_REG); > if (status & APCI1564_DI_IRQ_ENA) { > - /* disable the interrupt */ > + s->state = inl(dev->iobase + APCI1564_DI_INT_STATUS_REG); > + s->state |= APCI1564_EVENT_COS; <snip> > + if (s->state & APCI1564_EVENT_MASK) { > + comedi_buf_write_samples(s, &s->state, 1); > + comedi_handle_events(dev, s); > + } > + > return IRQ_HANDLED; > } > I'm struggling to see how that works. It looks like once > APCI1564_EVENT_COS has been set in s->state, it never gets cleared (or > at least it only gets cleared momentarily), and after that, the `if > (s->state & APCI1564_EVENT_MASK)` test will always be true, and so it > will write a sample to the buffer on every interrupt. Ian, 1) When an interrupt occurs the last "events" are masked off (leaving the last state of the digital inputs). 2) If the digital inputs caused the interrupt, the state of the digital inputs is read and the event bit APCI1564_EVENT_COS is set. 3) Similarly, the other patches add support for the timer and counter subdevices and set the APCI1564_EVENT_TIMER or APCI1564_EVENT_COUNTER(chan) bits as appropriate. 4) At the end of the interrupt if any of the event bits are set the sample will be written to the buffer and the event handled. This seemed like the best way to handle the timer and counter interrupts without adding unnecessary async command support. Regards, Hartley _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel