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 19EF82B9B7; Sun, 22 Feb 2026 16:13:19 +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=1771776800; cv=none; b=Ze3NgITpWGx1wpfu8AiCCtMNtOho+x4DS4xgCqw/mU9NncsBUaYy2GntwO2Cpruw0jlW4bvCLP9ytJWygMTBGB5F5FQKutpLAlhVnOaRxjrA9hBK93dCtAsdusDm1OcN89hqBhmgEkrGpabe6SyQbxR6xelLjIk4rzClKbW+BJY= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1771776800; c=relaxed/simple; bh=0uHqYO6AEOaaPI4NW1bgjpfZn41WJvwRuhen1mUaE+E=; h=Mime-Version:Content-Type:Date:Message-Id:Subject:Cc:To:From: References:In-Reply-To; b=DlHsWOiuHpp5GGp7ifGfYn8i8Kqs61j43Ma4W/07TjbejKx/MWIadQxkP/ENEVuPV13JbC+cs6rqxz7j2kVu9qcvsbGpjBa6WhCXuvPqB1549X+4E0aHCeaHkHHy/2judzIaiYBsOqxlhnklTA8krH0L722thcB4NQ0YihSqS00= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=hbeuThbf; 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="hbeuThbf" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 255C0C116D0; Sun, 22 Feb 2026 16:13:16 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1771776799; bh=0uHqYO6AEOaaPI4NW1bgjpfZn41WJvwRuhen1mUaE+E=; h=Date:Subject:Cc:To:From:References:In-Reply-To:From; b=hbeuThbfKjBK4SIYQnXPSrNFtWEleAP7kq+KK5BS57OsY6+8+NipjfQZLSQZtqgLD sQNbeXmnJY4Htz0vrBD9XiCkN+8Z5sseOe4STR3T7fvZ9/t4GjZYUWRAFaPND74HPa 5Ir3WbmXv02msxj9kcFa5MItERu+czPqZ1f/wolQKXVSy2xKiT1tQH9wlFMyLRHt2t Jr7Z7gq2p6/VZZLvD4GZcPJIRA1/LuS7y8jDZKqRVPtg8x7jETx+4CY648wnszcDHg zRMm6dhCMdA2S1IuWHjbL9ToKoF/XNccPCbql85srI5xPnEWfXVfkFAol1cqUKAu3E WKa3Ptb97OVeA== Precedence: bulk X-Mailing-List: rust-for-linux@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: Sun, 22 Feb 2026 17:13:14 +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 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], t= his should work perfectly fine, but it might be slightly less convinient: 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::()?; ... } } We can improve this by avoiding that the driver has to extract its bus devi= ce private data all the time by doing this instead: Example 2: impl rtc::Ops for MyRtcOps { type BusDeviceType =3D platform::Device; type BusDeviceData =3D MyDriver; fn read_time( parent: &platform::Device, drvdata: &MyDriver, time: &mut rtc::Time, ) -> Result { ... } } However, this would require to type the rtc::Device over the bus device pri= vate 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, } // The class device private data. struct MyRtcData { 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> { ... }, } } } (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, rtc: ARef>, } But again, I just want to make sure we do not encourage to just throw all t= he 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 dev= ice over bus device private data. [1] https://lore.kernel.org/all/DGLLJ541SEJW.160MET6OCQHKS@kernel.org/