Re: [PATCH v6 3/6] rust: irq: add support for non-threaded IRQs and handlers

Danilo Krummrich
2025-07-13T10:24:34Z

On Sun Jul 13, 2025 at 1:32 AM CEST, Daniel Almeida wrote:
>
>
>> On 12 Jul 2025, at 18:24, Danilo Krummrich <dakr@kernel.org> wrote:
>>
>> On Thu Jul 3, 2025 at 9:30 PM CEST, Daniel Almeida wrote:
>>> +/// Callbacks for an IRQ handler.
>>> +pub trait Handler: Sync {
>>> + /// The hard IRQ handler.
>>> + ///
>>> + /// This is executed in interrupt context, hence all corresponding
>>> + /// limitations do apply.
>>> + ///
>>> + /// All work that does not necessarily need to be executed from
>>> + /// interrupt context, should be deferred to a threaded handler.
>>> + /// See also [`ThreadedRegistration`].
>>> + fn handle(&self) -> IrqReturn;
>>> +}
>>
>> One thing I forgot, the IRQ handlers should have a &Device<Bound> argument,
>> i.e.:
>>
>> fn handle(&self, dev: &Device<Bound>) -> IrqReturn
>>
>> IRQ registrations naturally give us this guarantee, so we should take advantage
>> of that.
>>
>> - Danilo
>
> Hi Danilo,
>
> I do not immediately see a way to get a Device<Bound> from here:
>
> unsafe extern "C" fn handle_irq_callback<T: Handler>(_irq: i32, ptr: *mut c_void) -> c_uint {
>
> Refall that we've established `ptr` to be the address of the handler. This
> came after some back and forth and after the extensive discussion that Benno
> and Boqun had w.r.t to pinning in request_irq().

You can just wrap the Handler in a new type and store the pointer there:

#[pin_data]
struct Wrapper {
#[pin]
handler: T,
dev: NonNull<Device<Bound>>,
}

And then pass a pointer to the Wrapper field to request_irq();
handle_irq_callback() can construct a &T and a &Device<Bound> from this.

Note that storing a device pointer, without its own reference count, is
perfectly fine, since inner (Devres<RegistrationInner>) already holds a
reference to the device and guarantees the bound scope for the handler
callbacks.

It makes sense to document this as an invariant of Wrapper (or whatever we end
up calling it).