Re: [RFC PATCH v2 5/5] rust: add PL031 RTC driver

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

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.