Message-ID: <87349seqsj.fsf@t14s.mail-host-address-is-not-set>
Date: Fri, 15 Aug 2025 14:02:36 +0200
From: Andreas Hindborg <a.hindborg@...nel.org>
To: Daniel Almeida <daniel.almeida@...labora.com>, Miguel Ojeda
 <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>, Boqun Feng
 <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>, Björn
 Roy Baron
 <bjorn3_gh@...tonmail.com>, Alice Ryhl <aliceryhl@...gle.com>, Trevor
 Gross <tmgross@...ch.edu>, Danilo Krummrich <dakr@...nel.org>, Greg
 Kroah-Hartman <gregkh@...uxfoundation.org>, "Rafael J. Wysocki"
 <rafael@...nel.org>, Thomas Gleixner <tglx@...utronix.de>, Bjorn Helgaas
 <bhelgaas@...gle.com>, Krzysztof Wilczyński
 <kwilczynski@...nel.org>,
 Benno Lossin <lossin@...nel.org>
Cc: linux-kernel@...r.kernel.org, rust-for-linux@...r.kernel.org,
 linux-pci@...r.kernel.org, Joel Fernandes <joelagnelf@...dia.com>, Dirk
 Behme <dirk.behme@...bosch.com>, Daniel Almeida
 <daniel.almeida@...labora.com>
Subject: Re: [PATCH v9 2/7] rust: irq: add flags module

"Daniel Almeida" <daniel.almeida@...labora.com> writes:

> 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@...gle.com>
> Tested-by: Joel Fernandes <joelagnelf@...dia.com>
> Tested-by: Dirk Behme <dirk.behme@...bosch.com>
> Signed-off-by: Daniel Almeida <daniel.almeida@...labora.com>
> ---
>  rust/kernel/irq.rs       |   5 ++
>  rust/kernel/irq/flags.rs | 124 +++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 129 insertions(+)
>
> diff --git a/rust/kernel/irq.rs b/rust/kernel/irq.rs
> index fae7b15effc80c936d6bffbd5b4150000d6c2898..068df2fea31de51115c30344f7ebdb4da4ad86cc 100644
> --- a/rust/kernel/irq.rs
> +++ b/rust/kernel/irq.rs
> @@ -9,3 +9,8 @@
>  //! 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.
> +mod flags;
> +
> +pub use flags::Flags;
> diff --git a/rust/kernel/irq/flags.rs b/rust/kernel/irq/flags.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..e62820ea67755123b4f96e4331244bbb4fbcfd9d
> --- /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

`rustdoc` will complain about this being undefined.

> +/// [`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 [`Flags::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 [`Flags::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`.

Should we link `PERCPU` here?

> +    pub const NO_DEBUG: Flags = Flags::new(bindings::IRQF_NO_DEBUG);
> +
> +    pub(crate) fn into_inner(self) -> c_ulong {

You need `#[expect(dead_code)]` here.

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

I am curious about this line. Can you add a short explanation?

I would think this can never assert. That would require c_ulong to be
less than 32 bits, right? Are there any configurations where that is the case?


Best regards,
Andreas Hindborg

Powered by blists - more mailing lists