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

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

On Wed Feb 25, 2026 at 2:33 PM CET, Rafael J. Wysocki wrote:
> On Tue, Feb 24, 2026 at 11:23 PM Danilo Krummrich <dakr@kernel.org> wrote:
>> Here's also some sketched up code for what I wrote above:
>>
>>         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: pdev.iomap_region_sized::<BAR0_SIZE>(0, c"my_rtc/bar0")?,
>>                 hw_variant: VendorVariant::StV1,
>>                 irq <- irq::Registration::new(...),
>>             });
>>
>>             let rtc = rtc::Device::new(dev, rtc_data)?;
>>
>>             rtc::Registration::register(rtc)?;
>>
>>             Ok(Self { rtc })
>>         }
>>
>> Note that if any of the RTC callbacks would ever need to call irq.synchronize(),
>> irq.disable(), etc. the compiler would enforce correct ordering, as there would
>> not be any other possibility to put the irq::Registration other than into the
>> rtc_data that goes into rtc::Device::new().
>
> IIUC, the interrupt handler can only access the rtc_data because the
> parent's driver_data may not exist yet when it runs.  Or am I missing
> something?

In the code above the IRQ handler can also not access rtc_data, as struct
SampleRtcData might not be fully initialized when it runs, i.e.

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

would not compile in the first place.

irq::Registration, for this purpose, has its own private data on the handler
itself, see also [1]. In fact, the C code has the same concept with the dev_id
argument in request_threaded_irq() [2].

The difference is that the C compiler does not ensure that the IRQ handler
actually owns the data behind the dev_id pointer. I.e. the driver has to somehow
ensure that whatever is behind the dev_id pointer remains valid for the duration
the IRQ handler is registered.

In the Rust implementation the compiler does ensure that what is behind the
dev_id pointer remains valid for the duration of the lifetime of the
irq::Registration.

Having that said, I assume you wonder what we would pass into the
irq::Registration instead, if it is not rtc_data.

The answer is it depends; it depends on what's actually needed, what other
entities interact with the IRQ (e.g. some scheduled work, etc.) and maybe even
preference to some extend.

Here is one example:

	let irq_data = impl_pin_init!(SampleIrqData {
	    io <- pdev.iomap_region_sized::<BAR0_SIZE>(0, c"my_rtc/bar0")?,
	    hw_variant: VendorVariant::StV1,
	});

	let rtc_data = impl_pin_init!(SampleRtcData {
	    irq <- irq::Registration::new(..., irq_data),
	    ...,
	});

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

This would compile as it ensures that irq_data (struct SampleIrqData) is fully
initialized before irq::Registration::new() is called.

At a first glance this might look like we need an additional allocation, one for
irq_data and one for rtc_data, but that is not the case. irq_data is an
initializer that is passed to another initializer, i.e. rtc_data is still an
initializer.

The actual (single) allocation happens in rtc::Device::new().

In terms of accessing it through the the rtc::Device in an RTC device callback,
we would likely use accessor methods to make it a bit more convinient, i.e.

	    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 => { ... },
	        }
	    }

As mentioned above there are a few other options to implement this, depending on
what's required, etc.

For instance, if the I/O bar is actually shared between multiple entities we
might want to initialize it within an Arc [3] (reference count it) for shared
ownership.

For the future we will also be able to support references within initializers to
other pinned fields, which make things a bit more convinient, so you could do
things like this:

	let irq_data = impl_pin_init!(SampleIrqData {
	    io <- pdev.iomap_region_sized::<BAR0_SIZE>(0, c"my_rtc/bar0")?,
	    hw_variant: VendorVariant::StV1,
	});

	let rtc_data = impl_pin_init!(SampleRtcData {
	    irq <- irq::Registration::new(..., irq_data),
	    io: &irq.io,
	    ...,
	});

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

Note the additional `io: &irq.io,` in the rtc_data initializer. This would be
legal as we know that `irq` is pinned within `rtc_data`, hence it is valid to
hold a reference to one of its pinned fields.

I am not sure how far we are from having this supported, I assume Benno and Gary
can say more about this.

I hope this helps, and thanks for asking those questions!

[1] https://rust.docs.kernel.org/kernel/irq/struct.Registration.html
[2] https://elixir.bootlin.com/linux/v6.19.3/source/kernel/irq/manage.c#L2090
[3] https://rust.docs.kernel.org/kernel/sync/struct.Arc.html