From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 704ED70810; Tue, 24 Feb 2026 00:12:37 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1771891957; cv=none; b=StTDAH3TxT/ruyA/3E4y1s4h2+Gwhy0Xknr4cOC+mZumEjMCld/VQsLQYoSFFrJf2aibyp/cYEajZBldjec6JJywEALnuVgdOvArosqmxEt4X6NfQR61th/Rdzmd0P1tiZqeikcE+HFVajqknjZu+g77ymRAdLlw7bJRz3oD9T4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1771891957; c=relaxed/simple; bh=5MoNpm6w13iH1OdpASvRWeXDOeQYYcPrWRjlmiy22yA=; h=Mime-Version:Content-Type:Date:Message-Id:Subject:Cc:To:From: References:In-Reply-To; b=IrQm6v3XqXfDZGWNLgg8lL9fVKgJfg9u5Fp1/j9SoiTzpeVaoMUr/G6G32QagWFj+2rONjpMz9SAFzaCYV4LcEp7QuvOv7zPd6WKRX2dtu3nz19cWDZqjbhVyulAYQyDdp9pXG/IH+r3VDhMLxGmClvRXAeUQNvFLqCtmXwOMYk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=TsypuPUr; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="TsypuPUr" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 6F7A6C116C6; Tue, 24 Feb 2026 00:12:34 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1771891957; bh=5MoNpm6w13iH1OdpASvRWeXDOeQYYcPrWRjlmiy22yA=; h=Date:Subject:Cc:To:From:References:In-Reply-To:From; b=TsypuPUrgxou8ammBptWG3KG0R28ghyv62RNx5KnQrVjmf1hUtNklXYtMhQH6DV6S ZTgB5CGJmv4R9mEDFfk9cToIjs5vOzCjxKl50Eyq/jtYoW+0KsRppOpXi6uhaUIpMx 1CYmg4xENqSJPeO/plt0lJ8QjKfrGXo8+P9p3OiRDAct7qMbrpsrgEHS9jM4DBjH5s JQ2QCIGwir7alEsyL6gPf51i+KZTjYzm9QCk56gpBtQICAXfjxF83QlBN6HQqaNwtC S/YW895b5wflgRbfPf9tadxJGSZ18mu2sFLkDujITNvLWrYZi1BYFpu0qXMlFjiVtB O2qUPEFkk6+vQ== Precedence: bulk X-Mailing-List: linux-rtc@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Tue, 24 Feb 2026 01:12:32 +0100 Message-Id: Subject: Re: [RFC PATCH v3 1/5] rtc: add device selector for rtc_class_ops callbacks Cc: "Alexandre Belloni" , "Alvin Sun" , "Miguel Ojeda" , "Boqun Feng" , "Gary Guo" , =?utf-8?q?Bj=C3=B6rn_Roy_Baron?= , "Benno Lossin" , "Andreas Hindborg" , "Alice Ryhl" , "Trevor Gross" , , , "Greg Kroah-Hartman" To: "Rafael J. Wysocki" From: "Danilo Krummrich" References: <20260116162203.296844-1-sunke@kylinos.cn> <20260116162203.296844-2-sunke@kylinos.cn> <77d373dc-c5f2-4dca-b0d2-b5cee6a21b3b@gmail.com> <20260220225341c5eeb835@mail.local> <20260221111619162a41a1@mail.local> <20260222000556ea1938c0@mail.local> In-Reply-To: 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 thi= s, since it actually does not work perfectly fine without further changes that would (re-)introduce ordering constraints for drivers on the probe() side o= f 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 =3D platform::Device; > > fn read_time( > parent: &platform::Device, > time: &mut rtc::Time, > ) -> Result { > let drvdata =3D pdev.as_ref().drvdata::()?; > > ... > } > } 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, rtc: ARef, ..., } impl pci::Driver for SampleDriver { fn probe(pdev: &pci::Device, info: &Self::IdInfo) -> impl PinIni= t { let dev =3D pdev.as_ref(); let rtc =3D 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> ty= pe, 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, rtc: ARef, ..., } impl pci::Driver for SampleDriver { fn probe<'a>(pdev: pci::ProbeDevice<'a>, info: &Self::IdInfo) -> Resul= t { let dev =3D pdev.as_ref(); let rtc =3D rtc::Device::new(dev)?; let data =3D impl_pin_init!(Self { irq <- irq::Registration::new(...), rtc, }); // The `pci::ProbeDevice` has been consumed by value, and // returned a `&pci::Device`. let pdev =3D pdev.set_drvdata(data)?; devres::register(rtc::Registration::new(rtc)) } } However, note how this wouldn't solve the same problem for irq::Registratio= n, 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 safe= ty reasons. We could probably find other ways to somehow handle this, e.g. by introduci= ng new device context states, additional callbacks, etc., but in the end it wo= uld 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 thi= s 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 operatio= ns, timers, etc. I.e. there is a clear separation of ownership and lifetime, which makes mor= e or less (often more) suble ordering constraints obsolete. In fact, in the beginning I did not even plan to expose Device::drvdata() a= t 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 alread= y provide access to the driver's device private data through a &sel= f 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::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 th= e 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, > }; > > // The bus device private data. > #[pin_data] > struct SampleDriver { > #[pin] > irq: irq::Registration, > rtc: ARef>, > } > > // The class device private data. > struct SampleRtcData { > io: Devres>, > hw_variant: VendorVariant, > } > > impl rtc::Ops for MyRtcOps { > type BusDeviceType =3D platform::Device; > > fn read_time( > rtc: &rtc::Device > parent: &platform::Device, > time: &mut rtc::Time, > ) -> Result { > let io =3D rtc.io.access(parent)?; > > match rtc.hw_variant { > VendorVariant::Arm | VendorVariant::StV1 =3D> { > let my_time =3D io.read(...); > > my_time.write_into(time); > }, > VendorVariant::StV2 =3D> { ... }, > } > } > } > impl pci::Driver for SampleDriver { fn probe(pdev: &pci::Device, info: &Self::IdInfo) -> impl PinIni= t { let dev =3D pdev.as_ref(); let rtc_data =3D impl_pin_init!(SampleRtcData { io: iomap_region_sized::(0, c"my_rtc/bar0")?, hw_variant: VendorVariant::StV1, }); let rtc =3D 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 no= t compile in the first place, which is a huge improvement over the situation = we have in C.)