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