On Sat May 30, 2026 at 3:13 AM CEST, Markus Probst via B4 Relay wrote:
> + extern "C" fn probe_callback(sdev: *mut bindings::serdev_device) ->
> kernel::ffi::c_int {
> + // SAFETY: The serial device bus only ever calls the probe callback
> with a valid pointer to
> + // a `struct serdev_device`.
> + //
> + // INVARIANT: `sdev` is valid for the duration of `probe_callback()`.
> + let sdev = unsafe {
> &*sdev.cast::<Device<device::CoreInternal<'_>>>() };
> + let info = <Self as driver::Adapter>::id_info(sdev.as_ref());
> +
> + from_result(|| {
> + let private_data = devres::register(
> + sdev.as_ref(),
> + try_pin_init!(PrivateData {
> + probe_complete <- Completion::new(),
> + error: false.into(),
> + }),
> + GFP_KERNEL,
> + )?;
> +
> + // SAFETY: `sdev.as_raw()` is guaranteed to be a valid pointer
> to `serdev_device`.
> + unsafe {
> + (*sdev.as_raw()).rust_private_data =
> + (&raw const *private_data).cast::<c_void>().cast_mut()
> + };
> +
> + // SAFETY: `sdev.as_raw()` is guaranteed to be a valid pointer
> to `serdev_device`.
> + unsafe { bindings::serdev_device_set_client_ops(sdev.as_raw(),
> Self::OPS) };
> +
> + // SAFETY: The serial device bus only ever calls the probe
> callback with a valid pointer
> + // to a `serdev_device`.
> + to_result(unsafe {
> + bindings::devm_serdev_device_open(sdev.as_ref().as_raw(),
> sdev.as_raw())
> + })?;
The bus device private data drops before devres callbacks run, thus you can't
use the devm helper here. I suggest to use serdev_device_open() and call
serdev_device_close() from remove_callback() instead.
You may also want to consider whether you potentially want T::unbind() to allow
to have concurrent transmits in flight regardless. IOW, you may want to call
serdev_device_close() before T::unbind() anyway.
> +
> + let data = T::probe(sdev, info);
> + let result = sdev.as_ref().set_drvdata(data);
> +
> + // SAFETY: We have exclusive access to `private_data.error`.
> + unsafe { *private_data.error.get() = result.is_err() };
> +
> + private_data.probe_complete.complete_all();
> +
> + result.map(|()| 0)
> + })
> + }
> +
> + extern "C" fn remove_callback(sdev: *mut bindings::serdev_device) {
> + // SAFETY: The serial device bus only ever calls the remove callback
> with a valid pointer
> + // to a `struct serdev_device`.
> + //
> + // INVARIANT: `sdev` is valid for the duration of
> `remove_callback()`.
> + let sdev = unsafe {
> &*sdev.cast::<Device<device::CoreInternal<'_>>>() };
> +
> + // SAFETY: `remove_callback` is only ever called after a successful
> call to
> + // `probe_callback`, hence it's guaranteed that
> `Device::set_drvdata()` has been called
> + // and stored a `Pin<KBox<T::Data<'_>>>`.
> + let data = unsafe { sdev.as_ref().drvdata_borrow::<T::Data<'_>>() };
> +
> + T::unbind(sdev, data);
> + }
> +
> + extern "C" fn receive_buf_callback(
> + sdev: *mut bindings::serdev_device,
> + buf: *const u8,
> + length: usize,
> + ) -> usize {
> + // SAFETY: The serial device bus only ever calls the receive buf
> callback with a valid
> + // pointer to a `struct serdev_device`.
> + //
> + // INVARIANT: `sdev` is valid for the duration of
> `receive_buf_callback()`.
> + let sdev = unsafe {
> &*sdev.cast::<Device<device::CoreInternal<'_>>>() };
CoreInternal dereferences to Core, which is technically unsound within this
callback.
I think you want CoreInternal for drvdata_borrow(), so we should add a
BoundInternal type state for this; I will send a patch for this.
With this and the above fixed,
Reviewed-by: Danilo Krummrich <[email protected]>
from the driver-core side of things.
> + // SAFETY:
> + // - The serial device bus only ever calls the receive buf callback
> with a valid pointer to
> + // a `struct serdev_device`.
> + // - `receive_buf_callback` is only ever called after a successful
> call to
> + // `probe_callback`, hence it's guaranteed that
> `sdev.private_data` is a pointer
> + // to a valid `PrivateData`.
> + let private_data = unsafe {
> &*(*sdev.as_raw()).rust_private_data.cast::<PrivateData>() };
> +
> + private_data.probe_complete.wait_for_completion();
> +
> + // SAFETY: No one has exclusive access to `private_data.error`.
> + if unsafe { *private_data.error.get() } {
> + return length;
> + }
> +
> + // SAFETY: `receive_buf_callback` is only ever called after a
> successful call to
> + // `probe_callback`, hence it's guaranteed that
> `Device::set_drvdata()` has been called
> + // and stored a `Pin<KBox<T::Data<'_>>>`.
> + let data = unsafe { sdev.as_ref().drvdata_borrow::<T::Data<'_>>() };
> +
> + // SAFETY: `buf` is guaranteed to be non-null and has the size of
> `length`.
> + let buf = unsafe { core::slice::from_raw_parts(buf, length) };
> +
> + T::receive(sdev, data, buf)
> + }
> +}
