[PATCH] ACPI: TAD: report expired alarm status through RTC

raoxu posted 1 patch 2 months ago
drivers/acpi/acpi_tad.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
[PATCH] ACPI: TAD: report expired alarm status through RTC
Posted by raoxu 2 months ago
From: Xu Rao <raoxu@uniontech.com>

acpi_tad_rtc_read_alarm() always sets rtc_wkalrm::pending to zero, so
RTC_WKALM_RD cannot report that a TAD alarm has expired.

ACPI 6.6, Section 9.17.5 defines _GWS as returning a DWORD bit field.
Bit 0 indicates that the selected timer has expired, bit 1 indicates
that it caused a platform wake, and bits 31:2 are reserved. Therefore,
only bit 0 maps to rtc_wkalrm::pending; converting the complete _GWS
value to bool would incorrectly treat the wake-source bit or a nonzero
reserved bit as an expired alarm.

Read _GWS for the AC timer, reject values wider than the specified
DWORD, and report its expired bit through rtc_wkalrm::pending. This
matches the existing read path, which reads the AC timer because alarms
programmed through the RTC interface set the AC and DC timers to the same
value.

Fixes: 7572dcabe38d ("ACPI: TAD: Add alarm support to the RTC class device interface")
Cc: stable@vger.kernel.org
Signed-off-by: Xu Rao <raoxu@uniontech.com>
---
 drivers/acpi/acpi_tad.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/acpi_tad.c b/drivers/acpi/acpi_tad.c
index fc43df083738..610840c6240d 100644
--- a/drivers/acpi/acpi_tad.c
+++ b/drivers/acpi/acpi_tad.c
@@ -51,6 +51,9 @@ MODULE_AUTHOR("Rafael J. Wysocki");
 #define ACPI_TAD_AC_TIMER	(u32)0
 #define ACPI_TAD_DC_TIMER	(u32)1

+/* ACPI TAD wake alarm status flags (ACPI 6.6, Section 9.17.5) */
+#define ACPI_TAD_WAKE_STATUS_EXPIRED	BIT(0)
+
 /* Special value for disabled timer or expired timer wake policy. */
 #define ACPI_TAD_WAKE_DISABLED	(~(u32)0)

@@ -709,6 +712,7 @@ static int acpi_tad_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *t)
 static int acpi_tad_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *t)
 {
 	unsigned long long retval;
+	unsigned long long status;
 	struct rtc_time tm_now;
 	struct acpi_tad_rt rt;
 	int ret;
@@ -740,7 +744,14 @@ static int acpi_tad_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *t)
 	if (retval > U32_MAX)
 		return -ENODATA;

-	t->pending = 0;
+	ret = __acpi_tad_wake_read(dev, "_GWS", ACPI_TAD_AC_TIMER, &status);
+	if (ret)
+		return ret;
+
+	if (status > U32_MAX)
+		return -ENODATA;
+
+	t->pending = !!(status & ACPI_TAD_WAKE_STATUS_EXPIRED);

 	if (retval != ACPI_TAD_WAKE_DISABLED) {
 		t->enabled = 1;
--
2.50.1
Re: [PATCH] ACPI: TAD: report expired alarm status through RTC
Posted by Rafael J. Wysocki (Intel) 1 month, 3 weeks ago
On Fri, Jul 31, 2026 at 9:07 AM raoxu <raoxu@uniontech.com> wrote:
>
> From: Xu Rao <raoxu@uniontech.com>
>
> acpi_tad_rtc_read_alarm() always sets rtc_wkalrm::pending to zero, so
> RTC_WKALM_RD cannot report that a TAD alarm has expired.

Well, "pending" doesn't mean "expired".

> ACPI 6.6, Section 9.17.5 defines _GWS as returning a DWORD bit field.
> Bit 0 indicates that the selected timer has expired, bit 1 indicates
> that it caused a platform wake, and bits 31:2 are reserved. Therefore,
> only bit 0 maps to rtc_wkalrm::pending; converting the complete _GWS
> value to bool would incorrectly treat the wake-source bit or a nonzero
> reserved bit as an expired alarm.
>
> Read _GWS for the AC timer, reject values wider than the specified
> DWORD, and report its expired bit through rtc_wkalrm::pending. This
> matches the existing read path, which reads the AC timer because alarms
> programmed through the RTC interface set the AC and DC timers to the same
> value.
>
> Fixes: 7572dcabe38d ("ACPI: TAD: Add alarm support to the RTC class device interface")
> Cc: stable@vger.kernel.org
> Signed-off-by: Xu Rao <raoxu@uniontech.com>
> ---
>  drivers/acpi/acpi_tad.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/acpi_tad.c b/drivers/acpi/acpi_tad.c
> index fc43df083738..610840c6240d 100644
> --- a/drivers/acpi/acpi_tad.c
> +++ b/drivers/acpi/acpi_tad.c
> @@ -51,6 +51,9 @@ MODULE_AUTHOR("Rafael J. Wysocki");
>  #define ACPI_TAD_AC_TIMER      (u32)0
>  #define ACPI_TAD_DC_TIMER      (u32)1
>
> +/* ACPI TAD wake alarm status flags (ACPI 6.6, Section 9.17.5) */
> +#define ACPI_TAD_WAKE_STATUS_EXPIRED   BIT(0)
> +
>  /* Special value for disabled timer or expired timer wake policy. */
>  #define ACPI_TAD_WAKE_DISABLED (~(u32)0)
>
> @@ -709,6 +712,7 @@ static int acpi_tad_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *t)
>  static int acpi_tad_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *t)
>  {
>         unsigned long long retval;
> +       unsigned long long status;
>         struct rtc_time tm_now;
>         struct acpi_tad_rt rt;
>         int ret;
> @@ -740,7 +744,14 @@ static int acpi_tad_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *t)
>         if (retval > U32_MAX)
>                 return -ENODATA;
>
> -       t->pending = 0;
> +       ret = __acpi_tad_wake_read(dev, "_GWS", ACPI_TAD_AC_TIMER, &status);
> +       if (ret)
> +               return ret;
> +
> +       if (status > U32_MAX)
> +               return -ENODATA;
> +
> +       t->pending = !!(status & ACPI_TAD_WAKE_STATUS_EXPIRED);

ACPI_TAD_WAKE_STATUS_EXPIRED doesn't actually mean a "pending interrupt".

>
>         if (retval != ACPI_TAD_WAKE_DISABLED) {
>                 t->enabled = 1;
> --
Re: [PATCH] ACPI: TAD: report expired alarm status through RTC
Posted by raoxu 1 month, 3 weeks ago
On Thu, Aug 6, 2026 at 1:36 AM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> Well, "pending" doesn't mean "expired".
>
> ACPI_TAD_WAKE_STATUS_EXPIRED doesn't actually mean a "pending interrupt".

You are right.

_GWS bit 0 reports the timer-expired status, including cases where the
timer expiration did not generate a pending alarm interrupt.
rtc_wkalrm::pending is not meant to report that state.

So mapping the _GWS expired bit to rtc_wkalrm::pending is incorrect. I
will drop this patch.

Thanks for the clarification.