[RFC PATCH v3 0/5] rust: Add RTC driver support

[RFC PATCH v3 0/5] rust: Add RTC driver support

[Ke Sun]

This patch series adds RTC (Real-Time Clock) driver support for the Rust
kernel, including the necessary infrastructure and a complete driver
implementation for the PL031 RTC.

---
v3:
- Add rtc_ops_dev() helper with RTC_OPS_USE_RTC_DEV flag to allow drivers to
  choose device pointer for rtc_class_ops callbacks. This enables Rust RTC
  drivers to store drvdata on rtc->dev while maintaining backward compatibility
  for existing C drivers without requiring driver modifications
- Refactor AMBA and RTC abstractions to address v2 review feedback

v2: https://lore.kernel.org/rust-for-linux/20260107143738.3021892-1-sunke@kylinos.cn/
- Migrate RTC driver data storage from parent device to RTC device for
  unified interface
- Expand AMBA bus abstractions to full driver support with enhanced
  functionality
- Refactor device wakeup API by moving wake IRQ setup to IRQ layer
- Simplify RTC core framework by removing multi-bus abstractions,
  focusing on core operations
- Optimize PL031 driver implementation and remove build assertion
  dependency

v1: https://lore.kernel.org/rust-for-linux/20260104060621.3757812-1-sunke@kylinos.cn/
- Add AMBA bus abstractions
- Add device wakeup support
- Add RTC core framework with multi-bus support
- Add PL031 RTC driver

---

Ke Sun (5):
  rtc: add device selector for rtc_class_ops callbacks
  rust: add AMBA bus driver support
  rust: add device wakeup capability support
  rust: add RTC core abstractions and data structures
  rust: add PL031 RTC driver

 drivers/rtc/Kconfig             |    9 +
 drivers/rtc/Makefile            |    1 +
 drivers/rtc/dev.c               |    6 +-
 drivers/rtc/interface.c         |   18 +-
 drivers/rtc/proc.c              |    2 +-
 drivers/rtc/rtc_pl031_rust.rs   |  513 ++++++++++++++++
 include/linux/rtc.h             |   15 +
 rust/bindings/bindings_helper.h |    3 +
 rust/helpers/device.c           |    6 +
 rust/helpers/helpers.c          |    1 +
 rust/helpers/rtc.c              |    9 +
 rust/kernel/amba.rs             |  441 ++++++++++++++
 rust/kernel/device.rs           |   18 +-
 rust/kernel/irq/request.rs      |   17 +
 rust/kernel/lib.rs              |    4 +
 rust/kernel/rtc.rs              | 1008 +++++++++++++++++++++++++++++++
 16 files changed, 2057 insertions(+), 14 deletions(-)
 create mode 100644 drivers/rtc/rtc_pl031_rust.rs
 create mode 100644 rust/helpers/rtc.c
 create mode 100644 rust/kernel/amba.rs
 create mode 100644 rust/kernel/rtc.rs


base-commit: 944aacb68baf7624ab8d277d0ebf07f025ca137c
-- 
2.43.0

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

[Ke Sun]

Add rtc_ops_dev() helper to allow drivers to choose whether
rtc_class_ops callbacks receive rtc->dev or rtc->dev.parent. This
enables Rust RTC drivers to store driver data on the RTC device.

The helper checks RTC_OPS_USE_RTC_DEV flag: if set, returns &rtc->dev;
otherwise returns rtc->dev.parent.

Update all rtc_class_ops callback invocations to use rtc_ops_dev(rtc)
instead of directly accessing rtc->dev.parent.

Maintains backward compatibility for existing C drivers.

Signed-off-by: Ke Sun <sunke@kylinos.cn>
---
 drivers/rtc/dev.c       |  6 +++---
 drivers/rtc/interface.c | 18 +++++++++---------
 drivers/rtc/proc.c      |  2 +-
 include/linux/rtc.h     | 15 +++++++++++++++
 4 files changed, 28 insertions(+), 13 deletions(-)

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, &param);
+				err = rtc->ops->param_get(rtc_ops_dev(rtc), &param);
 			else
 				err = -EINVAL;
 		}
@@ -440,7 +440,7 @@ static long rtc_dev_ioctl(struct file *file,
 
 		default:
 			if (rtc->ops->param_set)
-				err = rtc->ops->param_set(rtc->dev.parent, &param);
+				err = rtc->ops->param_set(rtc_ops_dev(rtc), &param);
 			else
 				err = -EINVAL;
 		}
@@ -450,7 +450,7 @@ static long rtc_dev_ioctl(struct file *file,
 	default:
 		/* Finally try the driver's ioctl interface */
 		if (ops->ioctl) {
-			err = ops->ioctl(rtc->dev.parent, cmd, arg);
+			err = ops->ioctl(rtc_ops_dev(rtc), cmd, arg);
 			if (err == -ENOIOCTLCMD)
 				err = -ENOTTY;
 		} else {
diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
index b8b298efd9a9c..4c81130fb0394 100644
--- a/drivers/rtc/interface.c
+++ b/drivers/rtc/interface.c
@@ -91,7 +91,7 @@ static int __rtc_read_time(struct rtc_device *rtc, struct rtc_time *tm)
 		err = -EINVAL;
 	} else {
 		memset(tm, 0, sizeof(struct rtc_time));
-		err = rtc->ops->read_time(rtc->dev.parent, tm);
+		err = rtc->ops->read_time(rtc_ops_dev(rtc), tm);
 		if (err < 0) {
 			dev_dbg(&rtc->dev, "read_time: fail to read: %d\n",
 				err);
@@ -155,7 +155,7 @@ int rtc_set_time(struct rtc_device *rtc, struct rtc_time *tm)
 	if (!rtc->ops)
 		err = -ENODEV;
 	else if (rtc->ops->set_time)
-		err = rtc->ops->set_time(rtc->dev.parent, tm);
+		err = rtc->ops->set_time(rtc_ops_dev(rtc), tm);
 	else
 		err = -EINVAL;
 
@@ -200,7 +200,7 @@ static int rtc_read_alarm_internal(struct rtc_device *rtc,
 		alarm->time.tm_wday = -1;
 		alarm->time.tm_yday = -1;
 		alarm->time.tm_isdst = -1;
-		err = rtc->ops->read_alarm(rtc->dev.parent, alarm);
+		err = rtc->ops->read_alarm(rtc_ops_dev(rtc), alarm);
 	}
 
 	mutex_unlock(&rtc->ops_lock);
@@ -441,7 +441,7 @@ static int __rtc_set_alarm(struct rtc_device *rtc, struct rtc_wkalrm *alarm)
 	else if (!test_bit(RTC_FEATURE_ALARM, rtc->features))
 		err = -EINVAL;
 	else
-		err = rtc->ops->set_alarm(rtc->dev.parent, alarm);
+		err = rtc->ops->set_alarm(rtc_ops_dev(rtc), alarm);
 
 	/*
 	 * Check for potential race described above. If the waiting for next
@@ -568,7 +568,7 @@ int rtc_alarm_irq_enable(struct rtc_device *rtc, unsigned int enabled)
 	else if (!test_bit(RTC_FEATURE_ALARM, rtc->features) || !rtc->ops->alarm_irq_enable)
 		err = -EINVAL;
 	else
-		err = rtc->ops->alarm_irq_enable(rtc->dev.parent, enabled);
+		err = rtc->ops->alarm_irq_enable(rtc_ops_dev(rtc), enabled);
 
 	mutex_unlock(&rtc->ops_lock);
 
@@ -618,7 +618,7 @@ int rtc_update_irq_enable(struct rtc_device *rtc, unsigned int enabled)
 		rtc->uie_rtctimer.period = ktime_set(1, 0);
 		err = rtc_timer_enqueue(rtc, &rtc->uie_rtctimer);
 		if (!err && rtc->ops && rtc->ops->alarm_irq_enable)
-			err = rtc->ops->alarm_irq_enable(rtc->dev.parent, 1);
+			err = rtc->ops->alarm_irq_enable(rtc_ops_dev(rtc), 1);
 		if (err)
 			goto out;
 	} else {
@@ -874,7 +874,7 @@ static void rtc_alarm_disable(struct rtc_device *rtc)
 	if (!rtc->ops || !test_bit(RTC_FEATURE_ALARM, rtc->features) || !rtc->ops->alarm_irq_enable)
 		return;
 
-	rtc->ops->alarm_irq_enable(rtc->dev.parent, false);
+	rtc->ops->alarm_irq_enable(rtc_ops_dev(rtc), false);
 	trace_rtc_alarm_irq_enable(0, 0);
 }
 
@@ -1076,7 +1076,7 @@ int rtc_read_offset(struct rtc_device *rtc, long *offset)
 		return -EINVAL;
 
 	mutex_lock(&rtc->ops_lock);
-	ret = rtc->ops->read_offset(rtc->dev.parent, offset);
+	ret = rtc->ops->read_offset(rtc_ops_dev(rtc), offset);
 	mutex_unlock(&rtc->ops_lock);
 
 	trace_rtc_read_offset(*offset, ret);
@@ -1111,7 +1111,7 @@ int rtc_set_offset(struct rtc_device *rtc, long offset)
 		return -EINVAL;
 
 	mutex_lock(&rtc->ops_lock);
-	ret = rtc->ops->set_offset(rtc->dev.parent, offset);
+	ret = rtc->ops->set_offset(rtc_ops_dev(rtc), offset);
 	mutex_unlock(&rtc->ops_lock);
 
 	trace_rtc_set_offset(offset, ret);
diff --git a/drivers/rtc/proc.c b/drivers/rtc/proc.c
index cbcdbb19d848e..bf688079d0fbb 100644
--- a/drivers/rtc/proc.c
+++ b/drivers/rtc/proc.c
@@ -73,7 +73,7 @@ static int rtc_proc_show(struct seq_file *seq, void *offset)
 	seq_printf(seq, "24hr\t\t: yes\n");
 
 	if (ops->proc)
-		ops->proc(rtc->dev.parent, seq);
+		ops->proc(rtc_ops_dev(rtc), seq);
 
 	return 0;
 }
diff --git a/include/linux/rtc.h b/include/linux/rtc.h
index 95da051fb155d..1dd4a45d0186e 100644
--- a/include/linux/rtc.h
+++ b/include/linux/rtc.h
@@ -83,6 +83,7 @@ struct rtc_timer {
 /* flags */
 #define RTC_DEV_BUSY 0
 #define RTC_NO_CDEV  1
+#define RTC_OPS_USE_RTC_DEV 2
 
 struct rtc_device {
 	struct device dev;
@@ -167,6 +168,20 @@ struct rtc_device {
 #define rtc_lock(d) mutex_lock(&d->ops_lock)
 #define rtc_unlock(d) mutex_unlock(&d->ops_lock)
 
+/**
+ * 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;
+}
+
 /* useful timestamps */
 #define RTC_TIMESTAMP_BEGIN_0000	-62167219200ULL /* 0000-01-01 00:00:00 */
 #define RTC_TIMESTAMP_BEGIN_1900	-2208988800LL /* 1900-01-01 00:00:00 */
-- 
2.43.0

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

[Ke Sun]

Hi Alexandre,

I have an alternative solution based on our previous v2 discussion [1]. I've
completed modifications for most of the drivers and am currently doing a 
manual
review. If you're interested, I can share it for discussion.

[1] 
https://lore.kernel.org/rust-for-linux/7c6af8a1-9c5e-46b1-8c17-8ffd443fa6aa@kylinos.cn/

Best regards,
Ke Sun

On 1/17/26 00:21, Ke Sun wrote:
> Add rtc_ops_dev() helper to allow drivers to choose whether
> rtc_class_ops callbacks receive rtc->dev or rtc->dev.parent. This
> enables Rust RTC drivers to store driver data on the RTC device.
>
> The helper checks RTC_OPS_USE_RTC_DEV flag: if set, returns &rtc->dev;
> otherwise returns rtc->dev.parent.
>
> Update all rtc_class_ops callback invocations to use rtc_ops_dev(rtc)
> instead of directly accessing rtc->dev.parent.
>
> Maintains backward compatibility for existing C drivers.
>
> Signed-off-by: Ke Sun <sunke@kylinos.cn>
> ---
>   drivers/rtc/dev.c       |  6 +++---
>   drivers/rtc/interface.c | 18 +++++++++---------
>   drivers/rtc/proc.c      |  2 +-
>   include/linux/rtc.h     | 15 +++++++++++++++
>   4 files changed, 28 insertions(+), 13 deletions(-)
>
> 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, &param);
> +				err = rtc->ops->param_get(rtc_ops_dev(rtc), &param);
>   			else
>   				err = -EINVAL;
>   		}
> @@ -440,7 +440,7 @@ static long rtc_dev_ioctl(struct file *file,
>   
>   		default:
>   			if (rtc->ops->param_set)
> -				err = rtc->ops->param_set(rtc->dev.parent, &param);
> +				err = rtc->ops->param_set(rtc_ops_dev(rtc), &param);
>   			else
>   				err = -EINVAL;
>   		}
> @@ -450,7 +450,7 @@ static long rtc_dev_ioctl(struct file *file,
>   	default:
>   		/* Finally try the driver's ioctl interface */
>   		if (ops->ioctl) {
> -			err = ops->ioctl(rtc->dev.parent, cmd, arg);
> +			err = ops->ioctl(rtc_ops_dev(rtc), cmd, arg);
>   			if (err == -ENOIOCTLCMD)
>   				err = -ENOTTY;
>   		} else {
> diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
> index b8b298efd9a9c..4c81130fb0394 100644
> --- a/drivers/rtc/interface.c
> +++ b/drivers/rtc/interface.c
> @@ -91,7 +91,7 @@ static int __rtc_read_time(struct rtc_device *rtc, struct rtc_time *tm)
>   		err = -EINVAL;
>   	} else {
>   		memset(tm, 0, sizeof(struct rtc_time));
> -		err = rtc->ops->read_time(rtc->dev.parent, tm);
> +		err = rtc->ops->read_time(rtc_ops_dev(rtc), tm);
>   		if (err < 0) {
>   			dev_dbg(&rtc->dev, "read_time: fail to read: %d\n",
>   				err);
> @@ -155,7 +155,7 @@ int rtc_set_time(struct rtc_device *rtc, struct rtc_time *tm)
>   	if (!rtc->ops)
>   		err = -ENODEV;
>   	else if (rtc->ops->set_time)
> -		err = rtc->ops->set_time(rtc->dev.parent, tm);
> +		err = rtc->ops->set_time(rtc_ops_dev(rtc), tm);
>   	else
>   		err = -EINVAL;
>   
> @@ -200,7 +200,7 @@ static int rtc_read_alarm_internal(struct rtc_device *rtc,
>   		alarm->time.tm_wday = -1;
>   		alarm->time.tm_yday = -1;
>   		alarm->time.tm_isdst = -1;
> -		err = rtc->ops->read_alarm(rtc->dev.parent, alarm);
> +		err = rtc->ops->read_alarm(rtc_ops_dev(rtc), alarm);
>   	}
>   
>   	mutex_unlock(&rtc->ops_lock);
> @@ -441,7 +441,7 @@ static int __rtc_set_alarm(struct rtc_device *rtc, struct rtc_wkalrm *alarm)
>   	else if (!test_bit(RTC_FEATURE_ALARM, rtc->features))
>   		err = -EINVAL;
>   	else
> -		err = rtc->ops->set_alarm(rtc->dev.parent, alarm);
> +		err = rtc->ops->set_alarm(rtc_ops_dev(rtc), alarm);
>   
>   	/*
>   	 * Check for potential race described above. If the waiting for next
> @@ -568,7 +568,7 @@ int rtc_alarm_irq_enable(struct rtc_device *rtc, unsigned int enabled)
>   	else if (!test_bit(RTC_FEATURE_ALARM, rtc->features) || !rtc->ops->alarm_irq_enable)
>   		err = -EINVAL;
>   	else
> -		err = rtc->ops->alarm_irq_enable(rtc->dev.parent, enabled);
> +		err = rtc->ops->alarm_irq_enable(rtc_ops_dev(rtc), enabled);
>   
>   	mutex_unlock(&rtc->ops_lock);
>   
> @@ -618,7 +618,7 @@ int rtc_update_irq_enable(struct rtc_device *rtc, unsigned int enabled)
>   		rtc->uie_rtctimer.period = ktime_set(1, 0);
>   		err = rtc_timer_enqueue(rtc, &rtc->uie_rtctimer);
>   		if (!err && rtc->ops && rtc->ops->alarm_irq_enable)
> -			err = rtc->ops->alarm_irq_enable(rtc->dev.parent, 1);
> +			err = rtc->ops->alarm_irq_enable(rtc_ops_dev(rtc), 1);
>   		if (err)
>   			goto out;
>   	} else {
> @@ -874,7 +874,7 @@ static void rtc_alarm_disable(struct rtc_device *rtc)
>   	if (!rtc->ops || !test_bit(RTC_FEATURE_ALARM, rtc->features) || !rtc->ops->alarm_irq_enable)
>   		return;
>   
> -	rtc->ops->alarm_irq_enable(rtc->dev.parent, false);
> +	rtc->ops->alarm_irq_enable(rtc_ops_dev(rtc), false);
>   	trace_rtc_alarm_irq_enable(0, 0);
>   }
>   
> @@ -1076,7 +1076,7 @@ int rtc_read_offset(struct rtc_device *rtc, long *offset)
>   		return -EINVAL;
>   
>   	mutex_lock(&rtc->ops_lock);
> -	ret = rtc->ops->read_offset(rtc->dev.parent, offset);
> +	ret = rtc->ops->read_offset(rtc_ops_dev(rtc), offset);
>   	mutex_unlock(&rtc->ops_lock);
>   
>   	trace_rtc_read_offset(*offset, ret);
> @@ -1111,7 +1111,7 @@ int rtc_set_offset(struct rtc_device *rtc, long offset)
>   		return -EINVAL;
>   
>   	mutex_lock(&rtc->ops_lock);
> -	ret = rtc->ops->set_offset(rtc->dev.parent, offset);
> +	ret = rtc->ops->set_offset(rtc_ops_dev(rtc), offset);
>   	mutex_unlock(&rtc->ops_lock);
>   
>   	trace_rtc_set_offset(offset, ret);
> diff --git a/drivers/rtc/proc.c b/drivers/rtc/proc.c
> index cbcdbb19d848e..bf688079d0fbb 100644
> --- a/drivers/rtc/proc.c
> +++ b/drivers/rtc/proc.c
> @@ -73,7 +73,7 @@ static int rtc_proc_show(struct seq_file *seq, void *offset)
>   	seq_printf(seq, "24hr\t\t: yes\n");
>   
>   	if (ops->proc)
> -		ops->proc(rtc->dev.parent, seq);
> +		ops->proc(rtc_ops_dev(rtc), seq);
>   
>   	return 0;
>   }
> diff --git a/include/linux/rtc.h b/include/linux/rtc.h
> index 95da051fb155d..1dd4a45d0186e 100644
> --- a/include/linux/rtc.h
> +++ b/include/linux/rtc.h
> @@ -83,6 +83,7 @@ struct rtc_timer {
>   /* flags */
>   #define RTC_DEV_BUSY 0
>   #define RTC_NO_CDEV  1
> +#define RTC_OPS_USE_RTC_DEV 2
>   
>   struct rtc_device {
>   	struct device dev;
> @@ -167,6 +168,20 @@ struct rtc_device {
>   #define rtc_lock(d) mutex_lock(&d->ops_lock)
>   #define rtc_unlock(d) mutex_unlock(&d->ops_lock)
>   
> +/**
> + * 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;
> +}
> +
>   /* useful timestamps */
>   #define RTC_TIMESTAMP_BEGIN_0000	-62167219200ULL /* 0000-01-01 00:00:00 */
>   #define RTC_TIMESTAMP_BEGIN_1900	-2208988800LL /* 1900-01-01 00:00:00 */

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

[Danilo Krummrich]

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, &param);
> +				err = rtc->ops->param_get(rtc_ops_dev(rtc), &param);
>  			else
>  				err = -EINVAL;
>  		}

<snip>

> +/**
> + * 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.

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

[Ke Sun]

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, &param);
>> +				err = rtc->ops->param_get(rtc_ops_dev(rtc), &param);
>>   			else
>>   				err = -EINVAL;
>>   		}
> <snip>
>
>> +/**
>> + * 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.

Best regard,
Ke Sun

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

[Alexandre Belloni]

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, &param);
> > > +				err = rtc->ops->param_get(rtc_ops_dev(rtc), &param);
> > >   			else
> > >   				err = -EINVAL;
> > >   		}
> > <snip>
> > 
> > > +/**
> > > + * 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() ?
Out of 29 drivers, 18 are doing so.

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

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

[Alvin Sun]

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, &param);
>>>> +				err = rtc->ops->param_get(rtc_ops_dev(rtc), &param);
>>>>    			else
>>>>    				err = -EINVAL;
>>>>    		}
>>> <snip>
>>>
>>>> +/**
>>>> + * 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.
>

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

[Alexandre Belloni]

On 21/02/2026 17:31:09+0800, Alvin Sun wrote:
> 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.

But this is wrong, how do you then handle the class device on
suspend/resume or on .remove?

> 
> 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.
> > 

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

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

[Rafael J. Wysocki]

On Sat, Feb 21, 2026 at 12:16 PM Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
>
> On 21/02/2026 17:31:09+0800, Alvin Sun wrote:
> > 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.
>
> But this is wrong, how do you then handle the class device on
> suspend/resume or on .remove?
>
> >
> > 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.

+1

The vast majority of around 50 platform drivers I've inspected
recently use platform_set_drvdata() or equivalent in probe.

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

[Danilo Krummrich]

On Sat Feb 21, 2026 at 12:19 PM CET, Rafael J. Wysocki wrote:
> On Sat, Feb 21, 2026 at 12:16 PM Alexandre Belloni
> <alexandre.belloni@bootlin.com> wrote:
>> > > Out of 29 drivers, 18 are doing so.
>
> The vast majority of around 50 platform drivers I've inspected
> recently use platform_set_drvdata() or equivalent in probe.

This thread seems to contain quite a bit of confusion and misunderstandings --
let me try to clarify.

  (1) How Rust handles bus device private data.

  In Rust the probe() function of a bus implementation (platform, PCI, etc.)
  returns an initializer (impl PinInit<T, Error>) for the driver's device
  private data.

  The bus implementation takes this initializer and passes it (together with the
  underlying struct device) to the driver-core. The driver-core allocates the
  required memory, initializes the memory with the given initializer and stores
  a pointer to the corresponding object with dev_set_drvdata().

  So, technically, in Rust all platform drivers call platform_set_drvdata().

  (Note that this is also true when the driver's device private data type is
  empty (i.e. it has no fields). In this case it could still have a destructor
  that must be called when the device private data structure is destroyed. Of
  course there is no real memory allocation when the struct's size is zero.)

  The driver's device private data can only be accessed when the bus device is
  bound to the driver, i.e. the driver can only access it with a &Device<Bound>;
  it (the driver's device private data) is automatically freed by the
  driver-core when remove() and all devres callbacks have been completed.

  I.e. the rules are - of course - the same as on the C side, but they are
  enforced by the type system and the driver-core code.

  (2) Bus device private data vs. class device private data.

  The change to pass a struct rtc_device in class device callbacks of RTC,
  rather than the base struct device of the corresponding bus device (e.g. AMBA,
  platform, etc.) should not aim at storing all data in rtc->dev.private_data
  that was previously stored in rtc->dev.parent->private_data.

  Instead, it gives drivers the option to differentiate in terms of ownership
  and lifetime.

  While the bus device private data has a very defined lifetime from probe()
  until the device is unbound from the driver, class device private data might
  live shorter than this, or might even out-live driver unbind in some cases. It
  really depends on the lifetime of the class device itself, which is not
  generally defined.

  Now, from a C side point of view this may not look like a big deal, as it
  (unfortunately) is not that uncommon that struct fields are just initialized
  and destroyed whenever needed and the code just takes it into account.

  But at the same time, this is what leads to a lot of lifetime problems and
  memory bugs and it is one of those things that Rust aims at avoiding by being
  very strict about initialization, ownership and lifetimes.

  However, I do also recognize that drivers creating an RTC device are typically
  very simple and in practice I would not be surprised if it turns out that it
  happens that drivers keep the struct rtc_device alive from probe() until the
  bus device is unbound from the driver, i.e. lifetimes just end up being almost
  the same. But I don't know if that's always the case.

  Regardless of that, I think it would be good to keep driver authors finding a
  common pattern, where class device callbacks carry the corresponding class
  device struct (instead of the parent base struct device).

  Especially on the Rust side we now have the chance to make the experience of
  writing drivers as consistent as possible, which should help (new) driver
  authors a lot in terms of learning the driver lifetime patterns.

I hope this helps.

- Danilo

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

[Alexandre Belloni]

On 21/02/2026 15:33:48+0100, Danilo Krummrich wrote:
> On Sat Feb 21, 2026 at 12:19 PM CET, Rafael J. Wysocki wrote:
> > On Sat, Feb 21, 2026 at 12:16 PM Alexandre Belloni
> > <alexandre.belloni@bootlin.com> wrote:
> >> > > Out of 29 drivers, 18 are doing so.
> >
> > The vast majority of around 50 platform drivers I've inspected
> > recently use platform_set_drvdata() or equivalent in probe.
> 
> This thread seems to contain quite a bit of confusion and misunderstandings --
> let me try to clarify.
> 
>   (1) How Rust handles bus device private data.
> 
>   In Rust the probe() function of a bus implementation (platform, PCI, etc.)
>   returns an initializer (impl PinInit<T, Error>) for the driver's device
>   private data.
> 
>   The bus implementation takes this initializer and passes it (together with the
>   underlying struct device) to the driver-core. The driver-core allocates the
>   required memory, initializes the memory with the given initializer and stores
>   a pointer to the corresponding object with dev_set_drvdata().
> 
>   So, technically, in Rust all platform drivers call platform_set_drvdata().
> 
>   (Note that this is also true when the driver's device private data type is
>   empty (i.e. it has no fields). In this case it could still have a destructor
>   that must be called when the device private data structure is destroyed. Of
>   course there is no real memory allocation when the struct's size is zero.)
> 
>   The driver's device private data can only be accessed when the bus device is
>   bound to the driver, i.e. the driver can only access it with a &Device<Bound>;
>   it (the driver's device private data) is automatically freed by the
>   driver-core when remove() and all devres callbacks have been completed.
> 
>   I.e. the rules are - of course - the same as on the C side, but they are
>   enforced by the type system and the driver-core code.
> 

This still doesn't explain how you get the class private data that you
need when you are in a driver callback that is called from the bus (e.g.
suspend/resume) from what you explain, the driver doesn't have any
chance to pass it. The whole goal of a device driver is to be the glue
between a class device and a bus device as essentially this is the exact
physical device, just represented differently.

An i2c RTC is not either an i2c device or an RTC, it is a single device
and we need to be able to tell the framework when something happens on
the bus or we need to be able to talk on the bus when we want to do
something with the device.


>   (2) Bus device private data vs. class device private data.
> 
>   The change to pass a struct rtc_device in class device callbacks of RTC,
>   rather than the base struct device of the corresponding bus device (e.g. AMBA,
>   platform, etc.) should not aim at storing all data in rtc->dev.private_data
>   that was previously stored in rtc->dev.parent->private_data.
> 

But what you explain here is that the drive is forbidden to use
rtc->dev.parent->private_data at all because the rust core is already
using it. What I'm saying is that it won't work because more than half
of the drivers currently need it.

I get that you are trying to avoid most of the issues by using devres
but I'm pretty sure there are cases where this doesn't work.

>   Instead, it gives drivers the option to differentiate in terms of ownership
>   and lifetime.
> 
>   While the bus device private data has a very defined lifetime from probe()
>   until the device is unbound from the driver, class device private data might
>   live shorter than this, or might even out-live driver unbind in some cases. It
>   really depends on the lifetime of the class device itself, which is not
>   generally defined.
> 
>   Now, from a C side point of view this may not look like a big deal, as it
>   (unfortunately) is not that uncommon that struct fields are just initialized
>   and destroyed whenever needed and the code just takes it into account.
> 
>   But at the same time, this is what leads to a lot of lifetime problems and
>   memory bugs and it is one of those things that Rust aims at avoiding by being
>   very strict about initialization, ownership and lifetimes.
> 
>   However, I do also recognize that drivers creating an RTC device are typically
>   very simple and in practice I would not be surprised if it turns out that it
>   happens that drivers keep the struct rtc_device alive from probe() until the
>   bus device is unbound from the driver, i.e. lifetimes just end up being almost
>   the same. But I don't know if that's always the case.
> 
>   Regardless of that, I think it would be good to keep driver authors finding a
>   common pattern, where class device callbacks carry the corresponding class
>   device struct (instead of the parent base struct device).
> 
>   Especially on the Rust side we now have the chance to make the experience of
>   writing drivers as consistent as possible, which should help (new) driver
>   authors a lot in terms of learning the driver lifetime patterns.
> 

The whole point of the RTC is to outlive the system but as Linux can't do
anything with the RTC unless it has a bus device to talk to, the class
device and the bus device have the exact same lifetime and we do handle
device appearing and disappearing from the bus. You may think RTC
drivers afe simple but there are plenty of lifetime issues that you will
never get with any other devices because they simply die with the
system.


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

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

[Danilo Krummrich]

On Sun Feb 22, 2026 at 1:05 AM CET, Alexandre Belloni wrote:
> On 21/02/2026 15:33:48+0100, Danilo Krummrich wrote:
>>   (2) Bus device private data vs. class device private data.
>> 
>>   The change to pass a struct rtc_device in class device callbacks of RTC,
>>   rather than the base struct device of the corresponding bus device (e.g. AMBA,
>>   platform, etc.) should not aim at storing all data in rtc->dev.private_data
>>   that was previously stored in rtc->dev.parent->private_data.
>> 
>
> But what you explain here is that the drive is forbidden to use
> rtc->dev.parent->private_data at all because the rust core is already
> using it. What I'm saying is that it won't work because more than half
> of the drivers currently need it.

I think I was explaining the exact opposite, i.e. the driver's bus device
private data is stored in the exact same way as in C, but it has a defined
lifetime (from probe() until remove() and all devres callbacks have been
completed) and is managed by the driver-core.

Look at this example:

	// The bus device private data.
	struct SampleDriver {
	   foo: u32,
	   bar: u32,
	}
	
	impl pci::Driver for SampleDriver {
	    fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> impl PinInit<Self, Error> {
	        // Return the driver's bus device private data. The driver-core
	        // will do all the work for us and eventually call
	        // `dev_set_drvdata()` on the base `struct device` of `pdev`.
	        Ok(Foo {
	            foo: 42,
	            bar: 24,
	        })
	    }
	
	    fn unbind(pdev: &pci::Device<Core>, self: Pin<&Self>) {
	        // Use the driver's device private data from the method's `self` argument.
	        dev_info!(pdev, "foo: {}, bar: {}\n", self.foo, self.bar);
	
	        // There's also the `Device<Bound>::drvdata()` accessor, let's
	        // use that as well.
	
	        // Get a generic `device::Device` from the `pci::Device`.
	        let dev = pdev.as_ref();
	
	        // Get the `drvdata`; we have to assert the type and let the driver-core
	        // validate that the asserted type is correct.
	        let drvdata = dev.drvdata::<SampleDriver>()?;
	
	        dev_info!(pdev, "foo: {}, bar: {}\n", drvdata.foo, drvdata.bar);
	    }
	}

>> This thread seems to contain quite a bit of confusion and misunderstandings --
>> let me try to clarify.
>> 
>>   (1) How Rust handles bus device private data.
>> 
>>   In Rust the probe() function of a bus implementation (platform, PCI, etc.)
>>   returns an initializer (impl PinInit<T, Error>) for the driver's device
>>   private data.
>> 
>>   The bus implementation takes this initializer and passes it (together with the
>>   underlying struct device) to the driver-core. The driver-core allocates the
>>   required memory, initializes the memory with the given initializer and stores
>>   a pointer to the corresponding object with dev_set_drvdata().
>> 
>>   So, technically, in Rust all platform drivers call platform_set_drvdata().
>> 
>>   (Note that this is also true when the driver's device private data type is
>>   empty (i.e. it has no fields). In this case it could still have a destructor
>>   that must be called when the device private data structure is destroyed. Of
>>   course there is no real memory allocation when the struct's size is zero.)
>> 
>>   The driver's device private data can only be accessed when the bus device is
>>   bound to the driver, i.e. the driver can only access it with a &Device<Bound>;
>>   it (the driver's device private data) is automatically freed by the
>>   driver-core when remove() and all devres callbacks have been completed.
>> 
>>   I.e. the rules are - of course - the same as on the C side, but they are
>>   enforced by the type system and the driver-core code.
>> 
>
> This still doesn't explain how you get the class private data that you
> need when you are in a driver callback that is called from the bus (e.g.
> suspend/resume) from what you explain, the driver doesn't have any
> chance to pass it. The whole goal of a device driver is to be the glue
> between a class device and a bus device as essentially this is the exact
> physical device, just represented differently.

It is not always as simple as "one bus device corresponds to one class device".

Sometimes drivers have to deal with multiple class devices for a single bus
device, sometimes they are separated through the auxiliary bus or MFD.

For instance, take struct net_device. In the context of cfg80211 wireless
drivers may create arbitrary struct net_device instances, depending on how often
the add_virtual_intf() callback is called (through netlink); example in [1].

Now, regarding your question "How to access class device private data from bus
device callbacks?". Nothing prevents a driver from embedding the class device in
its bus device private data in one or the other way.

In the net device example above, the driver would probably keep a list (or
xarray, etc.) of net devices in its bus device private data, as they can be
created and removed at any point of time.

(Note that this is also a good example for when the class device private data
lives shorter than the bus device private data.)

Here is an example of this looks like in code using a DRM device as class
device.

	struct SampleIrqData;

	// The class device private data.
	struct SampleDrm {
	    sched: drm::GpuScheduler,
	}

	// The bus device private data.
	#[pin_data]
	struct SampleDriver {
	    #[pin]
	    irq: irq::Registration<SampleIrqData>,
	    drm: ARef<drm::Device<SampleDrm>, // Refcount of a `drm::Device`.
	}
	
	impl pci::Driver for SampleDriver {
	    fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> impl PinInit<Self, Error> {
	        let dev = pdev.as_ref();

	        let drm = drm::Device::new(dev, SampleDrm::new()?)?;
	        drm.register();

	        Ok(impl_pin_init!(Self {
	            irq <- irq::Registration::new(...);
	            drm,
	        })
	    }
	
	    fn unbind(pdev: &pci::Device<Core>, self: Pin<&Self>) {
	        self.irq.synchronize();
	        self.drm.flush_scheduler();
	    }
	}

Note that the irq::Registration is dropped when the bus device is unbound,
whereas the GpuScheduler is dropped when the DRM device is dropped.

Of course, depending on the actual class device this difference may be more or
less important, i.e. absolutely crucial in the net device example, less
important for RTC apparently. But it is always good to be precise about
ownership and lifetime of data.

As I already mentioned, I don't think it is good to fold ownership and lifetime
for bus and class devices together in cases where it happens that they are
mostly identical, i.e.:

>>   I think it would be good to keep driver authors finding a common pattern,
>>   where class device callbacks carry the corresponding class device struct
>>   (instead of the parent base struct device).
>>
>>   Especially on the Rust side we now have the chance to make the experience
>>   of writing drivers as consistent as possible, which should help (new)
>>   driver authors a lot in terms of learning the driver lifetime patterns.

[1] https://elixir.bootlin.com/linux/v6.19.2/source/drivers/net/wireless/microchip/wilc1000/netdev.c#L948

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

[Rafael J. Wysocki]

On Sun, Feb 22, 2026 at 1:49 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Sun Feb 22, 2026 at 1:05 AM CET, Alexandre Belloni wrote:
> > On 21/02/2026 15:33:48+0100, Danilo Krummrich wrote:
> >>   (2) Bus device private data vs. class device private data.
> >>
> >>   The change to pass a struct rtc_device in class device callbacks of RTC,
> >>   rather than the base struct device of the corresponding bus device (e.g. AMBA,
> >>   platform, etc.) should not aim at storing all data in rtc->dev.private_data
> >>   that was previously stored in rtc->dev.parent->private_data.
> >>
> >
> > But what you explain here is that the drive is forbidden to use
> > rtc->dev.parent->private_data at all because the rust core is already
> > using it. What I'm saying is that it won't work because more than half
> > of the drivers currently need it.
>
> I think I was explaining the exact opposite, i.e. the driver's bus device
> private data is stored in the exact same way as in C, but it has a defined
> lifetime (from probe() until remove() and all devres callbacks have been
> completed) and is managed by the driver-core.
>
> Look at this example:
>
>         // The bus device private data.
>         struct SampleDriver {
>            foo: u32,
>            bar: u32,
>         }
>
>         impl pci::Driver for SampleDriver {
>             fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> impl PinInit<Self, Error> {
>                 // Return the driver's bus device private data. The driver-core
>                 // will do all the work for us and eventually call
>                 // `dev_set_drvdata()` on the base `struct device` of `pdev`.

So that's how I thought it would work.

>                 Ok(Foo {
>                     foo: 42,
>                     bar: 24,
>                 })
>             }
>
>             fn unbind(pdev: &pci::Device<Core>, self: Pin<&Self>) {
>                 // Use the driver's device private data from the method's `self` argument.
>                 dev_info!(pdev, "foo: {}, bar: {}\n", self.foo, self.bar);
>
>                 // There's also the `Device<Bound>::drvdata()` accessor, let's
>                 // use that as well.
>
>                 // Get a generic `device::Device` from the `pci::Device`.
>                 let dev = pdev.as_ref();
>
>                 // Get the `drvdata`; we have to assert the type and let the driver-core
>                 // validate that the asserted type is correct.
>                 let drvdata = dev.drvdata::<SampleDriver>()?;
>
>                 dev_info!(pdev, "foo: {}, bar: {}\n", drvdata.foo, drvdata.bar);
>             }
>         }

This helps, thanks!

> >> This thread seems to contain quite a bit of confusion and misunderstandings --
> >> let me try to clarify.
> >>
> >>   (1) How Rust handles bus device private data.
> >>
> >>   In Rust the probe() function of a bus implementation (platform, PCI, etc.)
> >>   returns an initializer (impl PinInit<T, Error>) for the driver's device
> >>   private data.
> >>
> >>   The bus implementation takes this initializer and passes it (together with the
> >>   underlying struct device) to the driver-core. The driver-core allocates the
> >>   required memory, initializes the memory with the given initializer and stores
> >>   a pointer to the corresponding object with dev_set_drvdata().
> >>
> >>   So, technically, in Rust all platform drivers call platform_set_drvdata().
> >>
> >>   (Note that this is also true when the driver's device private data type is
> >>   empty (i.e. it has no fields). In this case it could still have a destructor
> >>   that must be called when the device private data structure is destroyed. Of
> >>   course there is no real memory allocation when the struct's size is zero.)
> >>
> >>   The driver's device private data can only be accessed when the bus device is
> >>   bound to the driver, i.e. the driver can only access it with a &Device<Bound>;
> >>   it (the driver's device private data) is automatically freed by the
> >>   driver-core when remove() and all devres callbacks have been completed.
> >>
> >>   I.e. the rules are - of course - the same as on the C side, but they are
> >>   enforced by the type system and the driver-core code.
> >>
> >
> > This still doesn't explain how you get the class private data that you
> > need when you are in a driver callback that is called from the bus (e.g.
> > suspend/resume) from what you explain, the driver doesn't have any
> > chance to pass it. The whole goal of a device driver is to be the glue
> > between a class device and a bus device as essentially this is the exact
> > physical device, just represented differently.
>
> It is not always as simple as "one bus device corresponds to one class device".
>
> Sometimes drivers have to deal with multiple class devices for a single bus
> device, sometimes they are separated through the auxiliary bus or MFD.
>
> For instance, take struct net_device. In the context of cfg80211 wireless
> drivers may create arbitrary struct net_device instances, depending on how often
> the add_virtual_intf() callback is called (through netlink); example in [1].
>
> Now, regarding your question "How to access class device private data from bus
> device callbacks?". Nothing prevents a driver from embedding the class device in
> its bus device private data in one or the other way.
>
> In the net device example above, the driver would probably keep a list (or
> xarray, etc.) of net devices in its bus device private data, as they can be
> created and removed at any point of time.
>
> (Note that this is also a good example for when the class device private data
> lives shorter than the bus device private data.)
>
> Here is an example of this looks like in code using a DRM device as class
> device.
>
>         struct SampleIrqData;
>
>         // The class device private data.
>         struct SampleDrm {
>             sched: drm::GpuScheduler,
>         }
>
>         // The bus device private data.
>         #[pin_data]
>         struct SampleDriver {
>             #[pin]
>             irq: irq::Registration<SampleIrqData>,
>             drm: ARef<drm::Device<SampleDrm>, // Refcount of a `drm::Device`.
>         }
>
>         impl pci::Driver for SampleDriver {
>             fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> impl PinInit<Self, Error> {
>                 let dev = pdev.as_ref();
>
>                 let drm = drm::Device::new(dev, SampleDrm::new()?)?;
>                 drm.register();
>
>                 Ok(impl_pin_init!(Self {
>                     irq <- irq::Registration::new(...);
>                     drm,
>                 })
>             }
>
>             fn unbind(pdev: &pci::Device<Core>, self: Pin<&Self>) {
>                 self.irq.synchronize();
>                 self.drm.flush_scheduler();
>             }
>         }
>
> Note that the irq::Registration is dropped when the bus device is unbound,
> whereas the GpuScheduler is dropped when the DRM device is dropped.
>
> Of course, depending on the actual class device this difference may be more or
> less important, i.e. absolutely crucial in the net device example, less
> important for RTC apparently. But it is always good to be precise about
> ownership and lifetime of data.
>
> As I already mentioned, I don't think it is good to fold ownership and lifetime
> for bus and class devices together in cases where it happens that they are
> mostly identical, i.e.:
>
> >>   I think it would be good to keep driver authors finding a common pattern,
> >>   where class device callbacks carry the corresponding class device struct
> >>   (instead of the parent base struct device).
> >>
> >>   Especially on the Rust side we now have the chance to make the experience
> >>   of writing drivers as consistent as possible, which should help (new)
> >>   driver authors a lot in terms of learning the driver lifetime patterns.
>
> [1] https://elixir.bootlin.com/linux/v6.19.2/source/drivers/net/wireless/microchip/wilc1000/netdev.c#L948

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?

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

[Danilo Krummrich]

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/

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

[Danilo Krummrich]

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 this,
since it actually does not work perfectly fine without further changes that
would (re-)introduce ordering constraints for drivers on the probe() side of
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 = platform::Device<Bound>;
>
> 	    fn read_time(
> 	        parent: &platform::Device<Bound>,
> 	        time: &mut rtc::Time,
> 	    ) -> Result {
> 	        let drvdata = pdev.as_ref().drvdata::<MyDriver>()?;
>
> 	        ...
> 	    }
> 	}

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<SampleIrqData>,
	    rtc: ARef<rtc::Device>,
	    ...,
	}

	impl pci::Driver for SampleDriver {
	    fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> impl PinInit<Self, Error> {
	        let dev = pdev.as_ref();

	        let rtc = 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> type,
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<SampleIrqData>,
	    rtc: ARef<rtc::Device>,
	    ...,
	}

	impl pci::Driver for SampleDriver {
	    fn probe<'a>(pdev: pci::ProbeDevice<'a>, info: &Self::IdInfo) -> Result {
	        let dev = pdev.as_ref();

	        let rtc = rtc::Device::new(dev)?;

	        let data = impl_pin_init!(Self {
	            irq <- irq::Registration::new(...),
	            rtc,
	        });

	        // The `pci::ProbeDevice` has been consumed by value, and
	        // returned a `&pci::Device<Core>`.
	        let pdev = pdev.set_drvdata(data)?;

	        devres::register(rtc::Registration::new(rtc))
	    }
	}

However, note how this wouldn't solve the same problem for irq::Registration, 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 safety
reasons.

We could probably find other ways to somehow handle this, e.g. by introducing
new device context states, additional callbacks, etc., but in the end it would
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 this
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 operations,
timers, etc.

I.e. there is a clear separation of ownership and lifetime, which makes more or
less (often more) suble ordering constraints obsolete.

In fact, in the beginning I did not even plan to expose Device::drvdata() at
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 already
          provide access to the driver's device private data through a &self
          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<Bound>::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 the
    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<rtc::Device>,
>	};
>
> 	// The bus device private data.
> 	#[pin_data]
> 	struct SampleDriver {
> 	    #[pin]
> 	    irq: irq::Registration<SampleIrqHandler>,
> 	    rtc: ARef<rtc::Device<SampleRtcData>>,
> 	}
>
> 	// The class device private data.
> 	struct SampleRtcData {
> 	    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<SampleRtcData>
> 	        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 => { ... },
> 	        }
> 	    }
> 	}
>

	impl pci::Driver for SampleDriver {
	    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: iomap_region_sized::<BAR0_SIZE>(0, c"my_rtc/bar0")?,
	            hw_variant: VendorVariant::StV1,
	        });

	        let rtc = 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 not
compile in the first place, which is a huge improvement over the situation we
have in C.)

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

[Rafael J. Wysocki]

On Tue, Feb 24, 2026 at 1:12 AM Danilo Krummrich <dakr@kernel.org> wrote:
>
> 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 this,
> since it actually does not work perfectly fine without further changes that
> would (re-)introduce ordering constraints for drivers on the probe() side of
> 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 = platform::Device<Bound>;
> >
> >           fn read_time(
> >               parent: &platform::Device<Bound>,
> >               time: &mut rtc::Time,
> >           ) -> Result {
> >               let drvdata = pdev.as_ref().drvdata::<MyDriver>()?;
> >
> >               ...
> >           }
> >       }
>
> 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<SampleIrqData>,
>             rtc: ARef<rtc::Device>,
>             ...,
>         }
>
>         impl pci::Driver for SampleDriver {
>             fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> impl PinInit<Self, Error> {
>                 let dev = pdev.as_ref();
>
>                 let rtc = 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> type,
> 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<SampleIrqData>,
>             rtc: ARef<rtc::Device>,
>             ...,
>         }
>
>         impl pci::Driver for SampleDriver {
>             fn probe<'a>(pdev: pci::ProbeDevice<'a>, info: &Self::IdInfo) -> Result {
>                 let dev = pdev.as_ref();
>
>                 let rtc = rtc::Device::new(dev)?;
>
>                 let data = impl_pin_init!(Self {
>                     irq <- irq::Registration::new(...),
>                     rtc,
>                 });
>
>                 // The `pci::ProbeDevice` has been consumed by value, and
>                 // returned a `&pci::Device<Core>`.
>                 let pdev = pdev.set_drvdata(data)?;
>
>                 devres::register(rtc::Registration::new(rtc))
>             }
>         }
>
> However, note how this wouldn't solve the same problem for irq::Registration, 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 safety
> reasons.
>
> We could probably find other ways to somehow handle this, e.g. by introducing
> new device context states, additional callbacks, etc., but in the end it would
> 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 this
> 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 operations,
> timers, etc.
>
> I.e. there is a clear separation of ownership and lifetime, which makes more or
> less (often more) suble ordering constraints obsolete.
>
> In fact, in the beginning I did not even plan to expose Device::drvdata() at
> 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 already
>           provide access to the driver's device private data through a &self
>           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<Bound>::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 the
>     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<rtc::Device>,
> >       };
> >
> >       // The bus device private data.
> >       #[pin_data]
> >       struct SampleDriver {
> >           #[pin]
> >           irq: irq::Registration<SampleIrqHandler>,
> >           rtc: ARef<rtc::Device<SampleRtcData>>,
> >       }
> >
> >       // The class device private data.
> >       struct SampleRtcData {
> >           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<SampleRtcData>
> >               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 => { ... },
> >               }
> >           }
> >       }
> >
>
>         impl pci::Driver for SampleDriver {
>             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: iomap_region_sized::<BAR0_SIZE>(0, c"my_rtc/bar0")?,
>                     hw_variant: VendorVariant::StV1,
>                 });
>
>                 let rtc = 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 not
> compile in the first place, which is a huge improvement over the situation we
> have in C.)

Thanks for all of this information and let me confirm my understanding.

In both Example 1 and Example 3 there is a dependency between rtc's
read_time() callback and the PCI driver probe() because calling the
former before the latter is complete would be premature (and it would
fail).  However, in Example 1 the compiler does not take that
dependency into account because rtc is not passed to the callback,
while in Example 3 the dependency is visible to the compiler and it
will refuse to compile the code if the dependency may be missed.

If the above is correct, the current calling convention of RTC class
callbacks is problematic because it causes correctness checks done by
the Rust compiler to be bypassed (which may allow certain ordering
issues to be missed and functional problems to appear going forward).

That's convincing and I think that adding an RTC device pointer to the
list of parameters of RTC class callbacks on the C side would not be a
very intrusive change.  Arguably, it would be less intrusive than
replacing the parent device pointer with the RTC device pointer in
them.

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

[Alexandre Belloni]

On 24/02/2026 14:28:48+0100, Rafael J. Wysocki wrote:
> On Tue, Feb 24, 2026 at 1:12 AM Danilo Krummrich <dakr@kernel.org> wrote:
> >
> > 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 this,
> > since it actually does not work perfectly fine without further changes that
> > would (re-)introduce ordering constraints for drivers on the probe() side of
> > 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 = platform::Device<Bound>;
> > >
> > >           fn read_time(
> > >               parent: &platform::Device<Bound>,
> > >               time: &mut rtc::Time,
> > >           ) -> Result {
> > >               let drvdata = pdev.as_ref().drvdata::<MyDriver>()?;
> > >
> > >               ...
> > >           }
> > >       }
> >
> > 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<SampleIrqData>,
> >             rtc: ARef<rtc::Device>,
> >             ...,
> >         }
> >
> >         impl pci::Driver for SampleDriver {
> >             fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> impl PinInit<Self, Error> {
> >                 let dev = pdev.as_ref();
> >
> >                 let rtc = 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> type,
> > 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<SampleIrqData>,
> >             rtc: ARef<rtc::Device>,
> >             ...,
> >         }
> >
> >         impl pci::Driver for SampleDriver {
> >             fn probe<'a>(pdev: pci::ProbeDevice<'a>, info: &Self::IdInfo) -> Result {
> >                 let dev = pdev.as_ref();
> >
> >                 let rtc = rtc::Device::new(dev)?;
> >
> >                 let data = impl_pin_init!(Self {
> >                     irq <- irq::Registration::new(...),
> >                     rtc,
> >                 });
> >
> >                 // The `pci::ProbeDevice` has been consumed by value, and
> >                 // returned a `&pci::Device<Core>`.
> >                 let pdev = pdev.set_drvdata(data)?;
> >
> >                 devres::register(rtc::Registration::new(rtc))
> >             }
> >         }
> >
> > However, note how this wouldn't solve the same problem for irq::Registration, 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 safety
> > reasons.
> >
> > We could probably find other ways to somehow handle this, e.g. by introducing
> > new device context states, additional callbacks, etc., but in the end it would
> > 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 this
> > 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 operations,
> > timers, etc.
> >
> > I.e. there is a clear separation of ownership and lifetime, which makes more or
> > less (often more) suble ordering constraints obsolete.
> >
> > In fact, in the beginning I did not even plan to expose Device::drvdata() at
> > 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 already
> >           provide access to the driver's device private data through a &self
> >           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<Bound>::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 the
> >     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<rtc::Device>,
> > >       };
> > >
> > >       // The bus device private data.
> > >       #[pin_data]
> > >       struct SampleDriver {
> > >           #[pin]
> > >           irq: irq::Registration<SampleIrqHandler>,
> > >           rtc: ARef<rtc::Device<SampleRtcData>>,
> > >       }
> > >
> > >       // The class device private data.
> > >       struct SampleRtcData {
> > >           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<SampleRtcData>
> > >               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 => { ... },
> > >               }
> > >           }
> > >       }
> > >
> >
> >         impl pci::Driver for SampleDriver {
> >             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: iomap_region_sized::<BAR0_SIZE>(0, c"my_rtc/bar0")?,
> >                     hw_variant: VendorVariant::StV1,
> >                 });
> >
> >                 let rtc = 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 not
> > compile in the first place, which is a huge improvement over the situation we
> > have in C.)
> 
> Thanks for all of this information and let me confirm my understanding.
> 
> In both Example 1 and Example 3 there is a dependency between rtc's
> read_time() callback and the PCI driver probe() because calling the
> former before the latter is complete would be premature (and it would
> fail).  However, in Example 1 the compiler does not take that
> dependency into account because rtc is not passed to the callback,
> while in Example 3 the dependency is visible to the compiler and it
> will refuse to compile the code if the dependency may be missed.
> 
> If the above is correct, the current calling convention of RTC class
> callbacks is problematic because it causes correctness checks done by
> the Rust compiler to be bypassed (which may allow certain ordering
> issues to be missed and functional problems to appear going forward).
> 
> That's convincing and I think that adding an RTC device pointer to the
> list of parameters of RTC class callbacks on the C side would not be a
> very intrusive change.  Arguably, it would be less intrusive than
> replacing the parent device pointer with the RTC device pointer in
> them.

But there is no way the rtc callbacks will be called before the rtc is
registered which will only happen in probe, when we have an actual
device to talk to...

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

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

[Rafael J. Wysocki]

On Tue, Feb 24, 2026 at 3:57 PM Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
>
> On 24/02/2026 14:28:48+0100, Rafael J. Wysocki wrote:
> > On Tue, Feb 24, 2026 at 1:12 AM Danilo Krummrich <dakr@kernel.org> wrote:
> > >
> > > 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 this,
> > > since it actually does not work perfectly fine without further changes that
> > > would (re-)introduce ordering constraints for drivers on the probe() side of
> > > 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 = platform::Device<Bound>;
> > > >
> > > >           fn read_time(
> > > >               parent: &platform::Device<Bound>,
> > > >               time: &mut rtc::Time,
> > > >           ) -> Result {
> > > >               let drvdata = pdev.as_ref().drvdata::<MyDriver>()?;
> > > >
> > > >               ...
> > > >           }
> > > >       }
> > >
> > > 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<SampleIrqData>,
> > >             rtc: ARef<rtc::Device>,
> > >             ...,
> > >         }
> > >
> > >         impl pci::Driver for SampleDriver {
> > >             fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> impl PinInit<Self, Error> {
> > >                 let dev = pdev.as_ref();
> > >
> > >                 let rtc = 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> type,
> > > 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<SampleIrqData>,
> > >             rtc: ARef<rtc::Device>,
> > >             ...,
> > >         }
> > >
> > >         impl pci::Driver for SampleDriver {
> > >             fn probe<'a>(pdev: pci::ProbeDevice<'a>, info: &Self::IdInfo) -> Result {
> > >                 let dev = pdev.as_ref();
> > >
> > >                 let rtc = rtc::Device::new(dev)?;
> > >
> > >                 let data = impl_pin_init!(Self {
> > >                     irq <- irq::Registration::new(...),
> > >                     rtc,
> > >                 });
> > >
> > >                 // The `pci::ProbeDevice` has been consumed by value, and
> > >                 // returned a `&pci::Device<Core>`.
> > >                 let pdev = pdev.set_drvdata(data)?;
> > >
> > >                 devres::register(rtc::Registration::new(rtc))
> > >             }
> > >         }
> > >
> > > However, note how this wouldn't solve the same problem for irq::Registration, 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 safety
> > > reasons.
> > >
> > > We could probably find other ways to somehow handle this, e.g. by introducing
> > > new device context states, additional callbacks, etc., but in the end it would
> > > 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 this
> > > 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 operations,
> > > timers, etc.
> > >
> > > I.e. there is a clear separation of ownership and lifetime, which makes more or
> > > less (often more) suble ordering constraints obsolete.
> > >
> > > In fact, in the beginning I did not even plan to expose Device::drvdata() at
> > > 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 already
> > >           provide access to the driver's device private data through a &self
> > >           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<Bound>::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 the
> > >     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<rtc::Device>,
> > > >       };
> > > >
> > > >       // The bus device private data.
> > > >       #[pin_data]
> > > >       struct SampleDriver {
> > > >           #[pin]
> > > >           irq: irq::Registration<SampleIrqHandler>,
> > > >           rtc: ARef<rtc::Device<SampleRtcData>>,
> > > >       }
> > > >
> > > >       // The class device private data.
> > > >       struct SampleRtcData {
> > > >           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<SampleRtcData>
> > > >               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 => { ... },
> > > >               }
> > > >           }
> > > >       }
> > > >
> > >
> > >         impl pci::Driver for SampleDriver {
> > >             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: iomap_region_sized::<BAR0_SIZE>(0, c"my_rtc/bar0")?,
> > >                     hw_variant: VendorVariant::StV1,
> > >                 });
> > >
> > >                 let rtc = 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 not
> > > compile in the first place, which is a huge improvement over the situation we
> > > have in C.)
> >
> > Thanks for all of this information and let me confirm my understanding.
> >
> > In both Example 1 and Example 3 there is a dependency between rtc's
> > read_time() callback and the PCI driver probe() because calling the
> > former before the latter is complete would be premature (and it would
> > fail).  However, in Example 1 the compiler does not take that
> > dependency into account because rtc is not passed to the callback,
> > while in Example 3 the dependency is visible to the compiler and it
> > will refuse to compile the code if the dependency may be missed.
> >
> > If the above is correct, the current calling convention of RTC class
> > callbacks is problematic because it causes correctness checks done by
> > the Rust compiler to be bypassed (which may allow certain ordering
> > issues to be missed and functional problems to appear going forward).
> >
> > That's convincing and I think that adding an RTC device pointer to the
> > list of parameters of RTC class callbacks on the C side would not be a
> > very intrusive change.  Arguably, it would be less intrusive than
> > replacing the parent device pointer with the RTC device pointer in
> > them.
>
> But there is no way the rtc callbacks will be called before the rtc is
> registered which will only happen in probe, when we have an actual
> device to talk to...

On the C side it really depends on what probe (of the parent) does.
If it sets driver_data of the parent before registering the RTC
device, then it'll all work.  If it registers the RTC device before
setting driver_data for the parent, there may be a race.

IIUC, the idea on the Rust side is to use the capabilities of the
compiler to eliminate the possibility of messing up the ordering in a
driver.

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

[Danilo Krummrich]

On Tue Feb 24, 2026 at 4:23 PM CET, Rafael J. Wysocki wrote:
> On the C side it really depends on what probe (of the parent) does.
> If it sets driver_data of the parent before registering the RTC
> device, then it'll all work.  If it registers the RTC device before
> setting driver_data for the parent, there may be a race.
>
> IIUC, the idea on the Rust side is to use the capabilities of the
> compiler to eliminate the possibility of messing up the ordering in a
> driver.

Yes, this is all spot-on.

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

[Alexandre Belloni]

On 24/02/2026 01:12:32+0100, Danilo Krummrich wrote:
> 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<rtc::Device>,
> >	};
> >
> > 	// The bus device private data.
> > 	#[pin_data]
> > 	struct SampleDriver {
> > 	    #[pin]
> > 	    irq: irq::Registration<SampleIrqHandler>,
> > 	    rtc: ARef<rtc::Device<SampleRtcData>>,
> > 	}
> >
> > 	// The class device private data.
> > 	struct SampleRtcData {
> > 	    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<SampleRtcData>
> > 	        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 => { ... },
> > 	        }
> > 	    }
> > 	}
> >
> 
> 	impl pci::Driver for SampleDriver {
> 	    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: iomap_region_sized::<BAR0_SIZE>(0, c"my_rtc/bar0")?,
> 	            hw_variant: VendorVariant::StV1,
> 	        });
> 
> 	        let rtc = 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,
> 	        })

I can't really read rust yet but this seems to open a race condition
with userspace if irq::Registration::new(...) fails, there is an
ordering constraint you missed.

> 	    }
> 	}
> 
> 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 not
> compile in the first place, which is a huge improvement over the situation we
> have in C.)

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

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

[Danilo Krummrich]

On Tue Feb 24, 2026 at 4:01 PM CET, Alexandre Belloni wrote:
> On 24/02/2026 01:12:32+0100, Danilo Krummrich wrote:
>> 	impl pci::Driver for SampleDriver {
>> 	    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: iomap_region_sized::<BAR0_SIZE>(0, c"my_rtc/bar0")?,
>> 	            hw_variant: VendorVariant::StV1,
>> 	        });
>> 
>> 	        let rtc = 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,
>> 	        })
>
> I can't really read rust yet but this seems to open a race condition
> with userspace if irq::Registration::new(...) fails, there is an
> ordering constraint you missed.

(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.

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

[Danilo Krummrich]

On Tue Feb 24, 2026 at 5:35 PM CET, Danilo Krummrich wrote:
> 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.

I.e. as long as we can model a data dependency we can catch those things at
compile time.

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

[Alexandre Belloni]

On 24/02/2026 17:35:23+0100, Danilo Krummrich wrote:
> On Tue Feb 24, 2026 at 4:01 PM CET, Alexandre Belloni wrote:
> > On 24/02/2026 01:12:32+0100, Danilo Krummrich wrote:
> >> 	impl pci::Driver for SampleDriver {
> >> 	    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: iomap_region_sized::<BAR0_SIZE>(0, c"my_rtc/bar0")?,
> >> 	            hw_variant: VendorVariant::StV1,
> >> 	        });
> >> 
> >> 	        let rtc = 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,
> >> 	        })
> >
> > I can't really read rust yet but this seems to open a race condition
> > with userspace if irq::Registration::new(...) fails, there is an
> > ordering constraint you missed.
> 
> (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.

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

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

[Danilo Krummrich]

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().

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).

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.)

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

[Alexandre Belloni]

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

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

[Gary Guo]

On 2026-02-24 22:44, Alexandre Belloni wrote:
> 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.

I think it is not realistic to require module init to always complete once a char
dev is registered. What if you're trying to register multiple char devs?

I think this requires that either `rtc_device_unregister` to wait until all
opened fds on the char dev to be closed before returning, or a reference
count to the module is kept. (Or having the file ops being swapped out so
further operation on the fd doesn't hit the module anymore).

Anyhow, this looks exactly what all the driver-core revocable discussion is about.

Best,
Gary

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

[Rafael J. Wysocki]

On Tue, Feb 24, 2026 at 11:23 PM Danilo Krummrich <dakr@kernel.org> 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().

IIUC, the interrupt handler can only access the rtc_data because the
parent's driver_data may not exist yet when it runs.  Or am I missing
something?

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

[Danilo Krummrich]

On Wed Feb 25, 2026 at 2:33 PM CET, Rafael J. Wysocki wrote:
> On Tue, Feb 24, 2026 at 11:23 PM Danilo Krummrich <dakr@kernel.org> wrote:
>> 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().
>
> IIUC, the interrupt handler can only access the rtc_data because the
> parent's driver_data may not exist yet when it runs.  Or am I missing
> something?

In the code above the IRQ handler can also not access rtc_data, as struct
SampleRtcData might not be fully initialized when it runs, i.e.

	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(..., rtc_data),
	});

would not compile in the first place.

irq::Registration, for this purpose, has its own private data on the handler
itself, see also [1]. In fact, the C code has the same concept with the dev_id
argument in request_threaded_irq() [2].

The difference is that the C compiler does not ensure that the IRQ handler
actually owns the data behind the dev_id pointer. I.e. the driver has to somehow
ensure that whatever is behind the dev_id pointer remains valid for the duration
the IRQ handler is registered.

In the Rust implementation the compiler does ensure that what is behind the
dev_id pointer remains valid for the duration of the lifetime of the
irq::Registration.

Having that said, I assume you wonder what we would pass into the
irq::Registration instead, if it is not rtc_data.

The answer is it depends; it depends on what's actually needed, what other
entities interact with the IRQ (e.g. some scheduled work, etc.) and maybe even
preference to some extend.

Here is one example:

	let irq_data = impl_pin_init!(SampleIrqData {
	    io <- pdev.iomap_region_sized::<BAR0_SIZE>(0, c"my_rtc/bar0")?,
	    hw_variant: VendorVariant::StV1,
	});

	let rtc_data = impl_pin_init!(SampleRtcData {
	    irq <- irq::Registration::new(..., irq_data),
	    ...,
	});

	let rtc = rtc::Device::new(dev, rtc_data)?;

This would compile as it ensures that irq_data (struct SampleIrqData) is fully
initialized before irq::Registration::new() is called.

At a first glance this might look like we need an additional allocation, one for
irq_data and one for rtc_data, but that is not the case. irq_data is an
initializer that is passed to another initializer, i.e. rtc_data is still an
initializer.

The actual (single) allocation happens in rtc::Device::new().

In terms of accessing it through the the rtc::Device in an RTC device callback,
we would likely use accessor methods to make it a bit more convinient, i.e.

	    fn read_time(
	        rtc: &rtc::Device<SampleRtcData>
	        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 => { ... },
	        }
	    }

As mentioned above there are a few other options to implement this, depending on
what's required, etc.

For instance, if the I/O bar is actually shared between multiple entities we
might want to initialize it within an Arc [3] (reference count it) for shared
ownership.

For the future we will also be able to support references within initializers to
other pinned fields, which make things a bit more convinient, so you could do
things like this:

	let irq_data = impl_pin_init!(SampleIrqData {
	    io <- pdev.iomap_region_sized::<BAR0_SIZE>(0, c"my_rtc/bar0")?,
	    hw_variant: VendorVariant::StV1,
	});

	let rtc_data = impl_pin_init!(SampleRtcData {
	    irq <- irq::Registration::new(..., irq_data),
	    io: &irq.io,
	    ...,
	});

	let rtc = rtc::Device::new(dev, rtc_data)?;

Note the additional `io: &irq.io,` in the rtc_data initializer. This would be
legal as we know that `irq` is pinned within `rtc_data`, hence it is valid to
hold a reference to one of its pinned fields.

I am not sure how far we are from having this supported, I assume Benno and Gary
can say more about this.

I hope this helps, and thanks for asking those questions!

[1] https://rust.docs.kernel.org/kernel/irq/struct.Registration.html
[2] https://elixir.bootlin.com/linux/v6.19.3/source/kernel/irq/manage.c#L2090
[3] https://rust.docs.kernel.org/kernel/sync/struct.Arc.html

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

[Rafael J. Wysocki]

On Wed, Feb 25, 2026 at 5:26 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Wed Feb 25, 2026 at 2:33 PM CET, Rafael J. Wysocki wrote:
> > On Tue, Feb 24, 2026 at 11:23 PM Danilo Krummrich <dakr@kernel.org> wrote:
> >> 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().
> >
> > IIUC, the interrupt handler can only access the rtc_data because the
> > parent's driver_data may not exist yet when it runs.  Or am I missing
> > something?
>
> In the code above the IRQ handler can also not access rtc_data, as struct
> SampleRtcData might not be fully initialized when it runs, i.e.
>
>         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(..., rtc_data),
>         });
>
> would not compile in the first place.
>
> irq::Registration, for this purpose, has its own private data on the handler
> itself, see also [1]. In fact, the C code has the same concept with the dev_id
> argument in request_threaded_irq() [2].
>
> The difference is that the C compiler does not ensure that the IRQ handler
> actually owns the data behind the dev_id pointer. I.e. the driver has to somehow
> ensure that whatever is behind the dev_id pointer remains valid for the duration
> the IRQ handler is registered.
>
> In the Rust implementation the compiler does ensure that what is behind the
> dev_id pointer remains valid for the duration of the lifetime of the
> irq::Registration.
>
> Having that said, I assume you wonder what we would pass into the
> irq::Registration instead, if it is not rtc_data.
>
> The answer is it depends; it depends on what's actually needed, what other
> entities interact with the IRQ (e.g. some scheduled work, etc.) and maybe even
> preference to some extend.
>
> Here is one example:
>
>         let irq_data = impl_pin_init!(SampleIrqData {
>             io <- pdev.iomap_region_sized::<BAR0_SIZE>(0, c"my_rtc/bar0")?,
>             hw_variant: VendorVariant::StV1,
>         });
>
>         let rtc_data = impl_pin_init!(SampleRtcData {
>             irq <- irq::Registration::new(..., irq_data),
>             ...,
>         });
>
>         let rtc = rtc::Device::new(dev, rtc_data)?;
>
> This would compile as it ensures that irq_data (struct SampleIrqData) is fully
> initialized before irq::Registration::new() is called.
>
> At a first glance this might look like we need an additional allocation, one for
> irq_data and one for rtc_data, but that is not the case. irq_data is an
> initializer that is passed to another initializer, i.e. rtc_data is still an
> initializer.
>
> The actual (single) allocation happens in rtc::Device::new().
>
> In terms of accessing it through the the rtc::Device in an RTC device callback,
> we would likely use accessor methods to make it a bit more convinient, i.e.
>
>             fn read_time(
>                 rtc: &rtc::Device<SampleRtcData>
>                 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 => { ... },
>                 }
>             }
>
> As mentioned above there are a few other options to implement this, depending on
> what's required, etc.
>
> For instance, if the I/O bar is actually shared between multiple entities we
> might want to initialize it within an Arc [3] (reference count it) for shared
> ownership.
>
> For the future we will also be able to support references within initializers to
> other pinned fields, which make things a bit more convinient, so you could do
> things like this:
>
>         let irq_data = impl_pin_init!(SampleIrqData {
>             io <- pdev.iomap_region_sized::<BAR0_SIZE>(0, c"my_rtc/bar0")?,
>             hw_variant: VendorVariant::StV1,
>         });
>
>         let rtc_data = impl_pin_init!(SampleRtcData {
>             irq <- irq::Registration::new(..., irq_data),
>             io: &irq.io,
>             ...,
>         });
>
>         let rtc = rtc::Device::new(dev, rtc_data)?;
>
> Note the additional `io: &irq.io,` in the rtc_data initializer. This would be
> legal as we know that `irq` is pinned within `rtc_data`, hence it is valid to
> hold a reference to one of its pinned fields.
>
> I am not sure how far we are from having this supported, I assume Benno and Gary
> can say more about this.
>
> I hope this helps, and thanks for asking those questions!

Well, I'm using an opportunity to learn something, thank you!

> [1] https://rust.docs.kernel.org/kernel/irq/struct.Registration.html
> [2] https://elixir.bootlin.com/linux/v6.19.3/source/kernel/irq/manage.c#L2090
> [3] https://rust.docs.kernel.org/kernel/sync/struct.Arc.html

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

[Rafael J. Wysocki]

On Wed, Feb 25, 2026 at 5:26 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Wed Feb 25, 2026 at 2:33 PM CET, Rafael J. Wysocki wrote:
> > On Tue, Feb 24, 2026 at 11:23 PM Danilo Krummrich <dakr@kernel.org> wrote:
> >> 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().
> >
> > IIUC, the interrupt handler can only access the rtc_data because the
> > parent's driver_data may not exist yet when it runs.  Or am I missing
> > something?
>
> In the code above the IRQ handler can also not access rtc_data, as struct
> SampleRtcData might not be fully initialized when it runs, i.e.
>
>         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(..., rtc_data),
>         });
>
> would not compile in the first place.
>
> irq::Registration, for this purpose, has its own private data on the handler
> itself, see also [1]. In fact, the C code has the same concept with the dev_id
> argument in request_threaded_irq() [2].
>
> The difference is that the C compiler does not ensure that the IRQ handler
> actually owns the data behind the dev_id pointer. I.e. the driver has to somehow
> ensure that whatever is behind the dev_id pointer remains valid for the duration
> the IRQ handler is registered.
>
> In the Rust implementation the compiler does ensure that what is behind the
> dev_id pointer remains valid for the duration of the lifetime of the
> irq::Registration.
>
> Having that said, I assume you wonder what we would pass into the
> irq::Registration instead, if it is not rtc_data.
>
> The answer is it depends; it depends on what's actually needed, what other
> entities interact with the IRQ (e.g. some scheduled work, etc.) and maybe even
> preference to some extend.
>
> Here is one example:
>
>         let irq_data = impl_pin_init!(SampleIrqData {
>             io <- pdev.iomap_region_sized::<BAR0_SIZE>(0, c"my_rtc/bar0")?,
>             hw_variant: VendorVariant::StV1,
>         });
>
>         let rtc_data = impl_pin_init!(SampleRtcData {
>             irq <- irq::Registration::new(..., irq_data),
>             ...,
>         });
>
>         let rtc = rtc::Device::new(dev, rtc_data)?;
>
> This would compile as it ensures that irq_data (struct SampleIrqData) is fully
> initialized before irq::Registration::new() is called.
>
> At a first glance this might look like we need an additional allocation, one for
> irq_data and one for rtc_data, but that is not the case. irq_data is an
> initializer that is passed to another initializer, i.e. rtc_data is still an
> initializer.
>
> The actual (single) allocation happens in rtc::Device::new().

I think that the key observation here is that C and Rust are
substantially different with respect to how things get initialized.

In C, we first allocate memory, then initialize it, and then start
services that will refer to it.  All of these steps need to be taken
explicitly and separately in the right order and the compiler simply
processes the high-level language into CPU instructions.  On the way
out, all of that needs to be cleaned up directly, most of the time in
reverse order.  If anything is missed or forgotten, or the ordering is
messed up, troubles ensue.

In Rust, IIUC, the compiler is essentially told about what data will
be there in the memory, how to initialize it and what services to
start and all of that happens in one go when memory gets allocated
(so, apparently, a good part of the code doesn't even produce CPU
instructions at all, as it is all about feeding the information to the
compiler).  So long as the compiler has complete information, it can
figure out the right ordering automatically and it will complain if
something is not right.  The difficulty here is to find a way to
provide the compiler with complete information.

> In terms of accessing it through the the rtc::Device in an RTC device callback,
> we would likely use accessor methods to make it a bit more convinient, i.e.
>
>             fn read_time(
>                 rtc: &rtc::Device<SampleRtcData>
>                 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 => { ... },
>                 }
>             }
>
> As mentioned above there are a few other options to implement this, depending on
> what's required, etc.
>
> For instance, if the I/O bar is actually shared between multiple entities we
> might want to initialize it within an Arc [3] (reference count it) for shared
> ownership.
>
> For the future we will also be able to support references within initializers to
> other pinned fields, which make things a bit more convinient, so you could do
> things like this:
>
>         let irq_data = impl_pin_init!(SampleIrqData {
>             io <- pdev.iomap_region_sized::<BAR0_SIZE>(0, c"my_rtc/bar0")?,
>             hw_variant: VendorVariant::StV1,
>         });
>
>         let rtc_data = impl_pin_init!(SampleRtcData {
>             irq <- irq::Registration::new(..., irq_data),
>             io: &irq.io,
>             ...,
>         });
>
>         let rtc = rtc::Device::new(dev, rtc_data)?;
>
> Note the additional `io: &irq.io,` in the rtc_data initializer. This would be
> legal as we know that `irq` is pinned within `rtc_data`, hence it is valid to
> hold a reference to one of its pinned fields.
>
> I am not sure how far we are from having this supported, I assume Benno and Gary
> can say more about this.
>
> I hope this helps, and thanks for asking those questions!
>
> [1] https://rust.docs.kernel.org/kernel/irq/struct.Registration.html
> [2] https://elixir.bootlin.com/linux/v6.19.3/source/kernel/irq/manage.c#L2090
> [3] https://rust.docs.kernel.org/kernel/sync/struct.Arc.html

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

[Benno Lossin]

On Wed Feb 25, 2026 at 5:26 PM CET, Danilo Krummrich wrote:
> For the future we will also be able to support references within initializers to
> other pinned fields, which make things a bit more convinient, so you could do
> things like this:
>
> 	let irq_data = impl_pin_init!(SampleIrqData {
> 	    io <- pdev.iomap_region_sized::<BAR0_SIZE>(0, c"my_rtc/bar0")?,
> 	    hw_variant: VendorVariant::StV1,
> 	});
>
> 	let rtc_data = impl_pin_init!(SampleRtcData {
> 	    irq <- irq::Registration::new(..., irq_data),
> 	    io: &irq.io,
> 	    ...,
> 	});
>
> 	let rtc = rtc::Device::new(dev, rtc_data)?;
>
> Note the additional `io: &irq.io,` in the rtc_data initializer. This would be
> legal as we know that `irq` is pinned within `rtc_data`, hence it is valid to
> hold a reference to one of its pinned fields.
>
> I am not sure how far we are from having this supported, I assume Benno and Gary
> can say more about this.

We added that support in 42415d163e5d ("rust: pin-init: add references
to previously initialized fields"). Note that any references created
during the initializer are only valid inside of that initializer. You
can convert into a raw pointer though and if the struct is `!Unpin` it
is sound to use that pointer at the same time as references to the
pinned value.

Cheers,
Benno

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

[Rafael J. Wysocki]

On Sat, Feb 21, 2026 at 3:33 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Sat Feb 21, 2026 at 12:19 PM CET, Rafael J. Wysocki wrote:
> > On Sat, Feb 21, 2026 at 12:16 PM Alexandre Belloni
> > <alexandre.belloni@bootlin.com> wrote:
> >> > > Out of 29 drivers, 18 are doing so.
> >
> > The vast majority of around 50 platform drivers I've inspected
> > recently use platform_set_drvdata() or equivalent in probe.
>
> This thread seems to contain quite a bit of confusion and misunderstandings --
> let me try to clarify.
>
>   (1) How Rust handles bus device private data.
>
>   In Rust the probe() function of a bus implementation (platform, PCI, etc.)
>   returns an initializer (impl PinInit<T, Error>) for the driver's device
>   private data.
>
>   The bus implementation takes this initializer and passes it (together with the
>   underlying struct device) to the driver-core. The driver-core allocates the
>   required memory, initializes the memory with the given initializer and stores
>   a pointer to the corresponding object with dev_set_drvdata().
>
>   So, technically, in Rust all platform drivers call platform_set_drvdata().

So do I understand correctly that the driver is required to tell the
core what type its driver_data will be and then the core will allocate
memory for it and clean it up on remove?

>   (Note that this is also true when the driver's device private data type is
>   empty (i.e. it has no fields). In this case it could still have a destructor
>   that must be called when the device private data structure is destroyed. Of
>   course there is no real memory allocation when the struct's size is zero.)

So in the simplest case when the driver doesn't need driver_data at
all, it will just use a struct with no fields as that driver_data
type, IIUC.

>   The driver's device private data can only be accessed when the bus device is
>   bound to the driver, i.e. the driver can only access it with a &Device<Bound>;
>   it (the driver's device private data) is automatically freed by the
>   driver-core when remove() and all devres callbacks have been completed.

Well, that's what happens on the C side of things too most of the time
because driver_data is allocated at probe time, very often using
devm_kzalloc() or equivalent.

>   I.e. the rules are - of course - the same as on the C side, but they are
>   enforced by the type system and the driver-core code.

OK

>   (2) Bus device private data vs. class device private data.
>
>   The change to pass a struct rtc_device in class device callbacks of RTC,
>   rather than the base struct device of the corresponding bus device (e.g. AMBA,
>   platform, etc.) should not aim at storing all data in rtc->dev.private_data
>   that was previously stored in rtc->dev.parent->private_data.
>
>   Instead, it gives drivers the option to differentiate in terms of ownership
>   and lifetime.
>
>   While the bus device private data has a very defined lifetime from probe()
>   until the device is unbound from the driver, class device private data might
>   live shorter than this, or might even out-live driver unbind in some cases. It
>   really depends on the lifetime of the class device itself, which is not
>   generally defined.
>
>   Now, from a C side point of view this may not look like a big deal, as it
>   (unfortunately) is not that uncommon that struct fields are just initialized
>   and destroyed whenever needed and the code just takes it into account.
>
>   But at the same time, this is what leads to a lot of lifetime problems and
>   memory bugs and it is one of those things that Rust aims at avoiding by being
>   very strict about initialization, ownership and lifetimes.

As a general rule, I agree, but I would advise against applying
general rules automatically everywhere.

>   However, I do also recognize that drivers creating an RTC device are typically
>   very simple and in practice I would not be surprised if it turns out that it
>   happens that drivers keep the struct rtc_device alive from probe() until the
>   bus device is unbound from the driver, i.e. lifetimes just end up being almost
>   the same. But I don't know if that's always the case.
>
>   Regardless of that, I think it would be good to keep driver authors finding a
>   common pattern, where class device callbacks carry the corresponding class
>   device struct (instead of the parent base struct device).

TBH I'm not really convinced about this particular thing and I think I
can provide an illustrative example.

Namely, quite incidentally, I've recently set out to add an RTC class
device to an existing driver, which is the ACPI time and alarm device
(TAD) one.  The TAD functionality is based on ACPI control methods
provided by the platform firmware and it may (or may not) include
RTC-equivalent functions.  So far, the driver has been providing a
completely custom sysfs interface to user space, but since more and
more platforms contain an ACPI TAD and some of them may not contain a
"traditional" RTC, having an RTC class device interface in that driver
may be quite useful.

I have a prototype of the requisite change (I'll post it shortly for
reference) and it turns out that because the RTC class callbacks take
the parent device pointer as an argument, wrapping them around the
existing driver routines backing the existing sysfs interface is
super-straightforward.  Had the RTC class passed an RTC device pointer
to those callbacks, the driver would have had to do something to get
back from it to the parent device (which is what the driver really
works with).  If there are more similar drivers, that would have led
to some code duplication that is in fact unnecessary.

Moreover, the RTC device pointer doesn't even need to be stored
anywhere in that driver because the driver need not use it directly at
all and the RTC device object memory is freed by the core when the
driver unbinds.

>   Especially on the Rust side we now have the chance to make the experience of
>   writing drivers as consistent as possible, which should help (new) driver
>   authors a lot in terms of learning the driver lifetime patterns.

Well, I'm not sure if "the experience of writing drivers as consistent
as possible" is more important than less code duplication and simpler
code.

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

[Rafael J. Wysocki]

On Sun, Feb 22, 2026 at 1:25 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Sat, Feb 21, 2026 at 3:33 PM Danilo Krummrich <dakr@kernel.org> wrote:
> >
> > On Sat Feb 21, 2026 at 12:19 PM CET, Rafael J. Wysocki wrote:
> > > On Sat, Feb 21, 2026 at 12:16 PM Alexandre Belloni
> > > <alexandre.belloni@bootlin.com> wrote:
> > >> > > Out of 29 drivers, 18 are doing so.
> > >
> > > The vast majority of around 50 platform drivers I've inspected
> > > recently use platform_set_drvdata() or equivalent in probe.
> >
> > This thread seems to contain quite a bit of confusion and misunderstandings --
> > let me try to clarify.
> >
> >   (1) How Rust handles bus device private data.
> >
> >   In Rust the probe() function of a bus implementation (platform, PCI, etc.)
> >   returns an initializer (impl PinInit<T, Error>) for the driver's device
> >   private data.
> >
> >   The bus implementation takes this initializer and passes it (together with the
> >   underlying struct device) to the driver-core. The driver-core allocates the
> >   required memory, initializes the memory with the given initializer and stores
> >   a pointer to the corresponding object with dev_set_drvdata().
> >
> >   So, technically, in Rust all platform drivers call platform_set_drvdata().
>
> So do I understand correctly that the driver is required to tell the
> core what type its driver_data will be and then the core will allocate
> memory for it and clean it up on remove?
>
> >   (Note that this is also true when the driver's device private data type is
> >   empty (i.e. it has no fields). In this case it could still have a destructor
> >   that must be called when the device private data structure is destroyed. Of
> >   course there is no real memory allocation when the struct's size is zero.)
>
> So in the simplest case when the driver doesn't need driver_data at
> all, it will just use a struct with no fields as that driver_data
> type, IIUC.
>
> >   The driver's device private data can only be accessed when the bus device is
> >   bound to the driver, i.e. the driver can only access it with a &Device<Bound>;
> >   it (the driver's device private data) is automatically freed by the
> >   driver-core when remove() and all devres callbacks have been completed.
>
> Well, that's what happens on the C side of things too most of the time
> because driver_data is allocated at probe time, very often using
> devm_kzalloc() or equivalent.
>
> >   I.e. the rules are - of course - the same as on the C side, but they are
> >   enforced by the type system and the driver-core code.
>
> OK
>
> >   (2) Bus device private data vs. class device private data.
> >
> >   The change to pass a struct rtc_device in class device callbacks of RTC,
> >   rather than the base struct device of the corresponding bus device (e.g. AMBA,
> >   platform, etc.) should not aim at storing all data in rtc->dev.private_data
> >   that was previously stored in rtc->dev.parent->private_data.
> >
> >   Instead, it gives drivers the option to differentiate in terms of ownership
> >   and lifetime.
> >
> >   While the bus device private data has a very defined lifetime from probe()
> >   until the device is unbound from the driver, class device private data might
> >   live shorter than this, or might even out-live driver unbind in some cases. It
> >   really depends on the lifetime of the class device itself, which is not
> >   generally defined.
> >
> >   Now, from a C side point of view this may not look like a big deal, as it
> >   (unfortunately) is not that uncommon that struct fields are just initialized
> >   and destroyed whenever needed and the code just takes it into account.
> >
> >   But at the same time, this is what leads to a lot of lifetime problems and
> >   memory bugs and it is one of those things that Rust aims at avoiding by being
> >   very strict about initialization, ownership and lifetimes.
>
> As a general rule, I agree, but I would advise against applying
> general rules automatically everywhere.
>
> >   However, I do also recognize that drivers creating an RTC device are typically
> >   very simple and in practice I would not be surprised if it turns out that it
> >   happens that drivers keep the struct rtc_device alive from probe() until the
> >   bus device is unbound from the driver, i.e. lifetimes just end up being almost
> >   the same. But I don't know if that's always the case.
> >
> >   Regardless of that, I think it would be good to keep driver authors finding a
> >   common pattern, where class device callbacks carry the corresponding class
> >   device struct (instead of the parent base struct device).
>
> TBH I'm not really convinced about this particular thing and I think I
> can provide an illustrative example.
>
> Namely, quite incidentally, I've recently set out to add an RTC class
> device to an existing driver, which is the ACPI time and alarm device
> (TAD) one.  The TAD functionality is based on ACPI control methods
> provided by the platform firmware and it may (or may not) include
> RTC-equivalent functions.  So far, the driver has been providing a
> completely custom sysfs interface to user space, but since more and
> more platforms contain an ACPI TAD and some of them may not contain a
> "traditional" RTC, having an RTC class device interface in that driver
> may be quite useful.
>
> I have a prototype of the requisite change (I'll post it shortly for
> reference) and it turns out that because the RTC class callbacks take
> the parent device pointer as an argument, wrapping them around the
> existing driver routines backing the existing sysfs interface is
> super-straightforward.  Had the RTC class passed an RTC device pointer
> to those callbacks, the driver would have had to do something to get
> back from it to the parent device (which is what the driver really
> works with).  If there are more similar drivers, that would have led
> to some code duplication that is in fact unnecessary.
>
> Moreover, the RTC device pointer doesn't even need to be stored
> anywhere in that driver because the driver need not use it directly at
> all and the RTC device object memory is freed by the core when the
> driver unbinds.

The above is referring specifically to this patch:

https://lore.kernel.org/linux-acpi/10819001.nUPlyArG6x@rafael.j.wysocki/

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

[Danilo Krummrich]

On Sun Feb 22, 2026 at 1:25 PM CET, Rafael J. Wysocki wrote:
> On Sat, Feb 21, 2026 at 3:33 PM Danilo Krummrich <dakr@kernel.org> wrote:
>>
>> On Sat Feb 21, 2026 at 12:19 PM CET, Rafael J. Wysocki wrote:
>> > On Sat, Feb 21, 2026 at 12:16 PM Alexandre Belloni
>> > <alexandre.belloni@bootlin.com> wrote:
>> >> > > Out of 29 drivers, 18 are doing so.
>> >
>> > The vast majority of around 50 platform drivers I've inspected
>> > recently use platform_set_drvdata() or equivalent in probe.
>>
>> This thread seems to contain quite a bit of confusion and misunderstandings --
>> let me try to clarify.
>>
>>   (1) How Rust handles bus device private data.
>>
>>   In Rust the probe() function of a bus implementation (platform, PCI, etc.)
>>   returns an initializer (impl PinInit<T, Error>) for the driver's device
>>   private data.
>>
>>   The bus implementation takes this initializer and passes it (together with the
>>   underlying struct device) to the driver-core. The driver-core allocates the
>>   required memory, initializes the memory with the given initializer and stores
>>   a pointer to the corresponding object with dev_set_drvdata().
>>
>>   So, technically, in Rust all platform drivers call platform_set_drvdata().
>
> So do I understand correctly that the driver is required to tell the
> core what type its driver_data will be and then the core will allocate
> memory for it and clean it up on remove?

Yes, but it's not really that the driver actively has to tell the driver-core,
etc.

probe() functions return an initializer for the driver's device private data, so
the type is known anyways.

	    fn probe(
	        pdev: &pci::Device<Core>,
	        info: &Self::IdInfo,
	    ) -> impl PinInit<T, Error> {
	        ...
	    }

So, the return type is a fallible initializer for T, where T is the type of the
driver's device private data.

I assume this may sound a bit odd with little or no Rust experience. Hence, for
people without much Rust experience reading along a quick explanation:

On the more general side of things, Rust has a very powerful type system, which
includes generics, hence modeling such kind of things with generics is pretty
straight forward and preferred over passing void pointers.

But there is also a much more specific reason; In C dev_get_drvdata() has two
pitfalls:

  (1) The pointer returned by dev_get_drvdata() is only valid as long as the
      bus device is bound to the driver.

  (2) The driver has to cast the pointer returned by dev_get_drvdata() to the
      correct type.

Since Rust is a memory safe language, we can't allow UB for safe APIs. Hence,
the Rust Device::drvdata() [1] method has to consider both (1) and (2).

(1) is rather simple as we have a type state found bound devices, i.e.
Device<Bound>, so drvdata() is simply only implemented for Device<Bound>.

(2) is much more tricky as we can't statically type the device over its private
data, as a single device instance can be bound to multiple drivers at runtime.

Hence, we need a runtime check, which the driver-core does for us. When a driver
calls the drvdata() method it looks like this:

	fn foo(dev: &Device<Bound>) -> Result {
	    let data = dev.drvdata::<MyDataType>()?;

	    data.bar();
	}

The driver-core takes care of checking that the private data associated with
`dev` actually is of type MyDataType. If this is not the case, the call simply
fails.

The alternative would be an infallible unsafe API, such as:

	fn foo(dev: &Device<Bound>) -> Result {
	    // SAFETY:
	    // - `dev` is guaranteed to be bound, because ...
	    // - The private data type of `dev` is guaranteed to be
	    //   `MyDataType`, since ...
	    let data = unsafe { dev.drvdata::<MyDataType>() };

	    data.bar();
	}

[1] https://rust.docs.kernel.org/kernel/device/struct.Device.html#method.drvdata

>>   (Note that this is also true when the driver's device private data type is
>>   empty (i.e. it has no fields). In this case it could still have a destructor
>>   that must be called when the device private data structure is destroyed. Of
>>   course there is no real memory allocation when the struct's size is zero.)
>
> So in the simplest case when the driver doesn't need driver_data at
> all, it will just use a struct with no fields as that driver_data
> type, IIUC.

Yes, it would look like this:

	struct SampleDriver;
	
	impl pci::Driver for SampleDriver {
	    fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> impl PinInit<Self, Error> {
	        if !validate_something() {
	            return Err(EINVAL);
	        }
	
	        Ok(Self)
	    }
	}

>>   But at the same time, this is what leads to a lot of lifetime problems and
>>   memory bugs and it is one of those things that Rust aims at avoiding by being
>>   very strict about initialization, ownership and lifetimes.
>
> As a general rule, I agree, but I would advise against applying
> general rules automatically everywhere.
>
>>   However, I do also recognize that drivers creating an RTC device are typically
>>   very simple and in practice I would not be surprised if it turns out that it
>>   happens that drivers keep the struct rtc_device alive from probe() until the
>>   bus device is unbound from the driver, i.e. lifetimes just end up being almost
>>   the same. But I don't know if that's always the case.
>>
>>   Regardless of that, I think it would be good to keep driver authors finding a
>>   common pattern, where class device callbacks carry the corresponding class
>>   device struct (instead of the parent base struct device).
>
> TBH I'm not really convinced about this particular thing and I think I
> can provide an illustrative example.
>
> Namely, quite incidentally, I've recently set out to add an RTC class
> device to an existing driver, which is the ACPI time and alarm device
> (TAD) one.  The TAD functionality is based on ACPI control methods
> provided by the platform firmware and it may (or may not) include
> RTC-equivalent functions.  So far, the driver has been providing a
> completely custom sysfs interface to user space, but since more and
> more platforms contain an ACPI TAD and some of them may not contain a
> "traditional" RTC, having an RTC class device interface in that driver
> may be quite useful.
>
> I have a prototype of the requisite change (I'll post it shortly for
> reference) and it turns out that because the RTC class callbacks take
> the parent device pointer as an argument, wrapping them around the
> existing driver routines backing the existing sysfs interface is
> super-straightforward.  Had the RTC class passed an RTC device pointer
> to those callbacks, the driver would have had to do something to get
> back from it to the parent device (which is what the driver really
> works with).  If there are more similar drivers, that would have led
> to some code duplication that is in fact unnecessary.

The type system in Rust is powerfuly enough, so drivers can even get the exact
type of the parent device the RTC device has as an additional argument to the
callback. The infrastructure for this is in place and it is used by subsystems.
I.e. in RTC we can do something like this:

	impl rtc::Ops for MyRtcOps {
	    type BusDeviceType = platform::Device<Bound>;
	
	    fn read_time(
	        rtc: &rtc::Device<MyRtcData>,
	        parent: &platform::Device<Bound>,
	        time: rtc::Time,
	    ) -> Result {
	        ...
	    }
	}

where the corresponding rtc::Device::register() method would ensure that the
associated BusDeviceType matches the parent device type that is passed to
rtc::Device::register().

A real example of this can be found in the LED class device abstractions [2].

Note that in contrast to a bus device, class devices can be statically typed
over their private data: rtc::Device<MyRtcData>.

We usually also implment the Deref trait, such that rtc::Device<MyRtcData>
automatically dereferences to MyRtcData.

Having that said, if RTC drivers *never* have any private data that should be
associated with the RTC device exclusively (i.e. data that is only relevant in
the context of class device callbacks and class device infrastructure in
general, or logically belongs to the class device), then the `rdev` argument
would indeed be always unused and hence pointless.

I usually would assume that there are such cases, but if that's really never the
case for any RTC drivers, then I agree we should change the above code to:

	impl rtc::Ops for MyRtcOps {
	    type BusDeviceType = platform::Device<Bound>;
	
	    fn read_time(
	        parent: &platform::Device<Bound>,
	        time: rtc::Time,
	    ) -> Result {
	        ...
	    }
	}

This way it is at least clear what kind of device is passed through the class
device callbacks.

[2] https://lore.kernel.org/all/20260207-rust_leds-v12-1-fdb518417b75@posteo.de/

> Moreover, the RTC device pointer doesn't even need to be stored
> anywhere in that driver because the driver need not use it directly at
> all and the RTC device object memory is freed by the core when the
> driver unbinds.

I don't think that is true, I think there are a few drivers accessing the RTC
device from IRQs or workqueues.

Besides that, quite a lot of RTC drivers actually seem to save a pointer to the
struct rtc_device within their bus device private data, e.g. [3] and [4].

[3] https://elixir.bootlin.com/linux/v6.19.2/source/drivers/rtc/rtc-ac100.c#L91
[4] https://elixir.bootlin.com/linux/v6.19.2/source/drivers/rtc/rtc-cros-ec.c#L30

>>   Especially on the Rust side we now have the chance to make the experience of
>>   writing drivers as consistent as possible, which should help (new) driver
>>   authors a lot in terms of learning the driver lifetime patterns.
>
> Well, I'm not sure if "the experience of writing drivers as consistent
> as possible" is more important than less code duplication and simpler
> code.

Yeah, it always depends, and if there *really* is no point in having any class
device private data in RTC, then that's of course fine too.

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

I saw this a lot in C drivers and it actually causes ownership and lifetime
problems.

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

[Rafael J. Wysocki]

On Sun, Feb 22, 2026 at 4:29 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Sun Feb 22, 2026 at 1:25 PM CET, Rafael J. Wysocki wrote:
> > On Sat, Feb 21, 2026 at 3:33 PM Danilo Krummrich <dakr@kernel.org> wrote:
> >>
> >> On Sat Feb 21, 2026 at 12:19 PM CET, Rafael J. Wysocki wrote:
> >> > On Sat, Feb 21, 2026 at 12:16 PM Alexandre Belloni
> >> > <alexandre.belloni@bootlin.com> wrote:
> >> >> > > Out of 29 drivers, 18 are doing so.
> >> >
> >> > The vast majority of around 50 platform drivers I've inspected
> >> > recently use platform_set_drvdata() or equivalent in probe.
> >>
> >> This thread seems to contain quite a bit of confusion and misunderstandings --
> >> let me try to clarify.
> >>
> >>   (1) How Rust handles bus device private data.
> >>
> >>   In Rust the probe() function of a bus implementation (platform, PCI, etc.)
> >>   returns an initializer (impl PinInit<T, Error>) for the driver's device
> >>   private data.
> >>
> >>   The bus implementation takes this initializer and passes it (together with the
> >>   underlying struct device) to the driver-core. The driver-core allocates the
> >>   required memory, initializes the memory with the given initializer and stores
> >>   a pointer to the corresponding object with dev_set_drvdata().
> >>
> >>   So, technically, in Rust all platform drivers call platform_set_drvdata().
> >
> > So do I understand correctly that the driver is required to tell the
> > core what type its driver_data will be and then the core will allocate
> > memory for it and clean it up on remove?
>
> Yes, but it's not really that the driver actively has to tell the driver-core,
> etc.
>
> probe() functions return an initializer for the driver's device private data, so
> the type is known anyways.
>
>             fn probe(
>                 pdev: &pci::Device<Core>,
>                 info: &Self::IdInfo,
>             ) -> impl PinInit<T, Error> {
>                 ...
>             }
>
> So, the return type is a fallible initializer for T, where T is the type of the
> driver's device private data.
>
> I assume this may sound a bit odd with little or no Rust experience. Hence, for
> people without much Rust experience reading along a quick explanation:
>
> On the more general side of things, Rust has a very powerful type system, which
> includes generics, hence modeling such kind of things with generics is pretty
> straight forward and preferred over passing void pointers.
>
> But there is also a much more specific reason; In C dev_get_drvdata() has two
> pitfalls:
>
>   (1) The pointer returned by dev_get_drvdata() is only valid as long as the
>       bus device is bound to the driver.
>
>   (2) The driver has to cast the pointer returned by dev_get_drvdata() to the
>       correct type.
>
> Since Rust is a memory safe language, we can't allow UB for safe APIs. Hence,
> the Rust Device::drvdata() [1] method has to consider both (1) and (2).
>
> (1) is rather simple as we have a type state found bound devices, i.e.
> Device<Bound>, so drvdata() is simply only implemented for Device<Bound>.
>
> (2) is much more tricky as we can't statically type the device over its private
> data, as a single device instance can be bound to multiple drivers at runtime.
>
> Hence, we need a runtime check, which the driver-core does for us. When a driver
> calls the drvdata() method it looks like this:
>
>         fn foo(dev: &Device<Bound>) -> Result {
>             let data = dev.drvdata::<MyDataType>()?;
>
>             data.bar();
>         }
>
> The driver-core takes care of checking that the private data associated with
> `dev` actually is of type MyDataType. If this is not the case, the call simply
> fails.
>
> The alternative would be an infallible unsafe API, such as:
>
>         fn foo(dev: &Device<Bound>) -> Result {
>             // SAFETY:
>             // - `dev` is guaranteed to be bound, because ...
>             // - The private data type of `dev` is guaranteed to be
>             //   `MyDataType`, since ...
>             let data = unsafe { dev.drvdata::<MyDataType>() };
>
>             data.bar();
>         }
>
> [1] https://rust.docs.kernel.org/kernel/device/struct.Device.html#method.drvdata
>
> >>   (Note that this is also true when the driver's device private data type is
> >>   empty (i.e. it has no fields). In this case it could still have a destructor
> >>   that must be called when the device private data structure is destroyed. Of
> >>   course there is no real memory allocation when the struct's size is zero.)
> >
> > So in the simplest case when the driver doesn't need driver_data at
> > all, it will just use a struct with no fields as that driver_data
> > type, IIUC.
>
> Yes, it would look like this:
>
>         struct SampleDriver;
>
>         impl pci::Driver for SampleDriver {
>             fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> impl PinInit<Self, Error> {
>                 if !validate_something() {
>                     return Err(EINVAL);
>                 }
>
>                 Ok(Self)
>             }
>         }
>
> >>   But at the same time, this is what leads to a lot of lifetime problems and
> >>   memory bugs and it is one of those things that Rust aims at avoiding by being
> >>   very strict about initialization, ownership and lifetimes.
> >
> > As a general rule, I agree, but I would advise against applying
> > general rules automatically everywhere.
> >
> >>   However, I do also recognize that drivers creating an RTC device are typically
> >>   very simple and in practice I would not be surprised if it turns out that it
> >>   happens that drivers keep the struct rtc_device alive from probe() until the
> >>   bus device is unbound from the driver, i.e. lifetimes just end up being almost
> >>   the same. But I don't know if that's always the case.
> >>
> >>   Regardless of that, I think it would be good to keep driver authors finding a
> >>   common pattern, where class device callbacks carry the corresponding class
> >>   device struct (instead of the parent base struct device).
> >
> > TBH I'm not really convinced about this particular thing and I think I
> > can provide an illustrative example.
> >
> > Namely, quite incidentally, I've recently set out to add an RTC class
> > device to an existing driver, which is the ACPI time and alarm device
> > (TAD) one.  The TAD functionality is based on ACPI control methods
> > provided by the platform firmware and it may (or may not) include
> > RTC-equivalent functions.  So far, the driver has been providing a
> > completely custom sysfs interface to user space, but since more and
> > more platforms contain an ACPI TAD and some of them may not contain a
> > "traditional" RTC, having an RTC class device interface in that driver
> > may be quite useful.
> >
> > I have a prototype of the requisite change (I'll post it shortly for
> > reference) and it turns out that because the RTC class callbacks take
> > the parent device pointer as an argument, wrapping them around the
> > existing driver routines backing the existing sysfs interface is
> > super-straightforward.  Had the RTC class passed an RTC device pointer
> > to those callbacks, the driver would have had to do something to get
> > back from it to the parent device (which is what the driver really
> > works with).  If there are more similar drivers, that would have led
> > to some code duplication that is in fact unnecessary.
>
> The type system in Rust is powerfuly enough, so drivers can even get the exact
> type of the parent device the RTC device has as an additional argument to the
> callback. The infrastructure for this is in place and it is used by subsystems.
> I.e. in RTC we can do something like this:
>
>         impl rtc::Ops for MyRtcOps {
>             type BusDeviceType = platform::Device<Bound>;
>
>             fn read_time(
>                 rtc: &rtc::Device<MyRtcData>,
>                 parent: &platform::Device<Bound>,
>                 time: rtc::Time,
>             ) -> Result {
>                 ...
>             }
>         }
>
> where the corresponding rtc::Device::register() method would ensure that the
> associated BusDeviceType matches the parent device type that is passed to
> rtc::Device::register().
>
> A real example of this can be found in the LED class device abstractions [2].
>
> Note that in contrast to a bus device, class devices can be statically typed
> over their private data: rtc::Device<MyRtcData>.
>
> We usually also implment the Deref trait, such that rtc::Device<MyRtcData>
> automatically dereferences to MyRtcData.
>
> Having that said, if RTC drivers *never* have any private data that should be
> associated with the RTC device exclusively (i.e. data that is only relevant in
> the context of class device callbacks and class device infrastructure in
> general, or logically belongs to the class device), then the `rdev` argument
> would indeed be always unused and hence pointless.
>
> I usually would assume that there are such cases, but if that's really never the
> case for any RTC drivers, then I agree we should change the above code to:
>
>         impl rtc::Ops for MyRtcOps {
>             type BusDeviceType = platform::Device<Bound>;
>
>             fn read_time(
>                 parent: &platform::Device<Bound>,
>                 time: rtc::Time,
>             ) -> Result {
>                 ...
>             }
>         }
>
> This way it is at least clear what kind of device is passed through the class
> device callbacks.
>
> [2] https://lore.kernel.org/all/20260207-rust_leds-v12-1-fdb518417b75@posteo.de/
>
> > Moreover, the RTC device pointer doesn't even need to be stored
> > anywhere in that driver because the driver need not use it directly at
> > all and the RTC device object memory is freed by the core when the
> > driver unbinds.
>
> I don't think that is true, I think there are a few drivers accessing the RTC
> device from IRQs or workqueues.

I was referring to the specific case I had in mind, I didn't mean that
this was always the case.  Sorry for being unclear.

> Besides that, quite a lot of RTC drivers actually seem to save a pointer to the
> struct rtc_device within their bus device private data, e.g. [3] and [4].
>
> [3] https://elixir.bootlin.com/linux/v6.19.2/source/drivers/rtc/rtc-ac100.c#L91
> [4] https://elixir.bootlin.com/linux/v6.19.2/source/drivers/rtc/rtc-cros-ec.c#L30

Right.

> >>   Especially on the Rust side we now have the chance to make the experience of
> >>   writing drivers as consistent as possible, which should help (new) driver
> >>   authors a lot in terms of learning the driver lifetime patterns.
> >
> > Well, I'm not sure if "the experience of writing drivers as consistent
> > as possible" is more important than less code duplication and simpler
> > code.
>
> Yeah, it always depends, and if there *really* is no point in having any class
> device private data in RTC, then that's of course fine too.
>
> I just want to make sure we do not encourage to just through all the private
> data a driver may potentially have into the bus device private data struct.
>
> I saw this a lot in C drivers and it actually causes ownership and lifetime
> problems.

Sure.

Alexandre doesn't seem to be convinced that it is necessary to change
the RTC class device interface, so if there are any existing RTC
drivers that would benefit from doing so, it would be good to refer to
them specifically as examples.  Alvin, can you find any?

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

[Alvin Sun]

On 2/21/26 19:16, Alexandre Belloni wrote:
> On 21/02/2026 17:31:09+0800, Alvin Sun wrote:
>> 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.
> But this is wrong, how do you then handle the class device on
> suspend/resume or on .remove?

There is a patch adding runtime PM for Tyr (platform device driver):

https://gitlab.freedesktop.org/panfrost/linux/-/merge_requests/60/diffs#dbdd2c5024317f2c80128c91a823e224b3a41550_240_256

If you are interested in adding Rust support for RTC, We can do some
research on top of it for RTC Rust drivers.

>
>> 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.
>>>

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

[Danilo Krummrich]

On Sat Feb 21, 2026 at 12:16 PM CET, Alexandre Belloni wrote:
> On 21/02/2026 17:31:09+0800, Alvin Sun wrote:
>> 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.
>
> But this is wrong, how do you then handle the class device on
> suspend/resume or on .remove?

I think you misunderstand what Alvin is saying here. We do have bus device
private data in all drivers. It's just that drivers do not set it randomly
through dev_set_drvdata() et al. Please see [1] for details.

[1] https://lore.kernel.org/all/DGKPPQI0QE73.S8I1M5NCI2BV@kernel.org/

[RFC PATCH v3 2/5] rust: add AMBA bus driver support

[Ke Sun]

Add Rust abstractions for ARM AMBA bus drivers, enabling Rust drivers
to interact with AMBA devices through a type-safe interface.

Signed-off-by: Ke Sun <sunke@kylinos.cn>
---
 rust/bindings/bindings_helper.h |   1 +
 rust/kernel/amba.rs             | 441 ++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs              |   2 +
 3 files changed, 444 insertions(+)
 create mode 100644 rust/kernel/amba.rs

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index a067038b4b422..fa697287cf71b 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -29,6 +29,7 @@
 #include <linux/hrtimer_types.h>
 
 #include <linux/acpi.h>
+#include <linux/amba/bus.h>
 #include <drm/drm_device.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_file.h>
diff --git a/rust/kernel/amba.rs b/rust/kernel/amba.rs
new file mode 100644
index 0000000000000..0564595c3655d
--- /dev/null
+++ b/rust/kernel/amba.rs
@@ -0,0 +1,441 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! ARM AMBA bus abstractions.
+//!
+//! C header: [`include/linux/amba/bus.h`](srctree/include/linux/amba/bus.h)
+
+use crate::{
+    bindings,
+    device,
+    device_id::{
+        RawDeviceId,
+        RawDeviceIdIndex, //
+    },
+    driver,
+    error::{
+        from_result,
+        to_result, //
+    },
+    io::{
+        mem::IoRequest,
+        resource::Resource, //
+    },
+    irq::{
+        self,
+        IrqRequest, //
+    },
+    prelude::*,
+    sync::aref::AlwaysRefCounted,
+    types::Opaque,
+    ThisModule, //
+};
+
+use core::{
+    marker::PhantomData,
+    mem::offset_of,
+    ptr::NonNull, //
+};
+
+/// Device ID table type for AMBA drivers.
+pub type IdTable<T> = &'static dyn kernel::device_id::IdTable<DeviceId, T>;
+
+/// AMBA device identifier.
+///
+/// Wraps the C `struct amba_id` from `include/linux/amba/bus.h`.
+#[repr(transparent)]
+#[derive(Clone, Copy)]
+pub struct DeviceId(pub(crate) bindings::amba_id);
+
+// SAFETY: `DeviceId` is a transparent wrapper over `amba_id` with no
+// additional invariants.
+unsafe impl RawDeviceId for DeviceId {
+    type RawType = bindings::amba_id;
+}
+
+// SAFETY: The offset matches the `data` field in `struct amba_id`.
+unsafe impl RawDeviceIdIndex for DeviceId {
+    const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::amba_id, data);
+
+    fn index(&self) -> usize {
+        self.0.data as usize
+    }
+}
+
+impl DeviceId {
+    /// Creates a new device ID from an AMBA device ID and mask.
+    #[inline(always)]
+    pub const fn new(id: u32, mask: u32) -> Self {
+        let mut amba: bindings::amba_id = pin_init::zeroed();
+        amba.id = id;
+        amba.mask = mask;
+        Self(amba)
+    }
+
+    /// Returns the device ID.
+    #[inline(always)]
+    pub const fn id(&self) -> u32 {
+        self.0.id
+    }
+
+    /// Returns the device ID mask.
+    #[inline(always)]
+    pub const fn mask(&self) -> u32 {
+        self.0.mask
+    }
+}
+
+/// Creates an AMBA device ID table with a module alias for modpost.
+#[macro_export]
+macro_rules! amba_device_table {
+    ($table_name:ident, $module_table_name:ident, $id_info_type: ty, $table_data: expr) => {
+        const $table_name: $crate::device_id::IdArray<
+            $crate::amba::DeviceId,
+            $id_info_type,
+            { $table_data.len() },
+        > = $crate::device_id::IdArray::new($table_data);
+
+        $crate::module_device_table!("amba", $module_table_name, $table_name);
+    };
+}
+
+/// The AMBA device representation.
+///
+/// This structure represents the Rust abstraction for a C `struct
+/// amba_device`. The implementation abstracts the usage of an already
+/// existing C `struct amba_device` within Rust code that we get passed
+/// from the C side.
+///
+/// # Invariants
+///
+/// A [`Device`] instance represents a valid `struct amba_device` created
+/// by the C portion of the kernel.
+#[repr(transparent)]
+pub struct Device<Ctx: device::DeviceContext = device::Normal>(
+    Opaque<bindings::amba_device>,
+    PhantomData<Ctx>,
+);
+
+impl<Ctx: device::DeviceContext> Device<Ctx> {
+    fn as_raw(&self) -> *mut bindings::amba_device {
+        self.0.get()
+    }
+
+    /// Returns the memory resource, if any.
+    ///
+    /// AMBA devices have a single memory resource that can be accessed
+    /// via this method.
+    pub fn resource(&self) -> Option<&Resource> {
+        // SAFETY: `self.as_raw()` returns a valid pointer to a `struct
+        // amba_device`.
+        let resource = unsafe { &raw mut (*self.as_raw()).res };
+        if resource.is_null() {
+            None
+        } else {
+            // SAFETY: `resource` is a valid pointer to a `struct resource`
+            // as returned by the AMBA bus.
+            Some(unsafe { Resource::from_raw(resource) })
+        }
+    }
+}
+
+impl<Ctx: device::DeviceContext> AsRef<device::Device<Ctx>> for Device<Ctx> {
+    fn as_ref(&self) -> &device::Device<Ctx> {
+        // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a
+        // pointer to a valid `struct amba_device`.
+        let dev = unsafe { &raw mut (*self.as_raw()).dev };
+        // SAFETY: `dev` points to a valid `struct device`.
+        unsafe { device::Device::from_raw(dev) }
+    }
+}
+
+// SAFETY: `Device` is a transparent wrapper that doesn't depend on its
+// generic argument.
+crate::impl_device_context_deref!(unsafe { Device });
+crate::impl_device_context_into_aref!(Device);
+
+impl Device<device::Bound> {
+    /// Returns an [`IoRequest`] for the memory resource, if any.
+    ///
+    /// This method provides access to the device's memory-mapped I/O region.
+    pub fn io_request(&self) -> Option<IoRequest<'_>> {
+        self.resource()
+            // SAFETY: `resource` is a valid resource for `&self` during the
+            // lifetime of the `IoRequest`.
+            .map(|resource| unsafe { IoRequest::new(self.as_ref(), resource) })
+    }
+
+    /// Returns an [`IrqRequest`] for the IRQ at the given index, if any.
+    ///
+    /// # Errors
+    ///
+    /// Returns `EINVAL` if `index` is out of bounds, or `ENXIO` if the
+    /// IRQ at that index is not configured.
+    pub fn irq_by_index(&self, index: u32) -> Result<IrqRequest<'_>> {
+        if index >= bindings::AMBA_NR_IRQS {
+            return Err(EINVAL);
+        }
+        // SAFETY: `self.as_raw()` returns a valid pointer to a `struct
+        // amba_device`.
+        let irq = unsafe { (*self.as_raw()).irq[index as usize] };
+        if irq == 0 {
+            return Err(ENXIO);
+        }
+        // SAFETY: `irq` is guaranteed to be a valid IRQ number for
+        // `&self` as it was read from the device structure.
+        Ok(unsafe { IrqRequest::new(self.as_ref(), irq) })
+    }
+
+    /// Returns a [`irq::Registration`] for the IRQ at the given index.
+    ///
+    /// This method registers an interrupt handler for the IRQ at the
+    /// specified index.
+    pub fn request_irq_by_index<'a, T: irq::Handler + 'static>(
+        &'a self,
+        flags: irq::Flags,
+        index: u32,
+        name: &'static CStr,
+        handler: impl PinInit<T, Error> + 'a,
+    ) -> impl PinInit<irq::Registration<T>, Error> + 'a {
+        pin_init::pin_init_scope(move || {
+            let request = self.irq_by_index(index)?;
+            Ok(irq::Registration::<T>::new(request, flags, name, handler))
+        })
+    }
+}
+
+// SAFETY: Instances of `Device` are always reference-counted.
+unsafe impl AlwaysRefCounted for Device {
+    fn inc_ref(&self) {
+        // SAFETY: The existence of a shared reference guarantees that
+        // the refcount is non-zero.
+        unsafe { bindings::get_device(self.as_ref().as_raw()) };
+    }
+
+    unsafe fn dec_ref(obj: NonNull<Self>) {
+        let adev: *mut bindings::amba_device = obj.cast().as_ptr();
+        // SAFETY: `amba_device` contains `device` as its first field.
+        let dev: *mut bindings::device = unsafe { &raw mut (*adev).dev };
+        // SAFETY: The safety requirements guarantee that the refcount is
+        // non-zero.
+        unsafe { bindings::put_device(dev) }
+    }
+}
+
+// SAFETY: `Device` is a transparent wrapper of `struct amba_device`.
+// The offset is guaranteed to point to a valid device field inside
+// `Device`.
+unsafe impl<Ctx: device::DeviceContext> device::AsBusDevice<Ctx> for Device<Ctx> {
+    const OFFSET: usize = offset_of!(bindings::amba_device, dev);
+}
+
+// SAFETY: A `Device` is always reference-counted and can be released
+// from any thread.
+unsafe impl Send for Device {}
+
+// SAFETY: `Device` can be shared among threads because all methods of
+// `Device` (i.e., `Device<Normal>`) are thread-safe.
+unsafe impl Sync for Device {}
+
+/// An adapter for the registration of AMBA drivers.
+///
+/// This adapter bridges the gap between the Rust [`Driver`] trait and
+/// the C `struct amba_driver`. It handles the registration and
+/// unregistration of AMBA drivers with the kernel's AMBA bus subsystem.
+pub struct Adapter<T: Driver>(T);
+
+// SAFETY: A call to `unregister` for a given instance of `RegType` is
+// guaranteed to be valid if a preceding call to `register` has been
+// successful.
+unsafe impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> {
+    type RegType = bindings::amba_driver;
+
+    unsafe fn register(
+        adrv: &Opaque<Self::RegType>,
+        name: &'static CStr,
+        module: &'static ThisModule,
+    ) -> Result {
+        let amba_table = T::AMBA_ID_TABLE.as_ptr();
+        // SAFETY: It's safe to set the fields of `struct amba_driver` on
+        // initialization.
+        unsafe {
+            (*adrv.get()).drv.name = name.as_char_ptr();
+            (*adrv.get()).probe = Some(Self::probe_callback);
+            (*adrv.get()).remove = Some(Self::remove_callback);
+            (*adrv.get()).shutdown = Some(Self::shutdown_callback);
+            (*adrv.get()).id_table = amba_table;
+        }
+        // SAFETY: `adrv` is guaranteed to be a valid `RegType`.
+        to_result(unsafe { bindings::__amba_driver_register(adrv.get(), module.0) })
+    }
+
+    unsafe fn unregister(adrv: &Opaque<Self::RegType>) {
+        // SAFETY: `adrv` is guaranteed to be a valid `RegType`.
+        unsafe { bindings::amba_driver_unregister(adrv.get()) };
+    }
+}
+
+impl<T: Driver + 'static> Adapter<T> {
+    extern "C" fn probe_callback(
+        adev: *mut bindings::amba_device,
+        id: *const bindings::amba_id,
+    ) -> kernel::ffi::c_int {
+        // SAFETY: The AMBA bus only ever calls the probe callback with a
+        // valid pointer to a `struct amba_device`.
+        //
+        // INVARIANT: `adev` is valid for the duration of `probe_callback()`.
+        let dev = unsafe { &*adev.cast::<Device<device::CoreInternal>>() };
+        let info = Self::amba_id_info(dev, id);
+        from_result(|| {
+            let datapin = T::probe(dev, info);
+            dev.as_ref().set_drvdata(datapin)?;
+            Ok(0)
+        })
+    }
+
+    extern "C" fn remove_callback(adev: *mut bindings::amba_device) {
+        // SAFETY: The AMBA bus only ever calls the remove callback with a
+        // valid pointer to a `struct amba_device`.
+        //
+        // INVARIANT: `adev` is valid for the duration of `remove_callback()`.
+        let dev = unsafe { &*adev.cast::<Device<device::CoreInternal>>() };
+        // SAFETY: `remove_callback` is only ever called after a successful
+        // call to `probe_callback`, hence it's guaranteed that
+        // `Device::set_drvdata()` has been called and stored a
+        // `Pin<KBox<T>>`.
+        let data = unsafe { dev.as_ref().drvdata_obtain::<T>() };
+        T::unbind(dev, data.as_ref());
+    }
+
+    extern "C" fn shutdown_callback(adev: *mut bindings::amba_device) {
+        // SAFETY: `shutdown_callback` is only ever called for a valid `adev`.
+        //
+        // INVARIANT: `adev` is valid for the duration of `shutdown_callback()`.
+        let dev = unsafe { &*adev.cast::<Device<device::CoreInternal>>() };
+        // SAFETY: `shutdown_callback` is only ever called after a
+        // successful call to `probe_callback`, hence it's guaranteed that
+        // `Device::set_drvdata()` has been called and stored a
+        // `Pin<KBox<T>>`.
+        let data = unsafe { dev.as_ref().drvdata_borrow::<T>() };
+        T::shutdown(dev, data.as_ref());
+    }
+
+    /// Extracts the device ID information from a matched AMBA device ID.
+    ///
+    /// This function looks up the driver-specific information associated
+    /// with the matched `struct amba_id` in the driver's device ID table.
+    fn amba_id_info(_dev: &Device, id: *const bindings::amba_id) -> Option<&'static T::IdInfo> {
+        if id.is_null() {
+            return None;
+        }
+        let table = T::AMBA_ID_TABLE;
+        // SAFETY: `id` is a valid pointer to a `struct amba_id` that was
+        // matched by the kernel. `DeviceId` is a `#[repr(transparent)]`
+        // wrapper of `struct amba_id` and does not add additional
+        // invariants, so it's safe to transmute.
+        let device_id = unsafe { &*id.cast::<DeviceId>() };
+        Some(table.info(<DeviceId as RawDeviceIdIndex>::index(device_id)))
+    }
+}
+
+/// The AMBA driver trait.
+///
+/// Drivers must implement this trait in order to get an AMBA driver registered.
+///
+/// # Examples
+///
+/// ```
+/// # use kernel::{amba, bindings, c_str, device::Core};
+///
+/// struct MyDriver;
+///
+/// kernel::amba_device_table!(
+///     AMBA_TABLE,
+///     MODULE_AMBA_TABLE,
+///     <MyDriver as amba::Driver>::IdInfo,
+///     [
+///         (amba::DeviceId::new(0x00041031, 0x000fffff), ())
+///     ]
+/// );
+///
+/// impl amba::Driver for MyDriver {
+///     type IdInfo = ();
+///     const AMBA_ID_TABLE: amba::IdTable<Self::IdInfo> = &AMBA_TABLE;
+///
+///     fn probe(
+///         _adev: &amba::Device<Core>,
+///         _id_info: Option<&Self::IdInfo>,
+///     ) -> impl PinInit<Self, Error> {
+///         Err(ENODEV)
+///     }
+/// }
+/// ```
+pub trait Driver: Send {
+    /// The type holding driver private data about each device id
+    /// supported by the driver.
+    type IdInfo: 'static;
+    /// The table of device ids supported by the driver.
+    const AMBA_ID_TABLE: IdTable<Self::IdInfo>;
+
+    /// AMBA driver probe.
+    ///
+    /// Called when a new AMBA device is added or discovered.
+    /// Implementers should attempt to initialize the device here.
+    fn probe(
+        dev: &Device<device::Core>,
+        id_info: Option<&Self::IdInfo>,
+    ) -> impl PinInit<Self, Error>;
+
+    /// AMBA driver shutdown.
+    ///
+    /// Called by the kernel during system reboot or power-off to allow
+    /// the [`Driver`] to bring the [`Device`] into a safe state.
+    /// Implementing this callback is optional.
+    ///
+    /// Typical actions include stopping transfers, disabling interrupts,
+    /// or resetting the hardware to prevent undesired behavior during
+    /// shutdown.
+    ///
+    /// This callback is distinct from final resource cleanup, as the
+    /// driver instance remains valid after it returns. Any deallocation
+    /// or teardown of driver-owned resources should instead be handled in
+    /// `Self::drop`.
+    fn shutdown(dev: &Device<device::Core>, this: Pin<&Self>) {
+        let _ = (dev, this);
+    }
+
+    /// AMBA driver unbind.
+    ///
+    /// Called when the [`Device`] is unbound from its bound [`Driver`].
+    /// Implementing this callback is optional.
+    ///
+    /// This callback serves as a place for drivers to perform teardown
+    /// operations that require a `&Device<Core>` or `&Device<Bound>`
+    /// reference. For instance, drivers may try to perform I/O operations
+    /// to gracefully tear down the device.
+    ///
+    /// Otherwise, release operations for driver resources should be
+    /// performed in `Self::drop`.
+    fn unbind(dev: &Device<device::Core>, this: Pin<&Self>) {
+        let _ = (dev, this);
+    }
+}
+
+/// Declares a kernel module that exposes a single AMBA driver.
+///
+/// # Examples
+///
+/// ```ignore
+/// kernel::module_amba_driver! {
+///     type: MyDriver,
+///     name: "Module name",
+///     authors: ["Author name"],
+///     description: "Description",
+///     license: "GPL v2",
+/// }
+/// ```
+#[macro_export]
+macro_rules! module_amba_driver {
+    ($($f:tt)*) => {
+        $crate::module_driver!(<T>, $crate::amba::Adapter<T>, { $($f)* });
+    };
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index f812cf1200428..3e557335fc5fe 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -66,6 +66,8 @@
 
 pub mod acpi;
 pub mod alloc;
+#[cfg(CONFIG_ARM_AMBA)]
+pub mod amba;
 #[cfg(CONFIG_AUXILIARY_BUS)]
 pub mod auxiliary;
 pub mod bitmap;
-- 
2.43.0

[RFC PATCH v3 3/5] rust: add device wakeup capability support

[Ke Sun]

Add Rust bindings and wrappers for device wakeup functionality,
including devm_device_init_wakeup() and dev_pm_set_wake_irq().

Signed-off-by: Ke Sun <sunke@kylinos.cn>
---
 rust/bindings/bindings_helper.h |  2 ++
 rust/helpers/device.c           |  6 ++++++
 rust/kernel/device.rs           | 18 +++++++++++++++++-
 rust/kernel/irq/request.rs      | 17 +++++++++++++++++
 4 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index fa697287cf71b..d6c2b06ac4107 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -88,6 +88,8 @@
 #include <linux/workqueue.h>
 #include <linux/xarray.h>
 #include <trace/events/rust_sample.h>
+#include <linux/pm_wakeup.h>
+#include <linux/pm_wakeirq.h>
 
 /*
  * The driver-core Rust code needs to know about some C driver-core private
diff --git a/rust/helpers/device.c b/rust/helpers/device.c
index 9a4316bafedfb..5e31e42e8ff00 100644
--- a/rust/helpers/device.c
+++ b/rust/helpers/device.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 
 #include <linux/device.h>
+#include <linux/pm_wakeup.h>
 
 int rust_helper_devm_add_action(struct device *dev,
 				void (*action)(void *),
@@ -25,3 +26,8 @@ void rust_helper_dev_set_drvdata(struct device *dev, void *data)
 {
 	dev_set_drvdata(dev, data);
 }
+
+int rust_helper_devm_device_init_wakeup(struct device *dev)
+{
+	return devm_device_init_wakeup(dev);
+}
diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index 71b200df0f400..b8f7e185e32dd 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -5,7 +5,9 @@
 //! C header: [`include/linux/device.h`](srctree/include/linux/device.h)
 
 use crate::{
-    bindings, fmt,
+    bindings,
+    error::to_result,
+    fmt,
     prelude::*,
     sync::aref::ARef,
     types::{ForeignOwnable, Opaque},
@@ -324,6 +326,20 @@ pub fn drvdata<T: 'static>(&self) -> Result<Pin<&T>> {
         // - We've just checked that the type of the driver's private data is in fact `T`.
         Ok(unsafe { self.drvdata_unchecked() })
     }
+
+    /// Initialize device wakeup capability.
+    ///
+    /// Marks the device as wakeup-capable and enables wakeup. Both the
+    /// wakeup capability and wakeup enable state are automatically
+    /// cleared when the device is removed (resource-managed).
+    ///
+    /// Returns `Ok(())` on success, or an error code on failure.
+    pub fn devm_init_wakeup(&self) -> Result {
+        // SAFETY: `self.as_raw()` is a valid pointer to a `struct device`.
+        // The function is exported from bindings_helper module via pub use.
+        let ret = unsafe { bindings::devm_device_init_wakeup(self.as_raw()) };
+        to_result(ret)
+    }
 }
 
 impl<Ctx: DeviceContext> Device<Ctx> {
diff --git a/rust/kernel/irq/request.rs b/rust/kernel/irq/request.rs
index b150563fdef80..2484e4b53cdc3 100644
--- a/rust/kernel/irq/request.rs
+++ b/rust/kernel/irq/request.rs
@@ -120,6 +120,23 @@ pub(crate) unsafe fn new(dev: &'a Device<Bound>, irq: u32) -> Self {
     pub fn irq(&self) -> u32 {
         self.irq
     }
+
+    /// Attach the IRQ as a device wake IRQ.
+    ///
+    /// Attaches the device IO interrupt as a wake IRQ. The wake IRQ gets
+    /// automatically configured for wake-up from suspend based on the
+    /// device's sysfs wakeup entry. Typically called during driver probe
+    /// after calling `devm_init_wakeup()`.
+    ///
+    /// The wake IRQ is automatically cleared when the device is removed
+    /// (resource-managed).
+    ///
+    /// Returns `Ok(())` on success, or an error code on failure.
+    pub fn devm_set_wake_irq(&self) -> Result {
+        // SAFETY: `self.as_raw()` is a valid pointer to a `struct device`.
+        let ret = unsafe { bindings::devm_pm_set_wake_irq(self.dev.as_raw(), self.irq as i32) };
+        to_result(ret)
+    }
 }
 
 /// A registration of an IRQ handler for a given IRQ line.
-- 
2.43.0

[RFC PATCH v3 3/5] rust: add device wakeup capability support

[Ke Sun]

Add Rust bindings and wrappers for device wakeup functionality,
including devm_device_init_wakeup() and dev_pm_set_wake_irq().

Signed-off-by: Ke Sun <sunke@kylinos.cn>
---
Changes:
- Add new include headers in alphabetical order
---
 rust/bindings/bindings_helper.h |  2 ++
 rust/helpers/device.c           |  6 ++++++
 rust/kernel/device.rs           | 18 +++++++++++++++++-
 rust/kernel/irq/request.rs      | 17 +++++++++++++++++
 4 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index fa697287cf71b..327298fb39b00 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -72,6 +72,8 @@
 #include <linux/pid_namespace.h>
 #include <linux/platform_device.h>
 #include <linux/pm_opp.h>
+#include <linux/pm_wakeirq.h>
+#include <linux/pm_wakeup.h>
 #include <linux/poll.h>
 #include <linux/property.h>
 #include <linux/pwm.h>
diff --git a/rust/helpers/device.c b/rust/helpers/device.c
index 9a4316bafedfb..5e31e42e8ff00 100644
--- a/rust/helpers/device.c
+++ b/rust/helpers/device.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 
 #include <linux/device.h>
+#include <linux/pm_wakeup.h>
 
 int rust_helper_devm_add_action(struct device *dev,
 				void (*action)(void *),
@@ -25,3 +26,8 @@ void rust_helper_dev_set_drvdata(struct device *dev, void *data)
 {
 	dev_set_drvdata(dev, data);
 }
+
+int rust_helper_devm_device_init_wakeup(struct device *dev)
+{
+	return devm_device_init_wakeup(dev);
+}
diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index 71b200df0f400..b8f7e185e32dd 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -5,7 +5,9 @@
 //! C header: [`include/linux/device.h`](srctree/include/linux/device.h)
 
 use crate::{
-    bindings, fmt,
+    bindings,
+    error::to_result,
+    fmt,
     prelude::*,
     sync::aref::ARef,
     types::{ForeignOwnable, Opaque},
@@ -324,6 +326,20 @@ pub fn drvdata<T: 'static>(&self) -> Result<Pin<&T>> {
         // - We've just checked that the type of the driver's private data is in fact `T`.
         Ok(unsafe { self.drvdata_unchecked() })
     }
+
+    /// Initialize device wakeup capability.
+    ///
+    /// Marks the device as wakeup-capable and enables wakeup. Both the
+    /// wakeup capability and wakeup enable state are automatically
+    /// cleared when the device is removed (resource-managed).
+    ///
+    /// Returns `Ok(())` on success, or an error code on failure.
+    pub fn devm_init_wakeup(&self) -> Result {
+        // SAFETY: `self.as_raw()` is a valid pointer to a `struct device`.
+        // The function is exported from bindings_helper module via pub use.
+        let ret = unsafe { bindings::devm_device_init_wakeup(self.as_raw()) };
+        to_result(ret)
+    }
 }
 
 impl<Ctx: DeviceContext> Device<Ctx> {
diff --git a/rust/kernel/irq/request.rs b/rust/kernel/irq/request.rs
index b150563fdef80..2484e4b53cdc3 100644
--- a/rust/kernel/irq/request.rs
+++ b/rust/kernel/irq/request.rs
@@ -120,6 +120,23 @@ pub(crate) unsafe fn new(dev: &'a Device<Bound>, irq: u32) -> Self {
     pub fn irq(&self) -> u32 {
         self.irq
     }
+
+    /// Attach the IRQ as a device wake IRQ.
+    ///
+    /// Attaches the device IO interrupt as a wake IRQ. The wake IRQ gets
+    /// automatically configured for wake-up from suspend based on the
+    /// device's sysfs wakeup entry. Typically called during driver probe
+    /// after calling `devm_init_wakeup()`.
+    ///
+    /// The wake IRQ is automatically cleared when the device is removed
+    /// (resource-managed).
+    ///
+    /// Returns `Ok(())` on success, or an error code on failure.
+    pub fn devm_set_wake_irq(&self) -> Result {
+        // SAFETY: `self.as_raw()` is a valid pointer to a `struct device`.
+        let ret = unsafe { bindings::devm_pm_set_wake_irq(self.dev.as_raw(), self.irq as i32) };
+        to_result(ret)
+    }
 }
 
 /// A registration of an IRQ handler for a given IRQ line.
-- 
2.43.0

[RFC PATCH v3 4/5] rust: add RTC core abstractions and data structures

[Ke Sun]

Add Rust abstractions for RTC subsystem, including RtcDevice,
RtcTime, RtcWkAlrm data structures, RtcOps trait for driver
operations, and devm-managed device registration support.

Signed-off-by: Ke Sun <sunke@kylinos.cn>
---
 rust/helpers/helpers.c |    1 +
 rust/helpers/rtc.c     |    9 +
 rust/kernel/lib.rs     |    2 +
 rust/kernel/rtc.rs     | 1008 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 1020 insertions(+)
 create mode 100644 rust/helpers/rtc.c
 create mode 100644 rust/kernel/rtc.rs

diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index 79c72762ad9c4..1a5c103fb24ba 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -48,6 +48,7 @@
 #include "rcu.c"
 #include "refcount.c"
 #include "regulator.c"
+#include "rtc.c"
 #include "scatterlist.c"
 #include "security.c"
 #include "signal.c"
diff --git a/rust/helpers/rtc.c b/rust/helpers/rtc.c
new file mode 100644
index 0000000000000..862cd61670bfc
--- /dev/null
+++ b/rust/helpers/rtc.c
@@ -0,0 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/rtc.h>
+
+int rust_helper_devm_rtc_register_device(struct rtc_device *rtc)
+{
+	return __devm_rtc_register_device(THIS_MODULE, rtc);
+}
+
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 3e557335fc5fe..1390073e4ae27 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -135,6 +135,8 @@
 pub mod rbtree;
 pub mod regulator;
 pub mod revocable;
+#[cfg(CONFIG_RTC_CLASS)]
+pub mod rtc;
 pub mod scatterlist;
 pub mod security;
 pub mod seq_file;
diff --git a/rust/kernel/rtc.rs b/rust/kernel/rtc.rs
new file mode 100644
index 0000000000000..27066ef900f0a
--- /dev/null
+++ b/rust/kernel/rtc.rs
@@ -0,0 +1,1008 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! RTC (Real-Time Clock) device support.
+//!
+//! C header: [`include/linux/rtc.h`](srctree/include/linux/rtc.h).
+use crate::{
+    bindings,
+    bitmap::Bitmap,
+    container_of,
+    device, //
+    error::{
+        from_err_ptr,
+        to_result,
+        Error,
+        VTABLE_DEFAULT_ERROR, //
+    },
+    prelude::*,
+    seq_file::SeqFile,
+    sync::aref::{
+        ARef,
+        AlwaysRefCounted, //
+    },
+    types::{
+        ForeignOwnable,
+        Opaque, //
+    },
+};
+
+use core::{
+    ffi::c_void,
+    marker::PhantomData,
+    ptr::NonNull, //
+};
+
+/// RTC time structure.
+/// Mirrors [`struct rtc_time`](srctree/include/uapi/linux/rtc.h).
+#[repr(transparent)]
+#[derive(Clone, Copy)]
+pub struct RtcTime(pub bindings::rtc_time);
+
+impl RtcTime {
+    /// Creates a new `RtcTime` from a time64_t value.
+    pub fn from_time64(time: i64) -> Self {
+        let mut tm = Self(pin_init::zeroed());
+        // SAFETY: `rtc_time64_to_tm` is a pure function that only writes to
+        // the provided `struct rtc_time` pointer. The pointer is valid
+        // because `tm.0` is a valid mutable reference, and the function does
+        // not retain any references to it.
+        unsafe { bindings::rtc_time64_to_tm(time, &mut tm.0) };
+
+        tm
+    }
+
+    /// Converts this `RtcTime` to a time64_t value.
+    pub fn to_time64(&self) -> i64 {
+        // SAFETY: `rtc_tm_to_time64` is a pure function that only reads from
+        // the provided `struct rtc_time` pointer. The pointer is valid because
+        // `self.0` is a valid reference, and the function does not retain any
+        // references to it.
+        unsafe { bindings::rtc_tm_to_time64(core::ptr::from_ref(&self.0).cast_mut()) }
+    }
+
+    /// Sets this `RtcTime` from a time64_t value.
+    pub fn set_from_time64(&mut self, time: i64) {
+        // SAFETY: `rtc_time64_to_tm` is a pure function that only writes to
+        // the provided `struct rtc_time` pointer. The pointer is valid
+        // because `self.0` is a valid mutable reference, and the function
+        // does not retain any references to it.
+        unsafe { bindings::rtc_time64_to_tm(time, &mut self.0) };
+    }
+
+    /// Converts a time64_t value to an RTC time structure.
+    #[inline]
+    pub fn time64_to_tm(time: i64, tm: &mut Self) {
+        tm.set_from_time64(time);
+    }
+
+    /// Converts an RTC time structure to a time64_t value.
+    #[inline]
+    pub fn tm_to_time64(tm: &Self) -> i64 {
+        tm.to_time64()
+    }
+
+    /// Calculates the day of the year (0-365) from the date components.
+    #[inline]
+    pub fn year_days(&self) -> i32 {
+        // SAFETY: `rtc_year_days` is a pure function that only performs
+        // calculations based on the provided parameters. The values should be
+        // from valid RTC time structures and non-negative, but the function
+        // itself is safe to call with any values.
+        unsafe {
+            bindings::rtc_year_days(
+                self.0.tm_mday as u32,
+                self.0.tm_mon as u32,
+                self.0.tm_year as u32,
+            )
+        }
+    }
+
+    /// Returns the seconds field (0-59).
+    #[inline]
+    pub fn tm_sec(&self) -> i32 {
+        self.0.tm_sec
+    }
+
+    /// Sets the seconds field (0-59).
+    #[inline]
+    pub fn set_tm_sec(&mut self, sec: i32) {
+        self.0.tm_sec = sec;
+    }
+
+    /// Returns the minutes field (0-59).
+    #[inline]
+    pub fn tm_min(&self) -> i32 {
+        self.0.tm_min
+    }
+
+    /// Sets the minutes field (0-59).
+    #[inline]
+    pub fn set_tm_min(&mut self, min: i32) {
+        self.0.tm_min = min;
+    }
+
+    /// Returns the hours field (0-23).
+    #[inline]
+    pub fn tm_hour(&self) -> i32 {
+        self.0.tm_hour
+    }
+
+    /// Sets the hours field (0-23).
+    #[inline]
+    pub fn set_tm_hour(&mut self, hour: i32) {
+        self.0.tm_hour = hour;
+    }
+
+    /// Returns the day of the month (1-31).
+    #[inline]
+    pub fn tm_mday(&self) -> i32 {
+        self.0.tm_mday
+    }
+
+    /// Sets the day of the month (1-31).
+    #[inline]
+    pub fn set_tm_mday(&mut self, mday: i32) {
+        self.0.tm_mday = mday;
+    }
+
+    /// Returns the month (0-11, where 0 is January).
+    #[inline]
+    pub fn tm_mon(&self) -> i32 {
+        self.0.tm_mon
+    }
+
+    /// Sets the month (0-11, where 0 is January).
+    #[inline]
+    pub fn set_tm_mon(&mut self, mon: i32) {
+        self.0.tm_mon = mon;
+    }
+
+    /// Returns the year (years since 1900).
+    #[inline]
+    pub fn tm_year(&self) -> i32 {
+        self.0.tm_year
+    }
+
+    /// Sets the year (years since 1900).
+    #[inline]
+    pub fn set_tm_year(&mut self, year: i32) {
+        self.0.tm_year = year;
+    }
+
+    /// Returns the day of the week (0-6, where 0 is Sunday).
+    #[inline]
+    pub fn tm_wday(&self) -> i32 {
+        self.0.tm_wday
+    }
+
+    /// Sets the day of the week (0-6, where 0 is Sunday).
+    #[inline]
+    pub fn set_tm_wday(&mut self, wday: i32) {
+        self.0.tm_wday = wday;
+    }
+
+    /// Returns the day of the year (0-365).
+    #[inline]
+    pub fn tm_yday(&self) -> i32 {
+        self.0.tm_yday
+    }
+
+    /// Sets the day of the year (0-365).
+    #[inline]
+    pub fn set_tm_yday(&mut self, yday: i32) {
+        self.0.tm_yday = yday;
+    }
+
+    /// Returns the daylight saving time flag.
+    #[inline]
+    pub fn tm_isdst(&self) -> i32 {
+        self.0.tm_isdst
+    }
+
+    /// Sets the daylight saving time flag.
+    #[inline]
+    pub fn set_tm_isdst(&mut self, isdst: i32) {
+        self.0.tm_isdst = isdst;
+    }
+}
+
+impl Default for RtcTime {
+    fn default() -> Self {
+        Self(pin_init::zeroed())
+    }
+}
+
+/// RTC wake alarm structure.
+/// Mirrors [`struct rtc_wkalrm`](srctree/include/uapi/linux/rtc.h).
+#[repr(transparent)]
+#[derive(Clone, Copy)]
+pub struct RtcWkAlrm(pub bindings::rtc_wkalrm);
+
+impl Default for RtcWkAlrm {
+    fn default() -> Self {
+        Self(pin_init::zeroed())
+    }
+}
+
+impl RtcWkAlrm {
+    /// Returns a reference to the alarm time.
+    #[inline]
+    pub fn get_time(&self) -> &RtcTime {
+        // SAFETY: `RtcTime` is `#[repr(transparent)]` over
+        // `bindings::rtc_time`, so the memory layout is identical. We can
+        // safely reinterpret the reference.
+        unsafe { &*(&raw const self.0.time).cast::<RtcTime>() }
+    }
+
+    /// Returns a mutable reference to the alarm time.
+    #[inline]
+    pub fn get_time_mut(&mut self) -> &mut RtcTime {
+        // SAFETY: `RtcTime` is `#[repr(transparent)]` over
+        // `bindings::rtc_time`, so the memory layout is identical. We can
+        // safely reinterpret the reference.
+        unsafe { &mut *(&raw mut self.0.time).cast::<RtcTime>() }
+    }
+
+    /// Sets the alarm time.
+    #[inline]
+    pub fn set_time(&mut self, time: RtcTime) {
+        self.0.time = time.0;
+    }
+
+    /// Returns the enabled field.
+    #[inline]
+    pub fn enabled(&self) -> bool {
+        self.0.enabled != 0
+    }
+
+    /// Sets the `enabled` field.
+    #[inline]
+    pub fn set_enabled(&mut self, enabled: bool) {
+        self.0.enabled = u8::from(enabled);
+    }
+
+    /// Returns the pending field.
+    #[inline]
+    pub fn pending(&self) -> bool {
+        self.0.pending != 0
+    }
+
+    /// Sets the `pending` field.
+    #[inline]
+    pub fn set_pending(&mut self, pending: bool) {
+        self.0.pending = u8::from(pending);
+    }
+}
+
+/// RTC parameter structure.
+/// Mirrors [`struct rtc_param`](srctree/include/uapi/linux/rtc.h).
+#[repr(transparent)]
+#[derive(Clone, Copy)]
+pub struct RtcParam(pub bindings::rtc_param);
+
+impl Default for RtcParam {
+    fn default() -> Self {
+        Self(pin_init::zeroed())
+    }
+}
+
+/// Wrapper for an RTC device
+/// [`struct rtc_device`](srctree/include/linux/rtc.h).
+///
+/// # Invariants
+///
+/// A [`RtcDevice`] instance holds a pointer to a valid
+/// [`struct rtc_device`] that is registered and managed by the kernel.
+///
+#[repr(transparent)]
+pub struct RtcDevice<T: 'static = ()>(Opaque<bindings::rtc_device>, PhantomData<T>);
+
+impl<T: 'static> RtcDevice<T> {
+    /// Returns a raw pointer to the underlying `rtc_device`.
+    #[inline]
+    pub fn as_raw(&self) -> *mut bindings::rtc_device {
+        self.0.get()
+    }
+
+    /// Sets the minimum time range for the RTC device.
+    #[inline]
+    pub fn set_range_min(&self, min: i64) {
+        // SAFETY: By the type invariants, self.as_raw() is a valid pointer to a
+        // `struct rtc_device`, and we're only writing to the `range_min` field.
+        unsafe { (*self.as_raw()).range_min = min };
+    }
+
+    /// Sets the maximum time range for the RTC device.
+    #[inline]
+    pub fn set_range_max(&self, max: u64) {
+        // SAFETY: By the type invariants, self.as_raw() is a valid pointer to a
+        // `struct rtc_device`, and we're only writing to the `range_max` field.
+        unsafe { (*self.as_raw()).range_max = max };
+    }
+
+    /// Gets the minimum time range for the RTC device.
+    #[inline]
+    pub fn range_min(&self) -> i64 {
+        // SAFETY: By the type invariants, self.as_raw() is a valid pointer to a
+        // `struct rtc_device`, and we're only reading the `range_min` field.
+        unsafe { (*self.as_raw()).range_min }
+    }
+
+    /// Gets the maximum time range for the RTC device.
+    #[inline]
+    pub fn range_max(&self) -> u64 {
+        // SAFETY: By the type invariants, self.as_raw() is a valid pointer to a
+        // `struct rtc_device`, and we're only reading the `range_max` field.
+        unsafe { (*self.as_raw()).range_max }
+    }
+
+    /// Notifies the RTC framework that an interrupt has occurred.
+    #[inline]
+    pub fn update_irq(&self, num: usize, events: usize) {
+        // SAFETY: By the type invariants, self.as_raw() is a valid pointer
+        // to a `struct rtc_device`. The rtc_update_irq function handles
+        // NULL/ERR checks internally.
+        unsafe { bindings::rtc_update_irq(self.as_raw(), num, events) };
+    }
+
+    /// Clears a feature bit in the RTC device.
+    #[inline]
+    pub fn clear_feature(&self, feature: u32) {
+        // SAFETY: By the type invariants, self.as_raw() is a valid pointer to a
+        // `struct rtc_device`, and features is a valid bitmap array with
+        // RTC_FEATURE_CNT bits.
+        let features_bitmap = unsafe {
+            Bitmap::from_raw_mut(
+                (*self.as_raw()).features.as_mut_ptr().cast::<usize>(),
+                bindings::RTC_FEATURE_CNT as usize,
+            )
+        };
+        features_bitmap.clear_bit(feature as usize);
+    }
+}
+
+impl<T: 'static> AsRef<device::Device> for RtcDevice<T> {
+    fn as_ref(&self) -> &device::Device {
+        let raw = self.as_raw();
+        // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a
+        // pointer to a valid `struct rtc_device`.
+        let dev = unsafe { &raw mut (*raw).dev };
+
+        // SAFETY: `dev` points to a valid `struct device`.
+        unsafe { device::Device::from_raw(dev) }
+    }
+}
+
+// SAFETY: Instances of `RtcDevice` are always reference-counted via the
+// underlying `device`. The `struct rtc_device` contains a `struct device dev`
+// as its first field, and the reference counting is managed through
+// `get_device`/`put_device` on the `dev` field.
+unsafe impl<T: 'static> AlwaysRefCounted for RtcDevice<T> {
+    fn inc_ref(&self) {
+        let dev: &device::Device = self.as_ref();
+        // SAFETY: The existence of a shared reference guarantees that the
+        // refcount is non-zero. `dev.as_raw()` is a valid pointer to a
+        // `struct device` with a non-zero refcount.
+        unsafe { bindings::get_device(dev.as_raw()) };
+    }
+
+    unsafe fn dec_ref(obj: NonNull<Self>) {
+        let rtc: *mut bindings::rtc_device = obj.cast().as_ptr();
+
+        // SAFETY: By the type invariant of `Self`, `rtc` is a pointer to a
+        // valid `struct rtc_device`. The `dev` field is the first field of
+        // `struct rtc_device`, so we can safely access it.
+        let dev = unsafe { &raw mut (*rtc).dev };
+
+        // SAFETY: The safety requirements guarantee that the refcount is
+        // non-zero.
+        unsafe { bindings::put_device(dev) };
+    }
+}
+
+// SAFETY: `RtcDevice` is reference-counted and can be released from any thread.
+unsafe impl<T: 'static> Send for RtcDevice<T> {}
+
+// SAFETY: `RtcDevice` can be shared among threads because all immutable
+// methods are protected by the synchronization in `struct rtc_device`
+// (via `ops_lock` mutex).
+unsafe impl<T: 'static> Sync for RtcDevice<T> {}
+
+impl<T: RtcOps> RtcDevice<T> {
+    /// Allocates a new RTC device managed by devres.
+    pub fn new(
+        parent_dev: &device::Device<device::Bound>,
+        init: impl PinInit<T, Error>,
+    ) -> Result<ARef<Self>> {
+        // Allocate RTC device.
+        // SAFETY: `devm_rtc_allocate_device` returns a pointer to a
+        // devm-managed rtc_device. We use `dev_internal.as_raw()` which is
+        // `pub(crate)`, but we can access it through the same device pointer.
+        let rtc: *mut bindings::rtc_device =
+            from_err_ptr(unsafe { bindings::devm_rtc_allocate_device(parent_dev.as_raw()) })?;
+
+        // Set the RTC device ops.
+        // SAFETY: We just allocated the RTC device, so it's safe to set
+        // the ops.
+        unsafe { (*rtc).ops = Adapter::<T>::VTABLE.as_raw() };
+
+        // SAFETY: `rtc` is a valid pointer to a newly allocated rtc_device.
+        // `RtcDevice` is `#[repr(transparent)]` over `Opaque<rtc_device>`, so
+        // we can safely cast.
+        let rtcdev = unsafe { &*rtc.cast::<Self>() };
+        // SAFETY: `rtc_device.as_raw()` is a valid pointer to a `struct
+        // rtc_device`.
+        let flags = unsafe {
+            Bitmap::from_raw_mut(
+                &mut (*rtcdev.as_raw()).flags,
+                bindings::RTC_FEATURE_CNT as usize,
+            )
+        };
+        flags.set_bit(bindings::RTC_OPS_USE_RTC_DEV as usize);
+
+        rtcdev.set_drvdata(init)?;
+        Ok(rtcdev.into())
+    }
+
+    /// Store a pointer to the bound driver's private data.
+    fn set_drvdata(&self, data: impl PinInit<T, Error>) -> Result {
+        let data = KBox::pin_init(data, GFP_KERNEL)?;
+
+        // SAFETY: `self.as_raw()` is a valid pointer to a `struct rtc_device`.
+        unsafe { bindings::dev_set_drvdata(self.as_ref().as_raw(), data.into_foreign().cast()) };
+        Ok(())
+    }
+
+    /// Borrows the driver's private data bound to this [`RtcDevice`].
+    pub fn drvdata(&self) -> Result<Pin<&T>> {
+        // SAFETY: `self.as_raw()` is a valid pointer to a `struct device`.
+        let ptr = unsafe { bindings::dev_get_drvdata(self.as_ref().as_raw()) };
+
+        if ptr.is_null() {
+            return Err(ENOENT);
+        }
+
+        // SAFETY: The caller ensures that `ptr` is valid and writable.
+        Ok(unsafe { Pin::<KBox<T>>::borrow(ptr.cast()) })
+    }
+
+    /// Registers this RTC device with the RTC subsystem.
+    pub fn register(&self) -> Result {
+        // Registers an RTC device with the RTC subsystem.
+        // SAFETY: The device will be automatically unregistered when the parent
+        // device is removed (devm cleanup). The helper function uses
+        // `THIS_MODULE` internally.
+        to_result(unsafe { bindings::devm_rtc_register_device(self.as_raw()) })
+    }
+
+    /// Returns a reference to the parent device of this RTC device.
+    ///
+    /// # Safety
+    ///
+    /// The caller must guarantee that the parent device exists and is bound.
+    /// This is guaranteed by the RTC core during `RtcOps` callbacks.
+    pub unsafe fn bound_parent_device(&self) -> &device::Device<device::Bound> {
+        // SAFETY: Per the function's safety contract, the parent device exists.
+        let parent = unsafe { self.as_ref().parent().unwrap_unchecked() };
+
+        // SAFETY: Per the function's safety contract, the parent device is
+        // bound. This is guaranteed by the RTC core during `RtcOps`
+        // callbacks.
+        unsafe { parent.as_bound() }
+    }
+}
+
+impl<T: 'static> Drop for RtcDevice<T> {
+    fn drop(&mut self) {
+        let dev = self.as_ref().as_raw();
+
+        // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to
+        // a `struct device`.
+        let ptr: *mut c_void = unsafe { bindings::dev_get_drvdata(dev) };
+
+        // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to
+        // a `struct device`.
+        unsafe { bindings::dev_set_drvdata(dev, core::ptr::null_mut()) };
+
+        if !ptr.is_null() {
+            // SAFETY: `ptr` comes from a previous call to `into_foreign()`,
+            // and `dev_get_drvdata()` guarantees to return the same pointer
+            // given to `dev_set_drvdata()`.
+            unsafe { drop(Pin::<KBox<T>>::from_foreign(ptr.cast())) };
+        }
+    }
+}
+
+/// Options for creating an RTC device.
+#[derive(Copy, Clone)]
+pub struct RtcDeviceOptions {
+    /// The name of the RTC device.
+    pub name: &'static CStr,
+}
+
+/// Trait defining the operations for an RTC driver.
+#[vtable]
+pub trait RtcOps: Sized + 'static {
+    /// Reads the current time from the RTC.
+    fn read_time(
+        _rtcdev: &RtcDevice<Self>,
+        _tm: &mut RtcTime,
+        _parent_dev: &device::Device<device::Bound>,
+    ) -> Result {
+        build_error!(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Sets the time in the RTC.
+    fn set_time(
+        _rtcdev: &RtcDevice<Self>,
+        _tm: &RtcTime,
+        _parent_dev: &device::Device<device::Bound>,
+    ) -> Result {
+        build_error!(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Reads the alarm time from the RTC.
+    fn read_alarm(
+        _rtcdev: &RtcDevice<Self>,
+        _alarm: &mut RtcWkAlrm,
+        _parent_dev: &device::Device<device::Bound>,
+    ) -> Result {
+        build_error!(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Sets the alarm time in the RTC.
+    fn set_alarm(
+        _rtcdev: &RtcDevice<Self>,
+        _alarm: &RtcWkAlrm,
+        _parent_dev: &device::Device<device::Bound>,
+    ) -> Result {
+        build_error!(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Enables or disables the alarm interrupt.
+    fn alarm_irq_enable(
+        _rtcdev: &RtcDevice<Self>,
+        _enabled: bool,
+        _parent_dev: &device::Device<device::Bound>,
+    ) -> Result {
+        build_error!(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Handles custom ioctl commands.
+    fn ioctl(
+        _rtcdev: &RtcDevice<Self>,
+        _cmd: u32,
+        _arg: c_ulong,
+        _parent_dev: &device::Device<device::Bound>,
+    ) -> Result<c_int> {
+        build_error!(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Shows information in /proc/driver/rtc.
+    fn proc(
+        _rtcdev: &RtcDevice<Self>,
+        _seq: &mut SeqFile,
+        _parent_dev: &device::Device<device::Bound>,
+    ) -> Result {
+        build_error!(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Reads the time offset.
+    fn read_offset(
+        _rtcdev: &RtcDevice<Self>,
+        _parent_dev: &device::Device<device::Bound>,
+    ) -> Result<i64> {
+        build_error!(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Sets the time offset.
+    fn set_offset(
+        _rtcdev: &RtcDevice<Self>,
+        _offset: i64,
+        _parent_dev: &device::Device<device::Bound>,
+    ) -> Result {
+        build_error!(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Gets an RTC parameter.
+    fn param_get(
+        _rtcdev: &RtcDevice<Self>,
+        _param: &mut RtcParam,
+        _parent_dev: &device::Device<device::Bound>,
+    ) -> Result {
+        build_error!(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Sets an RTC parameter.
+    fn param_set(
+        _rtcdev: &RtcDevice<Self>,
+        _param: &RtcParam,
+        _parent_dev: &device::Device<device::Bound>,
+    ) -> Result {
+        build_error!(VTABLE_DEFAULT_ERROR)
+    }
+}
+
+struct Adapter<T: RtcOps> {
+    _p: PhantomData<T>,
+}
+
+impl<T: RtcOps> Adapter<T> {
+    const VTABLE: RtcOpsVTable = create_rtc_ops::<T>();
+
+    /// # Safety
+    ///
+    /// `dev` must be a valid pointer to the `struct device` embedded in a
+    /// `struct rtc_device`.
+    /// `tm` must be a valid pointer to a `struct rtc_time`.
+    unsafe extern "C" fn read_time(
+        dev: *mut bindings::device,
+        tm: *mut bindings::rtc_time,
+    ) -> c_int {
+        // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
+        // `AsBusDevice` to get it.
+        let rtc_dev =
+            unsafe { &*container_of!(dev, bindings::rtc_device, dev).cast::<RtcDevice<T>>() };
+        // SAFETY: The caller ensures that `tm` is valid and writable.
+        // `RtcTime` is `#[repr(transparent)]` over `bindings::rtc_time`, so we
+        // can safely cast.
+        let rtc_tm = unsafe { &mut *tm.cast::<RtcTime>() };
+        // SAFETY: The RTC core guarantees that the parent device exists and is
+        // bound during `RtcOps` callbacks.
+        let parent_dev = unsafe { rtc_dev.bound_parent_device() };
+
+        match T::read_time(rtc_dev, rtc_tm, parent_dev) {
+            Ok(()) => 0,
+            Err(err) => err.to_errno(),
+        }
+    }
+
+    /// # Safety
+    ///
+    /// `dev` must be a valid pointer to the `struct device` embedded in a
+    /// `struct rtc_device`.
+    /// `tm` must be a valid pointer to a `struct rtc_time`.
+    unsafe extern "C" fn set_time(
+        dev: *mut bindings::device,
+        tm: *mut bindings::rtc_time,
+    ) -> c_int {
+        // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
+        // `AsBusDevice` to get it.
+        let rtc_dev =
+            unsafe { &*container_of!(dev, bindings::rtc_device, dev).cast::<RtcDevice<T>>() };
+        // SAFETY: The caller ensures that `tm` is valid.
+        // `RtcTime` is `#[repr(transparent)]` over `bindings::rtc_time`, so we
+        // can safely cast.
+        let rtc_tm = unsafe { &*tm.cast::<RtcTime>() };
+        // SAFETY: The RTC core guarantees that the parent device exists and is
+        // bound during `RtcOps` callbacks.
+        let parent_dev = unsafe { rtc_dev.bound_parent_device() };
+
+        match T::set_time(rtc_dev, rtc_tm, parent_dev) {
+            Ok(()) => 0,
+            Err(err) => err.to_errno(),
+        }
+    }
+
+    /// # Safety
+    ///
+    /// `dev` must be a valid pointer to the `struct device` embedded in a
+    /// `struct rtc_device`.
+    /// `alarm` must be a valid pointer to a `struct rtc_wkalrm`.
+    unsafe extern "C" fn read_alarm(
+        dev: *mut bindings::device,
+        alarm: *mut bindings::rtc_wkalrm,
+    ) -> c_int {
+        // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
+        // `AsBusDevice` to get it.
+        let rtc_dev =
+            unsafe { &*container_of!(dev, bindings::rtc_device, dev).cast::<RtcDevice<T>>() };
+        // SAFETY: The caller ensures that `alarm` is valid and writable.
+        // `RtcWkAlrm` is `#[repr(transparent)]` over `bindings::rtc_wkalrm`, so
+        // we can safely cast.
+        let rtc_alarm = unsafe { &mut *alarm.cast::<RtcWkAlrm>() };
+        // SAFETY: The RTC core guarantees that the parent device exists and is
+        // bound during `RtcOps` callbacks.
+        let parent_dev = unsafe { rtc_dev.bound_parent_device() };
+
+        match T::read_alarm(rtc_dev, rtc_alarm, parent_dev) {
+            Ok(()) => 0,
+            Err(err) => err.to_errno(),
+        }
+    }
+
+    /// # Safety
+    ///
+    /// `dev` must be a valid pointer to the `struct device` embedded in a
+    /// `struct rtc_device`.
+    /// `alarm` must be a valid pointer to a `struct rtc_wkalrm`.
+    unsafe extern "C" fn set_alarm(
+        dev: *mut bindings::device,
+        alarm: *mut bindings::rtc_wkalrm,
+    ) -> c_int {
+        // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
+        // `AsBusDevice` to get it.
+        let rtc_dev =
+            unsafe { &*container_of!(dev, bindings::rtc_device, dev).cast::<RtcDevice<T>>() };
+        // SAFETY: The caller ensures that `alarm` is valid.
+        // `RtcWkAlrm` is `#[repr(transparent)]` over `bindings::rtc_wkalrm`, so
+        // we can safely cast.
+        let rtc_alarm = unsafe { &*alarm.cast::<RtcWkAlrm>() };
+        // SAFETY: The RTC core guarantees that the parent device exists and is
+        // bound during `RtcOps` callbacks.
+        let parent_dev = unsafe { rtc_dev.bound_parent_device() };
+
+        match T::set_alarm(rtc_dev, rtc_alarm, parent_dev) {
+            Ok(()) => 0,
+            Err(err) => err.to_errno(),
+        }
+    }
+
+    /// # Safety
+    ///
+    /// `dev` must be a valid pointer to the `struct device` embedded in a
+    /// `struct rtc_device`.
+    unsafe extern "C" fn alarm_irq_enable(dev: *mut bindings::device, enabled: c_uint) -> c_int {
+        // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
+        // `AsBusDevice` to get it.
+        let rtc_dev =
+            unsafe { &*container_of!(dev, bindings::rtc_device, dev).cast::<RtcDevice<T>>() };
+        // SAFETY: The RTC core guarantees that the parent device exists and is
+        // bound during `RtcOps` callbacks.
+        let parent_dev = unsafe { rtc_dev.bound_parent_device() };
+
+        match T::alarm_irq_enable(rtc_dev, enabled != 0, parent_dev) {
+            Ok(()) => 0,
+            Err(err) => err.to_errno(),
+        }
+    }
+
+    /// # Safety
+    ///
+    /// `dev` must be a valid pointer to the `struct device` embedded in a
+    /// `struct rtc_device`.
+    unsafe extern "C" fn ioctl(dev: *mut bindings::device, cmd: c_uint, arg: c_ulong) -> c_int {
+        // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
+        // `AsBusDevice` to get it.
+        let rtc_dev =
+            unsafe { &*container_of!(dev, bindings::rtc_device, dev).cast::<RtcDevice<T>>() };
+        // SAFETY: The RTC core guarantees that the parent device exists and is
+        // bound during `RtcOps` callbacks.
+        let parent_dev = unsafe { rtc_dev.bound_parent_device() };
+
+        match T::ioctl(rtc_dev, cmd, arg, parent_dev) {
+            Ok(ret) => ret,
+            Err(err) => err.to_errno(),
+        }
+    }
+
+    /// # Safety
+    ///
+    /// `dev` must be a valid pointer to the `struct device` embedded in a
+    /// `struct rtc_device`.
+    /// `seq` must be a valid pointer to a `struct seq_file`.
+    unsafe extern "C" fn proc(dev: *mut bindings::device, seq: *mut bindings::seq_file) -> c_int {
+        // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
+        // `AsBusDevice` to get it.
+        let rtc_dev =
+            unsafe { &*container_of!(dev, bindings::rtc_device, dev).cast::<RtcDevice<T>>() };
+        // SAFETY: The caller ensures that `seq` is valid and writable.
+        let seq_file = unsafe { &mut *seq.cast::<SeqFile>() };
+        // SAFETY: The RTC core guarantees that the parent device exists and is
+        // bound during `RtcOps` callbacks.
+        let parent_dev = unsafe { rtc_dev.bound_parent_device() };
+
+        match T::proc(rtc_dev, seq_file, parent_dev) {
+            Ok(()) => 0,
+            Err(err) => err.to_errno(),
+        }
+    }
+
+    /// # Safety
+    ///
+    /// `dev` must be a valid pointer to the `struct device` embedded in a
+    /// `struct rtc_device`.
+    /// `offset` must be a valid pointer to a `long`.
+    unsafe extern "C" fn read_offset(dev: *mut bindings::device, offset: *mut c_long) -> c_int {
+        // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
+        // `AsBusDevice` to get it.
+        let rtc_dev =
+            unsafe { &*container_of!(dev, bindings::rtc_device, dev).cast::<RtcDevice<T>>() };
+        // SAFETY: The RTC core guarantees that the parent device exists and is
+        // bound during `RtcOps` callbacks.
+        let parent_dev = unsafe { rtc_dev.bound_parent_device() };
+
+        match T::read_offset(rtc_dev, parent_dev) {
+            Ok(offset_val) => {
+                // SAFETY: The caller ensures that `offset` is valid and
+                // writable.
+                unsafe { *offset.cast() = offset_val as c_long };
+                0
+            }
+            Err(err) => err.to_errno(),
+        }
+    }
+
+    /// # Safety
+    ///
+    /// `dev` must be a valid pointer to the `struct device` embedded in a
+    /// `struct rtc_device`.
+    unsafe extern "C" fn set_offset(dev: *mut bindings::device, offset: c_long) -> c_int {
+        // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
+        // `AsBusDevice` to get it.
+        let rtc_dev =
+            unsafe { &*container_of!(dev, bindings::rtc_device, dev).cast::<RtcDevice<T>>() };
+        // SAFETY: The RTC core guarantees that the parent device exists and is
+        // bound during `RtcOps` callbacks.
+        let parent_dev = unsafe { rtc_dev.bound_parent_device() };
+
+        match T::set_offset(rtc_dev, offset as i64, parent_dev) {
+            Ok(()) => 0,
+            Err(err) => err.to_errno(),
+        }
+    }
+
+    /// # Safety
+    ///
+    /// `dev` must be a valid pointer to the `struct device` embedded in a
+    /// `struct rtc_device`.
+    /// `param` must be a valid pointer to a `struct rtc_param`.
+    unsafe extern "C" fn param_get(
+        dev: *mut bindings::device,
+        param: *mut bindings::rtc_param,
+    ) -> c_int {
+        // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
+        // `AsBusDevice` to get it.
+        let rtc_dev =
+            unsafe { &*container_of!(dev, bindings::rtc_device, dev).cast::<RtcDevice<T>>() };
+        // SAFETY: The caller ensures that `param` is valid and writable.
+        // `RtcParam` is `#[repr(transparent)]` over `bindings::rtc_param`,
+        // so we can safely cast.
+        let rtc_param = unsafe { &mut *param.cast::<RtcParam>() };
+        // SAFETY: The RTC core guarantees that the parent device exists and is
+        // bound during `RtcOps` callbacks.
+        let parent_dev = unsafe { rtc_dev.bound_parent_device() };
+
+        match T::param_get(rtc_dev, rtc_param, parent_dev) {
+            Ok(()) => 0,
+            Err(err) => err.to_errno(),
+        }
+    }
+
+    /// # Safety
+    ///
+    /// `dev` must be a valid pointer to the `struct device` embedded in a
+    /// `struct rtc_device`.
+    /// `param` must be a valid pointer to a `struct rtc_param`.
+    unsafe extern "C" fn param_set(
+        dev: *mut bindings::device,
+        param: *mut bindings::rtc_param,
+    ) -> c_int {
+        // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
+        // `AsBusDevice` to get it.
+        let rtc_dev =
+            unsafe { &*container_of!(dev, bindings::rtc_device, dev).cast::<RtcDevice<T>>() };
+        // SAFETY: The caller ensures that `param` is valid.
+        // `RtcParam` is `#[repr(transparent)]` over `bindings::rtc_param`,
+        // so we can safely cast.
+        let rtc_param = unsafe { &*param.cast::<RtcParam>() };
+        // SAFETY: The RTC core guarantees that the parent device exists and is
+        // bound during `RtcOps` callbacks.
+        let parent_dev = unsafe { rtc_dev.bound_parent_device() };
+
+        match T::param_set(rtc_dev, rtc_param, parent_dev) {
+            Ok(()) => 0,
+            Err(err) => err.to_errno(),
+        }
+    }
+}
+
+/// VTable structure wrapper for RTC operations.
+/// Mirrors [`struct rtc_class_ops`](srctree/include/linux/rtc.h).
+#[repr(transparent)]
+pub struct RtcOpsVTable(bindings::rtc_class_ops);
+
+// SAFETY: RtcOpsVTable is Send. The vtable contains only function pointers,
+// which are simple data types that can be safely moved across threads. The
+// thread-safety of calling these functions is handled by the kernel's locking
+// mechanisms.
+unsafe impl Send for RtcOpsVTable {}
+
+// SAFETY: RtcOpsVTable is Sync. The vtable is immutable after it is created,
+// so it can be safely referenced and accessed concurrently by multiple threads
+// e.g. to read the function pointers.
+unsafe impl Sync for RtcOpsVTable {}
+
+impl RtcOpsVTable {
+    /// Returns a raw pointer to the underlying `rtc_class_ops` struct.
+    pub(crate) const fn as_raw(&self) -> *const bindings::rtc_class_ops {
+        &self.0
+    }
+}
+
+/// Creates an RTC operations vtable for a type `T` that implements
+/// `RtcOps`.
+pub const fn create_rtc_ops<T: RtcOps>() -> RtcOpsVTable {
+    let mut ops: bindings::rtc_class_ops = pin_init::zeroed();
+
+    ops.read_time = if T::HAS_READ_TIME {
+        Some(Adapter::<T>::read_time)
+    } else {
+        None
+    };
+    ops.set_time = if T::HAS_SET_TIME {
+        Some(Adapter::<T>::set_time)
+    } else {
+        None
+    };
+    ops.read_alarm = if T::HAS_READ_ALARM {
+        Some(Adapter::<T>::read_alarm)
+    } else {
+        None
+    };
+    ops.set_alarm = if T::HAS_SET_ALARM {
+        Some(Adapter::<T>::set_alarm)
+    } else {
+        None
+    };
+    ops.alarm_irq_enable = if T::HAS_ALARM_IRQ_ENABLE {
+        Some(Adapter::<T>::alarm_irq_enable)
+    } else {
+        None
+    };
+    ops.ioctl = if T::HAS_IOCTL {
+        Some(Adapter::<T>::ioctl)
+    } else {
+        None
+    };
+    ops.proc_ = if T::HAS_PROC {
+        Some(Adapter::<T>::proc)
+    } else {
+        None
+    };
+    ops.read_offset = if T::HAS_READ_OFFSET {
+        Some(Adapter::<T>::read_offset)
+    } else {
+        None
+    };
+    ops.set_offset = if T::HAS_SET_OFFSET {
+        Some(Adapter::<T>::set_offset)
+    } else {
+        None
+    };
+    ops.param_get = if T::HAS_PARAM_GET {
+        Some(Adapter::<T>::param_get)
+    } else {
+        None
+    };
+    ops.param_set = if T::HAS_PARAM_SET {
+        Some(Adapter::<T>::param_set)
+    } else {
+        None
+    };
+
+    RtcOpsVTable(ops)
+}
+
+/// Declares a kernel module that exposes a single RTC driver.
+///
+/// # Examples
+///
+///```ignore
+/// kernel::module_rtc_platform_driver! {
+///     type: MyDriver,
+///     name: "Module name",
+///     authors: ["Author name"],
+///     description: "Description",
+///     license: "GPL v2",
+/// }
+///```
+#[macro_export]
+macro_rules! module_rtc_platform_driver {
+    ($($user_args:tt)*) => {
+        $crate::module_platform_driver! {
+            $($user_args)*
+            imports_ns: ["RTC"],
+        }
+    };
+}
-- 
2.43.0

[RFC PATCH v3 5/5] rust: add PL031 RTC driver

[Ke Sun]

Add Rust implementation of the PL031 RTC driver.

Signed-off-by: Ke Sun <sunke@kylinos.cn>
---
 drivers/rtc/Kconfig           |   9 +
 drivers/rtc/Makefile          |   1 +
 drivers/rtc/rtc_pl031_rust.rs | 513 ++++++++++++++++++++++++++++++++++
 3 files changed, 523 insertions(+)
 create mode 100644 drivers/rtc/rtc_pl031_rust.rs

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 50dc779f7f983..137cea1824edd 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1591,6 +1591,15 @@ config RTC_DRV_PL031
 	  To compile this driver as a module, choose M here: the
 	  module will be called rtc-pl031.
 
+config RTC_DRV_PL031_RUST
+	tristate "ARM AMBA PL031 RTC (Rust)"
+	depends on RUST && RTC_CLASS
+	help
+	  This is the Rust implementation of the PL031 RTC driver.
+	  It provides the same functionality as the C driver but is
+	  written in Rust for improved memory safety. The driver supports
+	  ARM, ST v1, and ST v2 variants of the PL031 RTC controller.
+
 config RTC_DRV_AT91RM9200
 	tristate "AT91RM9200 or some AT91SAM9 RTC"
 	depends on ARCH_AT91 || COMPILE_TEST
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 6cf7e066314e1..10f540e7409b4 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -139,6 +139,7 @@ obj-$(CONFIG_RTC_DRV_PCF8583)	+= rtc-pcf8583.o
 obj-$(CONFIG_RTC_DRV_PIC32)	+= rtc-pic32.o
 obj-$(CONFIG_RTC_DRV_PL030)	+= rtc-pl030.o
 obj-$(CONFIG_RTC_DRV_PL031)	+= rtc-pl031.o
+obj-$(CONFIG_RTC_DRV_PL031_RUST)	+= rtc_pl031_rust.o
 obj-$(CONFIG_RTC_DRV_PM8XXX)	+= rtc-pm8xxx.o
 obj-$(CONFIG_RTC_DRV_POLARFIRE_SOC)	+= rtc-mpfs.o
 obj-$(CONFIG_RTC_DRV_PS3)	+= rtc-ps3.o
diff --git a/drivers/rtc/rtc_pl031_rust.rs b/drivers/rtc/rtc_pl031_rust.rs
new file mode 100644
index 0000000000000..8ae48d96d1f94
--- /dev/null
+++ b/drivers/rtc/rtc_pl031_rust.rs
@@ -0,0 +1,513 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+//! Rust ARM AMBA PrimeCell 031 RTC driver
+//!
+//! This driver provides Real Time Clock functionality for ARM AMBA PrimeCell
+//! 031 RTC controllers and their ST Microelectronics derivatives.
+
+use core::marker::PhantomPinned;
+use kernel::{
+    amba,
+    bindings,
+    c_str,
+    device::{
+        self,
+        Core, //
+    },
+    devres::Devres,
+    io::mem::IoMem,
+    irq::{
+        self,
+        Handler,
+        IrqReturn, //
+    },
+    prelude::*,
+    rtc::{
+        RtcDevice,
+        RtcOps,
+        RtcTime,
+        RtcWkAlrm, //
+    },
+    sync::aref::ARef, //
+};
+
+// Register offsets
+const RTC_DR: usize = 0x00; // Data read register
+const RTC_MR: usize = 0x04; // Match register
+const RTC_LR: usize = 0x08; // Data load register
+const RTC_CR: usize = 0x0c; // Control register
+const RTC_IMSC: usize = 0x10; // Interrupt mask and set register
+const RTC_RIS: usize = 0x14; // Raw interrupt status register
+const RTC_MIS: usize = 0x18; // Masked interrupt status register
+const RTC_ICR: usize = 0x1c; // Interrupt clear register
+
+// ST variants have additional timer functionality
+#[allow(dead_code)]
+const RTC_TDR: usize = 0x20; // Timer data read register
+#[allow(dead_code)]
+const RTC_TLR: usize = 0x24; // Timer data load register
+#[allow(dead_code)]
+const RTC_TCR: usize = 0x28; // Timer control register
+const RTC_YDR: usize = 0x30; // Year data read register
+const RTC_YMR: usize = 0x34; // Year match register
+const RTC_YLR: usize = 0x38; // Year data load register
+const PL031_REG_SIZE: usize = RTC_YLR + 4;
+
+// Control register bits
+const RTC_CR_EN: u32 = 1 << 0; // Counter enable bit
+const RTC_CR_CWEN: u32 = 1 << 26; // Clockwatch enable bit
+
+#[allow(dead_code)]
+const RTC_TCR_EN: u32 = 1 << 1; // Periodic timer enable bit
+
+// Interrupt status and control register bits
+const RTC_BIT_AI: u32 = 1 << 0; // Alarm interrupt bit
+#[allow(dead_code)]
+const RTC_BIT_PI: u32 = 1 << 1; // Periodic interrupt bit (ST variants only)
+
+// RTC event flags
+#[allow(dead_code)]
+const RTC_AF: u32 = bindings::RTC_AF;
+#[allow(dead_code)]
+const RTC_IRQF: u32 = bindings::RTC_IRQF;
+
+// ST v2 time format bit definitions
+const RTC_SEC_SHIFT: u32 = 0;
+const RTC_SEC_MASK: u32 = 0x3F << RTC_SEC_SHIFT; // Second [0-59]
+const RTC_MIN_SHIFT: u32 = 6;
+const RTC_MIN_MASK: u32 = 0x3F << RTC_MIN_SHIFT; // Minute [0-59]
+const RTC_HOUR_SHIFT: u32 = 12;
+const RTC_HOUR_MASK: u32 = 0x1F << RTC_HOUR_SHIFT; // Hour [0-23]
+const RTC_WDAY_SHIFT: u32 = 17;
+const RTC_WDAY_MASK: u32 = 0x7 << RTC_WDAY_SHIFT; // Day of Week [1-7] 1=Sunday
+const RTC_MDAY_SHIFT: u32 = 20;
+const RTC_MDAY_MASK: u32 = 0x1F << RTC_MDAY_SHIFT; // Day of Month [1-31]
+const RTC_MON_SHIFT: u32 = 25;
+const RTC_MON_MASK: u32 = 0xF << RTC_MON_SHIFT; // Month [1-12] 1=January
+
+/// Converts a binary value to BCD.
+fn bin2bcd(val: u8) -> u8 {
+    ((val / 10) << 4) | (val % 10)
+}
+
+/// Converts a BCD value to binary.
+fn bcd2bin(val: u8) -> u8 {
+    ((val >> 4) * 10) + (val & 0x0F)
+}
+
+/// Converts a Gregorian date to ST v2 RTC packed BCD format.
+///
+/// Returns a tuple of (packed_time, bcd_year) where packed_time contains
+/// month, day, weekday, hour, minute, and second in a single 32-bit value.
+fn stv2_tm_to_time(tm: &RtcTime) -> Result<(u32, u32)> {
+    let year = tm.tm_year() + 1900;
+    let mut wday = tm.tm_wday();
+
+    // Hardware wday masking doesn't work, so wday must be valid.
+    if !(-1..=6).contains(&wday) {
+        return Err(EINVAL);
+    } else if wday == -1 {
+        // wday is not provided, calculate it here.
+        let time64 = tm.to_time64();
+        let mut calc_tm = RtcTime::default();
+        calc_tm.set_from_time64(time64);
+        wday = calc_tm.tm_wday();
+    }
+
+    // Convert year to BCD.
+    let bcd_year =
+        (u32::from(bin2bcd((year % 100) as u8))) | (u32::from(bin2bcd((year / 100) as u8)) << 8);
+
+    let st_time = ((tm.tm_mon() + 1) as u32) << RTC_MON_SHIFT
+        | (tm.tm_mday() as u32) << RTC_MDAY_SHIFT
+        | ((wday + 1) as u32) << RTC_WDAY_SHIFT
+        | (tm.tm_hour() as u32) << RTC_HOUR_SHIFT
+        | (tm.tm_min() as u32) << RTC_MIN_SHIFT
+        | (tm.tm_sec() as u32) << RTC_SEC_SHIFT;
+
+    Ok((st_time, bcd_year))
+}
+
+/// Converts ST v2 RTC packed BCD format to a Gregorian date.
+///
+/// Extracts time components from the packed 32-bit value and BCD year register,
+/// then returns an RtcTime structure.
+fn stv2_time_to_tm(st_time: u32, bcd_year: u32) -> RtcTime {
+    let year_low = bcd2bin((bcd_year & 0xFF) as u8);
+    let year_high = bcd2bin(((bcd_year >> 8) & 0xFF) as u8);
+    let mut tm = RtcTime::default();
+    tm.set_tm_year(i32::from(year_low) + i32::from(year_high) * 100);
+    tm.set_tm_mon((((st_time & RTC_MON_MASK) >> RTC_MON_SHIFT) - 1) as i32);
+    tm.set_tm_mday(((st_time & RTC_MDAY_MASK) >> RTC_MDAY_SHIFT) as i32);
+    tm.set_tm_wday((((st_time & RTC_WDAY_MASK) >> RTC_WDAY_SHIFT) - 1) as i32);
+    tm.set_tm_hour(((st_time & RTC_HOUR_MASK) >> RTC_HOUR_SHIFT) as i32);
+    tm.set_tm_min(((st_time & RTC_MIN_MASK) >> RTC_MIN_SHIFT) as i32);
+    tm.set_tm_sec(((st_time & RTC_SEC_MASK) >> RTC_SEC_SHIFT) as i32);
+
+    // Values are from valid RTC time structures and are non-negative.
+    tm.set_tm_yday(tm.year_days());
+    tm.set_tm_year(tm.tm_year() - 1900);
+    tm
+}
+
+/// Vendor-specific variant identifier for PL031 RTC controllers.
+#[derive(Copy, Clone, PartialEq)]
+enum VendorVariant {
+    /// Original ARM version with 32-bit Unix timestamp format.
+    Arm,
+    /// First ST derivative with clockwatch mode and weekday support.
+    StV1,
+    /// Second ST derivative with packed BCD time format and year register.
+    StV2,
+}
+
+impl VendorVariant {
+    fn clockwatch(&self) -> bool {
+        matches!(self, VendorVariant::StV1 | VendorVariant::StV2)
+    }
+
+    #[allow(dead_code)]
+    fn st_weekday(&self) -> bool {
+        matches!(self, VendorVariant::StV1 | VendorVariant::StV2)
+    }
+
+    #[allow(dead_code)]
+    fn range_min(&self) -> i64 {
+        match self {
+            VendorVariant::Arm | VendorVariant::StV1 => 0,
+            VendorVariant::StV2 => bindings::RTC_TIMESTAMP_BEGIN_0000,
+        }
+    }
+
+    #[allow(dead_code)]
+    fn range_max(&self) -> u64 {
+        match self {
+            VendorVariant::Arm | VendorVariant::StV1 => u64::from(u32::MAX),
+            VendorVariant::StV2 => bindings::RTC_TIMESTAMP_END_9999,
+        }
+    }
+}
+
+/// The driver's private data struct. It holds all necessary devres managed
+/// resources.
+#[pin_data]
+struct Pl031DrvData {
+    #[pin]
+    regs: Devres<IoMem<PL031_REG_SIZE>>,
+    hw_variant: VendorVariant,
+}
+
+// SAFETY: `Pl031DrvData` contains only `Send`/`Sync` types: `Devres`
+// (Send+Sync) and `VendorVariant` (Copy).
+unsafe impl Send for Pl031DrvData {}
+// SAFETY: `Pl031DrvData` contains only `Send`/`Sync` types: `Devres`
+// (Send+Sync) and `VendorVariant` (Copy).
+unsafe impl Sync for Pl031DrvData {}
+
+/// Vendor-specific variant identifier used in AMBA device table.
+#[derive(Copy, Clone)]
+struct Pl031Variant {
+    variant: VendorVariant,
+}
+
+impl Pl031Variant {
+    const ARM: Self = Self {
+        variant: VendorVariant::Arm,
+    };
+    const STV1: Self = Self {
+        variant: VendorVariant::StV1,
+    };
+    const STV2: Self = Self {
+        variant: VendorVariant::StV2,
+    };
+}
+
+// Use AMBA device table for matching
+kernel::amba_device_table!(
+    ID_TABLE,
+    MODULE_ID_TABLE,
+    <Pl031AmbaDriver as amba::Driver>::IdInfo,
+    [
+        (
+            amba::DeviceId::new(0x00041031, 0x000fffff),
+            Pl031Variant::ARM
+        ),
+        (
+            amba::DeviceId::new(0x00180031, 0x00ffffff),
+            Pl031Variant::STV1
+        ),
+        (
+            amba::DeviceId::new(0x00280031, 0x00ffffff),
+            Pl031Variant::STV2
+        ),
+    ]
+);
+
+#[pin_data]
+struct Pl031AmbaDriver {
+    #[pin]
+    irqreg: irq::Registration<Pl031IrqHandler>,
+}
+
+impl amba::Driver for Pl031AmbaDriver {
+    type IdInfo = Pl031Variant;
+    const AMBA_ID_TABLE: amba::IdTable<Self::IdInfo> = &ID_TABLE;
+
+    fn probe(
+        adev: &amba::Device<Core>,
+        id_info: Option<&Self::IdInfo>,
+    ) -> impl PinInit<Self, Error> {
+        pin_init::pin_init_scope(move || {
+            let dev = adev.as_ref();
+            let io_request = adev.io_request().ok_or(ENODEV)?;
+            let variant = id_info
+                .map(|info| info.variant)
+                .unwrap_or(VendorVariant::Arm);
+
+            let rtcdev = RtcDevice::<Pl031DrvData>::new(
+                dev,
+                try_pin_init!(Pl031DrvData {
+                    regs <- IoMem::new(io_request),
+                    hw_variant: variant,
+                }),
+            )?;
+
+            dev.devm_init_wakeup()?;
+
+            let drvdata = rtcdev.drvdata()?;
+            let regs = drvdata.regs.access(dev)?;
+
+            // Enable the clockwatch on ST Variants
+            let mut cr = regs.read32(RTC_CR);
+            if variant.clockwatch() {
+                cr |= RTC_CR_CWEN;
+            } else {
+                cr |= RTC_CR_EN;
+            }
+            regs.write32(cr, RTC_CR);
+
+            // On ST PL031 variants, the RTC reset value does not provide
+            // correct weekday for 2000-01-01. Correct the erroneous sunday
+            // to saturday.
+            if variant.st_weekday() {
+                let bcd_year = regs.read32(RTC_YDR);
+                if bcd_year == 0x2000 {
+                    let st_time = regs.read32(RTC_DR);
+                    if (st_time & (RTC_MON_MASK | RTC_MDAY_MASK | RTC_WDAY_MASK)) == 0x02120000 {
+                        regs.write32(0x2000, RTC_YLR);
+                        regs.write32(st_time | (0x7 << RTC_WDAY_SHIFT), RTC_LR);
+                    }
+                }
+            }
+
+            rtcdev.set_range_min(variant.range_min());
+            rtcdev.set_range_max(variant.range_max());
+
+            // This variant shares the IRQ with another block and must not
+            // suspend that IRQ line.
+            let irq_flags = if variant == VendorVariant::StV2 {
+                kernel::irq::Flags::SHARED | kernel::irq::Flags::COND_SUSPEND
+            } else {
+                kernel::irq::Flags::SHARED
+            };
+
+            if adev
+                .irq_by_index(0)
+                .and_then(|irq| irq.devm_set_wake_irq())
+                .is_err()
+            {
+                rtcdev.clear_feature(bindings::RTC_FEATURE_ALARM);
+            }
+
+            rtcdev.register()?;
+
+            Ok(try_pin_init!(Pl031AmbaDriver {
+                irqreg <- adev.request_irq_by_index(
+                    irq_flags,
+                    0,
+                    c_str!("rtc-pl031"),
+                    try_pin_init!(Pl031IrqHandler {
+                        _pin: PhantomPinned,
+                        rtcdev: rtcdev.clone(),
+                    }),
+                ),
+            }))
+        })
+    }
+}
+
+/// Interrupt handler for PL031 RTC alarm events.
+#[pin_data]
+struct Pl031IrqHandler {
+    #[pin]
+    _pin: PhantomPinned,
+    rtcdev: ARef<RtcDevice<Pl031DrvData>>,
+}
+
+impl Handler for Pl031IrqHandler {
+    fn handle(&self, dev: &device::Device<device::Bound>) -> IrqReturn {
+        // Get driver data using drvdata.
+        let drvdata = match self.rtcdev.drvdata() {
+            Ok(drvdata) => drvdata,
+            Err(_) => return IrqReturn::None,
+        };
+
+        // Access the MMIO registers.
+        let regs = match drvdata.regs.access(dev) {
+            Ok(regs) => regs,
+            Err(_) => return IrqReturn::None,
+        };
+
+        // Read masked interrupt status.
+        let rtcmis = regs.read32(RTC_MIS);
+
+        if (rtcmis & RTC_BIT_AI) != 0 {
+            regs.write32(RTC_BIT_AI, RTC_ICR);
+            self.rtcdev.update_irq(1, (RTC_AF | RTC_IRQF) as usize);
+            return IrqReturn::Handled;
+        }
+
+        IrqReturn::None
+    }
+}
+
+#[vtable]
+impl RtcOps for Pl031DrvData {
+    fn read_time(
+        rtcdev: &RtcDevice<Self>,
+        tm: &mut RtcTime,
+        parent_dev: &device::Device<device::Bound>,
+    ) -> Result {
+        let drvdata = rtcdev.drvdata()?;
+        let regs = drvdata.regs.access(parent_dev)?;
+
+        match drvdata.hw_variant {
+            VendorVariant::Arm | VendorVariant::StV1 => {
+                let time32: u32 = regs.read32(RTC_DR);
+                let time64 = i64::from(time32);
+                tm.set_from_time64(time64);
+            }
+            VendorVariant::StV2 => {
+                let st_time = regs.read32(RTC_DR);
+                let bcd_year = regs.read32(RTC_YDR);
+                *tm = stv2_time_to_tm(st_time, bcd_year);
+            }
+        }
+
+        Ok(())
+    }
+
+    fn set_time(
+        rtcdev: &RtcDevice<Self>,
+        tm: &RtcTime,
+        parent_dev: &device::Device<device::Bound>,
+    ) -> Result {
+        let dev: &device::Device = rtcdev.as_ref();
+        let drvdata = rtcdev.drvdata()?;
+        let regs = drvdata.regs.access(parent_dev)?;
+
+        match drvdata.hw_variant {
+            VendorVariant::Arm | VendorVariant::StV1 => {
+                let time64 = tm.to_time64();
+                regs.write32(time64 as u32, RTC_LR);
+            }
+            VendorVariant::StV2 => {
+                let (st_time, bcd_year) = stv2_tm_to_time(tm).inspect_err(|&err| {
+                    if err == EINVAL {
+                        dev_err!(dev, "invalid wday value {}\n", tm.tm_wday());
+                    }
+                })?;
+                regs.write32(bcd_year, RTC_YLR);
+                regs.write32(st_time, RTC_LR);
+            }
+        }
+
+        Ok(())
+    }
+
+    fn read_alarm(
+        rtcdev: &RtcDevice<Self>,
+        alarm: &mut RtcWkAlrm,
+        parent_dev: &device::Device<device::Bound>,
+    ) -> Result {
+        let drvdata = rtcdev.drvdata()?;
+        let regs = drvdata.regs.access(parent_dev)?;
+
+        match drvdata.hw_variant {
+            VendorVariant::Arm | VendorVariant::StV1 => {
+                let time32: u32 = regs.read32(RTC_MR);
+                let time64 = i64::from(time32);
+                RtcTime::time64_to_tm(time64, alarm.get_time_mut());
+            }
+            VendorVariant::StV2 => {
+                let st_time = regs.read32(RTC_MR);
+                let bcd_year = regs.read32(RTC_YMR);
+                *alarm.get_time_mut() = stv2_time_to_tm(st_time, bcd_year);
+            }
+        }
+
+        alarm.set_pending((regs.read32(RTC_RIS) & RTC_BIT_AI) != 0);
+        alarm.set_enabled((regs.read32(RTC_IMSC) & RTC_BIT_AI) != 0);
+
+        Ok(())
+    }
+
+    fn set_alarm(
+        rtcdev: &RtcDevice<Self>,
+        alarm: &RtcWkAlrm,
+        parent_dev: &device::Device<device::Bound>,
+    ) -> Result {
+        let dev: &device::Device = rtcdev.as_ref();
+        let drvdata = rtcdev.drvdata()?;
+        let regs = drvdata.regs.access(parent_dev)?;
+
+        match drvdata.hw_variant {
+            VendorVariant::Arm | VendorVariant::StV1 => {
+                let time64 = alarm.get_time().to_time64();
+                regs.write32(time64 as u32, RTC_MR);
+            }
+            VendorVariant::StV2 => {
+                let (st_time, bcd_year) =
+                    stv2_tm_to_time(alarm.get_time()).inspect_err(|&err| {
+                        if err == EINVAL {
+                            dev_err!(dev, "invalid wday value {}\n", alarm.get_time().tm_wday());
+                        }
+                    })?;
+                regs.write32(bcd_year, RTC_YMR);
+                regs.write32(st_time, RTC_MR);
+            }
+        }
+
+        Self::alarm_irq_enable(rtcdev, alarm.enabled(), parent_dev)
+    }
+
+    fn alarm_irq_enable(
+        rtcdev: &RtcDevice<Self>,
+        enabled: bool,
+        parent_dev: &device::Device<device::Bound>,
+    ) -> Result {
+        let drvdata = rtcdev.drvdata()?;
+        let regs = drvdata.regs.access(parent_dev)?;
+
+        // Clear any pending alarm interrupts.
+        regs.write32(RTC_BIT_AI, RTC_ICR);
+
+        let mut imsc = regs.read32(RTC_IMSC);
+        if enabled {
+            imsc |= RTC_BIT_AI;
+        } else {
+            imsc &= !RTC_BIT_AI;
+        }
+        regs.write32(imsc, RTC_IMSC);
+
+        Ok(())
+    }
+}
+
+kernel::module_amba_driver! {
+    type: Pl031AmbaDriver,
+    name: "rtc-pl031-rust",
+    authors: ["Ke Sun <sunke@kylinos.cn>"],
+    description: "Rust PL031 RTC driver",
+    license: "GPL v2",
+    imports_ns: ["RTC"],
+}
-- 
2.43.0

Re: [RFC PATCH v3 5/5] rust: add PL031 RTC driver

[Ke Sun]

This driver has been tested on QEMU with the following command:
qemu-system-aarch64 -machine virt,virtualization=on -cpu max -smp 2 -m 4g \
   -serial mon:stdio -kernel arch/arm64/boot/Image  -hda 
PATHTO/virtualdisk.qcow2 \
   -append "root=/dev/vdaX no_console_suspend loglevel=8 console=ttyAMA0" \
   -display none -nic user,hostfwd=tcp::10022-:22

Test Results:

Driver Registration:
    [    1.886444][    T1] rtc-pl031-rust 9010000.pl031: registered as rtc0
    [    1.888070][    T1] rtc-pl031-rust 9010000.pl031: setting system 
clock to 2026-01-19T08:49:36 UTC (1768812576)

Interrupt Registration:
    # cat /proc/interrupts |grep pl031
     21:          0          0    GICv2  34 Level     rtc-pl031

RTC Time Reading:
    # hwclock -r
    2026-01-19 09:03:24.961787+08:00

Suspend/Resume with Alarm:
    # rtcwake --mode mem --seconds 5
    rtcwake: wakeup from "mem" using /dev/rtc0 at Mon Jan 19 09:04:13 2026
    [  242.699314][ T2832] PM: suspend entry (s2idle)
    [  242.717296][   T12] Filesystems sync: 0.009 seconds
    [  242.741471][ T2832] Freezing user space processes
    [  242.761472][ T2832] Freezing user space processes completed 
(elapsed 0.019 seconds)
    [  242.763281][ T2832] OOM killer disabled.
    [  242.763681][ T2832] Freezing remaining freezable tasks
    [  242.766629][ T2832] Freezing remaining freezable tasks completed 
(elapsed 0.002 seconds)
    [  248.678304][   T82] virtio_blk virtio1: 2/0/0 default/read/poll 
queues
    [  248.692577][ T2832] OOM killer enabled.
    [  248.692724][ T2832] Restarting tasks: Starting
    [  248.744504][ T2832] Restarting tasks: Done
    [  248.745530][ T2832] random: crng reseeded on system resumption
    [  248.746749][ T2832] PM: suspend exit

Driver Rebind:
    /sys/bus/amba/drivers/rtc-pl031-rust# echo 9010000.pl031 > unbind
    /sys/bus/amba/drivers/rtc-pl031-rust# echo 9010000.pl031 > bind
    [  334.060940][ T1837] rtc-pl031-rust 9010000.pl031: registered as rtc1

All tests passed successfully.

On 1/17/26 00:34, Ke Sun wrote:
> Add Rust implementation of the PL031 RTC driver.
>
> Signed-off-by: Ke Sun <sunke@kylinos.cn>
> ---
>   drivers/rtc/Kconfig           |   9 +
>   drivers/rtc/Makefile          |   1 +
>   drivers/rtc/rtc_pl031_rust.rs | 513 ++++++++++++++++++++++++++++++++++
>   3 files changed, 523 insertions(+)
>   create mode 100644 drivers/rtc/rtc_pl031_rust.rs
>
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index 50dc779f7f983..137cea1824edd 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -1591,6 +1591,15 @@ config RTC_DRV_PL031
>   	  To compile this driver as a module, choose M here: the
>   	  module will be called rtc-pl031.
>   
> +config RTC_DRV_PL031_RUST
> +	tristate "ARM AMBA PL031 RTC (Rust)"
> +	depends on RUST && RTC_CLASS
> +	help
> +	  This is the Rust implementation of the PL031 RTC driver.
> +	  It provides the same functionality as the C driver but is
> +	  written in Rust for improved memory safety. The driver supports
> +	  ARM, ST v1, and ST v2 variants of the PL031 RTC controller.
> +
>   config RTC_DRV_AT91RM9200
>   	tristate "AT91RM9200 or some AT91SAM9 RTC"
>   	depends on ARCH_AT91 || COMPILE_TEST
> diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
> index 6cf7e066314e1..10f540e7409b4 100644
> --- a/drivers/rtc/Makefile
> +++ b/drivers/rtc/Makefile
> @@ -139,6 +139,7 @@ obj-$(CONFIG_RTC_DRV_PCF8583)	+= rtc-pcf8583.o
>   obj-$(CONFIG_RTC_DRV_PIC32)	+= rtc-pic32.o
>   obj-$(CONFIG_RTC_DRV_PL030)	+= rtc-pl030.o
>   obj-$(CONFIG_RTC_DRV_PL031)	+= rtc-pl031.o
> +obj-$(CONFIG_RTC_DRV_PL031_RUST)	+= rtc_pl031_rust.o
>   obj-$(CONFIG_RTC_DRV_PM8XXX)	+= rtc-pm8xxx.o
>   obj-$(CONFIG_RTC_DRV_POLARFIRE_SOC)	+= rtc-mpfs.o
>   obj-$(CONFIG_RTC_DRV_PS3)	+= rtc-ps3.o
> diff --git a/drivers/rtc/rtc_pl031_rust.rs b/drivers/rtc/rtc_pl031_rust.rs
> new file mode 100644
> index 0000000000000..8ae48d96d1f94
> --- /dev/null
> +++ b/drivers/rtc/rtc_pl031_rust.rs
> @@ -0,0 +1,513 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +//! Rust ARM AMBA PrimeCell 031 RTC driver
> +//!
> +//! This driver provides Real Time Clock functionality for ARM AMBA PrimeCell
> +//! 031 RTC controllers and their ST Microelectronics derivatives.
> +
> +use core::marker::PhantomPinned;
> +use kernel::{
> +    amba,
> +    bindings,
> +    c_str,
> +    device::{
> +        self,
> +        Core, //
> +    },
> +    devres::Devres,
> +    io::mem::IoMem,
> +    irq::{
> +        self,
> +        Handler,
> +        IrqReturn, //
> +    },
> +    prelude::*,
> +    rtc::{
> +        RtcDevice,
> +        RtcOps,
> +        RtcTime,
> +        RtcWkAlrm, //
> +    },
> +    sync::aref::ARef, //
> +};
> +
> +// Register offsets
> +const RTC_DR: usize = 0x00; // Data read register
> +const RTC_MR: usize = 0x04; // Match register
> +const RTC_LR: usize = 0x08; // Data load register
> +const RTC_CR: usize = 0x0c; // Control register
> +const RTC_IMSC: usize = 0x10; // Interrupt mask and set register
> +const RTC_RIS: usize = 0x14; // Raw interrupt status register
> +const RTC_MIS: usize = 0x18; // Masked interrupt status register
> +const RTC_ICR: usize = 0x1c; // Interrupt clear register
> +
> +// ST variants have additional timer functionality
> +#[allow(dead_code)]
> +const RTC_TDR: usize = 0x20; // Timer data read register
> +#[allow(dead_code)]
> +const RTC_TLR: usize = 0x24; // Timer data load register
> +#[allow(dead_code)]
> +const RTC_TCR: usize = 0x28; // Timer control register
> +const RTC_YDR: usize = 0x30; // Year data read register
> +const RTC_YMR: usize = 0x34; // Year match register
> +const RTC_YLR: usize = 0x38; // Year data load register
> +const PL031_REG_SIZE: usize = RTC_YLR + 4;
> +
> +// Control register bits
> +const RTC_CR_EN: u32 = 1 << 0; // Counter enable bit
> +const RTC_CR_CWEN: u32 = 1 << 26; // Clockwatch enable bit
> +
> +#[allow(dead_code)]
> +const RTC_TCR_EN: u32 = 1 << 1; // Periodic timer enable bit
> +
> +// Interrupt status and control register bits
> +const RTC_BIT_AI: u32 = 1 << 0; // Alarm interrupt bit
> +#[allow(dead_code)]
> +const RTC_BIT_PI: u32 = 1 << 1; // Periodic interrupt bit (ST variants only)
> +
> +// RTC event flags
> +#[allow(dead_code)]
> +const RTC_AF: u32 = bindings::RTC_AF;
> +#[allow(dead_code)]
> +const RTC_IRQF: u32 = bindings::RTC_IRQF;
> +
> +// ST v2 time format bit definitions
> +const RTC_SEC_SHIFT: u32 = 0;
> +const RTC_SEC_MASK: u32 = 0x3F << RTC_SEC_SHIFT; // Second [0-59]
> +const RTC_MIN_SHIFT: u32 = 6;
> +const RTC_MIN_MASK: u32 = 0x3F << RTC_MIN_SHIFT; // Minute [0-59]
> +const RTC_HOUR_SHIFT: u32 = 12;
> +const RTC_HOUR_MASK: u32 = 0x1F << RTC_HOUR_SHIFT; // Hour [0-23]
> +const RTC_WDAY_SHIFT: u32 = 17;
> +const RTC_WDAY_MASK: u32 = 0x7 << RTC_WDAY_SHIFT; // Day of Week [1-7] 1=Sunday
> +const RTC_MDAY_SHIFT: u32 = 20;
> +const RTC_MDAY_MASK: u32 = 0x1F << RTC_MDAY_SHIFT; // Day of Month [1-31]
> +const RTC_MON_SHIFT: u32 = 25;
> +const RTC_MON_MASK: u32 = 0xF << RTC_MON_SHIFT; // Month [1-12] 1=January
> +
> +/// Converts a binary value to BCD.
> +fn bin2bcd(val: u8) -> u8 {
> +    ((val / 10) << 4) | (val % 10)
> +}
> +
> +/// Converts a BCD value to binary.
> +fn bcd2bin(val: u8) -> u8 {
> +    ((val >> 4) * 10) + (val & 0x0F)
> +}
> +
> +/// Converts a Gregorian date to ST v2 RTC packed BCD format.
> +///
> +/// Returns a tuple of (packed_time, bcd_year) where packed_time contains
> +/// month, day, weekday, hour, minute, and second in a single 32-bit value.
> +fn stv2_tm_to_time(tm: &RtcTime) -> Result<(u32, u32)> {
> +    let year = tm.tm_year() + 1900;
> +    let mut wday = tm.tm_wday();
> +
> +    // Hardware wday masking doesn't work, so wday must be valid.
> +    if !(-1..=6).contains(&wday) {
> +        return Err(EINVAL);
> +    } else if wday == -1 {
> +        // wday is not provided, calculate it here.
> +        let time64 = tm.to_time64();
> +        let mut calc_tm = RtcTime::default();
> +        calc_tm.set_from_time64(time64);
> +        wday = calc_tm.tm_wday();
> +    }
> +
> +    // Convert year to BCD.
> +    let bcd_year =
> +        (u32::from(bin2bcd((year % 100) as u8))) | (u32::from(bin2bcd((year / 100) as u8)) << 8);
> +
> +    let st_time = ((tm.tm_mon() + 1) as u32) << RTC_MON_SHIFT
> +        | (tm.tm_mday() as u32) << RTC_MDAY_SHIFT
> +        | ((wday + 1) as u32) << RTC_WDAY_SHIFT
> +        | (tm.tm_hour() as u32) << RTC_HOUR_SHIFT
> +        | (tm.tm_min() as u32) << RTC_MIN_SHIFT
> +        | (tm.tm_sec() as u32) << RTC_SEC_SHIFT;
> +
> +    Ok((st_time, bcd_year))
> +}
> +
> +/// Converts ST v2 RTC packed BCD format to a Gregorian date.
> +///
> +/// Extracts time components from the packed 32-bit value and BCD year register,
> +/// then returns an RtcTime structure.
> +fn stv2_time_to_tm(st_time: u32, bcd_year: u32) -> RtcTime {
> +    let year_low = bcd2bin((bcd_year & 0xFF) as u8);
> +    let year_high = bcd2bin(((bcd_year >> 8) & 0xFF) as u8);
> +    let mut tm = RtcTime::default();
> +    tm.set_tm_year(i32::from(year_low) + i32::from(year_high) * 100);
> +    tm.set_tm_mon((((st_time & RTC_MON_MASK) >> RTC_MON_SHIFT) - 1) as i32);
> +    tm.set_tm_mday(((st_time & RTC_MDAY_MASK) >> RTC_MDAY_SHIFT) as i32);
> +    tm.set_tm_wday((((st_time & RTC_WDAY_MASK) >> RTC_WDAY_SHIFT) - 1) as i32);
> +    tm.set_tm_hour(((st_time & RTC_HOUR_MASK) >> RTC_HOUR_SHIFT) as i32);
> +    tm.set_tm_min(((st_time & RTC_MIN_MASK) >> RTC_MIN_SHIFT) as i32);
> +    tm.set_tm_sec(((st_time & RTC_SEC_MASK) >> RTC_SEC_SHIFT) as i32);
> +
> +    // Values are from valid RTC time structures and are non-negative.
> +    tm.set_tm_yday(tm.year_days());
> +    tm.set_tm_year(tm.tm_year() - 1900);
> +    tm
> +}
> +
> +/// Vendor-specific variant identifier for PL031 RTC controllers.
> +#[derive(Copy, Clone, PartialEq)]
> +enum VendorVariant {
> +    /// Original ARM version with 32-bit Unix timestamp format.
> +    Arm,
> +    /// First ST derivative with clockwatch mode and weekday support.
> +    StV1,
> +    /// Second ST derivative with packed BCD time format and year register.
> +    StV2,
> +}
> +
> +impl VendorVariant {
> +    fn clockwatch(&self) -> bool {
> +        matches!(self, VendorVariant::StV1 | VendorVariant::StV2)
> +    }
> +
> +    #[allow(dead_code)]
> +    fn st_weekday(&self) -> bool {
> +        matches!(self, VendorVariant::StV1 | VendorVariant::StV2)
> +    }
> +
> +    #[allow(dead_code)]
> +    fn range_min(&self) -> i64 {
> +        match self {
> +            VendorVariant::Arm | VendorVariant::StV1 => 0,
> +            VendorVariant::StV2 => bindings::RTC_TIMESTAMP_BEGIN_0000,
> +        }
> +    }
> +
> +    #[allow(dead_code)]
> +    fn range_max(&self) -> u64 {
> +        match self {
> +            VendorVariant::Arm | VendorVariant::StV1 => u64::from(u32::MAX),
> +            VendorVariant::StV2 => bindings::RTC_TIMESTAMP_END_9999,
> +        }
> +    }
> +}
> +
> +/// The driver's private data struct. It holds all necessary devres managed
> +/// resources.
> +#[pin_data]
> +struct Pl031DrvData {
> +    #[pin]
> +    regs: Devres<IoMem<PL031_REG_SIZE>>,
> +    hw_variant: VendorVariant,
> +}
> +
> +// SAFETY: `Pl031DrvData` contains only `Send`/`Sync` types: `Devres`
> +// (Send+Sync) and `VendorVariant` (Copy).
> +unsafe impl Send for Pl031DrvData {}
> +// SAFETY: `Pl031DrvData` contains only `Send`/`Sync` types: `Devres`
> +// (Send+Sync) and `VendorVariant` (Copy).
> +unsafe impl Sync for Pl031DrvData {}
> +
> +/// Vendor-specific variant identifier used in AMBA device table.
> +#[derive(Copy, Clone)]
> +struct Pl031Variant {
> +    variant: VendorVariant,
> +}
> +
> +impl Pl031Variant {
> +    const ARM: Self = Self {
> +        variant: VendorVariant::Arm,
> +    };
> +    const STV1: Self = Self {
> +        variant: VendorVariant::StV1,
> +    };
> +    const STV2: Self = Self {
> +        variant: VendorVariant::StV2,
> +    };
> +}
> +
> +// Use AMBA device table for matching
> +kernel::amba_device_table!(
> +    ID_TABLE,
> +    MODULE_ID_TABLE,
> +    <Pl031AmbaDriver as amba::Driver>::IdInfo,
> +    [
> +        (
> +            amba::DeviceId::new(0x00041031, 0x000fffff),
> +            Pl031Variant::ARM
> +        ),
> +        (
> +            amba::DeviceId::new(0x00180031, 0x00ffffff),
> +            Pl031Variant::STV1
> +        ),
> +        (
> +            amba::DeviceId::new(0x00280031, 0x00ffffff),
> +            Pl031Variant::STV2
> +        ),
> +    ]
> +);
> +
> +#[pin_data]
> +struct Pl031AmbaDriver {
> +    #[pin]
> +    irqreg: irq::Registration<Pl031IrqHandler>,
> +}
> +
> +impl amba::Driver for Pl031AmbaDriver {
> +    type IdInfo = Pl031Variant;
> +    const AMBA_ID_TABLE: amba::IdTable<Self::IdInfo> = &ID_TABLE;
> +
> +    fn probe(
> +        adev: &amba::Device<Core>,
> +        id_info: Option<&Self::IdInfo>,
> +    ) -> impl PinInit<Self, Error> {
> +        pin_init::pin_init_scope(move || {
> +            let dev = adev.as_ref();
> +            let io_request = adev.io_request().ok_or(ENODEV)?;
> +            let variant = id_info
> +                .map(|info| info.variant)
> +                .unwrap_or(VendorVariant::Arm);
> +
> +            let rtcdev = RtcDevice::<Pl031DrvData>::new(
> +                dev,
> +                try_pin_init!(Pl031DrvData {
> +                    regs <- IoMem::new(io_request),
> +                    hw_variant: variant,
> +                }),
> +            )?;
> +
> +            dev.devm_init_wakeup()?;
> +
> +            let drvdata = rtcdev.drvdata()?;
> +            let regs = drvdata.regs.access(dev)?;
> +
> +            // Enable the clockwatch on ST Variants
> +            let mut cr = regs.read32(RTC_CR);
> +            if variant.clockwatch() {
> +                cr |= RTC_CR_CWEN;
> +            } else {
> +                cr |= RTC_CR_EN;
> +            }
> +            regs.write32(cr, RTC_CR);
> +
> +            // On ST PL031 variants, the RTC reset value does not provide
> +            // correct weekday for 2000-01-01. Correct the erroneous sunday
> +            // to saturday.
> +            if variant.st_weekday() {
> +                let bcd_year = regs.read32(RTC_YDR);
> +                if bcd_year == 0x2000 {
> +                    let st_time = regs.read32(RTC_DR);
> +                    if (st_time & (RTC_MON_MASK | RTC_MDAY_MASK | RTC_WDAY_MASK)) == 0x02120000 {
> +                        regs.write32(0x2000, RTC_YLR);
> +                        regs.write32(st_time | (0x7 << RTC_WDAY_SHIFT), RTC_LR);
> +                    }
> +                }
> +            }
> +
> +            rtcdev.set_range_min(variant.range_min());
> +            rtcdev.set_range_max(variant.range_max());
> +
> +            // This variant shares the IRQ with another block and must not
> +            // suspend that IRQ line.
> +            let irq_flags = if variant == VendorVariant::StV2 {
> +                kernel::irq::Flags::SHARED | kernel::irq::Flags::COND_SUSPEND
> +            } else {
> +                kernel::irq::Flags::SHARED
> +            };
> +
> +            if adev
> +                .irq_by_index(0)
> +                .and_then(|irq| irq.devm_set_wake_irq())
> +                .is_err()
> +            {
> +                rtcdev.clear_feature(bindings::RTC_FEATURE_ALARM);
> +            }
> +
> +            rtcdev.register()?;
> +
> +            Ok(try_pin_init!(Pl031AmbaDriver {
> +                irqreg <- adev.request_irq_by_index(
> +                    irq_flags,
> +                    0,
> +                    c_str!("rtc-pl031"),
> +                    try_pin_init!(Pl031IrqHandler {
> +                        _pin: PhantomPinned,
> +                        rtcdev: rtcdev.clone(),
> +                    }),
> +                ),
> +            }))
> +        })
> +    }
> +}
> +
> +/// Interrupt handler for PL031 RTC alarm events.
> +#[pin_data]
> +struct Pl031IrqHandler {
> +    #[pin]
> +    _pin: PhantomPinned,
> +    rtcdev: ARef<RtcDevice<Pl031DrvData>>,
> +}
> +
> +impl Handler for Pl031IrqHandler {
> +    fn handle(&self, dev: &device::Device<device::Bound>) -> IrqReturn {
> +        // Get driver data using drvdata.
> +        let drvdata = match self.rtcdev.drvdata() {
> +            Ok(drvdata) => drvdata,
> +            Err(_) => return IrqReturn::None,
> +        };
> +
> +        // Access the MMIO registers.
> +        let regs = match drvdata.regs.access(dev) {
> +            Ok(regs) => regs,
> +            Err(_) => return IrqReturn::None,
> +        };
> +
> +        // Read masked interrupt status.
> +        let rtcmis = regs.read32(RTC_MIS);
> +
> +        if (rtcmis & RTC_BIT_AI) != 0 {
> +            regs.write32(RTC_BIT_AI, RTC_ICR);
> +            self.rtcdev.update_irq(1, (RTC_AF | RTC_IRQF) as usize);
> +            return IrqReturn::Handled;
> +        }
> +
> +        IrqReturn::None
> +    }
> +}
> +
> +#[vtable]
> +impl RtcOps for Pl031DrvData {
> +    fn read_time(
> +        rtcdev: &RtcDevice<Self>,
> +        tm: &mut RtcTime,
> +        parent_dev: &device::Device<device::Bound>,
> +    ) -> Result {
> +        let drvdata = rtcdev.drvdata()?;
> +        let regs = drvdata.regs.access(parent_dev)?;
> +
> +        match drvdata.hw_variant {
> +            VendorVariant::Arm | VendorVariant::StV1 => {
> +                let time32: u32 = regs.read32(RTC_DR);
> +                let time64 = i64::from(time32);
> +                tm.set_from_time64(time64);
> +            }
> +            VendorVariant::StV2 => {
> +                let st_time = regs.read32(RTC_DR);
> +                let bcd_year = regs.read32(RTC_YDR);
> +                *tm = stv2_time_to_tm(st_time, bcd_year);
> +            }
> +        }
> +
> +        Ok(())
> +    }
> +
> +    fn set_time(
> +        rtcdev: &RtcDevice<Self>,
> +        tm: &RtcTime,
> +        parent_dev: &device::Device<device::Bound>,
> +    ) -> Result {
> +        let dev: &device::Device = rtcdev.as_ref();
> +        let drvdata = rtcdev.drvdata()?;
> +        let regs = drvdata.regs.access(parent_dev)?;
> +
> +        match drvdata.hw_variant {
> +            VendorVariant::Arm | VendorVariant::StV1 => {
> +                let time64 = tm.to_time64();
> +                regs.write32(time64 as u32, RTC_LR);
> +            }
> +            VendorVariant::StV2 => {
> +                let (st_time, bcd_year) = stv2_tm_to_time(tm).inspect_err(|&err| {
> +                    if err == EINVAL {
> +                        dev_err!(dev, "invalid wday value {}\n", tm.tm_wday());
> +                    }
> +                })?;
> +                regs.write32(bcd_year, RTC_YLR);
> +                regs.write32(st_time, RTC_LR);
> +            }
> +        }
> +
> +        Ok(())
> +    }
> +
> +    fn read_alarm(
> +        rtcdev: &RtcDevice<Self>,
> +        alarm: &mut RtcWkAlrm,
> +        parent_dev: &device::Device<device::Bound>,
> +    ) -> Result {
> +        let drvdata = rtcdev.drvdata()?;
> +        let regs = drvdata.regs.access(parent_dev)?;
> +
> +        match drvdata.hw_variant {
> +            VendorVariant::Arm | VendorVariant::StV1 => {
> +                let time32: u32 = regs.read32(RTC_MR);
> +                let time64 = i64::from(time32);
> +                RtcTime::time64_to_tm(time64, alarm.get_time_mut());
> +            }
> +            VendorVariant::StV2 => {
> +                let st_time = regs.read32(RTC_MR);
> +                let bcd_year = regs.read32(RTC_YMR);
> +                *alarm.get_time_mut() = stv2_time_to_tm(st_time, bcd_year);
> +            }
> +        }
> +
> +        alarm.set_pending((regs.read32(RTC_RIS) & RTC_BIT_AI) != 0);
> +        alarm.set_enabled((regs.read32(RTC_IMSC) & RTC_BIT_AI) != 0);
> +
> +        Ok(())
> +    }
> +
> +    fn set_alarm(
> +        rtcdev: &RtcDevice<Self>,
> +        alarm: &RtcWkAlrm,
> +        parent_dev: &device::Device<device::Bound>,
> +    ) -> Result {
> +        let dev: &device::Device = rtcdev.as_ref();
> +        let drvdata = rtcdev.drvdata()?;
> +        let regs = drvdata.regs.access(parent_dev)?;
> +
> +        match drvdata.hw_variant {
> +            VendorVariant::Arm | VendorVariant::StV1 => {
> +                let time64 = alarm.get_time().to_time64();
> +                regs.write32(time64 as u32, RTC_MR);
> +            }
> +            VendorVariant::StV2 => {
> +                let (st_time, bcd_year) =
> +                    stv2_tm_to_time(alarm.get_time()).inspect_err(|&err| {
> +                        if err == EINVAL {
> +                            dev_err!(dev, "invalid wday value {}\n", alarm.get_time().tm_wday());
> +                        }
> +                    })?;
> +                regs.write32(bcd_year, RTC_YMR);
> +                regs.write32(st_time, RTC_MR);
> +            }
> +        }
> +
> +        Self::alarm_irq_enable(rtcdev, alarm.enabled(), parent_dev)
> +    }
> +
> +    fn alarm_irq_enable(
> +        rtcdev: &RtcDevice<Self>,
> +        enabled: bool,
> +        parent_dev: &device::Device<device::Bound>,
> +    ) -> Result {
> +        let drvdata = rtcdev.drvdata()?;
> +        let regs = drvdata.regs.access(parent_dev)?;
> +
> +        // Clear any pending alarm interrupts.
> +        regs.write32(RTC_BIT_AI, RTC_ICR);
> +
> +        let mut imsc = regs.read32(RTC_IMSC);
> +        if enabled {
> +            imsc |= RTC_BIT_AI;
> +        } else {
> +            imsc &= !RTC_BIT_AI;
> +        }
> +        regs.write32(imsc, RTC_IMSC);
> +
> +        Ok(())
> +    }
> +}
> +
> +kernel::module_amba_driver! {
> +    type: Pl031AmbaDriver,
> +    name: "rtc-pl031-rust",
> +    authors: ["Ke Sun <sunke@kylinos.cn>"],
> +    description: "Rust PL031 RTC driver",
> +    license: "GPL v2",
> +    imports_ns: ["RTC"],
> +}