* [RFC PATCH v2 0/5] rust: Add RTC driver support
@ 2026-01-07 14:37 Ke Sun
2026-01-07 14:37 ` [RFC PATCH v2 1/5] rtc: migrate driver data to RTC device Ke Sun
` (4 more replies)
0 siblings, 5 replies; 32+ messages in thread
From: Ke Sun @ 2026-01-07 14:37 UTC (permalink / raw)
To: Alexandre Belloni, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich
Cc: linux-rtc, rust-for-linux, Alvin Sun, Ke Sun
This patch series adds RTC (Real-Time Clock) driver support for the Rust
kernel, including the necessary infrastructure and a complete driver implementation
for the PL031 RTC.
---
v2:
- Migrate RTC driver data storage from parent device to RTC device for unified interface
- Expand AMBA bus abstractions to full driver support with enhanced functionality
- Refactor device wakeup API by moving wake IRQ setup to IRQ layer
- Simplify RTC core framework by removing multi-bus abstractions, focusing on core operations
- Optimize PL031 driver implementation and remove build assertion dependency
v1: https://lore.kernel.org/rust-for-linux/20260104060621.3757812-1-sunke@kylinos.cn/
- Add AMBA bus abstractions
- Add device wakeup support
- Add RTC core framework with multi-bus support
- Add PL031 RTC driver
base-commit:
---
Ke Sun (5):
rtc: migrate driver data to RTC device
rust: add AMBA bus driver support
rust: add device wakeup capability support
rust: add RTC core abstractions and data structures
rust: add PL031 RTC driver
drivers/rtc/Kconfig | 9 +
drivers/rtc/Makefile | 1 +
drivers/rtc/dev.c | 4 +-
drivers/rtc/interface.c | 18 +-
drivers/rtc/rtc-pl031.c | 9 +-
drivers/rtc/rtc_pl031_rust.rs | 503 ++++++++++++++++
rust/bindings/bindings_helper.h | 3 +
rust/helpers/device.c | 7 +
rust/helpers/helpers.c | 1 +
rust/helpers/rtc.c | 9 +
rust/kernel/amba.rs | 396 +++++++++++++
rust/kernel/device.rs | 17 +-
rust/kernel/irq/request.rs | 7 +
rust/kernel/lib.rs | 4 +
rust/kernel/rtc.rs | 985 ++++++++++++++++++++++++++++++++
15 files changed, 1954 insertions(+), 19 deletions(-)
create mode 100644 drivers/rtc/rtc_pl031_rust.rs
create mode 100644 rust/helpers/rtc.c
create mode 100644 rust/kernel/amba.rs
create mode 100644 rust/kernel/rtc.rs
--
2.43.0
^ permalink raw reply [flat|nested] 32+ messages in thread
* [RFC PATCH v2 1/5] rtc: migrate driver data to RTC device
2026-01-07 14:37 [RFC PATCH v2 0/5] rust: Add RTC driver support Ke Sun
@ 2026-01-07 14:37 ` Ke Sun
2026-01-07 14:41 ` Ke Sun
` (2 more replies)
2026-01-07 14:37 ` [RFC PATCH v2 2/5] rust: add AMBA bus driver support Ke Sun
` (3 subsequent siblings)
4 siblings, 3 replies; 32+ messages in thread
From: Ke Sun @ 2026-01-07 14:37 UTC (permalink / raw)
To: Alexandre Belloni, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich
Cc: linux-rtc, rust-for-linux, Alvin Sun, Ke Sun
Unify RTC driver interface by storing driver data on the RTC device
instead of the parent device. Update RTC ops callbacks to pass the RTC
device itself rather than its parent. This change enables better
support for Rust RTC drivers that store data on the RTC device.
Signed-off-by: Ke Sun <sunke@kylinos.cn>
---
drivers/rtc/dev.c | 4 ++--
drivers/rtc/interface.c | 18 +++++++++---------
drivers/rtc/rtc-pl031.c | 9 ++-------
3 files changed, 13 insertions(+), 18 deletions(-)
diff --git a/drivers/rtc/dev.c b/drivers/rtc/dev.c
index baf1a8ca8b2b1..0f62ba9342e3e 100644
--- a/drivers/rtc/dev.c
+++ b/drivers/rtc/dev.c
@@ -410,7 +410,7 @@ static long rtc_dev_ioctl(struct file *file,
}
default:
if (rtc->ops->param_get)
- err = rtc->ops->param_get(rtc->dev.parent, ¶m);
+ err = rtc->ops->param_get(&rtc->dev, ¶m);
else
err = -EINVAL;
}
@@ -440,7 +440,7 @@ static long rtc_dev_ioctl(struct file *file,
default:
if (rtc->ops->param_set)
- err = rtc->ops->param_set(rtc->dev.parent, ¶m);
+ err = rtc->ops->param_set(&rtc->dev, ¶m);
else
err = -EINVAL;
}
diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
index b8b298efd9a9c..783a3ec3bb93d 100644
--- a/drivers/rtc/interface.c
+++ b/drivers/rtc/interface.c
@@ -91,7 +91,7 @@ static int __rtc_read_time(struct rtc_device *rtc, struct rtc_time *tm)
err = -EINVAL;
} else {
memset(tm, 0, sizeof(struct rtc_time));
- err = rtc->ops->read_time(rtc->dev.parent, tm);
+ err = rtc->ops->read_time(&rtc->dev, tm);
if (err < 0) {
dev_dbg(&rtc->dev, "read_time: fail to read: %d\n",
err);
@@ -155,7 +155,7 @@ int rtc_set_time(struct rtc_device *rtc, struct rtc_time *tm)
if (!rtc->ops)
err = -ENODEV;
else if (rtc->ops->set_time)
- err = rtc->ops->set_time(rtc->dev.parent, tm);
+ err = rtc->ops->set_time(&rtc->dev, tm);
else
err = -EINVAL;
@@ -200,7 +200,7 @@ static int rtc_read_alarm_internal(struct rtc_device *rtc,
alarm->time.tm_wday = -1;
alarm->time.tm_yday = -1;
alarm->time.tm_isdst = -1;
- err = rtc->ops->read_alarm(rtc->dev.parent, alarm);
+ err = rtc->ops->read_alarm(&rtc->dev, alarm);
}
mutex_unlock(&rtc->ops_lock);
@@ -441,7 +441,7 @@ static int __rtc_set_alarm(struct rtc_device *rtc, struct rtc_wkalrm *alarm)
else if (!test_bit(RTC_FEATURE_ALARM, rtc->features))
err = -EINVAL;
else
- err = rtc->ops->set_alarm(rtc->dev.parent, alarm);
+ err = rtc->ops->set_alarm(&rtc->dev, alarm);
/*
* Check for potential race described above. If the waiting for next
@@ -568,7 +568,7 @@ int rtc_alarm_irq_enable(struct rtc_device *rtc, unsigned int enabled)
else if (!test_bit(RTC_FEATURE_ALARM, rtc->features) || !rtc->ops->alarm_irq_enable)
err = -EINVAL;
else
- err = rtc->ops->alarm_irq_enable(rtc->dev.parent, enabled);
+ err = rtc->ops->alarm_irq_enable(&rtc->dev, enabled);
mutex_unlock(&rtc->ops_lock);
@@ -618,7 +618,7 @@ int rtc_update_irq_enable(struct rtc_device *rtc, unsigned int enabled)
rtc->uie_rtctimer.period = ktime_set(1, 0);
err = rtc_timer_enqueue(rtc, &rtc->uie_rtctimer);
if (!err && rtc->ops && rtc->ops->alarm_irq_enable)
- err = rtc->ops->alarm_irq_enable(rtc->dev.parent, 1);
+ err = rtc->ops->alarm_irq_enable(&rtc->dev, 1);
if (err)
goto out;
} else {
@@ -874,7 +874,7 @@ static void rtc_alarm_disable(struct rtc_device *rtc)
if (!rtc->ops || !test_bit(RTC_FEATURE_ALARM, rtc->features) || !rtc->ops->alarm_irq_enable)
return;
- rtc->ops->alarm_irq_enable(rtc->dev.parent, false);
+ rtc->ops->alarm_irq_enable(&rtc->dev, false);
trace_rtc_alarm_irq_enable(0, 0);
}
@@ -1076,7 +1076,7 @@ int rtc_read_offset(struct rtc_device *rtc, long *offset)
return -EINVAL;
mutex_lock(&rtc->ops_lock);
- ret = rtc->ops->read_offset(rtc->dev.parent, offset);
+ ret = rtc->ops->read_offset(&rtc->dev, offset);
mutex_unlock(&rtc->ops_lock);
trace_rtc_read_offset(*offset, ret);
@@ -1111,7 +1111,7 @@ int rtc_set_offset(struct rtc_device *rtc, long offset)
return -EINVAL;
mutex_lock(&rtc->ops_lock);
- ret = rtc->ops->set_offset(rtc->dev.parent, offset);
+ ret = rtc->ops->set_offset(&rtc->dev, offset);
mutex_unlock(&rtc->ops_lock);
trace_rtc_set_offset(offset, ret);
diff --git a/drivers/rtc/rtc-pl031.c b/drivers/rtc/rtc-pl031.c
index eab39dfa4e5fe..a605034d44cb7 100644
--- a/drivers/rtc/rtc-pl031.c
+++ b/drivers/rtc/rtc-pl031.c
@@ -284,10 +284,6 @@ static int pl031_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
static void pl031_remove(struct amba_device *adev)
{
- struct pl031_local *ldata = dev_get_drvdata(&adev->dev);
-
- if (adev->irq[0])
- free_irq(adev->irq[0], ldata);
amba_release_regions(adev);
}
@@ -320,8 +316,6 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
goto out;
}
- amba_set_drvdata(adev, ldata);
-
dev_dbg(&adev->dev, "designer ID = 0x%02x\n", amba_manf(adev));
dev_dbg(&adev->dev, "revision = 0x%01x\n", amba_rev(adev));
@@ -356,6 +350,7 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
ret = PTR_ERR(ldata->rtc);
goto out;
}
+ dev_set_drvdata(&ldata->rtc->dev, ldata);
if (!adev->irq[0])
clear_bit(RTC_FEATURE_ALARM, ldata->rtc->features);
@@ -369,7 +364,7 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
goto out;
if (adev->irq[0]) {
- ret = request_irq(adev->irq[0], pl031_interrupt,
+ ret = devm_request_irq(&adev->dev, adev->irq[0], pl031_interrupt,
vendor->irqflags, "rtc-pl031", ldata);
if (ret)
goto out;
--
2.43.0
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [RFC PATCH v2 2/5] rust: add AMBA bus driver support
2026-01-07 14:37 [RFC PATCH v2 0/5] rust: Add RTC driver support Ke Sun
2026-01-07 14:37 ` [RFC PATCH v2 1/5] rtc: migrate driver data to RTC device Ke Sun
@ 2026-01-07 14:37 ` Ke Sun
2026-01-08 11:29 ` Danilo Krummrich
2026-01-07 14:37 ` [RFC PATCH v2 3/5] rust: add device wakeup capability support Ke Sun
` (2 subsequent siblings)
4 siblings, 1 reply; 32+ messages in thread
From: Ke Sun @ 2026-01-07 14:37 UTC (permalink / raw)
To: Alexandre Belloni, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich
Cc: linux-rtc, rust-for-linux, Alvin Sun, Ke Sun
Add Rust abstractions for ARM AMBA bus drivers, enabling Rust drivers
to interact with AMBA devices through a type-safe interface.
Signed-off-by: Ke Sun <sunke@kylinos.cn>
---
rust/bindings/bindings_helper.h | 1 +
rust/kernel/amba.rs | 396 ++++++++++++++++++++++++++++++++
rust/kernel/lib.rs | 2 +
3 files changed, 399 insertions(+)
create mode 100644 rust/kernel/amba.rs
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index a067038b4b422..fa697287cf71b 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -29,6 +29,7 @@
#include <linux/hrtimer_types.h>
#include <linux/acpi.h>
+#include <linux/amba/bus.h>
#include <drm/drm_device.h>
#include <drm/drm_drv.h>
#include <drm/drm_file.h>
diff --git a/rust/kernel/amba.rs b/rust/kernel/amba.rs
new file mode 100644
index 0000000000000..29b37fd3d1b56
--- /dev/null
+++ b/rust/kernel/amba.rs
@@ -0,0 +1,396 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! ARM AMBA bus abstractions.
+//!
+//! C header: [`include/linux/amba/bus.h`](srctree/include/linux/amba/bus.h)
+
+use crate::{
+ acpi,
+ bindings,
+ device,
+ device_id::{
+ RawDeviceId,
+ RawDeviceIdIndex, //
+ },
+ driver,
+ error::{
+ from_result,
+ to_result, //
+ },
+ io::{
+ mem::IoRequest,
+ resource::Resource, //
+ },
+ irq::{
+ self,
+ IrqRequest, //
+ },
+ of,
+ prelude::*,
+ sync::aref::AlwaysRefCounted,
+ types::Opaque,
+ ThisModule, //
+};
+use core::{
+ marker::PhantomData,
+ mem::offset_of,
+ ptr::NonNull, //
+};
+
+/// Device ID table type for AMBA drivers.
+pub type IdTable<T> = &'static dyn kernel::device_id::IdTable<DeviceId, T>;
+
+/// AMBA device identifier.
+///
+/// Wraps the C `struct amba_id` from `include/linux/amba/bus.h`.
+#[repr(transparent)]
+#[derive(Clone, Copy)]
+pub struct DeviceId(pub(crate) bindings::amba_id);
+
+// SAFETY: `DeviceId` is a transparent wrapper over `amba_id` with no additional invariants.
+unsafe impl RawDeviceId for DeviceId {
+ type RawType = bindings::amba_id;
+}
+
+// SAFETY: The offset matches the `data` field in `struct amba_id`.
+unsafe impl RawDeviceIdIndex for DeviceId {
+ const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::amba_id, data);
+
+ fn index(&self) -> usize {
+ self.0.data as usize
+ }
+}
+
+impl DeviceId {
+ /// Creates a new device ID from an AMBA device ID and mask.
+ #[inline(always)]
+ pub const fn new(id: u32, mask: u32) -> Self {
+ let mut amba: bindings::amba_id = pin_init::zeroed();
+ amba.id = id;
+ amba.mask = mask;
+ Self(amba)
+ }
+
+ /// Creates a new device ID with driver-specific data.
+ #[inline(always)]
+ pub const fn new_with_data(id: u32, mask: u32, data: usize) -> Self {
+ let mut amba: bindings::amba_id = pin_init::zeroed();
+ amba.id = id;
+ amba.mask = mask;
+ amba.data = data as *mut core::ffi::c_void;
+ Self(amba)
+ }
+
+ /// Returns the device ID.
+ #[inline(always)]
+ pub const fn id(&self) -> u32 {
+ self.0.id
+ }
+
+ /// Returns the device ID mask.
+ #[inline(always)]
+ pub const fn mask(&self) -> u32 {
+ self.0.mask
+ }
+}
+
+/// Creates an AMBA device ID table with a module alias for modpost.
+#[macro_export]
+macro_rules! amba_device_table {
+ ($table_name:ident, $module_table_name:ident, $id_info_type: ty, $table_data: expr) => {
+ const $table_name: $crate::device_id::IdArray<
+ $crate::amba::DeviceId,
+ $id_info_type,
+ { $table_data.len() },
+ > = $crate::device_id::IdArray::new($table_data);
+
+ $crate::module_device_table!("amba", $module_table_name, $table_name);
+ };
+}
+
+/// The AMBA device representation.
+///
+/// This structure represents the Rust abstraction for a C `struct amba_device`. The
+/// implementation abstracts the usage of an already existing C `struct amba_device` within Rust
+/// code that we get passed from the C side.
+///
+/// # Invariants
+///
+/// A [`Device`] instance represents a valid `struct amba_device` created by the C portion of
+/// the kernel.
+#[repr(transparent)]
+pub struct Device<Ctx: device::DeviceContext = device::Normal>(
+ Opaque<bindings::amba_device>,
+ PhantomData<Ctx>,
+);
+
+impl<Ctx: device::DeviceContext> Device<Ctx> {
+ fn as_raw(&self) -> *mut bindings::amba_device {
+ self.0.get()
+ }
+
+ /// Returns the resource at `index`, if any.
+ pub fn resource(&self) -> Option<&Resource> {
+ // SAFETY: `self.as_raw()` returns a valid pointer to a `struct amba_device`.
+ let resource = unsafe { &raw mut (*self.as_raw()).res };
+ if resource.is_null() {
+ None
+ } else {
+ // SAFETY: `resource` is a valid pointer to a `struct resource`.
+ Some(unsafe { Resource::from_raw(resource) })
+ }
+ }
+}
+
+impl<Ctx: device::DeviceContext> AsRef<device::Device<Ctx>> for Device<Ctx> {
+ fn as_ref(&self) -> &device::Device<Ctx> {
+ // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a
+ // valid `struct amba_device`.
+ let dev = unsafe { &raw mut (*self.as_raw()).dev };
+ // SAFETY: `dev` points to a valid `struct device`.
+ unsafe { device::Device::from_raw(dev) }
+ }
+}
+
+// SAFETY: `Device` is a transparent wrapper that doesn't depend on its generic
+// argument.
+crate::impl_device_context_deref!(unsafe { Device });
+crate::impl_device_context_into_aref!(Device);
+
+impl Device<device::Core> {}
+
+impl Device<device::Bound> {
+ /// Returns an [`IoRequest`] for the memory resource.
+ pub fn io_request(&self) -> Option<IoRequest<'_>> {
+ self.resource()
+ // SAFETY: `resource` is valid for the lifetime of the `IoRequest`.
+ .map(|resource| unsafe { IoRequest::new(self.as_ref(), resource) })
+ }
+
+ /// Returns an [`IrqRequest`] for the IRQ at the given index.
+ pub fn irq_by_index(&self, index: u32) -> Result<IrqRequest<'_>> {
+ if index >= bindings::AMBA_NR_IRQS {
+ return Err(EINVAL);
+ }
+ // SAFETY: `self.as_raw()` returns a valid pointer to a `struct amba_device`.
+ let irq = unsafe { (*self.as_raw()).irq[index as usize] };
+ if irq == 0 {
+ return Err(ENXIO);
+ }
+ // SAFETY: `irq` is guaranteed to be a valid IRQ number for `&self`.
+ Ok(unsafe { IrqRequest::new(self.as_ref(), irq) })
+ }
+
+ /// Returns a [`irq::Registration`] for the IRQ at the given index.
+ pub fn request_irq_by_index<'a, T: irq::Handler + 'static>(
+ &'a self,
+ flags: irq::Flags,
+ index: u32,
+ name: &'static CStr,
+ handler: impl PinInit<T, Error> + 'a,
+ ) -> impl PinInit<irq::Registration<T>, Error> + 'a {
+ pin_init::pin_init_scope(move || {
+ let request = self.irq_by_index(index).map_err(|_| EINVAL)?;
+ Ok(irq::Registration::<T>::new(request, flags, name, handler))
+ })
+ }
+}
+
+// SAFETY: `Device` instances are always reference-counted via the underlying `device`.
+unsafe impl AlwaysRefCounted for Device {
+ fn inc_ref(&self) {
+ // SAFETY: A shared reference guarantees the refcount is non-zero.
+ unsafe { bindings::get_device(self.as_ref().as_raw()) };
+ }
+
+ unsafe fn dec_ref(obj: NonNull<Self>) {
+ let adev: *mut bindings::amba_device = obj.cast().as_ptr();
+ // SAFETY: `amba_device` contains `device` as its first field.
+ let dev: *mut bindings::device = unsafe { &raw mut (*adev).dev };
+ // SAFETY: The safety requirements guarantee that the refcount is non-zero.
+ unsafe { bindings::put_device(dev) }
+ }
+}
+
+// SAFETY: `Device` is a transparent wrapper of `struct amba_device`.
+unsafe impl<Ctx: device::DeviceContext> device::AsBusDevice<Ctx> for Device<Ctx> {
+ const OFFSET: usize = offset_of!(bindings::amba_device, dev);
+}
+
+// SAFETY: `Device` is reference-counted and can be released from any thread.
+unsafe impl Send for Device {}
+
+// SAFETY: All methods of `Device` (i.e., `Device<Normal>`) are thread-safe.
+unsafe impl Sync for Device {}
+
+/// An adapter for the registration of AMBA drivers.
+pub struct Adapter<T: Driver>(T);
+
+// SAFETY: A call to `unregister` for a given instance of `RegType` is guaranteed to be valid if
+// a preceding call to `register` has been successful.
+unsafe impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> {
+ type RegType = bindings::amba_driver;
+
+ unsafe fn register(
+ adrv: &Opaque<Self::RegType>,
+ name: &'static CStr,
+ module: &'static ThisModule,
+ ) -> Result {
+ let amba_table = match T::AMBA_ID_TABLE {
+ Some(table) => table.as_ptr(),
+ None => core::ptr::null(),
+ };
+ // SAFETY: It's safe to set the fields of `struct amba_driver` on initialization.
+ unsafe {
+ (*adrv.get()).drv.name = name.as_char_ptr();
+ (*adrv.get()).probe = Some(Self::probe_callback);
+ (*adrv.get()).remove = Some(Self::remove_callback);
+ (*adrv.get()).shutdown = Some(Self::shutdown_callback);
+ (*adrv.get()).id_table = amba_table;
+ }
+ // SAFETY: `adrv` is guaranteed to be a valid `RegType`.
+ to_result(unsafe { bindings::__amba_driver_register(adrv.get(), module.0) })
+ }
+
+ unsafe fn unregister(adrv: &Opaque<Self::RegType>) {
+ // SAFETY: `adrv` is guaranteed to be a valid `RegType`.
+ unsafe { bindings::amba_driver_unregister(adrv.get()) };
+ }
+}
+
+impl<T: Driver + 'static> Adapter<T> {
+ extern "C" fn probe_callback(
+ adev: *mut bindings::amba_device,
+ id: *const bindings::amba_id,
+ ) -> kernel::ffi::c_int {
+ // SAFETY: The AMBA bus only ever calls the probe callback with a valid pointer to a
+ // `struct amba_device`. `adev` is valid for the duration of `probe_callback()`.
+ let adev_internal = unsafe { &*adev.cast::<Device<device::CoreInternal>>() };
+ let info = Self::amba_id_info(adev_internal, id);
+ from_result(|| {
+ let datapin = T::probe(adev_internal, info);
+ adev_internal.as_ref().set_drvdata(datapin)?;
+ Ok(0)
+ })
+ }
+
+ extern "C" fn remove_callback(adev: *mut bindings::amba_device) {
+ // SAFETY: The AMBA bus only ever calls the remove callback with a valid pointer to a
+ // `struct amba_device`. `adev` is valid for the duration of `remove_callback()`.
+ let adev = unsafe { &*adev.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>>`.
+ let data = unsafe { adev.as_ref().drvdata_obtain::<T>() };
+ T::unbind(adev, data.as_ref());
+ }
+
+ extern "C" fn shutdown_callback(adev: *mut bindings::amba_device) {
+ // SAFETY: `shutdown_callback` is only ever called for a valid `adev`.
+ let adev = unsafe { &*adev.cast::<Device<device::CoreInternal>>() };
+ // SAFETY: `shutdown_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>>`.
+ let data = unsafe { adev.as_ref().drvdata_obtain::<T>() };
+ T::shutdown(adev, data.as_ref());
+ }
+
+ fn amba_id_table() -> Option<IdTable<<Self as driver::Adapter>::IdInfo>> {
+ T::AMBA_ID_TABLE
+ }
+
+ fn amba_id_info(
+ _dev: &Device,
+ id: *const bindings::amba_id,
+ ) -> Option<&'static <Self as driver::Adapter>::IdInfo> {
+ if id.is_null() {
+ return None;
+ }
+ let table = Self::amba_id_table()?;
+ // SAFETY: `id` is a valid pointer to a `struct amba_id` that was matched by the kernel.
+ // `DeviceId` is a `#[repr(transparent)]` wrapper of `struct amba_id` and does not add
+ // additional invariants, so it's safe to transmute.
+ let device_id = unsafe { &*id.cast::<DeviceId>() };
+ Some(table.info(<DeviceId as RawDeviceIdIndex>::index(device_id)))
+ }
+}
+
+impl<T: Driver + 'static> driver::Adapter for Adapter<T> {
+ type IdInfo = T::IdInfo;
+
+ fn acpi_id_table() -> Option<acpi::IdTable<Self::IdInfo>> {
+ None
+ }
+
+ fn of_id_table() -> Option<of::IdTable<Self::IdInfo>> {
+ None
+ }
+}
+
+/// The AMBA driver trait.
+///
+/// Drivers must implement this trait in order to get an AMBA driver registered.
+pub trait Driver: Send {
+ /// The type holding information about each device id supported by the driver.
+ type IdInfo: 'static;
+ /// The table of device ids supported by the driver.
+ const AMBA_ID_TABLE: Option<IdTable<Self::IdInfo>> = None;
+
+ /// AMBA driver probe.
+ ///
+ /// Called when a new AMBA device is added or discovered.
+ /// Implementers should attempt to initialize the device here.
+ fn probe(
+ dev: &Device<device::Core>,
+ id_info: Option<&Self::IdInfo>,
+ ) -> impl PinInit<Self, Error>;
+
+ /// AMBA driver shutdown.
+ ///
+ /// Called by the kernel during system reboot or power-off to allow the [`Driver`] to bring the
+ /// [`Device`] into a safe state. Implementing this callback is optional.
+ ///
+ /// Typical actions include stopping transfers, disabling interrupts, or resetting the hardware
+ /// to prevent undesired behavior during shutdown.
+ ///
+ /// This callback is distinct from final resource cleanup, as the driver instance remains valid
+ /// after it returns. Any deallocation or teardown of driver-owned resources should instead be
+ /// handled in `Self::drop`.
+ fn shutdown(dev: &Device<device::Core>, this: Pin<&Self>) {
+ let _ = (dev, this);
+ }
+
+ /// AMBA driver unbind.
+ ///
+ /// Called when the [`Device`] is unbound from its bound [`Driver`]. Implementing this
+ /// callback is optional.
+ ///
+ /// This callback serves as a place for drivers to perform teardown operations that require a
+ /// `&Device<Core>` or `&Device<Bound>` reference. For instance, drivers may try to perform I/O
+ /// operations to gracefully tear down the device.
+ ///
+ /// Otherwise, release operations for driver resources should be performed in `Self::drop`.
+ fn unbind(dev: &Device<device::Core>, this: Pin<&Self>) {
+ let _ = (dev, this);
+ }
+}
+
+/// Declares a kernel module that exposes a single AMBA driver.
+///
+/// # Examples
+///
+/// ```ignore
+/// kernel::module_amba_driver! {
+/// type: MyDriver,
+/// name: "Module name",
+/// authors: ["Author name"],
+/// description: "Description",
+/// license: "GPL v2",
+/// }
+/// ```
+#[macro_export]
+macro_rules! module_amba_driver {
+ ($($f:tt)*) => {
+ $crate::module_driver!(<T>, $crate::amba::Adapter<T>, { $($f)* });
+ };
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index f812cf1200428..3e557335fc5fe 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -66,6 +66,8 @@
pub mod acpi;
pub mod alloc;
+#[cfg(CONFIG_ARM_AMBA)]
+pub mod amba;
#[cfg(CONFIG_AUXILIARY_BUS)]
pub mod auxiliary;
pub mod bitmap;
--
2.43.0
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [RFC PATCH v2 3/5] rust: add device wakeup capability support
2026-01-07 14:37 [RFC PATCH v2 0/5] rust: Add RTC driver support Ke Sun
2026-01-07 14:37 ` [RFC PATCH v2 1/5] rtc: migrate driver data to RTC device Ke Sun
2026-01-07 14:37 ` [RFC PATCH v2 2/5] rust: add AMBA bus driver support Ke Sun
@ 2026-01-07 14:37 ` Ke Sun
2026-01-07 14:57 ` Greg KH
2026-01-07 14:37 ` [RFC PATCH v2 4/5] rust: add RTC core abstractions and data structures Ke Sun
2026-01-07 14:37 ` [RFC PATCH v2 5/5] rust: add PL031 RTC driver Ke Sun
4 siblings, 1 reply; 32+ messages in thread
From: Ke Sun @ 2026-01-07 14:37 UTC (permalink / raw)
To: Alexandre Belloni, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich
Cc: linux-rtc, rust-for-linux, Alvin Sun, Ke Sun
Add Rust bindings and wrappers for device wakeup functionality,
including devm_device_init_wakeup() and dev_pm_set_wake_irq().
Signed-off-by: Ke Sun <sunke@kylinos.cn>
---
rust/bindings/bindings_helper.h | 2 ++
rust/helpers/device.c | 7 +++++++
rust/kernel/device.rs | 17 ++++++++++++++++-
rust/kernel/irq/request.rs | 7 +++++++
4 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index fa697287cf71b..d6c2b06ac4107 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -88,6 +88,8 @@
#include <linux/workqueue.h>
#include <linux/xarray.h>
#include <trace/events/rust_sample.h>
+#include <linux/pm_wakeup.h>
+#include <linux/pm_wakeirq.h>
/*
* The driver-core Rust code needs to know about some C driver-core private
diff --git a/rust/helpers/device.c b/rust/helpers/device.c
index 9a4316bafedfb..cae26edd83696 100644
--- a/rust/helpers/device.c
+++ b/rust/helpers/device.c
@@ -1,6 +1,8 @@
// SPDX-License-Identifier: GPL-2.0
#include <linux/device.h>
+#include <linux/pm_wakeup.h>
+#include <linux/pm_wakeirq.h>
int rust_helper_devm_add_action(struct device *dev,
void (*action)(void *),
@@ -25,3 +27,8 @@ void rust_helper_dev_set_drvdata(struct device *dev, void *data)
{
dev_set_drvdata(dev, data);
}
+
+int rust_helper_devm_device_init_wakeup(struct device *dev)
+{
+ return devm_device_init_wakeup(dev);
+}
diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index c79be2e2bfe38..24fc69adf7bea 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -5,7 +5,9 @@
//! C header: [`include/linux/device.h`](srctree/include/linux/device.h)
use crate::{
- bindings, fmt,
+ bindings,
+ error::to_result,
+ fmt,
prelude::*,
sync::aref::ARef,
types::{ForeignOwnable, Opaque},
@@ -325,6 +327,19 @@ pub fn drvdata<T: 'static>(&self) -> Result<Pin<&T>> {
// - We've just checked that the type of the driver's private data is in fact `T`.
Ok(unsafe { self.drvdata_unchecked() })
}
+
+ /// Initialize device wakeup capability.
+ ///
+ /// Marks the device as wakeup-capable and enables wakeup. The wakeup capability is
+ /// automatically disabled when the device is removed (resource-managed).
+ ///
+ /// Returns `Ok(())` on success, or an error code on failure.
+ pub fn devm_init_wakeup(&self) -> Result {
+ // SAFETY: `self.as_raw()` is a valid pointer to a `struct device`.
+ // The function is exported from bindings_helper module via pub use.
+ let ret = unsafe { bindings::devm_device_init_wakeup(self.as_raw()) };
+ to_result(ret)
+ }
}
impl<Ctx: DeviceContext> Device<Ctx> {
diff --git a/rust/kernel/irq/request.rs b/rust/kernel/irq/request.rs
index b150563fdef80..c73e0c544fec7 100644
--- a/rust/kernel/irq/request.rs
+++ b/rust/kernel/irq/request.rs
@@ -120,6 +120,13 @@ pub(crate) unsafe fn new(dev: &'a Device<Bound>, irq: u32) -> Self {
pub fn irq(&self) -> u32 {
self.irq
}
+
+ /// Set the IRQ as a wake IRQ.
+ pub fn set_wake_irq(&self) -> Result {
+ // SAFETY: `self.as_raw()` is a valid pointer to a `struct device`.
+ let ret = unsafe { bindings::dev_pm_set_wake_irq(self.dev.as_raw(), self.irq as i32) };
+ to_result(ret)
+ }
}
/// A registration of an IRQ handler for a given IRQ line.
--
2.43.0
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [RFC PATCH v2 4/5] rust: add RTC core abstractions and data structures
2026-01-07 14:37 [RFC PATCH v2 0/5] rust: Add RTC driver support Ke Sun
` (2 preceding siblings ...)
2026-01-07 14:37 ` [RFC PATCH v2 3/5] rust: add device wakeup capability support Ke Sun
@ 2026-01-07 14:37 ` Ke Sun
2026-01-08 11:50 ` Danilo Krummrich
2026-01-08 23:31 ` Kari Argillander
2026-01-07 14:37 ` [RFC PATCH v2 5/5] rust: add PL031 RTC driver Ke Sun
4 siblings, 2 replies; 32+ messages in thread
From: Ke Sun @ 2026-01-07 14:37 UTC (permalink / raw)
To: Alexandre Belloni, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich
Cc: linux-rtc, rust-for-linux, Alvin Sun, Ke Sun
Add Rust abstractions for RTC subsystem, including RtcDevice,
RtcTime, RtcWkAlrm data structures, RtcOps trait for driver
operations, and devm-managed device registration support.
Signed-off-by: Ke Sun <sunke@kylinos.cn>
---
rust/helpers/helpers.c | 1 +
rust/helpers/rtc.c | 9 +
rust/kernel/lib.rs | 2 +
rust/kernel/rtc.rs | 985 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 997 insertions(+)
create mode 100644 rust/helpers/rtc.c
create mode 100644 rust/kernel/rtc.rs
diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index 79c72762ad9c4..1a5c103fb24ba 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -48,6 +48,7 @@
#include "rcu.c"
#include "refcount.c"
#include "regulator.c"
+#include "rtc.c"
#include "scatterlist.c"
#include "security.c"
#include "signal.c"
diff --git a/rust/helpers/rtc.c b/rust/helpers/rtc.c
new file mode 100644
index 0000000000000..862cd61670bfc
--- /dev/null
+++ b/rust/helpers/rtc.c
@@ -0,0 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/rtc.h>
+
+int rust_helper_devm_rtc_register_device(struct rtc_device *rtc)
+{
+ return __devm_rtc_register_device(THIS_MODULE, rtc);
+}
+
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 3e557335fc5fe..1390073e4ae27 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -135,6 +135,8 @@
pub mod rbtree;
pub mod regulator;
pub mod revocable;
+#[cfg(CONFIG_RTC_CLASS)]
+pub mod rtc;
pub mod scatterlist;
pub mod security;
pub mod seq_file;
diff --git a/rust/kernel/rtc.rs b/rust/kernel/rtc.rs
new file mode 100644
index 0000000000000..0c662b94b96f4
--- /dev/null
+++ b/rust/kernel/rtc.rs
@@ -0,0 +1,985 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! RTC (Real-Time Clock) device support.
+//!
+//! C headers: [`include/linux/rtc.h`](srctree/include/linux/rtc.h).
+//!
+//! Reference: <https://www.kernel.org/doc/html/latest/driver-api/rtc.html>
+use crate::{
+ bindings,
+ bitmap::Bitmap,
+ device::{
+ self,
+ AsBusDevice, //
+ },
+ devres,
+ error::Error,
+ prelude::*,
+ seq_file::SeqFile,
+ sync::aref::{
+ ARef, //
+ AlwaysRefCounted,
+ },
+ types::{
+ ForeignOwnable,
+ Opaque, //
+ }, //
+};
+
+use core::{
+ ffi::c_void,
+ marker::PhantomData,
+ ptr::NonNull, //
+};
+
+/// RTC time structure.
+///
+/// Wraps the C `struct rtc_time` from `include/uapi/linux/rtc.h`.
+#[repr(transparent)]
+#[derive(Clone, Copy)]
+pub struct RtcTime(pub bindings::rtc_time);
+
+impl RtcTime {
+ /// Creates a new `RtcTime` from a time64_t value (seconds since 1970-01-01 00:00:00 UTC).
+ pub fn from_time64(time: i64) -> Self {
+ let mut tm = Self(pin_init::zeroed());
+ // SAFETY: `rtc_time64_to_tm` is a pure function that only writes to the provided
+ // `struct rtc_time` pointer. The pointer is valid because `tm.0` is a valid mutable
+ // reference, and the function does not retain any references to it.
+ unsafe {
+ bindings::rtc_time64_to_tm(time, &mut tm.0);
+ }
+ tm
+ }
+
+ /// Converts this `RtcTime` to a time64_t value (seconds since 1970-01-01 00:00:00 UTC).
+ pub fn to_time64(&self) -> i64 {
+ // SAFETY: `rtc_tm_to_time64` is a pure function that only reads from the provided
+ // `struct rtc_time` pointer. The pointer is valid because `self.0` is a valid reference,
+ // and the function does not retain any references to it.
+ unsafe { bindings::rtc_tm_to_time64(core::ptr::from_ref(&self.0).cast_mut()) }
+ }
+
+ /// Sets this `RtcTime` from a time64_t value (seconds since 1970-01-01 00:00:00 UTC).
+ pub fn set_from_time64(&mut self, time: i64) {
+ // SAFETY: `rtc_time64_to_tm` is a pure function that only writes to the provided
+ // `struct rtc_time` pointer. The pointer is valid because `self.0` is a valid mutable
+ // reference, and the function does not retain any references to it.
+ unsafe {
+ bindings::rtc_time64_to_tm(time, &mut self.0);
+ }
+ }
+
+ /// Converts a time64_t value to an RTC time structure.
+ #[inline]
+ pub fn time64_to_tm(time: i64, tm: &mut Self) {
+ tm.set_from_time64(time);
+ }
+
+ /// Converts an RTC time structure to a time64_t value (seconds since 1970-01-01 00:00:00 UTC).
+ #[inline]
+ pub fn tm_to_time64(tm: &Self) -> i64 {
+ tm.to_time64()
+ }
+
+ /// Calculates the day of the year (0-365) from the date components.
+ #[inline]
+ pub fn year_days(&self) -> i32 {
+ // SAFETY: `rtc_year_days` is a pure function that only performs calculations based on
+ // the provided parameters. The values should be from valid RTC time structures and
+ // non-negative, but the function itself is safe to call with any values.
+ unsafe {
+ bindings::rtc_year_days(
+ self.0.tm_mday as u32,
+ self.0.tm_mon as u32,
+ self.0.tm_year as u32,
+ )
+ }
+ }
+
+ /// Returns the seconds field (0-59).
+ #[inline]
+ pub fn tm_sec(&self) -> i32 {
+ self.0.tm_sec
+ }
+
+ /// Sets the seconds field (0-59).
+ #[inline]
+ pub fn set_tm_sec(&mut self, sec: i32) {
+ self.0.tm_sec = sec;
+ }
+
+ /// Returns the minutes field (0-59).
+ #[inline]
+ pub fn tm_min(&self) -> i32 {
+ self.0.tm_min
+ }
+
+ /// Sets the minutes field (0-59).
+ #[inline]
+ pub fn set_tm_min(&mut self, min: i32) {
+ self.0.tm_min = min;
+ }
+
+ /// Returns the hours field (0-23).
+ #[inline]
+ pub fn tm_hour(&self) -> i32 {
+ self.0.tm_hour
+ }
+
+ /// Sets the hours field (0-23).
+ #[inline]
+ pub fn set_tm_hour(&mut self, hour: i32) {
+ self.0.tm_hour = hour;
+ }
+
+ /// Returns the day of the month (1-31).
+ #[inline]
+ pub fn tm_mday(&self) -> i32 {
+ self.0.tm_mday
+ }
+
+ /// Sets the day of the month (1-31).
+ #[inline]
+ pub fn set_tm_mday(&mut self, mday: i32) {
+ self.0.tm_mday = mday;
+ }
+
+ /// Returns the month (0-11, where 0 is January).
+ #[inline]
+ pub fn tm_mon(&self) -> i32 {
+ self.0.tm_mon
+ }
+
+ /// Sets the month (0-11, where 0 is January).
+ #[inline]
+ pub fn set_tm_mon(&mut self, mon: i32) {
+ self.0.tm_mon = mon;
+ }
+
+ /// Returns the year (years since 1900).
+ #[inline]
+ pub fn tm_year(&self) -> i32 {
+ self.0.tm_year
+ }
+
+ /// Sets the year (years since 1900).
+ #[inline]
+ pub fn set_tm_year(&mut self, year: i32) {
+ self.0.tm_year = year;
+ }
+
+ /// Returns the day of the week (0-6, where 0 is Sunday).
+ #[inline]
+ pub fn tm_wday(&self) -> i32 {
+ self.0.tm_wday
+ }
+
+ /// Sets the day of the week (0-6, where 0 is Sunday).
+ #[inline]
+ pub fn set_tm_wday(&mut self, wday: i32) {
+ self.0.tm_wday = wday;
+ }
+
+ /// Returns the day of the year (0-365).
+ #[inline]
+ pub fn tm_yday(&self) -> i32 {
+ self.0.tm_yday
+ }
+
+ /// Sets the day of the year (0-365).
+ #[inline]
+ pub fn set_tm_yday(&mut self, yday: i32) {
+ self.0.tm_yday = yday;
+ }
+
+ /// Returns the daylight saving time flag.
+ #[inline]
+ pub fn tm_isdst(&self) -> i32 {
+ self.0.tm_isdst
+ }
+
+ /// Sets the daylight saving time flag.
+ #[inline]
+ pub fn set_tm_isdst(&mut self, isdst: i32) {
+ self.0.tm_isdst = isdst;
+ }
+}
+
+impl Default for RtcTime {
+ fn default() -> Self {
+ Self(pin_init::zeroed())
+ }
+}
+
+/// RTC wake alarm structure.
+///
+/// Wraps the C `struct rtc_wkalrm` from `include/uapi/linux/rtc.h`.
+#[repr(transparent)]
+#[derive(Clone, Copy)]
+pub struct RtcWkAlrm(pub bindings::rtc_wkalrm);
+
+impl Default for RtcWkAlrm {
+ fn default() -> Self {
+ Self(pin_init::zeroed())
+ }
+}
+
+impl RtcWkAlrm {
+ /// Returns a reference to the alarm time.
+ #[inline]
+ pub fn get_time(&self) -> &RtcTime {
+ // SAFETY: `RtcTime` is `#[repr(transparent)]` over `bindings::rtc_time`, so the memory
+ // layout is identical. We can safely reinterpret the reference.
+ unsafe { &*(&raw const self.0.time).cast::<RtcTime>() }
+ }
+
+ /// Returns a mutable reference to the alarm time.
+ #[inline]
+ pub fn get_time_mut(&mut self) -> &mut RtcTime {
+ // SAFETY: `RtcTime` is `#[repr(transparent)]` over `bindings::rtc_time`, so the memory
+ // layout is identical. We can safely reinterpret the reference.
+ unsafe { &mut *(&raw mut self.0.time).cast::<RtcTime>() }
+ }
+
+ /// Sets the alarm time from a `RtcTime` value.
+ #[inline]
+ pub fn set_time(&mut self, time: RtcTime) {
+ self.0.time = time.0;
+ }
+
+ /// Returns the enabled field.
+ #[inline]
+ pub fn enabled(&self) -> u8 {
+ self.0.enabled
+ }
+
+ /// Sets the `enabled` field.
+ #[inline]
+ pub fn set_enabled(&mut self, enabled: u8) {
+ self.0.enabled = enabled;
+ }
+
+ /// Returns the pending field.
+ #[inline]
+ pub fn pending(&self) -> u8 {
+ self.0.pending
+ }
+
+ /// Sets the `pending` field.
+ #[inline]
+ pub fn set_pending(&mut self, pending: u8) {
+ self.0.pending = pending;
+ }
+}
+
+/// RTC parameter structure.
+///
+/// Wraps the C `struct rtc_param` from `include/uapi/linux/rtc.h`.
+#[repr(transparent)]
+#[derive(Clone, Copy)]
+pub struct RtcParam(pub bindings::rtc_param);
+
+impl Default for RtcParam {
+ fn default() -> Self {
+ Self(pin_init::zeroed())
+ }
+}
+
+/// 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(())
+/// // }
+/// ```
+///
+/// [`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> {
+ 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);
+}
+
+// SAFETY: Instances of `RtcDevice` are always reference-counted via the underlying `device`.
+// The `struct rtc_device` contains a `struct device dev` as its first field, and the
+// reference counting is managed through `get_device`/`put_device` on the `dev` field.
+unsafe impl<T: 'static> AlwaysRefCounted for RtcDevice<T> {
+ fn inc_ref(&self) {
+ let dev: &device::Device = self.as_ref();
+ // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
+ // `dev.as_raw()` is a valid pointer to a `struct device` with a non-zero refcount.
+ unsafe { bindings::get_device(dev.as_raw()) };
+ }
+
+ unsafe fn dec_ref(obj: NonNull<Self>) {
+ let rtc: *mut bindings::rtc_device = obj.cast().as_ptr();
+
+ // SAFETY: By the type invariant of `Self`, `rtc` is a pointer to a valid
+ // `struct rtc_device`. The `dev` field is the first field of `struct rtc_device`,
+ // so we can safely access it.
+ let dev = unsafe { &raw mut (*rtc).dev };
+
+ // SAFETY: The safety requirements guarantee that the refcount is non-zero.
+ unsafe { bindings::put_device(dev) };
+ }
+}
+
+// SAFETY: `RtcDevice` is reference-counted and can be released from any thread.
+unsafe impl<T: 'static> Send for RtcDevice<T> {}
+
+// SAFETY: `RtcDevice` can be shared among threads because all immutable methods are
+// protected by the synchronization in `struct rtc_device` (via `ops_lock` mutex).
+unsafe impl<T: 'static> Sync for RtcDevice<T> {}
+
+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,
+ 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 {
+ 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(())
+ }
+
+ /// Borrow the driver's private data bound to this [`RtcDevice`].
+ pub fn drvdata(&self) -> Result<Pin<&T>> {
+ let dev: &device::Device<device::Bound> = self.as_ref();
+
+ // SAFETY: `self.as_raw()` is a valid pointer to a `struct device`.
+ let ptr = unsafe { bindings::dev_get_drvdata(dev.as_raw()) };
+
+ if ptr.is_null() {
+ return Err(ENOENT);
+ }
+
+ // SAFETY: The caller ensures that `ptr` is valid and writable.
+ Ok(unsafe { Pin::<KBox<T>>::borrow(ptr.cast()) })
+ }
+}
+
+impl<T: 'static> Drop for RtcDevice<T> {
+ fn drop(&mut self) {
+ let dev: &device::Device<device::Bound> = self.as_ref();
+ // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
+ let ptr: *mut c_void = unsafe { bindings::dev_get_drvdata(dev.as_raw()) };
+
+ // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
+ unsafe { bindings::dev_set_drvdata(dev.as_raw(), core::ptr::null_mut()) };
+
+ if !ptr.is_null() {
+ // SAFETY: `ptr` comes from a previous call to `into_foreign()`, and
+ // `dev_get_drvdata()` guarantees to return the same pointer given to
+ // `dev_set_drvdata()`.
+ unsafe { drop(Pin::<KBox<T>>::from_foreign(ptr.cast())) };
+ }
+ }
+}
+
+/// 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)
+ }
+}
+
+/// 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 {
+ /// Read the current time from the RTC.
+ ///
+ /// This is a required method and must be implemented.
+ fn read_time(_rtcdev: &RtcDevice<Self>, _tm: &mut RtcTime) -> Result {
+ Err(ENOTSUPP)
+ }
+
+ /// Set the time in the RTC.
+ ///
+ /// This is a required method and must be implemented.
+ ///
+ /// Note: The parameter is `&mut` to match the C API signature, even though
+ /// it's conceptually read-only from the Rust side.
+ fn set_time(_rtcdev: &RtcDevice<Self>, _tm: &mut RtcTime) -> Result {
+ Err(ENOTSUPP)
+ }
+
+ /// Read the alarm time from the RTC.
+ fn read_alarm(_rtcdev: &RtcDevice<Self>, _alarm: &mut RtcWkAlrm) -> Result {
+ Err(ENOTSUPP)
+ }
+
+ /// Set the alarm time in the RTC.
+ ///
+ /// Note: The parameter is `&mut` to match the C API signature, even though
+ /// it's conceptually read-only from the Rust side.
+ fn set_alarm(_rtcdev: &RtcDevice<Self>, _alarm: &mut RtcWkAlrm) -> Result {
+ Err(ENOTSUPP)
+ }
+
+ /// Enable or disable the alarm interrupt.
+ ///
+ /// `enabled` is non-zero to enable, zero to disable.
+ fn alarm_irq_enable(_rtcdev: &RtcDevice<Self>, _enabled: u32) -> Result {
+ Err(ENOTSUPP)
+ }
+
+ /// Handle custom ioctl commands.
+ fn ioctl(_rtcdev: &RtcDevice<Self>, _cmd: u32, _arg: c_ulong) -> Result<c_int> {
+ Err(ENOTSUPP)
+ }
+
+ /// Show information in /proc/driver/rtc.
+ fn proc(_rtcdev: &RtcDevice<Self>, _seq: &mut SeqFile) -> Result {
+ Err(ENOTSUPP)
+ }
+
+ /// Read the time offset.
+ fn read_offset(_rtcdev: &RtcDevice<Self>, _offset: &mut i64) -> Result {
+ Err(ENOTSUPP)
+ }
+
+ /// Set the time offset.
+ fn set_offset(_rtcdev: &RtcDevice<Self>, _offset: i64) -> Result {
+ Err(ENOTSUPP)
+ }
+
+ /// Get an RTC parameter.
+ fn param_get(_rtcdev: &RtcDevice<Self>, _param: &mut RtcParam) -> Result {
+ Err(ENOTSUPP)
+ }
+
+ /// Set an RTC parameter.
+ ///
+ /// Note: The parameter is `&mut` to match the C API signature, even though
+ /// it's conceptually read-only from the Rust side.
+ fn param_set(_rtcdev: &RtcDevice<Self>, _param: &mut RtcParam) -> Result {
+ Err(ENOTSUPP)
+ }
+}
+
+struct Adapter<T: RtcOps> {
+ _p: PhantomData<T>,
+}
+
+impl<T: RtcOps> Adapter<T> {
+ const VTABLE: RtcOpsVTable = create_rtc_ops::<T>();
+
+ /// # Safety
+ ///
+ /// `dev` must be a valid pointer to the `struct device` embedded in a `struct rtc_device`.
+ /// `tm` must be a valid pointer to a `struct rtc_time`.
+ unsafe extern "C" fn read_time(
+ dev: *mut bindings::device,
+ tm: *mut bindings::rtc_time,
+ ) -> c_int {
+ // SAFETY: The caller ensures that `dev` is a valid pointer to a `struct device`.
+ let device_dev: &device::Device = unsafe { device::Device::from_raw(dev) };
+ // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
+ // `AsBusDevice` to get it.
+ let rtc_dev = unsafe { RtcDevice::<T>::from_device(device_dev) };
+ // SAFETY: The caller ensures that `tm` is valid and writable.
+ // `RtcTime` is `#[repr(transparent)]` over `bindings::rtc_time`, so we can safely cast.
+ let rtc_tm = unsafe { &mut *tm.cast::<RtcTime>() };
+
+ match T::read_time(rtc_dev, rtc_tm) {
+ Ok(()) => 0,
+ Err(err) => err.to_errno(),
+ }
+ }
+
+ /// # Safety
+ ///
+ /// `dev` must be a valid pointer to the `struct device` embedded in a `struct rtc_device`.
+ /// `tm` must be a valid pointer to a `struct rtc_time`.
+ unsafe extern "C" fn set_time(
+ dev: *mut bindings::device,
+ tm: *mut bindings::rtc_time,
+ ) -> c_int {
+ // SAFETY: The caller ensures that `dev` is a valid pointer to a `struct device`.
+ let device_dev: &device::Device = unsafe { device::Device::from_raw(dev) };
+ // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
+ // `AsBusDevice` to get it.
+ let rtc_dev = unsafe { RtcDevice::<T>::from_device(device_dev) };
+ // SAFETY: The caller ensures that `tm` is valid and writable.
+ // `RtcTime` is `#[repr(transparent)]` over `bindings::rtc_time`, so we can safely cast.
+ let rtc_tm = unsafe { &mut *tm.cast::<RtcTime>() };
+
+ match T::set_time(rtc_dev, rtc_tm) {
+ Ok(()) => 0,
+ Err(err) => err.to_errno(),
+ }
+ }
+
+ /// # Safety
+ ///
+ /// `dev` must be a valid pointer to the `struct device` embedded in a `struct rtc_device`.
+ /// `alarm` must be a valid pointer to a `struct rtc_wkalrm`.
+ unsafe extern "C" fn read_alarm(
+ dev: *mut bindings::device,
+ alarm: *mut bindings::rtc_wkalrm,
+ ) -> c_int {
+ // SAFETY: The caller ensures that `dev` is a valid pointer to a `struct device`.
+ let device_dev: &device::Device = unsafe { device::Device::from_raw(dev) };
+ // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
+ // `AsBusDevice` to get it.
+ let rtc_dev = unsafe { RtcDevice::<T>::from_device(device_dev) };
+ // SAFETY: The caller ensures that `alarm` is valid and writable.
+ // `RtcWkAlrm` is `#[repr(transparent)]` over `bindings::rtc_wkalrm`, so we can safely cast.
+ let rtc_alarm = unsafe { &mut *alarm.cast::<RtcWkAlrm>() };
+
+ match T::read_alarm(rtc_dev, rtc_alarm) {
+ Ok(()) => 0,
+ Err(err) => err.to_errno(),
+ }
+ }
+
+ /// # Safety
+ ///
+ /// `dev` must be a valid pointer to the `struct device` embedded in a `struct rtc_device`.
+ /// `alarm` must be a valid pointer to a `struct rtc_wkalrm`.
+ unsafe extern "C" fn set_alarm(
+ dev: *mut bindings::device,
+ alarm: *mut bindings::rtc_wkalrm,
+ ) -> c_int {
+ // SAFETY: The caller ensures that `dev` is a valid pointer to a `struct device`.
+ let device_dev: &device::Device = unsafe { device::Device::from_raw(dev) };
+ // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
+ // `AsBusDevice` to get it.
+ let rtc_dev = unsafe { RtcDevice::<T>::from_device(device_dev) };
+ // SAFETY: The caller ensures that `alarm` is valid and writable.
+ // `RtcWkAlrm` is `#[repr(transparent)]` over `bindings::rtc_wkalrm`, so we can safely cast.
+ let rtc_alarm = unsafe { &mut *alarm.cast::<RtcWkAlrm>() };
+
+ match T::set_alarm(rtc_dev, rtc_alarm) {
+ Ok(()) => 0,
+ Err(err) => err.to_errno(),
+ }
+ }
+
+ /// # Safety
+ ///
+ /// `dev` must be a valid pointer to the `struct device` embedded in a `struct rtc_device`.
+ unsafe extern "C" fn alarm_irq_enable(dev: *mut bindings::device, enabled: c_uint) -> c_int {
+ // SAFETY: The caller ensures that `dev` is a valid pointer to a `struct device`.
+ let device_dev: &device::Device = unsafe { device::Device::from_raw(dev) };
+ // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
+ // `AsBusDevice` to get it.
+ let rtc_dev = unsafe { RtcDevice::<T>::from_device(device_dev) };
+
+ match T::alarm_irq_enable(rtc_dev, enabled) {
+ Ok(()) => 0,
+ Err(err) => err.to_errno(),
+ }
+ }
+
+ /// # Safety
+ ///
+ /// `dev` must be a valid pointer to the `struct device` embedded in a `struct rtc_device`.
+ unsafe extern "C" fn ioctl(dev: *mut bindings::device, cmd: c_uint, arg: c_ulong) -> c_int {
+ // SAFETY: The caller ensures that `dev` is a valid pointer to a `struct device`.
+ let device_dev: &device::Device = unsafe { device::Device::from_raw(dev) };
+ // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
+ // `AsBusDevice` to get it.
+ let rtc_dev = unsafe { RtcDevice::<T>::from_device(device_dev) };
+
+ match T::ioctl(rtc_dev, cmd, arg) {
+ Ok(ret) => ret,
+ Err(err) => err.to_errno(),
+ }
+ }
+
+ /// # Safety
+ ///
+ /// `dev` must be a valid pointer to the `struct device` embedded in a `struct rtc_device`.
+ /// `seq` must be a valid pointer to a `struct seq_file`.
+ unsafe extern "C" fn proc(dev: *mut bindings::device, seq: *mut bindings::seq_file) -> c_int {
+ // SAFETY: The caller ensures that `dev` is a valid pointer to a `struct device`.
+ let device_dev: &device::Device = unsafe { device::Device::from_raw(dev) };
+ // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
+ // `AsBusDevice` to get it.
+ let rtc_dev = unsafe { RtcDevice::<T>::from_device(device_dev) };
+ // SAFETY: The caller ensures that `seq` is valid and writable.
+ let seq_file = unsafe { &mut *seq.cast::<SeqFile>() };
+
+ match T::proc(rtc_dev, seq_file) {
+ Ok(()) => 0,
+ Err(err) => err.to_errno(),
+ }
+ }
+
+ /// # Safety
+ ///
+ /// `dev` must be a valid pointer to the `struct device` embedded in a `struct rtc_device`.
+ /// `offset` must be a valid pointer to a `long`.
+ unsafe extern "C" fn read_offset(dev: *mut bindings::device, offset: *mut c_long) -> c_int {
+ // SAFETY: The caller ensures that `dev` is a valid pointer to a `struct device`.
+ let device_dev: &device::Device = unsafe { device::Device::from_raw(dev) };
+ // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
+ // `AsBusDevice` to get it.
+ let rtc_dev = unsafe { RtcDevice::<T>::from_device(device_dev) };
+ // SAFETY: The caller ensures that `offset` is valid and writable.
+ let mut offset_val: i64 = unsafe { *offset.cast() };
+
+ match T::read_offset(rtc_dev, &mut offset_val) {
+ Ok(()) => {
+ // SAFETY: The caller ensures that `offset` is valid and writable.
+ unsafe { *offset.cast() = offset_val as c_long };
+ 0
+ }
+ Err(err) => err.to_errno(),
+ }
+ }
+
+ /// # Safety
+ ///
+ /// `dev` must be a valid pointer to the `struct device` embedded in a `struct rtc_device`.
+ unsafe extern "C" fn set_offset(dev: *mut bindings::device, offset: c_long) -> c_int {
+ // SAFETY: The caller ensures that `dev` is a valid pointer to a `struct device`.
+ let device_dev: &device::Device = unsafe { device::Device::from_raw(dev) };
+ // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
+ // `AsBusDevice` to get it.
+ let rtc_dev = unsafe { RtcDevice::<T>::from_device(device_dev) };
+
+ match T::set_offset(rtc_dev, offset as i64) {
+ Ok(()) => 0,
+ Err(err) => err.to_errno(),
+ }
+ }
+
+ /// # Safety
+ ///
+ /// `dev` must be a valid pointer to the `struct device` embedded in a `struct rtc_device`.
+ /// `param` must be a valid pointer to a `struct rtc_param`.
+ unsafe extern "C" fn param_get(
+ dev: *mut bindings::device,
+ param: *mut bindings::rtc_param,
+ ) -> c_int {
+ // SAFETY: The caller ensures that `dev` is a valid pointer to a `struct device`.
+ let device_dev: &device::Device = unsafe { device::Device::from_raw(dev) };
+ // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
+ // `AsBusDevice` to get it.
+ let rtc_dev = unsafe { RtcDevice::<T>::from_device(device_dev) };
+ // SAFETY: The caller ensures that `param` is valid and writable.
+ // `RtcParam` is `#[repr(transparent)]` over `bindings::rtc_param`, so we can safely cast.
+ let rtc_param = unsafe { &mut *param.cast::<RtcParam>() };
+
+ match T::param_get(rtc_dev, rtc_param) {
+ Ok(()) => 0,
+ Err(err) => err.to_errno(),
+ }
+ }
+
+ /// # Safety
+ ///
+ /// `dev` must be a valid pointer to the `struct device` embedded in a `struct rtc_device`.
+ /// `param` must be a valid pointer to a `struct rtc_param`.
+ unsafe extern "C" fn param_set(
+ dev: *mut bindings::device,
+ param: *mut bindings::rtc_param,
+ ) -> c_int {
+ // SAFETY: The caller ensures that `dev` is a valid pointer to a `struct device`.
+ let device_dev: &device::Device = unsafe { device::Device::from_raw(dev) };
+ // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
+ // `AsBusDevice` to get it.
+ let rtc_dev = unsafe { RtcDevice::<T>::from_device(device_dev) };
+ // SAFETY: The caller ensures that `param` is valid and writable.
+ // `RtcParam` is `#[repr(transparent)]` over `bindings::rtc_param`, so we can safely cast.
+ let rtc_param = unsafe { &mut *param.cast::<RtcParam>() };
+
+ match T::param_set(rtc_dev, rtc_param) {
+ Ok(()) => 0,
+ Err(err) => err.to_errno(),
+ }
+ }
+}
+
+/// VTable structure wrapper for RTC operations.
+/// Mirrors [`struct rtc_class_ops`](srctree/include/linux/rtc.h).
+#[repr(transparent)]
+pub struct RtcOpsVTable(bindings::rtc_class_ops);
+
+// SAFETY: RtcOpsVTable is Send. The vtable contains only function pointers,
+// which are simple data types that can be safely moved across threads.
+// The thread-safety of calling these functions is handled by the kernel's
+// locking mechanisms.
+unsafe impl Send for RtcOpsVTable {}
+
+// SAFETY: RtcOpsVTable is Sync. The vtable is immutable after it is created,
+// so it can be safely referenced and accessed concurrently by multiple threads
+// e.g. to read the function pointers.
+unsafe impl Sync for RtcOpsVTable {}
+
+impl RtcOpsVTable {
+ /// Returns a raw pointer to the underlying `rtc_class_ops` struct.
+ pub(crate) const fn as_raw(&self) -> *const bindings::rtc_class_ops {
+ &self.0
+ }
+}
+
+/// Creates an RTC operations vtable for a type `T` that implements `RtcOps`.
+///
+/// This is used to bridge Rust trait implementations to the C `struct rtc_class_ops`
+/// expected by the kernel.
+pub const fn create_rtc_ops<T: RtcOps>() -> RtcOpsVTable {
+ let mut ops: bindings::rtc_class_ops = pin_init::zeroed();
+
+ ops.read_time = if T::HAS_READ_TIME {
+ Some(Adapter::<T>::read_time)
+ } else {
+ None
+ };
+ ops.set_time = if T::HAS_SET_TIME {
+ Some(Adapter::<T>::set_time)
+ } else {
+ None
+ };
+ ops.read_alarm = if T::HAS_READ_ALARM {
+ Some(Adapter::<T>::read_alarm)
+ } else {
+ None
+ };
+ ops.set_alarm = if T::HAS_SET_ALARM {
+ Some(Adapter::<T>::set_alarm)
+ } else {
+ None
+ };
+ ops.alarm_irq_enable = if T::HAS_ALARM_IRQ_ENABLE {
+ Some(Adapter::<T>::alarm_irq_enable)
+ } else {
+ None
+ };
+ ops.ioctl = if T::HAS_IOCTL {
+ Some(Adapter::<T>::ioctl)
+ } else {
+ None
+ };
+ ops.proc_ = if T::HAS_PROC {
+ Some(Adapter::<T>::proc)
+ } else {
+ None
+ };
+ ops.read_offset = if T::HAS_READ_OFFSET {
+ Some(Adapter::<T>::read_offset)
+ } else {
+ None
+ };
+ ops.set_offset = if T::HAS_SET_OFFSET {
+ Some(Adapter::<T>::set_offset)
+ } else {
+ None
+ };
+ ops.param_get = if T::HAS_PARAM_GET {
+ Some(Adapter::<T>::param_get)
+ } else {
+ None
+ };
+ ops.param_set = if T::HAS_PARAM_SET {
+ Some(Adapter::<T>::param_set)
+ } else {
+ None
+ };
+
+ RtcOpsVTable(ops)
+}
+
+/// Declares a kernel module that exposes a single RTC AMBA driver.
+///
+/// # Examples
+///
+///```ignore
+/// kernel::module_rtc_amba_driver! {
+/// type: MyDriver,
+/// name: "Module name",
+/// authors: ["Author name"],
+/// description: "Description",
+/// license: "GPL v2",
+/// }
+///```
+#[macro_export]
+macro_rules! module_rtc_amba_driver {
+ ($($user_args:tt)*) => {
+ $crate::module_amba_driver! {
+ $($user_args)*
+ imports_ns: ["RTC"],
+ }
+ };
+}
--
2.43.0
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [RFC PATCH v2 5/5] rust: add PL031 RTC driver
2026-01-07 14:37 [RFC PATCH v2 0/5] rust: Add RTC driver support Ke Sun
` (3 preceding siblings ...)
2026-01-07 14:37 ` [RFC PATCH v2 4/5] rust: add RTC core abstractions and data structures Ke Sun
@ 2026-01-07 14:37 ` Ke Sun
2026-01-08 11:57 ` Danilo Krummrich
4 siblings, 1 reply; 32+ messages in thread
From: Ke Sun @ 2026-01-07 14:37 UTC (permalink / raw)
To: Alexandre Belloni, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich
Cc: linux-rtc, rust-for-linux, Alvin Sun, Ke Sun
Add Rust implementation of the PL031 RTC driver.
Signed-off-by: Ke Sun <sunke@kylinos.cn>
---
drivers/rtc/Kconfig | 9 +
drivers/rtc/Makefile | 1 +
drivers/rtc/rtc_pl031_rust.rs | 503 ++++++++++++++++++++++++++++++++++
3 files changed, 513 insertions(+)
create mode 100644 drivers/rtc/rtc_pl031_rust.rs
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 50dc779f7f983..137cea1824edd 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1591,6 +1591,15 @@ config RTC_DRV_PL031
To compile this driver as a module, choose M here: the
module will be called rtc-pl031.
+config RTC_DRV_PL031_RUST
+ tristate "ARM AMBA PL031 RTC (Rust)"
+ depends on RUST && RTC_CLASS
+ help
+ This is the Rust implementation of the PL031 RTC driver.
+ It provides the same functionality as the C driver but is
+ written in Rust for improved memory safety. The driver supports
+ ARM, ST v1, and ST v2 variants of the PL031 RTC controller.
+
config RTC_DRV_AT91RM9200
tristate "AT91RM9200 or some AT91SAM9 RTC"
depends on ARCH_AT91 || COMPILE_TEST
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 6cf7e066314e1..10f540e7409b4 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -139,6 +139,7 @@ obj-$(CONFIG_RTC_DRV_PCF8583) += rtc-pcf8583.o
obj-$(CONFIG_RTC_DRV_PIC32) += rtc-pic32.o
obj-$(CONFIG_RTC_DRV_PL030) += rtc-pl030.o
obj-$(CONFIG_RTC_DRV_PL031) += rtc-pl031.o
+obj-$(CONFIG_RTC_DRV_PL031_RUST) += rtc_pl031_rust.o
obj-$(CONFIG_RTC_DRV_PM8XXX) += rtc-pm8xxx.o
obj-$(CONFIG_RTC_DRV_POLARFIRE_SOC) += rtc-mpfs.o
obj-$(CONFIG_RTC_DRV_PS3) += rtc-ps3.o
diff --git a/drivers/rtc/rtc_pl031_rust.rs b/drivers/rtc/rtc_pl031_rust.rs
new file mode 100644
index 0000000000000..f3cca5c6daa1b
--- /dev/null
+++ b/drivers/rtc/rtc_pl031_rust.rs
@@ -0,0 +1,503 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+//! Rust ARM AMBA PrimeCell 031 RTC driver
+//!
+//! This driver provides Real Time Clock functionality for ARM AMBA PrimeCell 031
+//! RTC controllers and their ST Microelectronics derivatives.
+
+use core::{
+ marker::PhantomPinned,
+ ops::Deref, //
+};
+use kernel::{
+ amba,
+ bindings,
+ c_str,
+ device::{
+ self,
+ Core, //
+ },
+ devres::Devres,
+ io::mem::IoMem,
+ irq::{
+ Handler,
+ IrqReturn, //
+ },
+ prelude::*,
+ rtc::{
+ self,
+ Registration,
+ RtcDevice,
+ RtcOps,
+ RtcTime,
+ RtcWkAlrm, //
+ },
+ sync::aref::ARef, //
+};
+
+// Register offsets
+const RTC_DR: usize = 0x00;
+const RTC_MR: usize = 0x04;
+const RTC_LR: usize = 0x08;
+const RTC_CR: usize = 0x0c;
+const RTC_IMSC: usize = 0x10;
+const RTC_RIS: usize = 0x14;
+const RTC_MIS: usize = 0x18;
+const RTC_ICR: usize = 0x1c;
+// ST variants have additional timer functionality
+#[allow(dead_code)]
+const RTC_TDR: usize = 0x20;
+#[allow(dead_code)]
+const RTC_TLR: usize = 0x24;
+#[allow(dead_code)]
+const RTC_TCR: usize = 0x28;
+const RTC_YDR: usize = 0x30;
+const RTC_YMR: usize = 0x34;
+const RTC_YLR: usize = 0x38;
+const PL031_REG_SIZE: usize = RTC_YLR + 4;
+
+// Control register bits
+const RTC_CR_EN: u32 = 1 << 0;
+const RTC_CR_CWEN: u32 = 1 << 26;
+
+#[allow(dead_code)]
+const RTC_TCR_EN: u32 = 1 << 1;
+
+// Interrupt status and control register bits
+const RTC_BIT_AI: u32 = 1 << 0;
+#[allow(dead_code)]
+const RTC_BIT_PI: u32 = 1 << 1;
+
+// RTC event flags
+#[allow(dead_code)]
+const RTC_AF: u32 = bindings::RTC_AF;
+#[allow(dead_code)]
+const RTC_IRQF: u32 = bindings::RTC_IRQF;
+
+// ST v2 time format bit definitions
+const RTC_SEC_SHIFT: u32 = 0;
+const RTC_SEC_MASK: u32 = 0x3F << RTC_SEC_SHIFT;
+const RTC_MIN_SHIFT: u32 = 6;
+const RTC_MIN_MASK: u32 = 0x3F << RTC_MIN_SHIFT;
+const RTC_HOUR_SHIFT: u32 = 12;
+const RTC_HOUR_MASK: u32 = 0x1F << RTC_HOUR_SHIFT;
+const RTC_WDAY_SHIFT: u32 = 17;
+const RTC_WDAY_MASK: u32 = 0x7 << RTC_WDAY_SHIFT;
+const RTC_MDAY_SHIFT: u32 = 20;
+const RTC_MDAY_MASK: u32 = 0x1F << RTC_MDAY_SHIFT;
+const RTC_MON_SHIFT: u32 = 25;
+const RTC_MON_MASK: u32 = 0xF << RTC_MON_SHIFT;
+
+/// Vendor-specific variant identifier for PL031 RTC controllers.
+#[derive(Copy, Clone, PartialEq)]
+enum VendorVariant {
+ /// Original ARM version with 32-bit Unix timestamp format.
+ Arm,
+ /// First ST derivative with clockwatch mode and weekday support.
+ StV1,
+ /// Second ST derivative with packed BCD time format and year register.
+ StV2,
+}
+
+impl VendorVariant {
+ fn clockwatch(&self) -> bool {
+ matches!(self, VendorVariant::StV1 | VendorVariant::StV2)
+ }
+
+ #[allow(dead_code)]
+ fn st_weekday(&self) -> bool {
+ matches!(self, VendorVariant::StV1 | VendorVariant::StV2)
+ }
+
+ #[allow(dead_code)]
+ fn range_min(&self) -> i64 {
+ match self {
+ VendorVariant::Arm | VendorVariant::StV1 => 0,
+ VendorVariant::StV2 => bindings::RTC_TIMESTAMP_BEGIN_0000,
+ }
+ }
+
+ #[allow(dead_code)]
+ fn range_max(&self) -> u64 {
+ match self {
+ VendorVariant::Arm | VendorVariant::StV1 => u64::from(u32::MAX),
+ VendorVariant::StV2 => bindings::RTC_TIMESTAMP_END_9999,
+ }
+ }
+}
+
+/// The driver's private data struct. It holds all necessary devres managed resources.
+#[pin_data(PinnedDrop)]
+struct Pl031DrvData {
+ #[pin]
+ base: Devres<IoMem<PL031_REG_SIZE>>,
+ variant: VendorVariant,
+}
+
+// SAFETY: `Pl031DrvData` contains only `Send`/`Sync` types: `Devres` (Send+Sync)
+// and `VendorVariant` (Copy).
+unsafe impl Send for Pl031DrvData {}
+// SAFETY: `Pl031DrvData` contains only `Send`/`Sync` types: `Devres` (Send+Sync)
+// and `VendorVariant` (Copy).
+unsafe impl Sync for Pl031DrvData {}
+
+/// Vendor-specific variant identifier used in AMBA device table.
+#[derive(Copy, Clone)]
+struct Pl031Variant {
+ variant: VendorVariant,
+}
+
+impl Pl031Variant {
+ const ARM: Self = Self {
+ variant: VendorVariant::Arm,
+ };
+ const STV1: Self = Self {
+ variant: VendorVariant::StV1,
+ };
+ const STV2: Self = Self {
+ variant: VendorVariant::StV2,
+ };
+}
+
+// Use AMBA device table for matching
+kernel::amba_device_table!(
+ ID_TABLE,
+ MODULE_ID_TABLE,
+ <Pl031AmbaDriver as amba::Driver>::IdInfo,
+ [
+ (
+ amba::DeviceId::new(0x00041031, 0x000fffff),
+ Pl031Variant::ARM
+ ),
+ (
+ amba::DeviceId::new(0x00180031, 0x00ffffff),
+ Pl031Variant::STV1
+ ),
+ (
+ amba::DeviceId::new(0x00280031, 0x00ffffff),
+ Pl031Variant::STV2
+ ),
+ ]
+);
+
+struct Pl031AmbaDriver;
+
+impl amba::Driver for Pl031AmbaDriver {
+ type IdInfo = Pl031Variant;
+ const AMBA_ID_TABLE: Option<amba::IdTable<Self::IdInfo>> = Some(&ID_TABLE);
+
+ fn probe(
+ adev: &amba::Device<Core>,
+ id_info: Option<&Self::IdInfo>,
+ ) -> impl PinInit<Self, Error> {
+ let dev = adev.as_ref();
+ let io_request = adev.io_request().ok_or(ENODEV)?;
+ let variant = id_info
+ .map(|info| info.variant)
+ .unwrap_or(VendorVariant::Arm);
+
+ let rtcdev = RtcDevice::<Pl031DrvData>::new(
+ dev,
+ try_pin_init!(Pl031DrvData {
+ base <- IoMem::new(io_request),
+ variant,
+ }),
+ )?;
+
+ dev.devm_init_wakeup()?;
+
+ let drvdata = rtcdev.drvdata()?;
+ let base_guard = drvdata.base.try_access().ok_or(ENXIO)?;
+ let base = base_guard.deref();
+
+ let mut cr = base.read32(RTC_CR);
+ if variant.clockwatch() {
+ cr |= RTC_CR_CWEN;
+ } else {
+ cr |= RTC_CR_EN;
+ }
+ base.write32(cr, RTC_CR);
+
+ if variant.st_weekday() {
+ let bcd_year = base.read32(RTC_YDR);
+ if bcd_year == 0x2000 {
+ let st_time = base.read32(RTC_DR);
+ if (st_time & (RTC_MON_MASK | RTC_MDAY_MASK | RTC_WDAY_MASK)) == 0x02120000 {
+ base.write32(0x2000, RTC_YLR);
+ base.write32(st_time | (0x7 << RTC_WDAY_SHIFT), RTC_LR);
+ }
+ }
+ }
+
+ rtcdev.set_range_min(variant.range_min());
+ rtcdev.set_range_max(variant.range_max());
+
+ let irq_flags = if variant == VendorVariant::StV2 {
+ kernel::irq::Flags::SHARED | kernel::irq::Flags::COND_SUSPEND
+ } else {
+ kernel::irq::Flags::SHARED
+ };
+
+ let rtcdev_clone = rtcdev.clone();
+ let init = adev.request_irq_by_index(
+ irq_flags,
+ 0,
+ c_str!("rtc-pl031"),
+ try_pin_init!(Pl031IrqHandler {
+ _pin: PhantomPinned,
+ rtcdev: rtcdev_clone,
+ }),
+ );
+
+ match kernel::devres::register(dev, init, GFP_KERNEL) {
+ Ok(()) => {
+ if let Ok(irq) = adev.irq_by_index(0) {
+ irq.set_wake_irq()?;
+ }
+ }
+ Err(_) => rtcdev.clear_feature(bindings::RTC_FEATURE_ALARM),
+ }
+
+ Registration::<Pl031DrvData>::register(dev, rtcdev)?;
+ Ok(Pl031AmbaDriver)
+ }
+}
+
+#[pinned_drop]
+impl PinnedDrop for Pl031DrvData {
+ fn drop(self: Pin<&mut Self>) {
+ // Resources are automatically cleaned up by devres.
+ }
+}
+
+/// Converts a Gregorian date to ST v2 RTC packed BCD format.
+///
+/// Returns a tuple of (packed_time, bcd_year) where packed_time contains
+/// month, day, weekday, hour, minute, and second in a single 32-bit value.
+fn stv2_tm_to_time(tm: &RtcTime) -> Result<(u32, u32)> {
+ let year = tm.tm_year() + 1900;
+ let mut wday = tm.tm_wday();
+
+ // Hardware wday masking doesn't work, so wday must be valid.
+ if !(-1..=6).contains(&wday) {
+ return Err(EINVAL);
+ } else if wday == -1 {
+ // wday is not provided, calculate it here.
+ let time64 = tm.to_time64();
+ let mut calc_tm = RtcTime::default();
+ calc_tm.set_from_time64(time64);
+ wday = calc_tm.tm_wday();
+ }
+
+ // Convert year to BCD.
+ let bcd_year =
+ (u32::from(bin2bcd((year % 100) as u8))) | (u32::from(bin2bcd((year / 100) as u8)) << 8);
+
+ let st_time = ((tm.tm_mon() + 1) as u32) << RTC_MON_SHIFT
+ | (tm.tm_mday() as u32) << RTC_MDAY_SHIFT
+ | ((wday + 1) as u32) << RTC_WDAY_SHIFT
+ | (tm.tm_hour() as u32) << RTC_HOUR_SHIFT
+ | (tm.tm_min() as u32) << RTC_MIN_SHIFT
+ | (tm.tm_sec() as u32) << RTC_SEC_SHIFT;
+
+ Ok((st_time, bcd_year))
+}
+
+/// Converts ST v2 RTC packed BCD format to a Gregorian date.
+///
+/// Extracts time components from the packed 32-bit value and BCD year register,
+/// then populates the RtcTime structure.
+fn stv2_time_to_tm(st_time: u32, bcd_year: u32, tm: &mut RtcTime) {
+ let year_low = bcd2bin((bcd_year & 0xFF) as u8);
+ let year_high = bcd2bin(((bcd_year >> 8) & 0xFF) as u8);
+ tm.set_tm_year(i32::from(year_low) + i32::from(year_high) * 100);
+ tm.set_tm_mon((((st_time & RTC_MON_MASK) >> RTC_MON_SHIFT) - 1) as i32);
+ tm.set_tm_mday(((st_time & RTC_MDAY_MASK) >> RTC_MDAY_SHIFT) as i32);
+ tm.set_tm_wday((((st_time & RTC_WDAY_MASK) >> RTC_WDAY_SHIFT) - 1) as i32);
+ tm.set_tm_hour(((st_time & RTC_HOUR_MASK) >> RTC_HOUR_SHIFT) as i32);
+ tm.set_tm_min(((st_time & RTC_MIN_MASK) >> RTC_MIN_SHIFT) as i32);
+ tm.set_tm_sec(((st_time & RTC_SEC_MASK) >> RTC_SEC_SHIFT) as i32);
+
+ // Values are from valid RTC time structures and are non-negative.
+ tm.set_tm_yday(tm.year_days());
+ tm.set_tm_year(tm.tm_year() - 1900);
+}
+
+/// Converts a binary value to BCD.
+fn bin2bcd(val: u8) -> u8 {
+ ((val / 10) << 4) | (val % 10)
+}
+
+/// Converts a BCD value to binary.
+fn bcd2bin(val: u8) -> u8 {
+ ((val >> 4) * 10) + (val & 0x0F)
+}
+
+/// Interrupt handler for PL031 RTC alarm events.
+#[pin_data]
+struct Pl031IrqHandler {
+ #[pin]
+ _pin: PhantomPinned,
+ rtcdev: ARef<RtcDevice<Pl031DrvData>>,
+}
+
+impl Handler for Pl031IrqHandler {
+ fn handle(&self, _dev: &device::Device<device::Bound>) -> IrqReturn {
+ // Get driver data using drvdata.
+ let drvdata = match self.rtcdev.drvdata() {
+ Ok(drvdata) => drvdata,
+ Err(_) => return IrqReturn::None,
+ };
+
+ // Access the MMIO base.
+ let base_guard = match drvdata.base.try_access() {
+ Some(guard) => guard,
+ None => return IrqReturn::None,
+ };
+ let base = base_guard.deref();
+
+ // Read masked interrupt status.
+ let rtcmis = base.read32(RTC_MIS);
+
+ if (rtcmis & RTC_BIT_AI) != 0 {
+ base.write32(RTC_BIT_AI, RTC_ICR);
+ self.rtcdev.update_irq(1, (RTC_AF | RTC_IRQF) as usize);
+ return IrqReturn::Handled;
+ }
+
+ IrqReturn::None
+ }
+}
+
+#[vtable]
+impl RtcOps for Pl031DrvData {
+ fn read_time(rtcdev: &RtcDevice<Self>, tm: &mut RtcTime) -> Result {
+ let drvdata = rtcdev.drvdata()?;
+ let base_guard = drvdata.base.try_access().ok_or(ENXIO)?;
+ let base = base_guard.deref();
+
+ match drvdata.variant {
+ VendorVariant::Arm | VendorVariant::StV1 => {
+ let time32: u32 = base.read32(RTC_DR);
+ let time64 = i64::from(time32);
+ tm.set_from_time64(time64);
+ }
+ VendorVariant::StV2 => {
+ let st_time = base.read32(RTC_DR);
+ let bcd_year = base.read32(RTC_YDR);
+ stv2_time_to_tm(st_time, bcd_year, tm);
+ }
+ }
+
+ Ok(())
+ }
+
+ fn set_time(rtcdev: &RtcDevice<Self>, tm: &mut RtcTime) -> Result {
+ let dev: &device::Device<device::Bound> = rtcdev.as_ref();
+ let drvdata = rtcdev.drvdata()?;
+ let base_guard = drvdata.base.try_access().ok_or(ENXIO)?;
+ let base = base_guard.deref();
+
+ match drvdata.variant {
+ VendorVariant::Arm | VendorVariant::StV1 => {
+ let time64 = tm.to_time64();
+ base.write32(time64 as u32, RTC_LR);
+ }
+ VendorVariant::StV2 => {
+ let (st_time, bcd_year) = stv2_tm_to_time(tm).inspect_err(|&err| {
+ if err == EINVAL {
+ dev_err!(dev, "invalid wday value {}\n", tm.tm_wday());
+ }
+ })?;
+ base.write32(bcd_year, RTC_YLR);
+ base.write32(st_time, RTC_LR);
+ }
+ }
+
+ Ok(())
+ }
+
+ fn read_alarm(rtcdev: &RtcDevice<Self>, alarm: &mut RtcWkAlrm) -> Result {
+ let drvdata = rtcdev.drvdata()?;
+ let base_guard = drvdata.base.try_access().ok_or(ENXIO)?;
+ let base = base_guard.deref();
+
+ match drvdata.variant {
+ VendorVariant::Arm | VendorVariant::StV1 => {
+ let time32: u32 = base.read32(RTC_MR);
+ let time64 = i64::from(time32);
+ crate::rtc::RtcTime::time64_to_tm(time64, alarm.get_time_mut());
+ }
+ VendorVariant::StV2 => {
+ let st_time = base.read32(RTC_MR);
+ let bcd_year = base.read32(RTC_YMR);
+ stv2_time_to_tm(st_time, bcd_year, alarm.get_time_mut());
+ }
+ }
+
+ alarm.set_pending(if (base.read32(RTC_RIS) & RTC_BIT_AI) != 0 {
+ 1
+ } else {
+ 0
+ });
+ alarm.set_enabled(if (base.read32(RTC_IMSC) & RTC_BIT_AI) != 0 {
+ 1
+ } else {
+ 0
+ });
+
+ Ok(())
+ }
+
+ fn set_alarm(rtcdev: &RtcDevice<Self>, alarm: &mut RtcWkAlrm) -> Result {
+ let dev: &device::Device<device::Bound> = rtcdev.as_ref();
+ let drvdata = rtcdev.drvdata()?;
+ let base_guard = drvdata.base.try_access().ok_or(ENXIO)?;
+ let base = base_guard.deref();
+
+ match drvdata.variant {
+ VendorVariant::Arm | VendorVariant::StV1 => {
+ let time64 = alarm.get_time().to_time64();
+ base.write32(time64 as u32, RTC_MR);
+ }
+ VendorVariant::StV2 => {
+ let (st_time, bcd_year) =
+ stv2_tm_to_time(alarm.get_time()).inspect_err(|&err| {
+ if err == EINVAL {
+ dev_err!(dev, "invalid wday value {}\n", alarm.get_time().tm_wday());
+ }
+ })?;
+ base.write32(bcd_year, RTC_YMR);
+ base.write32(st_time, RTC_MR);
+ }
+ }
+
+ Self::alarm_irq_enable(rtcdev, u32::from(alarm.enabled()))
+ }
+
+ fn alarm_irq_enable(rtcdev: &RtcDevice<Self>, enabled: u32) -> Result {
+ let drvdata = rtcdev.drvdata()?;
+ let base_guard = drvdata.base.try_access().ok_or(ENXIO)?;
+ let base = base_guard.deref();
+
+ // Clear any pending alarm interrupts.
+ base.write32(RTC_BIT_AI, RTC_ICR);
+
+ let mut imsc = base.read32(RTC_IMSC);
+ if enabled == 1 {
+ imsc |= RTC_BIT_AI;
+ } else {
+ imsc &= !RTC_BIT_AI;
+ }
+ base.write32(imsc, RTC_IMSC);
+
+ Ok(())
+ }
+}
+
+kernel::module_rtc_amba_driver! {
+ type: Pl031AmbaDriver,
+ name: "rtc-pl031-rust",
+ authors: ["Ke Sun <sunke@kylinos.cn>"],
+ description: "Rust PL031 RTC driver",
+ license: "GPL v2",
+}
--
2.43.0
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [RFC PATCH v2 1/5] rtc: migrate driver data to RTC device
2026-01-07 14:37 ` [RFC PATCH v2 1/5] rtc: migrate driver data to RTC device Ke Sun
@ 2026-01-07 14:41 ` Ke Sun
2026-01-07 16:12 ` Greg KH
2026-01-08 11:12 ` Danilo Krummrich
2 siblings, 0 replies; 32+ messages in thread
From: Ke Sun @ 2026-01-07 14:41 UTC (permalink / raw)
To: Alexandre Belloni, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich
Cc: linux-rtc, rust-for-linux, Alvin Sun
This is merely a technical validation. If confirmed viable, I will
update other RTC drivers.
Best regards,
Ke Sun
On 1/7/26 22:37, Ke Sun wrote:
> Unify RTC driver interface by storing driver data on the RTC device
> instead of the parent device. Update RTC ops callbacks to pass the RTC
> device itself rather than its parent. This change enables better
> support for Rust RTC drivers that store data on the RTC device.
>
> Signed-off-by: Ke Sun <sunke@kylinos.cn>
> ---
> drivers/rtc/dev.c | 4 ++--
> drivers/rtc/interface.c | 18 +++++++++---------
> drivers/rtc/rtc-pl031.c | 9 ++-------
> 3 files changed, 13 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/rtc/dev.c b/drivers/rtc/dev.c
> index baf1a8ca8b2b1..0f62ba9342e3e 100644
> --- a/drivers/rtc/dev.c
> +++ b/drivers/rtc/dev.c
> @@ -410,7 +410,7 @@ static long rtc_dev_ioctl(struct file *file,
> }
> default:
> if (rtc->ops->param_get)
> - err = rtc->ops->param_get(rtc->dev.parent, ¶m);
> + err = rtc->ops->param_get(&rtc->dev, ¶m);
> else
> err = -EINVAL;
> }
> @@ -440,7 +440,7 @@ static long rtc_dev_ioctl(struct file *file,
>
> default:
> if (rtc->ops->param_set)
> - err = rtc->ops->param_set(rtc->dev.parent, ¶m);
> + err = rtc->ops->param_set(&rtc->dev, ¶m);
> else
> err = -EINVAL;
> }
> diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
> index b8b298efd9a9c..783a3ec3bb93d 100644
> --- a/drivers/rtc/interface.c
> +++ b/drivers/rtc/interface.c
> @@ -91,7 +91,7 @@ static int __rtc_read_time(struct rtc_device *rtc, struct rtc_time *tm)
> err = -EINVAL;
> } else {
> memset(tm, 0, sizeof(struct rtc_time));
> - err = rtc->ops->read_time(rtc->dev.parent, tm);
> + err = rtc->ops->read_time(&rtc->dev, tm);
> if (err < 0) {
> dev_dbg(&rtc->dev, "read_time: fail to read: %d\n",
> err);
> @@ -155,7 +155,7 @@ int rtc_set_time(struct rtc_device *rtc, struct rtc_time *tm)
> if (!rtc->ops)
> err = -ENODEV;
> else if (rtc->ops->set_time)
> - err = rtc->ops->set_time(rtc->dev.parent, tm);
> + err = rtc->ops->set_time(&rtc->dev, tm);
> else
> err = -EINVAL;
>
> @@ -200,7 +200,7 @@ static int rtc_read_alarm_internal(struct rtc_device *rtc,
> alarm->time.tm_wday = -1;
> alarm->time.tm_yday = -1;
> alarm->time.tm_isdst = -1;
> - err = rtc->ops->read_alarm(rtc->dev.parent, alarm);
> + err = rtc->ops->read_alarm(&rtc->dev, alarm);
> }
>
> mutex_unlock(&rtc->ops_lock);
> @@ -441,7 +441,7 @@ static int __rtc_set_alarm(struct rtc_device *rtc, struct rtc_wkalrm *alarm)
> else if (!test_bit(RTC_FEATURE_ALARM, rtc->features))
> err = -EINVAL;
> else
> - err = rtc->ops->set_alarm(rtc->dev.parent, alarm);
> + err = rtc->ops->set_alarm(&rtc->dev, alarm);
>
> /*
> * Check for potential race described above. If the waiting for next
> @@ -568,7 +568,7 @@ int rtc_alarm_irq_enable(struct rtc_device *rtc, unsigned int enabled)
> else if (!test_bit(RTC_FEATURE_ALARM, rtc->features) || !rtc->ops->alarm_irq_enable)
> err = -EINVAL;
> else
> - err = rtc->ops->alarm_irq_enable(rtc->dev.parent, enabled);
> + err = rtc->ops->alarm_irq_enable(&rtc->dev, enabled);
>
> mutex_unlock(&rtc->ops_lock);
>
> @@ -618,7 +618,7 @@ int rtc_update_irq_enable(struct rtc_device *rtc, unsigned int enabled)
> rtc->uie_rtctimer.period = ktime_set(1, 0);
> err = rtc_timer_enqueue(rtc, &rtc->uie_rtctimer);
> if (!err && rtc->ops && rtc->ops->alarm_irq_enable)
> - err = rtc->ops->alarm_irq_enable(rtc->dev.parent, 1);
> + err = rtc->ops->alarm_irq_enable(&rtc->dev, 1);
> if (err)
> goto out;
> } else {
> @@ -874,7 +874,7 @@ static void rtc_alarm_disable(struct rtc_device *rtc)
> if (!rtc->ops || !test_bit(RTC_FEATURE_ALARM, rtc->features) || !rtc->ops->alarm_irq_enable)
> return;
>
> - rtc->ops->alarm_irq_enable(rtc->dev.parent, false);
> + rtc->ops->alarm_irq_enable(&rtc->dev, false);
> trace_rtc_alarm_irq_enable(0, 0);
> }
>
> @@ -1076,7 +1076,7 @@ int rtc_read_offset(struct rtc_device *rtc, long *offset)
> return -EINVAL;
>
> mutex_lock(&rtc->ops_lock);
> - ret = rtc->ops->read_offset(rtc->dev.parent, offset);
> + ret = rtc->ops->read_offset(&rtc->dev, offset);
> mutex_unlock(&rtc->ops_lock);
>
> trace_rtc_read_offset(*offset, ret);
> @@ -1111,7 +1111,7 @@ int rtc_set_offset(struct rtc_device *rtc, long offset)
> return -EINVAL;
>
> mutex_lock(&rtc->ops_lock);
> - ret = rtc->ops->set_offset(rtc->dev.parent, offset);
> + ret = rtc->ops->set_offset(&rtc->dev, offset);
> mutex_unlock(&rtc->ops_lock);
>
> trace_rtc_set_offset(offset, ret);
> diff --git a/drivers/rtc/rtc-pl031.c b/drivers/rtc/rtc-pl031.c
> index eab39dfa4e5fe..a605034d44cb7 100644
> --- a/drivers/rtc/rtc-pl031.c
> +++ b/drivers/rtc/rtc-pl031.c
> @@ -284,10 +284,6 @@ static int pl031_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
>
> static void pl031_remove(struct amba_device *adev)
> {
> - struct pl031_local *ldata = dev_get_drvdata(&adev->dev);
> -
> - if (adev->irq[0])
> - free_irq(adev->irq[0], ldata);
> amba_release_regions(adev);
> }
>
> @@ -320,8 +316,6 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
> goto out;
> }
>
> - amba_set_drvdata(adev, ldata);
> -
> dev_dbg(&adev->dev, "designer ID = 0x%02x\n", amba_manf(adev));
> dev_dbg(&adev->dev, "revision = 0x%01x\n", amba_rev(adev));
>
> @@ -356,6 +350,7 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
> ret = PTR_ERR(ldata->rtc);
> goto out;
> }
> + dev_set_drvdata(&ldata->rtc->dev, ldata);
>
> if (!adev->irq[0])
> clear_bit(RTC_FEATURE_ALARM, ldata->rtc->features);
> @@ -369,7 +364,7 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
> goto out;
>
> if (adev->irq[0]) {
> - ret = request_irq(adev->irq[0], pl031_interrupt,
> + ret = devm_request_irq(&adev->dev, adev->irq[0], pl031_interrupt,
> vendor->irqflags, "rtc-pl031", ldata);
> if (ret)
> goto out;
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH v2 3/5] rust: add device wakeup capability support
2026-01-07 14:37 ` [RFC PATCH v2 3/5] rust: add device wakeup capability support Ke Sun
@ 2026-01-07 14:57 ` Greg KH
2026-01-07 23:35 ` Ke Sun
0 siblings, 1 reply; 32+ messages in thread
From: Greg KH @ 2026-01-07 14:57 UTC (permalink / raw)
To: Ke Sun
Cc: Alexandre Belloni, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, linux-rtc, rust-for-linux,
Alvin Sun
On Wed, Jan 07, 2026 at 10:37:35PM +0800, Ke Sun wrote:
> Add Rust bindings and wrappers for device wakeup functionality,
> including devm_device_init_wakeup() and dev_pm_set_wake_irq().
>
> Signed-off-by: Ke Sun <sunke@kylinos.cn>
> ---
> rust/bindings/bindings_helper.h | 2 ++
> rust/helpers/device.c | 7 +++++++
> rust/kernel/device.rs | 17 ++++++++++++++++-
> rust/kernel/irq/request.rs | 7 +++++++
> 4 files changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index fa697287cf71b..d6c2b06ac4107 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -88,6 +88,8 @@
> #include <linux/workqueue.h>
> #include <linux/xarray.h>
> #include <trace/events/rust_sample.h>
> +#include <linux/pm_wakeup.h>
> +#include <linux/pm_wakeirq.h>
Aren't these sorted?
> /*
> * The driver-core Rust code needs to know about some C driver-core private
> diff --git a/rust/helpers/device.c b/rust/helpers/device.c
> index 9a4316bafedfb..cae26edd83696 100644
> --- a/rust/helpers/device.c
> +++ b/rust/helpers/device.c
> @@ -1,6 +1,8 @@
> // SPDX-License-Identifier: GPL-2.0
>
> #include <linux/device.h>
> +#include <linux/pm_wakeup.h>
> +#include <linux/pm_wakeirq.h>
Why are both of these needed for just one function call?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH v2 1/5] rtc: migrate driver data to RTC device
2026-01-07 14:37 ` [RFC PATCH v2 1/5] rtc: migrate driver data to RTC device Ke Sun
2026-01-07 14:41 ` Ke Sun
@ 2026-01-07 16:12 ` Greg KH
2026-01-07 23:18 ` Ke Sun
2026-01-08 11:12 ` Danilo Krummrich
2 siblings, 1 reply; 32+ messages in thread
From: Greg KH @ 2026-01-07 16:12 UTC (permalink / raw)
To: Ke Sun
Cc: Alexandre Belloni, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, linux-rtc, rust-for-linux,
Alvin Sun
On Wed, Jan 07, 2026 at 10:37:33PM +0800, Ke Sun wrote:
> Unify RTC driver interface by storing driver data on the RTC device
> instead of the parent device. Update RTC ops callbacks to pass the RTC
> device itself rather than its parent. This change enables better
> support for Rust RTC drivers that store data on the RTC device.
>
> Signed-off-by: Ke Sun <sunke@kylinos.cn>
> ---
> drivers/rtc/dev.c | 4 ++--
> drivers/rtc/interface.c | 18 +++++++++---------
> drivers/rtc/rtc-pl031.c | 9 ++-------
> 3 files changed, 13 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/rtc/dev.c b/drivers/rtc/dev.c
> index baf1a8ca8b2b1..0f62ba9342e3e 100644
> --- a/drivers/rtc/dev.c
> +++ b/drivers/rtc/dev.c
> @@ -410,7 +410,7 @@ static long rtc_dev_ioctl(struct file *file,
> }
> default:
> if (rtc->ops->param_get)
> - err = rtc->ops->param_get(rtc->dev.parent, ¶m);
> + err = rtc->ops->param_get(&rtc->dev, ¶m);
> else
> err = -EINVAL;
> }
> @@ -440,7 +440,7 @@ static long rtc_dev_ioctl(struct file *file,
>
> default:
> if (rtc->ops->param_set)
> - err = rtc->ops->param_set(rtc->dev.parent, ¶m);
> + err = rtc->ops->param_set(&rtc->dev, ¶m);
> else
> err = -EINVAL;
> }
> diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
> index b8b298efd9a9c..783a3ec3bb93d 100644
> --- a/drivers/rtc/interface.c
> +++ b/drivers/rtc/interface.c
> @@ -91,7 +91,7 @@ static int __rtc_read_time(struct rtc_device *rtc, struct rtc_time *tm)
> err = -EINVAL;
> } else {
> memset(tm, 0, sizeof(struct rtc_time));
> - err = rtc->ops->read_time(rtc->dev.parent, tm);
> + err = rtc->ops->read_time(&rtc->dev, tm);
> if (err < 0) {
> dev_dbg(&rtc->dev, "read_time: fail to read: %d\n",
> err);
> @@ -155,7 +155,7 @@ int rtc_set_time(struct rtc_device *rtc, struct rtc_time *tm)
> if (!rtc->ops)
> err = -ENODEV;
> else if (rtc->ops->set_time)
> - err = rtc->ops->set_time(rtc->dev.parent, tm);
> + err = rtc->ops->set_time(&rtc->dev, tm);
> else
> err = -EINVAL;
>
> @@ -200,7 +200,7 @@ static int rtc_read_alarm_internal(struct rtc_device *rtc,
> alarm->time.tm_wday = -1;
> alarm->time.tm_yday = -1;
> alarm->time.tm_isdst = -1;
> - err = rtc->ops->read_alarm(rtc->dev.parent, alarm);
> + err = rtc->ops->read_alarm(&rtc->dev, alarm);
> }
>
> mutex_unlock(&rtc->ops_lock);
> @@ -441,7 +441,7 @@ static int __rtc_set_alarm(struct rtc_device *rtc, struct rtc_wkalrm *alarm)
> else if (!test_bit(RTC_FEATURE_ALARM, rtc->features))
> err = -EINVAL;
> else
> - err = rtc->ops->set_alarm(rtc->dev.parent, alarm);
> + err = rtc->ops->set_alarm(&rtc->dev, alarm);
>
> /*
> * Check for potential race described above. If the waiting for next
> @@ -568,7 +568,7 @@ int rtc_alarm_irq_enable(struct rtc_device *rtc, unsigned int enabled)
> else if (!test_bit(RTC_FEATURE_ALARM, rtc->features) || !rtc->ops->alarm_irq_enable)
> err = -EINVAL;
> else
> - err = rtc->ops->alarm_irq_enable(rtc->dev.parent, enabled);
> + err = rtc->ops->alarm_irq_enable(&rtc->dev, enabled);
>
> mutex_unlock(&rtc->ops_lock);
>
> @@ -618,7 +618,7 @@ int rtc_update_irq_enable(struct rtc_device *rtc, unsigned int enabled)
> rtc->uie_rtctimer.period = ktime_set(1, 0);
> err = rtc_timer_enqueue(rtc, &rtc->uie_rtctimer);
> if (!err && rtc->ops && rtc->ops->alarm_irq_enable)
> - err = rtc->ops->alarm_irq_enable(rtc->dev.parent, 1);
> + err = rtc->ops->alarm_irq_enable(&rtc->dev, 1);
> if (err)
> goto out;
> } else {
> @@ -874,7 +874,7 @@ static void rtc_alarm_disable(struct rtc_device *rtc)
> if (!rtc->ops || !test_bit(RTC_FEATURE_ALARM, rtc->features) || !rtc->ops->alarm_irq_enable)
> return;
>
> - rtc->ops->alarm_irq_enable(rtc->dev.parent, false);
> + rtc->ops->alarm_irq_enable(&rtc->dev, false);
> trace_rtc_alarm_irq_enable(0, 0);
> }
>
> @@ -1076,7 +1076,7 @@ int rtc_read_offset(struct rtc_device *rtc, long *offset)
> return -EINVAL;
>
> mutex_lock(&rtc->ops_lock);
> - ret = rtc->ops->read_offset(rtc->dev.parent, offset);
> + ret = rtc->ops->read_offset(&rtc->dev, offset);
> mutex_unlock(&rtc->ops_lock);
>
> trace_rtc_read_offset(*offset, ret);
> @@ -1111,7 +1111,7 @@ int rtc_set_offset(struct rtc_device *rtc, long offset)
> return -EINVAL;
>
> mutex_lock(&rtc->ops_lock);
> - ret = rtc->ops->set_offset(rtc->dev.parent, offset);
> + ret = rtc->ops->set_offset(&rtc->dev, offset);
> mutex_unlock(&rtc->ops_lock);
>
> trace_rtc_set_offset(offset, ret);
> diff --git a/drivers/rtc/rtc-pl031.c b/drivers/rtc/rtc-pl031.c
> index eab39dfa4e5fe..a605034d44cb7 100644
> --- a/drivers/rtc/rtc-pl031.c
> +++ b/drivers/rtc/rtc-pl031.c
> @@ -284,10 +284,6 @@ static int pl031_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
>
> static void pl031_remove(struct amba_device *adev)
> {
> - struct pl031_local *ldata = dev_get_drvdata(&adev->dev);
> -
> - if (adev->irq[0])
> - free_irq(adev->irq[0], ldata);
> amba_release_regions(adev);
> }
>
> @@ -320,8 +316,6 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
> goto out;
> }
>
> - amba_set_drvdata(adev, ldata);
> -
> dev_dbg(&adev->dev, "designer ID = 0x%02x\n", amba_manf(adev));
> dev_dbg(&adev->dev, "revision = 0x%01x\n", amba_rev(adev));
>
> @@ -356,6 +350,7 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
> ret = PTR_ERR(ldata->rtc);
> goto out;
> }
> + dev_set_drvdata(&ldata->rtc->dev, ldata);
>
> if (!adev->irq[0])
> clear_bit(RTC_FEATURE_ALARM, ldata->rtc->features);
> @@ -369,7 +364,7 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
> goto out;
>
> if (adev->irq[0]) {
> - ret = request_irq(adev->irq[0], pl031_interrupt,
> + ret = devm_request_irq(&adev->dev, adev->irq[0], pl031_interrupt,
> vendor->irqflags, "rtc-pl031", ldata);
Are you _SURE_ you can use devm for this? it is a functional change,
one that trips lots of people up. I wouldn't make this change without
at least saying why you are doing so in the changelog text, which I
didn't see at all here.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH v2 1/5] rtc: migrate driver data to RTC device
2026-01-07 16:12 ` Greg KH
@ 2026-01-07 23:18 ` Ke Sun
2026-01-08 0:24 ` Ke Sun
2026-01-08 5:46 ` Greg KH
0 siblings, 2 replies; 32+ messages in thread
From: Ke Sun @ 2026-01-07 23:18 UTC (permalink / raw)
To: Greg KH
Cc: Alexandre Belloni, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, linux-rtc, rust-for-linux,
Alvin Sun
On 1/8/26 00:12, Greg KH wrote:
> On Wed, Jan 07, 2026 at 10:37:33PM +0800, Ke Sun wrote:
>> Unify RTC driver interface by storing driver data on the RTC device
>> instead of the parent device. Update RTC ops callbacks to pass the RTC
>> device itself rather than its parent. This change enables better
>> support for Rust RTC drivers that store data on the RTC device.
>>
>> Signed-off-by: Ke Sun <sunke@kylinos.cn>
>> ---
>> drivers/rtc/dev.c | 4 ++--
>> drivers/rtc/interface.c | 18 +++++++++---------
>> drivers/rtc/rtc-pl031.c | 9 ++-------
>> 3 files changed, 13 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/rtc/dev.c b/drivers/rtc/dev.c
>> index baf1a8ca8b2b1..0f62ba9342e3e 100644
>> --- a/drivers/rtc/dev.c
>> +++ b/drivers/rtc/dev.c
>> @@ -410,7 +410,7 @@ static long rtc_dev_ioctl(struct file *file,
>> }
>> default:
>> if (rtc->ops->param_get)
>> - err = rtc->ops->param_get(rtc->dev.parent, ¶m);
>> + err = rtc->ops->param_get(&rtc->dev, ¶m);
>> else
>> err = -EINVAL;
>> }
>> @@ -440,7 +440,7 @@ static long rtc_dev_ioctl(struct file *file,
>>
>> default:
>> if (rtc->ops->param_set)
>> - err = rtc->ops->param_set(rtc->dev.parent, ¶m);
>> + err = rtc->ops->param_set(&rtc->dev, ¶m);
>> else
>> err = -EINVAL;
>> }
>> diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
>> index b8b298efd9a9c..783a3ec3bb93d 100644
>> --- a/drivers/rtc/interface.c
>> +++ b/drivers/rtc/interface.c
>> @@ -91,7 +91,7 @@ static int __rtc_read_time(struct rtc_device *rtc, struct rtc_time *tm)
>> err = -EINVAL;
>> } else {
>> memset(tm, 0, sizeof(struct rtc_time));
>> - err = rtc->ops->read_time(rtc->dev.parent, tm);
>> + err = rtc->ops->read_time(&rtc->dev, tm);
>> if (err < 0) {
>> dev_dbg(&rtc->dev, "read_time: fail to read: %d\n",
>> err);
>> @@ -155,7 +155,7 @@ int rtc_set_time(struct rtc_device *rtc, struct rtc_time *tm)
>> if (!rtc->ops)
>> err = -ENODEV;
>> else if (rtc->ops->set_time)
>> - err = rtc->ops->set_time(rtc->dev.parent, tm);
>> + err = rtc->ops->set_time(&rtc->dev, tm);
>> else
>> err = -EINVAL;
>>
>> @@ -200,7 +200,7 @@ static int rtc_read_alarm_internal(struct rtc_device *rtc,
>> alarm->time.tm_wday = -1;
>> alarm->time.tm_yday = -1;
>> alarm->time.tm_isdst = -1;
>> - err = rtc->ops->read_alarm(rtc->dev.parent, alarm);
>> + err = rtc->ops->read_alarm(&rtc->dev, alarm);
>> }
>>
>> mutex_unlock(&rtc->ops_lock);
>> @@ -441,7 +441,7 @@ static int __rtc_set_alarm(struct rtc_device *rtc, struct rtc_wkalrm *alarm)
>> else if (!test_bit(RTC_FEATURE_ALARM, rtc->features))
>> err = -EINVAL;
>> else
>> - err = rtc->ops->set_alarm(rtc->dev.parent, alarm);
>> + err = rtc->ops->set_alarm(&rtc->dev, alarm);
>>
>> /*
>> * Check for potential race described above. If the waiting for next
>> @@ -568,7 +568,7 @@ int rtc_alarm_irq_enable(struct rtc_device *rtc, unsigned int enabled)
>> else if (!test_bit(RTC_FEATURE_ALARM, rtc->features) || !rtc->ops->alarm_irq_enable)
>> err = -EINVAL;
>> else
>> - err = rtc->ops->alarm_irq_enable(rtc->dev.parent, enabled);
>> + err = rtc->ops->alarm_irq_enable(&rtc->dev, enabled);
>>
>> mutex_unlock(&rtc->ops_lock);
>>
>> @@ -618,7 +618,7 @@ int rtc_update_irq_enable(struct rtc_device *rtc, unsigned int enabled)
>> rtc->uie_rtctimer.period = ktime_set(1, 0);
>> err = rtc_timer_enqueue(rtc, &rtc->uie_rtctimer);
>> if (!err && rtc->ops && rtc->ops->alarm_irq_enable)
>> - err = rtc->ops->alarm_irq_enable(rtc->dev.parent, 1);
>> + err = rtc->ops->alarm_irq_enable(&rtc->dev, 1);
>> if (err)
>> goto out;
>> } else {
>> @@ -874,7 +874,7 @@ static void rtc_alarm_disable(struct rtc_device *rtc)
>> if (!rtc->ops || !test_bit(RTC_FEATURE_ALARM, rtc->features) || !rtc->ops->alarm_irq_enable)
>> return;
>>
>> - rtc->ops->alarm_irq_enable(rtc->dev.parent, false);
>> + rtc->ops->alarm_irq_enable(&rtc->dev, false);
>> trace_rtc_alarm_irq_enable(0, 0);
>> }
>>
>> @@ -1076,7 +1076,7 @@ int rtc_read_offset(struct rtc_device *rtc, long *offset)
>> return -EINVAL;
>>
>> mutex_lock(&rtc->ops_lock);
>> - ret = rtc->ops->read_offset(rtc->dev.parent, offset);
>> + ret = rtc->ops->read_offset(&rtc->dev, offset);
>> mutex_unlock(&rtc->ops_lock);
>>
>> trace_rtc_read_offset(*offset, ret);
>> @@ -1111,7 +1111,7 @@ int rtc_set_offset(struct rtc_device *rtc, long offset)
>> return -EINVAL;
>>
>> mutex_lock(&rtc->ops_lock);
>> - ret = rtc->ops->set_offset(rtc->dev.parent, offset);
>> + ret = rtc->ops->set_offset(&rtc->dev, offset);
>> mutex_unlock(&rtc->ops_lock);
>>
>> trace_rtc_set_offset(offset, ret);
>> diff --git a/drivers/rtc/rtc-pl031.c b/drivers/rtc/rtc-pl031.c
>> index eab39dfa4e5fe..a605034d44cb7 100644
>> --- a/drivers/rtc/rtc-pl031.c
>> +++ b/drivers/rtc/rtc-pl031.c
>> @@ -284,10 +284,6 @@ static int pl031_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
>>
>> static void pl031_remove(struct amba_device *adev)
>> {
>> - struct pl031_local *ldata = dev_get_drvdata(&adev->dev);
>> -
>> - if (adev->irq[0])
>> - free_irq(adev->irq[0], ldata);
>> amba_release_regions(adev);
>> }
>>
>> @@ -320,8 +316,6 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
>> goto out;
>> }
>>
>> - amba_set_drvdata(adev, ldata);
>> -
>> dev_dbg(&adev->dev, "designer ID = 0x%02x\n", amba_manf(adev));
>> dev_dbg(&adev->dev, "revision = 0x%01x\n", amba_rev(adev));
>>
>> @@ -356,6 +350,7 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
>> ret = PTR_ERR(ldata->rtc);
>> goto out;
>> }
>> + dev_set_drvdata(&ldata->rtc->dev, ldata);
>>
>> if (!adev->irq[0])
>> clear_bit(RTC_FEATURE_ALARM, ldata->rtc->features);
>> @@ -369,7 +364,7 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
>> goto out;
>>
>> if (adev->irq[0]) {
>> - ret = request_irq(adev->irq[0], pl031_interrupt,
>> + ret = devm_request_irq(&adev->dev, adev->irq[0], pl031_interrupt,
>> vendor->irqflags, "rtc-pl031", ldata);
> Are you _SURE_ you can use devm for this? it is a functional change,
Since ldata's lifecycle is now tied to the RTC device (stored via
dev_set_drvdata(&ldata->rtc->dev, ldata)), and the RTC device's lifecycle
is tied to the amba_device (via devm_rtc_allocate_device(&adev->dev)),
using devm_request_irq(&adev->dev, ...) allows us to remove the manual IRQ
release in pl031_remove, as the IRQ will be automatically released along
with the amba_device lifecycle.
Is this reasoning correct, or should I handle this differently?
thanks,
Ke Sun
> one that trips lots of people up. I wouldn't make this change without
> at least saying why you are doing so in the changelog text, which I
> didn't see at all here.
>
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH v2 3/5] rust: add device wakeup capability support
2026-01-07 14:57 ` Greg KH
@ 2026-01-07 23:35 ` Ke Sun
0 siblings, 0 replies; 32+ messages in thread
From: Ke Sun @ 2026-01-07 23:35 UTC (permalink / raw)
To: Greg KH
Cc: Alexandre Belloni, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, linux-rtc, rust-for-linux,
Alvin Sun
On 1/7/26 22:57, Greg KH wrote:
> On Wed, Jan 07, 2026 at 10:37:35PM +0800, Ke Sun wrote:
>> Add Rust bindings and wrappers for device wakeup functionality,
>> including devm_device_init_wakeup() and dev_pm_set_wake_irq().
>>
>> Signed-off-by: Ke Sun <sunke@kylinos.cn>
>> ---
>> rust/bindings/bindings_helper.h | 2 ++
>> rust/helpers/device.c | 7 +++++++
>> rust/kernel/device.rs | 17 ++++++++++++++++-
>> rust/kernel/irq/request.rs | 7 +++++++
>> 4 files changed, 32 insertions(+), 1 deletion(-)
>>
>> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
>> index fa697287cf71b..d6c2b06ac4107 100644
>> --- a/rust/bindings/bindings_helper.h
>> +++ b/rust/bindings/bindings_helper.h
>> @@ -88,6 +88,8 @@
>> #include <linux/workqueue.h>
>> #include <linux/xarray.h>
>> #include <trace/events/rust_sample.h>
>> +#include <linux/pm_wakeup.h>
>> +#include <linux/pm_wakeirq.h>
> Aren't these sorted?
>
>> /*
>> * The driver-core Rust code needs to know about some C driver-core private
>> diff --git a/rust/helpers/device.c b/rust/helpers/device.c
>> index 9a4316bafedfb..cae26edd83696 100644
>> --- a/rust/helpers/device.c
>> +++ b/rust/helpers/device.c
>> @@ -1,6 +1,8 @@
>> // SPDX-License-Identifier: GPL-2.0
>>
>> #include <linux/device.h>
>> +#include <linux/pm_wakeup.h>
>> +#include <linux/pm_wakeirq.h>
> Why are both of these needed for just one function call?
Hi Greg,
You're absolutely right. `#include <linux/pm_wakeirq.h>` is not needed in
device.c - that was my mistake. I'll also sort the includes in
bindings_helper.h
alphabetically.
I will address both issues in the next version.
Thanks,
Ke Sun
>
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH v2 1/5] rtc: migrate driver data to RTC device
2026-01-07 23:18 ` Ke Sun
@ 2026-01-08 0:24 ` Ke Sun
2026-01-08 11:06 ` Danilo Krummrich
2026-01-08 5:46 ` Greg KH
1 sibling, 1 reply; 32+ messages in thread
From: Ke Sun @ 2026-01-08 0:24 UTC (permalink / raw)
To: Ke Sun, Greg KH
Cc: Alexandre Belloni, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, linux-rtc, rust-for-linux
On 1/8/26 07:18, Ke Sun wrote:
>
> On 1/8/26 00:12, Greg KH wrote:
>> On Wed, Jan 07, 2026 at 10:37:33PM +0800, Ke Sun wrote:
>>> Unify RTC driver interface by storing driver data on the RTC device
>>> instead of the parent device. Update RTC ops callbacks to pass the RTC
>>> device itself rather than its parent. This change enables better
>>> support for Rust RTC drivers that store data on the RTC device.
>>>
>>> Signed-off-by: Ke Sun <sunke@kylinos.cn>
>>> ---
>>> drivers/rtc/dev.c | 4 ++--
>>> drivers/rtc/interface.c | 18 +++++++++---------
>>> drivers/rtc/rtc-pl031.c | 9 ++-------
>>> 3 files changed, 13 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/drivers/rtc/dev.c b/drivers/rtc/dev.c
>>> index baf1a8ca8b2b1..0f62ba9342e3e 100644
>>> --- a/drivers/rtc/dev.c
>>> +++ b/drivers/rtc/dev.c
>>> @@ -410,7 +410,7 @@ static long rtc_dev_ioctl(struct file *file,
>>> }
>>> default:
>>> if (rtc->ops->param_get)
>>> - err = rtc->ops->param_get(rtc->dev.parent, ¶m);
>>> + err = rtc->ops->param_get(&rtc->dev, ¶m);
>>> else
>>> err = -EINVAL;
>>> }
>>> @@ -440,7 +440,7 @@ static long rtc_dev_ioctl(struct file *file,
>>> default:
>>> if (rtc->ops->param_set)
>>> - err = rtc->ops->param_set(rtc->dev.parent, ¶m);
>>> + err = rtc->ops->param_set(&rtc->dev, ¶m);
>>> else
>>> err = -EINVAL;
>>> }
>>> diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
>>> index b8b298efd9a9c..783a3ec3bb93d 100644
>>> --- a/drivers/rtc/interface.c
>>> +++ b/drivers/rtc/interface.c
>>> @@ -91,7 +91,7 @@ static int __rtc_read_time(struct rtc_device *rtc,
>>> struct rtc_time *tm)
>>> err = -EINVAL;
>>> } else {
>>> memset(tm, 0, sizeof(struct rtc_time));
>>> - err = rtc->ops->read_time(rtc->dev.parent, tm);
>>> + err = rtc->ops->read_time(&rtc->dev, tm);
>>> if (err < 0) {
>>> dev_dbg(&rtc->dev, "read_time: fail to read: %d\n",
>>> err);
>>> @@ -155,7 +155,7 @@ int rtc_set_time(struct rtc_device *rtc, struct
>>> rtc_time *tm)
>>> if (!rtc->ops)
>>> err = -ENODEV;
>>> else if (rtc->ops->set_time)
>>> - err = rtc->ops->set_time(rtc->dev.parent, tm);
>>> + err = rtc->ops->set_time(&rtc->dev, tm);
>>> else
>>> err = -EINVAL;
>>> @@ -200,7 +200,7 @@ static int rtc_read_alarm_internal(struct
>>> rtc_device *rtc,
>>> alarm->time.tm_wday = -1;
>>> alarm->time.tm_yday = -1;
>>> alarm->time.tm_isdst = -1;
>>> - err = rtc->ops->read_alarm(rtc->dev.parent, alarm);
>>> + err = rtc->ops->read_alarm(&rtc->dev, alarm);
>>> }
>>> mutex_unlock(&rtc->ops_lock);
>>> @@ -441,7 +441,7 @@ static int __rtc_set_alarm(struct rtc_device
>>> *rtc, struct rtc_wkalrm *alarm)
>>> else if (!test_bit(RTC_FEATURE_ALARM, rtc->features))
>>> err = -EINVAL;
>>> else
>>> - err = rtc->ops->set_alarm(rtc->dev.parent, alarm);
>>> + err = rtc->ops->set_alarm(&rtc->dev, alarm);
>>> /*
>>> * Check for potential race described above. If the waiting
>>> for next
>>> @@ -568,7 +568,7 @@ int rtc_alarm_irq_enable(struct rtc_device *rtc,
>>> unsigned int enabled)
>>> else if (!test_bit(RTC_FEATURE_ALARM, rtc->features) ||
>>> !rtc->ops->alarm_irq_enable)
>>> err = -EINVAL;
>>> else
>>> - err = rtc->ops->alarm_irq_enable(rtc->dev.parent, enabled);
>>> + err = rtc->ops->alarm_irq_enable(&rtc->dev, enabled);
>>> mutex_unlock(&rtc->ops_lock);
>>> @@ -618,7 +618,7 @@ int rtc_update_irq_enable(struct rtc_device
>>> *rtc, unsigned int enabled)
>>> rtc->uie_rtctimer.period = ktime_set(1, 0);
>>> err = rtc_timer_enqueue(rtc, &rtc->uie_rtctimer);
>>> if (!err && rtc->ops && rtc->ops->alarm_irq_enable)
>>> - err = rtc->ops->alarm_irq_enable(rtc->dev.parent, 1);
>>> + err = rtc->ops->alarm_irq_enable(&rtc->dev, 1);
>>> if (err)
>>> goto out;
>>> } else {
>>> @@ -874,7 +874,7 @@ static void rtc_alarm_disable(struct rtc_device
>>> *rtc)
>>> if (!rtc->ops || !test_bit(RTC_FEATURE_ALARM, rtc->features)
>>> || !rtc->ops->alarm_irq_enable)
>>> return;
>>> - rtc->ops->alarm_irq_enable(rtc->dev.parent, false);
>>> + rtc->ops->alarm_irq_enable(&rtc->dev, false);
>>> trace_rtc_alarm_irq_enable(0, 0);
>>> }
>>> @@ -1076,7 +1076,7 @@ int rtc_read_offset(struct rtc_device *rtc,
>>> long *offset)
>>> return -EINVAL;
>>> mutex_lock(&rtc->ops_lock);
>>> - ret = rtc->ops->read_offset(rtc->dev.parent, offset);
>>> + ret = rtc->ops->read_offset(&rtc->dev, offset);
>>> mutex_unlock(&rtc->ops_lock);
>>> trace_rtc_read_offset(*offset, ret);
>>> @@ -1111,7 +1111,7 @@ int rtc_set_offset(struct rtc_device *rtc,
>>> long offset)
>>> return -EINVAL;
>>> mutex_lock(&rtc->ops_lock);
>>> - ret = rtc->ops->set_offset(rtc->dev.parent, offset);
>>> + ret = rtc->ops->set_offset(&rtc->dev, offset);
>>> mutex_unlock(&rtc->ops_lock);
>>> trace_rtc_set_offset(offset, ret);
>>> diff --git a/drivers/rtc/rtc-pl031.c b/drivers/rtc/rtc-pl031.c
>>> index eab39dfa4e5fe..a605034d44cb7 100644
>>> --- a/drivers/rtc/rtc-pl031.c
>>> +++ b/drivers/rtc/rtc-pl031.c
>>> @@ -284,10 +284,6 @@ static int pl031_set_alarm(struct device *dev,
>>> struct rtc_wkalrm *alarm)
>>> static void pl031_remove(struct amba_device *adev)
>>> {
>>> - struct pl031_local *ldata = dev_get_drvdata(&adev->dev);
>>> -
>>> - if (adev->irq[0])
>>> - free_irq(adev->irq[0], ldata);
>>> amba_release_regions(adev);
>>> }
>>> @@ -320,8 +316,6 @@ static int pl031_probe(struct amba_device
>>> *adev, const struct amba_id *id)
>>> goto out;
>>> }
>>> - amba_set_drvdata(adev, ldata);
>>> -
>>> dev_dbg(&adev->dev, "designer ID = 0x%02x\n", amba_manf(adev));
>>> dev_dbg(&adev->dev, "revision = 0x%01x\n", amba_rev(adev));
>>> @@ -356,6 +350,7 @@ static int pl031_probe(struct amba_device
>>> *adev, const struct amba_id *id)
>>> ret = PTR_ERR(ldata->rtc);
>>> goto out;
>>> }
>>> + dev_set_drvdata(&ldata->rtc->dev, ldata);
>>> if (!adev->irq[0])
>>> clear_bit(RTC_FEATURE_ALARM, ldata->rtc->features);
>>> @@ -369,7 +364,7 @@ static int pl031_probe(struct amba_device *adev,
>>> const struct amba_id *id)
>>> goto out;
>>> if (adev->irq[0]) {
>>> - ret = request_irq(adev->irq[0], pl031_interrupt,
>>> + ret = devm_request_irq(&adev->dev, adev->irq[0],
>>> pl031_interrupt,
>>> vendor->irqflags, "rtc-pl031", ldata);
>> Are you _SURE_ you can use devm for this? it is a functional change,
>
> Since ldata's lifecycle is now tied to the RTC device (stored via
> dev_set_drvdata(&ldata->rtc->dev, ldata)), and the RTC device's lifecycle
> is tied to the amba_device (via devm_rtc_allocate_device(&adev->dev)),
> using devm_request_irq(&adev->dev, ...) allows us to remove the manual
> IRQ
> release in pl031_remove, as the IRQ will be automatically released along
> with the amba_device lifecycle.
>
>
> Is this reasoning correct, or should I handle this differently?
Correction: Should use devm_request_irq(&ldata->rtc->dev, ...) instead
of devm_request_irq(&adev->dev, ...).
While a lifecycle chain exists (ldata -> rtc -> amba_device), the IRQ
should
be bound to the device that uses it (the RTC device), matching where driver
data is stored, to avoid UAF.
>
>
> thanks,
>
> Ke Sun
>
>> one that trips lots of people up. I wouldn't make this change without
>> at least saying why you are doing so in the changelog text, which I
>> didn't see at all here.
>>
>> thanks,
>>
>> greg k-h
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH v2 1/5] rtc: migrate driver data to RTC device
2026-01-07 23:18 ` Ke Sun
2026-01-08 0:24 ` Ke Sun
@ 2026-01-08 5:46 ` Greg KH
2026-01-08 9:02 ` Ke Sun
1 sibling, 1 reply; 32+ messages in thread
From: Greg KH @ 2026-01-08 5:46 UTC (permalink / raw)
To: Ke Sun
Cc: Alexandre Belloni, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, linux-rtc, rust-for-linux,
Alvin Sun
On Thu, Jan 08, 2026 at 07:18:30AM +0800, Ke Sun wrote:
>
> On 1/8/26 00:12, Greg KH wrote:
> > On Wed, Jan 07, 2026 at 10:37:33PM +0800, Ke Sun wrote:
> > > Unify RTC driver interface by storing driver data on the RTC device
> > > instead of the parent device. Update RTC ops callbacks to pass the RTC
> > > device itself rather than its parent. This change enables better
> > > support for Rust RTC drivers that store data on the RTC device.
> > >
> > > Signed-off-by: Ke Sun <sunke@kylinos.cn>
> > > ---
> > > drivers/rtc/dev.c | 4 ++--
> > > drivers/rtc/interface.c | 18 +++++++++---------
> > > drivers/rtc/rtc-pl031.c | 9 ++-------
> > > 3 files changed, 13 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/drivers/rtc/dev.c b/drivers/rtc/dev.c
> > > index baf1a8ca8b2b1..0f62ba9342e3e 100644
> > > --- a/drivers/rtc/dev.c
> > > +++ b/drivers/rtc/dev.c
> > > @@ -410,7 +410,7 @@ static long rtc_dev_ioctl(struct file *file,
> > > }
> > > default:
> > > if (rtc->ops->param_get)
> > > - err = rtc->ops->param_get(rtc->dev.parent, ¶m);
> > > + err = rtc->ops->param_get(&rtc->dev, ¶m);
> > > else
> > > err = -EINVAL;
> > > }
> > > @@ -440,7 +440,7 @@ static long rtc_dev_ioctl(struct file *file,
> > > default:
> > > if (rtc->ops->param_set)
> > > - err = rtc->ops->param_set(rtc->dev.parent, ¶m);
> > > + err = rtc->ops->param_set(&rtc->dev, ¶m);
> > > else
> > > err = -EINVAL;
> > > }
> > > diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
> > > index b8b298efd9a9c..783a3ec3bb93d 100644
> > > --- a/drivers/rtc/interface.c
> > > +++ b/drivers/rtc/interface.c
> > > @@ -91,7 +91,7 @@ static int __rtc_read_time(struct rtc_device *rtc, struct rtc_time *tm)
> > > err = -EINVAL;
> > > } else {
> > > memset(tm, 0, sizeof(struct rtc_time));
> > > - err = rtc->ops->read_time(rtc->dev.parent, tm);
> > > + err = rtc->ops->read_time(&rtc->dev, tm);
> > > if (err < 0) {
> > > dev_dbg(&rtc->dev, "read_time: fail to read: %d\n",
> > > err);
> > > @@ -155,7 +155,7 @@ int rtc_set_time(struct rtc_device *rtc, struct rtc_time *tm)
> > > if (!rtc->ops)
> > > err = -ENODEV;
> > > else if (rtc->ops->set_time)
> > > - err = rtc->ops->set_time(rtc->dev.parent, tm);
> > > + err = rtc->ops->set_time(&rtc->dev, tm);
> > > else
> > > err = -EINVAL;
> > > @@ -200,7 +200,7 @@ static int rtc_read_alarm_internal(struct rtc_device *rtc,
> > > alarm->time.tm_wday = -1;
> > > alarm->time.tm_yday = -1;
> > > alarm->time.tm_isdst = -1;
> > > - err = rtc->ops->read_alarm(rtc->dev.parent, alarm);
> > > + err = rtc->ops->read_alarm(&rtc->dev, alarm);
> > > }
> > > mutex_unlock(&rtc->ops_lock);
> > > @@ -441,7 +441,7 @@ static int __rtc_set_alarm(struct rtc_device *rtc, struct rtc_wkalrm *alarm)
> > > else if (!test_bit(RTC_FEATURE_ALARM, rtc->features))
> > > err = -EINVAL;
> > > else
> > > - err = rtc->ops->set_alarm(rtc->dev.parent, alarm);
> > > + err = rtc->ops->set_alarm(&rtc->dev, alarm);
> > > /*
> > > * Check for potential race described above. If the waiting for next
> > > @@ -568,7 +568,7 @@ int rtc_alarm_irq_enable(struct rtc_device *rtc, unsigned int enabled)
> > > else if (!test_bit(RTC_FEATURE_ALARM, rtc->features) || !rtc->ops->alarm_irq_enable)
> > > err = -EINVAL;
> > > else
> > > - err = rtc->ops->alarm_irq_enable(rtc->dev.parent, enabled);
> > > + err = rtc->ops->alarm_irq_enable(&rtc->dev, enabled);
> > > mutex_unlock(&rtc->ops_lock);
> > > @@ -618,7 +618,7 @@ int rtc_update_irq_enable(struct rtc_device *rtc, unsigned int enabled)
> > > rtc->uie_rtctimer.period = ktime_set(1, 0);
> > > err = rtc_timer_enqueue(rtc, &rtc->uie_rtctimer);
> > > if (!err && rtc->ops && rtc->ops->alarm_irq_enable)
> > > - err = rtc->ops->alarm_irq_enable(rtc->dev.parent, 1);
> > > + err = rtc->ops->alarm_irq_enable(&rtc->dev, 1);
> > > if (err)
> > > goto out;
> > > } else {
> > > @@ -874,7 +874,7 @@ static void rtc_alarm_disable(struct rtc_device *rtc)
> > > if (!rtc->ops || !test_bit(RTC_FEATURE_ALARM, rtc->features) || !rtc->ops->alarm_irq_enable)
> > > return;
> > > - rtc->ops->alarm_irq_enable(rtc->dev.parent, false);
> > > + rtc->ops->alarm_irq_enable(&rtc->dev, false);
> > > trace_rtc_alarm_irq_enable(0, 0);
> > > }
> > > @@ -1076,7 +1076,7 @@ int rtc_read_offset(struct rtc_device *rtc, long *offset)
> > > return -EINVAL;
> > > mutex_lock(&rtc->ops_lock);
> > > - ret = rtc->ops->read_offset(rtc->dev.parent, offset);
> > > + ret = rtc->ops->read_offset(&rtc->dev, offset);
> > > mutex_unlock(&rtc->ops_lock);
> > > trace_rtc_read_offset(*offset, ret);
> > > @@ -1111,7 +1111,7 @@ int rtc_set_offset(struct rtc_device *rtc, long offset)
> > > return -EINVAL;
> > > mutex_lock(&rtc->ops_lock);
> > > - ret = rtc->ops->set_offset(rtc->dev.parent, offset);
> > > + ret = rtc->ops->set_offset(&rtc->dev, offset);
> > > mutex_unlock(&rtc->ops_lock);
> > > trace_rtc_set_offset(offset, ret);
> > > diff --git a/drivers/rtc/rtc-pl031.c b/drivers/rtc/rtc-pl031.c
> > > index eab39dfa4e5fe..a605034d44cb7 100644
> > > --- a/drivers/rtc/rtc-pl031.c
> > > +++ b/drivers/rtc/rtc-pl031.c
> > > @@ -284,10 +284,6 @@ static int pl031_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
> > > static void pl031_remove(struct amba_device *adev)
> > > {
> > > - struct pl031_local *ldata = dev_get_drvdata(&adev->dev);
> > > -
> > > - if (adev->irq[0])
> > > - free_irq(adev->irq[0], ldata);
> > > amba_release_regions(adev);
> > > }
> > > @@ -320,8 +316,6 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
> > > goto out;
> > > }
> > > - amba_set_drvdata(adev, ldata);
> > > -
> > > dev_dbg(&adev->dev, "designer ID = 0x%02x\n", amba_manf(adev));
> > > dev_dbg(&adev->dev, "revision = 0x%01x\n", amba_rev(adev));
> > > @@ -356,6 +350,7 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
> > > ret = PTR_ERR(ldata->rtc);
> > > goto out;
> > > }
> > > + dev_set_drvdata(&ldata->rtc->dev, ldata);
> > > if (!adev->irq[0])
> > > clear_bit(RTC_FEATURE_ALARM, ldata->rtc->features);
> > > @@ -369,7 +364,7 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
> > > goto out;
> > > if (adev->irq[0]) {
> > > - ret = request_irq(adev->irq[0], pl031_interrupt,
> > > + ret = devm_request_irq(&adev->dev, adev->irq[0], pl031_interrupt,
> > > vendor->irqflags, "rtc-pl031", ldata);
> > Are you _SURE_ you can use devm for this? it is a functional change,
>
> Since ldata's lifecycle is now tied to the RTC device (stored via
> dev_set_drvdata(&ldata->rtc->dev, ldata)), and the RTC device's lifecycle
> is tied to the amba_device (via devm_rtc_allocate_device(&adev->dev)),
> using devm_request_irq(&adev->dev, ...) allows us to remove the manual IRQ
> release in pl031_remove, as the IRQ will be automatically released along
> with the amba_device lifecycle.
Please test this. There are loads of race conditions that can happen
when irqs are bound to devm lifecycles. You are changing the behavior
here, so be very careful.
And again, this is a change that was not documented in the changelog,
and should not be part of this patch, it should be stand-alone.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH v2 1/5] rtc: migrate driver data to RTC device
2026-01-08 5:46 ` Greg KH
@ 2026-01-08 9:02 ` Ke Sun
2026-01-08 9:10 ` Greg KH
0 siblings, 1 reply; 32+ messages in thread
From: Ke Sun @ 2026-01-08 9:02 UTC (permalink / raw)
To: Greg KH
Cc: Alexandre Belloni, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, linux-rtc, rust-for-linux,
Alvin Sun
On 1/8/26 13:46, Greg KH wrote:
> On Thu, Jan 08, 2026 at 07:18:30AM +0800, Ke Sun wrote:
>> On 1/8/26 00:12, Greg KH wrote:
>>> On Wed, Jan 07, 2026 at 10:37:33PM +0800, Ke Sun wrote:
>>>> Unify RTC driver interface by storing driver data on the RTC device
>>>> instead of the parent device. Update RTC ops callbacks to pass the RTC
>>>> device itself rather than its parent. This change enables better
>>>> support for Rust RTC drivers that store data on the RTC device.
>>>>
>>>> Signed-off-by: Ke Sun <sunke@kylinos.cn>
>>>> ---
>>>> drivers/rtc/dev.c | 4 ++--
>>>> drivers/rtc/interface.c | 18 +++++++++---------
>>>> drivers/rtc/rtc-pl031.c | 9 ++-------
>>>> 3 files changed, 13 insertions(+), 18 deletions(-)
>>>>
>>>> diff --git a/drivers/rtc/dev.c b/drivers/rtc/dev.c
>>>> index baf1a8ca8b2b1..0f62ba9342e3e 100644
>>>> --- a/drivers/rtc/dev.c
>>>> +++ b/drivers/rtc/dev.c
>>>> @@ -410,7 +410,7 @@ static long rtc_dev_ioctl(struct file *file,
>>>> }
>>>> default:
>>>> if (rtc->ops->param_get)
>>>> - err = rtc->ops->param_get(rtc->dev.parent, ¶m);
>>>> + err = rtc->ops->param_get(&rtc->dev, ¶m);
>>>> else
>>>> err = -EINVAL;
>>>> }
>>>> @@ -440,7 +440,7 @@ static long rtc_dev_ioctl(struct file *file,
>>>> default:
>>>> if (rtc->ops->param_set)
>>>> - err = rtc->ops->param_set(rtc->dev.parent, ¶m);
>>>> + err = rtc->ops->param_set(&rtc->dev, ¶m);
>>>> else
>>>> err = -EINVAL;
>>>> }
>>>> diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
>>>> index b8b298efd9a9c..783a3ec3bb93d 100644
>>>> --- a/drivers/rtc/interface.c
>>>> +++ b/drivers/rtc/interface.c
>>>> @@ -91,7 +91,7 @@ static int __rtc_read_time(struct rtc_device *rtc, struct rtc_time *tm)
>>>> err = -EINVAL;
>>>> } else {
>>>> memset(tm, 0, sizeof(struct rtc_time));
>>>> - err = rtc->ops->read_time(rtc->dev.parent, tm);
>>>> + err = rtc->ops->read_time(&rtc->dev, tm);
>>>> if (err < 0) {
>>>> dev_dbg(&rtc->dev, "read_time: fail to read: %d\n",
>>>> err);
>>>> @@ -155,7 +155,7 @@ int rtc_set_time(struct rtc_device *rtc, struct rtc_time *tm)
>>>> if (!rtc->ops)
>>>> err = -ENODEV;
>>>> else if (rtc->ops->set_time)
>>>> - err = rtc->ops->set_time(rtc->dev.parent, tm);
>>>> + err = rtc->ops->set_time(&rtc->dev, tm);
>>>> else
>>>> err = -EINVAL;
>>>> @@ -200,7 +200,7 @@ static int rtc_read_alarm_internal(struct rtc_device *rtc,
>>>> alarm->time.tm_wday = -1;
>>>> alarm->time.tm_yday = -1;
>>>> alarm->time.tm_isdst = -1;
>>>> - err = rtc->ops->read_alarm(rtc->dev.parent, alarm);
>>>> + err = rtc->ops->read_alarm(&rtc->dev, alarm);
>>>> }
>>>> mutex_unlock(&rtc->ops_lock);
>>>> @@ -441,7 +441,7 @@ static int __rtc_set_alarm(struct rtc_device *rtc, struct rtc_wkalrm *alarm)
>>>> else if (!test_bit(RTC_FEATURE_ALARM, rtc->features))
>>>> err = -EINVAL;
>>>> else
>>>> - err = rtc->ops->set_alarm(rtc->dev.parent, alarm);
>>>> + err = rtc->ops->set_alarm(&rtc->dev, alarm);
>>>> /*
>>>> * Check for potential race described above. If the waiting for next
>>>> @@ -568,7 +568,7 @@ int rtc_alarm_irq_enable(struct rtc_device *rtc, unsigned int enabled)
>>>> else if (!test_bit(RTC_FEATURE_ALARM, rtc->features) || !rtc->ops->alarm_irq_enable)
>>>> err = -EINVAL;
>>>> else
>>>> - err = rtc->ops->alarm_irq_enable(rtc->dev.parent, enabled);
>>>> + err = rtc->ops->alarm_irq_enable(&rtc->dev, enabled);
>>>> mutex_unlock(&rtc->ops_lock);
>>>> @@ -618,7 +618,7 @@ int rtc_update_irq_enable(struct rtc_device *rtc, unsigned int enabled)
>>>> rtc->uie_rtctimer.period = ktime_set(1, 0);
>>>> err = rtc_timer_enqueue(rtc, &rtc->uie_rtctimer);
>>>> if (!err && rtc->ops && rtc->ops->alarm_irq_enable)
>>>> - err = rtc->ops->alarm_irq_enable(rtc->dev.parent, 1);
>>>> + err = rtc->ops->alarm_irq_enable(&rtc->dev, 1);
>>>> if (err)
>>>> goto out;
>>>> } else {
>>>> @@ -874,7 +874,7 @@ static void rtc_alarm_disable(struct rtc_device *rtc)
>>>> if (!rtc->ops || !test_bit(RTC_FEATURE_ALARM, rtc->features) || !rtc->ops->alarm_irq_enable)
>>>> return;
>>>> - rtc->ops->alarm_irq_enable(rtc->dev.parent, false);
>>>> + rtc->ops->alarm_irq_enable(&rtc->dev, false);
>>>> trace_rtc_alarm_irq_enable(0, 0);
>>>> }
>>>> @@ -1076,7 +1076,7 @@ int rtc_read_offset(struct rtc_device *rtc, long *offset)
>>>> return -EINVAL;
>>>> mutex_lock(&rtc->ops_lock);
>>>> - ret = rtc->ops->read_offset(rtc->dev.parent, offset);
>>>> + ret = rtc->ops->read_offset(&rtc->dev, offset);
>>>> mutex_unlock(&rtc->ops_lock);
>>>> trace_rtc_read_offset(*offset, ret);
>>>> @@ -1111,7 +1111,7 @@ int rtc_set_offset(struct rtc_device *rtc, long offset)
>>>> return -EINVAL;
>>>> mutex_lock(&rtc->ops_lock);
>>>> - ret = rtc->ops->set_offset(rtc->dev.parent, offset);
>>>> + ret = rtc->ops->set_offset(&rtc->dev, offset);
>>>> mutex_unlock(&rtc->ops_lock);
>>>> trace_rtc_set_offset(offset, ret);
>>>> diff --git a/drivers/rtc/rtc-pl031.c b/drivers/rtc/rtc-pl031.c
>>>> index eab39dfa4e5fe..a605034d44cb7 100644
>>>> --- a/drivers/rtc/rtc-pl031.c
>>>> +++ b/drivers/rtc/rtc-pl031.c
>>>> @@ -284,10 +284,6 @@ static int pl031_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
>>>> static void pl031_remove(struct amba_device *adev)
>>>> {
>>>> - struct pl031_local *ldata = dev_get_drvdata(&adev->dev);
>>>> -
>>>> - if (adev->irq[0])
>>>> - free_irq(adev->irq[0], ldata);
>>>> amba_release_regions(adev);
>>>> }
>>>> @@ -320,8 +316,6 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
>>>> goto out;
>>>> }
>>>> - amba_set_drvdata(adev, ldata);
>>>> -
>>>> dev_dbg(&adev->dev, "designer ID = 0x%02x\n", amba_manf(adev));
>>>> dev_dbg(&adev->dev, "revision = 0x%01x\n", amba_rev(adev));
>>>> @@ -356,6 +350,7 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
>>>> ret = PTR_ERR(ldata->rtc);
>>>> goto out;
>>>> }
>>>> + dev_set_drvdata(&ldata->rtc->dev, ldata);
>>>> if (!adev->irq[0])
>>>> clear_bit(RTC_FEATURE_ALARM, ldata->rtc->features);
>>>> @@ -369,7 +364,7 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
>>>> goto out;
>>>> if (adev->irq[0]) {
>>>> - ret = request_irq(adev->irq[0], pl031_interrupt,
>>>> + ret = devm_request_irq(&adev->dev, adev->irq[0], pl031_interrupt,
>>>> vendor->irqflags, "rtc-pl031", ldata);
>>> Are you _SURE_ you can use devm for this? it is a functional change,
>> Since ldata's lifecycle is now tied to the RTC device (stored via
>> dev_set_drvdata(&ldata->rtc->dev, ldata)), and the RTC device's lifecycle
>> is tied to the amba_device (via devm_rtc_allocate_device(&adev->dev)),
>> using devm_request_irq(&adev->dev, ...) allows us to remove the manual IRQ
>> release in pl031_remove, as the IRQ will be automatically released along
>> with the amba_device lifecycle.
> Please test this. There are loads of race conditions that can happen
> when irqs are bound to devm lifecycles. You are changing the behavior
> here, so be very careful.
Yes, I'm testing this with pl031 (qemu), ds3231, rk808, and other devices.
Using rtcwake and hwclock for concurrent access, while continuously
unbinding/binding devices.
>
> And again, this is a change that was not documented in the changelog,
> and should not be part of this patch, it should be stand-alone.
Regarding the RTC C refactoring that affects 182 files, should I put all
changes in one patch, or create separate patches for each file?
Best regards,
Ke Sun
>
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH v2 1/5] rtc: migrate driver data to RTC device
2026-01-08 9:02 ` Ke Sun
@ 2026-01-08 9:10 ` Greg KH
0 siblings, 0 replies; 32+ messages in thread
From: Greg KH @ 2026-01-08 9:10 UTC (permalink / raw)
To: Ke Sun
Cc: Alexandre Belloni, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, linux-rtc, rust-for-linux,
Alvin Sun
On Thu, Jan 08, 2026 at 05:02:30PM +0800, Ke Sun wrote:
>
> On 1/8/26 13:46, Greg KH wrote:
> > On Thu, Jan 08, 2026 at 07:18:30AM +0800, Ke Sun wrote:
> > > On 1/8/26 00:12, Greg KH wrote:
> > > > On Wed, Jan 07, 2026 at 10:37:33PM +0800, Ke Sun wrote:
> > > > > Unify RTC driver interface by storing driver data on the RTC device
> > > > > instead of the parent device. Update RTC ops callbacks to pass the RTC
> > > > > device itself rather than its parent. This change enables better
> > > > > support for Rust RTC drivers that store data on the RTC device.
> > > > >
> > > > > Signed-off-by: Ke Sun <sunke@kylinos.cn>
> > > > > ---
> > > > > drivers/rtc/dev.c | 4 ++--
> > > > > drivers/rtc/interface.c | 18 +++++++++---------
> > > > > drivers/rtc/rtc-pl031.c | 9 ++-------
> > > > > 3 files changed, 13 insertions(+), 18 deletions(-)
> > > > >
> > > > > diff --git a/drivers/rtc/dev.c b/drivers/rtc/dev.c
> > > > > index baf1a8ca8b2b1..0f62ba9342e3e 100644
> > > > > --- a/drivers/rtc/dev.c
> > > > > +++ b/drivers/rtc/dev.c
> > > > > @@ -410,7 +410,7 @@ static long rtc_dev_ioctl(struct file *file,
> > > > > }
> > > > > default:
> > > > > if (rtc->ops->param_get)
> > > > > - err = rtc->ops->param_get(rtc->dev.parent, ¶m);
> > > > > + err = rtc->ops->param_get(&rtc->dev, ¶m);
> > > > > else
> > > > > err = -EINVAL;
> > > > > }
> > > > > @@ -440,7 +440,7 @@ static long rtc_dev_ioctl(struct file *file,
> > > > > default:
> > > > > if (rtc->ops->param_set)
> > > > > - err = rtc->ops->param_set(rtc->dev.parent, ¶m);
> > > > > + err = rtc->ops->param_set(&rtc->dev, ¶m);
> > > > > else
> > > > > err = -EINVAL;
> > > > > }
> > > > > diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
> > > > > index b8b298efd9a9c..783a3ec3bb93d 100644
> > > > > --- a/drivers/rtc/interface.c
> > > > > +++ b/drivers/rtc/interface.c
> > > > > @@ -91,7 +91,7 @@ static int __rtc_read_time(struct rtc_device *rtc, struct rtc_time *tm)
> > > > > err = -EINVAL;
> > > > > } else {
> > > > > memset(tm, 0, sizeof(struct rtc_time));
> > > > > - err = rtc->ops->read_time(rtc->dev.parent, tm);
> > > > > + err = rtc->ops->read_time(&rtc->dev, tm);
> > > > > if (err < 0) {
> > > > > dev_dbg(&rtc->dev, "read_time: fail to read: %d\n",
> > > > > err);
> > > > > @@ -155,7 +155,7 @@ int rtc_set_time(struct rtc_device *rtc, struct rtc_time *tm)
> > > > > if (!rtc->ops)
> > > > > err = -ENODEV;
> > > > > else if (rtc->ops->set_time)
> > > > > - err = rtc->ops->set_time(rtc->dev.parent, tm);
> > > > > + err = rtc->ops->set_time(&rtc->dev, tm);
> > > > > else
> > > > > err = -EINVAL;
> > > > > @@ -200,7 +200,7 @@ static int rtc_read_alarm_internal(struct rtc_device *rtc,
> > > > > alarm->time.tm_wday = -1;
> > > > > alarm->time.tm_yday = -1;
> > > > > alarm->time.tm_isdst = -1;
> > > > > - err = rtc->ops->read_alarm(rtc->dev.parent, alarm);
> > > > > + err = rtc->ops->read_alarm(&rtc->dev, alarm);
> > > > > }
> > > > > mutex_unlock(&rtc->ops_lock);
> > > > > @@ -441,7 +441,7 @@ static int __rtc_set_alarm(struct rtc_device *rtc, struct rtc_wkalrm *alarm)
> > > > > else if (!test_bit(RTC_FEATURE_ALARM, rtc->features))
> > > > > err = -EINVAL;
> > > > > else
> > > > > - err = rtc->ops->set_alarm(rtc->dev.parent, alarm);
> > > > > + err = rtc->ops->set_alarm(&rtc->dev, alarm);
> > > > > /*
> > > > > * Check for potential race described above. If the waiting for next
> > > > > @@ -568,7 +568,7 @@ int rtc_alarm_irq_enable(struct rtc_device *rtc, unsigned int enabled)
> > > > > else if (!test_bit(RTC_FEATURE_ALARM, rtc->features) || !rtc->ops->alarm_irq_enable)
> > > > > err = -EINVAL;
> > > > > else
> > > > > - err = rtc->ops->alarm_irq_enable(rtc->dev.parent, enabled);
> > > > > + err = rtc->ops->alarm_irq_enable(&rtc->dev, enabled);
> > > > > mutex_unlock(&rtc->ops_lock);
> > > > > @@ -618,7 +618,7 @@ int rtc_update_irq_enable(struct rtc_device *rtc, unsigned int enabled)
> > > > > rtc->uie_rtctimer.period = ktime_set(1, 0);
> > > > > err = rtc_timer_enqueue(rtc, &rtc->uie_rtctimer);
> > > > > if (!err && rtc->ops && rtc->ops->alarm_irq_enable)
> > > > > - err = rtc->ops->alarm_irq_enable(rtc->dev.parent, 1);
> > > > > + err = rtc->ops->alarm_irq_enable(&rtc->dev, 1);
> > > > > if (err)
> > > > > goto out;
> > > > > } else {
> > > > > @@ -874,7 +874,7 @@ static void rtc_alarm_disable(struct rtc_device *rtc)
> > > > > if (!rtc->ops || !test_bit(RTC_FEATURE_ALARM, rtc->features) || !rtc->ops->alarm_irq_enable)
> > > > > return;
> > > > > - rtc->ops->alarm_irq_enable(rtc->dev.parent, false);
> > > > > + rtc->ops->alarm_irq_enable(&rtc->dev, false);
> > > > > trace_rtc_alarm_irq_enable(0, 0);
> > > > > }
> > > > > @@ -1076,7 +1076,7 @@ int rtc_read_offset(struct rtc_device *rtc, long *offset)
> > > > > return -EINVAL;
> > > > > mutex_lock(&rtc->ops_lock);
> > > > > - ret = rtc->ops->read_offset(rtc->dev.parent, offset);
> > > > > + ret = rtc->ops->read_offset(&rtc->dev, offset);
> > > > > mutex_unlock(&rtc->ops_lock);
> > > > > trace_rtc_read_offset(*offset, ret);
> > > > > @@ -1111,7 +1111,7 @@ int rtc_set_offset(struct rtc_device *rtc, long offset)
> > > > > return -EINVAL;
> > > > > mutex_lock(&rtc->ops_lock);
> > > > > - ret = rtc->ops->set_offset(rtc->dev.parent, offset);
> > > > > + ret = rtc->ops->set_offset(&rtc->dev, offset);
> > > > > mutex_unlock(&rtc->ops_lock);
> > > > > trace_rtc_set_offset(offset, ret);
> > > > > diff --git a/drivers/rtc/rtc-pl031.c b/drivers/rtc/rtc-pl031.c
> > > > > index eab39dfa4e5fe..a605034d44cb7 100644
> > > > > --- a/drivers/rtc/rtc-pl031.c
> > > > > +++ b/drivers/rtc/rtc-pl031.c
> > > > > @@ -284,10 +284,6 @@ static int pl031_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
> > > > > static void pl031_remove(struct amba_device *adev)
> > > > > {
> > > > > - struct pl031_local *ldata = dev_get_drvdata(&adev->dev);
> > > > > -
> > > > > - if (adev->irq[0])
> > > > > - free_irq(adev->irq[0], ldata);
> > > > > amba_release_regions(adev);
> > > > > }
> > > > > @@ -320,8 +316,6 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
> > > > > goto out;
> > > > > }
> > > > > - amba_set_drvdata(adev, ldata);
> > > > > -
> > > > > dev_dbg(&adev->dev, "designer ID = 0x%02x\n", amba_manf(adev));
> > > > > dev_dbg(&adev->dev, "revision = 0x%01x\n", amba_rev(adev));
> > > > > @@ -356,6 +350,7 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
> > > > > ret = PTR_ERR(ldata->rtc);
> > > > > goto out;
> > > > > }
> > > > > + dev_set_drvdata(&ldata->rtc->dev, ldata);
> > > > > if (!adev->irq[0])
> > > > > clear_bit(RTC_FEATURE_ALARM, ldata->rtc->features);
> > > > > @@ -369,7 +364,7 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
> > > > > goto out;
> > > > > if (adev->irq[0]) {
> > > > > - ret = request_irq(adev->irq[0], pl031_interrupt,
> > > > > + ret = devm_request_irq(&adev->dev, adev->irq[0], pl031_interrupt,
> > > > > vendor->irqflags, "rtc-pl031", ldata);
> > > > Are you _SURE_ you can use devm for this? it is a functional change,
> > > Since ldata's lifecycle is now tied to the RTC device (stored via
> > > dev_set_drvdata(&ldata->rtc->dev, ldata)), and the RTC device's lifecycle
> > > is tied to the amba_device (via devm_rtc_allocate_device(&adev->dev)),
> > > using devm_request_irq(&adev->dev, ...) allows us to remove the manual IRQ
> > > release in pl031_remove, as the IRQ will be automatically released along
> > > with the amba_device lifecycle.
> > Please test this. There are loads of race conditions that can happen
> > when irqs are bound to devm lifecycles. You are changing the behavior
> > here, so be very careful.
>
> Yes, I'm testing this with pl031 (qemu), ds3231, rk808, and other devices.
>
> Using rtcwake and hwclock for concurrent access, while continuously
>
> unbinding/binding devices.
Ok, great, but again, make this a separate patch.
> > And again, this is a change that was not documented in the changelog,
> > and should not be part of this patch, it should be stand-alone.
>
> Regarding the RTC C refactoring that affects 182 files, should I put all
>
> changes in one patch, or create separate patches for each file?
Don't know, whatever you find easier. What would you want to review if
you were on the recieving end of it?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH v2 1/5] rtc: migrate driver data to RTC device
2026-01-08 0:24 ` Ke Sun
@ 2026-01-08 11:06 ` Danilo Krummrich
0 siblings, 0 replies; 32+ messages in thread
From: Danilo Krummrich @ 2026-01-08 11:06 UTC (permalink / raw)
To: Ke Sun
Cc: Ke Sun, Greg KH, Alexandre Belloni, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, linux-rtc, rust-for-linux
On Thu Jan 8, 2026 at 1:24 AM CET, Ke Sun wrote:
> Correction: Should use devm_request_irq(&ldata->rtc->dev, ...) instead
>
> of devm_request_irq(&adev->dev, ...).
>
>
> While a lifecycle chain exists (ldata -> rtc -> amba_device), the IRQ
> should
>
> be bound to the device that uses it (the RTC device), matching where driver
>
> data is stored, to avoid UAF.
No, that is wrong.
Your RTC device is a class device, not a bus device. I.e. it is not a
representation of physical device on the bus.
However, the requested IRQ is a physical device resource that belongs to a bus
device, which in your case is an AMBA device. (Please also see my reply [1] from
the previous version again.)
Therefore device resources are only valid to access and hold on to for a driver
as long as it is bound to the corresponding bus device.
This is the fundamental reason why it is desirable to guard the lifetime of the
class device and its callbacks with devres, to ensure that it is always valid to
access device resources from class device callbacks.
In C drivers, you just have to do the right thing, otherwise you indeed end up
with UAFs. In Rust, we can leverage the type system to make it impossible for
drivers to mess this up.
[1] https://lore.kernel.org/all/DFHJM2HAG7Q3.1HGZ3P7H55FD2@kernel.org/
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH v2 1/5] rtc: migrate driver data to RTC device
2026-01-07 14:37 ` [RFC PATCH v2 1/5] rtc: migrate driver data to RTC device Ke Sun
2026-01-07 14:41 ` Ke Sun
2026-01-07 16:12 ` Greg KH
@ 2026-01-08 11:12 ` Danilo Krummrich
2026-01-08 13:45 ` Ke Sun
2 siblings, 1 reply; 32+ messages in thread
From: Danilo Krummrich @ 2026-01-08 11:12 UTC (permalink / raw)
To: Ke Sun
Cc: Alexandre Belloni, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, linux-rtc, rust-for-linux, Alvin Sun
On Wed Jan 7, 2026 at 3:37 PM CET, Ke Sun wrote:
> diff --git a/drivers/rtc/dev.c b/drivers/rtc/dev.c
> index baf1a8ca8b2b1..0f62ba9342e3e 100644
> --- a/drivers/rtc/dev.c
> +++ b/drivers/rtc/dev.c
> @@ -410,7 +410,7 @@ static long rtc_dev_ioctl(struct file *file,
> }
> default:
> if (rtc->ops->param_get)
> - err = rtc->ops->param_get(rtc->dev.parent, ¶m);
> + err = rtc->ops->param_get(&rtc->dev, ¶m);
It would make more sense to just pass a struct rtc_device than the embedded
struct device in the RTC callbacks.
> @@ -369,7 +364,7 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
> goto out;
>
> if (adev->irq[0]) {
> - ret = request_irq(adev->irq[0], pl031_interrupt,
> + ret = devm_request_irq(&adev->dev, adev->irq[0], pl031_interrupt,
> vendor->irqflags, "rtc-pl031", ldata);
As Greg already mentioned that change should be a separate patch.
You also have to be careful with the devres order when using devm_request_irq().
In your case, you pass ldata, so you have to ensure that ldata (and its
contents) remain valid until the devres callback frees the IRQ request.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH v2 2/5] rust: add AMBA bus driver support
2026-01-07 14:37 ` [RFC PATCH v2 2/5] rust: add AMBA bus driver support Ke Sun
@ 2026-01-08 11:29 ` Danilo Krummrich
0 siblings, 0 replies; 32+ messages in thread
From: Danilo Krummrich @ 2026-01-08 11:29 UTC (permalink / raw)
To: Ke Sun
Cc: Alexandre Belloni, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, linux-rtc, rust-for-linux, Alvin Sun
On Wed Jan 7, 2026 at 3:37 PM CET, Ke Sun wrote:
> +impl DeviceId {
> + /// Creates a new device ID from an AMBA device ID and mask.
> + #[inline(always)]
> + pub const fn new(id: u32, mask: u32) -> Self {
> + let mut amba: bindings::amba_id = pin_init::zeroed();
> + amba.id = id;
> + amba.mask = mask;
> + Self(amba)
> + }
> +
> + /// Creates a new device ID with driver-specific data.
> + #[inline(always)]
> + pub const fn new_with_data(id: u32, mask: u32, data: usize) -> Self {
> + let mut amba: bindings::amba_id = pin_init::zeroed();
> + amba.id = id;
> + amba.mask = mask;
> + amba.data = data as *mut core::ffi::c_void;
I already mentioned that in the last version, you don't need this function and
writing the data pointer here is wrong, as it is already handled by common code,
i.e. through the offset given to RawDeviceIdIndex above.
> + Self(amba)
> + }
<snip>
> +impl Device<device::Core> {}
No need to add an empty impl block.
> +impl Device<device::Bound> {
> + /// Returns an [`IoRequest`] for the memory resource.
> + pub fn io_request(&self) -> Option<IoRequest<'_>> {
> + self.resource()
> + // SAFETY: `resource` is valid for the lifetime of the `IoRequest`.
> + .map(|resource| unsafe { IoRequest::new(self.as_ref(), resource) })
> + }
> +
> + /// Returns an [`IrqRequest`] for the IRQ at the given index.
> + pub fn irq_by_index(&self, index: u32) -> Result<IrqRequest<'_>> {
> + if index >= bindings::AMBA_NR_IRQS {
> + return Err(EINVAL);
> + }
> + // SAFETY: `self.as_raw()` returns a valid pointer to a `struct amba_device`.
> + let irq = unsafe { (*self.as_raw()).irq[index as usize] };
> + if irq == 0 {
> + return Err(ENXIO);
> + }
> + // SAFETY: `irq` is guaranteed to be a valid IRQ number for `&self`.
> + Ok(unsafe { IrqRequest::new(self.as_ref(), irq) })
> + }
> +
> + /// Returns a [`irq::Registration`] for the IRQ at the given index.
> + pub fn request_irq_by_index<'a, T: irq::Handler + 'static>(
> + &'a self,
> + flags: irq::Flags,
> + index: u32,
> + name: &'static CStr,
> + handler: impl PinInit<T, Error> + 'a,
> + ) -> impl PinInit<irq::Registration<T>, Error> + 'a {
> + pin_init::pin_init_scope(move || {
> + let request = self.irq_by_index(index).map_err(|_| EINVAL)?;
Why do you remap the error code from irq_by_index() to EINVAL unconditionally?
> + Ok(irq::Registration::<T>::new(request, flags, name, handler))
> + })
> + }
> +}
<snip>
> + extern "C" fn shutdown_callback(adev: *mut bindings::amba_device) {
> + // SAFETY: `shutdown_callback` is only ever called for a valid `adev`.
> + let adev = unsafe { &*adev.cast::<Device<device::CoreInternal>>() };
> + // SAFETY: `shutdown_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>>`.
> + let data = unsafe { adev.as_ref().drvdata_obtain::<T>() };
Please use drvdata_borrow() instead, we must not free the device private data on
shutdown().
> + T::shutdown(adev, data.as_ref());
> + }
> +
> + fn amba_id_table() -> Option<IdTable<<Self as driver::Adapter>::IdInfo>> {
> + T::AMBA_ID_TABLE
> + }
> +
> + fn amba_id_info(
> + _dev: &Device,
> + id: *const bindings::amba_id,
> + ) -> Option<&'static <Self as driver::Adapter>::IdInfo> {
> + if id.is_null() {
> + return None;
> + }
> + let table = Self::amba_id_table()?;
> + // SAFETY: `id` is a valid pointer to a `struct amba_id` that was matched by the kernel.
> + // `DeviceId` is a `#[repr(transparent)]` wrapper of `struct amba_id` and does not add
> + // additional invariants, so it's safe to transmute.
> + let device_id = unsafe { &*id.cast::<DeviceId>() };
> + Some(table.info(<DeviceId as RawDeviceIdIndex>::index(device_id)))
> + }
> +}
> +
> +impl<T: Driver + 'static> driver::Adapter for Adapter<T> {
> + type IdInfo = T::IdInfo;
> +
> + fn acpi_id_table() -> Option<acpi::IdTable<Self::IdInfo>> {
> + None
> + }
> +
> + fn of_id_table() -> Option<of::IdTable<Self::IdInfo>> {
> + None
> + }
There is no point implementing this trait with both functions returning None.
> +/// The AMBA driver trait.
> +///
> +/// Drivers must implement this trait in order to get an AMBA driver registered.
> +pub trait Driver: Send {
> + /// The type holding information about each device id supported by the driver.
> + type IdInfo: 'static;
> + /// The table of device ids supported by the driver.
> + const AMBA_ID_TABLE: Option<IdTable<Self::IdInfo>> = None;
If it is the only ID table an AMBA driver can be matched through, it does not
make sense for the ID table to be optional.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH v2 4/5] rust: add RTC core abstractions and data structures
2026-01-07 14:37 ` [RFC PATCH v2 4/5] rust: add RTC core abstractions and data structures Ke Sun
@ 2026-01-08 11:50 ` Danilo Krummrich
2026-01-08 13:17 ` Ke Sun
2026-01-08 23:31 ` Kari Argillander
1 sibling, 1 reply; 32+ messages in thread
From: Danilo Krummrich @ 2026-01-08 11:50 UTC (permalink / raw)
To: Ke Sun
Cc: Alexandre Belloni, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, linux-rtc, rust-for-linux, Alvin Sun
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/
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH v2 5/5] rust: add PL031 RTC driver
2026-01-07 14:37 ` [RFC PATCH v2 5/5] rust: add PL031 RTC driver Ke Sun
@ 2026-01-08 11:57 ` Danilo Krummrich
0 siblings, 0 replies; 32+ messages in thread
From: Danilo Krummrich @ 2026-01-08 11:57 UTC (permalink / raw)
To: Ke Sun
Cc: Alexandre Belloni, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, linux-rtc, rust-for-linux, Alvin Sun
On Wed Jan 7, 2026 at 3:37 PM CET, Ke Sun wrote:
> +impl amba::Driver for Pl031AmbaDriver {
> + type IdInfo = Pl031Variant;
> + const AMBA_ID_TABLE: Option<amba::IdTable<Self::IdInfo>> = Some(&ID_TABLE);
> +
> + fn probe(
> + adev: &amba::Device<Core>,
> + id_info: Option<&Self::IdInfo>,
> + ) -> impl PinInit<Self, Error> {
> + let dev = adev.as_ref();
> + let io_request = adev.io_request().ok_or(ENODEV)?;
> + let variant = id_info
> + .map(|info| info.variant)
> + .unwrap_or(VendorVariant::Arm);
> +
> + let rtcdev = RtcDevice::<Pl031DrvData>::new(
> + dev,
> + try_pin_init!(Pl031DrvData {
> + base <- IoMem::new(io_request),
> + variant,
> + }),
> + )?;
> +
> + dev.devm_init_wakeup()?;
> +
> + let drvdata = rtcdev.drvdata()?;
> + let base_guard = drvdata.base.try_access().ok_or(ENXIO)?;
You can just use drvdata.base.access(adev.as_ref()), the &amba::Device<Core>
(which derives to device::Device<Bound>) proves that this access is valid
without any sychronization.
(This is also the reason why you want the parent device as &Device<Bound> in
your class device callbacks.)
> + let base = base_guard.deref();
> +
> + let mut cr = base.read32(RTC_CR);
> + if variant.clockwatch() {
> + cr |= RTC_CR_CWEN;
> + } else {
> + cr |= RTC_CR_EN;
> + }
> + base.write32(cr, RTC_CR);
> +
> + if variant.st_weekday() {
> + let bcd_year = base.read32(RTC_YDR);
> + if bcd_year == 0x2000 {
> + let st_time = base.read32(RTC_DR);
> + if (st_time & (RTC_MON_MASK | RTC_MDAY_MASK | RTC_WDAY_MASK)) == 0x02120000 {
> + base.write32(0x2000, RTC_YLR);
> + base.write32(st_time | (0x7 << RTC_WDAY_SHIFT), RTC_LR);
> + }
> + }
> + }
> +
> + rtcdev.set_range_min(variant.range_min());
> + rtcdev.set_range_max(variant.range_max());
> +
> + let irq_flags = if variant == VendorVariant::StV2 {
> + kernel::irq::Flags::SHARED | kernel::irq::Flags::COND_SUSPEND
> + } else {
> + kernel::irq::Flags::SHARED
> + };
> +
> + let rtcdev_clone = rtcdev.clone();
> + let init = adev.request_irq_by_index(
> + irq_flags,
> + 0,
> + c_str!("rtc-pl031"),
> + try_pin_init!(Pl031IrqHandler {
> + _pin: PhantomPinned,
> + rtcdev: rtcdev_clone,
> + }),
> + );
> +
> + match kernel::devres::register(dev, init, GFP_KERNEL) {
This is not needed, request_irq_by_index() already returns an
impl PinInit<Devres<_>, Error>, just store this in Pl031AmbaDriver.
> + Ok(()) => {
> + if let Ok(irq) = adev.irq_by_index(0) {
> + irq.set_wake_irq()?;
> + }
> + }
> + Err(_) => rtcdev.clear_feature(bindings::RTC_FEATURE_ALARM),
> + }
> +
> + Registration::<Pl031DrvData>::register(dev, rtcdev)?;
> + Ok(Pl031AmbaDriver)
> + }
> +}
> +
> +#[pinned_drop]
> +impl PinnedDrop for Pl031DrvData {
> + fn drop(self: Pin<&mut Self>) {
> + // Resources are automatically cleaned up by devres.
> + }
> +}
No need for this empty impl.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH v2 4/5] rust: add RTC core abstractions and data structures
2026-01-08 11:50 ` Danilo Krummrich
@ 2026-01-08 13:17 ` Ke Sun
2026-01-08 13:49 ` Miguel Ojeda
0 siblings, 1 reply; 32+ messages in thread
From: Ke Sun @ 2026-01-08 13:17 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Alexandre Belloni, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, linux-rtc, rust-for-linux, Alvin Sun
On 1/8/26 19:50, Danilo Krummrich wrote:
> 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?
Yes. Dirk suggested doctest in another patch series, which I enabled. I also
run clippy checks and QEMU tests for every change.
❯ cat .config| grep DOCTEST
CONFIG_RUST_KERNEL_DOCTESTS=y
❯ make Image CLIPPY=1
CALL scripts/checksyscalls.sh
>
>> +/// ```
>> +///
>> +/// [`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/
Some code was implemented by mimicking existing patterns without fully
understanding the rationale behind them.
Thanks for the detailed review. I'll carefully go through your comments.
Best regards,
Ke Sun
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH v2 1/5] rtc: migrate driver data to RTC device
2026-01-08 11:12 ` Danilo Krummrich
@ 2026-01-08 13:45 ` Ke Sun
2026-01-08 13:52 ` Danilo Krummrich
0 siblings, 1 reply; 32+ messages in thread
From: Ke Sun @ 2026-01-08 13:45 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Alexandre Belloni, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, linux-rtc, rust-for-linux, Alvin Sun
On 1/8/26 19:12, Danilo Krummrich wrote:
> On Wed Jan 7, 2026 at 3:37 PM CET, Ke Sun wrote:
>> diff --git a/drivers/rtc/dev.c b/drivers/rtc/dev.c
>> index baf1a8ca8b2b1..0f62ba9342e3e 100644
>> --- a/drivers/rtc/dev.c
>> +++ b/drivers/rtc/dev.c
>> @@ -410,7 +410,7 @@ static long rtc_dev_ioctl(struct file *file,
>> }
>> default:
>> if (rtc->ops->param_get)
>> - err = rtc->ops->param_get(rtc->dev.parent, ¶m);
>> + err = rtc->ops->param_get(&rtc->dev, ¶m);
> It would make more sense to just pass a struct rtc_device than the embedded
> struct device in the RTC callbacks.
I considered passing struct rtc_device directly, but chose &rtc->dev
to minimize changes to existing drivers, since most callbacks use
dev_get_drvdata() on the device parameter.
>
>> @@ -369,7 +364,7 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
>> goto out;
>>
>> if (adev->irq[0]) {
>> - ret = request_irq(adev->irq[0], pl031_interrupt,
>> + ret = devm_request_irq(&adev->dev, adev->irq[0], pl031_interrupt,
>> vendor->irqflags, "rtc-pl031", ldata);
> As Greg already mentioned that change should be a separate patch.
>
> You also have to be careful with the devres order when using devm_request_irq().
>
> In your case, you pass ldata, so you have to ensure that ldata (and its
> contents) remain valid until the devres callback frees the IRQ request.
I'll only start modifying other RTC drivers after I have a clear
understanding of all the details, and I'll minimize any functional
changes.
These C RTC refactoring patches will be sent as a new patch series
to the RTC mailing list.
Best regards,
Ke Sun
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH v2 4/5] rust: add RTC core abstractions and data structures
2026-01-08 13:17 ` Ke Sun
@ 2026-01-08 13:49 ` Miguel Ojeda
2026-01-08 13:56 ` Ke Sun
0 siblings, 1 reply; 32+ messages in thread
From: Miguel Ojeda @ 2026-01-08 13:49 UTC (permalink / raw)
To: Ke Sun
Cc: Danilo Krummrich, Alexandre Belloni, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, linux-rtc, rust-for-linux, Alvin Sun
On Thu, Jan 8, 2026 at 2:17 PM Ke Sun <sunke@kylinos.cn> wrote:
>
> Yes. Dirk suggested doctest in another patch series, which I enabled. I also
>
> run clippy checks and QEMU tests for every change.
>
>
> ❯ cat .config| grep DOCTEST
> CONFIG_RUST_KERNEL_DOCTESTS=y
> ❯ make Image CLIPPY=1
> CALL scripts/checksyscalls.sh
It may be hard to see without syntax highlighting, but the code is
commented out:
/// // rtc.set_range_min(0);
So that is why the example "builds". That `rtc` variable is not defined.
It is also not well-formatted.
Please manually double-check.
Thanks!
Cheers,
Miguel
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH v2 1/5] rtc: migrate driver data to RTC device
2026-01-08 13:45 ` Ke Sun
@ 2026-01-08 13:52 ` Danilo Krummrich
2026-01-08 14:01 ` Ke Sun
2026-01-08 14:01 ` Alexandre Belloni
0 siblings, 2 replies; 32+ messages in thread
From: Danilo Krummrich @ 2026-01-08 13:52 UTC (permalink / raw)
To: Ke Sun
Cc: Alexandre Belloni, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, linux-rtc, rust-for-linux, Alvin Sun
On Thu Jan 8, 2026 at 2:45 PM CET, Ke Sun wrote:
>
> On 1/8/26 19:12, Danilo Krummrich wrote:
>> On Wed Jan 7, 2026 at 3:37 PM CET, Ke Sun wrote:
>>> diff --git a/drivers/rtc/dev.c b/drivers/rtc/dev.c
>>> index baf1a8ca8b2b1..0f62ba9342e3e 100644
>>> --- a/drivers/rtc/dev.c
>>> +++ b/drivers/rtc/dev.c
>>> @@ -410,7 +410,7 @@ static long rtc_dev_ioctl(struct file *file,
>>> }
>>> default:
>>> if (rtc->ops->param_get)
>>> - err = rtc->ops->param_get(rtc->dev.parent, ¶m);
>>> + err = rtc->ops->param_get(&rtc->dev, ¶m);
>> It would make more sense to just pass a struct rtc_device than the embedded
>> struct device in the RTC callbacks.
> I considered passing struct rtc_device directly, but chose &rtc->dev
> to minimize changes to existing drivers, since most callbacks use
> dev_get_drvdata() on the device parameter.
No, you should not expose the embedded base device. For accessing the private
data you should add helpers like rtc_get_drvdata(). This is what other
subsystems do as well, e.g. [1].
[1] https://elixir.bootlin.com/linux/v6.18.3/source/include/linux/i2c.h#L371
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH v2 4/5] rust: add RTC core abstractions and data structures
2026-01-08 13:49 ` Miguel Ojeda
@ 2026-01-08 13:56 ` Ke Sun
0 siblings, 0 replies; 32+ messages in thread
From: Ke Sun @ 2026-01-08 13:56 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Danilo Krummrich, Alexandre Belloni, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, linux-rtc, rust-for-linux, Alvin Sun
On 1/8/26 21:49, Miguel Ojeda wrote:
> On Thu, Jan 8, 2026 at 2:17 PM Ke Sun <sunke@kylinos.cn> wrote:
>> Yes. Dirk suggested doctest in another patch series, which I enabled. I also
>>
>> run clippy checks and QEMU tests for every change.
>>
>>
>> ❯ cat .config| grep DOCTEST
>> CONFIG_RUST_KERNEL_DOCTESTS=y
>> ❯ make Image CLIPPY=1
>> CALL scripts/checksyscalls.sh
> It may be hard to see without syntax highlighting, but the code is
> commented out:
>
> /// // rtc.set_range_min(0);
>
> So that is why the example "builds". That `rtc` variable is not defined.
>
> It is also not well-formatted.
>
> Please manually double-check.
Will do. I'll check more carefully.
Best regards,
Ke Sun
>
> Thanks!
>
> Cheers,
> Miguel
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH v2 1/5] rtc: migrate driver data to RTC device
2026-01-08 13:52 ` Danilo Krummrich
@ 2026-01-08 14:01 ` Ke Sun
2026-01-08 14:01 ` Alexandre Belloni
1 sibling, 0 replies; 32+ messages in thread
From: Ke Sun @ 2026-01-08 14:01 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Alexandre Belloni, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, linux-rtc, rust-for-linux, Alvin Sun
On 1/8/26 21:52, Danilo Krummrich wrote:
> On Thu Jan 8, 2026 at 2:45 PM CET, Ke Sun wrote:
>> On 1/8/26 19:12, Danilo Krummrich wrote:
>>> On Wed Jan 7, 2026 at 3:37 PM CET, Ke Sun wrote:
>>>> diff --git a/drivers/rtc/dev.c b/drivers/rtc/dev.c
>>>> index baf1a8ca8b2b1..0f62ba9342e3e 100644
>>>> --- a/drivers/rtc/dev.c
>>>> +++ b/drivers/rtc/dev.c
>>>> @@ -410,7 +410,7 @@ static long rtc_dev_ioctl(struct file *file,
>>>> }
>>>> default:
>>>> if (rtc->ops->param_get)
>>>> - err = rtc->ops->param_get(rtc->dev.parent, ¶m);
>>>> + err = rtc->ops->param_get(&rtc->dev, ¶m);
>>> It would make more sense to just pass a struct rtc_device than the embedded
>>> struct device in the RTC callbacks.
>> I considered passing struct rtc_device directly, but chose &rtc->dev
>> to minimize changes to existing drivers, since most callbacks use
>> dev_get_drvdata() on the device parameter.
> No, you should not expose the embedded base device. For accessing the private
> data you should add helpers like rtc_get_drvdata(). This is what other
> subsystems do as well, e.g. [1].
>
> [1] https://elixir.bootlin.com/linux/v6.18.3/source/include/linux/i2c.h#L371
Yes, that's the right approach. I'll use it in the new patch series.
Best regards,
Ke Sun
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH v2 1/5] rtc: migrate driver data to RTC device
2026-01-08 13:52 ` Danilo Krummrich
2026-01-08 14:01 ` Ke Sun
@ 2026-01-08 14:01 ` Alexandre Belloni
2026-01-08 14:06 ` Danilo Krummrich
2026-01-14 23:23 ` Ke Sun
1 sibling, 2 replies; 32+ messages in thread
From: Alexandre Belloni @ 2026-01-08 14:01 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Ke Sun, Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
linux-rtc, rust-for-linux, Alvin Sun
On 08/01/2026 14:52:08+0100, Danilo Krummrich wrote:
> On Thu Jan 8, 2026 at 2:45 PM CET, Ke Sun wrote:
> >
> > On 1/8/26 19:12, Danilo Krummrich wrote:
> >> On Wed Jan 7, 2026 at 3:37 PM CET, Ke Sun wrote:
> >>> diff --git a/drivers/rtc/dev.c b/drivers/rtc/dev.c
> >>> index baf1a8ca8b2b1..0f62ba9342e3e 100644
> >>> --- a/drivers/rtc/dev.c
> >>> +++ b/drivers/rtc/dev.c
> >>> @@ -410,7 +410,7 @@ static long rtc_dev_ioctl(struct file *file,
> >>> }
> >>> default:
> >>> if (rtc->ops->param_get)
> >>> - err = rtc->ops->param_get(rtc->dev.parent, ¶m);
> >>> + err = rtc->ops->param_get(&rtc->dev, ¶m);
> >> It would make more sense to just pass a struct rtc_device than the embedded
> >> struct device in the RTC callbacks.
> > I considered passing struct rtc_device directly, but chose &rtc->dev
> > to minimize changes to existing drivers, since most callbacks use
> > dev_get_drvdata() on the device parameter.
>
> No, you should not expose the embedded base device. For accessing the private
> data you should add helpers like rtc_get_drvdata(). This is what other
> subsystems do as well, e.g. [1].
>
> [1] https://elixir.bootlin.com/linux/v6.18.3/source/include/linux/i2c.h#L371
This is not a correct example as i2c is a bus, just like amba is...
Actually, I don't think the rework is necessary at all or this would
mean we need to rewor most of our existing subsystems.
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH v2 1/5] rtc: migrate driver data to RTC device
2026-01-08 14:01 ` Alexandre Belloni
@ 2026-01-08 14:06 ` Danilo Krummrich
2026-02-20 23:19 ` Alexandre Belloni
2026-01-14 23:23 ` Ke Sun
1 sibling, 1 reply; 32+ messages in thread
From: Danilo Krummrich @ 2026-01-08 14:06 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Ke Sun, Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
linux-rtc, rust-for-linux, Alvin Sun
On Thu Jan 8, 2026 at 3:01 PM CET, Alexandre Belloni wrote:
> On 08/01/2026 14:52:08+0100, Danilo Krummrich wrote:
>> On Thu Jan 8, 2026 at 2:45 PM CET, Ke Sun wrote:
>> >
>> > On 1/8/26 19:12, Danilo Krummrich wrote:
>> >> On Wed Jan 7, 2026 at 3:37 PM CET, Ke Sun wrote:
>> >>> diff --git a/drivers/rtc/dev.c b/drivers/rtc/dev.c
>> >>> index baf1a8ca8b2b1..0f62ba9342e3e 100644
>> >>> --- a/drivers/rtc/dev.c
>> >>> +++ b/drivers/rtc/dev.c
>> >>> @@ -410,7 +410,7 @@ static long rtc_dev_ioctl(struct file *file,
>> >>> }
>> >>> default:
>> >>> if (rtc->ops->param_get)
>> >>> - err = rtc->ops->param_get(rtc->dev.parent, ¶m);
>> >>> + err = rtc->ops->param_get(&rtc->dev, ¶m);
>> >> It would make more sense to just pass a struct rtc_device than the embedded
>> >> struct device in the RTC callbacks.
>> > I considered passing struct rtc_device directly, but chose &rtc->dev
>> > to minimize changes to existing drivers, since most callbacks use
>> > dev_get_drvdata() on the device parameter.
>>
>> No, you should not expose the embedded base device. For accessing the private
>> data you should add helpers like rtc_get_drvdata(). This is what other
>> subsystems do as well, e.g. [1].
>>
>> [1] https://elixir.bootlin.com/linux/v6.18.3/source/include/linux/i2c.h#L371
>
> This is not a correct example as i2c is a bus, just like amba is...
Yes, struct i2c_client is indeed a bus device. However, the core struct device
is what holds the device private data commonly in the same way, regardless of
whether it is embedded in a bus or class device.
If you look for a class device example, here's PWM [2] and input [3].
[2] https://elixir.bootlin.com/linux/v6.18.3/source/include/linux/pwm.h#L382
[3] https://elixir.bootlin.com/linux/v6.18.3/source/include/linux/input.h#L388
> Actually, I don't think the rework is necessary at all or this would
> mean we need to rewor most of our existing subsystems.
That's not true, subsystems do not pass the parent device (i.e. the bus device)
through their class device callbacks exclusively.
- Danilo
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH v2 4/5] rust: add RTC core abstractions and data structures
2026-01-07 14:37 ` [RFC PATCH v2 4/5] rust: add RTC core abstractions and data structures Ke Sun
2026-01-08 11:50 ` Danilo Krummrich
@ 2026-01-08 23:31 ` Kari Argillander
1 sibling, 0 replies; 32+ messages in thread
From: Kari Argillander @ 2026-01-08 23:31 UTC (permalink / raw)
To: Ke Sun
Cc: Alexandre Belloni, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, linux-rtc, rust-for-linux,
Alvin Sun
On Wed, 7 Jan 2026 at 19:57, Ke Sun <sunke@kylinos.cn> wrote:
>
> Add Rust abstractions for RTC subsystem, including RtcDevice,
> RtcTime, RtcWkAlrm data structures, RtcOps trait for driver
> operations, and devm-managed device registration support.
>
> Signed-off-by: Ke Sun <sunke@kylinos.cn>
> ---
> rust/helpers/helpers.c | 1 +
> rust/helpers/rtc.c | 9 +
> rust/kernel/lib.rs | 2 +
> rust/kernel/rtc.rs | 985 +++++++++++++++++++++++++++++++++++++++++
All new files needs MAINTAINER entry at the end. I know this is RFC but
just saying.
> 4 files changed, 997 insertions(+)
> create mode 100644 rust/helpers/rtc.c
> create mode 100644 rust/kernel/rtc.rs
>
> diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
> index 79c72762ad9c4..1a5c103fb24ba 100644
> --- a/rust/helpers/helpers.c
> +++ b/rust/helpers/helpers.c
> @@ -48,6 +48,7 @@
> #include "rcu.c"
> #include "refcount.c"
> #include "regulator.c"
> +#include "rtc.c"
> #include "scatterlist.c"
> #include "security.c"
> #include "signal.c"
> diff --git a/rust/helpers/rtc.c b/rust/helpers/rtc.c
> new file mode 100644
> index 0000000000000..862cd61670bfc
> --- /dev/null
> +++ b/rust/helpers/rtc.c
> @@ -0,0 +1,9 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/rtc.h>
> +
> +int rust_helper_devm_rtc_register_device(struct rtc_device *rtc)
> +{
> + return __devm_rtc_register_device(THIS_MODULE, rtc);
> +}
> +
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 3e557335fc5fe..1390073e4ae27 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -135,6 +135,8 @@
> pub mod rbtree;
> pub mod regulator;
> pub mod revocable;
> +#[cfg(CONFIG_RTC_CLASS)]
> +pub mod rtc;
> pub mod scatterlist;
> pub mod security;
> pub mod seq_file;
> diff --git a/rust/kernel/rtc.rs b/rust/kernel/rtc.rs
> new file mode 100644
> index 0000000000000..0c662b94b96f4
> --- /dev/null
> +++ b/rust/kernel/rtc.rs
> @@ -0,0 +1,985 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! RTC (Real-Time Clock) device support.
> +//!
> +//! C headers: [`include/linux/rtc.h`](srctree/include/linux/rtc.h).
> +//!
> +//! Reference: <https://www.kernel.org/doc/html/latest/driver-api/rtc.html>
> +use crate::{
> + bindings,
> + bitmap::Bitmap,
> + device::{
> + self,
> + AsBusDevice, //
> + },
> + devres,
> + error::Error,
> + prelude::*,
> + seq_file::SeqFile,
> + sync::aref::{
> + ARef, //
> + AlwaysRefCounted,
> + },
> + types::{
> + ForeignOwnable,
> + Opaque, //
> + }, //
> +};
> +
> +use core::{
> + ffi::c_void,
> + marker::PhantomData,
> + ptr::NonNull, //
> +};
> +
> +/// RTC time structure.
> +///
> +/// Wraps the C `struct rtc_time` from `include/uapi/linux/rtc.h`.
> +#[repr(transparent)]
> +#[derive(Clone, Copy)]
> +pub struct RtcTime(pub bindings::rtc_time);
Probably Debug would be nice for this. As rtc_time is same as
`struct tm` I wonder should we just make struct tm and make RtcTime
type alias for that. That way we do not need to duplicate all the
things.
> +impl RtcTime {
> + /// Creates a new `RtcTime` from a time64_t value (seconds since 1970-01-01 00:00:00 UTC).
> + pub fn from_time64(time: i64) -> Self {
> + let mut tm = Self(pin_init::zeroed());
> + // SAFETY: `rtc_time64_to_tm` is a pure function that only writes to the provided
> + // `struct rtc_time` pointer. The pointer is valid because `tm.0` is a valid mutable
> + // reference, and the function does not retain any references to it.
> + unsafe {
> + bindings::rtc_time64_to_tm(time, &mut tm.0);
> + }
> + tm
> + }
> +
> + /// Converts this `RtcTime` to a time64_t value (seconds since 1970-01-01 00:00:00 UTC).
> + pub fn to_time64(&self) -> i64 {
> + // SAFETY: `rtc_tm_to_time64` is a pure function that only reads from the provided
> + // `struct rtc_time` pointer. The pointer is valid because `self.0` is a valid reference,
> + // and the function does not retain any references to it.
> + unsafe { bindings::rtc_tm_to_time64(core::ptr::from_ref(&self.0).cast_mut()) }
> + }
> +
> + /// Sets this `RtcTime` from a time64_t value (seconds since 1970-01-01 00:00:00 UTC).
> + pub fn set_from_time64(&mut self, time: i64) {
> + // SAFETY: `rtc_time64_to_tm` is a pure function that only writes to the provided
> + // `struct rtc_time` pointer. The pointer is valid because `self.0` is a valid mutable
> + // reference, and the function does not retain any references to it.
> + unsafe {
> + bindings::rtc_time64_to_tm(time, &mut self.0);
> + }
> + }
> +
> + /// Converts a time64_t value to an RTC time structure.
> + #[inline]
> + pub fn time64_to_tm(time: i64, tm: &mut Self) {
> + tm.set_from_time64(time);
> + }
> +
> + /// Converts an RTC time structure to a time64_t value (seconds since 1970-01-01 00:00:00 UTC).
> + #[inline]
> + pub fn tm_to_time64(tm: &Self) -> i64 {
> + tm.to_time64()
> + }
> +
> + /// Calculates the day of the year (0-365) from the date components.
> + #[inline]
> + pub fn year_days(&self) -> i32 {
> + // SAFETY: `rtc_year_days` is a pure function that only performs calculations based on
> + // the provided parameters. The values should be from valid RTC time structures and
> + // non-negative, but the function itself is safe to call with any values.
> + unsafe {
> + bindings::rtc_year_days(
> + self.0.tm_mday as u32,
> + self.0.tm_mon as u32,
> + self.0.tm_year as u32,
> + )
> + }
> + }
> +
> + /// Returns the seconds field (0-59).
> + #[inline]
> + pub fn tm_sec(&self) -> i32 {
> + self.0.tm_sec
> + }
> +
> + /// Sets the seconds field (0-59).
> + #[inline]
> + pub fn set_tm_sec(&mut self, sec: i32) {
> + self.0.tm_sec = sec;
> + }
> +
> + /// Returns the minutes field (0-59).
> + #[inline]
> + pub fn tm_min(&self) -> i32 {
> + self.0.tm_min
> + }
> +
> + /// Sets the minutes field (0-59).
> + #[inline]
> + pub fn set_tm_min(&mut self, min: i32) {
> + self.0.tm_min = min;
> + }
> +
> + /// Returns the hours field (0-23).
> + #[inline]
> + pub fn tm_hour(&self) -> i32 {
> + self.0.tm_hour
> + }
> +
> + /// Sets the hours field (0-23).
> + #[inline]
> + pub fn set_tm_hour(&mut self, hour: i32) {
> + self.0.tm_hour = hour;
> + }
> +
> + /// Returns the day of the month (1-31).
> + #[inline]
> + pub fn tm_mday(&self) -> i32 {
> + self.0.tm_mday
> + }
> +
> + /// Sets the day of the month (1-31).
> + #[inline]
> + pub fn set_tm_mday(&mut self, mday: i32) {
> + self.0.tm_mday = mday;
> + }
> +
> + /// Returns the month (0-11, where 0 is January).
> + #[inline]
> + pub fn tm_mon(&self) -> i32 {
> + self.0.tm_mon
> + }
> +
> + /// Sets the month (0-11, where 0 is January).
> + #[inline]
> + pub fn set_tm_mon(&mut self, mon: i32) {
> + self.0.tm_mon = mon;
> + }
> +
> + /// Returns the year (years since 1900).
> + #[inline]
> + pub fn tm_year(&self) -> i32 {
> + self.0.tm_year
> + }
> +
> + /// Sets the year (years since 1900).
> + #[inline]
> + pub fn set_tm_year(&mut self, year: i32) {
> + self.0.tm_year = year;
> + }
> +
> + /// Returns the day of the week (0-6, where 0 is Sunday).
> + #[inline]
> + pub fn tm_wday(&self) -> i32 {
> + self.0.tm_wday
> + }
> +
> + /// Sets the day of the week (0-6, where 0 is Sunday).
> + #[inline]
> + pub fn set_tm_wday(&mut self, wday: i32) {
> + self.0.tm_wday = wday;
> + }
> +
> + /// Returns the day of the year (0-365).
> + #[inline]
> + pub fn tm_yday(&self) -> i32 {
> + self.0.tm_yday
> + }
> +
> + /// Sets the day of the year (0-365).
> + #[inline]
> + pub fn set_tm_yday(&mut self, yday: i32) {
> + self.0.tm_yday = yday;
> + }
> +
> + /// Returns the daylight saving time flag.
> + #[inline]
> + pub fn tm_isdst(&self) -> i32 {
> + self.0.tm_isdst
> + }
> +
> + /// Sets the daylight saving time flag.
> + #[inline]
> + pub fn set_tm_isdst(&mut self, isdst: i32) {
> + self.0.tm_isdst = isdst;
> + }
> +}
> +
> +impl Default for RtcTime {
> + fn default() -> Self {
Could be const.
> + Self(pin_init::zeroed())
> + }
> +}
> +
> +/// RTC wake alarm structure.
> +///
> +/// Wraps the C `struct rtc_wkalrm` from `include/uapi/linux/rtc.h`.
> +#[repr(transparent)]
> +#[derive(Clone, Copy)]
> +pub struct RtcWkAlrm(pub bindings::rtc_wkalrm);
> +
> +impl Default for RtcWkAlrm {
> + fn default() -> Self {
Could be const.
> + Self(pin_init::zeroed())
> + }
> +}
> +
> +impl RtcWkAlrm {
> + /// Returns a reference to the alarm time.
> + #[inline]
> + pub fn get_time(&self) -> &RtcTime {
> + // SAFETY: `RtcTime` is `#[repr(transparent)]` over `bindings::rtc_time`, so the memory
> + // layout is identical. We can safely reinterpret the reference.
> + unsafe { &*(&raw const self.0.time).cast::<RtcTime>() }
> + }
> +
> + /// Returns a mutable reference to the alarm time.
> + #[inline]
> + pub fn get_time_mut(&mut self) -> &mut RtcTime {
> + // SAFETY: `RtcTime` is `#[repr(transparent)]` over `bindings::rtc_time`, so the memory
> + // layout is identical. We can safely reinterpret the reference.
> + unsafe { &mut *(&raw mut self.0.time).cast::<RtcTime>() }
> + }
> +
> + /// Sets the alarm time from a `RtcTime` value.
> + #[inline]
> + pub fn set_time(&mut self, time: RtcTime) {
> + self.0.time = time.0;
> + }
> +
> + /// Returns the enabled field.
> + #[inline]
> + pub fn enabled(&self) -> u8 {
> + self.0.enabled
> + }
> +
> + /// Sets the `enabled` field.
> + #[inline]
> + pub fn set_enabled(&mut self, enabled: u8) {
> + self.0.enabled = enabled;
> + }
> +
> + /// Returns the pending field.
> + #[inline]
> + pub fn pending(&self) -> u8 {
> + self.0.pending
> + }
> +
> + /// Sets the `pending` field.
> + #[inline]
> + pub fn set_pending(&mut self, pending: u8) {
> + self.0.pending = pending;
> + }
> +}
> +
> +/// RTC parameter structure.
> +///
> +/// Wraps the C `struct rtc_param` from `include/uapi/linux/rtc.h`.
> +#[repr(transparent)]
> +#[derive(Clone, Copy)]
> +pub struct RtcParam(pub bindings::rtc_param);
> +
> +impl Default for RtcParam {
> + fn default() -> Self {
Could be made const.
> + Self(pin_init::zeroed())
> + }
> +}
> +
> +/// 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);
Currently this example is commented out.
> +/// // Ok(())
> +/// // }
> +/// ```
> +///
> +/// [`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;
> + }
Put ';' outside of unsafe that way this will be onliner. See also other places.
So
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> {
> + 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);
> +}
> +
> +// SAFETY: Instances of `RtcDevice` are always reference-counted via the underlying `device`.
> +// The `struct rtc_device` contains a `struct device dev` as its first field, and the
> +// reference counting is managed through `get_device`/`put_device` on the `dev` field.
> +unsafe impl<T: 'static> AlwaysRefCounted for RtcDevice<T> {
> + fn inc_ref(&self) {
> + let dev: &device::Device = self.as_ref();
> + // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
> + // `dev.as_raw()` is a valid pointer to a `struct device` with a non-zero refcount.
> + unsafe { bindings::get_device(dev.as_raw()) };
> + }
> +
> + unsafe fn dec_ref(obj: NonNull<Self>) {
> + let rtc: *mut bindings::rtc_device = obj.cast().as_ptr();
> +
> + // SAFETY: By the type invariant of `Self`, `rtc` is a pointer to a valid
> + // `struct rtc_device`. The `dev` field is the first field of `struct rtc_device`,
> + // so we can safely access it.
> + let dev = unsafe { &raw mut (*rtc).dev };
> +
> + // SAFETY: The safety requirements guarantee that the refcount is non-zero.
> + unsafe { bindings::put_device(dev) };
> + }
> +}
> +
> +// SAFETY: `RtcDevice` is reference-counted and can be released from any thread.
> +unsafe impl<T: 'static> Send for RtcDevice<T> {}
> +
> +// SAFETY: `RtcDevice` can be shared among threads because all immutable methods are
> +// protected by the synchronization in `struct rtc_device` (via `ops_lock` mutex).
> +unsafe impl<T: 'static> Sync for RtcDevice<T> {}
> +
> +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,
> + 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);
> + }
You could construct NonNull with new() and convert None to ENOMEM.
That way you do not need to use new_unchecked.
> +
> + // 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 {
> + 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(())
> + }
> +
> + /// Borrow the driver's private data bound to this [`RtcDevice`].
> + pub fn drvdata(&self) -> Result<Pin<&T>> {
> + let dev: &device::Device<device::Bound> = self.as_ref();
> +
> + // SAFETY: `self.as_raw()` is a valid pointer to a `struct device`.
> + let ptr = unsafe { bindings::dev_get_drvdata(dev.as_raw()) };
> +
> + if ptr.is_null() {
> + return Err(ENOENT);
> + }
> +
> + // SAFETY: The caller ensures that `ptr` is valid and writable.
> + Ok(unsafe { Pin::<KBox<T>>::borrow(ptr.cast()) })
> + }
> +}
> +
> +impl<T: 'static> Drop for RtcDevice<T> {
> + fn drop(&mut self) {
> + let dev: &device::Device<device::Bound> = self.as_ref();
> + // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
> + let ptr: *mut c_void = unsafe { bindings::dev_get_drvdata(dev.as_raw()) };
> +
> + // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
> + unsafe { bindings::dev_set_drvdata(dev.as_raw(), core::ptr::null_mut()) };
> +
> + if !ptr.is_null() {
> + // SAFETY: `ptr` comes from a previous call to `into_foreign()`, and
> + // `dev_get_drvdata()` guarantees to return the same pointer given to
> + // `dev_set_drvdata()`.
> + unsafe { drop(Pin::<KBox<T>>::from_foreign(ptr.cast())) };
> + }
> + }
> +}
> +
> +/// 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));
> + }
You can do
to_result(unsafe {
bindings::devm_rtc_register_device(rtc_device.as_raw()) })?;
> + let registration = Registration { rtc_device };
> +
> + devres::register(dev, registration, GFP_KERNEL)
> + }
> +}
> +
> +/// 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 {
> + /// Read the current time from the RTC.
> + ///
> + /// This is a required method and must be implemented.
> + fn read_time(_rtcdev: &RtcDevice<Self>, _tm: &mut RtcTime) -> Result {
> + Err(ENOTSUPP)
> + }
> +
> + /// Set the time in the RTC.
> + ///
> + /// This is a required method and must be implemented.
> + ///
> + /// Note: The parameter is `&mut` to match the C API signature, even though
> + /// it's conceptually read-only from the Rust side.
> + fn set_time(_rtcdev: &RtcDevice<Self>, _tm: &mut RtcTime) -> Result {
> + Err(ENOTSUPP)
> + }
> +
> + /// Read the alarm time from the RTC.
> + fn read_alarm(_rtcdev: &RtcDevice<Self>, _alarm: &mut RtcWkAlrm) -> Result {
> + Err(ENOTSUPP)
> + }
> +
> + /// Set the alarm time in the RTC.
> + ///
> + /// Note: The parameter is `&mut` to match the C API signature, even though
> + /// it's conceptually read-only from the Rust side.
> + fn set_alarm(_rtcdev: &RtcDevice<Self>, _alarm: &mut RtcWkAlrm) -> Result {
> + Err(ENOTSUPP)
> + }
> +
> + /// Enable or disable the alarm interrupt.
> + ///
> + /// `enabled` is non-zero to enable, zero to disable.
> + fn alarm_irq_enable(_rtcdev: &RtcDevice<Self>, _enabled: u32) -> Result {
> + Err(ENOTSUPP)
> + }
> +
> + /// Handle custom ioctl commands.
> + fn ioctl(_rtcdev: &RtcDevice<Self>, _cmd: u32, _arg: c_ulong) -> Result<c_int> {
> + Err(ENOTSUPP)
> + }
> +
> + /// Show information in /proc/driver/rtc.
> + fn proc(_rtcdev: &RtcDevice<Self>, _seq: &mut SeqFile) -> Result {
> + Err(ENOTSUPP)
> + }
> +
> + /// Read the time offset.
> + fn read_offset(_rtcdev: &RtcDevice<Self>, _offset: &mut i64) -> Result {
Do not just blindly copy C interface. This example would probably much
better be that we return offset. So Result<i64>. That is much more
idiomatic Rust.
> + Err(ENOTSUPP)
> + }
> +
> + /// Set the time offset.
> + fn set_offset(_rtcdev: &RtcDevice<Self>, _offset: i64) -> Result {
> + Err(ENOTSUPP)
> + }
Why not use
build_error!(VTABLE_DEFAULT_ERROR)
for these?
> + /// Get an RTC parameter.
> + fn param_get(_rtcdev: &RtcDevice<Self>, _param: &mut RtcParam) -> Result {
> + Err(ENOTSUPP)
> + }
> +
> + /// Set an RTC parameter.
> + ///
> + /// Note: The parameter is `&mut` to match the C API signature, even though
> + /// it's conceptually read-only from the Rust side.
> + fn param_set(_rtcdev: &RtcDevice<Self>, _param: &mut RtcParam) -> Result {
> + Err(ENOTSUPP)
> + }
> +}
> +
> +struct Adapter<T: RtcOps> {
> + _p: PhantomData<T>,
> +}
> +
> +impl<T: RtcOps> Adapter<T> {
> + const VTABLE: RtcOpsVTable = create_rtc_ops::<T>();
> +
> + /// # Safety
> + ///
> + /// `dev` must be a valid pointer to the `struct device` embedded in a `struct rtc_device`.
> + /// `tm` must be a valid pointer to a `struct rtc_time`.
> + unsafe extern "C" fn read_time(
> + dev: *mut bindings::device,
> + tm: *mut bindings::rtc_time,
> + ) -> c_int {
> + // SAFETY: The caller ensures that `dev` is a valid pointer to a `struct device`.
> + let device_dev: &device::Device = unsafe { device::Device::from_raw(dev) };
> + // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
> + // `AsBusDevice` to get it.
> + let rtc_dev = unsafe { RtcDevice::<T>::from_device(device_dev) };
> + // SAFETY: The caller ensures that `tm` is valid and writable.
> + // `RtcTime` is `#[repr(transparent)]` over `bindings::rtc_time`, so we can safely cast.
> + let rtc_tm = unsafe { &mut *tm.cast::<RtcTime>() };
> +
> + match T::read_time(rtc_dev, rtc_tm) {
> + Ok(()) => 0,
> + Err(err) => err.to_errno(),
> + }
> + }
> +
> + /// # Safety
> + ///
> + /// `dev` must be a valid pointer to the `struct device` embedded in a `struct rtc_device`.
> + /// `tm` must be a valid pointer to a `struct rtc_time`.
> + unsafe extern "C" fn set_time(
> + dev: *mut bindings::device,
> + tm: *mut bindings::rtc_time,
> + ) -> c_int {
> + // SAFETY: The caller ensures that `dev` is a valid pointer to a `struct device`.
> + let device_dev: &device::Device = unsafe { device::Device::from_raw(dev) };
> + // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
> + // `AsBusDevice` to get it.
> + let rtc_dev = unsafe { RtcDevice::<T>::from_device(device_dev) };
> + // SAFETY: The caller ensures that `tm` is valid and writable.
> + // `RtcTime` is `#[repr(transparent)]` over `bindings::rtc_time`, so we can safely cast.
> + let rtc_tm = unsafe { &mut *tm.cast::<RtcTime>() };
> +
> + match T::set_time(rtc_dev, rtc_tm) {
> + Ok(()) => 0,
> + Err(err) => err.to_errno(),
> + }
> + }
> +
> + /// # Safety
> + ///
> + /// `dev` must be a valid pointer to the `struct device` embedded in a `struct rtc_device`.
> + /// `alarm` must be a valid pointer to a `struct rtc_wkalrm`.
> + unsafe extern "C" fn read_alarm(
> + dev: *mut bindings::device,
> + alarm: *mut bindings::rtc_wkalrm,
> + ) -> c_int {
> + // SAFETY: The caller ensures that `dev` is a valid pointer to a `struct device`.
> + let device_dev: &device::Device = unsafe { device::Device::from_raw(dev) };
> + // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
> + // `AsBusDevice` to get it.
> + let rtc_dev = unsafe { RtcDevice::<T>::from_device(device_dev) };
> + // SAFETY: The caller ensures that `alarm` is valid and writable.
> + // `RtcWkAlrm` is `#[repr(transparent)]` over `bindings::rtc_wkalrm`, so we can safely cast.
> + let rtc_alarm = unsafe { &mut *alarm.cast::<RtcWkAlrm>() };
> +
> + match T::read_alarm(rtc_dev, rtc_alarm) {
> + Ok(()) => 0,
> + Err(err) => err.to_errno(),
> + }
> + }
> +
> + /// # Safety
> + ///
> + /// `dev` must be a valid pointer to the `struct device` embedded in a `struct rtc_device`.
> + /// `alarm` must be a valid pointer to a `struct rtc_wkalrm`.
> + unsafe extern "C" fn set_alarm(
> + dev: *mut bindings::device,
> + alarm: *mut bindings::rtc_wkalrm,
> + ) -> c_int {
> + // SAFETY: The caller ensures that `dev` is a valid pointer to a `struct device`.
> + let device_dev: &device::Device = unsafe { device::Device::from_raw(dev) };
> + // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
> + // `AsBusDevice` to get it.
> + let rtc_dev = unsafe { RtcDevice::<T>::from_device(device_dev) };
> + // SAFETY: The caller ensures that `alarm` is valid and writable.
> + // `RtcWkAlrm` is `#[repr(transparent)]` over `bindings::rtc_wkalrm`, so we can safely cast.
> + let rtc_alarm = unsafe { &mut *alarm.cast::<RtcWkAlrm>() };
> +
> + match T::set_alarm(rtc_dev, rtc_alarm) {
> + Ok(()) => 0,
> + Err(err) => err.to_errno(),
> + }
> + }
> +
> + /// # Safety
> + ///
> + /// `dev` must be a valid pointer to the `struct device` embedded in a `struct rtc_device`.
> + unsafe extern "C" fn alarm_irq_enable(dev: *mut bindings::device, enabled: c_uint) -> c_int {
> + // SAFETY: The caller ensures that `dev` is a valid pointer to a `struct device`.
> + let device_dev: &device::Device = unsafe { device::Device::from_raw(dev) };
> + // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
> + // `AsBusDevice` to get it.
> + let rtc_dev = unsafe { RtcDevice::<T>::from_device(device_dev) };
> +
> + match T::alarm_irq_enable(rtc_dev, enabled) {
> + Ok(()) => 0,
> + Err(err) => err.to_errno(),
> + }
> + }
> +
> + /// # Safety
> + ///
> + /// `dev` must be a valid pointer to the `struct device` embedded in a `struct rtc_device`.
> + unsafe extern "C" fn ioctl(dev: *mut bindings::device, cmd: c_uint, arg: c_ulong) -> c_int {
> + // SAFETY: The caller ensures that `dev` is a valid pointer to a `struct device`.
> + let device_dev: &device::Device = unsafe { device::Device::from_raw(dev) };
> + // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
> + // `AsBusDevice` to get it.
> + let rtc_dev = unsafe { RtcDevice::<T>::from_device(device_dev) };
> +
> + match T::ioctl(rtc_dev, cmd, arg) {
> + Ok(ret) => ret,
> + Err(err) => err.to_errno(),
> + }
> + }
> +
> + /// # Safety
> + ///
> + /// `dev` must be a valid pointer to the `struct device` embedded in a `struct rtc_device`.
> + /// `seq` must be a valid pointer to a `struct seq_file`.
> + unsafe extern "C" fn proc(dev: *mut bindings::device, seq: *mut bindings::seq_file) -> c_int {
> + // SAFETY: The caller ensures that `dev` is a valid pointer to a `struct device`.
> + let device_dev: &device::Device = unsafe { device::Device::from_raw(dev) };
> + // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
> + // `AsBusDevice` to get it.
> + let rtc_dev = unsafe { RtcDevice::<T>::from_device(device_dev) };
> + // SAFETY: The caller ensures that `seq` is valid and writable.
> + let seq_file = unsafe { &mut *seq.cast::<SeqFile>() };
> +
> + match T::proc(rtc_dev, seq_file) {
> + Ok(()) => 0,
> + Err(err) => err.to_errno(),
> + }
> + }
> +
> + /// # Safety
> + ///
> + /// `dev` must be a valid pointer to the `struct device` embedded in a `struct rtc_device`.
> + /// `offset` must be a valid pointer to a `long`.
> + unsafe extern "C" fn read_offset(dev: *mut bindings::device, offset: *mut c_long) -> c_int {
> + // SAFETY: The caller ensures that `dev` is a valid pointer to a `struct device`.
> + let device_dev: &device::Device = unsafe { device::Device::from_raw(dev) };
> + // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
> + // `AsBusDevice` to get it.
> + let rtc_dev = unsafe { RtcDevice::<T>::from_device(device_dev) };
> + // SAFETY: The caller ensures that `offset` is valid and writable.
> + let mut offset_val: i64 = unsafe { *offset.cast() };
> +
> + match T::read_offset(rtc_dev, &mut offset_val) {
> + Ok(()) => {
> + // SAFETY: The caller ensures that `offset` is valid and writable.
> + unsafe { *offset.cast() = offset_val as c_long };
> + 0
> + }
> + Err(err) => err.to_errno(),
> + }
> + }
> +
> + /// # Safety
> + ///
> + /// `dev` must be a valid pointer to the `struct device` embedded in a `struct rtc_device`.
> + unsafe extern "C" fn set_offset(dev: *mut bindings::device, offset: c_long) -> c_int {
> + // SAFETY: The caller ensures that `dev` is a valid pointer to a `struct device`.
> + let device_dev: &device::Device = unsafe { device::Device::from_raw(dev) };
> + // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
> + // `AsBusDevice` to get it.
> + let rtc_dev = unsafe { RtcDevice::<T>::from_device(device_dev) };
> +
> + match T::set_offset(rtc_dev, offset as i64) {
> + Ok(()) => 0,
> + Err(err) => err.to_errno(),
> + }
> + }
> +
> + /// # Safety
> + ///
> + /// `dev` must be a valid pointer to the `struct device` embedded in a `struct rtc_device`.
> + /// `param` must be a valid pointer to a `struct rtc_param`.
> + unsafe extern "C" fn param_get(
> + dev: *mut bindings::device,
> + param: *mut bindings::rtc_param,
> + ) -> c_int {
> + // SAFETY: The caller ensures that `dev` is a valid pointer to a `struct device`.
> + let device_dev: &device::Device = unsafe { device::Device::from_raw(dev) };
> + // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
> + // `AsBusDevice` to get it.
> + let rtc_dev = unsafe { RtcDevice::<T>::from_device(device_dev) };
> + // SAFETY: The caller ensures that `param` is valid and writable.
> + // `RtcParam` is `#[repr(transparent)]` over `bindings::rtc_param`, so we can safely cast.
> + let rtc_param = unsafe { &mut *param.cast::<RtcParam>() };
> +
> + match T::param_get(rtc_dev, rtc_param) {
> + Ok(()) => 0,
> + Err(err) => err.to_errno(),
> + }
> + }
> +
> + /// # Safety
> + ///
> + /// `dev` must be a valid pointer to the `struct device` embedded in a `struct rtc_device`.
> + /// `param` must be a valid pointer to a `struct rtc_param`.
> + unsafe extern "C" fn param_set(
> + dev: *mut bindings::device,
> + param: *mut bindings::rtc_param,
> + ) -> c_int {
> + // SAFETY: The caller ensures that `dev` is a valid pointer to a `struct device`.
> + let device_dev: &device::Device = unsafe { device::Device::from_raw(dev) };
> + // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
> + // `AsBusDevice` to get it.
> + let rtc_dev = unsafe { RtcDevice::<T>::from_device(device_dev) };
> + // SAFETY: The caller ensures that `param` is valid and writable.
> + // `RtcParam` is `#[repr(transparent)]` over `bindings::rtc_param`, so we can safely cast.
> + let rtc_param = unsafe { &mut *param.cast::<RtcParam>() };
> +
> + match T::param_set(rtc_dev, rtc_param) {
> + Ok(()) => 0,
> + Err(err) => err.to_errno(),
> + }
> + }
> +}
> +
> +/// VTable structure wrapper for RTC operations.
> +/// Mirrors [`struct rtc_class_ops`](srctree/include/linux/rtc.h).
> +#[repr(transparent)]
> +pub struct RtcOpsVTable(bindings::rtc_class_ops);
> +
> +// SAFETY: RtcOpsVTable is Send. The vtable contains only function pointers,
> +// which are simple data types that can be safely moved across threads.
> +// The thread-safety of calling these functions is handled by the kernel's
> +// locking mechanisms.
> +unsafe impl Send for RtcOpsVTable {}
> +
> +// SAFETY: RtcOpsVTable is Sync. The vtable is immutable after it is created,
> +// so it can be safely referenced and accessed concurrently by multiple threads
> +// e.g. to read the function pointers.
> +unsafe impl Sync for RtcOpsVTable {}
> +
> +impl RtcOpsVTable {
> + /// Returns a raw pointer to the underlying `rtc_class_ops` struct.
> + pub(crate) const fn as_raw(&self) -> *const bindings::rtc_class_ops {
> + &self.0
> + }
> +}
> +
> +/// Creates an RTC operations vtable for a type `T` that implements `RtcOps`.
> +///
> +/// This is used to bridge Rust trait implementations to the C `struct rtc_class_ops`
> +/// expected by the kernel.
> +pub const fn create_rtc_ops<T: RtcOps>() -> RtcOpsVTable {
> + let mut ops: bindings::rtc_class_ops = pin_init::zeroed();
> +
> + ops.read_time = if T::HAS_READ_TIME {
> + Some(Adapter::<T>::read_time)
> + } else {
> + None
> + };
> + ops.set_time = if T::HAS_SET_TIME {
> + Some(Adapter::<T>::set_time)
> + } else {
> + None
> + };
> + ops.read_alarm = if T::HAS_READ_ALARM {
> + Some(Adapter::<T>::read_alarm)
> + } else {
> + None
> + };
> + ops.set_alarm = if T::HAS_SET_ALARM {
> + Some(Adapter::<T>::set_alarm)
> + } else {
> + None
> + };
> + ops.alarm_irq_enable = if T::HAS_ALARM_IRQ_ENABLE {
> + Some(Adapter::<T>::alarm_irq_enable)
> + } else {
> + None
> + };
> + ops.ioctl = if T::HAS_IOCTL {
> + Some(Adapter::<T>::ioctl)
> + } else {
> + None
> + };
> + ops.proc_ = if T::HAS_PROC {
> + Some(Adapter::<T>::proc)
> + } else {
> + None
> + };
> + ops.read_offset = if T::HAS_READ_OFFSET {
> + Some(Adapter::<T>::read_offset)
> + } else {
> + None
> + };
> + ops.set_offset = if T::HAS_SET_OFFSET {
> + Some(Adapter::<T>::set_offset)
> + } else {
> + None
> + };
> + ops.param_get = if T::HAS_PARAM_GET {
> + Some(Adapter::<T>::param_get)
> + } else {
> + None
> + };
> + ops.param_set = if T::HAS_PARAM_SET {
> + Some(Adapter::<T>::param_set)
> + } else {
> + None
> + };
> +
> + RtcOpsVTable(ops)
> +}
> +
> +/// Declares a kernel module that exposes a single RTC AMBA driver.
> +///
> +/// # Examples
> +///
> +///```ignore
> +/// kernel::module_rtc_amba_driver! {
> +/// type: MyDriver,
> +/// name: "Module name",
> +/// authors: ["Author name"],
> +/// description: "Description",
> +/// license: "GPL v2",
> +/// }
> +///```
> +#[macro_export]
> +macro_rules! module_rtc_amba_driver {
> + ($($user_args:tt)*) => {
> + $crate::module_amba_driver! {
> + $($user_args)*
> + imports_ns: ["RTC"],
> + }
> + };
> +}
This does not make lot of sense. Usually these macros are to make life
easier for most common cases. Amba RTC is not common case. I looked at
little bit and I do not even know what is common. Platform, I2c or SPI are
most used but for now just do not have macro at all unless someone asks
for it.
Looking forward for this series :)
Argillander
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH v2 1/5] rtc: migrate driver data to RTC device
2026-01-08 14:01 ` Alexandre Belloni
2026-01-08 14:06 ` Danilo Krummrich
@ 2026-01-14 23:23 ` Ke Sun
2026-01-14 23:48 ` Danilo Krummrich
1 sibling, 1 reply; 32+ messages in thread
From: Ke Sun @ 2026-01-14 23:23 UTC (permalink / raw)
To: Alexandre Belloni, Danilo Krummrich, Greg KH
Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
linux-rtc, rust-for-linux, Alvin Sun
On 1/8/26 22:01, Alexandre Belloni wrote:
> On 08/01/2026 14:52:08+0100, Danilo Krummrich wrote:
>> On Thu Jan 8, 2026 at 2:45 PM CET, Ke Sun wrote:
>>> On 1/8/26 19:12, Danilo Krummrich wrote:
>>>> On Wed Jan 7, 2026 at 3:37 PM CET, Ke Sun wrote:
>>>>> diff --git a/drivers/rtc/dev.c b/drivers/rtc/dev.c
>>>>> index baf1a8ca8b2b1..0f62ba9342e3e 100644
>>>>> --- a/drivers/rtc/dev.c
>>>>> +++ b/drivers/rtc/dev.c
>>>>> @@ -410,7 +410,7 @@ static long rtc_dev_ioctl(struct file *file,
>>>>> }
>>>>> default:
>>>>> if (rtc->ops->param_get)
>>>>> - err = rtc->ops->param_get(rtc->dev.parent, ¶m);
>>>>> + err = rtc->ops->param_get(&rtc->dev, ¶m);
>>>> It would make more sense to just pass a struct rtc_device than the embedded
>>>> struct device in the RTC callbacks.
>>> I considered passing struct rtc_device directly, but chose &rtc->dev
>>> to minimize changes to existing drivers, since most callbacks use
>>> dev_get_drvdata() on the device parameter.
>> No, you should not expose the embedded base device. For accessing the private
>> data you should add helpers like rtc_get_drvdata(). This is what other
>> subsystems do as well, e.g. [1].
>>
>> [1] https://elixir.bootlin.com/linux/v6.18.3/source/include/linux/i2c.h#L371
> This is not a correct example as i2c is a bus, just like amba is...
> Actually, I don't think the rework is necessary at all or this would
> mean we need to rewor most of our existing subsystems.
RTC ops callbacks receive struct device * as the first parameter.
Traditionally this is rtc->dev.parent
(the physical bus device), but Rust drivers store driver data on
rtc->dev itself, so callbacks need &rtc->dev
to access it. We considered switching all callbacks to use rtc->dev
directly, but that would require modifying
182 RTC drivers and extensive testing/validation work. Instead, we
propose an alternative approach:
- Added RTC_OPS_USE_RTC_DEV flag (currently stored in rtc->features bitmap)
- Created rtc_ops_dev() helper that returns &rtc->dev if flag is set,
otherwise
rtc->dev.parent. Default behavior (returning rtc->dev.parent) maintains
backward compatibility
- Updated all rtc->ops->callback call sites to use rtc_ops_dev(rtc)
Best regard,
Ke Sun
>
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH v2 1/5] rtc: migrate driver data to RTC device
2026-01-14 23:23 ` Ke Sun
@ 2026-01-14 23:48 ` Danilo Krummrich
0 siblings, 0 replies; 32+ messages in thread
From: Danilo Krummrich @ 2026-01-14 23:48 UTC (permalink / raw)
To: Ke Sun
Cc: Alexandre Belloni, Greg KH, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, linux-rtc, rust-for-linux, Alvin Sun
On Thu Jan 15, 2026 at 12:23 AM CET, Ke Sun wrote:
> RTC ops callbacks receive struct device * as the first parameter.
> Traditionally this is rtc->dev.parent (the physical bus device), but Rust
> drivers store driver data on rtc->dev itself,
This is not only about Rust. Class device private data should be stored in the
driver_data field of the struct device embedded in the class device in general.
> so callbacks need &rtc->dev to access it.
Class device callbacks should just carry the class device itself, rather than
the embedded struct device.
> We considered switching all callbacks to use rtc->dev directly, but that would
> require modifying 182 RTC drivers and extensive testing/validation work.
I don't know if it's that bad, the change would be trivial. You just need to
repeat it pretty often. :) Tools like Coccinelle [1] can help a lot with such
refactorings.
> Instead, we propose an alternative approach:
>
> - Added RTC_OPS_USE_RTC_DEV flag (currently stored in rtc->features bitmap)
> - Created rtc_ops_dev() helper that returns &rtc->dev if flag is set,
> otherwise
> rtc->dev.parent. Default behavior (returning rtc->dev.parent) maintains
> backward compatibility
> - Updated all rtc->ops->callback call sites to use rtc_ops_dev(rtc)
Not sure if that intermediate step is needed, but it doesn't seem unreasonable
to me.
While eventually this is up to the RTC subsystem maintainer, from a driver-core
perspective this refactoring is encouraged:
Drivers should generally distinguish between stuff that is stored in the private
data of the bus device and private data of the class device, e.g. since they
have independent lifecycles and not all data might be relevant in all scopes.
Forcing drivers to also store the class device private data in the parent bus
device private data can be considered an anti-pattern.
[1] https://docs.kernel.org/dev-tools/coccinelle.html
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH v2 1/5] rtc: migrate driver data to RTC device
2026-01-08 14:06 ` Danilo Krummrich
@ 2026-02-20 23:19 ` Alexandre Belloni
0 siblings, 0 replies; 32+ messages in thread
From: Alexandre Belloni @ 2026-02-20 23:19 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Ke Sun, Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
linux-rtc, rust-for-linux, Alvin Sun
On 08/01/2026 15:06:46+0100, Danilo Krummrich wrote:
> On Thu Jan 8, 2026 at 3:01 PM CET, Alexandre Belloni wrote:
> > On 08/01/2026 14:52:08+0100, Danilo Krummrich wrote:
> >> On Thu Jan 8, 2026 at 2:45 PM CET, Ke Sun wrote:
> >> >
> >> > On 1/8/26 19:12, Danilo Krummrich wrote:
> >> >> On Wed Jan 7, 2026 at 3:37 PM CET, Ke Sun wrote:
> >> >>> diff --git a/drivers/rtc/dev.c b/drivers/rtc/dev.c
> >> >>> index baf1a8ca8b2b1..0f62ba9342e3e 100644
> >> >>> --- a/drivers/rtc/dev.c
> >> >>> +++ b/drivers/rtc/dev.c
> >> >>> @@ -410,7 +410,7 @@ static long rtc_dev_ioctl(struct file *file,
> >> >>> }
> >> >>> default:
> >> >>> if (rtc->ops->param_get)
> >> >>> - err = rtc->ops->param_get(rtc->dev.parent, ¶m);
> >> >>> + err = rtc->ops->param_get(&rtc->dev, ¶m);
> >> >> It would make more sense to just pass a struct rtc_device than the embedded
> >> >> struct device in the RTC callbacks.
> >> > I considered passing struct rtc_device directly, but chose &rtc->dev
> >> > to minimize changes to existing drivers, since most callbacks use
> >> > dev_get_drvdata() on the device parameter.
> >>
> >> No, you should not expose the embedded base device. For accessing the private
> >> data you should add helpers like rtc_get_drvdata(). This is what other
> >> subsystems do as well, e.g. [1].
> >>
> >> [1] https://elixir.bootlin.com/linux/v6.18.3/source/include/linux/i2c.h#L371
> >
> > This is not a correct example as i2c is a bus, just like amba is...
>
> Yes, struct i2c_client is indeed a bus device. However, the core struct device
> is what holds the device private data commonly in the same way, regardless of
> whether it is embedded in a bus or class device.
>
> If you look for a class device example, here's PWM [2] and input [3].
>
> [2] https://elixir.bootlin.com/linux/v6.18.3/source/include/linux/pwm.h#L382
> [3] https://elixir.bootlin.com/linux/v6.18.3/source/include/linux/input.h#L388
>
> > Actually, I don't think the rework is necessary at all or this would
> > mean we need to rewor most of our existing subsystems.
>
> That's not true, subsystems do not pass the parent device (i.e. the bus device)
> through their class device callbacks exclusively.
Like explained on the other thread, while it would be conceptually
better to pass a struct rtc_device to the callbacks, it doesn't solve
your issue. Let me take a random input drivers as an example:
https://elixir.bootlin.com/linux/v6.18.3/source/drivers/input/keyboard/pinephone-keyboard.c#L373
This sets its own private data on the parent device, it needs it later
on in the interrupt handler
https://elixir.bootlin.com/linux/v6.18.3/source/drivers/input/joystick/as5011.c#L313
It needs it later on in the remove callback
https://elixir.bootlin.com/linux/v6.18.3/source/drivers/input/misc/da7280.c#L1197
Needed later on for suspend/resume
So the input subsystem is not different from RTC
For PWM:
https://elixir.bootlin.com/linux/v6.18.6/source/drivers/pwm/pwm-pca9685.c#L450
Needed for suspend/resume and .remove()
https://elixir.bootlin.com/linux/v6.18.6/source/drivers/pwm/pwm-rockchip.c#L348
Needed for .remove()
Any other subsystem is going to have similar examples. I don't think
there is a pressing need to rewrite the rtc_class_ops callbacks.
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2026-02-20 23:19 UTC | newest]
Thread overview: 32+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-07 14:37 [RFC PATCH v2 0/5] rust: Add RTC driver support Ke Sun
2026-01-07 14:37 ` [RFC PATCH v2 1/5] rtc: migrate driver data to RTC device Ke Sun
2026-01-07 14:41 ` Ke Sun
2026-01-07 16:12 ` Greg KH
2026-01-07 23:18 ` Ke Sun
2026-01-08 0:24 ` Ke Sun
2026-01-08 11:06 ` Danilo Krummrich
2026-01-08 5:46 ` Greg KH
2026-01-08 9:02 ` Ke Sun
2026-01-08 9:10 ` Greg KH
2026-01-08 11:12 ` Danilo Krummrich
2026-01-08 13:45 ` Ke Sun
2026-01-08 13:52 ` Danilo Krummrich
2026-01-08 14:01 ` Ke Sun
2026-01-08 14:01 ` Alexandre Belloni
2026-01-08 14:06 ` Danilo Krummrich
2026-02-20 23:19 ` Alexandre Belloni
2026-01-14 23:23 ` Ke Sun
2026-01-14 23:48 ` Danilo Krummrich
2026-01-07 14:37 ` [RFC PATCH v2 2/5] rust: add AMBA bus driver support Ke Sun
2026-01-08 11:29 ` Danilo Krummrich
2026-01-07 14:37 ` [RFC PATCH v2 3/5] rust: add device wakeup capability support Ke Sun
2026-01-07 14:57 ` Greg KH
2026-01-07 23:35 ` Ke Sun
2026-01-07 14:37 ` [RFC PATCH v2 4/5] rust: add RTC core abstractions and data structures Ke Sun
2026-01-08 11:50 ` Danilo Krummrich
2026-01-08 13:17 ` Ke Sun
2026-01-08 13:49 ` Miguel Ojeda
2026-01-08 13:56 ` Ke Sun
2026-01-08 23:31 ` Kari Argillander
2026-01-07 14:37 ` [RFC PATCH v2 5/5] rust: add PL031 RTC driver Ke Sun
2026-01-08 11:57 ` Danilo Krummrich
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox