linux-kernel - Re: [PATCH] rtc: sc27xx: Always read normal alarm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 18 Nov 2020 14:31:14 +0800
From:   Chunyan Zhang <zhang.lyra@...il.com>
To:     Alexandre Belloni <alexandre.belloni@...tlin.com>
Cc:     Alessandro Zummo <a.zummo@...ertech.it>,
        Orson Zhai <orsonzhai@...il.com>,
        Baolin Wang <baolin.wang7@...il.com>,
        Bartosz Golaszewski <bgolaszewski@...libre.com>,
        linux-rtc@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] rtc: sc27xx: Always read normal alarm

Hi Alexandre,

Thanks for this cleanup.

On Wed, 18 Nov 2020 at 05:22, Alexandre Belloni
<alexandre.belloni@...tlin.com> wrote:
>
> The RTC core only reads the alarm from the hardware at boot time, to know
> whether an alarm was already set before booting. It keeps track of all the
> alarms after that so there is no need to ever read the auxiliary alarm.
>
> Commit 3822d1bb0df1 ("rtc: sc27xx: Always read normal alarm when
> registering RTC device") already effectively removed the capability to read
> the auxiliary alarm a .read_alarm is always called with rtc->registered set

one nit suggestion: add a comma before "a .read_alarm is ..."

> to false.
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@...tlin.com>

Reviewed-by: Chunyan Zhang <zhang.lyra@...il.com>

Cheers,
Chunyan

> ---
>  drivers/rtc/rtc-sc27xx.c | 38 ++------------------------------------
>  1 file changed, 2 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/rtc/rtc-sc27xx.c b/drivers/rtc/rtc-sc27xx.c
> index 6e65f68ea86d..a953bc0a5a5b 100644
> --- a/drivers/rtc/rtc-sc27xx.c
> +++ b/drivers/rtc/rtc-sc27xx.c
> @@ -299,33 +299,6 @@ static int sprd_rtc_set_secs(struct sprd_rtc *rtc, enum sprd_rtc_reg_types type,
>                             sts_mask);
>  }
>
> -static int sprd_rtc_read_aux_alarm(struct device *dev, struct rtc_wkalrm *alrm)
> -{
> -       struct sprd_rtc *rtc = dev_get_drvdata(dev);
> -       time64_t secs;
> -       u32 val;
> -       int ret;
> -
> -       ret = sprd_rtc_get_secs(rtc, SPRD_RTC_AUX_ALARM, &secs);
> -       if (ret)
> -               return ret;
> -
> -       rtc_time64_to_tm(secs, &alrm->time);
> -
> -       ret = regmap_read(rtc->regmap, rtc->base + SPRD_RTC_INT_EN, &val);
> -       if (ret)
> -               return ret;
> -
> -       alrm->enabled = !!(val & SPRD_RTC_AUXALM_EN);
> -
> -       ret = regmap_read(rtc->regmap, rtc->base + SPRD_RTC_INT_RAW_STS, &val);
> -       if (ret)
> -               return ret;
> -
> -       alrm->pending = !!(val & SPRD_RTC_AUXALM_EN);
> -       return 0;
> -}
> -
>  static int sprd_rtc_set_aux_alarm(struct device *dev, struct rtc_wkalrm *alrm)
>  {
>         struct sprd_rtc *rtc = dev_get_drvdata(dev);
> @@ -415,16 +388,9 @@ static int sprd_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
>         u32 val;
>
>         /*
> -        * Before RTC device is registered, it will check to see if there is an
> -        * alarm already set in RTC hardware, and we always read the normal
> -        * alarm at this time.
> -        *
> -        * Or if aie_timer is enabled, we should get the normal alarm time.
> -        * Otherwise we should get auxiliary alarm time.
> +        * The RTC core checks to see if there is an alarm already set in RTC
> +        * hardware, and we always read the normal alarm at this time.
>          */
> -       if (rtc->rtc && rtc->rtc->registered && rtc->rtc->aie_timer.enabled == 0)
> -               return sprd_rtc_read_aux_alarm(dev, alrm);
> -
>         ret = sprd_rtc_get_secs(rtc, SPRD_RTC_ALARM, &secs);
>         if (ret)
>                 return ret;
> --
> 2.28.0
>