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

[Alexandre Belloni <alexandre.belloni@bootlin.com>]

On 24/02/2026 23:23:29+0100, Danilo Krummrich wrote:
> On Tue Feb 24, 2026 at 6:28 PM CET, Alexandre Belloni wrote:
> > On 24/02/2026 17:35:23+0100, Danilo Krummrich wrote:
> >> (I did not have any specific hardware in mind when sketching this up (e.g. an
> >> IRQ could also only be needed in bus device callbacks, e.g. for loading firmware
> >> etc.). But for RTC it obviously is common that it is relevant to the class
> >> device too.)
> >> 
> >> So, I assume you mean because there could already be an ioctl before the IRQ has
> >> been successfully registered, and this ioctl may wait for an IRQ?
> >> 
> >> In this case the irq::Registration should go into rtc_data instead to account
> >> for this dependency. Unfortunately, this is a semantic dependency that we can't
> >> always catch at compile time.
> >> 
> >> The reason we sometimes can is because, if you would need access to the
> >> irq::Registration from ioctls (e.g. for calling synchronize(), enable(),
> >> disable() etc.) it would be caught, because you couldn't access it without it
> >> being in rtc_data in the first place, and being forced to have it in rtc_data
> >> guarantees that the ordering can't be wrong.
> >
> > No, once you register the rtc, the character device will appear in
> > userspace and may be opened, at this point, probe is not allowed to fail
> > anymore which you are allowing by trying to register the IRQ so late.
> 
> This does not seem to correspond to my previous reply -- may I kindly ask you to
> read it again?
> 
> 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().

Right but again, the issue is not about the irq or resource allocation
ordering, it is about probe failing after the character device creation.

> 
> Besides that, you above mentioned "probe is not allowed to fail anymore" after
> the RTC device is registered and the corresponding character device becomes
> visible to userspace.
> 
> While there most likely isn't any good reason for probe() to fail afterwards for
> RTC devices, it is not the case that this isn't allowed. We generally can unwind
> from a class device registration. In fact, this is not different to remove()
> being called (immediately).

It is actually different, this was the race back then:

CPU0:                                CPU1:
sys_load_module()
 do_init_module()
  do_one_initcall()
   cmos_do_probe()
    rtc_device_register()
     __register_chrdev()
     cdev->owner = struct module*
                                     open("/dev/rtc0")
    rtc_device_unregister()
  module_put()
  free_module()
   module_free(mod->module_core)
   /* struct module *module is now
      freed */
                                      chrdev_open()
                                       spin_lock(cdev_lock)
                                       cdev_get()
                                        try_module_get()
                                         module_is_live()
                                         /* dereferences already
                                            freed struct module* */


I don't think it has been solved since then.

> 
> Imagine a case where a driver registers multiple class devices, or a class
> device and an auxiliary device, etc.
> 
> (But I assume your point was more that for an RTC device specifically this would
> be odd or uncommon.)

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com