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

Boqun Feng
2025-07-14T06:42:17Z

On Sun, Jul 13, 2025 at 02:42:41PM +0200, Danilo Krummrich wrote:
[...]
> If we use container_of!() instead or just pass the address of Self (i.e.
> Registration) to request_irq() instead,
>
> pub fn device(&self) -> &Device
>
> is absolutely possible to add to Devres, of course.
>

One thing to notice is that in `Devres::new()`, `inner` is initialized
before `dev`:

try_pin_init!(&this in Self {
// INVARIANT: `inner` is properly initialized.
inner <- Opaque::pin_init(try_pin_init!(Inner {
data <- Revocable::new(data),
devm <- Completion::new(),
revoke <- Completion::new(),
})),

For `irq::Registration`, request_irq() is called at `inner`
initialization. So now interrupts can happen at any moment, but `dev` is
still uninitialized.

callback,
dev: {
// SAFETY: `this` is a valid pointer to uninitialized memory.
let inner = unsafe { &raw mut (*this.as_ptr()).inner };

// SAFETY:
// - `dev.as_raw()` is a pointer to a valid bound device.
// - `inner` is guaranteed to be a valid for the duration of the lifetime of `Self`.
// - `devm_add_action()` is guaranteed not to call `callback` until `this` has been
// properly initialized, because we require `dev` (i.e. the *bound* device) to
// live at least as long as the returned `impl PinInit<Self, Error>`.
to_result(unsafe {
bindings::devm_add_action(dev.as_raw(), Some(callback), inner.cast())
})?;

dev.into()
},
})

I think you need to reorder the initialization of `inner` to be after
`dev` for this. And it should be fine, because the whole device is in
bound state while the PinInit being called, so `inner.cast()` being a
pointer to uninitialized memory should be fine (because the `callback`
won't be called).

Regards,
Boqun

> > Depending on how (1) is ensured, we might just need an unsafe function
> > that turns `Device<Normal>` into `Device<Bound>`.
>
> `&Device<Normal>` in `&Device<Bound>`, yes. I have such a method locally
> already (but haven't sent it yet), because that's going to be a use-case for
> other abstractions as well. One specific example is the PWM Chip abstraction
> [1].
>
> [1] https://lore.kernel.org/lkml/20250710-rust-next-pwm-working-fan-for-sending-v11-3-93824a16f9ec@samsung.com/