Re: [PATCH v2 2/4] rust: pci: add managed Device::enable_device()

From: Maurice Hieronymus

Date: Sat Jun 20 2026 - 18:20:00 EST


On Sat, 2026-06-20 at 12:54 +0300, Onur Özkan wrote:
>
> >  impl<'a> Device<device::Core<'a>> {
> >      /// Enable memory resources for this device.
> > +    ///
> > +    /// This function is unmanaged and does not perform any
> > cleanup when the device is unbound.
> > +    /// For a managed function take a look at
> > [`Device::enable_device`].
> > +    #[inline]
> >      pub fn enable_device_mem(&self) -> Result {
> >          // SAFETY: `self.as_raw` is guaranteed to be a pointer to
> > a valid `struct pci_dev`.
> >          to_result(unsafe {
> > bindings::pci_enable_device_mem(self.as_raw()) })
> >      }
> >  
> > +    /// Enable I/O and memory resources for this device, with
> > automatic cleanup.
> > +    ///
> > +    /// This is the managed version of `pci_enable_device()`: it
> > enables the device's I/O and
> > +    /// memory resources and registers a `pci_disable_device()`
> > call that runs automatically
> > +    /// when the device is unbound from its driver. In contrast,
> > [`Device::enable_device_mem`]
> > +    /// is unmanaged and only enables memory resources.
> > +    #[inline]
> > +    pub fn enable_device(&self) -> Result {
> > +        // SAFETY: `self.as_raw` is guaranteed to be a pointer to
> > a valid `struct pci_dev`.
> > +        to_result(unsafe {
> > bindings::pcim_enable_device(self.as_raw()) })
> > +    }
> > +
>
> How about adding `_managed` and `_unmanaged` suffixes to the function
> names?
>
In general, I agree with you.

However, both functions differ slightly more than being only managed
and unmanaged. `enable_device` enables I/O and memory resources.
`enable_device_mem` only enables memory resources.

Isn't it confusing when we only have `enable_device_managed` and
`enable_device_mem_unmanaged` but not `enable_device_unmanaged` and
`enable_device_mem_managed`?

What is the general road ahead in the future? Do we want to continue to
have C-Code "magically" clean up our resources or would it be
preferable if we get some handle, which does the clean up when it is
dropped?
> - Onur
>
> >      /// Enable bus-mastering for this device.
> >      #[inline]
> >      pub fn set_master(&self) {
> >
> > --
> > 2.51.2
> >