On Sun, Jun 08, 2025 at 07:51:09PM -0300, Daniel Almeida wrote:
> +/// Callbacks for a threaded IRQ handler.
> +pub trait ThreadedHandler: Sync {
> + /// The actual handler function. As usual, sleeps are not allowed in IRQ
> + /// context.
> + fn handle_irq(&self) -> ThreadedIrqReturn;
> +
> + /// The threaded handler function. This function is called from the irq
> + /// handler thread, which is automatically created by the system.
> + fn thread_fn(&self) -> IrqReturn;
> +}
> +
> +impl<T: ?Sized + ThreadedHandler + Send> ThreadedHandler for Arc<T> {
> + fn handle_irq(&self) -> ThreadedIrqReturn {
> + T::handle_irq(self)
> + }
> +
> + fn thread_fn(&self) -> IrqReturn {
> + T::thread_fn(self)
> + }
> +}
In case you intend to be consistent with the function pointer names in
request_threaded_irq(), it'd need to be handler() and thread_fn(). But I don't
think there's a need for that, both aren't really nice for names of trait
methods.
What about irq::Handler::handle() and irq::Handler::handle_threaded() for
instance?
Alternatively, why not just
trait Handler {
fn handle(&self);
}
trait ThreadedHandler {
fn handle(&self);
}
and then we ask for `T: Handler + ThreadedHandler`.
> +
> +impl<T: ?Sized + ThreadedHandler, A: Allocator> ThreadedHandler for Box<T, A> {
> + fn handle_irq(&self) -> ThreadedIrqReturn {
> + T::handle_irq(self)
> + }
> +
> + fn thread_fn(&self) -> IrqReturn {
> + T::thread_fn(self)
> + }
> +}
> +
> +/// A registration of a threaded IRQ handler for a given IRQ line.
> +///
> +/// Two callbacks are required: one to handle the IRQ, and one to handle any
> +/// other work in a separate thread.
> +///
> +/// The thread handler is only called if the IRQ handler returns `WakeThread`.
> +///
> +/// # Examples
> +///
> +/// The following is an example of using `ThreadedRegistration`. It uses a
> +/// [`AtomicU32`](core::sync::AtomicU32) to provide the interior mutability.
> +///
> +/// ```
> +/// use core::sync::atomic::AtomicU32;
> +/// use core::sync::atomic::Ordering;
> +///
> +/// use kernel::prelude::*;
> +/// use kernel::device::Bound;
> +/// use kernel::irq::flags;
> +/// use kernel::irq::ThreadedIrqReturn;
> +/// use kernel::irq::ThreadedRegistration;
> +/// use kernel::irq::IrqReturn;
> +/// use kernel::platform;
> +/// use kernel::sync::Arc;
> +/// use kernel::sync::SpinLock;
> +/// use kernel::alloc::flags::GFP_KERNEL;
> +/// use kernel::c_str;
> +///
> +/// // Declare a struct that will be passed in when the interrupt fires. The u32
> +/// // merely serves as an example of some internal data.
> +/// struct Data(AtomicU32);
> +///
> +/// // [`handle_irq`] takes &self. This example illustrates interior
> +/// // mutability can be used when share the data between process context and IRQ
> +/// // context.
> +///
> +/// type Handler = Data;
> +///
> +/// impl kernel::irq::request::ThreadedHandler for Handler {
> +/// // This is executing in IRQ context in some CPU. Other CPUs can still
> +/// // try to access to data.
> +/// fn handle_irq(&self) -> ThreadedIrqReturn {
> +/// self.0.fetch_add(1, Ordering::Relaxed);
> +///
> +/// // By returning `WakeThread`, we indicate to the system that the
> +/// // thread function should be called. Otherwise, return
> +/// // ThreadedIrqReturn::Handled.
> +/// ThreadedIrqReturn::WakeThread
> +/// }
> +///
> +/// // This will run (in a separate kthread) if and only if `handle_irq`
> +/// // returns `WakeThread`.
> +/// fn thread_fn(&self) -> IrqReturn {
> +/// self.0.fetch_add(1, Ordering::Relaxed);
> +///
> +/// IrqReturn::Handled
> +/// }
> +/// }
> +///
> +/// // This is running in process context.
> +/// fn register_threaded_irq(handler: Handler, dev: &platform::Device<Bound>) -> Result<Arc<ThreadedRegistration<Handler>>> {
> +/// let registration = dev.threaded_irq_by_index(0, flags::SHARED, c_str!("my-device"), handler)?;
This doesn't compile (yet). I think this should be a "raw" example, i.e. the
function should take an IRQ number.
The example you sketch up here is for platform::Device::threaded_irq_by_index().
> +///
> +/// // You can have as many references to the registration as you want, so
> +/// // multiple parts of the driver can access it.
> +/// let registration = Arc::pin_init(registration, GFP_KERNEL)?;
> +///
> +/// // The handler may be called immediately after the function above
> +/// // returns, possibly in a different CPU.
> +///
> +/// {
> +/// // The data can be accessed from the process context too.
> +/// registration.handler().0.fetch_add(1, Ordering::Relaxed);
> +/// }
Why the extra scope?
> +///
> +/// Ok(registration)
> +/// }
> +///
> +///
> +/// # Ok::<(), Error>(())
> +///```
> +///
> +/// # Invariants
> +///
> +/// * We own an irq handler using `&self` as its private data.
> +///
> +#[pin_data]
> +pub struct ThreadedRegistration<T: ThreadedHandler + 'static> {
> + inner: Devres<RegistrationInner>,
> +
> + #[pin]
> + handler: T,
> +
> + /// Pinned because we need address stability so that we can pass a pointer
> + /// to the callback.
> + #[pin]
> + _pin: PhantomPinned,
> +}
Most of the code in this file is a duplicate of the non-threaded registration.
I think this would greatly generalize with specialization and an HandlerInternal
trait.
Without specialization I think we could use enums to generalize.
The most trivial solution would be to define the Handler trait as
trait Handler {
fn handle(&self);
fn handle_threaded(&self) {};
}
but that's pretty dodgy.
> +impl<T: ThreadedHandler + 'static> ThreadedRegistration<T> {
> + /// Registers the IRQ handler with the system for the given IRQ number.
> + pub(crate) fn register<'a>(
> + dev: &'a Device<Bound>,
> + irq: u32,
> + flags: Flags,
> + name: &'static CStr,
> + handler: T,
> + ) -> impl PinInit<Self, Error> + 'a {
What happens if `dev` does not match `irq`? The caller is responsible to only
provide an IRQ number that was obtained from this device.
This should be a safety requirement and a type invariant.
