[PATCH v0 0/6] ACPI: TAD: Assorted improvements and RTC class device interface

[PATCH v0 0/6] ACPI: TAD: Assorted improvements and RTC class device interface

[Rafael J. Wysocki]

Hi All,

This is an untested prototype (hence v0), so please take it with a grain of
salt.  It is posted mostly to illustrate a point made in the discussion
regarding the RTC class interface at:

https://lore.kernel.org/linux-rtc/DGKTYBF8G6KI.3Q96DGYKYHAFO@kernel.org/T/#r49e1d3b1f47101a1240158f4e18d02e3e0c8b75b

The first 4 patches are assorted improvements of the ACPI TAD driver,
patch [5/6] adds an RTC class device interface to it and patch [6/6]
simply updates the driver description (no functional changes).

Thanks!

[PATCH v0 1/6] ACPI: TAD: Create one attribute group

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Instead of creating three attribute groups, one for each supported
subset of capabilities, create just one and use an .is_visible()
callback in it to decide which attributes will be visible.

Also stop failing proble if the AC wakeup capability is not
supported.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/acpi_tad.c |   85 +++++++++++++++++++-----------------------------
 1 file changed, 35 insertions(+), 50 deletions(-)

--- a/drivers/acpi/acpi_tad.c
+++ b/drivers/acpi/acpi_tad.c
@@ -249,14 +249,6 @@ static ssize_t time_show(struct device *
 
 static DEVICE_ATTR_RW(time);
 
-static struct attribute *acpi_tad_time_attrs[] = {
-	&dev_attr_time.attr,
-	NULL,
-};
-static const struct attribute_group acpi_tad_time_attr_group = {
-	.attrs	= acpi_tad_time_attrs,
-};
-
 static int acpi_tad_wake_set(struct device *dev, char *method, u32 timer_id,
 			     u32 value)
 {
@@ -486,17 +478,6 @@ static ssize_t ac_status_show(struct dev
 
 static DEVICE_ATTR_RW(ac_status);
 
-static struct attribute *acpi_tad_attrs[] = {
-	&dev_attr_caps.attr,
-	&dev_attr_ac_alarm.attr,
-	&dev_attr_ac_policy.attr,
-	&dev_attr_ac_status.attr,
-	NULL,
-};
-static const struct attribute_group acpi_tad_attr_group = {
-	.attrs	= acpi_tad_attrs,
-};
-
 static ssize_t dc_alarm_store(struct device *dev, struct device_attribute *attr,
 			      const char *buf, size_t count)
 {
@@ -545,14 +526,45 @@ static ssize_t dc_status_show(struct dev
 
 static DEVICE_ATTR_RW(dc_status);
 
-static struct attribute *acpi_tad_dc_attrs[] = {
+static struct attribute *acpi_tad_attrs[] = {
+	&dev_attr_caps.attr,
+	&dev_attr_ac_alarm.attr,
+	&dev_attr_ac_policy.attr,
+	&dev_attr_ac_status.attr,
 	&dev_attr_dc_alarm.attr,
 	&dev_attr_dc_policy.attr,
 	&dev_attr_dc_status.attr,
+	&dev_attr_time.attr,
 	NULL,
 };
-static const struct attribute_group acpi_tad_dc_attr_group = {
-	.attrs	= acpi_tad_dc_attrs,
+
+static umode_t acpi_tad_attr_is_visible(struct kobject *kobj,
+					struct attribute *a, int n)
+{
+	struct acpi_tad_driver_data *dd = dev_get_drvdata(kobj_to_dev(kobj));
+
+	if (a == &dev_attr_caps.attr)
+		return a->mode;
+
+	if ((dd->capabilities & ACPI_TAD_AC_WAKE) &&
+	    (a == &dev_attr_ac_alarm.attr || a == &dev_attr_ac_policy.attr ||
+	     a == &dev_attr_ac_status.attr))
+		return a->mode;
+
+	if ((dd->capabilities & ACPI_TAD_DC_WAKE) &&
+	    (a == &dev_attr_dc_alarm.attr || a == &dev_attr_dc_policy.attr ||
+	     a == &dev_attr_dc_status.attr))
+		return a->mode;
+
+	if ((dd->capabilities & ACPI_TAD_RT) && a == &dev_attr_time.attr)
+		return a->mode;
+
+	return 0;
+}
+
+static const struct attribute_group acpi_tad_attr_group = {
+	.attrs	= acpi_tad_attrs,
+	.is_visible = acpi_tad_attr_is_visible,
 };
 
 static int acpi_tad_disable_timer(struct device *dev, u32 timer_id)
@@ -567,12 +579,6 @@ static void acpi_tad_remove(struct platf
 
 	device_init_wakeup(dev, false);
 
-	if (dd->capabilities & ACPI_TAD_RT)
-		sysfs_remove_group(&dev->kobj, &acpi_tad_time_attr_group);
-
-	if (dd->capabilities & ACPI_TAD_DC_WAKE)
-		sysfs_remove_group(&dev->kobj, &acpi_tad_dc_attr_group);
-
 	sysfs_remove_group(&dev->kobj, &acpi_tad_attr_group);
 
 	scoped_guard(pm_runtime_noresume, dev) {
@@ -607,11 +613,6 @@ static int acpi_tad_probe(struct platfor
 		return -ENODEV;
 	}
 
-	if (!(caps & ACPI_TAD_AC_WAKE)) {
-		dev_info(dev, "Unsupported capabilities\n");
-		return -ENODEV;
-	}
-
 	if (!acpi_has_method(handle, "_PRW")) {
 		dev_info(dev, "Missing _PRW\n");
 		return -ENODEV;
@@ -643,24 +644,8 @@ static int acpi_tad_probe(struct platfor
 
 	ret = sysfs_create_group(&dev->kobj, &acpi_tad_attr_group);
 	if (ret)
-		goto fail;
-
-	if (caps & ACPI_TAD_DC_WAKE) {
-		ret = sysfs_create_group(&dev->kobj, &acpi_tad_dc_attr_group);
-		if (ret)
-			goto fail;
-	}
-
-	if (caps & ACPI_TAD_RT) {
-		ret = sysfs_create_group(&dev->kobj, &acpi_tad_time_attr_group);
-		if (ret)
-			goto fail;
-	}
-
-	return 0;
+		acpi_tad_remove(pdev);
 
-fail:
-	acpi_tad_remove(pdev);
 	return ret;
 }
 

[PATCH v0 2/6] ACPI: TAD: Use __free() for cleanup in time_store()

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Use __free() for the automatic freeing of memory pointed to by local
variable str in time_store() which allows the code to become somewhat
easier to follow.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/acpi_tad.c |   28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

--- a/drivers/acpi/acpi_tad.c
+++ b/drivers/acpi/acpi_tad.c
@@ -167,57 +167,57 @@ static ssize_t time_store(struct device
 			  const char *buf, size_t count)
 {
 	struct acpi_tad_rt rt;
-	char *str, *s;
-	int val, ret = -ENODATA;
+	int val, ret;
+	char *s;
 
-	str = kmemdup_nul(buf, count, GFP_KERNEL);
+	char *str __free(kfree) = kmemdup_nul(buf, count, GFP_KERNEL);
 	if (!str)
 		return -ENOMEM;
 
 	s = acpi_tad_rt_next_field(str, &val);
 	if (!s)
-		goto out_free;
+		return -ENODATA;
 
 	rt.year = val;
 
 	s = acpi_tad_rt_next_field(s, &val);
 	if (!s)
-		goto out_free;
+		return -ENODATA;
 
 	rt.month = val;
 
 	s = acpi_tad_rt_next_field(s, &val);
 	if (!s)
-		goto out_free;
+		return -ENODATA;
 
 	rt.day = val;
 
 	s = acpi_tad_rt_next_field(s, &val);
 	if (!s)
-		goto out_free;
+		return -ENODATA;
 
 	rt.hour = val;
 
 	s = acpi_tad_rt_next_field(s, &val);
 	if (!s)
-		goto out_free;
+		return -ENODATA;
 
 	rt.minute = val;
 
 	s = acpi_tad_rt_next_field(s, &val);
 	if (!s)
-		goto out_free;
+		return -ENODATA;
 
 	rt.second = val;
 
 	s = acpi_tad_rt_next_field(s, &val);
 	if (!s)
-		goto out_free;
+		return -ENODATA;
 
 	rt.tz = val;
 
 	if (kstrtoint(s, 10, &val))
-		goto out_free;
+		return -ENODATA;
 
 	rt.daylight = val;
 
@@ -226,10 +226,10 @@ static ssize_t time_store(struct device
 	memset(rt.padding, 0, 3);
 
 	ret = acpi_tad_set_real_time(dev, &rt);
+	if (ret)
+		return ret;
 
-out_free:
-	kfree(str);
-	return ret ? ret : count;
+	return count;
 }
 
 static ssize_t time_show(struct device *dev, struct device_attribute *attr,

[PATCH v0 3/6] ACPI: TAD: Update RT data validation checking

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Move RT data validation checks from acpi_tad_set_real_time() to
a separate function called acpi_tad_rt_is_invalid() and use it
also in acpi_tad_get_real_time() to validate data coming from
the platform firmware.

Also make acpi_tad_set_real_time() return -EINVAL when the RT data
passed to it is invalid (instead of -ERANGE which is somewhat
confusing) and introduce ACPI_TAD_TZ_UNSPEC to represent the
"unspecified timezone" value.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/acpi_tad.c |   24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

--- a/drivers/acpi/acpi_tad.c
+++ b/drivers/acpi/acpi_tad.c
@@ -49,6 +49,9 @@ MODULE_AUTHOR("Rafael J. Wysocki");
 /* Special value for disabled timer or expired timer wake policy. */
 #define ACPI_TAD_WAKE_DISABLED	(~(u32)0)
 
+/* ACPI TAD RTC */
+#define ACPI_TAD_TZ_UNSPEC	2047
+
 struct acpi_tad_driver_data {
 	u32 capabilities;
 };
@@ -67,6 +70,16 @@ struct acpi_tad_rt {
 	u8 padding[3]; /* must be 0 */
 } __packed;
 
+static bool acpi_tad_rt_is_invalid(struct acpi_tad_rt *rt)
+{
+	return rt->year < 1900 || rt->year > 9999 ||
+	    rt->month < 1 || rt->month > 12 ||
+	    rt->hour > 23 || rt->minute > 59 || rt->second > 59 ||
+	    rt->tz < -1440 ||
+	    (rt->tz > 1440 && rt->tz != ACPI_TAD_TZ_UNSPEC) ||
+	    rt->daylight > 3;
+}
+
 static int acpi_tad_set_real_time(struct device *dev, struct acpi_tad_rt *rt)
 {
 	acpi_handle handle = ACPI_HANDLE(dev);
@@ -80,12 +93,8 @@ static int acpi_tad_set_real_time(struct
 	unsigned long long retval;
 	acpi_status status;
 
-	if (rt->year < 1900 || rt->year > 9999 ||
-	    rt->month < 1 || rt->month > 12 ||
-	    rt->hour > 23 || rt->minute > 59 || rt->second > 59 ||
-	    rt->tz < -1440 || (rt->tz > 1440 && rt->tz != 2047) ||
-	    rt->daylight > 3)
-		return -ERANGE;
+	if (acpi_tad_rt_is_invalid(rt))
+		return -EINVAL;
 
 	args[0].buffer.pointer = (u8 *)rt;
 	args[0].buffer.length = sizeof(*rt);
@@ -145,6 +154,9 @@ static int acpi_tad_get_real_time(struct
 	if (ret)
 		return ret;
 
+	if (acpi_tad_rt_is_invalid(rt))
+		return -ENODATA;
+
 	return 0;
 }
 

[PATCH v0 4/6] ACPI: TAD: Clear unused RT data in acpi_tad_set_real_time()

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Move the clearing of the fields in struct acpi_tad_rt that are not used
on the real time setting side to acpi_tad_set_real_time().

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/acpi_tad.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

--- a/drivers/acpi/acpi_tad.c
+++ b/drivers/acpi/acpi_tad.c
@@ -96,6 +96,10 @@ static int acpi_tad_set_real_time(struct
 	if (acpi_tad_rt_is_invalid(rt))
 		return -EINVAL;
 
+	rt->valid = 0;
+	rt->msec = 0;
+	memset(rt->padding, 0, 3);
+
 	args[0].buffer.pointer = (u8 *)rt;
 	args[0].buffer.length = sizeof(*rt);
 
@@ -233,10 +237,6 @@ static ssize_t time_store(struct device
 
 	rt.daylight = val;
 
-	rt.valid = 0;
-	rt.msec = 0;
-	memset(rt.padding, 0, 3);
-
 	ret = acpi_tad_set_real_time(dev, &rt);
 	if (ret)
 		return ret;

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

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Add an RTC class device interface allowing to read and set the real time
value and providing a procfs callback to the ACPI TAD driver.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/acpi_tad.c |   85 ++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 83 insertions(+), 2 deletions(-)

--- a/drivers/acpi/acpi_tad.c
+++ b/drivers/acpi/acpi_tad.c
@@ -25,6 +25,7 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
+#include <linux/rtc.h>
 #include <linux/suspend.h>
 
 MODULE_DESCRIPTION("ACPI Time and Alarm (TAD) Device Driver");
@@ -51,6 +52,7 @@ MODULE_AUTHOR("Rafael J. Wysocki");
 
 /* ACPI TAD RTC */
 #define ACPI_TAD_TZ_UNSPEC	2047
+#define ACPI_TAD_TIME_ISDST	3
 
 struct acpi_tad_driver_data {
 	u32 capabilities;
@@ -164,6 +166,8 @@ static int acpi_tad_get_real_time(struct
 	return 0;
 }
 
+/* sysfs interface */
+
 static char *acpi_tad_rt_next_field(char *s, int *val)
 {
 	char *p;
@@ -579,6 +583,77 @@ static const struct attribute_group acpi
 	.is_visible = acpi_tad_attr_is_visible,
 };
 
+/* RTC class device interface */
+
+static int acpi_tad_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+	struct acpi_tad_rt rt;
+
+	rt.year = tm->tm_year + 1900;
+	rt.month = tm->tm_mon + 1;
+	rt.day = tm->tm_mday;
+	rt.hour = tm->tm_hour;
+	rt.minute = tm->tm_min;
+	rt.second = tm->tm_sec;
+	rt.tz = ACPI_TAD_TZ_UNSPEC;
+	rt.daylight = ACPI_TAD_TIME_ISDST * !!tm->tm_isdst;
+
+	return acpi_tad_set_real_time(dev, &rt);
+}
+
+static int acpi_tad_rtc_read_time(struct device *dev, struct rtc_time *tm)
+{
+	struct acpi_tad_rt rt;
+	int ret;
+
+	ret = acpi_tad_get_real_time(dev, &rt);
+	if (ret)
+		return ret;
+
+	tm->tm_year = rt.year - 1900;
+	tm->tm_mon = rt.month - 1;
+	tm->tm_mday = rt.day;
+	tm->tm_hour = rt.hour;
+	tm->tm_min = rt.minute;
+	tm->tm_sec = rt.second;
+	tm->tm_isdst = rt.daylight == ACPI_TAD_TIME_ISDST;
+
+	return 0;
+}
+
+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,
+};
+
+/* 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);
+
+	return 0;
 }
 
 static const struct acpi_device_id acpi_tad_ids[] = {

[PATCH v0 6/6] ACPI: TAD: Update the driver description comment

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Update the preamble comment describing the driver to match the code
after previous changes along with the copyright information.

No functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/acpi_tad.c |   13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

--- a/drivers/acpi/acpi_tad.c
+++ b/drivers/acpi/acpi_tad.c
@@ -2,12 +2,10 @@
 /*
  * ACPI Time and Alarm (TAD) Device Driver
  *
- * Copyright (C) 2018 Intel Corporation
+ * Copyright (C) 2018 - 2026 Intel Corporation
  * Author: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
  *
- * This driver is based on Section 9.18 of the ACPI 6.2 specification revision.
- *
- * It only supports the system wakeup capabilities of the TAD.
+ * This driver is based on ACPI 6.6, Section 9.17.
  *
  * Provided are sysfs attributes, available under the TAD platform device,
  * allowing user space to manage the AC and DC wakeup timers of the TAD:
@@ -18,6 +16,11 @@
  *
  * The wakeup events handling and power management of the TAD is expected to
  * be taken care of by the ACPI PM domain attached to its platform device.
+ *
+ * If the TAD supports the get/set real time features, as indicated by the
+ * capability mask returned by _GCP under the TAD object, additional sysfs
+ * attributes are created allowing the real time to be set and read and an RTC
+ * class device is registered under the TAD platform device.
  */
 
 #include <linux/acpi.h>
@@ -32,7 +35,7 @@ MODULE_DESCRIPTION("ACPI Time and Alarm
 MODULE_LICENSE("GPL v2");
 MODULE_AUTHOR("Rafael J. Wysocki");
 
-/* ACPI TAD capability flags (ACPI 6.2, Section 9.18.2) */
+/* ACPI TAD capability flags (ACPI 6.6, Section 9.17.2) */
 #define ACPI_TAD_AC_WAKE	BIT(0)
 #define ACPI_TAD_DC_WAKE	BIT(1)
 #define ACPI_TAD_RT		BIT(2)

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

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

[Rafael J. Wysocki]

Hi,

On Fri, Feb 27, 2026 at 11:37 AM Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
>
> 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.

OK

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

I'll need to use them anyway later when I add read/set_alarm to this
device (because the alarm may not be supported in which case I'll need
to set a flag to indicate that before registering the RTC device,
IIUC).