[PATCH v7 0/6] rust: add support for request_irq

[PATCH v7 0/6] rust: add support for request_irq

[Daniel Almeida]

Changes in v7:
- Rebased on top of driver-core-next
- Added Flags::new(), which is a const fn. This lets us use build_assert!()
  to verify the casts (hopefully this is what you meant, Alice?)
- Changed the Flags inner type to take c_ulong directly, to minimize casts
  (Thanks, Alice)
- Moved the flag constants into Impl Flags, instead of using a separate
  module (Alice)
- Reverted to using #[repr(u32)] in Threaded/IrqReturn (Thanks Alice,
  Benno)
- Fixed all instances where the full path was specified for types in the
  prelude (Alice)
- Removed 'static from the CStr used to perform the lookup in the platform
  accessor (Alice)
- Renamed the PCI accessors, as asked by Danilo
- Added more docs to Flags, going into more detail on what they do and how
  to use them (Miguel)
- Fixed the indentation in some of the docs (Alice)
- Added Alice's r-b as appropriate
- Link to v6: https://lore.kernel.org/rust-for-linux/20250703-topics-tyr-request_irq-v6-0-74103bdc7c52@collabora.com/

Changes in v6:
- Fixed some typos in the docs (thanks, Dirk!)
- Reordered the arguments for the accessors in platform.rs (Danilo)
- Renamed handle_on_thread() to handle_threaded() (Danilo)
- Changed the documentation for Handler and ThreadedHandler to what
  Danilo suggested
- Link to v5: https://lore.kernel.org/r/20250627-topics-tyr-request_irq-v5-0-0545ee4dadf6@collabora.com

Changes in v5:

Thanks, Danilo {
  - Removed extra scope in the examples.
  - Renamed Registration::register() to Registration::new(),
  - Switched to try_pin_init! in Registration::new() (thanks for the
    code and the help, Boqun and Benno)
  - Renamed the trait functions to handle() and handle_on_thread().
  - Introduced IrqRequest with an unsafe pub(crate) constructor
  - Made both register() and the accessors that return IrqRequest public
    the idea is to allow both of these to work:
	// `irq` is an `irq::Registration`
	let irq = pdev.threaded_irq_by_name()?
  and
	// `req` is an `IrqRequest`.
	let req = pdev.irq_by_name()?;
	// `irq` is an `irq::Registration`
	let irq = irq::ThreadedRegistration::new(req)?;

  - Added another name in the byname variants. There's now one for the
    request part and the other one to register()
  - Reworked the examples in request.rs
  - Implemented the irq accessors in place for pci.rs
  - Split the platform accessor macros into two
}

- Added a rust helper for pci_irq_vectors if !CONFIG_PCI_MSI (thanks,
Intel 0day bot)
- Link to v4: https://lore.kernel.org/r/20250608-topics-tyr-request_irq-v4-0-81cb81fb8073@collabora.com

Changes in v4:

Thanks, Benno {
  - Split series into more patches (see patches 1-4)
  - Use cast() where possible
  - Merge pub use statements.
  - Add {Threaded}IrqReturn::into_inner() instead of #[repr(u32)]
  - Used AtomicU32 instead of SpinLock to add interior mutability to the
    handler's data. SpinLockIrq did not land yet.
  - Mention that `&self` is !Unpin and was initialized using pin_init in
    drop()
  - Fix the docs slightly
}

- Add {try_}synchronize_irq().
- Use Devres for the irq registration (see RegistrationInner). This idea
  was suggested by Danilo and Alice.
- Added PCI accessors (as asked by Joel Fernandez)
- Fix a major oversight: we were passing in a pointer to Registration
  in register_{threaded}_irq() but casting it to Handler/ThreadedHandler in
  the callbacks.
- Make register() pub(crate) so drivers can only retrieve registrations
  through device-specific accessors. This forbids drivers from trying to
  register an invalid irq.
- I think this will still go through a few rounds, so I'll defer the
  patch to update MAINTAINERS for now.

- Link to v3: https://lore.kernel.org/r/20250514-topics-tyr-request_irq-v3-0-d6fcc2591a88@collabora.com

Changes in v3:
- Rebased on driver-core-next
- Added patch to get the irq numbers from a platform device (thanks,
  Christian!)
- Split flags into its own file.
- Change iff to "if and only if"
- Implement PartialEq and Eq for Flags
- Fix some broken docs/markdown
- Reexport most things so users can elide ::request from the path
- Add a blanket implementation of ThreadedHandler and Handler for
  Arc/Box<T: Handler> that just forwards the call to the T. This lets us
  have Arc<Foo> and Box<Foo> as handlers if Foo: Handler.
- Rework the examples a bit.
- Remove "as _" casts in favor of "as u64" for flags. This is needed to
  cast the individual flags into u64.
- Use #[repr(u32)] for ThreadedIrqReturn and IrqReturn.
- Wrapped commit messages to < 75 characters

- Link to v2: https://lore.kernel.org/r/20250122163932.46697-1-daniel.almeida@collabora.com

Changes in v2:
- Added Co-developed-by tag to account for the work that Alice did in order to
figure out how to do this without Opaque<T> (Thanks!)
- Removed Opaque<T> in favor of plain T
- Fixed the examples
- Made sure that the invariants sections are the last entry in the docs
- Switched to slot.cast() where applicable,
- Mentioned in the safety comments that we require that T: Sync,
- Removed ThreadedFnReturn in favor of IrqReturn,
- Improved the commit message

Link to v1: https://lore.kernel.org/rust-for-linux/20241024-topic-panthor-rs-request_irq-v1-1-7cbc51c182ca@collabora.com/

---
Daniel Almeida (6):
      rust: irq: add irq module
      rust: irq: add flags module
      rust: irq: add support for non-threaded IRQs and handlers
      rust: irq: add support for threaded IRQs and handlers
      rust: platform: add irq accessors
      rust: pci: add irq accessors

 rust/bindings/bindings_helper.h |   1 +
 rust/helpers/helpers.c          |   1 +
 rust/helpers/irq.c              |   9 +
 rust/helpers/pci.c              |   8 +
 rust/kernel/irq.rs              |  22 ++
 rust/kernel/irq/flags.rs        | 124 ++++++++++
 rust/kernel/irq/request.rs      | 490 ++++++++++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs              |   1 +
 rust/kernel/pci.rs              |  45 +++-
 rust/kernel/platform.rs         | 146 +++++++++++-
 10 files changed, 844 insertions(+), 3 deletions(-)
---
base-commit: 3964d07dd821efe9680e90c51c86661a98e60a0f
change-id: 20250712-topics-tyr-request_irq2-ae7ee9b85854

Best regards,
-- 
Daniel Almeida <daniel.almeida@collabora.com>

[PATCH v7 1/6] rust: irq: add irq module

[Daniel Almeida]

Add the IRQ module. Future patches will then introduce support for IRQ
registrations and handlers.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
---
 rust/kernel/irq.rs | 11 +++++++++++
 rust/kernel/lib.rs |  1 +
 2 files changed, 12 insertions(+)

diff --git a/rust/kernel/irq.rs b/rust/kernel/irq.rs
new file mode 100644
index 0000000000000000000000000000000000000000..fae7b15effc80c936d6bffbd5b4150000d6c2898
--- /dev/null
+++ b/rust/kernel/irq.rs
@@ -0,0 +1,11 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! IRQ abstractions.
+//!
+//! An IRQ is an interrupt request from a device. It is used to get the CPU's
+//! attention so it can service a hardware event in a timely manner.
+//!
+//! The current abstractions handle IRQ requests and handlers, i.e.: it allows
+//! drivers to register a handler for a given IRQ line.
+//!
+//! C header: [`include/linux/device.h`](srctree/include/linux/interrupt.h)
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 5bbf3627212f0a26d34be0d6c160a370abf1e996..cdc31a89064e2144f1937a1588c460aea5f0ddf8 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -82,6 +82,7 @@
 pub mod init;
 pub mod io;
 pub mod ioctl;
+pub mod irq;
 pub mod jump_label;
 #[cfg(CONFIG_KUNIT)]
 pub mod kunit;

-- 
2.50.0

[PATCH v7 2/6] rust: irq: add flags module

[Daniel Almeida]

Manipulating IRQ flags (i.e.: IRQF_*) will soon be necessary, specially to
register IRQ handlers through bindings::request_irq().

Add a kernel::irq::Flags for that purpose.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
---
 rust/kernel/irq.rs       |   3 ++
 rust/kernel/irq/flags.rs | 124 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 127 insertions(+)

diff --git a/rust/kernel/irq.rs b/rust/kernel/irq.rs
index fae7b15effc80c936d6bffbd5b4150000d6c2898..9abd9a6dc36f3e3ecc1f92ad7b0040176b56a079 100644
--- a/rust/kernel/irq.rs
+++ b/rust/kernel/irq.rs
@@ -9,3 +9,6 @@
 //! drivers to register a handler for a given IRQ line.
 //!
 //! C header: [`include/linux/device.h`](srctree/include/linux/interrupt.h)
+
+/// Flags to be used when registering IRQ handlers.
+pub mod flags;
diff --git a/rust/kernel/irq/flags.rs b/rust/kernel/irq/flags.rs
new file mode 100644
index 0000000000000000000000000000000000000000..a4882851d21f3e841862875ad7286ea6a2dfd2dd
--- /dev/null
+++ b/rust/kernel/irq/flags.rs
@@ -0,0 +1,124 @@
+// SPDX-License-Identifier: GPL-2.0
+// SPDX-FileCopyrightText: Copyright 2025 Collabora ltd.
+
+use crate::bindings;
+use crate::prelude::*;
+
+/// Flags to be used when registering IRQ handlers.
+///
+/// Flags can be used to request specific behaviors when registering an IRQ
+/// handler, and can be combined using the `|`, `&`, and `!` operators to
+/// further control the system's behavior.
+///
+/// A common use case is to register a shared interrupt, as sharing the line
+/// between devices is increasingly common in modern systems and is even
+/// required for some buses. This requires setting [`Flags::SHARED`] when
+/// requesting the interrupt. Other use cases include setting the trigger type
+/// through `Flags::TRIGGER_*`, which determines when the interrupt fires, or
+/// controlling whether the interrupt is masked after the handler runs by using
+/// [`Flags::ONESHOT`].
+///
+/// If an invalid combination of flags is provided, the system will refuse to
+/// register the handler, and lower layers will enforce certain flags when
+/// necessary. This means, for example, that all the
+/// [`crate::irq::Registration`] for a shared interrupt have to agree on
+/// [`Flags::SHARED`] and on the same trigger type, if set.
+#[derive(Clone, Copy, PartialEq, Eq)]
+pub struct Flags(c_ulong);
+
+impl Flags {
+    /// Use the interrupt line as already configured.
+    pub const TRIGGER_NONE: Flags = Flags::new(bindings::IRQF_TRIGGER_NONE);
+
+    /// The interrupt is triggered when the signal goes from low to high.
+    pub const TRIGGER_RISING: Flags = Flags::new(bindings::IRQF_TRIGGER_RISING);
+
+    /// The interrupt is triggered when the signal goes from high to low.
+    pub const TRIGGER_FALLING: Flags = Flags::new(bindings::IRQF_TRIGGER_FALLING);
+
+    /// The interrupt is triggered while the signal is held high.
+    pub const TRIGGER_HIGH: Flags = Flags::new(bindings::IRQF_TRIGGER_HIGH);
+
+    /// The interrupt is triggered while the signal is held low.
+    pub const TRIGGER_LOW: Flags = Flags::new(bindings::IRQF_TRIGGER_LOW);
+
+    /// Allow sharing the IRQ among several devices.
+    pub const SHARED: Flags = Flags::new(bindings::IRQF_SHARED);
+
+    /// Set by callers when they expect sharing mismatches to occur.
+    pub const PROBE_SHARED: Flags = Flags::new(bindings::IRQF_PROBE_SHARED);
+
+    /// Flag to mark this interrupt as timer interrupt.
+    pub const TIMER: Flags = Flags::new(bindings::IRQF_TIMER);
+
+    /// Interrupt is per CPU.
+    pub const PERCPU: Flags = Flags::new(bindings::IRQF_PERCPU);
+
+    /// Flag to exclude this interrupt from irq balancing.
+    pub const NOBALANCING: Flags = Flags::new(bindings::IRQF_NOBALANCING);
+
+    /// Interrupt is used for polling (only the interrupt that is registered
+    /// first in a shared interrupt is considered for performance reasons).
+    pub const IRQPOLL: Flags = Flags::new(bindings::IRQF_IRQPOLL);
+
+    /// 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.
+    pub const ONESHOT: Flags = Flags::new(bindings::IRQF_ONESHOT);
+
+    /// Do not disable this IRQ during suspend. Does not guarantee that this
+    /// interrupt will wake the system from a suspended state.
+    pub const NO_SUSPEND: Flags = Flags::new(bindings::IRQF_NO_SUSPEND);
+
+    /// Force enable it on resume even if [`NO_SUSPEND`] is set.
+    pub const FORCE_RESUME: Flags = Flags::new(bindings::IRQF_FORCE_RESUME);
+
+    /// Interrupt cannot be threaded.
+    pub const NO_THREAD: Flags = Flags::new(bindings::IRQF_NO_THREAD);
+
+    /// Resume IRQ early during syscore instead of at device resume time.
+    pub const EARLY_RESUME: Flags = Flags::new(bindings::IRQF_EARLY_RESUME);
+
+    /// If the IRQ is shared with a [`NO_SUSPEND`] user, execute this interrupt
+    /// handler after suspending interrupts. For system wakeup devices users
+    /// need to implement wakeup detection in their interrupt handlers.
+    pub const COND_SUSPEND: Flags = Flags::new(bindings::IRQF_COND_SUSPEND);
+
+    /// Don't enable IRQ or NMI automatically when users request it. Users will
+    /// enable it explicitly by `enable_irq` or `enable_nmi` later.
+    pub const NO_AUTOEN: Flags = Flags::new(bindings::IRQF_NO_AUTOEN);
+
+    /// Exclude from runnaway detection for IPI and similar handlers, depends on
+    /// `PERCPU`.
+    pub const NO_DEBUG: Flags = Flags::new(bindings::IRQF_NO_DEBUG);
+
+    pub(crate) fn into_inner(self) -> c_ulong {
+        self.0
+    }
+
+    const fn new(value: u32) -> Self {
+        build_assert!(value as u64 <= c_ulong::MAX as u64);
+        Self(value as c_ulong)
+    }
+}
+
+impl core::ops::BitOr for Flags {
+    type Output = Self;
+    fn bitor(self, rhs: Self) -> Self::Output {
+        Self(self.0 | rhs.0)
+    }
+}
+
+impl core::ops::BitAnd for Flags {
+    type Output = Self;
+    fn bitand(self, rhs: Self) -> Self::Output {
+        Self(self.0 & rhs.0)
+    }
+}
+
+impl core::ops::Not for Flags {
+    type Output = Self;
+    fn not(self) -> Self::Output {
+        Self(!self.0)
+    }
+}

-- 
2.50.0

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

[Daniel Almeida]

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@collabora.com>
---
 rust/bindings/bindings_helper.h |   1 +
 rust/helpers/helpers.c          |   1 +
 rust/helpers/irq.c              |   9 ++
 rust/kernel/irq.rs              |   5 +
 rust/kernel/irq/request.rs      | 267 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 283 insertions(+)

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 7e8f2285064797d5bbac5583990ff823b76c6bdc..fc73b89ff9d539e536a5da9388e4926a91a6130e 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -52,6 +52,7 @@
 #include <linux/ethtool.h>
 #include <linux/file.h>
 #include <linux/firmware.h>
+#include <linux/interrupt.h>
 #include <linux/fs.h>
 #include <linux/jiffies.h>
 #include <linux/jump_label.h>
diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index 0b09bd0e3561c7bf80bf79faf1aebd7eeb851984..653c3f7b85c5f7192b1584c748a9d7e4af3796e9 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -22,6 +22,7 @@
 #include "dma.c"
 #include "drm.c"
 #include "err.c"
+#include "irq.c"
 #include "fs.c"
 #include "io.c"
 #include "jump_label.c"
diff --git a/rust/helpers/irq.c b/rust/helpers/irq.c
new file mode 100644
index 0000000000000000000000000000000000000000..1faca428e2c047a656dec3171855c1508d67e60b
--- /dev/null
+++ b/rust/helpers/irq.c
@@ -0,0 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/interrupt.h>
+
+int rust_helper_request_irq(unsigned int irq, irq_handler_t handler,
+			    unsigned long flags, const char *name, void *dev)
+{
+	return request_irq(irq, handler, flags, name, dev);
+}
diff --git a/rust/kernel/irq.rs b/rust/kernel/irq.rs
index 9abd9a6dc36f3e3ecc1f92ad7b0040176b56a079..01bd08884b72c2a3a9460897bce751c732a19794 100644
--- a/rust/kernel/irq.rs
+++ b/rust/kernel/irq.rs
@@ -12,3 +12,8 @@
 
 /// Flags to be used when registering IRQ handlers.
 pub mod flags;
+
+/// IRQ allocation and handling.
+pub mod request;
+
+pub use request::{Handler, IrqRequest, IrqReturn, Registration};
diff --git a/rust/kernel/irq/request.rs b/rust/kernel/irq/request.rs
new file mode 100644
index 0000000000000000000000000000000000000000..2f4637d8bc4c9fda23cbc8307687035957b0042a
--- /dev/null
+++ b/rust/kernel/irq/request.rs
@@ -0,0 +1,267 @@
+// 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.
+#[repr(u32)]
+pub enum IrqReturn {
+    /// The interrupt was not from this device or was not handled.
+    None = bindings::irqreturn_IRQ_NONE,
+
+    /// The interrupt was handled by this device.
+    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 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 }
+    }
+
+    /// Returns the IRQ number of an [`IrqRequest`].
+    pub fn irq(&self) -> u32 {
+        self.irq
+    }
+}
+
+/// A registration of an IRQ handler for a given IRQ line.
+///
+/// # Examples
+///
+/// The following is an example of using `Registration`. 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::Registration;
+/// use kernel::irq::IrqRequest;
+/// use kernel::irq::IrqReturn;
+/// use kernel::sync::Arc;
+/// use kernel::c_str;
+/// use kernel::alloc::flags::GFP_KERNEL;
+///
+/// // 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);
+///
+/// // [`kernel::irq::request::Handler::handle`] takes `&self`. This example
+/// // illustrates how interior mutability can be used when sharing the data
+/// // between process context and IRQ context.
+///
+/// type Handler = Data;
+///
+/// impl kernel::irq::request::Handler for Handler {
+///     // This is executing in IRQ context in some CPU. Other CPUs can still
+///     // try to access to data.
+///     fn handle(&self) -> IrqReturn {
+///         self.0.fetch_add(1, Ordering::Relaxed);
+///
+///         IrqReturn::Handled
+///     }
+/// }
+///
+/// // Registers an IRQ handler for the given IrqRequest.
+/// //
+/// // This is executing in process context and assumes that `request` was
+/// // previously acquired from a device.
+/// fn register_irq(handler: Handler, request: IrqRequest<'_>) -> Result<Arc<Registration<Handler>>> {
+///     let registration = Registration::new(request, flags::SHARED, c_str!("my_device"), handler);
+///
+///     let registration = Arc::pin_init(registration, GFP_KERNEL)?;
+///
+///     // The data can be accessed from process context too.
+///     registration.handler().0.fetch_add(1, Ordering::Relaxed);
+///
+///     Ok(registration)
+/// }
+/// # Ok::<(), Error>(())
+/// ```
+///
+/// # Invariants
+///
+/// * We own an irq handler using `&self.handler` as its private data.
+///
+#[pin_data]
+pub struct Registration<T: Handler + 'static> {
+    #[pin]
+    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,
+}
+
+impl<T: Handler + 'static> Registration<T> {
+    /// Registers the IRQ handler with the system for the given IRQ number.
+    pub fn new<'a>(
+        request: IrqRequest<'a>,
+        flags: Flags,
+        name: &'static CStr,
+        handler: T,
+    ) -> impl PinInit<Self, Error> + 'a {
+        try_pin_init!(&this in Self {
+            handler,
+            inner <- Devres::new(
+                request.dev,
+                try_pin_init!(RegistrationInner {
+                    // SAFETY: `this` is a valid pointer to the `Registration` instance
+                    cookie: unsafe { &raw mut (*this.as_ptr()).handler }.cast(),
+                    irq: {
+                        // SAFETY:
+                        // - The callbacks are valid for use with request_irq.
+                        // - If this succeeds, the slot is guaranteed to be valid until the
+                        //   destructor of Self runs, which will deregister the callbacks
+                        //   before the memory location becomes invalid.
+                        to_result(unsafe {
+                            bindings::request_irq(
+                                request.irq,
+                                Some(handle_irq_callback::<T>),
+                                flags.into_inner(),
+                                name.as_char_ptr(),
+                                (&raw mut (*this.as_ptr()).handler).cast(),
+                            )
+                        })?;
+                        request.irq
+                    }
+                })
+            ),
+            _pin: PhantomPinned,
+        })
+    }
+
+    /// Returns a reference to the handler that was registered with the system.
+    pub fn handler(&self) -> &T {
+        &self.handler
+    }
+
+    /// Wait for pending IRQ handlers on other CPUs.
+    ///
+    /// This will attempt to access the inner [`Devres`] container.
+    pub fn try_synchronize(&self) -> Result {
+        let inner = self.inner.try_access().ok_or(ENODEV)?;
+        inner.synchronize();
+        Ok(())
+    }
+
+    /// Wait for pending IRQ handlers on other CPUs.
+    pub fn synchronize(&self, dev: &Device<Bound>) -> Result {
+        let inner = self.inner.access(dev)?;
+        inner.synchronize();
+        Ok(())
+    }
+}
+
+/// # Safety
+///
+/// This function should be only used as the callback in `request_irq`.
+unsafe extern "C" fn handle_irq_callback<T: Handler>(_irq: i32, ptr: *mut c_void) -> c_uint {
+    // SAFETY: `ptr` is a pointer to T set in `Registration::new`
+    let handler = unsafe { &*(ptr as *const T) };
+    T::handle(handler) as c_uint
+}

-- 
2.50.0

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

[Daniel Almeida]

This patch adds support for threaded IRQs and handlers through
irq::ThreadedRegistration and the irq::ThreadedHandler trait.

Threaded interrupts are more permissive in the sense that further
processing is possible in a kthread. This means that said execution takes
place outside of interrupt context, which is rather restrictive in many
ways.

Registering a threaded irq is dependent upon having an IrqRequest that
was previously allocated by a given device. This will be introduced in
subsequent patches.

Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
---
 rust/kernel/irq.rs         |   5 +-
 rust/kernel/irq/request.rs | 227 ++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 229 insertions(+), 3 deletions(-)

diff --git a/rust/kernel/irq.rs b/rust/kernel/irq.rs
index 01bd08884b72c2a3a9460897bce751c732a19794..aaa40001bafca617c588c799bb41144921595cae 100644
--- a/rust/kernel/irq.rs
+++ b/rust/kernel/irq.rs
@@ -16,4 +16,7 @@
 /// IRQ allocation and handling.
 pub mod request;
 
-pub use request::{Handler, IrqRequest, IrqReturn, Registration};
+pub use request::{
+    Handler, IrqRequest, IrqReturn, Registration, ThreadedHandler, ThreadedIrqReturn,
+    ThreadedRegistration,
+};
diff --git a/rust/kernel/irq/request.rs b/rust/kernel/irq/request.rs
index 2f4637d8bc4c9fda23cbc8307687035957b0042a..aa1b957fa18f927df2f14b18076393e1be2cf1d6 100644
--- a/rust/kernel/irq/request.rs
+++ b/rust/kernel/irq/request.rs
@@ -1,8 +1,9 @@
 // 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.
+//! This module provides types like [`Registration`] and
+//! [`ThreadedRegistration`], which allow users to register handlers for a given
+//! IRQ line.
 
 use core::marker::PhantomPinned;
 
@@ -265,3 +266,225 @@ pub fn synchronize(&self, dev: &Device<Bound>) -> Result {
     let handler = unsafe { &*(ptr as *const T) };
     T::handle(handler) as c_uint
 }
+
+/// The value that can be returned from `ThreadedHandler::handle_irq`.
+#[repr(u32)]
+pub enum ThreadedIrqReturn {
+    /// The interrupt was not from this device or was not handled.
+    None = bindings::irqreturn_IRQ_NONE,
+
+    /// The interrupt was handled by this device.
+    Handled = bindings::irqreturn_IRQ_HANDLED,
+
+    /// The handler wants the handler thread to wake up.
+    WakeThread = bindings::irqreturn_IRQ_WAKE_THREAD,
+}
+
+/// Callbacks for a threaded IRQ handler.
+pub trait ThreadedHandler: 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 the threaded
+    /// handler, i.e. [`ThreadedHandler::handle_threaded`].
+    fn handle(&self) -> ThreadedIrqReturn;
+
+    /// The threaded IRQ handler.
+    ///
+    /// This is executed in process context. The kernel creates a dedicated
+    /// kthread for this purpose.
+    fn handle_threaded(&self) -> IrqReturn;
+}
+
+impl<T: ?Sized + ThreadedHandler + Send> ThreadedHandler for Arc<T> {
+    fn handle(&self) -> ThreadedIrqReturn {
+        T::handle(self)
+    }
+
+    fn handle_threaded(&self) -> IrqReturn {
+        T::handle_threaded(self)
+    }
+}
+
+impl<T: ?Sized + ThreadedHandler, A: Allocator> ThreadedHandler for Box<T, A> {
+    fn handle(&self) -> ThreadedIrqReturn {
+        T::handle(self)
+    }
+
+    fn handle_threaded(&self) -> IrqReturn {
+        T::handle_threaded(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::IrqRequest;
+/// use kernel::irq::IrqReturn;
+/// use kernel::sync::Arc;
+/// use kernel::c_str;
+/// use kernel::alloc::flags::GFP_KERNEL;
+///
+/// // 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);
+///
+/// // [`kernel::irq::request::ThreadedHandler::handle`] takes `&self`. This example
+/// // illustrates how interior mutability can be used when sharing 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 the data.
+///     fn handle(&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`
+///     // returns `WakeThread`.
+///     fn handle_threaded(&self) -> IrqReturn {
+///         self.0.fetch_add(1, Ordering::Relaxed);
+///         IrqReturn::Handled
+///     }
+/// }
+///
+/// // Registers a threaded IRQ handler for the given IrqRequest.
+/// //
+/// // This is executing in process context and assumes that `request` was
+/// // previously acquired from a device.
+/// fn register_threaded_irq(handler: Handler, request: IrqRequest<'_>) -> Result<Arc<ThreadedRegistration<Handler>>> {
+///     let registration = ThreadedRegistration::new(request, flags::SHARED, c_str!("my_device"), handler);
+///
+///     let registration = Arc::pin_init(registration, GFP_KERNEL)?;
+///
+///     // The data can be accessed from process context too.
+///     registration.handler().0.fetch_add(1, Ordering::Relaxed);
+///
+///     Ok(registration)
+/// }
+/// # Ok::<(), Error>(())
+/// ```
+///
+/// # Invariants
+///
+/// * We own an irq handler using `&T` as its private data.
+///
+#[pin_data]
+pub struct ThreadedRegistration<T: ThreadedHandler + 'static> {
+    #[pin]
+    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,
+}
+
+impl<T: ThreadedHandler + 'static> ThreadedRegistration<T> {
+    /// Registers the IRQ handler with the system for the given IRQ number.
+    pub fn new<'a>(
+        request: IrqRequest<'a>,
+        flags: Flags,
+        name: &'static CStr,
+        handler: T,
+    ) -> impl PinInit<Self, Error> + 'a {
+        try_pin_init!(&this in Self {
+            handler,
+            inner <- Devres::new(
+                request.dev,
+                try_pin_init!(RegistrationInner {
+                    // SAFETY: `this` is a valid pointer to the `ThreadedRegistration` instance.
+                    cookie: unsafe { &raw mut (*this.as_ptr()).handler }.cast(),
+                    irq: {
+                        // SAFETY:
+                        // - The callbacks are valid for use with request_threaded_irq.
+                        // - If this succeeds, the slot is guaranteed to be valid until the
+                        // destructor of Self runs, which will deregister the callbacks
+                        // before the memory location becomes invalid.
+                        to_result(unsafe {
+                            bindings::request_threaded_irq(
+                                request.irq,
+                                Some(handle_threaded_irq_callback::<T>),
+                                Some(thread_fn_callback::<T>),
+                                flags.into_inner() as usize,
+                                name.as_char_ptr(),
+                                (&raw mut (*this.as_ptr()).handler).cast(),
+                            )
+                        })?;
+                        request.irq
+                    }
+                })
+            ),
+            _pin: PhantomPinned,
+        })
+    }
+
+    /// Returns a reference to the handler that was registered with the system.
+    pub fn handler(&self) -> &T {
+        &self.handler
+    }
+
+    /// Wait for pending IRQ handlers on other CPUs.
+    ///
+    /// This will attempt to access the inner [`Devres`] container.
+    pub fn try_synchronize(&self) -> Result {
+        let inner = self.inner.try_access().ok_or(ENODEV)?;
+        inner.synchronize();
+        Ok(())
+    }
+
+    /// Wait for pending IRQ handlers on other CPUs.
+    pub fn synchronize(&self, dev: &Device<Bound>) -> Result {
+        let inner = self.inner.access(dev)?;
+        inner.synchronize();
+        Ok(())
+    }
+}
+
+/// # Safety
+///
+/// This function should be only used as the callback in `request_threaded_irq`.
+unsafe extern "C" fn handle_threaded_irq_callback<T: ThreadedHandler>(
+    _irq: i32,
+    ptr: *mut c_void,
+) -> c_uint {
+    // SAFETY: `ptr` is a pointer to T set in `ThreadedRegistration::new`
+    let handler = unsafe { &*(ptr as *const T) };
+    T::handle(handler) as c_uint
+}
+
+/// # Safety
+///
+/// This function should be only used as the callback in `request_threaded_irq`.
+unsafe extern "C" fn thread_fn_callback<T: ThreadedHandler>(_irq: i32, ptr: *mut c_void) -> c_uint {
+    // SAFETY: `ptr` is a pointer to T set in `ThreadedRegistration::new`
+    let handler = unsafe { &*(ptr as *const T) };
+    T::handle_threaded(handler) as c_uint
+}

-- 
2.50.0

[PATCH v7 5/6] rust: platform: add irq accessors

[Daniel Almeida]

These accessors can be used to retrieve a irq::Registration and
irq::ThreadedRegistration from a platform device by
index or name. Alternatively, drivers can retrieve an IrqRequest from a
bound platform device for later use.

These accessors ensure that only valid IRQ lines can ever be registered.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
---
 rust/kernel/platform.rs | 146 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 145 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
index ced43367d900bcb87c0ce1af3981f5e5af129840..ba334bfdaf1db4cb60a33ef275739958bc4c77d5 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -5,8 +5,11 @@
 //! C header: [`include/linux/platform_device.h`](srctree/include/linux/platform_device.h)
 
 use crate::{
-    acpi, bindings, container_of, device, driver,
+    acpi, bindings, container_of,
+    device::{self, Bound},
+    driver,
     error::{from_result, to_result, Result},
+    irq::{self, IrqRequest},
     of,
     prelude::*,
     types::Opaque,
@@ -226,6 +229,147 @@ fn as_raw(&self) -> *mut bindings::platform_device {
     }
 }
 
+macro_rules! define_irq_accessor_by_index {
+    ($(#[$meta:meta])* $fn_name:ident, $request_fn:ident, $reg_type:ident, $handler_trait:ident) => {
+        $(#[$meta])*
+        pub fn $fn_name<T: irq::$handler_trait + 'static>(
+            &self,
+            flags: irq::flags::Flags,
+            index: u32,
+            name: &'static CStr,
+            handler: T,
+        ) -> Result<impl PinInit<irq::$reg_type<T>, Error> + '_> {
+            let request = self.$request_fn(index)?;
+
+            Ok(irq::$reg_type::<T>::new(
+                request,
+                flags,
+                name,
+                handler,
+            ))
+        }
+    };
+}
+
+macro_rules! define_irq_accessor_by_name {
+    ($(#[$meta:meta])* $fn_name:ident, $request_fn:ident, $reg_type:ident, $handler_trait:ident) => {
+        $(#[$meta])*
+        pub fn $fn_name<T: irq::$handler_trait + 'static>(
+            &self,
+            flags: irq::flags::Flags,
+            irq_name: &CStr,
+            name: &'static CStr,
+            handler: T,
+        ) -> Result<impl PinInit<irq::$reg_type<T>, Error> + '_> {
+            let request = self.$request_fn(irq_name)?;
+
+            Ok(irq::$reg_type::<T>::new(
+                request,
+                flags,
+                name,
+                handler,
+            ))
+        }
+    };
+}
+
+impl Device<Bound> {
+    /// Returns an [`IrqRequest`] for the IRQ at the given index, if any.
+    pub fn irq_by_index(&self, index: u32) -> Result<IrqRequest<'_>> {
+        // SAFETY: `self.as_raw` returns a valid pointer to a `struct platform_device`.
+        let irq = unsafe { bindings::platform_get_irq(self.as_raw(), index) };
+
+        if irq < 0 {
+            return Err(Error::from_errno(irq));
+        }
+
+        // SAFETY: `irq` is guaranteed to be a valid IRQ number for `&self`.
+        Ok(unsafe { IrqRequest::new(self.as_ref(), irq as u32) })
+    }
+
+    /// Returns an [`IrqRequest`] for the IRQ at the given index, but does not
+    /// print an error if the IRQ cannot be obtained.
+    pub fn optional_irq_by_index(&self, index: u32) -> Result<IrqRequest<'_>> {
+        // SAFETY: `self.as_raw` returns a valid pointer to a `struct platform_device`.
+        let irq = unsafe { bindings::platform_get_irq_optional(self.as_raw(), index) };
+
+        if irq < 0 {
+            return Err(Error::from_errno(irq));
+        }
+
+        // SAFETY: `irq` is guaranteed to be a valid IRQ number for `&self`.
+        Ok(unsafe { IrqRequest::new(self.as_ref(), irq as u32) })
+    }
+
+    /// Returns an [`IrqRequest`] for the IRQ with the given name, if any.
+    pub fn irq_by_name(&self, name: &CStr) -> Result<IrqRequest<'_>> {
+        // SAFETY: `self.as_raw` returns a valid pointer to a `struct platform_device`.
+        let irq = unsafe { bindings::platform_get_irq_byname(self.as_raw(), name.as_char_ptr()) };
+
+        if irq < 0 {
+            return Err(Error::from_errno(irq));
+        }
+
+        // SAFETY: `irq` is guaranteed to be a valid IRQ number for `&self`.
+        Ok(unsafe { IrqRequest::new(self.as_ref(), irq as u32) })
+    }
+
+    /// Returns an [`IrqRequest`] for the IRQ with the given name, but does not
+    /// print an error if the IRQ cannot be obtained.
+    pub fn optional_irq_by_name(&self, name: &CStr) -> Result<IrqRequest<'_>> {
+        // SAFETY: `self.as_raw` returns a valid pointer to a `struct platform_device`.
+        let irq = unsafe {
+            bindings::platform_get_irq_byname_optional(self.as_raw(), name.as_char_ptr())
+        };
+
+        if irq < 0 {
+            return Err(Error::from_errno(irq));
+        }
+
+        // SAFETY: `irq` is guaranteed to be a valid IRQ number for `&self`.
+        Ok(unsafe { IrqRequest::new(self.as_ref(), irq as u32) })
+    }
+
+    define_irq_accessor_by_index!(
+        /// Returns a [`irq::Registration`] for the IRQ at the given index.
+        request_irq_by_index, irq_by_index, Registration, Handler
+    );
+    define_irq_accessor_by_name!(
+        /// Returns a [`irq::Registration`] for the IRQ with the given name.
+        request_irq_by_name, irq_by_name, Registration, Handler
+    );
+    define_irq_accessor_by_index!(
+        /// Does the same as [`Self::request_irq_by_index`], except that it does
+        /// not print an error message if the IRQ cannot be obtained.
+        request_optional_irq_by_index, optional_irq_by_index, Registration, Handler
+    );
+    define_irq_accessor_by_name!(
+        /// Does the same as [`Self::request_irq_by_name`], except that it does
+        /// not print an error message if the IRQ cannot be obtained.
+        request_optional_irq_by_name, optional_irq_by_name, Registration, Handler
+    );
+
+    define_irq_accessor_by_index!(
+        /// Returns a [`irq::ThreadedRegistration`] for the IRQ at the given index.
+        request_threaded_irq_by_index, irq_by_index, ThreadedRegistration, ThreadedHandler
+    );
+    define_irq_accessor_by_name!(
+        /// Returns a [`irq::ThreadedRegistration`] for the IRQ with the given name.
+        request_threaded_irq_by_name, irq_by_name, ThreadedRegistration, ThreadedHandler
+    );
+    define_irq_accessor_by_index!(
+        /// Does the same as [`Self::request_threaded_irq_by_index`], except
+        /// that it does not print an error message if the IRQ cannot be
+        /// obtained.
+        request_optional_threaded_irq_by_index, optional_irq_by_index, ThreadedRegistration, ThreadedHandler
+    );
+    define_irq_accessor_by_name!(
+        /// Does the same as [`Self::request_threaded_irq_by_name`], except that
+        /// it does not print an error message if the IRQ cannot be obtained.
+        request_optional_threaded_irq_by_name, optional_irq_by_name, ThreadedRegistration, ThreadedHandler
+    );
+}
+
 // SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic
 // argument.
 kernel::impl_device_context_deref!(unsafe { Device });

-- 
2.50.0

[PATCH v7 6/6] rust: pci: add irq accessors

[Daniel Almeida]

These accessors can be used to retrieve a irq::Registration or a
irq::ThreadedRegistration from a pci device. Alternatively, drivers can
retrieve an IrqRequest from a bound PCI device for later use.

These accessors ensure that only valid IRQ lines can ever be registered.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
---
 rust/helpers/pci.c |  8 ++++++++
 rust/kernel/pci.rs | 45 +++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/rust/helpers/pci.c b/rust/helpers/pci.c
index ef9cb38c81a6a5375f72c3676cd9730aad17757b..5bf56004478c6945dc3e1a394fcd787c656d8b2a 100644
--- a/rust/helpers/pci.c
+++ b/rust/helpers/pci.c
@@ -11,3 +11,11 @@ bool rust_helper_dev_is_pci(const struct device *dev)
 {
 	return dev_is_pci(dev);
 }
+
+#ifndef CONFIG_PCI_MSI
+int rust_helper_pci_irq_vector(struct pci_dev *pdev, unsigned int nvec)
+{
+	return pci_irq_vector(pdev, nvec);
+}
+
+#endif
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 8b884e324dcfcef2a2e69b009fe1e3071efe7066..1ae390245fc62a078ce9dfd6f67b27368a5aeba2 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -10,8 +10,8 @@
     devres::Devres,
     driver,
     error::{from_result, to_result, Result},
-    io::Io,
-    io::IoRaw,
+    io::{Io, IoRaw},
+    irq::{self, IrqRequest},
     str::CStr,
     types::{ARef, Opaque},
     ThisModule,
@@ -427,6 +427,47 @@ pub fn iomap_region<'a>(
     ) -> impl PinInit<Devres<Bar>, Error> + 'a {
         self.iomap_region_sized::<0>(bar, name)
     }
+
+    /// Returns an [`IrqRequest`] for the IRQ vector at the given index, if any.
+    pub fn irq_vector(&self, index: u32) -> Result<IrqRequest<'_>> {
+        // SAFETY: `self.as_raw` returns a valid pointer to a `struct pci_dev`.
+        let irq = unsafe { crate::bindings::pci_irq_vector(self.as_raw(), index) };
+        if irq < 0 {
+            return Err(crate::error::Error::from_errno(irq));
+        }
+        // SAFETY: `irq` is guaranteed to be a valid IRQ number for `&self`.
+        Ok(unsafe { IrqRequest::new(self.as_ref(), irq as u32) })
+    }
+
+    /// Returns a [`kernel::irq::Registration`] for the IRQ vector at the given
+    /// index.
+    pub fn request_irq<T: crate::irq::Handler + 'static>(
+        &self,
+        index: u32,
+        flags: irq::flags::Flags,
+        name: &'static CStr,
+        handler: T,
+    ) -> Result<impl PinInit<irq::Registration<T>, Error> + '_> {
+        let request = self.irq_vector(index)?;
+
+        Ok(irq::Registration::<T>::new(request, flags, name, handler))
+    }
+
+    /// Returns a [`kernel::irq::ThreadedRegistration`] for the IRQ vector at
+    /// the given index.
+    pub fn request_threaded_irq<T: crate::irq::ThreadedHandler + 'static>(
+        &self,
+        index: u32,
+        flags: irq::flags::Flags,
+        name: &'static CStr,
+        handler: T,
+    ) -> Result<impl PinInit<irq::ThreadedRegistration<T>, Error> + '_> {
+        let request = self.irq_vector(index)?;
+
+        Ok(irq::ThreadedRegistration::<T>::new(
+            request, flags, name, handler,
+        ))
+    }
 }
 
 impl Device<device::Core> {

-- 
2.50.0

Re: [PATCH v7 0/6] rust: add support for request_irq

[Danilo Krummrich]

On Tue Jul 15, 2025 at 5:16 PM CEST, Daniel Almeida wrote:
> Daniel Almeida (6):
>       rust: irq: add irq module
>       rust: irq: add flags module
>       rust: irq: add support for non-threaded IRQs and handlers
>       rust: irq: add support for threaded IRQs and handlers
>       rust: platform: add irq accessors
>       rust: pci: add irq accessors

(Mostly copy-paste from v6 [1].)

What's the intended merge path for this series? Should I take it through
driver-core, in case we make it for the upcoming merge window? I'd assume so,
given that, besides the series also containing platform and PCI patches, it
depends on patches that are in driver-core-next.

@Thomas: Is there any agreement on how the IRQ Rust code should be maintained?
What's your preference?

- Danilo

[1] https://lore.kernel.org/lkml/aGbkfa57bDa1mzI7@pollux/

Re: [PATCH v7 2/6] rust: irq: add flags module

[Daniel Almeida]

Alice,

[…]

> +
> +    const fn new(value: u32) -> Self {
> +        build_assert!(value as u64 <= c_ulong::MAX as u64);
> +        Self(value as c_ulong)
> +    }
> +}

Sanity check here, is this what you meant?

— Daniel

Re: [PATCH v7 2/6] rust: irq: add flags module

[Alice Ryhl]

On Wed, Jul 16, 2025 at 4:20 PM Daniel Almeida
<daniel.almeida@collabora.com> wrote:
>
> Alice,
>
> […]
>
> > +
> > +    const fn new(value: u32) -> Self {
> > +        build_assert!(value as u64 <= c_ulong::MAX as u64);
> > +        Self(value as c_ulong)
> > +    }
> > +}
>
> Sanity check here, is this what you meant?

That's fine.

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

[kernel test robot]

Hi Daniel,

kernel test robot noticed the following build errors:

[auto build test ERROR on 3964d07dd821efe9680e90c51c86661a98e60a0f]

url:    https://github.com/intel-lab-lkp/linux/commits/Daniel-Almeida/rust-irq-add-irq-module/20250715-232121
base:   3964d07dd821efe9680e90c51c86661a98e60a0f
patch link:    https://lore.kernel.org/r/20250715-topics-tyr-request_irq2-v7-3-d469c0f37c07%40collabora.com
patch subject: [PATCH v7 3/6] rust: irq: add support for non-threaded IRQs and handlers
config: x86_64-rhel-9.4-rust (https://download.01.org/0day-ci/archive/20250717/202507170718.AVqYqRan-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
rustc: rustc 1.88.0 (6b00bc388 2025-06-23)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250717/202507170718.AVqYqRan-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202507170718.AVqYqRan-lkp@intel.com/

All errors (new ones prefixed by >>):

>> error[E0425]: cannot find value `SHARED` in module `flags`
   --> rust/doctests_kernel_generated.rs:4790:58
   |
   4790 |     let registration = Registration::new(request, flags::SHARED, c_str!("my_device"), handler);
   |                                                          ^^^^^^ not found in `flags`
   |
   help: consider importing this constant
   |
   3    + use kernel::mm::virt::flags::SHARED;
   |
   help: if you import `SHARED`, refer to it directly
   |
   4790 -     let registration = Registration::new(request, flags::SHARED, c_str!("my_device"), handler);
   4790 +     let registration = Registration::new(request, SHARED, c_str!("my_device"), handler);
   |

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

[Daniel Almeida]

> On 16 Jul 2025, at 20:45, kernel test robot <lkp@intel.com> wrote:
> 
> Hi Daniel,
> 
> kernel test robot noticed the following build errors:
> 
> [auto build test ERROR on 3964d07dd821efe9680e90c51c86661a98e60a0f]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Daniel-Almeida/rust-irq-add-irq-module/20250715-232121
> base:   3964d07dd821efe9680e90c51c86661a98e60a0f
> patch link:    https://lore.kernel.org/r/20250715-topics-tyr-request_irq2-v7-3-d469c0f37c07%40collabora.com
> patch subject: [PATCH v7 3/6] rust: irq: add support for non-threaded IRQs and handlers
> config: x86_64-rhel-9.4-rust (https://download.01.org/0day-ci/archive/20250717/202507170718.AVqYqRan-lkp@intel.com/config)
> compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
> rustc: rustc 1.88.0 (6b00bc388 2025-06-23)
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250717/202507170718.AVqYqRan-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202507170718.AVqYqRan-lkp@intel.com/
> 
> All errors (new ones prefixed by >>):
> 
>>> error[E0425]: cannot find value `SHARED` in module `flags`
>   --> rust/doctests_kernel_generated.rs:4790:58
>   |
>   4790 |     let registration = Registration::new(request, flags::SHARED, c_str!("my_device"), handler);
>   |                                                          ^^^^^^ not found in `flags`
>   |
>   help: consider importing this constant
>   |
>   3    + use kernel::mm::virt::flags::SHARED;
>   |
>   help: if you import `SHARED`, refer to it directly
>   |
>   4790 -     let registration = Registration::new(request, flags::SHARED, c_str!("my_device"), handler);
>   4790 +     let registration = Registration::new(request, SHARED, c_str!("my_device"), handler);
>   |
> 
> -- 
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
> 

This is a single character fix, so I am waiting for the discussion on the cover
letter [0] to advance before sending a new version.

[0] https://lore.kernel.org/all/DBCQKJIBVGGM.1R0QNKO3TE4N0@kernel.org/#t

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

[Alice Ryhl]

On Thu, Jul 17, 2025 at 6:21 PM Daniel Almeida
<daniel.almeida@collabora.com> wrote:
>
>
>
> > On 16 Jul 2025, at 20:45, kernel test robot <lkp@intel.com> wrote:
> >
> > Hi Daniel,
> >
> > kernel test robot noticed the following build errors:
> >
> > [auto build test ERROR on 3964d07dd821efe9680e90c51c86661a98e60a0f]
> >
> > url:    https://github.com/intel-lab-lkp/linux/commits/Daniel-Almeida/rust-irq-add-irq-module/20250715-232121
> > base:   3964d07dd821efe9680e90c51c86661a98e60a0f
> > patch link:    https://lore.kernel.org/r/20250715-topics-tyr-request_irq2-v7-3-d469c0f37c07%40collabora.com
> > patch subject: [PATCH v7 3/6] rust: irq: add support for non-threaded IRQs and handlers
> > config: x86_64-rhel-9.4-rust (https://download.01.org/0day-ci/archive/20250717/202507170718.AVqYqRan-lkp@intel.com/config)
> > compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
> > rustc: rustc 1.88.0 (6b00bc388 2025-06-23)
> > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250717/202507170718.AVqYqRan-lkp@intel.com/reproduce)
> >
> > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > the same patch/commit), kindly add following tags
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Closes: https://lore.kernel.org/oe-kbuild-all/202507170718.AVqYqRan-lkp@intel.com/
> >
> > All errors (new ones prefixed by >>):
> >
> >>> error[E0425]: cannot find value `SHARED` in module `flags`
> >   --> rust/doctests_kernel_generated.rs:4790:58
> >   |
> >   4790 |     let registration = Registration::new(request, flags::SHARED, c_str!("my_device"), handler);
> >   |                                                          ^^^^^^ not found in `flags`
> >   |
> >   help: consider importing this constant
> >   |
> >   3    + use kernel::mm::virt::flags::SHARED;
> >   |
> >   help: if you import `SHARED`, refer to it directly
> >   |
> >   4790 -     let registration = Registration::new(request, flags::SHARED, c_str!("my_device"), handler);
> >   4790 +     let registration = Registration::new(request, SHARED, c_str!("my_device"), handler);
> >   |
> >
> > --
> > 0-DAY CI Kernel Test Service
> > https://github.com/intel/lkp-tests/wiki
> >
>
> This is a single character fix, so I am waiting for the discussion on the cover
> letter [0] to advance before sending a new version.
>
> [0] https://lore.kernel.org/all/DBCQKJIBVGGM.1R0QNKO3TE4N0@kernel.org/#t

My suggestion is to make the flags module private and re-export the
Flags type from the irq module. That way you don't have to write
use kernel::irq::flags::Flags;

Alice

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

[Alice Ryhl]

On Tue, Jul 15, 2025 at 12:16:40PM -0300, 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@collabora.com>

Overall LGTM. Some very minor nits below.

> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index 7e8f2285064797d5bbac5583990ff823b76c6bdc..fc73b89ff9d539e536a5da9388e4926a91a6130e 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -52,6 +52,7 @@
>  #include <linux/ethtool.h>
>  #include <linux/file.h>
>  #include <linux/firmware.h>
> +#include <linux/interrupt.h>
>  #include <linux/fs.h>
>  #include <linux/jiffies.h>
>  #include <linux/jump_label.h>
> diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
> index 0b09bd0e3561c7bf80bf79faf1aebd7eeb851984..653c3f7b85c5f7192b1584c748a9d7e4af3796e9 100644
> --- a/rust/helpers/helpers.c
> +++ b/rust/helpers/helpers.c
> @@ -22,6 +22,7 @@
>  #include "dma.c"
>  #include "drm.c"
>  #include "err.c"
> +#include "irq.c"
>  #include "fs.c"
>  #include "io.c"
>  #include "jump_label.c"
> diff --git a/rust/helpers/irq.c b/rust/helpers/irq.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..1faca428e2c047a656dec3171855c1508d67e60b
> --- /dev/null
> +++ b/rust/helpers/irq.c
> @@ -0,0 +1,9 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/interrupt.h>
> +
> +int rust_helper_request_irq(unsigned int irq, irq_handler_t handler,
> +			    unsigned long flags, const char *name, void *dev)
> +{
> +	return request_irq(irq, handler, flags, name, dev);
> +}
> diff --git a/rust/kernel/irq.rs b/rust/kernel/irq.rs
> index 9abd9a6dc36f3e3ecc1f92ad7b0040176b56a079..01bd08884b72c2a3a9460897bce751c732a19794 100644
> --- a/rust/kernel/irq.rs
> +++ b/rust/kernel/irq.rs
> @@ -12,3 +12,8 @@
>  
>  /// Flags to be used when registering IRQ handlers.
>  pub mod flags;
> +
> +/// IRQ allocation and handling.
> +pub mod request;
> +
> +pub use request::{Handler, IrqRequest, IrqReturn, Registration};
> diff --git a/rust/kernel/irq/request.rs b/rust/kernel/irq/request.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..2f4637d8bc4c9fda23cbc8307687035957b0042a
> --- /dev/null
> +++ b/rust/kernel/irq/request.rs
> @@ -0,0 +1,267 @@
> +// 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;

The usual style is to write this as:

use crate::device::{Bound, 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.

Missing links:

/// The value that can be returned from an [`IrqHandler`] or a [`ThreadedIrqHandler`].

> +#[repr(u32)]
> +pub enum IrqReturn {
> +    /// The interrupt was not from this device or was not handled.
> +    None = bindings::irqreturn_IRQ_NONE,
> +
> +    /// The interrupt was handled by this device.
> +    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 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 }
> +    }
> +
> +    /// Returns the IRQ number of an [`IrqRequest`].
> +    pub fn irq(&self) -> u32 {
> +        self.irq
> +    }
> +}
> +
> +/// A registration of an IRQ handler for a given IRQ line.
> +///
> +/// # Examples
> +///
> +/// The following is an example of using `Registration`. 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::Registration;
> +/// use kernel::irq::IrqRequest;
> +/// use kernel::irq::IrqReturn;

/// use kernel::irq::{Flags, IrqRequest, IrqReturn, Registration};

> +/// use kernel::sync::Arc;
> +/// use kernel::c_str;
> +/// use kernel::alloc::flags::GFP_KERNEL;

GFP_KERNEL is in the prelude.

> +/// // 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);
> +///
> +/// // [`kernel::irq::request::Handler::handle`] takes `&self`. This example
> +/// // illustrates how interior mutability can be used when sharing the data
> +/// // between process context and IRQ context.
> +///
> +/// type Handler = Data;
> +///
> +/// impl kernel::irq::request::Handler for Handler {
> +///     // This is executing in IRQ context in some CPU. Other CPUs can still
> +///     // try to access to data.
> +///     fn handle(&self) -> IrqReturn {
> +///         self.0.fetch_add(1, Ordering::Relaxed);
> +///
> +///         IrqReturn::Handled
> +///     }
> +/// }
> +///
> +/// // Registers an IRQ handler for the given IrqRequest.
> +/// //
> +/// // This is executing in process context and assumes that `request` was
> +/// // previously acquired from a device.
> +/// fn register_irq(handler: Handler, request: IrqRequest<'_>) -> Result<Arc<Registration<Handler>>> {
> +///     let registration = Registration::new(request, flags::SHARED, c_str!("my_device"), handler);
> +///
> +///     let registration = Arc::pin_init(registration, GFP_KERNEL)?;
> +///
> +///     // The data can be accessed from process context too.
> +///     registration.handler().0.fetch_add(1, Ordering::Relaxed);
> +///
> +///     Ok(registration)
> +/// }
> +/// # Ok::<(), Error>(())
> +/// ```
> +///
> +/// # Invariants
> +///
> +/// * We own an irq handler using `&self.handler` as its private data.
> +///
> +#[pin_data]
> +pub struct Registration<T: Handler + 'static> {
> +    #[pin]
> +    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,
> +}
> +
> +impl<T: Handler + 'static> Registration<T> {
> +    /// Registers the IRQ handler with the system for the given IRQ number.
> +    pub fn new<'a>(
> +        request: IrqRequest<'a>,
> +        flags: Flags,
> +        name: &'static CStr,
> +        handler: T,
> +    ) -> impl PinInit<Self, Error> + 'a {
> +        try_pin_init!(&this in Self {
> +            handler,
> +            inner <- Devres::new(
> +                request.dev,
> +                try_pin_init!(RegistrationInner {
> +                    // SAFETY: `this` is a valid pointer to the `Registration` instance
> +                    cookie: unsafe { &raw mut (*this.as_ptr()).handler }.cast(),
> +                    irq: {
> +                        // SAFETY:
> +                        // - The callbacks are valid for use with request_irq.
> +                        // - If this succeeds, the slot is guaranteed to be valid until the
> +                        //   destructor of Self runs, which will deregister the callbacks
> +                        //   before the memory location becomes invalid.
> +                        to_result(unsafe {
> +                            bindings::request_irq(
> +                                request.irq,
> +                                Some(handle_irq_callback::<T>),
> +                                flags.into_inner(),
> +                                name.as_char_ptr(),
> +                                (&raw mut (*this.as_ptr()).handler).cast(),
> +                            )
> +                        })?;
> +                        request.irq
> +                    }
> +                })
> +            ),
> +            _pin: PhantomPinned,
> +        })
> +    }
> +
> +    /// Returns a reference to the handler that was registered with the system.
> +    pub fn handler(&self) -> &T {
> +        &self.handler
> +    }
> +
> +    /// Wait for pending IRQ handlers on other CPUs.
> +    ///
> +    /// This will attempt to access the inner [`Devres`] container.
> +    pub fn try_synchronize(&self) -> Result {
> +        let inner = self.inner.try_access().ok_or(ENODEV)?;
> +        inner.synchronize();
> +        Ok(())
> +    }
> +
> +    /// Wait for pending IRQ handlers on other CPUs.
> +    pub fn synchronize(&self, dev: &Device<Bound>) -> Result {
> +        let inner = self.inner.access(dev)?;
> +        inner.synchronize();
> +        Ok(())
> +    }
> +}
> +
> +/// # Safety
> +///
> +/// This function should be only used as the callback in `request_irq`.
> +unsafe extern "C" fn handle_irq_callback<T: Handler>(_irq: i32, ptr: *mut c_void) -> c_uint {
> +    // SAFETY: `ptr` is a pointer to T set in `Registration::new`
> +    let handler = unsafe { &*(ptr as *const T) };
> +    T::handle(handler) as c_uint
> +}
> 
> -- 
> 2.50.0
> 

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

[Alice Ryhl]

On Tue, Jul 15, 2025 at 12:16:41PM -0300, Daniel Almeida wrote:
> This patch adds support for threaded IRQs and handlers through
> irq::ThreadedRegistration and the irq::ThreadedHandler trait.
> 
> Threaded interrupts are more permissive in the sense that further
> processing is possible in a kthread. This means that said execution takes
> place outside of interrupt context, which is rather restrictive in many
> ways.
> 
> Registering a threaded irq is dependent upon having an IrqRequest that
> was previously allocated by a given device. This will be introduced in
> subsequent patches.
> 
> Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>

A few of the same nits as on the non-threaded patch apply here too.

> +/// impl kernel::irq::request::ThreadedHandler for Handler {

If you import ThreadedHandler at the top of the example, then you don't
need this. If you want it to say irq in the path here, then import
kernel::irq::self and use `impl irq::ThreadedHandler for Handler` here.

The same could make sense for the flags. You can write
irq::Flags::SHARED if you import the irq module.

(This requires a re-export in the irq module if you don't have one
already. Also, I would make the irq module private so that end-users
import everything via the irq:: path without a sub-module.)

Alice

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

[Daniel Almeida]

Hi Alice, thanks for looking into this again :)


[…]

>> diff --git a/rust/kernel/irq/request.rs b/rust/kernel/irq/request.rs
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..2f4637d8bc4c9fda23cbc8307687035957b0042a
>> --- /dev/null
>> +++ b/rust/kernel/irq/request.rs
>> @@ -0,0 +1,267 @@
>> +// 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;
> 
> The usual style is to write this as:
> 
> use crate::device::{Bound, Device};

I dislike this syntax because I think it is a conflict magnet. Moreover, when
you get conflicts, they are harder to solve than they are when each import
is in its own line, at least IMHO.  

In any case, I don't think we have a guideline for imports at the moment?

[…]

>> +/// A registration of an IRQ handler for a given IRQ line.
>> +///
>> +/// # Examples
>> +///
>> +/// The following is an example of using `Registration`. 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::Registration;
>> +/// use kernel::irq::IrqRequest;
>> +/// use kernel::irq::IrqReturn;
> 
> /// use kernel::irq::{Flags, IrqRequest, IrqReturn, Registration};

Same here. I’d rather not do this, if it’s ok with others.

— Daniel

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

[Danilo Krummrich]

On Mon Jul 21, 2025 at 5:10 PM CEST, Daniel Almeida wrote:
> Hi Alice, thanks for looking into this again :)
>
>
> […]
>
>>> diff --git a/rust/kernel/irq/request.rs b/rust/kernel/irq/request.rs
>>> new file mode 100644
>>> index 0000000000000000000000000000000000000000..2f4637d8bc4c9fda23cbc8307687035957b0042a
>>> --- /dev/null
>>> +++ b/rust/kernel/irq/request.rs
>>> @@ -0,0 +1,267 @@
>>> +// 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;
>> 
>> The usual style is to write this as:
>> 
>> use crate::device::{Bound, Device};
>
> I dislike this syntax because I think it is a conflict magnet. Moreover, when
> you get conflicts, they are harder to solve than they are when each import
> is in its own line, at least IMHO.  

Intuitively, I would agree. However, I think practically it's not that bad.

While it's true that Rust has generally more conflict potential - especially in
the current phase - my feeling hasn't been that includes produce significantly
more conflicts then any other code so far.

> In any case, I don't think we have a guideline for imports at the moment?

No, but I think we should try to be as consistent as possible (at least within a
a certain logical unit, e.g. subsystem, module, etc.). Not sure where exactly
the IRQ stuff will end up yet. :)

>>> +/// A registration of an IRQ handler for a given IRQ line.
>>> +///
>>> +/// # Examples
>>> +///
>>> +/// The following is an example of using `Registration`. 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::Registration;
>>> +/// use kernel::irq::IrqRequest;
>>> +/// use kernel::irq::IrqReturn;
>> 
>> /// use kernel::irq::{Flags, IrqRequest, IrqReturn, Registration};
>
> Same here. I’d rather not do this, if it’s ok with others.
>
> — Daniel

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

[Daniel Almeida]

> On 21 Jul 2025, at 12:28, Danilo Krummrich <dakr@kernel.org> wrote:
> 
> On Mon Jul 21, 2025 at 5:10 PM CEST, Daniel Almeida wrote:
>> Hi Alice, thanks for looking into this again :)
>> 
>> 
>> […]
>> 
>>>> diff --git a/rust/kernel/irq/request.rs b/rust/kernel/irq/request.rs
>>>> new file mode 100644
>>>> index 0000000000000000000000000000000000000000..2f4637d8bc4c9fda23cbc8307687035957b0042a
>>>> --- /dev/null
>>>> +++ b/rust/kernel/irq/request.rs
>>>> @@ -0,0 +1,267 @@
>>>> +// 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;
>>> 
>>> The usual style is to write this as:
>>> 
>>> use crate::device::{Bound, Device};
>> 
>> I dislike this syntax because I think it is a conflict magnet. Moreover, when
>> you get conflicts, they are harder to solve than they are when each import
>> is in its own line, at least IMHO.  
> 
> Intuitively, I would agree. However, I think practically it's not that bad.
> 
> While it's true that Rust has generally more conflict potential - especially in
> the current phase - my feeling hasn't been that includes produce significantly
> more conflicts then any other code so far.

Hmm, I faced lots of conflicts for the platform I/O stuff, for example. They
were all on the imports and it was a bit hard to fix it by hand. i.e.: it’s
much simpler to discard the modifications and then ask rust-analyzer to figure
out what should be grouped where on the new code. This is a bit undesirable.


> 
>> In any case, I don't think we have a guideline for imports at the moment?
> 
> No, but I think we should try to be as consistent as possible (at least within a
> a certain logical unit, e.g. subsystem, module, etc.). Not sure where exactly
> the IRQ stuff will end up yet. :)


Sure, I just think we should discuss this at the kernel crate level at a future
point then, at least IMHO. I think it's something that Andreas had already
commented on, by the way.

— Daniel

Re: [PATCH v7 0/6] rust: add support for request_irq

[Dirk Behme]

On 15/07/2025 17:16, Daniel Almeida wrote:
> Changes in v7:
> - Rebased on top of driver-core-next
> - Added Flags::new(), which is a const fn. This lets us use build_assert!()
>   to verify the casts (hopefully this is what you meant, Alice?)
> - Changed the Flags inner type to take c_ulong directly, to minimize casts
>   (Thanks, Alice)
> - Moved the flag constants into Impl Flags, instead of using a separate
>   module (Alice)
> - Reverted to using #[repr(u32)] in Threaded/IrqReturn (Thanks Alice,
>   Benno)
> - Fixed all instances where the full path was specified for types in the
>   prelude (Alice)
> - Removed 'static from the CStr used to perform the lookup in the platform
>   accessor (Alice)
> - Renamed the PCI accessors, as asked by Danilo
> - Added more docs to Flags, going into more detail on what they do and how
>   to use them (Miguel)
> - Fixed the indentation in some of the docs (Alice)
> - Added Alice's r-b as appropriate
> - Link to v6: https://lore.kernel.org/rust-for-linux/20250703-topics-tyr-request_irq-v6-0-74103bdc7c52@collabora.com/


Looking for an easy way to test interrupts on an ARM64 Renesas RCar3 SoC
I found a quite simple timer unit (TMU) which has a configurable (start
value & frequency) count down. An interrupt is generated when the
counter reaches 0. And the counter restarts then. There is a C driver
for this already [1].

Using this patch series together with Alice's [2] I got a quite simple
periodic 1 min interrupt handling to run (just for testing, of course
not a full driver): [3] (output [4]).

With that:

Tested-by: Dirk Behme <dirk.behme@de.bosch.com>

Thanks to Daniel for the support!

Dirk

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clocksource/sh_tmu.c

[2]
https://lore.kernel.org/rust-for-linux/20250721-irq-bound-device-v1-1-4fb2af418a63@google.com/

[3]

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 645f517a1ac2..d009a0e3508c 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -554,7 +554,16 @@ config RENESAS_OSTM
 	  Enables the support for the Renesas OSTM.

 config SH_TIMER_TMU
-	bool "Renesas TMU timer driver" if COMPILE_TEST
+	bool "Renesas TMU timer driver"
+	depends on HAS_IOMEM
+	default SYS_SUPPORTS_SH_TMU
+	help
+	  This enables build of a clocksource and clockevent driver for
+	  the 32-bit Timer Unit (TMU) hardware available on a wide range
+	  SoCs from Renesas.
+
+config SH_TIMER_TMU_RUST
+	bool "Renesas TMU Rust timer driver"
 	depends on HAS_IOMEM
 	default SYS_SUPPORTS_SH_TMU
 	help
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index 205bf3b0a8f3..66567f871502 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_SH_TIMER_CMT)	+= sh_cmt.o
 obj-$(CONFIG_SH_TIMER_MTU2)	+= sh_mtu2.o
 obj-$(CONFIG_RENESAS_OSTM)	+= renesas-ostm.o
 obj-$(CONFIG_SH_TIMER_TMU)	+= sh_tmu.o
+obj-$(CONFIG_SH_TIMER_TMU_RUST)	+= sh_tmu_rust.o
 obj-$(CONFIG_EM_TIMER_STI)	+= em_sti.o
 obj-$(CONFIG_CLKBLD_I8253)	+= i8253.o
 obj-$(CONFIG_CLKSRC_MMIO)	+= mmio.o
diff --git a/drivers/clocksource/sh_tmu_rust.rs
b/drivers/clocksource/sh_tmu_rust.rs
new file mode 100644
index 000000000000..328f9541d1bb
--- /dev/null
+++ b/drivers/clocksource/sh_tmu_rust.rs
@@ -0,0 +1,113 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Rust Renesas TMU driver.
+
+use kernel::{
+    c_str,
+    device::{Core, Device, Bound},
+    devres::Devres,
+    io::mem::IoMem,
+    irq::{flags::Flags, IrqReturn, Registration},
+    of, platform,
+    prelude::*,
+    sync::Arc,
+    types::ARef,
+};
+
+struct RenesasTMUDriver {
+    pdev: ARef<platform::Device>,
+    _registration: Arc<Registration<Handler>>,
+    _iomem: Arc<Devres<IoMem>>,
+}
+
+struct Info;
+
+kernel::of_device_table!(
+    OF_TABLE,
+    MODULE_OF_TABLE,
+    <RenesasTMUDriver as platform::Driver>::IdInfo,
+    [(of::DeviceId::new(c_str!("renesas,tmu")), Info)]
+);
+
+const TSTR: usize =  0x4; //  8 Bit register
+const TCOR: usize =  0x8; // 32 Bit register
+const TCNT: usize =  0xC; // 32 Bit register
+const TCR:  usize = 0x10; // 16 Bit register
+
+struct Handler {
+    iomem: Arc<Devres<IoMem>>,
+}
+
+impl kernel::irq::request::Handler for Handler {
+    fn handle(&self, dev: &Device<Bound>) -> IrqReturn {
+        pr_info!("Renesas TMU IRQ handler called.\n");
+
+        // Reset the underflow flag
+        let io = self.iomem.access(dev).unwrap();
+        let tcr = io.try_read16_relaxed(TCR).unwrap_or(0);
+        if tcr & (0x1 << 8) != 0 {
+            io.try_write16_relaxed(tcr & !(0x1 << 8), TCR).unwrap_or(());
+        }
+
+        IrqReturn::Handled
+    }
+}
+
+impl platform::Driver for RenesasTMUDriver {
+    type IdInfo = Info;
+    const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_TABLE);
+
+    fn probe(
+        pdev: &platform::Device<Core>,
+        _info: Option<&Self::IdInfo>,
+    ) -> Result<Pin<KBox<Self>>> {
+        let dev = pdev.as_ref();
+
+        dev_dbg!(dev, "Probe Rust Renesas TMU driver.\n");
+
+        let request = pdev.request_io_by_index(0).ok_or(EINVAL)?;
+        let iomem = Arc::pin_init(request.iomap()?, GFP_KERNEL)?;
+        let io = Arc::pin_init(iomem.access(dev)?, GFP_KERNEL)?;
+
+        // Set count to 1 minute. Clock is 16.66MHz / 4 = 4.165MHz
+        let timeout = 4165000 * 60; // 1 minute in clock ticks
+        io.try_write32_relaxed(timeout, TCOR)?;
+        io.try_write32_relaxed(timeout, TCNT)?;
+        // Enable underflow interrupt (UNIE, Underflow Interrupt Control)
+        let tcr = io.try_read16_relaxed(TCR)?;
+        io.try_write16_relaxed(tcr | 0x1 << 5, TCR)?;
+
+        let request = pdev.irq_by_index(0)?;
+        dev_info!(dev, "IRQ:  {}\n", request.irq());
+        let registration = Registration::new(request, Flags::SHARED,
c_str!("tmu"), Handler{iomem: iomem.clone()});
+        let registration = Arc::pin_init(registration, GFP_KERNEL)?;
+
+        // Enable TMU
+        io.try_write8_relaxed(0x1, TSTR)?;
+        // Read back registers to verify
+        dev_info!(dev, "TSTR: 0x{:x}\n", io.try_read8_relaxed(TSTR)?);
+        dev_info!(dev, "TCOR: 0x{:x}\n", io.try_read32_relaxed(TCOR)?);
+        dev_info!(dev, "TCNT: 0x{:x}\n", io.try_read32_relaxed(TCNT)?);
+        dev_info!(dev, "TCR:  0x{:x}\n", io.try_read16_relaxed(TCR)?);
+
+        let drvdata = KBox::pin_init(Self { pdev: pdev.into(),
_registration: registration, _iomem: iomem.clone()}, GFP_KERNEL)?;
+
+        dev_info!(dev, "probe done\n");
+
+        Ok(drvdata)
+    }
+}
+
+impl Drop for RenesasTMUDriver {
+    fn drop(&mut self) {
+        dev_dbg!(self.pdev.as_ref(), "Remove Rust Renesas TMU driver.\n");
+    }
+}
+
+kernel::module_platform_driver! {
+    type: RenesasTMUDriver,
+    name: "rust_tmu",
+    authors: ["Dirk Behme"],
+    description: "Rust Renesas TMU driver",
+    license: "GPL v2",
+}

[4] Interrupt each 60s:

...
[  430.655055] rust_tmu: Renesas TMU IRQ handler called.
[  490.637054] rust_tmu: Renesas TMU IRQ handler called.
[  550.619052] rust_tmu: Renesas TMU IRQ handler called.
[  610.601050] rust_tmu: Renesas TMU IRQ handler called.
[  670.583049] rust_tmu: Renesas TMU IRQ handler called.
[  730.565047] rust_tmu: Renesas TMU IRQ handler called.
...

Re: [PATCH v7 0/6] rust: add support for request_irq

[Joel Fernandes]

On Tue, Jul 15, 2025 at 12:16:37PM -0300, Daniel Almeida wrote:
> Changes in v7:

Hello, Daniel,

I tested this series on an Nvidia 3090 GPU running the Nova project and I am
able to register and receive interrupts (there are several WIP patches for
Nova that are needed but your series is a dependency). We are looking forward
to having these patches upstream, please feel free to add my tag to the
patches:

Tested-by: Joel Fernandes <joelagnelf@nvidia.com>

thanks,

 - Joel


> - Rebased on top of driver-core-next
> - Added Flags::new(), which is a const fn. This lets us use build_assert!()
>   to verify the casts (hopefully this is what you meant, Alice?)
> - Changed the Flags inner type to take c_ulong directly, to minimize casts
>   (Thanks, Alice)
> - Moved the flag constants into Impl Flags, instead of using a separate
>   module (Alice)
> - Reverted to using #[repr(u32)] in Threaded/IrqReturn (Thanks Alice,
>   Benno)
> - Fixed all instances where the full path was specified for types in the
>   prelude (Alice)
> - Removed 'static from the CStr used to perform the lookup in the platform
>   accessor (Alice)
> - Renamed the PCI accessors, as asked by Danilo
> - Added more docs to Flags, going into more detail on what they do and how
>   to use them (Miguel)
> - Fixed the indentation in some of the docs (Alice)
> - Added Alice's r-b as appropriate
> - Link to v6: https://lore.kernel.org/rust-for-linux/20250703-topics-tyr-request_irq-v6-0-74103bdc7c52@collabora.com/
> 
> Changes in v6:
> - Fixed some typos in the docs (thanks, Dirk!)
> - Reordered the arguments for the accessors in platform.rs (Danilo)
> - Renamed handle_on_thread() to handle_threaded() (Danilo)
> - Changed the documentation for Handler and ThreadedHandler to what
>   Danilo suggested
> - Link to v5: https://lore.kernel.org/r/20250627-topics-tyr-request_irq-v5-0-0545ee4dadf6@collabora.com
> 
> Changes in v5:
> 
> Thanks, Danilo {
>   - Removed extra scope in the examples.
>   - Renamed Registration::register() to Registration::new(),
>   - Switched to try_pin_init! in Registration::new() (thanks for the
>     code and the help, Boqun and Benno)
>   - Renamed the trait functions to handle() and handle_on_thread().
>   - Introduced IrqRequest with an unsafe pub(crate) constructor
>   - Made both register() and the accessors that return IrqRequest public
>     the idea is to allow both of these to work:
> 	// `irq` is an `irq::Registration`
> 	let irq = pdev.threaded_irq_by_name()?
>   and
> 	// `req` is an `IrqRequest`.
> 	let req = pdev.irq_by_name()?;
> 	// `irq` is an `irq::Registration`
> 	let irq = irq::ThreadedRegistration::new(req)?;
> 
>   - Added another name in the byname variants. There's now one for the
>     request part and the other one to register()
>   - Reworked the examples in request.rs
>   - Implemented the irq accessors in place for pci.rs
>   - Split the platform accessor macros into two
> }
> 
> - Added a rust helper for pci_irq_vectors if !CONFIG_PCI_MSI (thanks,
> Intel 0day bot)
> - Link to v4: https://lore.kernel.org/r/20250608-topics-tyr-request_irq-v4-0-81cb81fb8073@collabora.com
> 
> Changes in v4:
> 
> Thanks, Benno {
>   - Split series into more patches (see patches 1-4)
>   - Use cast() where possible
>   - Merge pub use statements.
>   - Add {Threaded}IrqReturn::into_inner() instead of #[repr(u32)]
>   - Used AtomicU32 instead of SpinLock to add interior mutability to the
>     handler's data. SpinLockIrq did not land yet.
>   - Mention that `&self` is !Unpin and was initialized using pin_init in
>     drop()
>   - Fix the docs slightly
> }
> 
> - Add {try_}synchronize_irq().
> - Use Devres for the irq registration (see RegistrationInner). This idea
>   was suggested by Danilo and Alice.
> - Added PCI accessors (as asked by Joel Fernandez)
> - Fix a major oversight: we were passing in a pointer to Registration
>   in register_{threaded}_irq() but casting it to Handler/ThreadedHandler in
>   the callbacks.
> - Make register() pub(crate) so drivers can only retrieve registrations
>   through device-specific accessors. This forbids drivers from trying to
>   register an invalid irq.
> - I think this will still go through a few rounds, so I'll defer the
>   patch to update MAINTAINERS for now.
> 
> - Link to v3: https://lore.kernel.org/r/20250514-topics-tyr-request_irq-v3-0-d6fcc2591a88@collabora.com
> 
> Changes in v3:
> - Rebased on driver-core-next
> - Added patch to get the irq numbers from a platform device (thanks,
>   Christian!)
> - Split flags into its own file.
> - Change iff to "if and only if"
> - Implement PartialEq and Eq for Flags
> - Fix some broken docs/markdown
> - Reexport most things so users can elide ::request from the path
> - Add a blanket implementation of ThreadedHandler and Handler for
>   Arc/Box<T: Handler> that just forwards the call to the T. This lets us
>   have Arc<Foo> and Box<Foo> as handlers if Foo: Handler.
> - Rework the examples a bit.
> - Remove "as _" casts in favor of "as u64" for flags. This is needed to
>   cast the individual flags into u64.
> - Use #[repr(u32)] for ThreadedIrqReturn and IrqReturn.
> - Wrapped commit messages to < 75 characters
> 
> - Link to v2: https://lore.kernel.org/r/20250122163932.46697-1-daniel.almeida@collabora.com
> 
> Changes in v2:
> - Added Co-developed-by tag to account for the work that Alice did in order to
> figure out how to do this without Opaque<T> (Thanks!)
> - Removed Opaque<T> in favor of plain T
> - Fixed the examples
> - Made sure that the invariants sections are the last entry in the docs
> - Switched to slot.cast() where applicable,
> - Mentioned in the safety comments that we require that T: Sync,
> - Removed ThreadedFnReturn in favor of IrqReturn,
> - Improved the commit message
> 
> Link to v1: https://lore.kernel.org/rust-for-linux/20241024-topic-panthor-rs-request_irq-v1-1-7cbc51c182ca@collabora.com/
> 
> ---
> Daniel Almeida (6):
>       rust: irq: add irq module
>       rust: irq: add flags module
>       rust: irq: add support for non-threaded IRQs and handlers
>       rust: irq: add support for threaded IRQs and handlers
>       rust: platform: add irq accessors
>       rust: pci: add irq accessors
> 
>  rust/bindings/bindings_helper.h |   1 +
>  rust/helpers/helpers.c          |   1 +
>  rust/helpers/irq.c              |   9 +
>  rust/helpers/pci.c              |   8 +
>  rust/kernel/irq.rs              |  22 ++
>  rust/kernel/irq/flags.rs        | 124 ++++++++++
>  rust/kernel/irq/request.rs      | 490 ++++++++++++++++++++++++++++++++++++++++
>  rust/kernel/lib.rs              |   1 +
>  rust/kernel/pci.rs              |  45 +++-
>  rust/kernel/platform.rs         | 146 +++++++++++-
>  10 files changed, 844 insertions(+), 3 deletions(-)
> ---
> base-commit: 3964d07dd821efe9680e90c51c86661a98e60a0f
> change-id: 20250712-topics-tyr-request_irq2-ae7ee9b85854
> 
> Best regards,
> -- 
> Daniel Almeida <daniel.almeida@collabora.com>
> 

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

[Boqun Feng]

On Tue, Jul 15, 2025 at 12:16:40PM -0300, 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@collabora.com>
> ---
[...]
> diff --git a/rust/kernel/irq.rs b/rust/kernel/irq.rs
> index 9abd9a6dc36f3e3ecc1f92ad7b0040176b56a079..01bd08884b72c2a3a9460897bce751c732a19794 100644
> --- a/rust/kernel/irq.rs
> +++ b/rust/kernel/irq.rs
> @@ -12,3 +12,8 @@
>  
>  /// Flags to be used when registering IRQ handlers.
>  pub mod flags;
> +
> +/// IRQ allocation and handling.
> +pub mod request;
> +
> +pub use request::{Handler, IrqRequest, IrqReturn, Registration};

I woulde use #[doc(inline)] here for these re-export. It'll give a list
of struct/trait users can use in the `irq` module.

[...]
> +
> +/// 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 {

Missing "// INVARIANT" comment here.

> +        IrqRequest { dev, irq }
> +    }
> +
> +    /// Returns the IRQ number of an [`IrqRequest`].
> +    pub fn irq(&self) -> u32 {
> +        self.irq
> +    }
> +}
> +
> +/// A registration of an IRQ handler for a given IRQ line.
> +///
> +/// # Examples
> +///
> +/// The following is an example of using `Registration`. It uses a
> +/// [`AtomicU32`](core::sync::AtomicU32) to provide the interior mutability.

We are going to remove all usage of core::sync::Atomic* when the LKMM
atomics [1] land. You can probably use `Completion` here (handler does
complete_all(), and registration uses wait_for_completion()) because
`Completion` is irq-safe. And this brings my next comment..

> +///
> +/// ```
> +/// use core::sync::atomic::AtomicU32;
> +/// use core::sync::atomic::Ordering;
> +///
> +/// use kernel::prelude::*;
> +/// use kernel::device::Bound;
> +/// use kernel::irq::flags;
> +/// use kernel::irq::Registration;
> +/// use kernel::irq::IrqRequest;
> +/// use kernel::irq::IrqReturn;
> +/// use kernel::sync::Arc;
> +/// use kernel::c_str;
> +/// use kernel::alloc::flags::GFP_KERNEL;
> +///
> +/// // 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);
> +///
> +/// // [`kernel::irq::request::Handler::handle`] takes `&self`. This example
> +/// // illustrates how interior mutability can be used when sharing the data
> +/// // between process context and IRQ context.
> +///
> +/// type Handler = Data;
> +///
> +/// impl kernel::irq::request::Handler for Handler {
> +///     // This is executing in IRQ context in some CPU. Other CPUs can still
> +///     // try to access to data.
> +///     fn handle(&self) -> IrqReturn {
> +///         self.0.fetch_add(1, Ordering::Relaxed);
> +///
> +///         IrqReturn::Handled
> +///     }
> +/// }
> +///
> +/// // Registers an IRQ handler for the given IrqRequest.
> +/// //
> +/// // This is executing in process context and assumes that `request` was
> +/// // previously acquired from a device.
> +/// fn register_irq(handler: Handler, request: IrqRequest<'_>) -> Result<Arc<Registration<Handler>>> {
> +///     let registration = Registration::new(request, flags::SHARED, c_str!("my_device"), handler);
> +///
> +///     let registration = Arc::pin_init(registration, GFP_KERNEL)?;
> +///
> +///     // The data can be accessed from process context too.
> +///     registration.handler().0.fetch_add(1, Ordering::Relaxed);
> +///
> +///     Ok(registration)
> +/// }
> +/// # Ok::<(), Error>(())
> +/// ```
> +///
> +/// # Invariants
> +///
> +/// * We own an irq handler using `&self.handler` as its private data.
> +///
> +#[pin_data]
> +pub struct Registration<T: Handler + 'static> {
> +    #[pin]
> +    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,
> +}
> +
> +impl<T: Handler + 'static> Registration<T> {
> +    /// Registers the IRQ handler with the system for the given IRQ number.
> +    pub fn new<'a>(
> +        request: IrqRequest<'a>,
> +        flags: Flags,
> +        name: &'static CStr,
> +        handler: T,

... to use `Completion` which requires pin-init, it seems that the
current API is a bit limited, we can make this parameter be a:

	handler: impl PinInit<T, Error>

? It'll still support the current usage because we have blanket impl.

Regards,
Boqun

> +    ) -> impl PinInit<Self, Error> + 'a {
> +        try_pin_init!(&this in Self {
> +            handler,
> +            inner <- Devres::new(
> +                request.dev,
> +                try_pin_init!(RegistrationInner {
> +                    // SAFETY: `this` is a valid pointer to the `Registration` instance
> +                    cookie: unsafe { &raw mut (*this.as_ptr()).handler }.cast(),
> +                    irq: {
> +                        // SAFETY:
> +                        // - The callbacks are valid for use with request_irq.
> +                        // - If this succeeds, the slot is guaranteed to be valid until the
> +                        //   destructor of Self runs, which will deregister the callbacks
> +                        //   before the memory location becomes invalid.
> +                        to_result(unsafe {
> +                            bindings::request_irq(
> +                                request.irq,
> +                                Some(handle_irq_callback::<T>),
> +                                flags.into_inner(),
> +                                name.as_char_ptr(),
> +                                (&raw mut (*this.as_ptr()).handler).cast(),
> +                            )
> +                        })?;
> +                        request.irq
> +                    }
> +                })
> +            ),
> +            _pin: PhantomPinned,
> +        })
> +    }
[...]

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

[Boqun Feng]

On Tue, Jul 22, 2025 at 09:32:40PM -0700, Boqun Feng wrote:
[...]
> > +/// A registration of an IRQ handler for a given IRQ line.
> > +///
> > +/// # Examples
> > +///
> > +/// The following is an example of using `Registration`. It uses a
> > +/// [`AtomicU32`](core::sync::AtomicU32) to provide the interior mutability.
> 
> We are going to remove all usage of core::sync::Atomic* when the LKMM
> atomics [1] land. You can probably use `Completion` here (handler does
> complete_all(), and registration uses wait_for_completion()) because
> `Completion` is irq-safe. And this brings my next comment..
> 

Missed the link:

[1]: https://lore.kernel.org/rust-for-linux/20250719030827.61357-1-boqun.feng@gmail.com/

Regards,
Boqun

[..]

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

[Alice Ryhl]

On Wed, Jul 23, 2025 at 6:32 AM Boqun Feng <boqun.feng@gmail.com> wrote:
>
> On Tue, Jul 15, 2025 at 12:16:40PM -0300, 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@collabora.com>
> > ---
> [...]
> > diff --git a/rust/kernel/irq.rs b/rust/kernel/irq.rs
> > index 9abd9a6dc36f3e3ecc1f92ad7b0040176b56a079..01bd08884b72c2a3a9460897bce751c732a19794 100644
> > --- a/rust/kernel/irq.rs
> > +++ b/rust/kernel/irq.rs
> > @@ -12,3 +12,8 @@
> >
> >  /// Flags to be used when registering IRQ handlers.
> >  pub mod flags;
> > +
> > +/// IRQ allocation and handling.
> > +pub mod request;
> > +
> > +pub use request::{Handler, IrqRequest, IrqReturn, Registration};
>
> I woulde use #[doc(inline)] here for these re-export. It'll give a list
> of struct/trait users can use in the `irq` module.

You get the same effect by making `mod request` a private module.

Alice

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

[Boqun Feng]

On Wed, Jul 23, 2025 at 07:35:26AM +0200, Alice Ryhl wrote:
> On Wed, Jul 23, 2025 at 6:32 AM Boqun Feng <boqun.feng@gmail.com> wrote:
> >
> > On Tue, Jul 15, 2025 at 12:16:40PM -0300, 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@collabora.com>
> > > ---
> > [...]
> > > diff --git a/rust/kernel/irq.rs b/rust/kernel/irq.rs
> > > index 9abd9a6dc36f3e3ecc1f92ad7b0040176b56a079..01bd08884b72c2a3a9460897bce751c732a19794 100644
> > > --- a/rust/kernel/irq.rs
> > > +++ b/rust/kernel/irq.rs
> > > @@ -12,3 +12,8 @@
> > >
> > >  /// Flags to be used when registering IRQ handlers.
> > >  pub mod flags;
> > > +
> > > +/// IRQ allocation and handling.
> > > +pub mod request;
> > > +
> > > +pub use request::{Handler, IrqRequest, IrqReturn, Registration};
> >
> > I woulde use #[doc(inline)] here for these re-export. It'll give a list
> > of struct/trait users can use in the `irq` module.
> 
> You get the same effect by making `mod request` a private module.
> 

Oh yes, that also works! I think we probably should do that.

Regards,
Boqun

> Alice

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

[Daniel Almeida]

Hi Boqun,

[…]

>> +        IrqRequest { dev, irq }
>> +    }
>> +
>> +    /// Returns the IRQ number of an [`IrqRequest`].
>> +    pub fn irq(&self) -> u32 {
>> +        self.irq
>> +    }
>> +}
>> +
>> +/// A registration of an IRQ handler for a given IRQ line.
>> +///
>> +/// # Examples
>> +///
>> +/// The following is an example of using `Registration`. It uses a
>> +/// [`AtomicU32`](core::sync::AtomicU32) to provide the interior mutability.
> 
> We are going to remove all usage of core::sync::Atomic* when the LKMM
> atomics [1] land. You can probably use `Completion` here (handler does
> complete_all(), and registration uses wait_for_completion()) because
> `Completion` is irq-safe. And this brings my next comment..

How are completions equivalent to atomics? I am trying to highlight interior
mutability in this example.

Is the LKMM atomic series getting merged during the upcoming merge window? Because my
understanding was that the IRQ series was ready to go in 6.17, pending a reply
from Thomas and some minor comments that have been mentioned in v7.

If the LKMM series is not ready yet, my proposal is to leave the
Atomics->Completion change for a future patch (or really, to just use the new
Atomic types introduced by your series, because again, I don't think Completion
is the right thing to have there).


— Daniel

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

[Boqun Feng]

On Wed, Jul 23, 2025 at 10:55:20AM -0300, Daniel Almeida wrote:
> Hi Boqun,
> 
> [...]
> 
> >> +        IrqRequest { dev, irq }
> >> +    }
> >> +
> >> +    /// Returns the IRQ number of an [`IrqRequest`].
> >> +    pub fn irq(&self) -> u32 {
> >> +        self.irq
> >> +    }
> >> +}
> >> +
> >> +/// A registration of an IRQ handler for a given IRQ line.
> >> +///
> >> +/// # Examples
> >> +///
> >> +/// The following is an example of using `Registration`. It uses a
> >> +/// [`AtomicU32`](core::sync::AtomicU32) to provide the interior mutability.
> > 
> > We are going to remove all usage of core::sync::Atomic* when the LKMM
> > atomics [1] land. You can probably use `Completion` here (handler does
> > complete_all(), and registration uses wait_for_completion()) because
> > `Completion` is irq-safe. And this brings my next comment..
> 
> How are completions equivalent to atomics? I am trying to highlight interior
> mutability in this example.
> 

Well, `Completion` also has interior mutability.

> Is the LKMM atomic series getting merged during the upcoming merge window? Because my
> understanding was that the IRQ series was ready to go in 6.17, pending a reply

Nope, it's likely to be in 6.18.

> from Thomas and some minor comments that have been mentioned in v7.
> 
> If the LKMM series is not ready yet, my proposal is to leave the
> Atomics->Completion change for a future patch (or really, to just use the new
> Atomic types introduced by your series, because again, I don't think Completion
> is the right thing to have there).
> 

Why? I can find a few examples that an irq handler does a
complete_all(), e.g. gpi_process_ch_ctrl_irq() in
drivers/dma/qcom/gpi.c. I think it's very normal for a driver thread to
use completions to wait for an irq to happen.

But sure, this and the handler pinned initializer thing is not a blocker
issue. However, I would like to see them resolved as soon as possible
once merged.

Regards,
Boqun

> 
> - Daniel

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

[Danilo Krummrich]

On 7/23/25 4:26 PM, Boqun Feng wrote:
> On Wed, Jul 23, 2025 at 10:55:20AM -0300, Daniel Almeida wrote:
> But sure, this and the handler pinned initializer thing is not a blocker
> issue. However, I would like to see them resolved as soon as possible
> once merged.

I think it would be trivial to make the T an impl PinInit<T, E> and use a
completion as example instead of an atomic. So, we should do it right away.

- Danilo

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

[Daniel Almeida]

> On 23 Jul 2025, at 11:26, Boqun Feng <boqun.feng@gmail.com> wrote:
> 
> On Wed, Jul 23, 2025 at 10:55:20AM -0300, Daniel Almeida wrote:
>> Hi Boqun,
>> 
>> [...]
>> 
>>>> +        IrqRequest { dev, irq }
>>>> +    }
>>>> +
>>>> +    /// Returns the IRQ number of an [`IrqRequest`].
>>>> +    pub fn irq(&self) -> u32 {
>>>> +        self.irq
>>>> +    }
>>>> +}
>>>> +
>>>> +/// A registration of an IRQ handler for a given IRQ line.
>>>> +///
>>>> +/// # Examples
>>>> +///
>>>> +/// The following is an example of using `Registration`. It uses a
>>>> +/// [`AtomicU32`](core::sync::AtomicU32) to provide the interior mutability.
>>> 
>>> We are going to remove all usage of core::sync::Atomic* when the LKMM
>>> atomics [1] land. You can probably use `Completion` here (handler does
>>> complete_all(), and registration uses wait_for_completion()) because
>>> `Completion` is irq-safe. And this brings my next comment..
>> 
>> How are completions equivalent to atomics? I am trying to highlight interior
>> mutability in this example.
>> 
> 
> Well, `Completion` also has interior mutability.
> 
>> Is the LKMM atomic series getting merged during the upcoming merge window? Because my
>> understanding was that the IRQ series was ready to go in 6.17, pending a reply
> 
> Nope, it's likely to be in 6.18.
> 
>> from Thomas and some minor comments that have been mentioned in v7.
>> 
>> If the LKMM series is not ready yet, my proposal is to leave the
>> Atomics->Completion change for a future patch (or really, to just use the new
>> Atomic types introduced by your series, because again, I don't think Completion
>> is the right thing to have there).
>> 
> 
> Why? I can find a few examples that an irq handler does a
> complete_all(), e.g. gpi_process_ch_ctrl_irq() in
> drivers/dma/qcom/gpi.c. I think it's very normal for a driver thread to
> use completions to wait for an irq to happen.
> 
> But sure, this and the handler pinned initializer thing is not a blocker
> issue. However, I would like to see them resolved as soon as possible
> once merged.
> 
> Regards,
> Boqun
> 
>> 
>> - Daniel


Because it is not as explicit. The main thing we should be conveying to users
here is how to get a &mut or otherwise mutate the data when running the
handler. When people see AtomicU32, it's a quick jump to "I can make this work
by using other locks, like SpinLockIrq". Completions hide this, IMHO.

It's totally possible for someone to see this and say "ok, I can call
complete() on this, but how can I mutate the data in some random T struct?",
even though these are essentially the same thing from an interior mutability
point of view.

-- Daniel

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

[Daniel Almeida]

> On 23 Jul 2025, at 11:35, Danilo Krummrich <dakr@kernel.org> wrote:
> 
> On 7/23/25 4:26 PM, Boqun Feng wrote:
>> On Wed, Jul 23, 2025 at 10:55:20AM -0300, Daniel Almeida wrote:
>> But sure, this and the handler pinned initializer thing is not a blocker
>> issue. However, I would like to see them resolved as soon as possible
>> once merged.
> 
> I think it would be trivial to make the T an impl PinInit<T, E> and use a
> completion as example instead of an atomic. So, we should do it right away.
> 
> - Danilo


I agree that this is a trivial change to make. My point here is not to postpone
the work; I am actually somewhat against switching to completions, as per the
reasoning I provided in my latest reply to Boqun. My plan is to switch directly
to whatever will substitute AtomicU32.

The switch to impl PinInit is fine.

— Daniel

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

[Danilo Krummrich]

On Wed Jul 23, 2025 at 4:56 PM CEST, Daniel Almeida wrote:
>> On 23 Jul 2025, at 11:35, Danilo Krummrich <dakr@kernel.org> wrote:
>> On 7/23/25 4:26 PM, Boqun Feng wrote:
>>> On Wed, Jul 23, 2025 at 10:55:20AM -0300, Daniel Almeida wrote:
>>> But sure, this and the handler pinned initializer thing is not a blocker
>>> issue. However, I would like to see them resolved as soon as possible
>>> once merged.
>> 
>> I think it would be trivial to make the T an impl PinInit<T, E> and use a
>> completion as example instead of an atomic. So, we should do it right away.
>> 
>> - Danilo
>
>
> I agree that this is a trivial change to make. My point here is not to postpone
> the work; I am actually somewhat against switching to completions, as per the
> reasoning I provided in my latest reply to Boqun. My plan is to switch directly
> to whatever will substitute AtomicU32.

I mean, Boqun has a point. AFAIK, the Rust atomics are UB in the kernel.

So, this is a bit as if we would use spin_lock() instead of spin_lock_irq(),
it's just not correct. Hence, we may not want to showcase it until it's actually
resolved.

The plain truth is, currently there's no synchronization primitive for getting
interior mutability in interrupts.

You can use a normal spinlock or mutex in the threaded handler though.

And in the hard IRQ you can use a completion to indicate something has
completed.

Once we have proper atomics and spin_lock_irq() we can still change it.

> The switch to impl PinInit is fine.

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

[Alice Ryhl]

On Wed, Jul 23, 2025 at 05:03:12PM +0200, Danilo Krummrich wrote:
> On Wed Jul 23, 2025 at 4:56 PM CEST, Daniel Almeida wrote:
> >> On 23 Jul 2025, at 11:35, Danilo Krummrich <dakr@kernel.org> wrote:
> >> On 7/23/25 4:26 PM, Boqun Feng wrote:
> >>> On Wed, Jul 23, 2025 at 10:55:20AM -0300, Daniel Almeida wrote:
> >>> But sure, this and the handler pinned initializer thing is not a blocker
> >>> issue. However, I would like to see them resolved as soon as possible
> >>> once merged.
> >> 
> >> I think it would be trivial to make the T an impl PinInit<T, E> and use a
> >> completion as example instead of an atomic. So, we should do it right away.
> >> 
> >> - Danilo
> >
> >
> > I agree that this is a trivial change to make. My point here is not to postpone
> > the work; I am actually somewhat against switching to completions, as per the
> > reasoning I provided in my latest reply to Boqun. My plan is to switch directly
> > to whatever will substitute AtomicU32.
> 
> I mean, Boqun has a point. AFAIK, the Rust atomics are UB in the kernel.
> 
> So, this is a bit as if we would use spin_lock() instead of spin_lock_irq(),
> it's just not correct. Hence, we may not want to showcase it until it's actually
> resolved.
> 
> The plain truth is, currently there's no synchronization primitive for getting
> interior mutability in interrupts.

Is the actual argument here "we are getting rid of Rust atomics in the
next cycle, so please don't introduce any more users during the next
cycle because if you do it will take one cycle longer to get rid of
all Rust atomics"?

I can accept that argument. But I don't accept the argument that we
shouldn't use them here because of the UB technicality. That is an
isolated demand for rigor and I think it is unreasonable. Using Rust
atomics is an accepted workaround until the LKMM atomics land.

Alice

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

[Boqun Feng]

On Wed, Jul 23, 2025 at 11:54:09AM -0300, Daniel Almeida wrote:
> 
> 
> > On 23 Jul 2025, at 11:26, Boqun Feng <boqun.feng@gmail.com> wrote:
> > 
> > On Wed, Jul 23, 2025 at 10:55:20AM -0300, Daniel Almeida wrote:
> >> Hi Boqun,
> >> 
> >> [...]
> >> 
> >>>> +        IrqRequest { dev, irq }
> >>>> +    }
> >>>> +
> >>>> +    /// Returns the IRQ number of an [`IrqRequest`].
> >>>> +    pub fn irq(&self) -> u32 {
> >>>> +        self.irq
> >>>> +    }
> >>>> +}
> >>>> +
> >>>> +/// A registration of an IRQ handler for a given IRQ line.
> >>>> +///
> >>>> +/// # Examples
> >>>> +///
> >>>> +/// The following is an example of using `Registration`. It uses a
> >>>> +/// [`AtomicU32`](core::sync::AtomicU32) to provide the interior mutability.
> >>> 
> >>> We are going to remove all usage of core::sync::Atomic* when the LKMM
> >>> atomics [1] land. You can probably use `Completion` here (handler does
> >>> complete_all(), and registration uses wait_for_completion()) because
> >>> `Completion` is irq-safe. And this brings my next comment..
> >> 
> >> How are completions equivalent to atomics? I am trying to highlight interior
> >> mutability in this example.
> >> 
> > 
> > Well, `Completion` also has interior mutability.
> > 
> >> Is the LKMM atomic series getting merged during the upcoming merge window? Because my
> >> understanding was that the IRQ series was ready to go in 6.17, pending a reply
> > 
> > Nope, it's likely to be in 6.18.
> > 
> >> from Thomas and some minor comments that have been mentioned in v7.
> >> 
> >> If the LKMM series is not ready yet, my proposal is to leave the
> >> Atomics->Completion change for a future patch (or really, to just use the new
> >> Atomic types introduced by your series, because again, I don't think Completion
> >> is the right thing to have there).
> >> 
> > 
> > Why? I can find a few examples that an irq handler does a
> > complete_all(), e.g. gpi_process_ch_ctrl_irq() in
> > drivers/dma/qcom/gpi.c. I think it's very normal for a driver thread to
> > use completions to wait for an irq to happen.
> > 
> > But sure, this and the handler pinned initializer thing is not a blocker
> > issue. However, I would like to see them resolved as soon as possible
> > once merged.
> > 
> > Regards,
> > Boqun
> > 
> >> 
> >> - Daniel
> 
> 
> Because it is not as explicit. The main thing we should be conveying to users
> here is how to get a &mut or otherwise mutate the data when running the
> handler. When people see AtomicU32, it's a quick jump to "I can make this work
> by using other locks, like SpinLockIrq". Completions hide this, IMHO.
> 

I understand your argument. However, I'm not sure the example of
`irq::Registration` is the right place to do this. On one hand, it's one
of the usage of interior mutability as you said, but on the other hand,
for people who are familiar with interior mutability, the difference
between `AtomicU32` and `Completion` is not that much. That's kinda my
argument why using `Completion` in the example here is fine.

Sounds reasonable?

> It's totally possible for someone to see this and say "ok, I can call
> complete() on this, but how can I mutate the data in some random T struct?",
> even though these are essentially the same thing from an interior mutability
> point of view.
> 

We probably better assume that interior mutability is commmon knowledge
or we could make an link to some documentation of interior mutability,
for example [1], in the documentation of `handler`. Not saying your
effort and consideration is not valid, but at the project level,
interior mutability should be widely acknowledged IMO.

[1]: https://doc.rust-lang.org/reference/interior-mutability.html

Regards,
Boqun

> -- Daniel
> 
> 

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

[Danilo Krummrich]

On Wed Jul 23, 2025 at 5:44 PM CEST, Alice Ryhl wrote:
> On Wed, Jul 23, 2025 at 05:03:12PM +0200, Danilo Krummrich wrote:
>> On Wed Jul 23, 2025 at 4:56 PM CEST, Daniel Almeida wrote:
>> >> On 23 Jul 2025, at 11:35, Danilo Krummrich <dakr@kernel.org> wrote:
>> >> On 7/23/25 4:26 PM, Boqun Feng wrote:
>> >>> On Wed, Jul 23, 2025 at 10:55:20AM -0300, Daniel Almeida wrote:
>> >>> But sure, this and the handler pinned initializer thing is not a blocker
>> >>> issue. However, I would like to see them resolved as soon as possible
>> >>> once merged.
>> >> 
>> >> I think it would be trivial to make the T an impl PinInit<T, E> and use a
>> >> completion as example instead of an atomic. So, we should do it right away.
>> >> 
>> >> - Danilo
>> >
>> >
>> > I agree that this is a trivial change to make. My point here is not to postpone
>> > the work; I am actually somewhat against switching to completions, as per the
>> > reasoning I provided in my latest reply to Boqun. My plan is to switch directly
>> > to whatever will substitute AtomicU32.
>> 
>> I mean, Boqun has a point. AFAIK, the Rust atomics are UB in the kernel.
>> 
>> So, this is a bit as if we would use spin_lock() instead of spin_lock_irq(),
>> it's just not correct. Hence, we may not want to showcase it until it's actually
>> resolved.
>> 
>> The plain truth is, currently there's no synchronization primitive for getting
>> interior mutability in interrupts.
>
> Is the actual argument here "we are getting rid of Rust atomics in the
> next cycle, so please don't introduce any more users during the next
> cycle because if you do it will take one cycle longer to get rid of
> all Rust atomics"?

That's an argument as well, I guess.

> I can accept that argument. But I don't accept the argument that we
> shouldn't use them here because of the UB technicality. That is an
> isolated demand for rigor and I think it is unreasonable. Using Rust
> atomics is an accepted workaround until the LKMM atomics land.

I think there's a difference between actually needing them and using them to
showcase something. Or in other words, limiting workarounds to only those places
where we can't avoid them seems like the correct thing to do.

Using a completion in the hard IRQ and showing a spinlock or mutex example in
the threaded handler seems like a good mix to me.

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

[Daniel Almeida]

[…]

>> 
>> 
>> Because it is not as explicit. The main thing we should be conveying to users
>> here is how to get a &mut or otherwise mutate the data when running the
>> handler. When people see AtomicU32, it's a quick jump to "I can make this work
>> by using other locks, like SpinLockIrq". Completions hide this, IMHO.
>> 
> 
> I understand your argument. However, I'm not sure the example of
> `irq::Registration` is the right place to do this. On one hand, it's one
> of the usage of interior mutability as you said, but on the other hand,
> for people who are familiar with interior mutability, the difference
> between `AtomicU32` and `Completion` is not that much. That's kinda my
> argument why using `Completion` in the example here is fine.
> 
> Sounds reasonable?
> 
>> It's totally possible for someone to see this and say "ok, I can call
>> complete() on this, but how can I mutate the data in some random T struct?",
>> even though these are essentially the same thing from an interior mutability
>> point of view.
>> 
> 
> We probably better assume that interior mutability is commmon knowledge
> or we could make an link to some documentation of interior mutability,
> for example [1], in the documentation of `handler`. Not saying your
> effort and consideration is not valid, but at the project level,
> interior mutability should be widely acknowledged IMO.
> 
> [1]: https://doc.rust-lang.org/reference/interior-mutability.html
> 
> Regards,
> Boqun
> 
>> -- Daniel

I do expect mostly everbody (except brand-new newcomers) to be aware of
interior mutability. What I don't expect is for people _immediately_ see that
it's being used in Completion, and connect the dots from there.

Keyword here being "immediately", users will naturally realize this in a couple
of minutes at max, of course.

Anyways, I guess we can use Completion then. TBH I wasn't aware of the UB
thing, so I can see how you also have a point. On top of that, we can use the
words "interior mutability" somewhere in the example as well to make it even
clearer.

I'll change it for v8.

-- Daniel

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

[Danilo Krummrich]

On Wed Jul 23, 2025 at 6:07 PM CEST, Daniel Almeida wrote:
> On top of that, we can use the
> words "interior mutability" somewhere in the example as well to make it even
> clearer.

You *can* have this example and I encourage it, I think it is valuable. You can
have spinlock or mutex for this purpose in threaded handler, no?

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

[Daniel Almeida]

> On 23 Jul 2025, at 13:11, Danilo Krummrich <dakr@kernel.org> wrote:
> 
> On Wed Jul 23, 2025 at 6:07 PM CEST, Daniel Almeida wrote:
>> On top of that, we can use the
>> words "interior mutability" somewhere in the example as well to make it even
>> clearer.
> 
> You *can* have this example and I encourage it, I think it is valuable. You can
> have spinlock or mutex for this purpose in threaded handler, no?

Right, but then what goes in the hard-irq part for ThreadedHandler? I guess we
can leave that one blank then and only touch the data from the threaded part.

If that’s the case, then I think it can work too.

— Daniel 

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

[Danilo Krummrich]

On Wed Jul 23, 2025 at 6:18 PM CEST, Daniel Almeida wrote:
>> On 23 Jul 2025, at 13:11, Danilo Krummrich <dakr@kernel.org> wrote:
>> On Wed Jul 23, 2025 at 6:07 PM CEST, Daniel Almeida wrote:
>>> On top of that, we can use the
>>> words "interior mutability" somewhere in the example as well to make it even
>>> clearer.
>> 
>> You *can* have this example and I encourage it, I think it is valuable. You can
>> have spinlock or mutex for this purpose in threaded handler, no?
>
> Right, but then what goes in the hard-irq part for ThreadedHandler? I guess we
> can leave that one blank then and only touch the data from the threaded part.
>
> If that’s the case, then I think it can work too.

For instance, yes. It's a very common pattern to only have the threaded handler
but not the hard irq handler implemented.

IMHO, for ThreadedHandler the hard irq handler should even have a default blank
implementation.