From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-181.mta1.migadu.com (out-181.mta1.migadu.com [95.215.58.181]) (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 A0B0E24C676; Sat, 21 Feb 2026 09:31:29 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=95.215.58.181 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1771666298; cv=none; b=KZvKB78UOQV2cg8omCZeCGYn+T3IKeJ78IeUbk0UHJhpRsfOSS5hlnkpNVEchIBT50K8tv8A3exgO+1C8M0T+QzDfxKWcyIcJZ6DVn+8ENuMSj3Zn9oC44vjfQgAWIPPnsfNyQQEZB2FF1gVtB9Gx54r2l//BKxaP9BoUbxA0aw= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1771666298; c=relaxed/simple; bh=JZd3TUO8sHKc1E5nsz3EpGaka1sJimcYCK4bXDXs0Ig=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=RZK/Rp5yz3ji27yzFB4/VIKPXlCgpbXTOT9Q9RhVlyFJ1L3cvvTw3pH7jOH6U3/nzrutkfJy9AdgPol2iX4eYMeHs2OakzqQMEbK2O5HKpA5EzEH1DtBty2TaL27y7k5zOC9JYDe+THnHB/6QpzZ89zDhEI6NwxT22YzVzkGPAk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=Mv1kJVIE; arc=none smtp.client-ip=95.215.58.181 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="Mv1kJVIE" Message-ID: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1771666287; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=E0e3N+WFi6mKTSOmzqovnatex4YXABvp7cs8CmE8c8I=; b=Mv1kJVIEwjxprAlnE7ePaFZ2mmEYiE6FpB0p8vlY+HTS/ika+2crJrWqhBAZDnTEangKkG vF+x6eAzf15i1e6fPAryB+htLyuOxECps0kTkoZ577lF8yybbXLNT+C+B8UtwcG0IUHy/J QlJXS36aR4TenAW9wV8MjXXQbj9Zj0c= Date: Sat, 21 Feb 2026 17:31:09 +0800 Precedence: bulk X-Mailing-List: rust-for-linux@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Subject: Re: [RFC PATCH v3 1/5] rtc: add device selector for rtc_class_ops callbacks To: Alexandre Belloni , Danilo Krummrich Cc: Miguel Ojeda , Boqun Feng , Gary Guo , =?UTF-8?Q?Bj=C3=B6rn_Roy_Baron?= , Benno Lossin , Andreas Hindborg , Alice Ryhl , Trevor Gross , linux-rtc@vger.kernel.org, rust-for-linux@vger.kernel.org, Greg Kroah-Hartman , "Rafael J. Wysocki" References: <20260116162203.296844-1-sunke@kylinos.cn> <20260116162203.296844-2-sunke@kylinos.cn> <77d373dc-c5f2-4dca-b0d2-b5cee6a21b3b@gmail.com> <20260220225341c5eeb835@mail.local> X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Alvin Sun In-Reply-To: <20260220225341c5eeb835@mail.local> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Migadu-Flow: FLOW_OUT On 2/21/26 06:53, Alexandre Belloni wrote: > On 20/01/2026 16:01:40+0800, Ke Sun wrote: >> On 1/19/26 22:32, Danilo Krummrich wrote: >>> On Fri Jan 16, 2026 at 5:21 PM CET, Ke Sun wrote: >>>> diff --git a/drivers/rtc/dev.c b/drivers/rtc/dev.c >>>> index baf1a8ca8b2b1..eddcc5a69db3b 100644 >>>> --- a/drivers/rtc/dev.c >>>> +++ b/drivers/rtc/dev.c >>>> @@ -410,7 +410,7 @@ static long rtc_dev_ioctl(struct file *file, >>>> } >>>> default: >>>> if (rtc->ops->param_get) >>>> - err = rtc->ops->param_get(rtc->dev.parent, ¶m); >>>> + err = rtc->ops->param_get(rtc_ops_dev(rtc), ¶m); >>>> else >>>> err = -EINVAL; >>>> } >>> >>> >>>> +/** >>>> + * rtc_ops_dev - Get the device pointer for RTC ops callbacks >>>> + * @rtc: RTC device >>>> + * >>>> + * Returns &rtc->dev if RTC_OPS_USE_RTC_DEV flag is set, >>>> + * otherwise returns rtc->dev.parent. >>>> + */ >>>> +static inline struct device *rtc_ops_dev(struct rtc_device *rtc) >>>> +{ >>>> + if (test_bit(RTC_OPS_USE_RTC_DEV, &rtc->flags)) >>>> + return &rtc->dev; >>>> + return rtc->dev.parent; >>>> +} >>> I understand that the idea is to gradually convert all drivers to use the RTC >>> device, rather than it's parent device in RTC device callbacks. >>> >>> My main concern is that once that has been achieved it's still not what we want >>> to have eventually, i.e. RTC device callbacks should ideally take a struct >>> rtc_device as argument and not the embedded base struct device. >>> >>> I.e. we'd kick off a conversion process that won't reach the actual desired >>> state. >> Hi Danilo, >> >> This is indeed an intermediate step. >> >> Full cleanup is in progress, but it's large and untested. I'm working on a >> complete cleanup involving ~190+ files across arch/, drivers/rtc/, and >> drivers/virtio/. Most changes are straightforward interface replacements, >> but some drivers need additional modifications. Given the scale, I haven't >> fully tested everything and can't guarantee correctness yet. >> >> The intermediate step enables gradual migration, allowing us to: >> - Clean up and test each rtc driver incrementally >> - Ensure correctness through gradual changes >> - Avoid breaking existing functionality >> >> Once all cleanup is complete and tested, changing all rtc_class_ops >> callbacks to use struct rtc_device * will be much simpler and safer. >> >> Currently there seem to be only these two approaches. I'm still waiting >> for Alexandre's suggestion on how to proceed specifically, but haven't >> received a response yet. > I'm sorry, I still don't see the point of doing this. The driver will > almost always need to set its driver data in the parent device because > we need to be able to handle interrupts, suspend/resume or .remove(). So > while intellectually, this would be more satisfying to have the > callbacks take a struct rtc_device, functionally this doesn't have any > purpose because we need to use the parent drvdata anyway. > > Said differently, you should explain why a device driver must not call > amba_set_drvdata() ? As in platform.rs [1] and i2c.rs [2], set_drvdata is always called by the bus Adapter's probe_callback, not by the device driver. [1]: https://elixir.bootlin.com/linux/v6.19-rc5/source/rust/kernel/platform.rs#L80 [2]: https://elixir.bootlin.com/linux/v6.19-rc5/source/rust/kernel/i2c.rs#L160 In Rust the Adapter already sets drvdata on the bus device in probe. If the driver also calls amba_set_drvdata() there, it overwrites that pointer; on remove/shutdown the framework then gets wrong data and can hit use-after-free or crashes. So only the framework must set drvdata on the bus device. This applies only to the Rust implementation; in C, calling amba_set_drvdata() is fine. In the Rust design the bus device owns the bus device's drvdata and the class device owns the class device's drvdata, so the class driver must not set drvdata on the bus device. This is my understanding of the Rust device driver abstraction design. Danilo is the authority on this. Best regards, Ke Sun > Out of 29 drivers, 18 are doing so. >