Message-ID: <abf29ebe-996e-47f6-8548-afc61ad29a89@de.bosch.com>
Date: Tue, 8 Jul 2025 14:15:18 +0200
From: Dirk Behme <dirk.behme@...bosch.com>
To: Daniel Almeida <daniel.almeida@...labora.com>, Miguel Ojeda
	<ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>, Boqun Feng
	<boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
	Björn Roy Baron <bjorn3_gh@...tonmail.com>, Andreas Hindborg
	<a.hindborg@...nel.org>, Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross
	<tmgross@...ch.edu>, Danilo Krummrich <dakr@...nel.org>, Greg Kroah-Hartman
	<gregkh@...uxfoundation.org>, "Rafael J. Wysocki" <rafael@...nel.org>, Thomas
 Gleixner <tglx@...utronix.de>, Benno Lossin <lossin@...nel.org>, Bjorn
 Helgaas <bhelgaas@...gle.com>, Krzysztof Wilczyński
	<kwilczynski@...nel.org>
CC: <linux-kernel@...r.kernel.org>, <rust-for-linux@...r.kernel.org>,
	<linux-pci@...r.kernel.org>
Subject: Re: [PATCH v6 3/6] rust: irq: add support for non-threaded IRQs and
 handlers

Hi Daniel,

On 03/07/2025 21:30, Daniel Almeida wrote:
> This patch adds support for non-threaded IRQs and handlers through
> irq::Registration and the irq::Handler trait.
> 
> Registering an irq is dependent upon having a IrqRequest that was
> previously allocated by a given device. This will be introduced in
> subsequent patches.
> 
> Signed-off-by: Daniel Almeida <daniel.almeida@...labora.com>
...
> diff --git a/rust/kernel/irq/request.rs b/rust/kernel/irq/request.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..4f4beaa3c7887660440b9ddc52414020a0d165ac
> --- /dev/null
> +++ b/rust/kernel/irq/request.rs
> @@ -0,0 +1,273 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// SPDX-FileCopyrightText: Copyright 2025 Collabora ltd.
> +
> +//! This module provides types like [`Registration`] which allow users to
> +//! register handlers for a given IRQ line.
> +
> +use core::marker::PhantomPinned;
> +
> +use crate::alloc::Allocator;
> +use crate::device::Bound;
> +use crate::device::Device;
> +use crate::devres::Devres;
> +use crate::error::to_result;
> +use crate::irq::flags::Flags;
> +use crate::prelude::*;
> +use crate::str::CStr;
> +use crate::sync::Arc;
> +
> +/// The value that can be returned from an IrqHandler or a ThreadedIrqHandler.
> +pub enum IrqReturn {
> +    /// The interrupt was not from this device or was not handled.
> +    None,
> +
> +    /// The interrupt was handled by this device.
> +    Handled,
> +}
> +
> +impl IrqReturn {
> +    fn into_inner(self) -> u32 {
> +        match self {
> +            IrqReturn::None => bindings::irqreturn_IRQ_NONE,
> +            IrqReturn::Handled => bindings::irqreturn_IRQ_HANDLED,
> +        }
> +    }
> +}
> +
> +/// 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;
> +}
> +
> +impl<T: ?Sized + Handler + Send> Handler for Arc<T> {
> +    fn handle(&self) -> IrqReturn {
> +        T::handle(self)
> +    }
> +}
> +
> +impl<T: ?Sized + Handler, A: Allocator> Handler for Box<T, A> {
> +    fn handle(&self) -> IrqReturn {
> +        T::handle(self)
> +    }
> +}
> +
> +/// # Invariants
> +///
> +/// - `self.irq` is the same as the one passed to `request_{threaded}_irq`.
> +/// - `cookie` was passed to `request_{threaded}_irq` as the cookie. It
> +/// is guaranteed to be unique by the type system, since each call to
> +/// `new` will return a different instance of `Registration`.
> +#[pin_data(PinnedDrop)]
> +struct RegistrationInner {
> +    irq: u32,
> +    cookie: *mut kernel::ffi::c_void,
> +}
> +
> +impl RegistrationInner {
> +    fn synchronize(&self) {
> +        // SAFETY: safe as per the invariants of `RegistrationInner`
> +        unsafe { bindings::synchronize_irq(self.irq) };
> +    }
> +}
> +
> +#[pinned_drop]
> +impl PinnedDrop for RegistrationInner {
> +    fn drop(self: Pin<&mut Self>) {
> +        // SAFETY:
> +        //
> +        // Safe as per the invariants of `RegistrationInner` and:
> +        //
> +        // - The containing struct is `!Unpin` and was initialized using
> +        // pin-init, so it occupied the same memory location for the entirety of
> +        // its lifetime.
> +        //
> +        // Notice that this will block until all handlers finish executing,
> +        // i.e.: at no point will &self be invalid while the handler is running.
> +        unsafe { bindings::free_irq(self.irq, self.cookie) };
> +    }
> +}
> +
> +// SAFETY: We only use `inner` on drop, which called at most once with no
> +// concurrent access.
> +unsafe impl Sync for RegistrationInner {}
> +
> +// SAFETY: It is safe to send `RegistrationInner` across threads.
> +unsafe impl Send for RegistrationInner {}
> +
> +/// A request for an IRQ line for a given device.
> +///
> +/// # Invariants
> +///
> +/// - `ìrq` is the number of an interrupt source of `dev`.
> +/// - `irq` has not been registered yet.
> +pub struct IrqRequest<'a> {
> +    dev: &'a Device<Bound>,
> +    irq: u32,
> +}
> +
> +impl<'a> IrqRequest<'a> {
> +    /// Creates a new IRQ request for the given device and IRQ number.
> +    ///
> +    /// # Safety
> +    ///
> +    /// - `irq` should be a valid IRQ number for `dev`.
> +    pub(crate) unsafe fn new(dev: &'a Device<Bound>, irq: u32) -> Self {
> +        IrqRequest { dev, irq }
> +    }
> +}


For example for Resource the elements size, start, name and flags are
accessible. Inspired by that, what do you think about exposing the irq
number here, as well?

diff --git a/rust/kernel/irq/request.rs b/rust/kernel/irq/request.rs
index bd489b8d2386..767d66e3e6c7 100644
--- a/rust/kernel/irq/request.rs
+++ b/rust/kernel/irq/request.rs
@@ -123,6 +123,11 @@ impl<'a> IrqRequest<'a> {
     pub(crate) unsafe fn new(dev: &'a Device<Bound>, irq: u32) -> Self {
         IrqRequest { dev, irq }
     }
+
+    /// Returns the IRQ number of an [`IrqRequest`].
+    pub fn irq(&self) -> u32 {
+        self.irq
+    }
 }


I'm using that for some dev_info!().

Best regards

Dirk

Powered by blists - more mailing lists