[PATCH v1 0/4] ACPI: TAD: Fixes and cleanups on top of recent changes

[PATCH v1 0/4] ACPI: TAD: Fixes and cleanups on top of recent changes

[Rafael J. Wysocki]

Hi All,

This series fixes a couple of issues and makes a couple of cleanup
changes in the ACPI Time and Alarm Device (TAD) driver on top of
some updates of that driver that have been merged recently.

Patch [1/4] updates the driver to use the __ATTRIBUTE_GROUPS() macro
to simplify the code.

Patch [2/4] fixes the driver removal ordering that needs to take
the RTC class device interface into account now.

Patch [3/4] makes the RTC class device interface use rtc_tm_to_time64()
and rtc_time64_to_tm() in the alarm-related code that has a second
resolution anyway, which prevents ktime_t overflow from occurring when
attempting to convert large RTC time values to it.

Patch [4/4] updates a comment with incorrect English grammar.

All of the patches in this series are regarded as 7.1 material.

Thanks!

[PATCH v1 1/4] ACPI: TAD: Use __ATTRIBUTE_GROUPS() macro

[Rafael J. Wysocki]

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

Recent commit 93afe8ba9b01 ("ACPI: TAD: Use dev_groups in struct
device_driver") switched over the ACPI TAD driver to using device
attruibute groups instead of creating and removing the device sysfs
attributes directly, but it might go one step farther and use the
__ATTRIBUTE_GROUPS() macro which would reduce the code size slightly.

Do it now.

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

--- a/drivers/acpi/acpi_tad.c
+++ b/drivers/acpi/acpi_tad.c
@@ -605,15 +605,12 @@ static umode_t acpi_tad_attr_is_visible(
 	return 0;
 }
 
-static const struct attribute_group acpi_tad_attr_group = {
+static const struct attribute_group acpi_tad_group = {
 	.attrs	= acpi_tad_attrs,
 	.is_visible = acpi_tad_attr_is_visible,
 };
 
-static const struct attribute_group *acpi_tad_attr_groups[] = {
-	&acpi_tad_attr_group,
-	NULL,
-};
+__ATTRIBUTE_GROUPS(acpi_tad);
 
 #ifdef CONFIG_RTC_CLASS
 /* RTC class device interface */
@@ -885,7 +882,7 @@ static struct platform_driver acpi_tad_d
 	.driver = {
 		.name = "acpi-tad",
 		.acpi_match_table = acpi_tad_ids,
-		.dev_groups = acpi_tad_attr_groups,
+		.dev_groups = acpi_tad_groups,
 	},
 	.probe = acpi_tad_probe,
 	.remove = acpi_tad_remove,

[PATCH v1 2/4] ACPI: TAD: Use devres for all driver cleanup

[Rafael J. Wysocki]

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

The code in acpi_tad_remove() needs to run after the unregistration of
the devres-managed RTC class device so that it doesn't race with the
class callbacks of the latter.

To make that happen, pass it to devm_add_action_or_reset() before
registering the RTC class device.

Fixes: 7572dcabe38d ("ACPI: TAD: Add alarm support to the RTC class device interface")
Fixes: 8a1e7f4b1764 ("ACPI: TAD: Add RTC class device interface")
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/acpi_tad.c |   14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

--- a/drivers/acpi/acpi_tad.c
+++ b/drivers/acpi/acpi_tad.c
@@ -792,9 +792,9 @@ static int acpi_tad_disable_timer(struct
 	return acpi_tad_wake_set(dev, "_STV", timer_id, ACPI_TAD_WAKE_DISABLED);
 }
 
-static void acpi_tad_remove(struct platform_device *pdev)
+static void acpi_tad_remove(void *data)
 {
-	struct device *dev = &pdev->dev;
+	struct device *dev = data;
 	struct acpi_tad_driver_data *dd = dev_get_drvdata(dev);
 
 	device_init_wakeup(dev, false);
@@ -821,6 +821,7 @@ static int acpi_tad_probe(struct platfor
 	struct acpi_tad_driver_data *dd;
 	acpi_status status;
 	unsigned long long caps;
+	int ret;
 
 	/*
 	 * Initialization failure messages are mostly about firmware issues, so
@@ -867,6 +868,14 @@ static int acpi_tad_probe(struct platfor
 	pm_runtime_enable(dev);
 	pm_runtime_suspend(dev);
 
+	/*
+	 * acpi_tad_remove() needs to run after unregistering the RTC class
+	 * device to avoid racing with the latter's callbacks.
+	 */
+	ret = devm_add_action_or_reset(&pdev->dev, acpi_tad_remove, &pdev->dev);
+	if (ret)
+		return ret;
+
 	if (caps & ACPI_TAD_RT)
 		acpi_tad_register_rtc(dev, caps);
 
@@ -885,7 +894,6 @@ static struct platform_driver acpi_tad_d
 		.dev_groups = acpi_tad_groups,
 	},
 	.probe = acpi_tad_probe,
-	.remove = acpi_tad_remove,
 };
 MODULE_DEVICE_TABLE(acpi, acpi_tad_ids);
 

[PATCH v1 3/4] ACPI: TAD: RTC: Refine timer value computations and checks

[Rafael J. Wysocki]

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

Since rtc_tm_to_ktime() may overflow for large RTC time values and
full second granularity is sufficient in timer value computations
in acpi_tad_rtc_set_alarm() and acpi_tad_rtc_read_alarm(), use
rtc_tm_to_time64() instead of that function, which also allows the
computations to be simplified.

Moreover, U32_MAX is a special "timer disabled" value, so make
acpi_tad_rtc_set_alarm() reject it when attempting to program the
alarm timers.

Fixes: 7572dcabe38d ("ACPI: TAD: Add alarm support to the RTC class device interface")
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/acpi_tad.c |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

--- a/drivers/acpi/acpi_tad.c
+++ b/drivers/acpi/acpi_tad.c
@@ -680,9 +680,8 @@ static int acpi_tad_rtc_set_alarm(struct
 
 		acpi_tad_rt_to_tm(&rt, &tm_now);
 
-		value = ktime_divns(ktime_sub(rtc_tm_to_ktime(t->time),
-					      rtc_tm_to_ktime(tm_now)), NSEC_PER_SEC);
-		if (value <= 0 || value > U32_MAX)
+		value = rtc_tm_to_time64(&t->time) - rtc_tm_to_time64(&tm_now);
+		if (value <= 0 || value >= U32_MAX)
 			return -EINVAL;
 	}
 
@@ -745,8 +744,7 @@ static int acpi_tad_rtc_read_alarm(struc
 
 	if (retval != ACPI_TAD_WAKE_DISABLED) {
 		t->enabled = 1;
-		t->time = rtc_ktime_to_tm(ktime_add_ns(rtc_tm_to_ktime(tm_now),
-						       (u64)retval * NSEC_PER_SEC));
+		rtc_time64_to_tm(rtc_tm_to_time64(&tm_now) + retval, &t->time);
 	} else {
 		t->enabled = 0;
 		t->time = tm_now;

[PATCH v1 4/4] ACPI: TAD: Fix up a comment in acpi_tad_probe()

[Rafael J. Wysocki]

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

Fix grammar in the comment preceding the pm_runtime_set_active() call in
acpi_tad_probe().

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

--- a/drivers/acpi/acpi_tad.c
+++ b/drivers/acpi/acpi_tad.c
@@ -859,8 +859,8 @@ static int acpi_tad_probe(struct platfor
 	}
 
 	/*
-	 * The platform bus type layer tells the ACPI PM domain powers up the
-	 * device, so set the runtime PM status of it to "active".
+	 * The platform bus type probe callback tells the ACPI PM domain to
+	 * power up the device, so set the runtime PM status of it to "active".
 	 */
 	pm_runtime_set_active(dev);
 	pm_runtime_enable(dev);