[PATCH] rtc: sc27xx: Always read normal alarm

[PATCH] rtc: sc27xx: Always read normal alarm

[Alexandre Belloni]

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
to false.

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 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

Re: [PATCH] rtc: sc27xx: Always read normal alarm

[Chunyan Zhang]

Hi Alexandre,

Thanks for this cleanup.

On Wed, 18 Nov 2020 at 05:22, Alexandre Belloni
<alexandre.belloni@bootlin.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@bootlin.com>

Reviewed-by: Chunyan Zhang <zhang.lyra@gmail.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
>