iio: trigger: Software Resend of Triggers

iio: trigger: Software Resend of Triggers

[Ibrahim Bagriyanik]

Dear IIO Community,

I was developing a device driver for the MAX11040K ADC and have encountered an issue where the device begins sampling but stops after a few seconds. After debugging, I found that when an interrupt arrives while another trigger handler is still in progress -has not called iio_trigger_notify_done-, the poll calls seem to get lost.

As you know, the kernel provides the CONFIG_HARDIRQS_SW_RESEND option for software resends of IRQs. This feature masks IRQs in the control flow and resends them once the current IRQ handler finishes. I looked for a similar implementation in the IIO trigger source code but couldn't find anything.

Is there a similar mechanism within IIO, or how do others typically address this problem on slower processors? If there isn't, would implementing such a feature in IIO make sense? I tackled this by simply not using IIO triggers, and only thing I thought to make use of was simply blocking poll calls with synchronization primitives until the former trigger finishes. I would appreciate your thoughts and suggestions.

Best,
Ibrahim Bagriyanik.

Re: iio: trigger: Software Resend of Triggers

[Jonathan Cameron]

On Sat, 24 Aug 2024 22:05:42 +0300
Ibrahim Bagriyanik <ibrahim.bagriyanikb@gmail.com> wrote:

> Dear IIO Community,
> 
> I was developing a device driver for the MAX11040K ADC and have
> encountered an issue where the device begins sampling but stops after
> a few seconds. After debugging, I found that when an interrupt
> arrives while another trigger handler is still in progress -has not
> called iio_trigger_notify_done-, the poll calls seem to get lost.

This isn't unusual as it's often possible to get a sensor to sample
faster than the kernel can handle.

> 
> As you know, the kernel provides the CONFIG_HARDIRQS_SW_RESEND option
> for software resends of IRQs. This feature masks IRQs in the control
> flow and resends them once the current IRQ handler finishes. I looked
> for a similar implementation in the IIO trigger source code but
> couldn't find anything.

It shouldn't be necessary but things can get interesting with
devices not designed for use with a general purpose OS.

HARDIRQS_SW_RESEND is a workaround for broken irq controllers, not
intended for this (as far as I can tell as I hadn't come across that
before).

> 
> Is there a similar mechanism within IIO, or how do others typically
> address this problem on slower processors? If there isn't, would
> implementing such a feature in IIO make sense? I tackled this by
> simply not using IIO triggers, and only thing I thought to make use
> of was simply blocking poll calls with synchronization primitives
> until the former trigger finishes. I would appreciate your thoughts
> and suggestions.

Roughly speaking combination of using level interrupts for
the hardware irq (so we can come around later if the interrupt
won't refire on next sample) and being careful with the sequencing
in the driver.  Also IRQF_ONESHOT typically for the trigger
to ensure interrupt is only enabled when threads are done
(so if the level is still true, you immediately retrigger).
In some more fiddly cases it may be necessary to disable the interrupt
whilst doing handling but that is very rare and usually
because the device does something bad with the interrupt
line during data read out.

Good to have some more details of how you have tied everything
together. 

For the trigger, is it using a threaded interrupt, or calling
iio_trigger_poll() from the non threaded part?

If it's doing it from the threaded part then all the threads
in the IIO device /pollfunc side will finish before
the trigger interrupt (so the hardware one) is reenabled.
It's been a while since I looked at the flow where each
pollfunc had it's own interrupt context part and thread, but
I didn't think we had races there either.

So I'm assuming you are using the device drdyout as the indicator
of data.  Are you treating that as an edge interrupt? If so
you will run into lots of pain. It's a long time since it was
common to get interrupt controllers that couldn't do level
interrupts.

I suspect you want to treat it as a level interrupt with
IRQF_ONESHOT set to indicate that the interrupt should
not be reenabled until the thread is done. That thread
should only be done after all consumers of the trigger
have completed. 

Jonathan

> 
> Best,
> Ibrahim Bagriyanik.

Re: iio: trigger: Software Resend of Triggers

[Ibrahim Bagriyanik]

On Mon, 2024-08-26 at 12:29 +0100, Jonathan Cameron wrote:
> > Is there a similar mechanism within IIO, or how do others typically
> > address this problem on slower processors? If there isn't, would
> > implementing such a feature in IIO make sense? I tackled this by
> > simply not using IIO triggers, and only thing I thought to make use
> > of was simply blocking poll calls with synchronization primitives
> > until the former trigger finishes. I would appreciate your thoughts
> > and suggestions.
> 
> Roughly speaking combination of using level interrupts for
> the hardware irq (so we can come around later if the interrupt
> won't refire on next sample) and being careful with the sequencing
> in the driver.  Also IRQF_ONESHOT typically for the trigger
> to ensure interrupt is only enabled when threads are done
> (so if the level is still true, you immediately retrigger).
> In some more fiddly cases it may be necessary to disable the
> interrupt
> whilst doing handling but that is very rare and usually
> because the device does something bad with the interrupt
> line during data read out.
> 
> Good to have some more details of how you have tied everything
> together. 
> 
> For the trigger, is it using a threaded interrupt, or calling
> iio_trigger_poll() from the non threaded part?

I used a threaded_irq and polled in the threaded part. For me, to use
iio_trigger_generic_data_rdy_poll did not work somehow, neither in irq
nor in threaded_irq. I use 'iio_trigger_poll_chained/nested' for
polling although I have a single consumer. I do the things just like
MAX11410's driver. Using threaded irq but not using hard part, using
trigger but not using top half too. And polling chained/nested.
Whatever I have tried besides of it, it just did not work. Maybe I
couldn't get it done. 

I am doing it just how MAX11410's driver does it. But still, I do not
think this scenario will work out if HARDIRQS_SW_RESEND is not set in
case of a device using edge interrupt and does not automatically resets
its drdyout:
https://elixir.bootlin.com/linux/v6.10.6/source/drivers/iio/adc/max11410.c

I got back to device and tried level interrupts. Apparently, last time
I tried I messed things up and blamed level interrupts and has not
tried to use them again. Now I see I was mistaken because it works
great now. Thanks for the advice.

> So I'm assuming you are using the device drdyout as the indicator
> of data.  Are you treating that as an edge interrupt? If so
> you will run into lots of pain. It's a long time since it was
> common to get interrupt controllers that couldn't do level
> interrupts.
> I suspect you want to treat it as a level interrupt with
> IRQF_ONESHOT set to indicate that the interrupt should
> not be reenabled until the thread is done. That thread
> should only be done after all consumers of the trigger
> have completed. 

Isn't what some portion of IIO devices in kernel do? For example,
ADXL355, calls 'iio_trigger_poll' in hard interrupt context, and it
still works charmingly. It has edge interrupts in YAML documentation.
Maybe I should ask it's maintainers.

Overall, my problems caused by not lack of features, but my lack of
knowledge. Thanks for taking time to answer Jonathan. Also sorry for
bad formatting of my former email. It was my first plain-text email to
kernel.

Best, Ibrahim.

Re: iio: trigger: Software Resend of Triggers

[Jonathan Cameron]

On Wed, 28 Aug 2024 12:15:32 +0300
Ibrahim Bagriyanik <ibrahim.bagriyanikb@gmail.com> wrote:

> On Mon, 2024-08-26 at 12:29 +0100, Jonathan Cameron wrote:
> > > Is there a similar mechanism within IIO, or how do others typically
> > > address this problem on slower processors? If there isn't, would
> > > implementing such a feature in IIO make sense? I tackled this by
> > > simply not using IIO triggers, and only thing I thought to make use
> > > of was simply blocking poll calls with synchronization primitives
> > > until the former trigger finishes. I would appreciate your thoughts
> > > and suggestions.  
> > 
> > Roughly speaking combination of using level interrupts for
> > the hardware irq (so we can come around later if the interrupt
> > won't refire on next sample) and being careful with the sequencing
> > in the driver.  Also IRQF_ONESHOT typically for the trigger
> > to ensure interrupt is only enabled when threads are done
> > (so if the level is still true, you immediately retrigger).
> > In some more fiddly cases it may be necessary to disable the
> > interrupt
> > whilst doing handling but that is very rare and usually
> > because the device does something bad with the interrupt
> > line during data read out.
> > 
> > Good to have some more details of how you have tied everything
> > together. 
> > 
> > For the trigger, is it using a threaded interrupt, or calling
> > iio_trigger_poll() from the non threaded part?  
> 
> I used a threaded_irq and polled in the threaded part. For me, to use
> iio_trigger_generic_data_rdy_poll did not work somehow, neither in irq
> nor in threaded_irq. I use 'iio_trigger_poll_chained/nested' for
> polling although I have a single consumer. I do the things just like
> MAX11410's driver. Using threaded irq but not using hard part, using
> trigger but not using top half too. And polling chained/nested.
> Whatever I have tried besides of it, it just did not work. Maybe I
> couldn't get it done. 
> 
> I am doing it just how MAX11410's driver does it. But still, I do not
> think this scenario will work out if HARDIRQS_SW_RESEND is not set in
> case of a device using edge interrupt and does not automatically resets
> its drdyout:
> https://elixir.bootlin.com/linux/v6.10.6/source/drivers/iio/adc/max11410.c

That does look problematic and HARDIRQS_SW_RESEND is not a valid fix
as it's a workaround for an interrupt controller issue as I understand it.

> 
> I got back to device and tried level interrupts. Apparently, last time
> I tried I messed things up and blamed level interrupts and has not
> tried to use them again. Now I see I was mistaken because it works
> great now. Thanks for the advice.

No problem.  I remember a lot of pain I had years ago with
an interrupt controller that only supported edge interrupts.
I tried for a long time to get a device that produced level interrupts
(like yours) to work reliably and never managed to reliably close
the race :(  Even had some discussions at the time about whether
we could add something generic to Linux for that case but it is really
tricky to get right so it never happened.

> 
> > So I'm assuming you are using the device drdyout as the indicator
> > of data.  Are you treating that as an edge interrupt? If so
> > you will run into lots of pain. It's a long time since it was
> > common to get interrupt controllers that couldn't do level
> > interrupts.
> > I suspect you want to treat it as a level interrupt with
> > IRQF_ONESHOT set to indicate that the interrupt should
> > not be reenabled until the thread is done. That thread
> > should only be done after all consumers of the trigger
> > have completed.   
> 
> Isn't what some portion of IIO devices in kernel do? For example,
> ADXL355, calls 'iio_trigger_poll' in hard interrupt context, and it
> still works charmingly. It has edge interrupts in YAML documentation.
> Maybe I should ask it's maintainers.

That device has an 'autoclear' function that means that if we miss
an interrupt it will just send one on the next reading.

> 
> Overall, my problems caused by not lack of features, but my lack of
> knowledge. Thanks for taking time to answer Jonathan. Also sorry for
> bad formatting of my former email. It was my first plain-text email to
> kernel.

No problem.

Good luck and I hope you send your code upstream when you are ready!

Jonathan

> 
> Best, Ibrahim.
>