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 3:01 PM CET, Rafael J. Wysocki wrote:
> General considerations aside, regarding the $subject patch, I don't
> think that it is an improvement because it would confuse the existing
> RTC class device interface quite a bit.
>
> Regarding whether or not switching over the RTC class device interface
> to passing RTC device pointers to its class callbacks is a good idea,
> I think that the difference really boils down to what data needs to be
> accessed by the driver.
>
> As it stands today, in an RTC class callback, the driver gets directly
> to the parent of the given RTC device and if it does not need to get
> to the RTC device at all, that's convenient.  However, if the driver
> needs to get to the RTC device, it will most likely need to use
> driver_data in the parent device for this purpose (and store the RTC
> device pointer).
>
> If the interface were changed to pass RTC device pointers to class
> callbacks, the driver would get to the RTC device and from there it
> would need to get to the parent device in the majority of cases.  Even
> though getting from an RTC device to its parent device is
> straightforward (and does not require using driver_data at all), in
> some drivers that would be an extra step that would make the picture
> somewhat less clear than it is today (and arguably less clear than it
> really needs to be).  Still, the change of the interface might allow
> some other drivers that do access the RTC device in class callback
> functions to be simplified.
>
> Granted, the above options had already been there when the RTC class
> interface was designed and the choice was made at that time.  Revising
> it now would require clear technical reasons IMV.
>
> How hard would it be for Rust to cope with the existing calling
> convention in the RTC class?

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:

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>()?;

	        ...
	    }
	}

We can improve this by avoiding that the driver has to extract its bus device
private data all the time by doing this instead:

Example 2:

	impl rtc::Ops for MyRtcOps {
	    type BusDeviceType = platform::Device<Bound>;
	    type BusDeviceData = MyDriver;

	    fn read_time(
	        parent: &platform::Device<Bound>,
	        drvdata: &MyDriver,
	        time: &mut rtc::Time,
	    ) -> Result {
	        ...
	    }
	}

However, this would require to type the rtc::Device over the bus device private
data type, which works perfectly fine, but arguably is a bit odd.

The alternative to make it convinient for drivers and avoid this oddity is to
just let drivers store the data that needs to be accessed from class device
callbacks in the class device private data and data that is only needed in bus
device callbacks in the bus device private data. Additionally, drivers can of
course save a refcount of the class device in their bus device private data as
well.

This is mostly what patch 5 of this series currently does (with a few
modifications) and I personally think it's the most clean approach in terms of
how it turns out for drivers:

Example 3:

	// The bus device private data.
	#[pin_data]
	struct MyDriver {
	    #[pin]
	    irq: irq::Registration<MyIrqHandler>,
	}

	// The class device private data.
	struct MyRtcData {
	    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<MyRtcData>
	        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 => { ... },
	        }
	    }
	}

(Note how we can directly access the `io` and `hw_variant` fields from `rtc`.)

If the data from struct MyRtcData is also needed from other bus callbacks, we
can simply extend struct MyDriver:

	#[pin_data]
	struct MyDriver {
	    #[pin]
	    irq: irq::Registration<MyIrqHandler>,
	    rtc: ARef<rtc::Device<MyRtcData>>,
	}

But again, I just want to make sure we do not encourage to just throw all the
private data a driver may potentially have into the bus device private data
struct.

Besides that, it is of course a subsystem decision and I don't want to be
presumptuous or anything.

My recommendation is to go with something like in (3), otherwise I'd choose (1)
and (2) is my least favorite as I think it is a bit odd to type a class device
over bus device private data.

[1] https://lore.kernel.org/all/DGLLJ541SEJW.160MET6OCQHKS@kernel.org/