Re: [PATCH v0 5/6] ACPI: TAD: Add RTC class device interface

[Alexandre Belloni]

Hello,

On 22/02/2026 15:18:29+0100, Rafael J. Wysocki wrote:
> +static int acpi_tad_rtc_procfs(struct device *dev, struct seq_file *seq)
> +{
> +	struct acpi_tad_rt rt;
> +	int ret;
> +
> +	ret = acpi_tad_get_real_time(dev, &rt);
> +	if (ret)
> +		return ret;
> +
> +	seq_printf(seq,
> +		   "Time\t\t: %u:%u:%u\n"
> +		   "Date\t\t: %u-%u-%u\n"
> +		   "Daylight\t: %s\n",
> +		   rt.hour, rt.minute, rt.second,
> +		   rt.year, rt.month, rt.day,
> +		   str_yes_no(rt.daylight == ACPI_TAD_TIME_ISDST));
> +
> +	if (rt.tz == ACPI_TAD_TZ_UNSPEC)
> +		seq_puts(seq, "Timezone\t: unspecified\n");
> +	else
> +		seq_printf(seq, "Timezone\t: %d\n", rt.tz);
> +
> +	return 0;
> +}
> +
> +static const struct rtc_class_ops acpi_tad_rtc_ops = {
> +	.read_time = acpi_tad_rtc_read_time,
> +	.set_time = acpi_tad_rtc_set_time,
> +	.proc = acpi_tad_rtc_procfs,

I would avoid implementing .proc, it has been deprecated for a while and
doesn't bring much.

> +};
> +
> +/* Platform driver interface */
> +
>  static int acpi_tad_disable_timer(struct device *dev, u32 timer_id)
>  {
>  	return acpi_tad_wake_set(dev, "_STV", timer_id, ACPI_TAD_WAKE_DISABLED);
> @@ -655,10 +730,16 @@ static int acpi_tad_probe(struct platfor
>  	pm_runtime_suspend(dev);
>  
>  	ret = sysfs_create_group(&dev->kobj, &acpi_tad_attr_group);
> -	if (ret)
> +	if (ret) {
>  		acpi_tad_remove(pdev);
> +		return ret;
> +	}
>  
> -	return ret;
> +	if (caps & ACPI_TAD_RT)
> +		devm_rtc_device_register(dev, "acpi-tad-rtc", &acpi_tad_rtc_ops,
> +					 THIS_MODULE);
> +

Please use devm_rtc_allocate_device() and devm_rtc_register_device() so
you can set range_min and range_max. I get you don't need the rtc_device
later so we could have a helper that takes the range and registers the
rtc.

> +	return 0;
>  }
>  
>  static const struct acpi_device_id acpi_tad_ids[] = {
> 
> 
> 

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