On 11/27/13 10:59, Lars-Peter Clausen wrote:
> [...]
>>> I had to implement the IRQ handler since the soc_camera model does not use
>>> interrupt_service_routine subdevice callback and R-Car VIN knows nothing about adv7612
>>> interrupt routed to a GPIO pin.
>>> So I had to schedule a workqueue and call adv7604_isr from there in case an interrupt happens.
>>
>> For our systems the adv7604 interrupts is not always hooked up to a gpio irq, instead
>> a register has to be read to figure out which device actually produced the irq. So I
>> want to keep the interrupt_service_routine(). However, adding a gpio field to the
>> platform_data that, if set, will tell the driver to request an irq and setup a
>> workqueue that calls interrupt_service_routine() would be fine with me. That will
>> benefit a lot of people since using gpios is much more common.
>
> I'll look into adding that since I'm using a GPIO for the interrupt on my
> platform as well.
>
>>
>>> The driver enables multiple interrupts on the chip, however, the adv7604_isr callback doesn't
>>> seem to handle them correctly.
>>> According to the docs:
>>> "If an interrupt event occurs, and then a second interrupt event occurs before the system controller
>>> has cleared or masked the first interrupt event, the ADV7611 does not generate a second interrupt signal."
>>>
>>> However, the interrupt_service_routine doesn't account for that.
>>> For example, in case fmt_change interrupt happens while fmt_change_digital interrupt is being
>>> processed by the adv7604_isr routine. If fmt_change status is set just before we clear fmt_change_digital,
>>> we never clear fmt_change. Thus, we end up with fmt_change interrupt missed and therefore further interrupts disabled.
>>> I've tried to call the adv7604_isr routine in a loop and return from the worlqueue only when all interrupt status bits are cleared.
>>> This did help a bit, but sometimes I started getting lots of I2C read/write errors for some reason.
>>
>> I'm not sure if there is much that can be done about this. The code reads the
>> interrupt status, then clears the interrupts right after. There is always a
>> race condition there since this isn't atomic ('read and clear'). Unless Lars-Peter
>> has a better idea?
>>
>
> As far as I understand it the interrupts lines are level triggered and will
> stay asserted as long as there are unmasked, non-acked IRQS.
You are correct. If you are using level interrupts, then the current
implementation works fine. However, when using edge interrupts (and we
have one system that apparently has only edge-triggered GPIOs), then
it will fail.
The only way to handle that is to mask all interrupts at the start of
the isr, process the interrupts as usual, and unmask the interrupts
at the end of the isr. AFAICT this method should be safe as well with
level triggered interrupts.
Disregard my comment about clearing all interrupts, that's bogus.
> So the
> interrupt handler should be re-entered if there are still pending
> interrupts. Is it possible that you setup a edge triggered interrupt, in
> that case the handler wouldn't be reentered if the signal stays asserted?
>
> But that's just my understanding from the manual, I'll have to verify
> whether the hardware does indeed work like that.
>
> - Lars
>
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
