Re: [PATCH v2 4/4] rust: samples: add EDU PCI driver sample

[Ewan Chorynski]

Hi, 

On Fri Jul 3, 2026 at 11:12 PM CEST, Danilo Krummrich wrote:
> On Sun Jun 21, 2026 at 2:27 PM CEST, Danilo Krummrich wrote:
>> I'm going to send a patch to make the irq::Registration compatible with the
>> device driver lifetime rework soon. Please rebase onto that once it's sent, so
>> this sample can land as idiomatic as possible.
>
> Please find the patch in [1]; purely mechanical conversion of the EDU driver for
> testing purposes in [2].
>
> [1] https://lore.kernel.org/driver-core/20260703210936.1128698-1-dakr@xxxxxxxxxx/
>
> [2]
>
> diff --git a/samples/rust/rust_driver_edu.rs b/samples/rust/rust_driver_edu.rs
> index 5f4efd514032..301b780773cb 100644
> --- a/samples/rust/rust_driver_edu.rs
> +++ b/samples/rust/rust_driver_edu.rs
> @@ -6,7 +6,6 @@
>

<snip>

> @@ -275,76 +279,61 @@ fn probe<'bound>(
>                  .alloc_irq_vectors(1, 1, IrqTypes::default().with(pci::IrqType::Msi))
>                  .inspect_err(|e| dev_err!(pdev, "alloc_irq_vectors failed: {:?}\n", e))?;
>
> -            // State shared with the IRQ handler (the BAR and the completion the
> -            // handler signals) lives in an `Arc<EduDriverData>`. `EduDriverData`
> -            // itself implements `irq::Handler`, and the registration takes an
> -            // `Arc<T>` via the `impl Handler for Arc<T>` blanket impl. This keeps
> -            // the handler's state out of `EduDriver` and avoids a self-reference.
> -            let data = Arc::pin_init(
> -                try_pin_init!(EduDriverData {
> -                    bar <- pdev
> -                        .iomap_region_sized(0, c"rust_driver_edu")
> -                        .and_then(|bar| bar.into_devres()),
> -                    irq_test_completion <- Completion::new(),
> -                    irq_dma_completion <- Completion::new(),
> -                    dma: ca,
> -                }),
> -                GFP_KERNEL,
> -            )?;
> -
> -            let req = irq::Registration::new(
> -                (*irq.start()).try_into()?,
> -                Flags::TRIGGER_NONE,
> -                c"rust_edu_irq",
> -                Ok(data.clone()),
> -            );
> +            let bar = pdev.iomap_region_sized::<{ regs::END }>(0, c"rust_driver_edu")?;
>
> -            // Ordering matters: the handler is registered (`irq_handler <- req`)
> -            // *before* the `_:` block runs the self-tests, one of which raises an
> -            // interrupt and waits for the handler. Raising before the handler is
> -            // registered would hang (the completion is never signalled).
> -            Ok(try_pin_init!(Self {
> +            // SAFETY: The Registration is not leaked.
This comment uses the knowledge that the initializer returned by probe
won't create and leak the registration. Shouldn't the probe function be
unsafe to formalize this guarantee ? 

> +            let req = unsafe {
> +                irq::Registration::new(
> +                    (*irq.start()).try_into()?,
> +                    Flags::TRIGGER_NONE,
> +                    c"rust_edu_irq",
> +                    try_pin_init!(IrqHandler {
> +                        pdev,
> +                        bar,
> +                        irq_test_completion <- Completion::new(),
> +                        irq_dma_completion <- Completion::new(),
> +                        dma: ca,
> +                    }? Error),
> +                )
> +            };
> +
> +            Ok(try_pin_init!(EduDriverData {
>                  irq_handler <- req,
> -                // Side-effect block: run the staged self-tests against the mapped
> -                // BAR now that the handler is live. A failure here aborts probe.
> +                // Ordering matters: the handler is registered (`irq_handler <- req`)
> +                // *before* the `_:` block runs the self-tests, one of which raises an
> +                // interrupt and waits for the handler. Raising before the handler is
> +                // registered would hang (the completion is never signalled).
>                  _: {
> -                    let bar = data.bar.access(pdev.as_ref())?;
> -                    EduDriver::init(pdev, bar, &data)?;
> +                    let handler = irq_handler.handler();
> +                    EduDriver::init(pdev, &handler.bar, handler)?;
>                      dev_info!(
>                          pdev,
>                          "rust_driver_edu successfully initialized\n",
>                      );
>                  },
> -                data,
> -                pdev: pdev.into()
> +                pdev,
>              }))
>          })
>      }
>  }

Regards,
Ewan