On Sat, Jun 21, 2025 at 09:43:30PM +0200, Danilo Krummrich wrote:
> Take advantage of the generic drvdata accessors of the generic Device
> type.
>
> While at it, use from_result() instead of match.
>
> Signed-off-by: Danilo Krummrich <dakr@xxxxxxxxxx>
> ---
> rust/helpers/pci.c | 10 ----------
> rust/kernel/pci.rs | 31 ++++++++++++++-----------------
> 2 files changed, 14 insertions(+), 27 deletions(-)
>
> diff --git a/rust/helpers/pci.c b/rust/helpers/pci.c
> index cd0e6bf2cc4d..ef9cb38c81a6 100644
> --- a/rust/helpers/pci.c
> +++ b/rust/helpers/pci.c
> @@ -2,16 +2,6 @@
>
> #include <linux/pci.h>
>
> -void rust_helper_pci_set_drvdata(struct pci_dev *pdev, void *data)
> -{
> - pci_set_drvdata(pdev, data);
> -}
> -
> -void *rust_helper_pci_get_drvdata(struct pci_dev *pdev)
> -{
> - return pci_get_drvdata(pdev);
> -}
> -
> resource_size_t rust_helper_pci_resource_len(struct pci_dev *pdev, int bar)
> {
> return pci_resource_len(pdev, bar);
> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index 8435f8132e38..064e74a90904 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs
> @@ -10,11 +10,11 @@
> device_id::RawDeviceId,
> devres::Devres,
> driver,
> - error::{to_result, Result},
> + error::{from_result, to_result, Result},
> io::Io,
> io::IoRaw,
> str::CStr,
> - types::{ARef, ForeignOwnable, Opaque},
> + types::{ARef, Opaque},
> ThisModule,
> };
> use core::{
> @@ -66,35 +66,32 @@ extern "C" fn probe_callback(
> // `struct pci_dev`.
> //
> // INVARIANT: `pdev` is valid for the duration of `probe_callback()`.
> - let pdev = unsafe { &*pdev.cast::<Device<device::Core>>() };
> + let pdev = unsafe { &*pdev.cast::<Device<device::Internal>>() };
>
> // SAFETY: `DeviceId` is a `#[repr(transparent)` wrapper of `struct pci_device_id` and
> // does not add additional invariants, so it's safe to transmute.
> let id = unsafe { &*id.cast::<DeviceId>() };
> let info = T::ID_TABLE.info(id.index());
>
> - match T::probe(pdev, info) {
> - Ok(data) => {
> - // Let the `struct pci_dev` own a reference of the driver's private data.
> - // SAFETY: By the type invariant `pdev.as_raw` returns a valid pointer to a
> - // `struct pci_dev`.
> - unsafe { bindings::pci_set_drvdata(pdev.as_raw(), data.into_foreign() as _) };
> - }
> - Err(err) => return Error::to_errno(err),
> - }
> + from_result(|| {
> + let data = T::probe(pdev, info)?;
>
> - 0
> + pdev.as_ref().set_drvdata(data);
> + Ok(0)
> + })
> }
>
> extern "C" fn remove_callback(pdev: *mut bindings::pci_dev) {
> // SAFETY: The PCI bus only ever calls the remove callback with a valid pointer to a
> // `struct pci_dev`.
> - let ptr = unsafe { bindings::pci_get_drvdata(pdev) }.cast();
> + //
> + // INVARIANT: `pdev` is valid for the duration of `remove_callback()`.
> + let pdev = unsafe { &*pdev.cast::<Device<device::Internal>>() };
>
> // SAFETY: `remove_callback` is only ever called after a successful call to
> - // `probe_callback`, hence it's guaranteed that `ptr` points to a valid and initialized
> - // `KBox<T>` pointer created through `KBox::into_foreign`.
> - let _ = unsafe { KBox::<T>::from_foreign(ptr) };
> + // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called
> + // and stored a `Pin<KBox<T>>`.
> + let _ = unsafe { pdev.as_ref().drvdata_obtain::<Pin<KBox<T>>>() };
> }
> }
>
> --
> 2.49.0
>
>
Overall, I like this, same for the other bus types. But again, can't it
be part of Core?
thanks,
greg k-h
