* Re: [RFC PATCH v2 1/5] rtc: migrate driver data to RTC device
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
In-Reply-To: <66e517b1-bc83-4c73-b2ea-73a31757ab44@gmail.com>
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
* Re: [RFC PATCH v2 1/5] rtc: migrate driver data to RTC device
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
In-Reply-To: <20260107143738.3021892-2-sunke@kylinos.cn>
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
* Re: [RFC PATCH v2 2/5] rust: add AMBA bus driver support
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
In-Reply-To: <20260107143738.3021892-3-sunke@kylinos.cn>
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
* Re: [RFC PATCH v2 4/5] rust: add RTC core abstractions and data structures
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
In-Reply-To: <20260107143738.3021892-5-sunke@kylinos.cn>
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
* Re: [RFC PATCH v2 5/5] rust: add PL031 RTC driver
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
In-Reply-To: <20260107143738.3021892-6-sunke@kylinos.cn>
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
* Re: [PATCH v2 00/17] tee: Use bus callbacks instead of driver callbacks
From: Jarkko Sakkinen @ 2026-01-08 12:38 UTC (permalink / raw)
To: Jens Wiklander
Cc: Alexandre Belloni, Uwe Kleine-König, Jonathan Corbet,
Sumit Garg, Olivia Mackall, Herbert Xu, Clément Léger,
Ard Biesheuvel, Maxime Coquelin, Alexandre Torgue, Sumit Garg,
Ilias Apalodimas, Jan Kiszka, Sudeep Holla, Christophe JAILLET,
Rafał Miłecki, Michael Chan, Pavan Chebbi,
James Bottomley, Mimi Zohar, David Howells, Paul Moore,
James Morris, Serge E. Hallyn, Peter Huewe, op-tee, linux-kernel,
linux-doc, linux-crypto, linux-rtc, linux-efi, linux-stm32,
linux-arm-kernel, Cristian Marussi, arm-scmi, linux-mips, netdev,
linux-integrity, keyrings, linux-security-module, Jason Gunthorpe
In-Reply-To: <CAHUa44HqRbCJTXsrTCm0G5iwtkQtq+Si=yOspCjpAn-N2uVSVg@mail.gmail.com>
On Mon, Jan 05, 2026 at 10:16:09AM +0100, Jens Wiklander wrote:
> Hi,
>
> On Thu, Dec 18, 2025 at 5:29 PM Jens Wiklander
> <jens.wiklander@linaro.org> wrote:
> >
> > On Thu, Dec 18, 2025 at 2:53 PM Alexandre Belloni
> > <alexandre.belloni@bootlin.com> wrote:
> > >
> > > On 18/12/2025 08:21:27+0100, Jens Wiklander wrote:
> > > > Hi,
> > > >
> > > > On Mon, Dec 15, 2025 at 3:17 PM Uwe Kleine-König
> > > > <u.kleine-koenig@baylibre.com> wrote:
> > > > >
> > > > > Hello,
> > > > >
> > > > > the objective of this series is to make tee driver stop using callbacks
> > > > > in struct device_driver. These were superseded by bus methods in 2006
> > > > > (commit 594c8281f905 ("[PATCH] Add bus_type probe, remove, shutdown
> > > > > methods.")) but nobody cared to convert all subsystems accordingly.
> > > > >
> > > > > Here the tee drivers are converted. The first commit is somewhat
> > > > > unrelated, but simplifies the conversion (and the drivers). It
> > > > > introduces driver registration helpers that care about setting the bus
> > > > > and owner. (The latter is missing in all drivers, so by using these
> > > > > helpers the drivers become more correct.)
> > > > >
> > > > > v1 of this series is available at
> > > > > https://lore.kernel.org/all/cover.1765472125.git.u.kleine-koenig@baylibre.com
> > > > >
> > > > > Changes since v1:
> > > > >
> > > > > - rebase to v6.19-rc1 (no conflicts)
> > > > > - add tags received so far
> > > > > - fix whitespace issues pointed out by Sumit Garg
> > > > > - fix shutdown callback to shutdown and not remove
> > > > >
> > > > > As already noted in v1's cover letter, this series should go in during a
> > > > > single merge window as there are runtime warnings when the series is
> > > > > only applied partially. Sumit Garg suggested to apply the whole series
> > > > > via Jens Wiklander's tree.
> > > > > If this is done the dependencies in this series are honored, in case the
> > > > > plan changes: Patches #4 - #17 depend on the first two.
> > > > >
> > > > > Note this series is only build tested.
> > > > >
> > > > > Uwe Kleine-König (17):
> > > > > tee: Add some helpers to reduce boilerplate for tee client drivers
> > > > > tee: Add probe, remove and shutdown bus callbacks to tee_client_driver
> > > > > tee: Adapt documentation to cover recent additions
> > > > > hwrng: optee - Make use of module_tee_client_driver()
> > > > > hwrng: optee - Make use of tee bus methods
> > > > > rtc: optee: Migrate to use tee specific driver registration function
> > > > > rtc: optee: Make use of tee bus methods
> > > > > efi: stmm: Make use of module_tee_client_driver()
> > > > > efi: stmm: Make use of tee bus methods
> > > > > firmware: arm_scmi: optee: Make use of module_tee_client_driver()
> > > > > firmware: arm_scmi: Make use of tee bus methods
> > > > > firmware: tee_bnxt: Make use of module_tee_client_driver()
> > > > > firmware: tee_bnxt: Make use of tee bus methods
> > > > > KEYS: trusted: Migrate to use tee specific driver registration
> > > > > function
> > > > > KEYS: trusted: Make use of tee bus methods
> > > > > tpm/tpm_ftpm_tee: Make use of tee specific driver registration
> > > > > tpm/tpm_ftpm_tee: Make use of tee bus methods
> > > > >
> > > > > Documentation/driver-api/tee.rst | 18 +----
> > > > > drivers/char/hw_random/optee-rng.c | 26 ++----
> > > > > drivers/char/tpm/tpm_ftpm_tee.c | 31 +++++---
> > > > > drivers/firmware/arm_scmi/transports/optee.c | 32 +++-----
> > > > > drivers/firmware/broadcom/tee_bnxt_fw.c | 30 ++-----
> > > > > drivers/firmware/efi/stmm/tee_stmm_efi.c | 25 ++----
> > > > > drivers/rtc/rtc-optee.c | 27 ++-----
> > > > > drivers/tee/tee_core.c | 84 ++++++++++++++++++++
> > > > > include/linux/tee_drv.h | 12 +++
> > > > > security/keys/trusted-keys/trusted_tee.c | 17 ++--
> > > > > 10 files changed, 164 insertions(+), 138 deletions(-)
> > > > >
> > > > > base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
> > > > > --
> > > > > 2.47.3
> > > > >
> > > >
> > > > Thank you for the nice cleanup, Uwe.
> > > >
> > > > I've applied patch 1-3 to the branch tee_bus_callback_for_6.20 in my
> > > > tree at https://git.kernel.org/pub/scm/linux/kernel/git/jenswi/linux-tee.git/
> > > >
> > > > The branch is based on v6.19-rc1, and I'll try to keep it stable for
> > > > others to depend on, if needed. Let's see if we can agree on taking
> > > > the remaining patches via that branch.
> > >
> > > 6 and 7 can go through your branch.
> >
> > Good, I've added them to my branch now.
>
> This entire patch set should go in during a single merge window. I
> will not send any pull request until I'm sure all patches will be
> merged.
>
> So far (if I'm not mistaken), only the patches I've already added to
> next have appeared next. I can take the rest of the patches, too, but
> I need OK for the following:
>
> Jarkko, you seem happy with the following patches
> - KEYS: trusted: Migrate to use tee specific driver registration function
> - KEYS: trusted: Make use of tee bus methods
> - tpm/tpm_ftpm_tee: Make use of tee specific driver registration
> - tpm/tpm_ftpm_tee: Make use of tee bus methods
> OK if I take them via my tree, or would you rather take them yourself?
I don't mind.
>
> Herbert, you seem happy with the following patches
> - hwrng: optee - Make use of module_tee_client_driver()
> - hwrng: optee - Make use of tee bus methods
> OK if I take them via my tree, or would you rather take them yourself?
>
> Sudeep, you seem happy with the following patches
> - firmware: arm_scmi: optee: Make use of module_tee_client_driver()
> - firmware: arm_scmi: Make use of tee bus methods
> OK if I take them via my tree, or would you rather take them yourself?
>
> Michael, Pavan, are you OK with the following patches
> - firmware: tee_bnxt: Make use of module_tee_client_driver()
> - firmware: tee_bnxt: Make use of tee bus methods
> OK if I take them via my tree, or would you rather take them yourself?
>
> Thanks,
> Jens
BR, Jarkko
^ permalink raw reply
* [PATCH v2 0/5] rtc: zynqmp: fixes for read and set offset
From: Tomas Melin @ 2026-01-08 12:51 UTC (permalink / raw)
To: Alexandre Belloni, Michal Simek
Cc: linux-rtc, linux-arm-kernel, linux-kernel, Tomas Melin
Add improvements for read and set offset functions.
The basic functionality is still the same, but offset correction values
are now updated to match with expected.
The RTC calibration value operates with full ticks,
and fractional ticks which are a 1/16 of a full tick.
The 16 lowest bits in the calibration registers are for the full ticks
and value matches the external oscillator in Hz. Through that,
the maximum and minimum offset values can be calculated dynamically,
as they depend on the input frequency used.
For docs on the calibration register, see
https://docs.amd.com/r/en-US/ug1087-zynq-ultrascale-registers/CALIB_READ-RTC-Register
Due to rounding errors (different number of fract ticks),
offset readback will differ slightly depending on
if the offset is negative or positive.
For example
$ echo 34335 > offset
$ cat offset
34335
$ echo -34335 > offset
$ cat offset
-34326
Signed-off-by: Tomas Melin <tomas.melin@vaisala.com>
---
Changes in v2:
- Add commit introducing check for calibration value overflow
- Update comments
- Align data types across set and read
- Rename fract_tick as fract_data conforming to data sheet
- Further improve on set offset calculation logic
- Link to v1: https://lore.kernel.org/r/20251201-zynqmp-rtc-updates-v1-0-33875c1e385b@vaisala.com
---
Tomas Melin (5):
rtc: zynqmp: correct frequency value
rtc: zynqmp: check calibration max value
rtc: zynqmp: rework read_offset
rtc: zynqmp: rework set_offset
rtc: zynqmp: use dynamic max and min offset ranges
drivers/rtc/rtc-zynqmp.c | 74 ++++++++++++++++++++++++++----------------------
1 file changed, 40 insertions(+), 34 deletions(-)
---
base-commit: cd635e33b0113287c94021be53d2a7c61a1614e9
change-id: 20251201-zynqmp-rtc-updates-d260364cc01b
Best regards,
--
Tomas Melin <tomas.melin@vaisala.com>
^ permalink raw reply
* [PATCH v2 2/5] rtc: zynqmp: check calibration max value
From: Tomas Melin @ 2026-01-08 12:51 UTC (permalink / raw)
To: Alexandre Belloni, Michal Simek
Cc: linux-rtc, linux-arm-kernel, linux-kernel, Tomas Melin
In-Reply-To: <20260108-zynqmp-rtc-updates-v2-0-864c161fa83d@vaisala.com>
Enable check to not overflow the calibration
max value.
Signed-off-by: Tomas Melin <tomas.melin@vaisala.com>
---
drivers/rtc/rtc-zynqmp.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/rtc/rtc-zynqmp.c b/drivers/rtc/rtc-zynqmp.c
index 856bc1678e7d31144f320ae9f75fc58c742a2a64..caacce3725e2ef3803ea42d40e77ceaeb7d7b914 100644
--- a/drivers/rtc/rtc-zynqmp.c
+++ b/drivers/rtc/rtc-zynqmp.c
@@ -349,6 +349,11 @@ static int xlnx_rtc_probe(struct platform_device *pdev)
xrtcdev->freq--;
}
+ if (xrtcdev->freq > RTC_TICK_MASK) {
+ dev_err(&pdev->dev, "Invalid RTC calibration value\n");
+ return -EINVAL;
+ }
+
ret = readl(xrtcdev->reg_base + RTC_CALIB_RD);
if (!ret)
writel(xrtcdev->freq, (xrtcdev->reg_base + RTC_CALIB_WR));
--
2.47.3
^ permalink raw reply related
* [PATCH v2 3/5] rtc: zynqmp: rework read_offset
From: Tomas Melin @ 2026-01-08 12:51 UTC (permalink / raw)
To: Alexandre Belloni, Michal Simek
Cc: linux-rtc, linux-arm-kernel, linux-kernel, Tomas Melin
In-Reply-To: <20260108-zynqmp-rtc-updates-v2-0-864c161fa83d@vaisala.com>
read_offset() was using static frequency for determining
the tick offset. It was also using remainder from do_div()
operation as tick_mult value which caused the offset to be
incorrect.
At the same time, rework function to improve readability.
It is worth noting, that due to rounding errors, the offset
readback will differ slightly for positive and negative
calibration values.
Signed-off-by: Tomas Melin <tomas.melin@vaisala.com>
---
drivers/rtc/rtc-zynqmp.c | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/drivers/rtc/rtc-zynqmp.c b/drivers/rtc/rtc-zynqmp.c
index caacce3725e2ef3803ea42d40e77ceaeb7d7b914..6740c3aed1897d4b50a02c4823a746d9c2ae2655 100644
--- a/drivers/rtc/rtc-zynqmp.c
+++ b/drivers/rtc/rtc-zynqmp.c
@@ -178,21 +178,28 @@ static void xlnx_init_rtc(struct xlnx_rtc_dev *xrtcdev)
static int xlnx_rtc_read_offset(struct device *dev, long *offset)
{
struct xlnx_rtc_dev *xrtcdev = dev_get_drvdata(dev);
- unsigned long long rtc_ppb = RTC_PPB;
- unsigned int tick_mult = do_div(rtc_ppb, xrtcdev->freq);
- unsigned int calibval;
+ unsigned int calibval, fract_data, fract_part;
+ int freq = xrtcdev->freq;
+ int max_tick, tick_mult;
long offset_val;
+ /* Tick to offset multiplier */
+ tick_mult = DIV_ROUND_CLOSEST(RTC_PPB, freq);
+
calibval = readl(xrtcdev->reg_base + RTC_CALIB_RD);
/* Offset with seconds ticks */
- offset_val = calibval & RTC_TICK_MASK;
- offset_val = offset_val - RTC_CALIB_DEF;
- offset_val = offset_val * tick_mult;
+ max_tick = calibval & RTC_TICK_MASK;
+ offset_val = max_tick - freq;
+ /* Convert to ppb */
+ offset_val *= tick_mult;
/* Offset with fractional ticks */
- if (calibval & RTC_FR_EN)
- offset_val += ((calibval & RTC_FR_MASK) >> RTC_FR_DATSHIFT)
- * (tick_mult / RTC_FR_MAX_TICKS);
+ if (calibval & RTC_FR_EN) {
+ fract_data = (calibval & RTC_FR_MASK) >> RTC_FR_DATSHIFT;
+ fract_part = DIV_ROUND_UP(tick_mult, RTC_FR_MAX_TICKS);
+ offset_val += (fract_part * fract_data);
+ }
+
*offset = offset_val;
return 0;
--
2.47.3
^ permalink raw reply related
* [PATCH v2 4/5] rtc: zynqmp: rework set_offset
From: Tomas Melin @ 2026-01-08 12:51 UTC (permalink / raw)
To: Alexandre Belloni, Michal Simek
Cc: linux-rtc, linux-arm-kernel, linux-kernel, Tomas Melin
In-Reply-To: <20260108-zynqmp-rtc-updates-v2-0-864c161fa83d@vaisala.com>
set_offset was using remainder of do_div as tick_mult which resulted in
wrong offset. Calibration value also assumed builtin calibration default.
Update fract_offset to correctly calculate the value for
negative offset and replace the for loop with division.
Signed-off-by: Tomas Melin <tomas.melin@vaisala.com>
---
drivers/rtc/rtc-zynqmp.c | 33 +++++++++++++--------------------
1 file changed, 13 insertions(+), 20 deletions(-)
diff --git a/drivers/rtc/rtc-zynqmp.c b/drivers/rtc/rtc-zynqmp.c
index 6740c3aed1897d4b50a02c4823a746d9c2ae2655..d15c256e7ae56058ddc38849af6424cd29b8965e 100644
--- a/drivers/rtc/rtc-zynqmp.c
+++ b/drivers/rtc/rtc-zynqmp.c
@@ -208,13 +208,13 @@ static int xlnx_rtc_read_offset(struct device *dev, long *offset)
static int xlnx_rtc_set_offset(struct device *dev, long offset)
{
struct xlnx_rtc_dev *xrtcdev = dev_get_drvdata(dev);
- unsigned long long rtc_ppb = RTC_PPB;
- unsigned int tick_mult = do_div(rtc_ppb, xrtcdev->freq);
- unsigned char fract_tick = 0;
+ int max_tick, tick_mult, fract_offset, fract_part;
unsigned int calibval;
- short int max_tick;
- int fract_offset;
+ int fract_data = 0;
+ int freq = xrtcdev->freq;
+ /* Tick to offset multiplier */
+ tick_mult = DIV_ROUND_CLOSEST(RTC_PPB, xrtcdev->freq);
if (offset < RTC_MIN_OFFSET || offset > RTC_MAX_OFFSET)
return -ERANGE;
@@ -223,29 +223,22 @@ static int xlnx_rtc_set_offset(struct device *dev, long offset)
/* Number fractional ticks for given offset */
if (fract_offset) {
- if (fract_offset < 0) {
- fract_offset = fract_offset + tick_mult;
+ fract_part = DIV_ROUND_UP(tick_mult, RTC_FR_MAX_TICKS);
+ fract_data = fract_offset / fract_part;
+ /* Subtract one from max_tick while adding fract_offset */
+ if (fract_offset < 0 && fract_data) {
max_tick--;
- }
- if (fract_offset > (tick_mult / RTC_FR_MAX_TICKS)) {
- for (fract_tick = 1; fract_tick < 16; fract_tick++) {
- if (fract_offset <=
- (fract_tick *
- (tick_mult / RTC_FR_MAX_TICKS)))
- break;
- }
+ fract_data += RTC_FR_MAX_TICKS;
}
}
/* Zynqmp RTC uses second and fractional tick
* counters for compensation
*/
- calibval = max_tick + RTC_CALIB_DEF;
-
- if (fract_tick)
- calibval |= RTC_FR_EN;
+ calibval = max_tick + freq;
- calibval |= (fract_tick << RTC_FR_DATSHIFT);
+ if (fract_data)
+ calibval |= (RTC_FR_EN | (fract_data << RTC_FR_DATSHIFT));
writel(calibval, (xrtcdev->reg_base + RTC_CALIB_WR));
--
2.47.3
^ permalink raw reply related
* [PATCH v2 5/5] rtc: zynqmp: use dynamic max and min offset ranges
From: Tomas Melin @ 2026-01-08 12:51 UTC (permalink / raw)
To: Alexandre Belloni, Michal Simek
Cc: linux-rtc, linux-arm-kernel, linux-kernel, Tomas Melin
In-Reply-To: <20260108-zynqmp-rtc-updates-v2-0-864c161fa83d@vaisala.com>
Maximum and minimum offsets in ppb that can be handled are dependent on
the rtc clock frequency and what can fit in the 16-bit register field.
Signed-off-by: Tomas Melin <tomas.melin@vaisala.com>
---
drivers/rtc/rtc-zynqmp.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/rtc/rtc-zynqmp.c b/drivers/rtc/rtc-zynqmp.c
index d15c256e7ae56058ddc38849af6424cd29b8965e..f508c61f4046e906d9569cc779a1360474a85fd2 100644
--- a/drivers/rtc/rtc-zynqmp.c
+++ b/drivers/rtc/rtc-zynqmp.c
@@ -44,8 +44,6 @@
#define RTC_FR_MASK 0xF0000
#define RTC_FR_MAX_TICKS 16
#define RTC_PPB 1000000000LL
-#define RTC_MIN_OFFSET -32768000
-#define RTC_MAX_OFFSET 32767000
struct xlnx_rtc_dev {
struct rtc_device *rtc;
@@ -215,12 +213,12 @@ static int xlnx_rtc_set_offset(struct device *dev, long offset)
/* Tick to offset multiplier */
tick_mult = DIV_ROUND_CLOSEST(RTC_PPB, xrtcdev->freq);
- if (offset < RTC_MIN_OFFSET || offset > RTC_MAX_OFFSET)
- return -ERANGE;
-
/* Number ticks for given offset */
max_tick = div_s64_rem(offset, tick_mult, &fract_offset);
+ if (freq + max_tick > RTC_TICK_MASK || (freq + max_tick < 1))
+ return -ERANGE;
+
/* Number fractional ticks for given offset */
if (fract_offset) {
fract_part = DIV_ROUND_UP(tick_mult, RTC_FR_MAX_TICKS);
--
2.47.3
^ permalink raw reply related
* [PATCH v2 1/5] rtc: zynqmp: correct frequency value
From: Tomas Melin @ 2026-01-08 12:51 UTC (permalink / raw)
To: Alexandre Belloni, Michal Simek
Cc: linux-rtc, linux-arm-kernel, linux-kernel, Tomas Melin
In-Reply-To: <20260108-zynqmp-rtc-updates-v2-0-864c161fa83d@vaisala.com>
Fix calibration value in case a clock reference is provided.
The actual calibration value written into register is
frequency - 1.
Signed-off-by: Tomas Melin <tomas.melin@vaisala.com>
---
drivers/rtc/rtc-zynqmp.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/rtc/rtc-zynqmp.c b/drivers/rtc/rtc-zynqmp.c
index 3baa2b481d9f2008750046005283b98a0d546c5c..856bc1678e7d31144f320ae9f75fc58c742a2a64 100644
--- a/drivers/rtc/rtc-zynqmp.c
+++ b/drivers/rtc/rtc-zynqmp.c
@@ -345,7 +345,10 @@ static int xlnx_rtc_probe(struct platform_device *pdev)
&xrtcdev->freq);
if (ret)
xrtcdev->freq = RTC_CALIB_DEF;
+ } else {
+ xrtcdev->freq--;
}
+
ret = readl(xrtcdev->reg_base + RTC_CALIB_RD);
if (!ret)
writel(xrtcdev->freq, (xrtcdev->reg_base + RTC_CALIB_WR));
--
2.47.3
^ permalink raw reply related
* Re: [RFC PATCH v2 4/5] rust: add RTC core abstractions and data structures
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
In-Reply-To: <DFJ6P0ITWD1O.2PAYKPU63UFFC@kernel.org>
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
* Re: [RFC PATCH v2 1/5] rtc: migrate driver data to RTC device
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
In-Reply-To: <DFJ5VOQOFLJO.1YI2NXC3B8P7L@kernel.org>
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
* Re: [RFC PATCH v2 4/5] rust: add RTC core abstractions and data structures
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
In-Reply-To: <b69749e0-e66d-425f-9d95-2d1bd4104e19@kylinos.cn>
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
* Re: [RFC PATCH v2 1/5] rtc: migrate driver data to RTC device
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
In-Reply-To: <c834ef20-2d4b-46aa-94ed-310c077a4495@kylinos.cn>
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
* Re: [RFC PATCH v2 4/5] rust: add RTC core abstractions and data structures
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
In-Reply-To: <CANiq72nbBKpa_n8i1pwoC7yiurDYJQtY1efRqkvtXMZ1sXSfJg@mail.gmail.com>
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
* Re: [RFC PATCH v2 1/5] rtc: migrate driver data to RTC device
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
In-Reply-To: <DFJ99UZAU51H.JP1VEERVR81W@kernel.org>
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
* Re: [RFC PATCH v2 1/5] rtc: migrate driver data to RTC device
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
In-Reply-To: <DFJ99UZAU51H.JP1VEERVR81W@kernel.org>
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
* Re: [RFC PATCH v2 1/5] rtc: migrate driver data to RTC device
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
In-Reply-To: <202601081401239bbfff9d@mail.local>
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
* Re: [PATCH RESEND v6 00/17] Support ROHM BD72720 PMIC
From: Lee Jones @ 2026-01-08 17:27 UTC (permalink / raw)
To: Matti Vaittinen
Cc: Matti Vaittinen, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Sebastian Reichel, Liam Girdwood, Mark Brown,
Michael Turquette, Stephen Boyd, Linus Walleij,
Bartosz Golaszewski, Alexandre Belloni, linux-leds, devicetree,
linux-kernel, linux-pm, linux-clk, linux-gpio, linux-rtc,
Andreas Kemnade
In-Reply-To: <cover.1765804226.git.mazziesaccount@gmail.com>
On Mon, 15 Dec 2025, Matti Vaittinen wrote:
> Resending the v6
>
> Series is same as v6 _except_ being rebased on v6.19-rc1 - and adding rb
> tags which were replied to v6.
>
> The ROHM BD72720 is a new power management IC for portable, battery
> powered devices. It integrates 10 BUCKs and 11 LDOs, RTC, charger, LEDs,
> GPIOs and a clock gate. To me the BD72720 seems like a successor to the
> BD71828 and BD71815 PMICs.
>
> This series depends on
> 5bff79dad20a ("power: supply: Add bd718(15/28/78) charger driver")
> which is in power-supply tree, for-next. Thus, the series is based on
> it.
>
> The testing since v4 has suffered some hardware-issues after I
> accidentally enabled charging while the PMIC's battery pin was connected
> to the I/O domain. Some heat was generated, not terribly lot smoke
> though...
>
> After the incident I've had occasional I2C failures. I, however, suspect
> the root cause is HW damage in I/O lines.
>
> Revision history:
> v6 resend:
> - Rebased on v6.19-rc1 and collected rb-tags from v6.
>
> v5 => v6:
> - MFD fixes as suggested by Lee
> - Styling mostly
> - New patch to Fix comment style for MFD driver
> More accurate changelog in individual patches
>
> v4 => v5:
> - dt-binding fixes as discussed in v4 reviews.
> - Drop rohm,vdr-battery.yaml and add vdr properties to battery.yaml
> - Drop 'rohm,' -vendor-prefix from vdr properties
> - Link to v4:
> https://lore.kernel.org/all/cover.1763022807.git.mazziesaccount@gmail.com/
> More accurate changelog in individual patches
>
> v3 => v4:
> - dt-binding fixes to the BD72720 MFD example and regulator bindings
> More accurate changelog in individual patches
>
> v2 => v3:
> - rebased to power-supply/for-next as dependencies are merged to there
> - plenty of dt-binding changes as suggested by reviewers
> - add new patch to better document existing 'trickle-charging' property
> More accurate changelog in individual patches
>
> RFCv1 => v2:
> - Drop RFC status
> - Use stacked regmaps to hide secondary map from the sub-drivers
> - Quite a few styling fixes and improvements as suggested by
> reviewers. More accurate changelog in individual patches.
> - Link to v1:
> https://lore.kernel.org/all/cover.1759824376.git.mazziesaccount@gmail.com/
>
> ---
>
> Matti Vaittinen (17):
> dt-bindings: regulator: ROHM BD72720
> dt-bindings: battery: Clarify trickle-charge
> dt-bindings: battery: Add trickle-charge upper limit
> dt-bindings: battery: Voltage drop properties
> dt-bindings: mfd: ROHM BD72720
> dt-bindings: leds: bd72720: Add BD72720
> mfd: rohm-bd71828: Use regmap_reg_range()
> mfd: rohm-bd71828: Use standard file header format
> mfd: rohm-bd71828: Support ROHM BD72720
> regulator: bd71828: rename IC specific entities
> regulator: bd71828: Support ROHM BD72720
> gpio: Support ROHM BD72720 gpios
> clk: clk-bd718x7: Support BD72720 clk gate
> rtc: bd70528: Support BD72720 rtc
> power: supply: bd71828: Support wider register addresses
> power: supply: bd71828-power: Support ROHM BD72720
> MAINTAINERS: Add ROHM BD72720 PMIC
>
> .../bindings/leds/rohm,bd71828-leds.yaml | 7 +-
> .../bindings/mfd/rohm,bd72720-pmic.yaml | 339 ++++++
> .../bindings/power/supply/battery.yaml | 33 +-
> .../regulator/rohm,bd72720-regulator.yaml | 148 +++
> MAINTAINERS | 2 +
> drivers/clk/Kconfig | 4 +-
> drivers/clk/clk-bd718x7.c | 10 +-
> drivers/gpio/Kconfig | 9 +
> drivers/gpio/Makefile | 1 +
> drivers/gpio/gpio-bd72720.c | 281 +++++
> drivers/mfd/Kconfig | 18 +-
> drivers/mfd/rohm-bd71828.c | 555 ++++++++-
> drivers/power/supply/bd71828-power.c | 160 ++-
> drivers/regulator/Kconfig | 8 +-
> drivers/regulator/bd71828-regulator.c | 1025 ++++++++++++++++-
> drivers/rtc/Kconfig | 3 +-
> drivers/rtc/rtc-bd70528.c | 21 +-
> include/linux/mfd/rohm-bd72720.h | 634 ++++++++++
> include/linux/mfd/rohm-generic.h | 1 +
> 19 files changed, 3127 insertions(+), 132 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/mfd/rohm,bd72720-pmic.yaml
> create mode 100644 Documentation/devicetree/bindings/regulator/rohm,bd72720-regulator.yaml
> create mode 100644 drivers/gpio/gpio-bd72720.c
> create mode 100644 include/linux/mfd/rohm-bd72720.h
The MFD parts LGTM.
What Acks are you waiting on? What's the merge strategy?
> --
> 2.52.0
>
--
Lee Jones [李琼斯]
^ permalink raw reply
* Re: (subset) [PATCH v6 0/7] mfd: macsmc: add rtc, hwmon and hid subdevices
From: Lee Jones @ 2026-01-08 17:33 UTC (permalink / raw)
To: Sven Peter, Janne Grunau, Alyssa Rosenzweig, Neal Gompa,
Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Alexandre Belloni, Jean Delvare, Guenter Roeck, Dmitry Torokhov,
Jonathan Corbet, James Calligeros
Cc: asahi, linux-arm-kernel, devicetree, linux-kernel, linux-rtc,
linux-hwmon, linux-input, linux-doc, Hector Martin
In-Reply-To: <20251215-macsmc-subdevs-v6-0-0518cb5f28ae@gmail.com>
On Mon, 15 Dec 2025 19:37:44 +1000, James Calligeros wrote:
> This series adds support for the remaining SMC subdevices. These are the
> RTC, hwmon, and HID devices. They are being submitted together as the RTC
> and hwmon drivers both require changes to the SMC DT schema.
>
> The RTC driver is responsible for getting and setting the system clock,
> and requires an NVMEM cell. This series replaces Sven's original RTC driver
> submission [1].
>
> [...]
Applied, thanks!
[2/7] mfd: macsmc: Wire up Apple SMC RTC subdevice
commit: a13cc4981449b9108921981a5e7c8824eaaa7604
[3/7] mfd: macsmc: Wire up Apple SMC hwmon subdevice
commit: fb90c90aec3a9b7212a243f383f73a01bc653672
[5/7] mfd: macsmc: Wire up Apple SMC input subdevice
commit: 3e9271dcb2dfdc4a98e9f6dc35c9be13276c3cfd
--
Lee Jones [李琼斯]
^ permalink raw reply
* [PATCH] rtc: pcf8563: use correct of_node for output clock
From: John Keeping @ 2026-01-08 18:47 UTC (permalink / raw)
To: linux-rtc
Cc: John Keeping, stable, Alexandre Belloni, Nobuhiro Iwamatsu,
linux-kernel
When switching to regmap, the i2c_client pointer was removed from struct
pcf8563 so this function switched to using the RTC device instead. But
the RTC device is a child of the original I2C device and does not have
an associated of_node.
Reference the correct device's of_node to ensure that the output clock
can be found when referenced by other devices and so that the override
clock name is read correctly.
Cc: stable@vger.kernel.org
Fixes: 00f1bb9b8486b ("rtc: pcf8563: Switch to regmap")
Signed-off-by: John Keeping <jkeeping@inmusicbrands.com>
---
drivers/rtc/rtc-pcf8563.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/rtc/rtc-pcf8563.c b/drivers/rtc/rtc-pcf8563.c
index 4e61011fb7a96..b281e9489df1d 100644
--- a/drivers/rtc/rtc-pcf8563.c
+++ b/drivers/rtc/rtc-pcf8563.c
@@ -424,7 +424,7 @@ static const struct clk_ops pcf8563_clkout_ops = {
static struct clk *pcf8563_clkout_register_clk(struct pcf8563 *pcf8563)
{
- struct device_node *node = pcf8563->rtc->dev.of_node;
+ struct device_node *node = pcf8563->rtc->dev.parent->of_node;
struct clk_init_data init;
struct clk *clk;
int ret;
--
2.52.0
^ permalink raw reply related
* [PATCH 00/27] clk: remove deprecated API divider_round_rate() and friends
From: Brian Masney @ 2026-01-08 21:16 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd
Cc: linux-clk, linux-kernel, Brian Masney, Chen Wang, Inochi Amaoto,
sophgo, Chen-Yu Tsai, Maxime Ripard, Jernej Skrabec,
Samuel Holland, linux-arm-kernel, linux-sunxi, Alexandre Belloni,
linux-rtc, Andreas Färber, Manivannan Sadhasivam,
linux-actions, Keguang Zhang, linux-mips, Taichi Sugaya,
Takao Orito, Jacky Huang, Shan-Chun Hung, Vladimir Zapolskiy,
Piotr Wojtaszczyk, Bjorn Andersson, linux-arm-msm, Orson Zhai,
Baolin Wang, Chunyan Zhang, Maxime Coquelin, Alexandre Torgue,
linux-stm32, Michal Simek, Rob Clark, Dmitry Baryshkov,
David Airlie, Simona Vetter, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, dri-devel, freedreno, Vinod Koul,
Neil Armstrong, linux-phy
Here's a series that gets rid of the deprecated APIs
divider_round_rate(), divider_round_rate_parent(), and
divider_ro_round_rate_parent() since these functions are just wrappers
for the determine_rate variant.
Note that when I converted some of these drivers from round_rate to
determine_rate, this was mistakenly converted to the following in some
cases:
req->rate = divider_round_rate(...)
This is invalid in the case when an error occurs since it can set the
rate to a negative value. So this series fixes those bugs and removes
the deprecated APIs all in one go.
Three of the patches ended up being a more complicated migration, and I
put them as the first three patches in this series (clk: sophgo:
cv18xx-ip), (clk: sunxi-ng), and (rtc: ac100). The remaining patches I
feel are all straight forward.
Merge strategy
==============
Only three of the patches are outside of drivers/clk (drm/msm, phy, and
rtc). For simplicity, I think it would be easiest if Stephen takes this
whole series through the clk tree. Subsystem maintainers please leave an
Acked-by for Stephen. Thanks!
Signed-off-by: Brian Masney <bmasney@redhat.com>
---
Brian Masney (27):
clk: sophgo: cv18xx-ip: convert from divider_round_rate() to divider_determine_rate()
clk: sunxi-ng: convert from divider_round_rate_parent() to divider_determine_rate()
rtc: ac100: convert from divider_round_rate() to divider_determine_rate()
clk: actions: owl-composite: convert from owl_divider_helper_round_rate() to divider_determine_rate()
clk: actions: owl-divider: convert from divider_round_rate() to divider_determine_rate()
clk: bm1880: convert from divider_ro_round_rate() to divider_ro_determine_rate()
clk: bm1880: convert from divider_round_rate() to divider_determine_rate()
clk: hisilicon: clkdivider-hi6220: convert from divider_round_rate() to divider_determine_rate()
clk: loongson1: convert from divider_round_rate() to divider_determine_rate()
clk: milbeaut: convert from divider_ro_round_rate() to divider_ro_determine_rate()
clk: milbeaut: convert from divider_round_rate() to divider_determine_rate()
clk: nuvoton: ma35d1-divider: convert from divider_round_rate() to divider_determine_rate()
clk: nxp: lpc32xx: convert from divider_round_rate() to divider_determine_rate()
clk: qcom: alpha-pll: convert from divider_round_rate() to divider_determine_rate()
clk: qcom: regmap-divider: convert from divider_ro_round_rate() to divider_ro_determine_rate()
clk: qcom: regmap-divider: convert from divider_round_rate() to divider_determine_rate()
clk: sophgo: sg2042-clkgen: convert from divider_round_rate() to divider_determine_rate()
clk: sprd: div: convert from divider_round_rate() to divider_determine_rate()
clk: stm32: stm32-core: convert from divider_ro_round_rate() to divider_ro_determine_rate()
clk: stm32: stm32-core: convert from divider_round_rate_parent() to divider_determine_rate()
clk: versaclock3: convert from divider_round_rate() to divider_determine_rate()
clk: x86: cgu: convert from divider_round_rate() to divider_determine_rate()
clk: zynqmp: divider: convert from divider_round_rate() to divider_determine_rate()
drm/msm/dsi_phy_14nm: convert from divider_round_rate() to divider_determine_rate()
phy: ti: phy-j721e-wiz: convert from divider_round_rate() to divider_determine_rate()
clk: divider: remove divider_ro_round_rate_parent()
clk: divider: remove divider_round_rate() and divider_round_rate_parent()
drivers/clk/actions/owl-composite.c | 11 +--
drivers/clk/actions/owl-divider.c | 17 +---
drivers/clk/actions/owl-divider.h | 5 -
drivers/clk/clk-bm1880.c | 13 +--
drivers/clk/clk-divider.c | 44 ---------
drivers/clk/clk-loongson1.c | 5 +-
drivers/clk/clk-milbeaut.c | 15 +--
drivers/clk/clk-versaclock3.c | 7 +-
drivers/clk/hisilicon/clkdivider-hi6220.c | 6 +-
drivers/clk/nuvoton/clk-ma35d1-divider.c | 7 +-
drivers/clk/nxp/clk-lpc32xx.c | 6 +-
drivers/clk/qcom/clk-alpha-pll.c | 21 ++--
drivers/clk/qcom/clk-regmap-divider.c | 16 +--
drivers/clk/sophgo/clk-cv18xx-ip.c | 154 ++++++++++++++++-------------
drivers/clk/sophgo/clk-sg2042-clkgen.c | 15 +--
drivers/clk/sprd/div.c | 6 +-
drivers/clk/stm32/clk-stm32-core.c | 42 +++-----
drivers/clk/sunxi-ng/ccu_div.c | 25 +++--
drivers/clk/sunxi-ng/ccu_mp.c | 26 ++---
drivers/clk/sunxi-ng/ccu_mult.c | 16 +--
drivers/clk/sunxi-ng/ccu_mux.c | 49 +++++----
drivers/clk/sunxi-ng/ccu_mux.h | 8 +-
drivers/clk/sunxi-ng/ccu_nkm.c | 25 ++---
drivers/clk/x86/clk-cgu.c | 6 +-
drivers/clk/zynqmp/divider.c | 5 +-
drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c | 7 +-
drivers/phy/ti/phy-j721e-wiz.c | 5 +-
drivers/rtc/rtc-ac100.c | 75 +++++++-------
include/linux/clk-provider.h | 28 ------
29 files changed, 257 insertions(+), 408 deletions(-)
---
base-commit: f8f97927abf7c12382dddc93a144fc9df7919b77
change-id: 20260107-clk-divider-round-rate-1cfd117b0670
Best regards,
--
Brian Masney <bmasney@redhat.com>
^ permalink raw reply
* [PATCH 03/27] rtc: ac100: convert from divider_round_rate() to divider_determine_rate()
From: Brian Masney @ 2026-01-08 21:16 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd
Cc: linux-clk, linux-kernel, Brian Masney, Alexandre Belloni,
linux-rtc
In-Reply-To: <20260108-clk-divider-round-rate-v1-0-535a3ed73bf3@redhat.com>
The divider_round_rate() function is now deprecated, so let's migrate
to divider_determine_rate() instead so that this deprecated API can be
removed.
Signed-off-by: Brian Masney <bmasney@redhat.com>
---
To: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: linux-rtc@vger.kernel.org
---
drivers/rtc/rtc-ac100.c | 75 +++++++++++++++++++++++++------------------------
1 file changed, 38 insertions(+), 37 deletions(-)
diff --git a/drivers/rtc/rtc-ac100.c b/drivers/rtc/rtc-ac100.c
index 33626311fa781b5ce90dcc472f948dc933bbc897..16aca4431da8c029e6195d8a3c9fe75fa95d0bc0 100644
--- a/drivers/rtc/rtc-ac100.c
+++ b/drivers/rtc/rtc-ac100.c
@@ -140,42 +140,16 @@ static unsigned long ac100_clkout_recalc_rate(struct clk_hw *hw,
AC100_CLKOUT_DIV_WIDTH);
}
-static long ac100_clkout_round_rate(struct clk_hw *hw, unsigned long rate,
- unsigned long prate)
-{
- unsigned long best_rate = 0, tmp_rate, tmp_prate;
- int i;
-
- if (prate == AC100_RTC_32K_RATE)
- return divider_round_rate(hw, rate, &prate, NULL,
- AC100_CLKOUT_DIV_WIDTH,
- CLK_DIVIDER_POWER_OF_TWO);
-
- for (i = 0; ac100_clkout_prediv[i].div; i++) {
- tmp_prate = DIV_ROUND_UP(prate, ac100_clkout_prediv[i].val);
- tmp_rate = divider_round_rate(hw, rate, &tmp_prate, NULL,
- AC100_CLKOUT_DIV_WIDTH,
- CLK_DIVIDER_POWER_OF_TWO);
-
- if (tmp_rate > rate)
- continue;
- if (rate - tmp_rate < best_rate - tmp_rate)
- best_rate = tmp_rate;
- }
-
- return best_rate;
-}
-
static int ac100_clkout_determine_rate(struct clk_hw *hw,
struct clk_rate_request *req)
{
- struct clk_hw *best_parent;
+ int i, ret, num_parents = clk_hw_get_num_parents(hw);
+ struct clk_hw *best_parent = NULL;
unsigned long best = 0;
- int i, num_parents = clk_hw_get_num_parents(hw);
for (i = 0; i < num_parents; i++) {
struct clk_hw *parent = clk_hw_get_parent_by_index(hw, i);
- unsigned long tmp, prate;
+ unsigned long prate;
/*
* The clock has two parents, one is a fixed clock which is
@@ -199,13 +173,40 @@ static int ac100_clkout_determine_rate(struct clk_hw *hw,
prate = clk_hw_get_rate(parent);
- tmp = ac100_clkout_round_rate(hw, req->rate, prate);
-
- if (tmp > req->rate)
- continue;
- if (req->rate - tmp < req->rate - best) {
- best = tmp;
- best_parent = parent;
+ if (prate == AC100_RTC_32K_RATE) {
+ struct clk_rate_request div_req = *req;
+
+ div_req.best_parent_rate = prate;
+
+ ret = divider_determine_rate(hw, &div_req, NULL,
+ AC100_CLKOUT_DIV_WIDTH,
+ CLK_DIVIDER_POWER_OF_TWO);
+ if (ret != 0 || div_req.rate > req->rate)
+ continue;
+ else if (req->rate - div_req.rate < req->rate - best) {
+ best = div_req.rate;
+ best_parent = parent;
+ }
+ } else {
+ int j;
+
+ for (j = 0; ac100_clkout_prediv[j].div; j++) {
+ struct clk_rate_request div_req = *req;
+ unsigned long tmp_prate;
+
+ tmp_prate = DIV_ROUND_UP(prate, ac100_clkout_prediv[j].div);
+ div_req.best_parent_rate = tmp_prate;
+
+ ret = divider_determine_rate(hw, &div_req, NULL,
+ AC100_CLKOUT_DIV_WIDTH,
+ CLK_DIVIDER_POWER_OF_TWO);
+ if (ret != 0 || div_req.rate > req->rate)
+ continue;
+ else if (req->rate - div_req.rate < req->rate - best) {
+ best = div_req.rate;
+ best_parent = parent;
+ }
+ }
}
}
@@ -213,7 +214,7 @@ static int ac100_clkout_determine_rate(struct clk_hw *hw,
return -EINVAL;
req->best_parent_hw = best_parent;
- req->best_parent_rate = best;
+ req->best_parent_rate = clk_hw_get_rate(best_parent);
req->rate = best;
return 0;
--
2.52.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox