All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joel Fernandes <joelagnelf@nvidia.com>
To: Daniel Almeida <daniel.almeida@collabora.com>
Cc: "Miguel Ojeda" <ojeda@kernel.org>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Krzysztof Wilczyński" <kwilczynski@kernel.org>,
	"Benno Lossin" <lossin@kernel.org>,
	linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org,
	linux-pci@vger.kernel.org
Subject: Re: [PATCH v7 0/6] rust: add support for request_irq
Date: Tue, 22 Jul 2025 10:52:06 -0400	[thread overview]
Message-ID: <20250722145206.GA1474658@joelnvbox> (raw)
In-Reply-To: <20250715-topics-tyr-request_irq2-v7-0-d469c0f37c07@collabora.com>

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>
> 

      parent reply	other threads:[~2025-07-22 14:52 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-15 15:16 [PATCH v7 0/6] rust: add support for request_irq Daniel Almeida
2025-07-15 15:16 ` [PATCH v7 1/6] rust: irq: add irq module Daniel Almeida
2025-07-15 15:16 ` [PATCH v7 2/6] rust: irq: add flags module Daniel Almeida
2025-07-16 14:20   ` Daniel Almeida
2025-07-16 14:45     ` Alice Ryhl
2025-07-15 15:16 ` [PATCH v7 3/6] rust: irq: add support for non-threaded IRQs and handlers Daniel Almeida
2025-07-16 23:45   ` kernel test robot
2025-07-17 16:20     ` Daniel Almeida
2025-07-21 14:17       ` Alice Ryhl
2025-07-21 14:45   ` Alice Ryhl
2025-07-21 15:10     ` Daniel Almeida
2025-07-21 15:28       ` Danilo Krummrich
2025-07-21 15:39         ` Daniel Almeida
2025-07-23  4:32   ` Boqun Feng
2025-07-23  4:57     ` Boqun Feng
2025-07-23  5:35     ` Alice Ryhl
2025-07-23 13:51       ` Boqun Feng
2025-07-23 13:55     ` Daniel Almeida
2025-07-23 14:26       ` Boqun Feng
2025-07-23 14:35         ` Danilo Krummrich
2025-07-23 14:56           ` Daniel Almeida
2025-07-23 15:03             ` Danilo Krummrich
2025-07-23 15:44               ` Alice Ryhl
2025-07-23 15:52                 ` Danilo Krummrich
2025-07-23 14:54         ` Daniel Almeida
2025-07-23 15:50           ` Boqun Feng
2025-07-23 16:07             ` Daniel Almeida
2025-07-23 16:11               ` Danilo Krummrich
2025-07-23 16:18                 ` Daniel Almeida
2025-07-23 21:31                   ` Danilo Krummrich
2025-07-15 15:16 ` [PATCH v7 4/6] rust: irq: add support for threaded " Daniel Almeida
2025-07-21 14:48   ` Alice Ryhl
2025-07-15 15:16 ` [PATCH v7 5/6] rust: platform: add irq accessors Daniel Almeida
2025-07-15 15:16 ` [PATCH v7 6/6] rust: pci: " Daniel Almeida
2025-07-15 15:32 ` [PATCH v7 0/6] rust: add support for request_irq Danilo Krummrich
2025-07-22 11:41 ` Dirk Behme
2025-07-22 14:52 ` Joel Fernandes [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250722145206.GA1474658@joelnvbox \
    --to=joelagnelf@nvidia.com \
    --cc=a.hindborg@kernel.org \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=bhelgaas@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=gary@garyguo.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=kwilczynski@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rafael@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=tmgross@umich.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.