Re: [RFC PATCH v3 1/5] rtc: add device selector for rtc_class_ops callbacks

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

On Sun Feb 22, 2026 at 5:13 PM CET, Danilo Krummrich wrote:
> There is no limitation from the Rust side of things, as mentioned in [1], this
> should work perfectly fine, but it might be slightly less convinient:

When I wrote this on Sunday, I should have been much more precise about this,
since it actually does not work perfectly fine without further changes that
would (re-)introduce ordering constraints for drivers on the probe() side of
things that I want to avoid.

The "re-" is in braces as we never had those ordering constraints in Rust, but
we have quite a lot of them in C, where they cause endless problems. Which is
why I want to avoid them in Rust.

> Example 1:
>
> 	impl rtc::Ops for MyRtcOps {
> 	    type BusDeviceType = platform::Device<Bound>;
>
> 	    fn read_time(
> 	        parent: &platform::Device<Bound>,
> 	        time: &mut rtc::Time,
> 	    ) -> Result {
> 	        let drvdata = pdev.as_ref().drvdata::<MyDriver>()?;
>
> 	        ...
> 	    }
> 	}

Let's have a look at the corresponding probe() side of things.

	struct SampleIrqData;

	// The bus device private data.
	#[pin_data]
	struct SampleDriver {
	    #[pin]
	    irq: irq::Registration<SampleIrqData>,
	    rtc: ARef<rtc::Device>,
	    ...,
	}

	impl pci::Driver for SampleDriver {
	    fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> impl PinInit<Self, Error> {
	        let dev = pdev.as_ref();

	        let rtc = rtc::Device::new(dev)?;

	        Ok(impl_pin_init!(Self {
	            irq <- irq::Registration::new(...),
	            rtc,
	            _: {
	                devres::register(rtc::Registration::new(rtc))?;
	            }
	        })
	    }
	}

This is a problem, since even though the registration of the RTC device is the
last thing in the initializer, rtc::Ops::read_time() above could race and
Device::drvdata() could just fail as it might not be set yet.

This is fixable, but - as mentioned - at the price of introducing ordering
constraints for drivers.

For instance, this could be solved by introducing a pci::ProbeDevice<'a> type,
which ensures that pci::ProbeDevice::set_drvdata() can only be called from
probe() and can only be called once, as it consumes the pci::ProbeDevice.

	struct SampleIrqData;

	// The bus device private data.
	#[pin_data]
	struct SampleDriver {
	    #[pin]
	    irq: irq::Registration<SampleIrqData>,
	    rtc: ARef<rtc::Device>,
	    ...,
	}

	impl pci::Driver for SampleDriver {
	    fn probe<'a>(pdev: pci::ProbeDevice<'a>, info: &Self::IdInfo) -> Result {
	        let dev = pdev.as_ref();

	        let rtc = rtc::Device::new(dev)?;

	        let data = impl_pin_init!(Self {
	            irq <- irq::Registration::new(...),
	            rtc,
	        });

	        // The `pci::ProbeDevice` has been consumed by value, and
	        // returned a `&pci::Device<Core>`.
	        let pdev = pdev.set_drvdata(data)?;

	        devres::register(rtc::Registration::new(rtc))
	    }
	}

However, note how this wouldn't solve the same problem for irq::Registration, as
it lives within the the driver's device private data.

If this would be C you would probably just partially initialize the driver's
device private data, but in Rust uninitialized data is not allowed for safety
reasons.

We could probably find other ways to somehow handle this, e.g. by introducing
new device context states, additional callbacks, etc., but in the end it would
only be workarounds for not having defined ownership, that eventually ends up
being more complicated and more error prone.

For this reason an irq::Registration has its own private data (which in this
example is just an empty struct).

Similarly, all other kinds of registrations (and driver entry points) have their
own private data, such as class devices, work and workqueues, file operations,
timers, etc.

I.e. there is a clear separation of ownership and lifetime, which makes more or
less (often more) suble ordering constraints obsolete.

In fact, in the beginning I did not even plan to expose Device::drvdata() at
all, making the driver's device private data only available in bus device
callbacks, etc.

The reason why I added it was that it was required (and makes sense) when
drivers interact with other drivers, e.g. through an auxiliary device.

I did also mention this in the commit message of commit 6f61a2637abe ("rust:
device: introduce Device::drvdata()"):

      (2) Having a direct accessor to the driver's private data is not
          commonly required (at least in Rust): Bus callback methods already
          provide access to the driver's device private data through a &self
          argument, while other driver entry points such as IRQs,
          workqueues, timers, IOCTLs, etc. have their own private data with
          separate ownership and lifetime.

          In other words, a driver's device private data is only relevant
          for driver model contexts (such a file private is only relevant
          for file contexts).

    Having that said, the motivation for accessing the driver's device
    private data with Device<Bound>::drvdata() are interactions between
    drivers. For instance, when an auxiliary driver calls back into its
    parent, the parent has to be capable to derive its private data from the
    corresponding device (i.e. the parent of the auxiliary device).

Let's have a look at how probe() looks like for the example below, which is what
we do in other subsystems, such as DRM or PWM.

> Example 3:
>
>	struct SampleIrqData {
>	    rtc: ARef<rtc::Device>,
>	};
>
> 	// The bus device private data.
> 	#[pin_data]
> 	struct SampleDriver {
> 	    #[pin]
> 	    irq: irq::Registration<SampleIrqHandler>,
> 	    rtc: ARef<rtc::Device<SampleRtcData>>,
> 	}
>
> 	// The class device private data.
> 	struct SampleRtcData {
> 	    io: Devres<IoMem<PL031_REG_SIZE>>,
> 	    hw_variant: VendorVariant,
> 	}
>
> 	impl rtc::Ops for MyRtcOps {
> 	    type BusDeviceType = platform::Device<Bound>;
>
> 	    fn read_time(
> 	        rtc: &rtc::Device<SampleRtcData>
> 	        parent: &platform::Device<Bound>,
> 	        time: &mut rtc::Time,
> 	    ) -> Result {
> 	        let io = rtc.io.access(parent)?;
>
> 	        match rtc.hw_variant {
> 	            VendorVariant::Arm | VendorVariant::StV1 => {
> 	                let my_time = io.read(...);
>
> 	                my_time.write_into(time);
> 	            },
> 	            VendorVariant::StV2 => { ... },
> 	        }
> 	    }
> 	}
>

	impl pci::Driver for SampleDriver {
	    fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> impl PinInit<Self, Error> {
	        let dev = pdev.as_ref();

	        let rtc_data = impl_pin_init!(SampleRtcData {
	            io: iomap_region_sized::<BAR0_SIZE>(0, c"my_rtc/bar0")?,
	            hw_variant: VendorVariant::StV1,
	        });

	        let rtc = rtc::Device::new(dev, rtc_data)?;

	        // Internally calls `devres::register(rtc::Registration::new())`.
	        rtc::Registration::register(rtc)?;

	        Ok(impl_pin_init!(Self {
	            // Give the IRQ handler a reference count of the `rtc::Device`.
	            irq <- irq::Registration::new(..., rtc.clone()),
	            rtc,
	        })
	    }
	}

With this there are no (subtle) ordering constraints the driver has to get
right; ownership and lifetimes are well defined.

(I.e. whatever order a driver picks, it either works properly or it does not
compile in the first place, which is a huge improvement over the situation we
have in C.)