[PATCH v9 0/7] rust: add support for request_irq

Daniel Almeida posted 7 patches 1 month, 3 weeks ago
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              |  24 ++
rust/kernel/irq/flags.rs        | 124 ++++++++++
rust/kernel/irq/request.rs      | 507 ++++++++++++++++++++++++++++++++++++++++
rust/kernel/lib.rs              |   1 +
rust/kernel/pci.rs              |  45 +++-
rust/kernel/platform.rs         | 142 +++++++++++
10 files changed, 860 insertions(+), 2 deletions(-)
[PATCH v9 0/7] rust: add support for request_irq
Posted by Daniel Almeida 1 month, 3 weeks ago
Changes in v9:
- Added more tags (thanks, Alice!)
- Added Alice's patch for &dev: Device<Bound> support
- Applied a diff to account for the latest review comment on the patch
  above
- Added #[pin_data] as applicable to the examples
- Got rid of the "Handler" type alias in the examples
- Removed the leading "#" from the imports in the examples so that they show
  up in the docs
- Made all inner modules private, removed #[doc(inline)] from the
  re-exports
- Link to v8: https://lore.kernel.org/r/20250810-topics-tyr-request_irq2-v8-0-8163f4c4c3a6@collabora.com

Changes in v8:
- Rebased on top of v6.17-rc1
- Used Completion instead of Atomics in the examples for non-threaded IRQs
  (Boqun)
- Take in "impl PinInit<T, Error>" instead of T in
  [Threaded]Registration::new() (Boqun)
- Propagated the changes above to the platform/pci accessors.
- Used a Mutex instead of Atomics in the examples for threaded IRQs.
- Added more links in the docs as appropriate (Alice)
- Re-exported irq::flags::Flags through a "pub use" (Alice).
- Note: left the above as optional  as it does not hurt to specify the full
  path anyway. As a result, no modules were made private.
- Added #[doc(inline)] as appropriate to the re-exports (Boqun).
- Formatted all the examples using nightly rustfmt +
  "format_code_in_doc_comments"
- Fixed a few issues pointed out by make rustdoc
- Merged imports (Alice)
- Defaulted ThreadedIrqHandler::handle() to WakeThread (Danilo)
- Added tags (thanks, Joel & Dirk!)
- Link to v7: https://lore.kernel.org/r/20250715-topics-tyr-request_irq2-v7-0-d469c0f37c07@collabora.com

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/

---
Alice Ryhl (1):
      rust: irq: add &Device<Bound> argument to irq callbacks

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              |  24 ++
 rust/kernel/irq/flags.rs        | 124 ++++++++++
 rust/kernel/irq/request.rs      | 507 ++++++++++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs              |   1 +
 rust/kernel/pci.rs              |  45 +++-
 rust/kernel/platform.rs         | 142 +++++++++++
 10 files changed, 860 insertions(+), 2 deletions(-)
---
base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
change-id: 20250712-topics-tyr-request_irq2-ae7ee9b85854

Best regards,
-- 
Daniel Almeida <daniel.almeida@collabora.com>
Re: [PATCH v9 0/7] rust: add support for request_irq
Posted by Danilo Krummrich 1 month, 3 weeks ago
On Mon Aug 11, 2025 at 6:03 PM CEST, Daniel Almeida wrote:

Applied to driver-core-testing, thanks!

> Alice Ryhl (1):
>       rust: irq: add &Device<Bound> argument to irq callbacks
>
> Daniel Almeida (6):
>       rust: irq: add irq module
>       rust: irq: add flags module

    [ Use expect(dead_code) for into_inner(), fix broken intra-doc link and
      typo. - Danilo ]

>       rust: irq: add support for non-threaded IRQs and handlers

    [ Remove expect(dead_code) from Flags::into_inner(), add
      expect(dead_code) to IrqRequest::new(), fix intra-doc links. - Danilo ]

>       rust: irq: add support for threaded IRQs and handlers

    [ Add now available intra-doc links back in. - Danilo ]

>       rust: platform: add irq accessors

    [ Remove expect(dead_code) from IrqRequest::new(), re-format macros and
      macro invocations to not exceed 100 characters line length. - Danilo ]

>       rust: pci: add irq accessors
Re: [PATCH v9 0/7] rust: add support for request_irq
Posted by Andreas Hindborg 1 month, 2 weeks ago
"Danilo Krummrich" <dakr@kernel.org> writes:

> On Mon Aug 11, 2025 at 6:03 PM CEST, Daniel Almeida wrote:
>
> Applied to driver-core-testing, thanks!
>
>> Alice Ryhl (1):
>>       rust: irq: add &Device<Bound> argument to irq callbacks
>>
>> Daniel Almeida (6):
>>       rust: irq: add irq module
>>       rust: irq: add flags module
>
>     [ Use expect(dead_code) for into_inner(), fix broken intra-doc link and
>       typo. - Danilo ]
>
>>       rust: irq: add support for non-threaded IRQs and handlers
>
>     [ Remove expect(dead_code) from Flags::into_inner(), add
>       expect(dead_code) to IrqRequest::new(), fix intra-doc links. - Danilo ]
>
>>       rust: irq: add support for threaded IRQs and handlers
>
>     [ Add now available intra-doc links back in. - Danilo ]
>
>>       rust: platform: add irq accessors
>
>     [ Remove expect(dead_code) from IrqRequest::new(), re-format macros and
>       macro invocations to not exceed 100 characters line length. - Danilo ]
>
>>       rust: pci: add irq accessors

I somehow missed that you already applied this, so I just sent comments
for patch 3. I think there are some issues. If you want my comments for
the rest of the series, let me know. Otherwise I'll just skip that.


Best regards,
Andreas Hindborg
Re: [PATCH v9 0/7] rust: add support for request_irq
Posted by Danilo Krummrich 1 month, 2 weeks ago
On 8/18/25 10:23 AM, Andreas Hindborg wrote:
> "Danilo Krummrich" <dakr@kernel.org> writes:
> 
>> On Mon Aug 11, 2025 at 6:03 PM CEST, Daniel Almeida wrote:
>>
>> Applied to driver-core-testing, thanks!
>>
>>> Alice Ryhl (1):
>>>        rust: irq: add &Device<Bound> argument to irq callbacks
>>>
>>> Daniel Almeida (6):
>>>        rust: irq: add irq module
>>>        rust: irq: add flags module
>>
>>      [ Use expect(dead_code) for into_inner(), fix broken intra-doc link and
>>        typo. - Danilo ]
>>
>>>        rust: irq: add support for non-threaded IRQs and handlers
>>
>>      [ Remove expect(dead_code) from Flags::into_inner(), add
>>        expect(dead_code) to IrqRequest::new(), fix intra-doc links. - Danilo ]
>>
>>>        rust: irq: add support for threaded IRQs and handlers
>>
>>      [ Add now available intra-doc links back in. - Danilo ]
>>
>>>        rust: platform: add irq accessors
>>
>>      [ Remove expect(dead_code) from IrqRequest::new(), re-format macros and
>>        macro invocations to not exceed 100 characters line length. - Danilo ]
>>
>>>        rust: pci: add irq accessors
> 
> I somehow missed that you already applied this, so I just sent comments
> for patch 3. I think there are some issues. If you want my comments for
> the rest of the series, let me know. Otherwise I'll just skip that.

Sure, please do -- I'm sure Daniel is willing to send patches for any
improvements that may arise.
Re: [PATCH v9 0/7] rust: add support for request_irq
Posted by Daniel Almeida 1 month, 3 weeks ago
Hi Danilo,

> On 12 Aug 2025, at 16:07, Danilo Krummrich <dakr@kernel.org> wrote:
> 
> On Mon Aug 11, 2025 at 6:03 PM CEST, Daniel Almeida wrote:
> 
> Applied to driver-core-testing, thanks!

Awesome, thanks everyone involved here :)

> 
>> Alice Ryhl (1):
>>      rust: irq: add &Device<Bound> argument to irq callbacks
>> 
>> Daniel Almeida (6):
>>      rust: irq: add irq module
>>      rust: irq: add flags module
> 
>    [ Use expect(dead_code) for into_inner(), fix broken intra-doc link and
>      typo. - Danilo ]

Right, I’ll consider inter-patch issues like this in the future, sorry.

[…]

> 
>    [ Remove expect(dead_code) from IrqRequest::new(), re-format macros and
>      macro invocations to not exceed 100 characters line length. - Danilo ]
> 
>>      rust: pci: add irq accessors
> 

How? rustfmt doesn’t format this, so I assume that you just manually
wrapped the lines in a suitable way?

— Daniel
Re: [PATCH v9 0/7] rust: add support for request_irq
Posted by Danilo Krummrich 1 month, 3 weeks ago
On Tue Aug 12, 2025 at 9:17 PM CEST, Daniel Almeida wrote:
> How? rustfmt doesn’t format this, so I assume that you just manually
> wrapped the lines in a suitable way?

Yes, exactly.