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

On Fri, Jul 31, 2026 at 9:07 AM raoxu <[email protected]> wrote:
>
> From: Xu Rao <[email protected]>
>
> 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: [email protected]
> Signed-off-by: Xu Rao <[email protected]>
> ---
>  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;
> --