Re: [PATCH 3/4] media: dw100: Fix kernel oops with PREEMPT_RT enabled

[Stefan Klug <stefan.klug@ideasonboard.com>]

Hi Sebastian,

Thanks for your support.

Quoting Sebastian Andrzej Siewior (2026-01-12 12:43:13)
> On 2026-01-06 18:11:27 [+0100], Stefan Klug wrote:
> > Hah, if I knew that :-).
> > 
> > The pieces I have are:
> > In the DT the interrupt line is marked as IRQ_TYPE_LEVEL_HIGH. I don't
> > know why and couldn't find a reference to that in the reference manual.
> 
> It is either a LEVEL interrupt or just marked as such. But it seems to
> behave as such.
> 
> > Assuming it is a level interrupt, then it makes sense to treat it as ONESHOT,
> > otherwise it would fire again immediately after handling the hard
> > interrupt...but it was a hard interrupt in first place - huh.
> 
> So setting it ONESHOT while it is non-threaded does not seem to make
> sense, correct.
> 
> > I just realize that I still miss a bit of the puzzle:
> > ONESHOT is doumented as:
> > 
> > "Interrupt is not reenabled after the hardirq handler finished. Used by
> > threaded interrupts which need to keep the irq line disabled until the
> > threaded handler has been run."
> > 
> > That makes perfect sense. So ONESHOT disables the irq line until the
> > thread_fn has completed (if it was set). Now on preempt_rt inside
> > irq_setup_forced_threading() we don't force threading if ONESHOT is
> > requested. Why is that?
> 
> Because ONESHOT is usually used where there is no primary handler/ the
> primary handler does just a wake of thread.
> 
> > So I'm left with two questions:
> > - Why aren't ONESHOT irq handlers forced to threaded on preempt_rt?
> 
> See above. Also PREEMPT_RT just enforces the kernel command line
> threadirqs
> 
> > - Why was ONESHOT requested in first place as to my current knowledge it
> >   really only makes sense if a thread_fn is defined.
> 
> I would say it was a mistake and nobody noticed it. There is no visible
> difference if there is just the primary handler and the system does not
> use threadirqs (or PREEMPT_RT which enforces it).
> 
> > Did I just answer my own question? ONESHOT only makes sense if there is
> > a thread_fn and it is assumed that the hard handler is necessary. So
> > preempt_rt doesn't try to change that?
> 
> Yes. ONESHOT is used if the interrupt source within the IRQ chip has to
> be masked until after the thread completed. So setting ONESHOT without a
> threaded handler is dubious.
> 
> > That would mean the ONESHOT in the dw100 was not necessary in first
> > place but didn't do any harm until preempt_rt was enabled... And if
> > ONSHOT is *not* set preempt_rt would automatically force the irq handler
> > to be threaded and set the ONESHOT flag in irq_setup_forced_threading().
> 
> correct.
> 
> > So everything would be fine except that we'd still hit the timeout issue
> > from patch 4/4.
> > 
> > So if I got that right, the dw100 driver is in the unfortunate
> > situation, that the irq handler consists of two parts where the first
> > part *must* run in hard interrupt context and the second part *should* run
> > in hard interrupt context but it is fine if it becomes threaded due to
> > preempt_rt. As we can't model that, the best we can do is to always run
> > the second part threaded...
> 
> So happens if you avoid the IRQF_ONESHOT? Do you still get these
> timeout errors?

I did a bit more testing and got results that I fail to completely
understand.

If I enable IRQF_ONESHOT and use the threaded_fn, on a non PREEMPT_RT
system I regularly observe the timeout message.

If I pass irqflags=0 and use the hard handler on a PREEMPT_RT system I
expected the same behavior (as the hard handler gets changed to be
threaded and implicitely ONESHOT is set). But I don't see the timeout
messages.

Is there anything else that I need to do on PREEMPT_RT to force the
threaded behavior besides enabling the config? Or is the irq thread
running with higher priority and therefore possibly faster?

Running irqflags=0 and the hard handler on a non PREEMPT_RT system
didn't have any negative side effects. So maybe that is really the
solution...

I'll ping Xavier if he has more details on the hardware.

Best regards,
Stefan

> 
> > So patch 4/4 seems correct until we get new information about the
> > hardware.
> > 
> > Any thoughts?
> > 
> > Best regards,
> > Stefan
> 
> Sebastian
>