Jon Smirl wrote:
> I had to rework this portion of code several times in the IR code I posted.
>
> I had the core provide input_ir_queue() which was legal to call from
> interrupt context. Calling from interrupt context was an important
> aspect I missed in the first versions. I made this a common routine so
> that the code didn't get copied into all of the drivers. This code
> should have used kfifo but I didn't know about kfifo.
>
>>> The question is though, is the kfifo and work handler really
> necessary?
>
> Yes, otherwise it will get duplicated into all of the drivers that run
> in interrupt context like this GPIO one. Put this common code into the
> core so that the individual drivers writers don't mess it up.
>
> void input_ir_queue(struct input_dev *dev, int sample)
> {
> unsigned int next;
>
> spin_lock(dev->ir->queue.lock);
> dev->ir->queue.samples[dev->ir->queue.head] = sample;
> next = dev->ir->queue.head + 1;
> dev->ir->queue.head = (next >= MAX_SAMPLES ? 0 : next);
> spin_unlock(dev->ir->queue.lock);
>
> schedule_work(&dev->ir->work);
> }
The big advantage of using kfifo is that you don't need to use a spinlock, if
there's just one consumer of the event. On the implementation I did, just
one code writes to the kfifo (the driver) and just one code reads from the
kfifo, and multiplexing the data to the several decoders (and lirc_dev).
So, no locks.
>
> My GPIO implementation simply call input_it_queue() with the timing
> data. I collapsed multiple long space interrupts into one very long
> space. If you are using protocol engines, there is no need to detect
> the long trailing space. The protocol engine will trigger on the last
> pulse of the signal.
>
> On the other hand, LIRC in user space needs the last long space to
> know when to flush the buffer from kernel space into user space. The
> timeout for this flush should be implemented in the LIRC compatibility
> driver, not ir-core. In this case my GPIO driver doesn't ever generate
> an event for the long space at the end of the message (because it
> doesn't end). Instead the LIRC compatibility layer should start a
> timer and flush when no data has been received for 200ms or whatever.
Agreed.
> static irqreturn_t dpeak_ir_irq(int irq, void *_ir)
> {
> struct ir_gpt *ir_gpt = _ir;
> int sample, count, delta, bit, wrap;
>
> sample = in_be32(&ir_gpt->regs->status);
> out_be32(&ir_gpt->regs->status, 0xF);
>
> count = sample >> 16;
> wrap = (sample >> 12) & 7;
> bit = (sample >> 8) & 1;
>
> delta = count - ir_gpt->previous;
> delta += wrap * 0x10000;
>
> ir_gpt->previous = count;
>
> if (bit)
> delta = -delta;
>
> input_ir_queue(ir_gpt->input, delta);
>
> return IRQ_HANDLED;
> }
>
> For MSMCE I converted their format back into simple delays and fed it
> into input_ir_queue(). This was not done in interrupt context because
> of the way USB works. input_ir_queue() doesn't care - it works
> correctly when called from either context.
>
> if (ir->last.command == 0x80) {
> bit = ((ir->buf_in[i] & MCE_PULSE_BIT) != 0);
> delta = (ir->buf_in[i] & MCE_PULSE_MASK) * MCE_TIME_BASE;
>
> if ((ir->buf_in[i] & MCE_PULSE_MASK) == 0x7f) {
> if (ir->last.bit == bit)
> ir->last.delta += delta;
> else {
> ir->last.delta = delta;
> ir->last.bit = bit;
> }
> continue;
> }
> delta += ir->last.delta;
> ir->last.delta = 0;
> ir->last.bit = bit;
>
> dev_dbg(&ir->usbdev->dev, "bit %d delta %d\n", bit, delta);
> if (bit)
> delta = -delta;
>
> input_ir_queue(ir->input, delta);
> }
>
> These delay messages are then fed into the protocol engines which
> process the pulses in parallel. Processing in parallel works, because
> that's how IR receivers work. When you shine a remote on an equipment
> rack, all of the equipment sees the command in parallel. The protocols
> are designed so that parallel decode works properly.
On the implementation I did, each event is passed to each decoder serialized (yet, as one keystroke
is a series of events, it behaves as if they are processed in parallel). We might create separate
kthreads for each decoder, and use a spinlock at kfifo, but I suspect that the end result will be
very close and we'll have more threads interfering at the samples collect, especially on those
(broken) hardware that don't have IRQ's to indicate a state transition, so the driver needs
to poll the samples.
--
Cheers,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
