Re: [RFC PATCH v2 4/5] rust: add RTC core abstractions and data structures

["Danilo Krummrich" <dakr@kernel.org>]

On Wed Jan 7, 2026 at 3:37 PM CET, Ke Sun wrote:
> +/// A Rust wrapper for the C `struct rtc_device`.
> +///
> +/// This type provides safe access to RTC device operations. The underlying `rtc_device`
> +/// is managed by the kernel and remains valid for the lifetime of the `RtcDevice`.
> +///
> +/// # Invariants
> +///
> +/// A [`RtcDevice`] instance holds a pointer to a valid [`struct rtc_device`] that is
> +/// registered and managed by the kernel.
> +///
> +/// # Examples
> +///
> +/// ```rust
> +/// # use kernel::{
> +/// #     prelude::*,
> +/// #     rtc::RtcDevice, //
> +/// # };
> +/// // Example: Set the time range for the RTC device
> +/// // rtc.set_range_min(0);
> +/// // rtc.set_range_max(u64::MAX);
> +/// //     Ok(())
> +/// // }

This example looks pretty odd, and I don't think it does compile. Did you test
with CONFIG_RUST_KERNEL_DOCTESTS=y?

> +/// ```
> +///
> +/// [`struct rtc_device`]: https://docs.kernel.org/driver-api/rtc.html
> +#[repr(transparent)]
> +pub struct RtcDevice<T: 'static = ()>(Opaque<bindings::rtc_device>, PhantomData<T>);
> +
> +impl<T: 'static> RtcDevice<T> {
> +    /// Obtain the raw [`struct rtc_device`] pointer.
> +    #[inline]
> +    pub fn as_raw(&self) -> *mut bindings::rtc_device {
> +        self.0.get()
> +    }
> +
> +    /// Set the minimum time range for the RTC device.
> +    #[inline]
> +    pub fn set_range_min(&self, min: i64) {
> +        // SAFETY: By the type invariants, self.as_raw() is a valid pointer to a
> +        // `struct rtc_device`, and we're only writing to the `range_min` field.
> +        unsafe {
> +            (*self.as_raw()).range_min = min;
> +        }
> +    }
> +
> +    /// Set the maximum time range for the RTC device.
> +    #[inline]
> +    pub fn set_range_max(&self, max: u64) {
> +        // SAFETY: By the type invariants, self.as_raw() is a valid pointer to a
> +        // `struct rtc_device`, and we're only writing to the `range_max` field.
> +        unsafe {
> +            (*self.as_raw()).range_max = max;
> +        }
> +    }
> +
> +    /// Get the minimum time range for the RTC device.
> +    #[inline]
> +    pub fn range_min(&self) -> i64 {
> +        // SAFETY: By the type invariants, self.as_raw() is a valid pointer to a
> +        // `struct rtc_device`, and we're only reading the `range_min` field.
> +        unsafe { (*self.as_raw()).range_min }
> +    }
> +
> +    /// Get the maximum time range for the RTC device.
> +    #[inline]
> +    pub fn range_max(&self) -> u64 {
> +        // SAFETY: By the type invariants, self.as_raw() is a valid pointer to a
> +        // `struct rtc_device`, and we're only reading the `range_max` field.
> +        unsafe { (*self.as_raw()).range_max }
> +    }
> +
> +    /// Notify the RTC framework that an interrupt has occurred.
> +    ///
> +    /// Should be called from interrupt handlers. Schedules work to handle the interrupt
> +    /// in process context.
> +    #[inline]
> +    pub fn update_irq(&self, num: usize, events: usize) {
> +        // SAFETY: By the type invariants, self.as_raw() is a valid pointer to a
> +        // `struct rtc_device`. The rtc_update_irq function handles NULL/ERR checks internally.
> +        unsafe {
> +            bindings::rtc_update_irq(self.as_raw(), num, events);
> +        }
> +    }
> +
> +    /// Clear a feature bit in the RTC device.
> +    #[inline]
> +    pub fn clear_feature(&self, feature: u32) {
> +        // SAFETY: By the type invariants, self.as_raw() is a valid pointer to a
> +        // `struct rtc_device`, and features is a valid bitmap array with RTC_FEATURE_CNT bits.
> +        let features_bitmap = unsafe {
> +            Bitmap::from_raw_mut(
> +                (*self.as_raw()).features.as_mut_ptr().cast::<usize>(),
> +                bindings::RTC_FEATURE_CNT as usize,
> +            )
> +        };
> +        features_bitmap.clear_bit(feature as usize);
> +    }
> +}
> +
> +impl<T: 'static, Ctx: device::DeviceContext> AsRef<device::Device<Ctx>> for RtcDevice<T> {

This should just be

	impl<T: 'static> AsRef<device::Device> for RtcDevice<T>

as class devices do not have a device context.

> +    fn as_ref(&self) -> &device::Device<Ctx> {
> +        let raw = self.as_raw();
> +        // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid
> +        // `struct rtc_device`.
> +        let dev = unsafe { &raw mut (*raw).dev };
> +
> +        // SAFETY: `dev` points to a valid `struct device`.
> +        unsafe { device::Device::from_raw(dev) }
> +    }
> +}
> +
> +// SAFETY: `RtcDevice` is a transparent wrapper of `struct rtc_device`.
> +// The offset is guaranteed to point to a valid device field inside `RtcDevice`.
> +unsafe impl<T: 'static, Ctx: device::DeviceContext> device::AsBusDevice<Ctx> for RtcDevice<T> {
> +    const OFFSET: usize = core::mem::offset_of!(bindings::rtc_device, dev);
> +}

Please do not abuse this trait as container_of!(), as the name implies, it is
only for bus devices (hence also the device context generic). RTC devices are
class devices.

> +impl<T: RtcOps> RtcDevice<T> {
> +    /// Allocates a new RTC device managed by devres.
> +    ///
> +    /// This function allocates an RTC device and sets the driver data. The device will be
> +    /// automatically freed when the parent device is removed.
> +    pub fn new(
> +        parent_dev: &device::Device,

This must be a &Device<Bound>, otherwise you are not allowed to pass it to
devm_rtc_allocate_device().

> +        init: impl PinInit<T, Error>,
> +    ) -> Result<ARef<Self>> {
> +        // SAFETY: `Device<Bound>` and `Device<CoreInternal>` have the same layout.
> +        let dev_internal: &device::Device<device::CoreInternal> =
> +            unsafe { &*core::ptr::from_ref(parent_dev).cast() };
> +
> +        // Allocate RTC device.
> +        // SAFETY: `devm_rtc_allocate_device` returns a pointer to a devm-managed rtc_device.
> +        // We use `dev_internal.as_raw()` which is `pub(crate)`, but we can access it through
> +        // the same device pointer.
> +        let rtc: *mut bindings::rtc_device =
> +            unsafe { bindings::devm_rtc_allocate_device(dev_internal.as_raw()) };
> +        if rtc.is_null() {
> +            return Err(ENOMEM);
> +        }
> +
> +        // Set the RTC device ops.
> +        // SAFETY: We just allocated the RTC device, so it's safe to set the ops.
> +        unsafe {
> +            (*rtc).ops = Adapter::<T>::VTABLE.as_raw();
> +        }
> +
> +        // SAFETY: `rtc` is a valid pointer to a newly allocated rtc_device.
> +        // `RtcDevice` is `#[repr(transparent)]` over `Opaque<rtc_device>`, so we can safely cast.
> +        let rtc_device = unsafe { ARef::from_raw(NonNull::new_unchecked(rtc.cast::<Self>())) };
> +        rtc_device.set_drvdata(init)?;
> +        Ok(rtc_device)
> +    }
> +
> +    /// Store a pointer to the bound driver's private data.
> +    pub fn set_drvdata(&self, data: impl PinInit<T, Error>) -> Result {

This should not be public, as you should only use it in RtcDevice::new().

> +        let data = KBox::pin_init(data, GFP_KERNEL)?;
> +        let dev: &device::Device<device::Bound> = self.as_ref();
> +
> +        // SAFETY: `self.as_raw()` is a valid pointer to a `struct rtc_device`.
> +        unsafe { bindings::dev_set_drvdata(dev.as_raw(), data.into_foreign().cast()) };
> +        Ok(())
> +    }

<snip>

> +/// A resource guard that ensures the RTC device is properly registered.
> +///
> +/// This struct is intended to be managed by the `devres` framework by transferring its ownership
> +/// via [`devres::register`]. This ties the lifetime of the RTC device registration
> +/// to the lifetime of the underlying device.
> +pub struct Registration<T: 'static> {
> +    #[allow(dead_code)]
> +    rtc_device: ARef<RtcDevice<T>>,
> +}
> +
> +impl<T: 'static> Registration<T> {
> +    /// Registers an RTC device with the RTC subsystem.
> +    ///
> +    /// Transfers its ownership to the `devres` framework, which ties its lifetime
> +    /// to the parent device.
> +    /// On unbind of the parent device, the `devres` entry will be dropped, automatically
> +    /// cleaning up the RTC device. This function should be called from the driver's `probe`.
> +    pub fn register(dev: &device::Device<device::Bound>, rtc_device: ARef<RtcDevice<T>>) -> Result {
> +        let rtc_dev: &device::Device = rtc_device.as_ref();
> +        let rtc_parent = rtc_dev.parent().ok_or(EINVAL)?;
> +        if dev.as_raw() != rtc_parent.as_raw() {
> +            return Err(EINVAL);
> +        }
> +
> +        // Registers an RTC device with the RTC subsystem.
> +        // SAFETY: The device will be automatically unregistered when the parent device
> +        // is removed (devm cleanup). The helper function uses `THIS_MODULE` internally.
> +        let err = unsafe { bindings::devm_rtc_register_device(rtc_device.as_raw()) };
> +        if err != 0 {
> +            return Err(Error::from_errno(err));
> +        }
> +
> +        let registration = Registration { rtc_device };
> +
> +        devres::register(dev, registration, GFP_KERNEL)

You are using devm_rtc_register_device() above already, hence you neither need
an instance of Registration, nor do you need to manage this Registration with
devres through devres::register().

> +    }
> +}
> +
> +/// Options for creating an RTC device.
> +#[derive(Copy, Clone)]
> +pub struct RtcDeviceOptions {
> +    /// The name of the RTC device.
> +    pub name: &'static CStr,
> +}
> +
> +/// Trait implemented by RTC device operations.
> +///
> +/// This trait defines the operations that an RTC device driver must implement.
> +/// Most methods are optional and have default implementations that return an error.
> +#[vtable]
> +pub trait RtcOps: Sized + 'static {

Please utilize the AsBusDevice trait to be able to provide the parent device of
the RTC device as &Device<Bound>, similarly to what is done in [1].

[1] https://lore.kernel.org/all/20260106-rust_leds-v10-1-e0a1564884f9@posteo.de/