Re: [PATCH v4 4/6] rust: irq: add support for threaded IRQs and handlers

[Daniel Almeida]

Hi,

>> 
>>> +
>>> +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().
> 
> Yes, I originally had an example along the lines of what you mentioned. Except
> that with the changes in register() from pub to pub(crate) they stopped
> compiling.
> 
> I am not sure how the doctest to kunit machinery works, but I was expecting
> tests to have access to everything within the module they're defined in, but
> this is apparently not the case.

Does anybody have any input on this? Again, I tried it already before sending
the current version but it does not compile due to pub(crate).

— Daniel