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

[Onur Özkan]

On Sat, 20 Jun 2026 10:45:46 +0200
Maurice Hieronymus <mhi@xxxxxxxxxxx> wrote:

> Add a managed counterpart to Device::enable_device_mem() that wraps
> pcim_enable_device(). In addition to enabling the device, it registers a
> pci_disable_device() cleanup that runs automatically when the device is
> unbound from its driver, keeping the device's enable count balanced
> across unbind/rebind cycles.
> 
> The existing enable_device_mem() wraps the unmanaged
> pci_enable_device_mem() and has no disable counterpart, so the enable
> count is leaked on unbind. On the next probe pci_enable_device_flags()
> sees a non-zero enable count and returns early, skipping the power-state
> transition back to D0. For a device without a PCI power management
> capability the power state cannot be re-read from hardware and stays
> PCI_UNKNOWN, which makes __pci_enable_msi_range() reject the subsequent
> MSI allocation with -EINVAL.
> 
> Furthermore, make `enable_device_mem` inline and add a reference to
> `enable_device` in the docs.
> 
> Reviewed-by: Fiona Behrens <me@xxxxxxxxxx>
> Signed-off-by: Maurice Hieronymus <mhi@xxxxxxxxxxx>
> ---
>  rust/kernel/pci.rs | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index 5071cae6543f..d076a3691091 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs
> @@ -454,11 +454,27 @@ pub fn pci_class(&self) -> Class {
>  
>  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?

- Onur

>      /// Enable bus-mastering for this device.
>      #[inline]
>      pub fn set_master(&self) {
> 
> -- 
> 2.51.2
>